From 86f2bea8c58d25c220deb5a692398ea1660c3aaf Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Tue, 14 Dec 2021 15:55:35 -0500 Subject: [PATCH 1/3] pinniped CLI: allow all forms of http redirects For password based login on the CLI (i.e. no browser), this change relaxes the response code check to allow for any redirect code handled by the Go standard library. In the future, we can drop the rewriteStatusSeeOtherToStatusFoundForBrowserless logic from the server side code. Signed-off-by: Monis Khan --- .../oidc/callback/callback_handler_test.go | 2 +- pkg/oidcclient/login.go | 6 +- pkg/oidcclient/login_test.go | 113 +++++++++++++++++- 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 79c4afb3..48e04afd 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -174,7 +174,7 @@ func TestCallbackEndpoint(t *testing.T) { }, }, { - name: "GET with good state and cookie and successful upstream token exchange returns 302 to downstream client callback with its state and code", + name: "GET with good state and cookie and successful upstream token exchange returns 303 to downstream client callback with its state and code", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().Build()), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 2046d047..470ed3dd 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -435,7 +435,9 @@ func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) ( authorizeURL := h.oauth2Config.AuthCodeURL(h.state.String(), *authorizeOptions...) // Don't follow redirects automatically because we want to handle redirects here. + var sawRedirect bool h.httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + sawRedirect = true return http.ErrUseLastResponse } @@ -454,8 +456,8 @@ func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) ( } _ = authRes.Body.Close() // don't need the response body, and okay if it fails to close - // A successful authorization always results in a 302. - if authRes.StatusCode != http.StatusFound { + // A successful authorization always results in a redirect (we are flexible on the exact status code). + if !sawRedirect { return nil, fmt.Errorf( "error getting authorization: expected to be redirected, but response status was %s", authRes.Status) } diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index 8929c5ef..2bf4620c 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -900,7 +900,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo `/authorize?access_type=offline&client_id=test-client-id&code_challenge=VVaezYqum7reIhoavCHD1n2d-piN3r_mywoYj7fCR7g&code_challenge_method=S256&nonce=test-nonce&pinniped_idp_name=some-upstream-name&pinniped_idp_type=ldap&redirect_uri=http%3A%2F%2F127.0.0.1%3A0%2Fcallback&response_type=code&scope=test-scope&state=test-state": some error fetching authorize endpoint`, }, { - name: "ldap login when the OIDC provider authorization endpoint returns something other than a 302 redirect", + name: "ldap login when the OIDC provider authorization endpoint returns something other than a redirect", clientID: "test-client-id", opt: func(t *testing.T) Option { return func(h *handlerState) error { @@ -1215,6 +1215,117 @@ func TestLogin(t *testing.T) { // nolint:gocyclo }, wantToken: &testToken, }, + { + name: "successful ldap login with env vars for username and password, http.StatusSeeOther redirect", + clientID: "test-client-id", + opt: func(t *testing.T) Option { + return func(h *handlerState) error { + fakeAuthCode := "test-authcode-value" + + h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI { + mock := mockUpstream(t) + mock.EXPECT(). + ExchangeAuthcodeAndValidateTokens( + gomock.Any(), fakeAuthCode, pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), "http://127.0.0.1:0/callback"). + Return(&testToken, nil) + return mock + } + + h.generateState = func() (state.State, error) { return "test-state", nil } + h.generatePKCE = func() (pkce.Code, error) { return "test-pkce", nil } + h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil } + h.getEnv = func(key string) string { + switch key { + case "PINNIPED_USERNAME": + return "some-upstream-username" + case "PINNIPED_PASSWORD": + return "some-upstream-password" + default: + return "" // all other env vars are treated as if they are unset + } + } + h.promptForValue = func(_ context.Context, promptLabel string) (string, error) { + require.FailNow(t, fmt.Sprintf("saw unexpected prompt from the CLI: %q", promptLabel)) + return "", nil + } + h.promptForSecret = func(promptLabel string) (string, error) { + require.FailNow(t, fmt.Sprintf("saw unexpected prompt from the CLI: %q", promptLabel)) + return "", nil + } + + cache := &mockSessionCache{t: t, getReturnsToken: nil} + cacheKey := SessionCacheKey{ + Issuer: successServer.URL, + ClientID: "test-client-id", + Scopes: []string{"test-scope"}, + RedirectURI: "http://localhost:0/callback", + } + t.Cleanup(func() { + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawGetKeys) + require.Equal(t, []SessionCacheKey{cacheKey}, cache.sawPutKeys) + require.Equal(t, []*oidctypes.Token{&testToken}, cache.sawPutTokens) + }) + require.NoError(t, WithSessionCache(cache)(h)) + require.NoError(t, WithCLISendingCredentials()(h)) + require.NoError(t, WithUpstreamIdentityProvider("some-upstream-name", "ldap")(h)) + + discoveryRequestWasMade := false + authorizeRequestWasMade := false + t.Cleanup(func() { + require.True(t, discoveryRequestWasMade, "should have made an discovery request") + require.True(t, authorizeRequestWasMade, "should have made an authorize request") + }) + + require.NoError(t, WithClient(&http.Client{ + Transport: roundtripper.Func(func(req *http.Request) (*http.Response, error) { + switch req.URL.Scheme + "://" + req.URL.Host + req.URL.Path { + case "http://" + successServer.Listener.Addr().String() + "/.well-known/openid-configuration": + discoveryRequestWasMade = true + return defaultDiscoveryResponse(req) + case "http://" + successServer.Listener.Addr().String() + "/authorize": + authorizeRequestWasMade = true + require.Equal(t, "some-upstream-username", req.Header.Get("Pinniped-Username")) + require.Equal(t, "some-upstream-password", req.Header.Get("Pinniped-Password")) + require.Equal(t, url.Values{ + // This is the PKCE challenge which is calculated as base64(sha256("test-pkce")). For example: + // $ echo -n test-pkce | shasum -a 256 | cut -d" " -f1 | xxd -r -p | base64 | cut -d"=" -f1 + // VVaezYqum7reIhoavCHD1n2d+piN3r/mywoYj7fCR7g + "code_challenge": []string{"VVaezYqum7reIhoavCHD1n2d-piN3r_mywoYj7fCR7g"}, + "code_challenge_method": []string{"S256"}, + "response_type": []string{"code"}, + "scope": []string{"test-scope"}, + "nonce": []string{"test-nonce"}, + "state": []string{"test-state"}, + "access_type": []string{"offline"}, + "client_id": []string{"test-client-id"}, + "redirect_uri": []string{"http://127.0.0.1:0/callback"}, + "pinniped_idp_name": []string{"some-upstream-name"}, + "pinniped_idp_type": []string{"ldap"}, + }, req.URL.Query()) + return &http.Response{ + StatusCode: http.StatusSeeOther, + Header: http.Header{"Location": []string{ + fmt.Sprintf("http://127.0.0.1:0/callback?code=%s&state=test-state", fakeAuthCode), + }}, + }, nil + default: + // Note that "/token" requests should not be made. They are mocked by mocking calls to ExchangeAuthcodeAndValidateTokens(). + require.FailNow(t, fmt.Sprintf("saw unexpected http call from the CLI: %s", req.URL.String())) + return nil, nil + } + }), + })(h)) + return nil + } + }, + issuer: successServer.URL, + wantLogs: []string{ + "\"level\"=4 \"msg\"=\"Pinniped: Performing OIDC discovery\" \"issuer\"=\"" + successServer.URL + "\"", + "\"level\"=4 \"msg\"=\"Pinniped: Read username from environment variable\" \"name\"=\"PINNIPED_USERNAME\"", + "\"level\"=4 \"msg\"=\"Pinniped: Read password from environment variable\" \"name\"=\"PINNIPED_PASSWORD\"", + }, + wantToken: &testToken, + }, { name: "with requested audience, session cache hit with valid token, but discovery fails", clientID: "test-client-id", From a6085c96784ae6ad973131dd9f3bfacfc8468701 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 15 Dec 2021 09:39:46 -0500 Subject: [PATCH 2/3] Drop unsafe unwrapper for exec.roundTripper exec.roundTripper now implements utilnet.RoundTripperWrapper so this unsafe hack is no longer needed. Signed-off-by: Monis Khan --- internal/kubeclient/kubeclient.go | 33 ++------------------------ internal/kubeclient/kubeclient_test.go | 3 ++- 2 files changed, 4 insertions(+), 32 deletions(-) diff --git a/internal/kubeclient/kubeclient.go b/internal/kubeclient/kubeclient.go index c9b3106f..710dbb54 100644 --- a/internal/kubeclient/kubeclient.go +++ b/internal/kubeclient/kubeclient.go @@ -8,8 +8,6 @@ import ( "crypto/x509" "fmt" "net/http" - "reflect" - "unsafe" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -155,7 +153,7 @@ func createSecureKubeConfig(kubeConfig *restclient.Config) (*restclient.Config, } }() - tlsConfig, err := netTLSClientConfig(rt) + tlsConfig, err := net.TLSClientConfig(rt) if err != nil { // this assumes none of our production code calls Wrap or messes with WrapTransport. // this is a reasonable assumption because all such code should live in this package @@ -205,7 +203,7 @@ func AssertSecureConfig(kubeConfig *restclient.Config) error { } func AssertSecureTransport(rt http.RoundTripper) error { - tlsConfig, err := netTLSClientConfig(rt) + tlsConfig, err := net.TLSClientConfig(rt) if err != nil { return fmt.Errorf("failed to get TLS config: %w", err) } @@ -224,33 +222,6 @@ func AssertSecureTransport(rt http.RoundTripper) error { return nil } -func netTLSClientConfig(rt http.RoundTripper) (*tls.Config, error) { - tlsConfig, err := net.TLSClientConfig(rt) - if err == nil { - return tlsConfig, nil - } - - // TODO fix when we pick up https://github.com/kubernetes/kubernetes/pull/106014 - if err.Error() == "unknown transport type: *exec.roundTripper" { - return net.TLSClientConfig(extractRTUnsafe(rt)) - } - - return nil, err -} - -func extractRTUnsafe(rt http.RoundTripper) (out http.RoundTripper) { - for wrapper, ok := rt.(net.RoundTripperWrapper); ok; wrapper, ok = rt.(net.RoundTripperWrapper) { - // keep peeling the wrappers until we get to the exec.roundTripper - rt = wrapper.WrappedRoundTripper() - } - - // this is some dark magic to read a private field - baseField := reflect.ValueOf(rt).Elem().FieldByName("base") - basePointer := (*http.RoundTripper)(unsafe.Pointer(baseField.UnsafeAddr())) - - return *basePointer -} - func Secure(config *restclient.Config) (kubernetes.Interface, *restclient.Config, error) { // our middleware does not apply to the returned restclient.Config, therefore, this // client not having a leader election lock is irrelevant since it would not be enforced diff --git a/internal/kubeclient/kubeclient_test.go b/internal/kubeclient/kubeclient_test.go index a90e3bae..1fe6db38 100644 --- a/internal/kubeclient/kubeclient_test.go +++ b/internal/kubeclient/kubeclient_test.go @@ -19,6 +19,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/net" clientauthenticationv1 "k8s.io/client-go/pkg/apis/clientauthentication/v1" "k8s.io/client-go/rest" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -1109,7 +1110,7 @@ func testUnwrap(t *testing.T, client *Client, serverSubjects [][]byte) { t.Run(tt.name, func(t *testing.T) { t.Parallel() // make sure to run in parallel to confirm that our client-go TLS cache busting works (i.e. assert no data races) - tlsConfig, err := netTLSClientConfig(tt.rt) + tlsConfig, err := net.TLSClientConfig(tt.rt) require.NoError(t, err) require.NotNil(t, tlsConfig) From c155c6e629554bbe6f18a798c1375a9758d828da Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 15 Dec 2021 10:30:36 -0500 Subject: [PATCH 3/3] Clean up nits in AD code - Make everything private - Drop unused AuthTime field - Use %q format string instead of "%s" - Only rely on GetRawAttributeValues in AttributeUnchangedSinceLogin Signed-off-by: Monis Khan --- .../active_directory_upstream_watcher.go | 44 +++++---- .../active_directory_upstream_watcher_test.go | 92 +++++++++---------- .../provider/dynamic_upstream_idp_provider.go | 2 - internal/upstreamldap/upstreamldap.go | 56 +++++------ internal/upstreamldap/upstreamldap_test.go | 10 +- test/integration/ldap_client_test.go | 6 +- 6 files changed, 106 insertions(+), 104 deletions(-) diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 633fb438..c96d20a4 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -58,15 +58,15 @@ const ( defaultActiveDirectoryGroupSearchFilter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))" sAMAccountNameAttribute = "sAMAccountName" - // PwdLastSetAttribute is the date and time that the password for this account was last changed. + // pwdLastSetAttribute is the date and time that the password for this account was last changed. // https://docs.microsoft.com/en-us/windows/win32/adschema/a-pwdlastset - PwdLastSetAttribute = "pwdLastSet" - // UserAccountControlAttribute represents a bitmap of user properties. + pwdLastSetAttribute = "pwdLastSet" + // userAccountControlAttribute represents a bitmap of user properties. // https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties - UserAccountControlAttribute = "userAccountControl" - // UserAccountControlComputedAttribute represents a bitmap of user properties. + userAccountControlAttribute = "userAccountControl" + // userAccountControlComputedAttribute represents a bitmap of user properties. // https://docs.microsoft.com/en-us/windows/win32/adschema/a-msds-user-account-control-computed - UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" + userAccountControlComputedAttribute = "msDS-User-Account-Control-Computed" // 0x0002 ACCOUNTDISABLE in userAccountControl bitmap. accountDisabledBitmapValue = 2 // 0x0010 UF_LOCKOUT in msDS-User-Account-Control-Computed bitmap. @@ -334,17 +334,21 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context, Filter: adUpstreamImpl.Spec().GroupSearch().Filter(), GroupNameAttribute: adUpstreamImpl.Spec().GroupSearch().GroupNameAttribute(), }, - Dialer: c.ldapDialer, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + Dialer: c.ldapDialer, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){ + "objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"), + }, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ - PwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(PwdLastSetAttribute), - UserAccountControlAttribute: ValidUserAccountControl, - UserAccountControlComputedAttribute: ValidComputedUserAccountControl, + pwdLastSetAttribute: upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute), + userAccountControlAttribute: validUserAccountControl, + userAccountControlComputedAttribute: validComputedUserAccountControl, }, } if spec.GroupSearch.Attributes.GroupName == "" { - config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){defaultActiveDirectoryGroupNameAttributeName: GroupSAMAccountNameWithDomainSuffix} + config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){ + defaultActiveDirectoryGroupNameAttributeName: groupSAMAccountNameWithDomainSuffix, + } } conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config) @@ -378,7 +382,7 @@ func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, ups } } -func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) { +func microsoftUUIDFromBinaryAttr(attributeName string) func(entry *ldap.Entry) (string, error) { // validation has already been done so we can just get the attribute... return func(entry *ldap.Entry) (string, error) { binaryUUID := entry.GetRawAttributeValue(attributeName) @@ -399,18 +403,18 @@ func microsoftUUIDFromBinary(binaryUUID []byte) (string, error) { return uuidVal.String(), nil } -func GroupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) { +func groupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) { sAMAccountNameAttributeValues := entry.GetAttributeValues(sAMAccountNameAttribute) if len(sAMAccountNameAttributeValues) != 1 { - return "", fmt.Errorf(`found %d values for attribute "%s", but expected 1 result`, + return "", fmt.Errorf(`found %d values for attribute %q, but expected 1 result`, len(sAMAccountNameAttributeValues), sAMAccountNameAttribute, ) } sAMAccountName := sAMAccountNameAttributeValues[0] if len(sAMAccountName) == 0 { - return "", fmt.Errorf(`found empty value for attribute "%s", but expected value to be non-empty`, + return "", fmt.Errorf(`found empty value for attribute %q, but expected value to be non-empty`, sAMAccountNameAttribute, ) } @@ -433,8 +437,8 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) { return strings.Join(domainComponents[1:], "."), nil } -func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { - userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlAttribute)) +func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute)) if err != nil { return err } @@ -446,8 +450,8 @@ func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttribut return nil } -func ValidComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { - userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlComputedAttribute)) +func validComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error { + userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute)) if err != nil { return err } diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index 5c47a5b8..76a35ca2 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go @@ -220,11 +220,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, } @@ -541,11 +541,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -602,11 +602,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: "sAMAccountName", }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -666,11 +666,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -730,11 +730,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -793,11 +793,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -927,11 +927,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1056,11 +1056,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }}, }, wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{ @@ -1111,11 +1111,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1315,12 +1315,12 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))", GroupNameAttribute: "sAMAccountName", }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, - GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": GroupSAMAccountNameWithDomainSuffix}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, + GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1373,11 +1373,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1434,11 +1434,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1489,11 +1489,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1690,11 +1690,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) { Filter: testGroupSearchFilter, GroupNameAttribute: testGroupNameAttrName, }, - UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")}, + UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")}, RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{ "pwdLastSet": upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"), - "userAccountControl": ValidUserAccountControl, - "msDS-User-Account-Control-Computed": ValidComputedUserAccountControl, + "userAccountControl": validUserAccountControl, + "msDS-User-Account-Control-Computed": validComputedUserAccountControl, }, }, }, @@ -1931,7 +1931,7 @@ func TestGroupSAMAccountNameWithDomainSuffix(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - suffixedSAMAccountName, err := GroupSAMAccountNameWithDomainSuffix(tt.entry) + suffixedSAMAccountName, err := groupSAMAccountNameWithDomainSuffix(tt.entry) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) } else { @@ -2074,7 +2074,7 @@ func TestValidUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := ValidUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) @@ -2135,7 +2135,7 @@ func TestValidComputedUserAccountControl(t *testing.T) { for _, test := range tests { tt := test t.Run(tt.name, func(t *testing.T) { - err := ValidComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) + err := validComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{}) if tt.wantErr != "" { require.Error(t, err) diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 0770e2c3..2c34dcbe 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -7,7 +7,6 @@ import ( "context" "net/url" "sync" - "time" "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/types" @@ -101,7 +100,6 @@ type StoredRefreshAttributes struct { Username string Subject string DN string - AuthTime time.Time AdditionalAttributes map[string]string } diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 594aac1f..0a7e37a2 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -184,14 +184,14 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p // if any more or less than one entry, error. // we don't need to worry about logging this because we know it's a dn. if len(searchResult.Entries) != 1 { - return fmt.Errorf(`searching for user "%s" resulted in %d search results, but expected 1 result`, + return fmt.Errorf(`searching for user %q resulted in %d search results, but expected 1 result`, userDN, len(searchResult.Entries), ) } userEntry := searchResult.Entries[0] if len(userEntry.DN) == 0 { - return fmt.Errorf(`searching for user with original DN "%s" resulted in search result without DN`, userDN) + return fmt.Errorf(`searching for user with original DN %q resulted in search result without DN`, userDN) } newUsername, err := p.getSearchResultAttributeValue(p.c.UserSearch.UsernameAttribute, userEntry, userDN) @@ -199,7 +199,7 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p return err } if newUsername != storedRefreshAttributes.Username { - return fmt.Errorf(`searching for user "%s" returned a different username than the previous value. expected: "%s", actual: "%s"`, + return fmt.Errorf(`searching for user %q returned a different username than the previous value. expected: %q, actual: %q`, userDN, storedRefreshAttributes.Username, newUsername, ) } @@ -210,12 +210,12 @@ func (p *Provider) PerformRefresh(ctx context.Context, storedRefreshAttributes p } newSubject := downstreamsession.DownstreamLDAPSubject(newUID, *p.GetURL()) if newSubject != storedRefreshAttributes.Subject { - return fmt.Errorf(`searching for user "%s" produced a different subject than the previous value. expected: "%s", actual: "%s"`, userDN, storedRefreshAttributes.Subject, newSubject) + return fmt.Errorf(`searching for user %q produced a different subject than the previous value. expected: %q, actual: %q`, userDN, storedRefreshAttributes.Subject, newSubject) } for attribute, validateFunc := range p.c.RefreshAttributeChecks { err = validateFunc(userEntry, storedRefreshAttributes) if err != nil { - return fmt.Errorf(`validation for attribute "%s" failed during upstream refresh: %w`, attribute, err) + return fmt.Errorf(`validation for attribute %q failed during upstream refresh: %w`, attribute, err) } } // we checked that the user still exists and their information is the same, so just return. @@ -227,19 +227,19 @@ func (p *Provider) performRefresh(ctx context.Context, userDN string) (*ldap.Sea conn, err := p.dial(ctx) if err != nil { - return nil, fmt.Errorf(`error dialing host "%s": %w`, p.c.Host, err) + return nil, fmt.Errorf(`error dialing host %q: %w`, p.c.Host, err) } defer conn.Close() err = conn.Bind(p.c.BindUsername, p.c.BindPassword) if err != nil { - return nil, fmt.Errorf(`error binding as "%s" before user search: %w`, p.c.BindUsername, err) + return nil, fmt.Errorf(`error binding as %q before user search: %w`, p.c.BindUsername, err) } searchResult, err := conn.Search(search) if err != nil { - return nil, fmt.Errorf(`error searching for user "%s": %w`, userDN, err) + return nil, fmt.Errorf(`error searching for user %q: %w`, userDN, err) } return searchResult, nil } @@ -369,13 +369,13 @@ func (p *Provider) TestConnection(ctx context.Context) error { conn, err := p.dial(ctx) if err != nil { - return fmt.Errorf(`error dialing host "%s": %w`, p.c.Host, err) + return fmt.Errorf(`error dialing host %q: %w`, p.c.Host, err) } defer conn.Close() err = conn.Bind(p.c.BindUsername, p.c.BindPassword) if err != nil { - return fmt.Errorf(`error binding as "%s": %w`, p.c.BindUsername, err) + return fmt.Errorf(`error binding as %q: %w`, p.c.BindUsername, err) } return nil @@ -420,14 +420,14 @@ func (p *Provider) authenticateUserImpl(ctx context.Context, username string, bi conn, err := p.dial(ctx) if err != nil { p.traceAuthFailure(t, err) - return nil, false, fmt.Errorf(`error dialing host "%s": %w`, p.c.Host, err) + return nil, false, fmt.Errorf(`error dialing host %q: %w`, p.c.Host, err) } defer conn.Close() err = conn.Bind(p.c.BindUsername, p.c.BindPassword) if err != nil { p.traceAuthFailure(t, err) - return nil, false, fmt.Errorf(`error binding as "%s" before user search: %w`, p.c.BindUsername, err) + return nil, false, fmt.Errorf(`error binding as %q before user search: %w`, p.c.BindUsername, err) } response, err := p.searchAndBindUser(conn, username, bindFunc) @@ -455,7 +455,7 @@ func (p *Provider) searchGroupsForUserDN(conn Conn, userDN string) ([]string, er groupAttributeName = distinguishedNameAttributeName } - groups := []string{} + var groups []string entries: for _, groupEntry := range searchResult.Entries { if len(groupEntry.DN) == 0 { @@ -495,14 +495,14 @@ func (p *Provider) SearchForDefaultNamingContext(ctx context.Context) (string, e conn, err := p.dial(ctx) if err != nil { p.traceSearchBaseDiscoveryFailure(t, err) - return "", fmt.Errorf(`error dialing host "%s": %w`, p.c.Host, err) + return "", fmt.Errorf(`error dialing host %q: %w`, p.c.Host, err) } defer conn.Close() err = conn.Bind(p.c.BindUsername, p.c.BindPassword) if err != nil { p.traceSearchBaseDiscoveryFailure(t, err) - return "", fmt.Errorf(`error binding as "%s" before querying for defaultNamingContext: %w`, p.c.BindUsername, err) + return "", fmt.Errorf(`error binding as %q before querying for defaultNamingContext: %w`, p.c.BindUsername, err) } searchResult, err := conn.Search(p.defaultNamingContextRequest()) @@ -546,13 +546,13 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c // At this point, we have matched at least one entry, so we can be confident that the username is not actually // someone's password mistakenly entered into the username field, so we can log it without concern. if len(searchResult.Entries) > 1 { - return nil, fmt.Errorf(`searching for user "%s" resulted in %d search results, but expected 1 result`, + return nil, fmt.Errorf(`searching for user %q resulted in %d search results, but expected 1 result`, username, len(searchResult.Entries), ) } userEntry := searchResult.Entries[0] if len(userEntry.DN) == 0 { - return nil, fmt.Errorf(`searching for user "%s" resulted in search result without DN`, username) + return nil, fmt.Errorf(`searching for user %q resulted in search result without DN`, username) } mappedUsername, err := p.getSearchResultAttributeValue(p.c.UserSearch.UsernameAttribute, userEntry, username) @@ -567,7 +567,7 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c return nil, err } - mappedGroupNames := []string{} + var mappedGroupNames []string if len(p.c.GroupSearch.Base) > 0 { mappedGroupNames, err = p.searchGroupsForUserDN(conn, userEntry.DN) if err != nil { @@ -594,7 +594,7 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c if errors.As(err, &ldapErr) && ldapErr.ResultCode == ldap.LDAPResultInvalidCredentials { return nil, nil } - return nil, fmt.Errorf(`error binding for user "%s" using provided password against DN "%s": %w`, username, userEntry.DN, err) + return nil, fmt.Errorf(`error binding for user %q using provided password against DN %q: %w`, username, userEntry.DN, err) } if len(mappedUsername) == 0 || len(mappedUID) == 0 { @@ -675,7 +675,7 @@ func (p *Provider) refreshUserSearchRequest(dn string) *ldap.SearchRequest { } func (p *Provider) userSearchRequestedAttributes() []string { - attributes := []string{} + attributes := make([]string, 0, len(p.c.RefreshAttributeChecks)+2) if p.c.UserSearch.UsernameAttribute != distinguishedNameAttributeName { attributes = append(attributes, p.c.UserSearch.UsernameAttribute) } @@ -736,14 +736,14 @@ func (p *Provider) getSearchResultAttributeRawValueEncoded(attributeName string, attributeValues := entry.GetRawAttributeValues(attributeName) if len(attributeValues) != 1 { - return "", fmt.Errorf(`found %d values for attribute "%s" while searching for user "%s", but expected 1 result`, + return "", fmt.Errorf(`found %d values for attribute %q while searching for user %q, but expected 1 result`, len(attributeValues), attributeName, username, ) } attributeValue := attributeValues[0] if len(attributeValue) == 0 { - return "", fmt.Errorf(`found empty value for attribute "%s" while searching for user "%s", but expected value to be non-empty`, + return "", fmt.Errorf(`found empty value for attribute %q while searching for user %q, but expected value to be non-empty`, attributeName, username, ) } @@ -763,14 +763,14 @@ func (p *Provider) getSearchResultAttributeValue(attributeName string, entry *ld attributeValues := entry.GetAttributeValues(attributeName) if len(attributeValues) != 1 { - return "", fmt.Errorf(`found %d values for attribute "%s" while searching for user "%s", but expected 1 result`, + return "", fmt.Errorf(`found %d values for attribute %q while searching for user %q, but expected 1 result`, len(attributeValues), attributeName, username, ) } attributeValue := attributeValues[0] if len(attributeValue) == 0 { - return "", fmt.Errorf(`found empty value for attribute "%s" while searching for user "%s", but expected value to be non-empty`, + return "", fmt.Errorf(`found empty value for attribute %q while searching for user %q, but expected value to be non-empty`, attributeName, username, ) } @@ -805,14 +805,14 @@ func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) { func AttributeUnchangedSinceLogin(attribute string) func(*ldap.Entry, provider.StoredRefreshAttributes) error { return func(entry *ldap.Entry, storedAttributes provider.StoredRefreshAttributes) error { prevAttributeValue := storedAttributes.AdditionalAttributes[attribute] - newValues := entry.GetAttributeValues(attribute) + newValues := entry.GetRawAttributeValues(attribute) if len(newValues) != 1 { - return fmt.Errorf(`expected to find 1 value for "%s" attribute, but found %d`, attribute, len(newValues)) + return fmt.Errorf(`expected to find 1 value for %q attribute, but found %d`, attribute, len(newValues)) } - encodedNewValue := base64.RawURLEncoding.EncodeToString(entry.GetRawAttributeValue(attribute)) + encodedNewValue := base64.RawURLEncoding.EncodeToString(newValues[0]) if prevAttributeValue != encodedNewValue { - return fmt.Errorf(`value for attribute "%s" has changed since initial value at login`, attribute) + return fmt.Errorf(`value for attribute %q has changed since initial value at login`, attribute) } return nil } diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index f7c884fb..65a7629f 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -254,7 +254,7 @@ func TestEndUserAuthentication(t *testing.T) { }, wantAuthResponse: expectedAuthResponse(func(r *authenticators.Response) { info := r.User.(*user.DefaultInfo) - info.Groups = []string{} + info.Groups = nil }), }, { @@ -1410,8 +1410,8 @@ func TestUpstreamRefresh(t *testing.T) { ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, }, { - Name: pwdLastSetAttribute, - Values: []string{"132801740800000001"}, + Name: pwdLastSetAttribute, + ByteValues: [][]byte{[]byte("132801740800000001")}, }, }, }, @@ -1812,8 +1812,8 @@ func TestAttributeUnchangedSinceLogin(t *testing.T) { DN: "some-dn", Attributes: []*ldap.EntryAttribute{ { - Name: attributeName, - Values: []string{"val1", "val2"}, + Name: attributeName, + ByteValues: [][]byte{[]byte("val1"), []byte("val2")}, }, }, }, diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index 1897d924..47fa6684 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -244,7 +244,7 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Base = "" })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: nil}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", ExtraRefreshAttributes: map[string]string{}, }, @@ -257,7 +257,7 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Base = "ou=users,dc=pinniped,dc=dev" // there are no groups under this part of the tree })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: nil}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", ExtraRefreshAttributes: map[string]string{}, }, @@ -328,7 +328,7 @@ func TestLDAPSearch_Parallel(t *testing.T) { p.GroupSearch.Filter = "foobar={}" // foobar is not a valid attribute name for this LDAP server's schema })), wantAuthResponse: &authenticators.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: nil}, DN: "cn=pinny,ou=users,dc=pinniped,dc=dev", ExtraRefreshAttributes: map[string]string{}, },