From b3d0b28bd0fd70a0ec3711a759641efbb9fd1c58 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 12 Jul 2021 13:29:37 -0700 Subject: [PATCH] Integration test fixes, fixing objectGUID handling --- hack/prepare-for-integration-tests.sh | 10 ++++++---- internal/oidc/auth/auth_handler_test.go | 23 +++++++++++++++++++++++ internal/upstreamad/upstreamad.go | 10 +--------- internal/upstreamad/upstreamad_test.go | 3 ++- test/integration/supervisor_login_test.go | 11 +++++++++-- 5 files changed, 41 insertions(+), 16 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index ea57a557..bdc9ec42 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -381,15 +381,17 @@ if [[ -z "$(gcloud config list account --format "value(core.account)")" ]]; then exit 1 fi -export PINNIPED_TEST_AD_HOST="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-host' -))" -export PINNIPED_TEST_AD_BIND_ACCOUNT_USERNAME="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-bind-account-username' -))" +export PINNIPED_TEST_AD_HOST="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-host' -)" +export PINNIPED_TEST_AD_BIND_ACCOUNT_USERNAME="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-bind-account-username' -)" export PINNIPED_TEST_AD_BIND_ACCOUNT_PASSWORD="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-bind-account-password' -)" export PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_NAME="objectGUID" export PINNIPED_TEST_AD_USER_UNIQUE_ID_ATTRIBUTE_VALUE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-user-unique-id-attribute-value' -)" export PINNIPED_TEST_AD_USERNAME_ATTRIBUTE_NAME="sAMAccountName" -export PINNIPED_TEST_AD_USERNAME_ATTRIBUTE_VALUE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-user-sAMAccountName' -))" +export PINNIPED_TEST_AD_USERNAME_ATTRIBUTE_VALUE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-user-sAMAccountName' -)" export PINNIPED_TEST_AD_USER_PASSWORD="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-user-password' -)" -export PINNIPED_TEST_AD_LDAPS_CA_BUNDLE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-ca-data' -))" +export PINNIPED_TEST_AD_LDAPS_CA_BUNDLE="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-ca-data' -)" +export PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_DN="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-expected-direct-groups-dn' -)" +export PINNIPED_TEST_AD_USER_EXPECTED_GROUPS_CN="$(gcloud secrets versions access latest --secret="concourse-secrets" --project tanzu-user-authentication | yq e '.aws-ad-expected-direct-groups-cn' -)" fi read -r -d '' PINNIPED_TEST_CLUSTER_CAPABILITY_YAML << PINNIPED_TEST_CLUSTER_CAPABILITY_YAML_EOF || true diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 9a4a99ef..e12c1004 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -477,6 +477,29 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallenge: downstreamPKCEChallenge, wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, }, + { + name: "Active Directory upstream happy path using POST", + idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithActiveDirectory(&upstreamLDAPIdentityProvider).Build(), + method: http.MethodPost, + path: "/some/path", + contentType: "application/x-www-form-urlencoded", + body: encodeQuery(happyGetRequestQueryMap), + customUsernameHeader: pointer.StringPtr(happyLDAPUsername), + customPasswordHeader: pointer.StringPtr(happyLDAPPassword), + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantBodyStringWithLocationInHref: false, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, + wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, + wantDownstreamIDTokenGroups: happyLDAPGroups, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + }, { name: "OIDC upstream happy path with prompt param login passed through to redirect uri", idpLister: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&upstreamOIDCIdentityProvider).Build(), diff --git a/internal/upstreamad/upstreamad.go b/internal/upstreamad/upstreamad.go index c64be5bb..c6e457b8 100644 --- a/internal/upstreamad/upstreamad.go +++ b/internal/upstreamad/upstreamad.go @@ -369,8 +369,6 @@ func (p *Provider) searchAndBindUser(conn upstreamldap.Conn, username string, bi return "", "", nil, err } - // We would like to support binary typed attributes for UIDs, so always read them as binary and encode them, - // even when the attribute may not be binary. mappedUID, err := p.getSearchResultAttributeRawValueEncoded(p.uidAttribute(), userEntry, username) if err != nil { return "", "", nil, err @@ -523,13 +521,7 @@ func (p *Provider) getSearchResultAttributeRawValueEncoded(attributeName string, } if attributeName == objectGUIDAttributeName { - // In AD, objectGUID will be represented as a base64-encoded UUID. Convert it back to UUID encoding. - base64decoded, err := base64.StdEncoding.DecodeString(entry.GetAttributeValue(attributeName)) - if err != nil { - // TODO if there is an error, should we throw it or pass it through as base64? - return "", fmt.Errorf("Error decoding UID: %s", err.Error()) - } - uuidEntry, err := uuid.FromBytes(base64decoded) + uuidEntry, err := uuid.FromBytes(attributeValue) if err != nil { return "", fmt.Errorf("Error decoding UID: %s", err.Error()) } diff --git a/internal/upstreamad/upstreamad_test.go b/internal/upstreamad/upstreamad_test.go index 9c0c2a0b..346741b5 100644 --- a/internal/upstreamad/upstreamad_test.go +++ b/internal/upstreamad/upstreamad_test.go @@ -46,7 +46,7 @@ const ( testGroupSearchResultDNValue1 = "some-upstream-group-dn1" testGroupSearchResultDNValue2 = "some-upstream-group-dn2" testUserSearchResultUsernameAttributeValue = "some-upstream-username-value" - testUserSearchResultUIDAttributeValue = "Ej5FZ+ibEtOkVkJmFBdAAA==" // this is base64 encoded 123e4567-e89b-12d3-a456-426614174000 + testUserSearchResultUIDAttributeValue = "\x12>Eg\xe8\x9b\x12\u04e4VBf\x14\x17@\x00" // binary representation of 123e4567-e89b-12d3-a456-426614174000 testGroupSearchResultGroupNameAttributeValue1 = "some-upstream-group-name-value1" testGroupSearchResultGroupNameAttributeValue2 = "some-upstream-group-name-value2" @@ -207,6 +207,7 @@ func TestEndUserAuthentication(t *testing.T) { ConnectionProtocol: upstreamldap.TLS, BindUsername: testBindUsername, BindPassword: testBindPassword, + // no user search... that's all defaulted. GroupSearch: upstreamldap.GroupSearchConfig{ Base: testGroupSearchBase, Filter: testGroupSearchFilter, diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index aec98094..df53f302 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -254,6 +254,12 @@ func TestSupervisorLogin(t *testing.T) { TLS: &idpv1alpha1.TLSSpec{ CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.CABundle)), }, + UserSearch: idpv1alpha1.ActiveDirectoryIdentityProviderUserSearch{ + Base: "dc=activedirectory,dc=test,dc=pinniped,dc=dev", + }, + GroupSearch: idpv1alpha1.ActiveDirectoryIdentityProviderGroupSearch{ + Base: "dc=activedirectory,dc=test,dc=pinniped,dc=dev", + }, Bind: idpv1alpha1.ActiveDirectoryIdentityProviderBind{ SecretName: secret.Name, }, @@ -276,11 +282,12 @@ func TestSupervisorLogin(t *testing.T) { // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute wantDownstreamIDTokenSubjectToMatch: regexp.QuoteMeta( "ldaps://" + env.SupervisorUpstreamActiveDirectory.Host + - "&sub=" + base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue)), + "?base=" + url.QueryEscape("dc=activedirectory,dc=test,dc=pinniped,dc=dev") + + "&sub=" + env.SupervisorUpstreamActiveDirectory.TestUserUniqueIDAttributeValue, ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamActiveDirectory.TestUsernameAttributeValue), - wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsCNs, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamActiveDirectory.TestUserDirectGroupsDNs, }, } for _, test := range tests {