From 8254035f01a5fcd2ed47172ef72eee16864fb460 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:28:41 -0400 Subject: [PATCH 1/5] Fix case sensitive user attribute map breaking Vault logins --- ldap/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldap/client.go b/ldap/client.go index b57ebb8..e559c46 100644 --- a/ldap/client.go +++ b/ldap/client.go @@ -273,7 +273,7 @@ func (c *Client) Authenticate(ctx context.Context, username, password string, op return nil, fmt.Errorf("%s: failed to get user attributes: %w", op, err) } for _, a := range attrs { - userAttrs[a.Name] = a.Vals + userAttrs[strings.ToLower(a.Name)] = a.Vals } } if !opts.withGroups && !c.conf.IncludeUserGroups { From d1fc2bd6f6f759cef6b2ad35289e520d3a966a6e Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:19:57 -0400 Subject: [PATCH 2/5] Add lower case user attribute keys configs --- ldap/client.go | 6 +++++- ldap/config.go | 5 +++++ ldap/options.go | 31 ++++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/ldap/client.go b/ldap/client.go index e559c46..d2211f4 100644 --- a/ldap/client.go +++ b/ldap/client.go @@ -273,7 +273,11 @@ func (c *Client) Authenticate(ctx context.Context, username, password string, op return nil, fmt.Errorf("%s: failed to get user attributes: %w", op, err) } for _, a := range attrs { - userAttrs[strings.ToLower(a.Name)] = a.Vals + name := a.Name + if c.conf.LowerUserAttributeKeys || opts.withLowerUserAttributeKeys { + name = strings.ToLower(a.Name) + } + userAttrs[name] = a.Vals } } if !opts.withGroups && !c.conf.IncludeUserGroups { diff --git a/ldap/config.go b/ldap/config.go index 97bddca..b09f830 100644 --- a/ldap/config.go +++ b/ldap/config.go @@ -200,6 +200,11 @@ type ClientConfig struct { // AD (unicodePwd) will always be excluded. ExcludedUserAttributes []string + // LowerUserAttributeKeys optionally specifies that the authenticating user's + // DN and attributes be included in AuthResult use lowercase key names rather + // than the default camel case. + LowerUserAttributeKeys bool + // IncludeUserGroups optionally specifies that the authenticating user's // group membership be included an authentication AuthResult. IncludeUserGroups bool diff --git a/ldap/options.go b/ldap/options.go index 7a2a79d..6ea4619 100644 --- a/ldap/options.go +++ b/ldap/options.go @@ -8,15 +8,16 @@ package ldap type Option func(interface{}) type configOptions struct { - withURLs []string - withInsecureTLS bool - withTLSMinVersion string - withTLSMaxVersion string - withCertificates []string - withClientTLSCert string - withClientTLSKey string - withGroups bool - withUserAttributes bool + withURLs []string + withInsecureTLS bool + withTLSMinVersion string + withTLSMaxVersion string + withCertificates []string + withClientTLSCert string + withClientTLSKey string + withGroups bool + withUserAttributes bool + withLowerUserAttributeKeys bool } func configDefaults() configOptions { @@ -75,6 +76,18 @@ func WithUserAttributes() Option { } } +// WithLowerUserAttributeKeys returns a User Attribute map where the keys +// are all cast to lower case. This is neccessary for some clients, such as Vault, +// where user configured user attribute key names have always been stored lower case. +func WithLowerUserAttributeKeys() Option { + return func(o interface{}) { + switch v := o.(type) { + case *configOptions: + v.withLowerUserAttributeKeys = true + } + } +} + func withTLSMinVersion(version string) Option { return func(o interface{}) { switch v := o.(type) { From ee345958dc942dfc894e9c4b8a07f30ea8752ac2 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:33:15 -0400 Subject: [PATCH 3/5] add tests --- ldap/client_exported_test.go | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/ldap/client_exported_test.go b/ldap/client_exported_test.go index 8304f73..b8b5d38 100644 --- a/ldap/client_exported_test.go +++ b/ldap/client_exported_test.go @@ -182,6 +182,49 @@ func TestClient_Authenticate(t *testing.T) { }, wantUserDN: "cn=alice,ou=people,dc=example,dc=org", }, + { + name: "success-with-user-attributes-lower-case-keys", + username: "alice", + password: "password", + clientConfig: &ldap.ClientConfig{ + URLs: []string{fmt.Sprintf("ldaps://127.0.0.1:%d", td.Port())}, + Certificates: []string{td.Cert()}, + DiscoverDN: true, + UserDN: testdirectory.DefaultUserDN, + GroupDN: testdirectory.DefaultGroupDN, + ExcludedUserAttributes: []string{"password", "memberof"}, + IncludeUserAttributes: true, + LowerUserAttributeKeys: true, + }, + opts: []ldap.Option{ldap.WithUserAttributes()}, + wantUserAttributes: map[string][]string{ + "email": {"alice@example.com"}, + "name": {"alice"}, + "tokengroups": {"\x01\x00\x00\x00\x00\x00\x00\x01"}, + }, + wantUserDN: "cn=alice,ou=people,dc=example,dc=org", + }, + { + name: "success-with-user-attributes-lower-case-keys-opt", + username: "alice", + password: "password", + clientConfig: &ldap.ClientConfig{ + URLs: []string{fmt.Sprintf("ldaps://127.0.0.1:%d", td.Port())}, + Certificates: []string{td.Cert()}, + DiscoverDN: true, + UserDN: testdirectory.DefaultUserDN, + GroupDN: testdirectory.DefaultGroupDN, + ExcludedUserAttributes: []string{"password", "memberof"}, + IncludeUserAttributes: true, + }, + opts: []ldap.Option{ldap.WithUserAttributes(), ldap.WithLowerUserAttributeKeys()}, + wantUserAttributes: map[string][]string{ + "email": {"alice@example.com"}, + "name": {"alice"}, + "tokengroups": {"\x01\x00\x00\x00\x00\x00\x00\x01"}, + }, + wantUserDN: "cn=alice,ou=people,dc=example,dc=org", + }, { name: "success-include-user-groups", username: "alice", From 93ce396c12ce6976e335c481a4e0dffc319ec0d1 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:33:50 -0400 Subject: [PATCH 4/5] Update ldap/options.go Co-authored-by: Jim --- ldap/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldap/options.go b/ldap/options.go index 6ea4619..fec5da9 100644 --- a/ldap/options.go +++ b/ldap/options.go @@ -77,7 +77,7 @@ func WithUserAttributes() Option { } // WithLowerUserAttributeKeys returns a User Attribute map where the keys -// are all cast to lower case. This is neccessary for some clients, such as Vault, +// are all cast to lower case. This is necessary for some clients, such as Vault, // where user configured user attribute key names have always been stored lower case. func WithLowerUserAttributeKeys() Option { return func(o interface{}) { From a19ed947b9f08e3dc2f5e2403891caf687ce21c3 Mon Sep 17 00:00:00 2001 From: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:36:16 -0400 Subject: [PATCH 5/5] Add godoc --- ldap/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldap/client.go b/ldap/client.go index d2211f4..715a7de 100644 --- a/ldap/client.go +++ b/ldap/client.go @@ -215,7 +215,7 @@ type Attribute struct { // the WithGroups option is specified, it will also return the user's groups // from the directory. // -// Supported options: WithUserAttributes, WithGroups, WithDialer, WithURLs +// Supported options: WithUserAttributes, WithGroups, WithDialer, WithURLs, WithLowerUserAttributeKeys func (c *Client) Authenticate(ctx context.Context, username, password string, opt ...Option) (*AuthResult, error) { const op = "ldap.(Client).Authenticate" if username == "" {