From fb6c833eb52cefad92839ec46fd6f1d66579675f Mon Sep 17 00:00:00 2001 From: Amir Aslamov Date: Fri, 14 Mar 2025 12:14:33 -0400 Subject: [PATCH] =?UTF-8?q?check=20for=20case=20sensitivity=20at=20delete?= =?UTF-8?q?=20for=20user=20and=20group=20paths,=20modify=E2=80=A6=20(#2992?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * check for case sensitivity at delete for user and group paths, modify tests to cover proper deletions * add changelog --- builtin/credential/ldap/backend_test.go | 87 ++++++++++++++++++------- builtin/credential/ldap/path_groups.go | 15 ++++- builtin/credential/ldap/path_users.go | 15 ++++- changelog/29922.txt | 3 + 4 files changed, 96 insertions(+), 24 deletions(-) create mode 100644 changelog/29922.txt diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index b7f715d89d..9e8ef6675a 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -155,28 +155,6 @@ func TestLdapAuthBackend_CaseSensitivity(t *testing.T) { ctx := context.Background() testVals := func(caseSensitive bool) { - // Clear storage - userList, err := storage.List(ctx, "user/") - if err != nil { - t.Fatal(err) - } - for _, user := range userList { - err = storage.Delete(ctx, "user/"+user) - if err != nil { - t.Fatal(err) - } - } - groupList, err := storage.List(ctx, "group/") - if err != nil { - t.Fatal(err) - } - for _, group := range groupList { - err = storage.Delete(ctx, "group/"+group) - if err != nil { - t.Fatal(err) - } - } - configReq := &logical.Request{ Path: "config", Operation: logical.ReadOperation, @@ -284,6 +262,71 @@ func TestLdapAuthBackend_CaseSensitivity(t *testing.T) { if !reflect.DeepEqual(expected, resp.Auth.Policies) { t.Fatalf("bad: policies: expected: %q, actual: %q", expected, resp.Auth.Policies) } + + // Test proper deletion of users + userReqDel := &logical.Request{ + Operation: logical.DeleteOperation, + Data: map[string]interface{}{ + "groups": "EngineerS", + "policies": "userpolicy", + }, + Path: "users/hermeS conRad", + Storage: storage, + } + resp, err = b.HandleRequest(ctx, userReqDel) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + if caseSensitive { + // The online test server is actually case sensitive so we need to + // delete again so it works + userReq = &logical.Request{ + Operation: logical.DeleteOperation, + Data: map[string]interface{}{ + "groups": "EngineerS", + "policies": "userpolicy", + }, + Path: "users/Hermes Conrad", + Storage: storage, + Connection: &logical.Connection{}, + } + resp, err = b.HandleRequest(ctx, userReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + } + + // Expect storage for user path to be cleared + userList, err := storage.List(ctx, "user/") + if err != nil { + t.Fatal(err) + } + if userList != nil { + t.Fatalf("deletion of users failed") + } + + // Test proper deletion of groups + groupReqDel := &logical.Request{ + Operation: logical.DeleteOperation, + Data: map[string]interface{}{ + "policies": "grouppolicy", + }, + Path: "groups/EngineerS", + Storage: storage, + } + resp, err = b.HandleRequest(ctx, groupReqDel) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + + // Expect storage for group path to be cleared + groupList, err := storage.List(ctx, "group/") + if err != nil { + t.Fatal(err) + } + if groupList != nil { + t.Fatalf("deletion of groups failed") + } } cleanup, cfg := ldap.PrepareTestContainer(t, "master") diff --git a/builtin/credential/ldap/path_groups.go b/builtin/credential/ldap/path_groups.go index 645b6428fd..aa2500731a 100644 --- a/builtin/credential/ldap/path_groups.go +++ b/builtin/credential/ldap/path_groups.go @@ -87,7 +87,20 @@ func (b *backend) Group(ctx context.Context, s logical.Storage, n string) (*Grou } func (b *backend) pathGroupDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - err := req.Storage.Delete(ctx, "group/"+d.Get("name").(string)) + groupname := d.Get("name").(string) + + cfg, err := b.Config(ctx, req) + if err != nil { + return nil, err + } + if cfg == nil { + return logical.ErrorResponse("ldap backend not configured"), nil + } + if !*cfg.CaseSensitiveNames { + groupname = strings.ToLower(groupname) + } + + err = req.Storage.Delete(ctx, "group/"+groupname) if err != nil { return nil, err } diff --git a/builtin/credential/ldap/path_users.go b/builtin/credential/ldap/path_users.go index 55326f6408..07cc70b359 100644 --- a/builtin/credential/ldap/path_users.go +++ b/builtin/credential/ldap/path_users.go @@ -96,7 +96,20 @@ func (b *backend) User(ctx context.Context, s logical.Storage, n string) (*UserE } func (b *backend) pathUserDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - err := req.Storage.Delete(ctx, "user/"+d.Get("name").(string)) + username := d.Get("name").(string) + + cfg, err := b.Config(ctx, req) + if err != nil { + return nil, err + } + if cfg == nil { + return logical.ErrorResponse("ldap backend not configured"), nil + } + if !*cfg.CaseSensitiveNames { + username = strings.ToLower(username) + } + + err = req.Storage.Delete(ctx, "user/"+username) if err != nil { return nil, err } diff --git a/changelog/29922.txt b/changelog/29922.txt new file mode 100644 index 0000000000..e1997dd36c --- /dev/null +++ b/changelog/29922.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/ldap: Fix a bug that does not properly delete users and groups by first converting their names to lowercase when case senstivity option is off. +``` \ No newline at end of file