From 033e1f0399eb8115f7102768f1db8e7a1138c82b Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 26 May 2021 17:04:20 -0700 Subject: [PATCH 1/2] Add user search base to downstream subject for upstream LDAP - Also add some tests about UTF-8 characters in LDAP attributes --- internal/oidc/auth/auth_handler.go | 12 +++++++-- internal/oidc/auth/auth_handler_test.go | 15 ++++++----- .../provider/dynamic_upstream_idp_provider.go | 2 +- .../testutil/oidctestutil/oidctestutil.go | 6 +++-- internal/upstreamldap/upstreamldap.go | 19 ++++++++++---- internal/upstreamldap/upstreamldap_test.go | 15 +++++++++-- test/deploy/tools/ldap.yaml | 2 +- test/integration/ldap_client_test.go | 25 +++++++++++++++++++ test/integration/supervisor_login_test.go | 4 +-- 9 files changed, 79 insertions(+), 21 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 21aad56c..a2f6f566 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -15,6 +15,7 @@ import ( "github.com/ory/fosite/token/jwt" "github.com/pkg/errors" "golang.org/x/oauth2" + "k8s.io/apiserver/pkg/authentication/authenticator" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/httputil/securityheader" @@ -108,11 +109,10 @@ func handleAuthRequestForLDAPUpstream( return nil } - subject := fmt.Sprintf("%s?%s=%s", ldapUpstream.GetURL(), oidc.IDTokenSubjectClaim, authenticateResponse.User.GetUID()) now := time.Now().UTC() openIDSession := &openid.DefaultSession{ Claims: &jwt.IDTokenClaims{ - Subject: subject, + Subject: downstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse), RequestedAt: now, AuthTime: now, }, @@ -359,3 +359,11 @@ func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken return nil } + +func downstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentityProviderI, authenticateResponse *authenticator.Response) string { + ldapURL := *ldapUpstream.GetURL() + q := ldapURL.Query() + q.Set(oidc.IDTokenSubjectClaim, authenticateResponse.User.GetUID()) + ldapURL.RawQuery = q.Encode() + return ldapURL.String() +} diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index d65dbf4b..9c92301e 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -44,7 +44,7 @@ func TestAuthorizationEndpoint(t *testing.T) { downstreamPKCEChallengeMethod = "S256" happyState = "8b-state" downstreamClientID = "pinniped-cli" - upstreamLDAPURL = "ldaps://some-ldap-host:123" + upstreamLDAPURL = "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev" htmlContentType = "text/html; charset=utf-8" ) @@ -158,9 +158,12 @@ func TestAuthorizationEndpoint(t *testing.T) { happyLDAPUID := "some-ldap-uid" happyLDAPGroups := []string{"group1", "group2", "group3"} + parsedUpstreamLDAPURL, err := url.Parse(upstreamLDAPURL) + require.NoError(t, err) + upstreamLDAPIdentityProvider := oidctestutil.TestUpstreamLDAPIdentityProvider{ Name: "some-ldap-idp", - URL: upstreamLDAPURL, + URL: parsedUpstreamLDAPURL, AuthenticateFunc: func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { if username == "" || password == "" { return nil, false, fmt.Errorf("should not have passed empty username or password to the authenticator") @@ -384,7 +387,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, wantBodyStringWithLocationInHref: false, - wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -443,7 +446,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, wantBodyStringWithLocationInHref: false, - wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -525,7 +528,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, wantRedirectLocationRegexp: downstreamRedirectURIWithDifferentPort + `\?code=([^&]+)&scope=openid&state=` + happyState, wantBodyStringWithLocationInHref: false, - wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -941,7 +944,7 @@ func TestAuthorizationEndpoint(t *testing.T) { wantContentType: htmlContentType, wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyState, // no scopes granted wantBodyStringWithLocationInHref: false, - wantDownstreamIDTokenSubject: upstreamLDAPURL + "?sub=" + happyLDAPUID, + wantDownstreamIDTokenSubject: upstreamLDAPURL + "&sub=" + happyLDAPUID, wantDownstreamIDTokenUsername: happyLDAPUsernameFromAuthenticator, wantDownstreamIDTokenGroups: happyLDAPGroups, wantDownstreamRequestedScopes: []string{"email"}, // only email was requested diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 50965abc..af24e724 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -56,7 +56,7 @@ type UpstreamLDAPIdentityProviderI interface { // Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234". // This URL is not used for connecting to the provider, but rather is used for creating a globally unique user // identifier by being combined with the user's UID, since user UIDs are only unique within one provider. - GetURL() string + GetURL() *url.URL // A method for performing user authentication against the upstream LDAP provider. authenticators.UserAuthenticator diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index e4718270..b8e7b0de 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -51,10 +51,12 @@ type ExchangeAuthcodeAndValidateTokenArgs struct { type TestUpstreamLDAPIdentityProvider struct { Name string - URL string + URL *url.URL AuthenticateFunc func(ctx context.Context, username, password string) (*authenticator.Response, bool, error) } +var _ provider.UpstreamLDAPIdentityProviderI = &TestUpstreamLDAPIdentityProvider{} + func (u *TestUpstreamLDAPIdentityProvider) GetName() string { return u.Name } @@ -63,7 +65,7 @@ func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, return u.AuthenticateFunc(ctx, username, password) } -func (u *TestUpstreamLDAPIdentityProvider) GetURL() string { +func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL { return u.URL } diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index f857c730..a7732261 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -11,17 +11,19 @@ import ( "errors" "fmt" "net" + "net/url" "sort" "strings" "time" - "k8s.io/utils/trace" - "github.com/go-ldap/ldap/v3" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/utils/trace" + "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/endpointaddr" + "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" ) @@ -138,6 +140,9 @@ type Provider struct { c ProviderConfig } +var _ provider.UpstreamLDAPIdentityProviderI = &Provider{} +var _ authenticators.UserAuthenticator = &Provider{} + // Create a Provider. The config is not a pointer to ensure that a copy of the config is created, // making the resulting Provider use an effectively read-only configuration. func New(config ProviderConfig) *Provider { @@ -249,11 +254,15 @@ func (p *Provider) GetName() string { return p.c.Name } -// Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234". +// Return a URL which uniquely identifies this LDAP provider, e.g. "ldaps://host.example.com:1234?base=user-search-base". // This URL is not used for connecting to the provider, but rather is used for creating a globally unique user // identifier by being combined with the user's UID, since user UIDs are only unique within one provider. -func (p *Provider) GetURL() string { - return fmt.Sprintf("%s://%s", ldapsScheme, p.c.Host) +func (p *Provider) GetURL() *url.URL { + u := &url.URL{Scheme: ldapsScheme, Host: p.c.Host} + q := u.Query() + q.Set("base", p.c.UserSearch.Base) + u.RawQuery = q.Encode() + return u } // TestConnection provides a method for testing the connection and bind settings. It performs a dial and bind diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index fd8b9658..c9e88be3 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -1115,8 +1115,19 @@ func TestGetConfig(t *testing.T) { } func TestGetURL(t *testing.T) { - require.Equal(t, "ldaps://ldap.example.com:1234", New(ProviderConfig{Host: "ldap.example.com:1234"}).GetURL()) - require.Equal(t, "ldaps://ldap.example.com", New(ProviderConfig{Host: "ldap.example.com"}).GetURL()) + require.Equal(t, + "ldaps://ldap.example.com:1234?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev", + New(ProviderConfig{ + Host: "ldap.example.com:1234", + UserSearch: UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, + }).GetURL().String()) + + require.Equal(t, + "ldaps://ldap.example.com?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev", + New(ProviderConfig{ + Host: "ldap.example.com", + UserSearch: UserSearchConfig{Base: "ou=users,dc=pinniped,dc=dev"}, + }).GetURL().String()) } // Testing of host parsing, TLS negotiation, and CA bundle, etc. for the production code's dialer. diff --git a/test/deploy/tools/ldap.yaml b/test/deploy/tools/ldap.yaml index 25410fcb..ab771b10 100644 --- a/test/deploy/tools/ldap.yaml +++ b/test/deploy/tools/ldap.yaml @@ -41,7 +41,7 @@ ldap.ldif: | objectClass: shadowAccount cn: pinny sn: Seal - givenName: Pinny + givenName: Pinny the 🦭 mail: pinny.ldap@example.com userPassword: (@= data.values.pinny_ldap_password @) uid: pinny diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index 765b1f45..e95173f1 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -167,6 +167,31 @@ func TestLDAPSearch(t *testing.T) { User: &user.DefaultInfo{Name: "Seal", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, // note that the final answer has case preserved from the entry }, }, + { + name: "when the UsernameAttribute or UIDAttribute are attributes whose value contains UTF-8 data", + username: "pinny", + password: pinnyPassword, + provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { + p.UserSearch.Filter = "cn={}" + p.UserSearch.UsernameAttribute = "givenName" + p.UserSearch.UIDAttribute = "givenName" + })), + wantAuthResponse: &authenticator.Response{ + User: &user.DefaultInfo{Name: "Pinny the 🦭", UID: "Pinny the 🦭", Groups: []string{"ball-game-players", "seals"}}, + }, + }, + { + name: "when the search filter is searching on an attribute whose value contains UTF-8 data", + username: "Pinny the 🦭", + password: pinnyPassword, + provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { + p.UserSearch.Filter = "givenName={}" + p.UserSearch.UsernameAttribute = "cn" + })), + wantAuthResponse: &authenticator.Response{ + User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + }, + }, { name: "when the UsernameAttribute is dn and there is no user search filter provided", username: "cn=pinny,ou=users,dc=pinniped,dc=dev", diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 1690c109..12e4553e 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -119,7 +119,7 @@ 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.SupervisorUpstreamLDAP.Host + "?sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + "ldaps://" + env.SupervisorUpstreamLDAP.Host + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + "&sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue), @@ -176,7 +176,7 @@ 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.SupervisorUpstreamLDAP.StartTLSOnlyHost + "?sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + "ldaps://" + env.SupervisorUpstreamLDAP.StartTLSOnlyHost + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + "&sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN), From 81148866e0ef0a8e09341823243222491a89ec69 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 27 May 2021 09:25:48 -0700 Subject: [PATCH 2/2] URL query escape the upstream OIDC subject in the downstream subject URL --- internal/oidc/callback/callback_handler.go | 6 +++- .../oidc/callback/callback_handler_test.go | 31 ++++++++++--------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 5bece1d9..1b14c788 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -228,7 +228,7 @@ func getSubjectAndUsernameFromUpstreamIDToken( return "", "", httperr.New(http.StatusUnprocessableEntity, "subject claim in upstream ID token has invalid format") } - subject := fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, upstreamSubject) + subject := downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString, upstreamSubject) usernameClaimName := upstreamIDPConfig.GetUsernameClaim() if usernameClaimName == "" { @@ -282,6 +282,10 @@ func getSubjectAndUsernameFromUpstreamIDToken( return subject, username, nil } +func downstreamSubjectFromUpstreamOIDC(upstreamIssuerAsString string, upstreamSubject string) string { + return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, oidc.IDTokenSubjectClaim, url.QueryEscape(upstreamSubject)) +} + func getGroupsFromUpstreamIDToken( upstreamIDPConfig provider.UpstreamOIDCIdentityProviderI, idTokenClaims map[string]interface{}, diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index c999cba4..583ee943 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -28,9 +28,10 @@ import ( const ( happyUpstreamIDPName = "upstream-idp-name" - upstreamIssuer = "https://my-upstream-issuer.com" - upstreamSubject = "abc123-some-guid" - upstreamUsername = "test-pinniped-username" + upstreamIssuer = "https://my-upstream-issuer.com" + upstreamSubject = "abc123-some guid" // has a space character which should get escaped in URL + queryEscapedUpstreamSubject = "abc123-some+guid" + upstreamUsername = "test-pinniped-username" upstreamUsernameClaim = "the-user-claim" upstreamGroupsClaim = "the-groups-claim" @@ -141,7 +142,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: upstreamUsername, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -160,8 +161,8 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, - wantDownstreamIDTokenUsername: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, + wantDownstreamIDTokenUsername: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenGroups: []string{}, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, wantDownstreamGrantedScopes: happyDownstreamScopesGranted, @@ -180,7 +181,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: "joe@whitehouse.gov", wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -201,7 +202,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: "joe@whitehouse.gov", wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -223,7 +224,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, // succeed despite `email_verified=false` because we're not using the email claim for anything wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: "joe", wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -268,7 +269,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: upstreamSubject, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -287,7 +288,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: upstreamUsername, wantDownstreamIDTokenGroups: []string{"notAnArrayGroup1 notAnArrayGroup2"}, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -306,7 +307,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: upstreamUsername, wantDownstreamIDTokenGroups: []string{"group1", "group2"}, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, @@ -445,7 +446,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=&state=` + happyDownstreamState, wantDownstreamIDTokenUsername: upstreamUsername, - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamRequestedScopes: []string{"profile", "email"}, wantDownstreamIDTokenGroups: upstreamGroupMembership, wantDownstreamNonce: downstreamNonce, @@ -467,7 +468,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: downstreamRedirectURI + `\?code=([^&]+)&scope=openid\+offline_access&state=` + happyDownstreamState, wantDownstreamIDTokenUsername: upstreamUsername, - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamRequestedScopes: []string{"openid", "offline_access"}, wantDownstreamGrantedScopes: []string{"openid", "offline_access"}, wantDownstreamIDTokenGroups: upstreamGroupMembership, @@ -548,7 +549,7 @@ func TestCallbackEndpoint(t *testing.T) { wantStatus: http.StatusFound, wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, wantBody: "", - wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + upstreamSubject, + wantDownstreamIDTokenSubject: upstreamIssuer + "?sub=" + queryEscapedUpstreamSubject, wantDownstreamIDTokenUsername: upstreamUsername, wantDownstreamRequestedScopes: happyDownstreamScopesRequested, wantDownstreamGrantedScopes: happyDownstreamScopesGranted,