diff --git a/internal/federationdomain/downstreamsession/downstream_session.go b/internal/federationdomain/downstreamsession/downstream_session.go index f37e7d0df..6c4c9ec35 100644 --- a/internal/federationdomain/downstreamsession/downstream_session.go +++ b/internal/federationdomain/downstreamsession/downstream_session.go @@ -30,27 +30,29 @@ const idTransformUnexpectedErr = constable.Error("configured identity transforma type SessionConfig struct { UpstreamIdentity *resolvedprovider.Identity UpstreamLoginExtras *resolvedprovider.IdentityLoginExtras - - // The downstream username. - Username string - // The downstream groups. - Groups []string - // The ID of the client who started the new downstream session. ClientID string // The scopes that were granted for the new downstream session. GrantedScopes []string } -// NewPinnipedSession creates a downstream Pinniped session. +// NewPinnipedSession applies the configured FederationDomain identity transformations +// and creates a downstream Pinniped session. func NewPinnipedSession( + ctx context.Context, idp resolvedprovider.FederationDomainResolvedIdentityProvider, c *SessionConfig, -) *psession.PinnipedSession { +) (*psession.PinnipedSession, error) { now := time.Now().UTC() + downstreamUsername, downstreamGroups, err := applyIdentityTransformations(ctx, + idp.GetTransforms(), c.UpstreamIdentity.UpstreamUsername, c.UpstreamIdentity.UpstreamGroups) + if err != nil { + return nil, err + } + customSessionData := &psession.CustomSessionData{ - Username: c.Username, + Username: downstreamUsername, UpstreamUsername: c.UpstreamIdentity.UpstreamUsername, UpstreamGroups: c.UpstreamIdentity.UpstreamGroups, ProviderUID: idp.GetProvider().GetResourceUID(), @@ -76,15 +78,14 @@ func NewPinnipedSession( extras[oidcapi.IDTokenClaimAuthorizedParty] = c.ClientID if slices.Contains(c.GrantedScopes, oidcapi.ScopeUsername) { - extras[oidcapi.IDTokenClaimUsername] = c.Username + extras[oidcapi.IDTokenClaimUsername] = downstreamUsername } if slices.Contains(c.GrantedScopes, oidcapi.ScopeGroups) { - groups := c.Groups - if groups == nil { - groups = []string{} + if downstreamGroups == nil { + downstreamGroups = []string{} } - extras[oidcapi.IDTokenClaimGroups] = groups + extras[oidcapi.IDTokenClaimGroups] = downstreamGroups } if len(c.UpstreamLoginExtras.DownstreamAdditionalClaims) > 0 { @@ -93,7 +94,7 @@ func NewPinnipedSession( pinnipedSession.IDTokenClaims().Extra = extras - return pinnipedSession + return pinnipedSession, nil } // AutoApproveScopes auto-grants the scopes which we support and for which we do not require end-user approval, @@ -122,9 +123,9 @@ func AutoApproveScopes(authorizeRequester fosite.AuthorizeRequester) { } } -// ApplyIdentityTransformations applies an identity transformation pipeline to an upstream identity to transform +// applyIdentityTransformations applies an identity transformation pipeline to an upstream identity to transform // or potentially reject the identity. -func ApplyIdentityTransformations( +func applyIdentityTransformations( ctx context.Context, transforms *idtransform.TransformationPipeline, username string, diff --git a/internal/federationdomain/downstreamsession/downstream_session_test.go b/internal/federationdomain/downstreamsession/downstream_session_test.go index 91144400f..76d8f7f5e 100644 --- a/internal/federationdomain/downstreamsession/downstream_session_test.go +++ b/internal/federationdomain/downstreamsession/downstream_session_test.go @@ -82,7 +82,7 @@ func TestApplyIdentityTransformations(t *testing.T) { pipeline.AppendTransformation(compiledTransform) } - gotUsername, gotGroups, err := ApplyIdentityTransformations(context.Background(), pipeline, tt.username, tt.groups) + gotUsername, gotGroups, err := applyIdentityTransformations(context.Background(), pipeline, tt.username, tt.groups) if tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) require.Empty(t, gotUsername) diff --git a/internal/federationdomain/endpoints/auth/auth_handler.go b/internal/federationdomain/endpoints/auth/auth_handler.go index 74aff9524..49223936d 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler.go +++ b/internal/federationdomain/endpoints/auth/auth_handler.go @@ -209,20 +209,15 @@ func (h *authorizeHandler) authorizeWithoutBrowser( return err } - username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), - idp.GetTransforms(), identity.UpstreamUsername, identity.UpstreamGroups) - if err != nil { - return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) - } - - session := downstreamsession.NewPinnipedSession(idp, &downstreamsession.SessionConfig{ + session, err := downstreamsession.NewPinnipedSession(r.Context(), idp, &downstreamsession.SessionConfig{ UpstreamIdentity: identity, UpstreamLoginExtras: loginExtras, - Username: username, - Groups: groups, ClientID: authorizeRequester.GetClient().GetID(), GrantedScopes: authorizeRequester.GetGrantedScopes(), }) + if err != nil { + return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) + } oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, session, true) diff --git a/internal/federationdomain/endpoints/callback/callback_handler.go b/internal/federationdomain/endpoints/callback/callback_handler.go index 4313f4232..c812d3143 100644 --- a/internal/federationdomain/endpoints/callback/callback_handler.go +++ b/internal/federationdomain/endpoints/callback/callback_handler.go @@ -57,26 +57,20 @@ func NewHandler( // an error if the client requested a scope that they are not allowed to request, so we don't need to worry about that here. downstreamsession.AutoApproveScopes(authorizeRequester) - identity, loginExtras, err := idp.HandleCallback(r.Context(), authcode(r), state.PKCECode, state.Nonce, redirectURI) + identity, loginExtras, err := idp.LoginFromCallback(r.Context(), authcode(r), state.PKCECode, state.Nonce, redirectURI) if err != nil { return err } - username, groups, err := downstreamsession.ApplyIdentityTransformations( - r.Context(), idp.GetTransforms(), identity.UpstreamUsername, identity.UpstreamGroups, - ) - if err != nil { - return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) - } - - session := downstreamsession.NewPinnipedSession(idp, &downstreamsession.SessionConfig{ + session, err := downstreamsession.NewPinnipedSession(r.Context(), idp, &downstreamsession.SessionConfig{ UpstreamIdentity: identity, UpstreamLoginExtras: loginExtras, - Username: username, - Groups: groups, ClientID: authorizeRequester.GetClient().GetID(), GrantedScopes: authorizeRequester.GetGrantedScopes(), }) + if err != nil { + return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) + } authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, session) if err != nil { diff --git a/internal/federationdomain/endpoints/login/post_login_handler.go b/internal/federationdomain/endpoints/login/post_login_handler.go index 8c513acf0..f510e74b8 100644 --- a/internal/federationdomain/endpoints/login/post_login_handler.go +++ b/internal/federationdomain/endpoints/login/post_login_handler.go @@ -88,23 +88,18 @@ func NewPostHandler(issuerURL string, upstreamIDPs federationdomainproviders.Fed } } - username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), - idp.GetTransforms(), identity.UpstreamUsername, identity.UpstreamGroups) + session, err := downstreamsession.NewPinnipedSession(r.Context(), idp, &downstreamsession.SessionConfig{ + UpstreamIdentity: identity, + UpstreamLoginExtras: loginExtras, + ClientID: authorizeRequester.GetClient().GetID(), + GrantedScopes: authorizeRequester.GetGrantedScopes(), + }) if err != nil { err = fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, err, false) return nil } - session := downstreamsession.NewPinnipedSession(idp, &downstreamsession.SessionConfig{ - UpstreamIdentity: identity, - UpstreamLoginExtras: loginExtras, - Username: username, - Groups: groups, - ClientID: authorizeRequester.GetClient().GetID(), - GrantedScopes: authorizeRequester.GetGrantedScopes(), - }) - oidc.PerformAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, session, false) return nil diff --git a/internal/federationdomain/endpoints/token/token_handler.go b/internal/federationdomain/endpoints/token/token_handler.go index ac685b037..ef893c027 100644 --- a/internal/federationdomain/endpoints/token/token_handler.go +++ b/internal/federationdomain/endpoints/token/token_handler.go @@ -226,7 +226,7 @@ func validateSessionHasUsername(session *psession.PinnipedSession) error { return nil } -// applyIdentityTransformationsDuringRefresh is similar to downstreamsession.ApplyIdentityTransformations +// applyIdentityTransformationsDuringRefresh is similar to downstreamsession.applyIdentityTransformations // but with validation that the username has not changed, and with slightly different error messaging. func applyIdentityTransformationsDuringRefresh( ctx context.Context, diff --git a/internal/federationdomain/resolvedprovider/resolved_provider.go b/internal/federationdomain/resolvedprovider/resolved_provider.go index 73653b4fc..e1f84d54e 100644 --- a/internal/federationdomain/resolvedprovider/resolved_provider.go +++ b/internal/federationdomain/resolvedprovider/resolved_provider.go @@ -83,8 +83,8 @@ type RefreshedIdentity struct { // FederationDomainResolvedIdentityProvider.UpstreamAuthorizeRedirectURL to help formulate the upstream authorization // request. It includes the state param that should be sent in the upstream authorization request. It also includes // the information needed to create the PKCE and nonce parameters for the upstream authorization request. If the -// upstream authorization request does not need PKCE and/or nonce params, then implementations of -// FederationDomainResolvedIdentityProvider.UpstreamAuthorizeRedirectURL may choose to ignore those struct fields. +// upstream authorization request does not allow PKCE, then implementations of +// FederationDomainResolvedIdentityProvider.UpstreamAuthorizeRedirectURL may choose to ignore that struct field. type UpstreamAuthorizeRequestState struct { EncodedStateParam string PKCE pkce.Code @@ -131,6 +131,13 @@ type FederationDomainResolvedIdentityProvider interface { // the downstream browser-based authorization flow. Returned errors should be of type fosite.RFC6749Error. UpstreamAuthorizeRedirectURL(state *UpstreamAuthorizeRequestState, downstreamIssuerURL string) (string, error) + // LoginFromCallback handles an OAuth-style callback in a browser-based flow. This function should complete + // the authorization with the upstream identity provider using the authCode, extract their upstream + // identity, and transform it into their downstream identity. If the upstream does not allow PKCE, then + // the pkce parameter can be ignored. + // Returned errors should be from the httperr package. + LoginFromCallback(ctx context.Context, authCode string, pkce pkce.Code, nonce nonce.Nonce, redirectURI string) (*Identity, *IdentityLoginExtras, error) + // Login performs auth using a username and password that was submitted by the client, without a web browser. // This function should authenticate the user with the upstream identity provider, extract their upstream // identity, and transform it into their downstream identity. @@ -139,12 +146,6 @@ type FederationDomainResolvedIdentityProvider interface { // Returned errors should be of type fosite.RFC6749Error. Login(ctx context.Context, submittedUsername string, submittedPassword string, groupsWillBeIgnored bool) (*Identity, *IdentityLoginExtras, error) - // HandleCallback handles an OAuth-style callback in a browser-based flow. This function should complete - // the authorization with the upstream identity provider using the authCode, extract their upstream - // identity, and transform it into their downstream identity. - // Returned errors should be from the httperr package. - HandleCallback(ctx context.Context, authCode string, pkce pkce.Code, nonce nonce.Nonce, redirectURI string) (*Identity, *IdentityLoginExtras, error) - // UpstreamRefresh performs a refresh with the upstream provider. // The user's previous identity information is provided as a parameter. // Implementations may use this information to assist in refreshes, but mutations to this argument will be ignored. diff --git a/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go b/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go index 97cbed203..59255a099 100644 --- a/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go @@ -171,7 +171,7 @@ func (p *FederationDomainResolvedLDAPIdentityProvider) Login( nil } -func (p *FederationDomainResolvedLDAPIdentityProvider) HandleCallback( +func (p *FederationDomainResolvedLDAPIdentityProvider) LoginFromCallback( _ctx context.Context, _authCode string, _pkce pkce.Code, @@ -179,7 +179,7 @@ func (p *FederationDomainResolvedLDAPIdentityProvider) HandleCallback( _redirectURI string, ) (*resolvedprovider.Identity, *resolvedprovider.IdentityLoginExtras, error) { return nil, nil, httperr.New(http.StatusInternalServerError, - "HandleCallback() is not supported for LDAP and ActiveDirectory types of identity provider") + "LoginFromCallback() is not supported for LDAP and ActiveDirectory types of identity provider") } func (p *FederationDomainResolvedLDAPIdentityProvider) UpstreamRefresh( diff --git a/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go b/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go index 95172dc60..9fa096aa5 100644 --- a/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go @@ -177,7 +177,7 @@ func (p *FederationDomainResolvedOIDCIdentityProvider) Login( nil } -func (p *FederationDomainResolvedOIDCIdentityProvider) HandleCallback( +func (p *FederationDomainResolvedOIDCIdentityProvider) LoginFromCallback( ctx context.Context, authCode string, pkce pkce.Code,