diff --git a/internal/federationdomain/endpoints/auth/auth_handler.go b/internal/federationdomain/endpoints/auth/auth_handler.go index 7cc7b91ea..ee849e487 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler.go +++ b/internal/federationdomain/endpoints/auth/auth_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package auth provides a handler for the OIDC authorization endpoint. @@ -24,7 +24,7 @@ import ( "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/resolvedprovider" "go.pinniped.dev/internal/federationdomain/upstreamprovider" - "go.pinniped.dev/internal/httputil/httperr" + "go.pinniped.dev/internal/httputil/responseutil" "go.pinniped.dev/internal/httputil/securityheader" "go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/plog" @@ -38,6 +38,26 @@ const ( promptParamNone = "none" ) +var ( + errUnexpectedUpstreamError = &fosite.RFC6749Error{ + ErrorField: "error", // this string matches what fosite uses for generic errors + DescriptionField: "Unexpected error during upstream authentication.", + CodeField: http.StatusBadGateway, + } +) + +type authorizeHandler struct { + downstreamIssuer string + idpFinder federationdomainproviders.FederationDomainIdentityProvidersFinderI + oauthHelperWithoutStorage fosite.OAuth2Provider + oauthHelperWithStorage fosite.OAuth2Provider + generateCSRF func() (csrftoken.CSRFToken, error) + generatePKCE func() (pkce.Code, error) + generateNonce func() (nonce.Nonce, error) + upstreamStateEncoder oidc.Encoder + cookieCodec oidc.Codec +} + func NewHandler( downstreamIssuer string, idpFinder federationdomainproviders.FederationDomainIdentityProvidersFinderI, @@ -49,121 +69,215 @@ func NewHandler( upstreamStateEncoder oidc.Encoder, cookieCodec oidc.Codec, ) http.Handler { - handler := httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { - if r.Method != http.MethodPost && r.Method != http.MethodGet { - // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest - // Authorization Servers MUST support the use of the HTTP GET and POST methods defined in - // RFC 2616 [RFC2616] at the Authorization Endpoint. - return httperr.Newf(http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method) - } - - requestedBrowserlessFlow := len(r.Header.Values(oidcapi.AuthorizeUsernameHeaderName)) > 0 || - len(r.Header.Values(oidcapi.AuthorizePasswordHeaderName)) > 0 - - // Need to parse the request params so we can get the IDP name. The style and text of the error is inspired by - // fosite's implementation of NewAuthorizeRequest(). Fosite only calls ParseMultipartForm() there. However, - // although ParseMultipartForm() calls ParseForm(), it swallows errors from ParseForm() sometimes. To avoid - // having any errors swallowed, we call both. When fosite calls ParseMultipartForm() later, it will be a noop. - if err := r.ParseForm(); err != nil { - oidc.WriteAuthorizeError(r, w, - oauthHelperWithoutStorage, - fosite.NewAuthorizeRequest(), - fosite.ErrInvalidRequest. - WithHint("Unable to parse form params, make sure to send a properly formatted query params or form request body."). - WithWrap(err).WithDebug(err.Error()), - requestedBrowserlessFlow) - return nil - } - if err := r.ParseMultipartForm(1 << 20); err != nil && err != http.ErrNotMultipart { - oidc.WriteAuthorizeError(r, w, - oauthHelperWithoutStorage, - fosite.NewAuthorizeRequest(), - fosite.ErrInvalidRequest. - WithHint("Unable to parse multipart HTTP body, make sure to send a properly formatted form request body."). - WithWrap(err).WithDebug(err.Error()), - requestedBrowserlessFlow) - return nil - } - - // Note that the client might have used oidcapi.AuthorizeUpstreamIDPNameParamName and - // oidcapi.AuthorizeUpstreamIDPTypeParamName query (or form) params to request a certain upstream IDP. - // The Pinniped CLI has been sending these params since v0.9.0. - idpNameQueryParamValue := r.Form.Get(oidcapi.AuthorizeUpstreamIDPNameParamName) - - // Check if we are in a special case where we should inject an interstitial page to ask the user - // which IDP they would like to use. - if shouldShowIDPChooser(idpFinder, idpNameQueryParamValue, requestedBrowserlessFlow) { - // Redirect to the IDP chooser page with all the same query/form params. When the user chooses an IDP, - // it will redirect back to here with all the same params again, with the pinniped_idp_name param added. - http.Redirect(w, r, - fmt.Sprintf("%s%s?%s", downstreamIssuer, oidc.ChooseIDPEndpointPath, r.Form.Encode()), - http.StatusSeeOther, - ) - return nil - } - - oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpNameQueryParamValue, idpFinder) - if err != nil { - oidc.WriteAuthorizeError(r, w, - oauthHelperWithoutStorage, - fosite.NewAuthorizeRequest(), - fosite.ErrInvalidRequest. - WithHintf("%q param error: %s", oidcapi.AuthorizeUpstreamIDPNameParamName, err.Error()). - WithWrap(err).WithDebug(err.Error()), - requestedBrowserlessFlow) - return nil - } - - if oidcUpstream != nil { - if requestedBrowserlessFlow { - // The client set a username header, so they are trying to log in with a username/password. - return handleAuthRequestForOIDCUpstreamPasswordGrant(r, w, - oauthHelperWithStorage, - oidcUpstream.Provider, - oidcUpstream.Transforms, - oidcUpstream.DisplayName, - idpNameQueryParamValue, - ) - } - return handleAuthRequestForOIDCUpstreamBrowserFlow(r, w, - oauthHelperWithoutStorage, - generateCSRF, generateNonce, generatePKCE, - oidcUpstream, - downstreamIssuer, - upstreamStateEncoder, - cookieCodec, - idpNameQueryParamValue, - ) - } - - // We know it's an AD/LDAP upstream. - if requestedBrowserlessFlow { - // The client set a username header, so they are trying to log in with a username/password. - return handleAuthRequestForLDAPUpstreamCLIFlow(r, w, - oauthHelperWithStorage, - ldapUpstream.Provider, - ldapUpstream.SessionProviderType, - ldapUpstream.Transforms, - ldapUpstream.DisplayName, - idpNameQueryParamValue, - ) - } - return handleAuthRequestForLDAPUpstreamBrowserFlow(r, w, - oauthHelperWithoutStorage, - generateCSRF, generateNonce, generatePKCE, - ldapUpstream, - ldapUpstream.SessionProviderType, - downstreamIssuer, - upstreamStateEncoder, - cookieCodec, - idpNameQueryParamValue, - ) - }) - + h := &authorizeHandler{ + downstreamIssuer: downstreamIssuer, + idpFinder: idpFinder, + oauthHelperWithoutStorage: oauthHelperWithoutStorage, + oauthHelperWithStorage: oauthHelperWithStorage, + generateCSRF: generateCSRF, + generatePKCE: generatePKCE, + generateNonce: generateNonce, + upstreamStateEncoder: upstreamStateEncoder, + cookieCodec: cookieCodec, + } // During a response_mode=form_post auth request using the browser flow, the custom form_post html page may // be used to post certain errors back to the CLI from this handler's response, so allow the form_post // page's CSS and JS to run. - return securityheader.WrapWithCustomCSP(handler, formposthtml.ContentSecurityPolicy()) + return securityheader.WrapWithCustomCSP(h, formposthtml.ContentSecurityPolicy()) +} + +func (h *authorizeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost && r.Method != http.MethodGet { + // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + // Authorization Servers MUST support the use of the HTTP GET and POST methods defined in + // RFC 2616 [RFC2616] at the Authorization Endpoint. + responseutil.HTTPErrorf(w, http.StatusMethodNotAllowed, "%s (try GET or POST)", r.Method) + return + } + + // The client set a username or password header, so they are trying to log in without using a browser. + requestedBrowserlessFlow := len(r.Header.Values(oidcapi.AuthorizeUsernameHeaderName)) > 0 || + len(r.Header.Values(oidcapi.AuthorizePasswordHeaderName)) > 0 + + // Need to parse the request params, so we can get the IDP name. The style and text of the error is inspired by + // fosite's implementation of NewAuthorizeRequest(). Fosite only calls ParseMultipartForm() there. However, + // although ParseMultipartForm() calls ParseForm(), it swallows errors from ParseForm() sometimes. To avoid + // having any errors swallowed, we call both. When fosite calls ParseMultipartForm() later, it will be a noop. + if err := r.ParseForm(); err != nil { + oidc.WriteAuthorizeError(r, w, + h.oauthHelperWithoutStorage, + fosite.NewAuthorizeRequest(), + fosite.ErrInvalidRequest. + WithHint("Unable to parse form params, make sure to send a properly formatted query params or form request body."). + WithWrap(err).WithDebug(err.Error()), + requestedBrowserlessFlow) + return + } + if err := r.ParseMultipartForm(1 << 20); err != nil && err != http.ErrNotMultipart { + oidc.WriteAuthorizeError(r, w, + h.oauthHelperWithoutStorage, + fosite.NewAuthorizeRequest(), + fosite.ErrInvalidRequest. + WithHint("Unable to parse multipart HTTP body, make sure to send a properly formatted form request body."). + WithWrap(err).WithDebug(err.Error()), + requestedBrowserlessFlow) + return + } + + // Note that the client might have used oidcapi.AuthorizeUpstreamIDPNameParamName and + // oidcapi.AuthorizeUpstreamIDPTypeParamName query (or form) params to request a certain upstream IDP. + // The Pinniped CLI has been sending these params since v0.9.0. + idpNameQueryParamValue := r.Form.Get(oidcapi.AuthorizeUpstreamIDPNameParamName) + + // Check if we are in a special case where we should inject an interstitial page to ask the user + // which IDP they would like to use. + if shouldShowIDPChooser(h.idpFinder, idpNameQueryParamValue, requestedBrowserlessFlow) { + // Redirect to the IDP chooser page with all the same query/form params. When the user chooses an IDP, + // it will redirect back to here with all the same params again, with the pinniped_idp_name param added. + http.Redirect(w, r, + fmt.Sprintf("%s%s?%s", h.downstreamIssuer, oidc.ChooseIDPEndpointPath, r.Form.Encode()), + http.StatusSeeOther, + ) + return + } + + oidcUpstream, ldapUpstream, err := chooseUpstreamIDP(idpNameQueryParamValue, h.idpFinder) + if err != nil { + oidc.WriteAuthorizeError(r, w, + h.oauthHelperWithoutStorage, + fosite.NewAuthorizeRequest(), + fosite.ErrInvalidRequest. + WithHintf("%q param error: %s", oidcapi.AuthorizeUpstreamIDPNameParamName, err.Error()). + WithWrap(err).WithDebug(err.Error()), + requestedBrowserlessFlow) + return + } + + h.authorize(w, r, requestedBrowserlessFlow, idpNameQueryParamValue, oidcUpstream, ldapUpstream) +} + +func (h *authorizeHandler) authorize( + w http.ResponseWriter, + r *http.Request, + requestedBrowserlessFlow bool, + idpNameQueryParamValue string, + oidcUpstream *resolvedprovider.FederationDomainResolvedOIDCIdentityProvider, + ldapUpstream *resolvedprovider.FederationDomainResolvedLDAPIdentityProvider, +) { + // Browser flows do not need session storage at this step. For browser flows, the request parameters + // should be forwarded to the next step as upstream state parameters to avoid storing session state + // until the user successfully authenticates. + oauthHelper := h.oauthHelperWithoutStorage + if requestedBrowserlessFlow { + oauthHelper = h.oauthHelperWithStorage + } + + authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r) + if err != nil { + oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, err, requestedBrowserlessFlow) + return + } + + maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) + + // Automatically grant certain scopes, but only if they were requested. + // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. + // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope + // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. + // This is instead of asking the user to approve these scopes. Note that `NewAuthorizeRequest` would have returned + // 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) + + if requestedBrowserlessFlow { + err = h.authorizeWithoutBrowser(w, r, oauthHelper, authorizeRequester, oidcUpstream, ldapUpstream) + } else { + err = h.authorizeWithBrowser(w, r, oauthHelper, authorizeRequester, oidcUpstream, ldapUpstream) + } + if err != nil { + oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, err, requestedBrowserlessFlow) + } +} + +func (h *authorizeHandler) authorizeWithoutBrowser( + w http.ResponseWriter, + r *http.Request, + oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, + oidcUpstream *resolvedprovider.FederationDomainResolvedOIDCIdentityProvider, + ldapUpstream *resolvedprovider.FederationDomainResolvedLDAPIdentityProvider, +) error { + if err := requireStaticClientForUsernameAndPasswordHeaders(authorizeRequester); err != nil { + return err + } + + submittedUsername, submittedPassword, err := requireNonEmptyUsernameAndPasswordHeaders(r) + if err != nil { + return err + } + + switch { + case oidcUpstream != nil: + return handleAuthRequestForOIDCUpstreamPasswordGrant(r, w, + oauthHelper, + authorizeRequester, + oidcUpstream.Provider, + oidcUpstream.Transforms, + oidcUpstream.DisplayName, + submittedUsername, + submittedPassword, + ) + case ldapUpstream != nil: + return handleAuthRequestForLDAPUpstreamCLIFlow(r, w, + oauthHelper, + authorizeRequester, + ldapUpstream.Provider, + ldapUpstream.SessionProviderType, + ldapUpstream.Transforms, + ldapUpstream.DisplayName, + submittedUsername, + submittedPassword, + ) + default: + // It should not actually be possible to reach this case. + return fosite.ErrServerError.WithHint("Huh? Unknown upstream IDP type.") + } +} + +func (h *authorizeHandler) authorizeWithBrowser( + w http.ResponseWriter, + r *http.Request, + oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, + oidcUpstream *resolvedprovider.FederationDomainResolvedOIDCIdentityProvider, + ldapUpstream *resolvedprovider.FederationDomainResolvedLDAPIdentityProvider, +) error { + switch { + case oidcUpstream != nil: + return handleAuthRequestForOIDCUpstreamBrowserFlow(r, w, + oauthHelper, + authorizeRequester, + h.generateCSRF, h.generateNonce, h.generatePKCE, + oidcUpstream.DisplayName, + oidcUpstream.Provider, + h.downstreamIssuer, + h.upstreamStateEncoder, + h.cookieCodec, + ) + case ldapUpstream != nil: + return handleAuthRequestForLDAPUpstreamBrowserFlow(r, w, + oauthHelper, + authorizeRequester, + h.generateCSRF, h.generateNonce, h.generatePKCE, + ldapUpstream.DisplayName, + h.downstreamIssuer, + h.upstreamStateEncoder, + h.cookieCodec, + ldapUpstream.SessionProviderType, + ) + default: + // It should not actually be possible to reach this case. + return fosite.ErrServerError.WithHint("Huh? Unknown upstream IDP type.") + } } func shouldShowIDPChooser( @@ -184,37 +298,21 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, ldapUpstream upstreamprovider.UpstreamLDAPIdentityProviderI, idpType psession.ProviderType, identityTransforms *idtransform.TransformationPipeline, idpDisplayName string, - idpNameQueryParamValue string, + submittedUsername string, + submittedPassword string, ) error { - authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, true) - if !created { - return nil - } - - maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) - - if !requireStaticClientForUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) { - return nil - } - - submittedUsername, submittedPassword, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) - if !hadUsernamePasswordValues { - return nil - } - authenticateResponse, authenticated, err := ldapUpstream.AuthenticateUser(r.Context(), submittedUsername, submittedPassword, authorizeRequester.GetGrantedScopes()) if err != nil { plog.WarningErr("unexpected error during upstream LDAP authentication", err, "upstreamName", ldapUpstream.GetName()) - return httperr.New(http.StatusBadGateway, "unexpected error during upstream authentication") + return errUnexpectedUpstreamError.WithWrap(err) } if !authenticated { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Username/password not accepted by LDAP provider."), true) - return nil + return fosite.ErrAccessDenied.WithHint("Username/password not accepted by LDAP provider.") } subject := downstreamsession.DownstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse, idpDisplayName) @@ -223,10 +321,7 @@ func handleAuthRequestForLDAPUpstreamCLIFlow( username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), identityTransforms, upstreamUsername, upstreamGroups) if err != nil { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, - ) - return nil + return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) } customSessionData := downstreamsession.MakeDownstreamLDAPOrADCustomSessionData(ldapUpstream, idpType, authenticateResponse, username, upstreamUsername, upstreamGroups) @@ -241,71 +336,54 @@ func handleAuthRequestForLDAPUpstreamBrowserFlow( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), generatePKCE func() (pkce.Code, error), - ldapUpstream *resolvedprovider.FederationDomainResolvedLDAPIdentityProvider, - idpType psession.ProviderType, + idpDisplayName string, downstreamIssuer string, upstreamStateEncoder oidc.Encoder, cookieCodec oidc.Codec, - idpNameQueryParamValue string, + idpType psession.ProviderType, ) error { - authRequestState, err := handleBrowserFlowAuthRequest( - r, - w, + authRequestState, err := handleBrowserFlowAuthRequest(r, w, + authorizeRequester, oauthHelper, generateCSRF, generateNonce, generatePKCE, - ldapUpstream.DisplayName, + idpDisplayName, idpType, cookieCodec, upstreamStateEncoder, - idpNameQueryParamValue, ) if err != nil { return err } - if authRequestState == nil { - // There was an error but handleBrowserFlowAuthRequest() already took care of writing the response for it. - return nil + + err = login.RedirectToLoginPage(r, w, downstreamIssuer, authRequestState.encodedStateParam, login.ShowNoError) + if err != nil { + return fosite.ErrServerError.WithHint("Server could not formulate login UI URL for redirect.").WithWrap(err) } - return login.RedirectToLoginPage(r, w, downstreamIssuer, authRequestState.encodedStateParam, login.ShowNoError) + return nil } func handleAuthRequestForOIDCUpstreamPasswordGrant( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, oidcUpstream upstreamprovider.UpstreamOIDCIdentityProviderI, identityTransforms *idtransform.TransformationPipeline, idpDisplayName string, - idpNameQueryParamValue string, + submittedUsername string, + submittedPassword string, ) error { - authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, true) - if !created { - return nil - } - - maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) - - if !requireStaticClientForUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) { - return nil - } - - submittedUsername, submittedPassword, hadUsernamePasswordValues := requireNonEmptyUsernameAndPasswordHeaders(r, w, oauthHelper, authorizeRequester) - if !hadUsernamePasswordValues { - return nil - } - if !oidcUpstream.AllowsPasswordGrant() { // Return a user-friendly error for this case which is entirely within our control. - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHint( - "Resource owner password credentials grant is not allowed for this upstream provider according to its configuration."), true) - return nil + return fosite.ErrAccessDenied.WithHint( + "Resource owner password credentials grant is not allowed for this upstream provider according to its configuration.") } token, err := oidcUpstream.PasswordCredentialsGrantAndValidateTokens(r.Context(), submittedUsername, submittedPassword) @@ -317,9 +395,7 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( // However, the exact response is undefined in the sense that there is no such thing as a password grant in // the OIDC spec, so we don't try too hard to read the upstream errors in this case. (E.g. Dex departs from the // spec and returns something other than an "invalid_grant" error for bad resource owner credentials.) - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithDebug(err.Error()), true) // WithDebug hides the error from the client - return nil + return fosite.ErrAccessDenied.WithDebug(err.Error()) // WithDebug hides the error from the client } subject, upstreamUsername, upstreamGroups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken( @@ -327,28 +403,19 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( ) if err != nil { // Return a user-friendly error for this case which is entirely within our control. - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, - ) - return nil + return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) } username, groups, err := downstreamsession.ApplyIdentityTransformations(r.Context(), identityTransforms, upstreamUsername, upstreamGroups) if err != nil { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, - ) - return nil + return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) } additionalClaims := downstreamsession.MapAdditionalClaimsFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token, username, upstreamUsername, upstreamGroups) if err != nil { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, - ) - return nil + return fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()) } openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, @@ -363,43 +430,38 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, + authorizeRequester fosite.AuthorizeRequester, generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), generatePKCE func() (pkce.Code, error), - oidcUpstream *resolvedprovider.FederationDomainResolvedOIDCIdentityProvider, + idpDisplayName string, + oidcUpstream upstreamprovider.UpstreamOIDCIdentityProviderI, downstreamIssuer string, upstreamStateEncoder oidc.Encoder, cookieCodec oidc.Codec, - idpNameQueryParamValue string, ) error { - authRequestState, err := handleBrowserFlowAuthRequest( - r, - w, + authRequestState, err := handleBrowserFlowAuthRequest(r, w, + authorizeRequester, oauthHelper, generateCSRF, generateNonce, generatePKCE, - oidcUpstream.DisplayName, + idpDisplayName, psession.ProviderTypeOIDC, cookieCodec, upstreamStateEncoder, - idpNameQueryParamValue, ) if err != nil { return err } - if authRequestState == nil { - // There was an error but handleBrowserFlowAuthRequest() already took care of writing the response for it. - return nil - } upstreamOAuthConfig := oauth2.Config{ - ClientID: oidcUpstream.Provider.GetClientID(), + ClientID: oidcUpstream.GetClientID(), Endpoint: oauth2.Endpoint{ - AuthURL: oidcUpstream.Provider.GetAuthorizationURL().String(), + AuthURL: oidcUpstream.GetAuthorizationURL().String(), }, RedirectURL: fmt.Sprintf("%s/callback", downstreamIssuer), - Scopes: oidcUpstream.Provider.GetScopes(), + Scopes: oidcUpstream.GetScopes(), } authCodeOptions := []oauth2.AuthCodeOption{ @@ -408,7 +470,7 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( authRequestState.pkce.Method(), } - for key, val := range oidcUpstream.Provider.GetAdditionalAuthcodeParams() { + for key, val := range oidcUpstream.GetAdditionalAuthcodeParams() { authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(key, val)) } @@ -423,42 +485,20 @@ func handleAuthRequestForOIDCUpstreamBrowserFlow( return nil } -func requireStaticClientForUsernameAndPasswordHeaders(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) bool { - isStaticClient := authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI - if !isStaticClient { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("This client is not allowed to submit username or password headers to this endpoint."), true) +func requireStaticClientForUsernameAndPasswordHeaders(authorizeRequester fosite.AuthorizeRequester) error { + if !(authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI) { + return fosite.ErrAccessDenied.WithHint("This client is not allowed to submit username or password headers to this endpoint.") } - return isStaticClient + return nil } -func requireNonEmptyUsernameAndPasswordHeaders(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, authorizeRequester fosite.AuthorizeRequester) (string, string, bool) { +func requireNonEmptyUsernameAndPasswordHeaders(r *http.Request) (string, string, error) { username := r.Header.Get(oidcapi.AuthorizeUsernameHeaderName) password := r.Header.Get(oidcapi.AuthorizePasswordHeaderName) if username == "" || password == "" { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Missing or blank username or password."), true) - return "", "", false + return "", "", fosite.ErrAccessDenied.WithHint("Missing or blank username or password.") } - return username, password, true -} - -func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fosite.OAuth2Provider, isBrowserless bool) (fosite.AuthorizeRequester, bool) { - authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r) - if err != nil { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, err, isBrowserless) - return nil, false - } - - // Automatically grant certain scopes, but only if they were requested. - // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. - // There don't seem to be any validations inside `NewAuthorizeResponse` related to the offline_access scope - // at this time, however we will temporarily grant the scope just in case that changes in a future release of fosite. - // This is instead of asking the user to approve these scopes. Note that `NewAuthorizeRequest` would have returned - // 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) - - return authorizeRequester, true + return username, password, nil } func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken { @@ -526,6 +566,7 @@ type browserFlowAuthRequestState struct { func handleBrowserFlowAuthRequest( r *http.Request, w http.ResponseWriter, + authorizeRequester fosite.AuthorizeRequester, oauthHelper fosite.OAuth2Provider, generateCSRF func() (csrftoken.CSRFToken, error), generateNonce func() (nonce.Nonce, error), @@ -534,15 +575,7 @@ func handleBrowserFlowAuthRequest( idpType psession.ProviderType, cookieCodec oidc.Codec, upstreamStateEncoder oidc.Encoder, - idpNameQueryParamValue string, ) (*browserFlowAuthRequestState, error) { - authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false) - if !created { - return nil, nil // already wrote the error response, don't return error - } - - maybeLogDeprecationWarningForMissingIDPParam(idpNameQueryParamValue, authorizeRequester) - now := time.Now() _, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{ Fosite: &openid.DefaultSession{ @@ -555,14 +588,13 @@ func handleBrowserFlowAuthRequest( }, }) if err != nil { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, err, false) - return nil, nil // already wrote the error response, don't return error + return nil, err } csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE) if err != nil { plog.Error("authorize generate error", err) - return nil, err + return nil, fosite.ErrServerError.WithHint("Server could not generate necessary values.").WithWrap(err) } csrfFromCookie := readCSRFCookie(r, cookieCodec) if csrfFromCookie != "" { @@ -580,13 +612,12 @@ func handleBrowserFlowAuthRequest( ) if err != nil { plog.Error("authorize upstream state param error", err) - return nil, err + return nil, fosite.ErrServerError.WithHint("Error encoding upstream state param.").WithWrap(err) } promptParam := r.Form.Get(promptParamName) if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, oidcapi.ScopeOpenID) { - oidc.WriteAuthorizeError(r, w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false) - return nil, nil // already wrote the error response, don't return error + return nil, fosite.ErrLoginRequired } if csrfFromCookie == "" { @@ -594,7 +625,7 @@ func handleBrowserFlowAuthRequest( err = addCSRFSetCookieHeader(w, csrfValue, cookieCodec) if err != nil { plog.Error("error setting CSRF cookie", err) - return nil, err + return nil, fosite.ErrServerError.WithHint("Error encoding CSRF cookie.").WithWrap(err) } } @@ -612,15 +643,15 @@ func generateValues( ) (csrftoken.CSRFToken, nonce.Nonce, pkce.Code, error) { csrfValue, err := generateCSRF() if err != nil { - return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating CSRF token", err) + return "", "", "", fmt.Errorf("error generating CSRF token: %w", err) } nonceValue, err := generateNonce() if err != nil { - return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating nonce param", err) + return "", "", "", fmt.Errorf("error generating nonce param: %w", err) } pkceValue, err := generatePKCE() if err != nil { - return "", "", "", httperr.Wrap(http.StatusInternalServerError, "error generating PKCE param", err) + return "", "", "", fmt.Errorf("error generating PKCE param: %w", err) } return csrfValue, nonceValue, pkceValue, nil } @@ -650,7 +681,7 @@ func upstreamStateParam( } encodedStateParamValue, err := encoder.Encode(oidc.UpstreamStateParamEncodingName, stateParamData) if err != nil { - return "", httperr.Wrap(http.StatusInternalServerError, "error encoding upstream state param", err) + return "", fmt.Errorf("error encoding upstream state param: %w", err) } return encodedStateParamValue, nil } @@ -670,7 +701,7 @@ func removeCustomIDPParams(params url.Values) url.Values { func addCSRFSetCookieHeader(w http.ResponseWriter, csrfValue csrftoken.CSRFToken, codec oidc.Encoder) error { encodedCSRFValue, err := codec.Encode(oidc.CSRFCookieEncodingName, csrfValue) if err != nil { - return httperr.Wrap(http.StatusInternalServerError, "error encoding CSRF cookie", err) + return fmt.Errorf("error encoding CSRF cookie: %w", err) } http.SetCookie(w, &http.Cookie{ diff --git a/internal/federationdomain/endpoints/auth/auth_handler_test.go b/internal/federationdomain/endpoints/auth/auth_handler_test.go index dc847169d..d08558185 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler_test.go +++ b/internal/federationdomain/endpoints/auth/auth_handler_test.go @@ -236,6 +236,23 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo "error_description": "The Authorization Server requires End-User authentication.", "state": happyState, } + + fositeUpstreamAuthErrorQuery = map[string]string{ + "error": "error", + "error_description": "Unexpected error during upstream authentication.", + "state": happyState, + } + + fositeInternalServerErrorQueryWithHint = func(hint string) map[string]string { + return map[string]string{ + "error": "server_error", + "error_description": fmt.Sprintf( + "The authorization server encountered an unexpected condition that prevented it from fulfilling the request. %s", + hint, + ), + "state": happyState, + } + } ) hmacSecretFunc := func() []byte { return []byte("some secret - must have at least 32 bytes") } @@ -1569,9 +1586,10 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo path: happyGetRequestPathForLDAPUpstream, customUsernameHeader: ptr.To(happyLDAPUsername), customPasswordHeader: ptr.To(happyLDAPPassword), - wantStatus: http.StatusBadGateway, - wantContentType: htmlContentType, - wantBodyString: "Bad Gateway: unexpected error during upstream authentication\n", + wantStatus: http.StatusFound, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUpstreamAuthErrorQuery), + wantBodyString: "", }, { name: "error during upstream Active Directory authentication", @@ -1580,9 +1598,10 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo path: happyGetRequestPathForLDAPUpstream, customUsernameHeader: ptr.To(happyLDAPUsername), customPasswordHeader: ptr.To(happyLDAPPassword), - wantStatus: http.StatusBadGateway, - wantContentType: htmlContentType, - wantBodyString: "Bad Gateway: unexpected error during upstream authentication\n", + wantStatus: http.StatusFound, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeUpstreamAuthErrorQuery), + wantBodyString: "", }, { name: "wrong upstream credentials for OIDC password grant authentication", @@ -3232,74 +3251,79 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo wantBodyString: "", }, { - name: "error while encoding upstream state param using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), - generateCSRF: happyCSRFGenerator, - generatePKCE: happyPKCEGenerator, - generateNonce: happyNonceGenerator, - stateEncoder: &errorReturningEncoder{}, - cookieEncoder: happyCookieEncoder, - method: http.MethodGet, - path: happyGetRequestPathForOIDCUpstream, - wantStatus: http.StatusInternalServerError, - wantContentType: plainContentType, - wantBodyString: "Internal Server Error: error encoding upstream state param\n", + name: "error while encoding upstream state param using OIDC upstream browser flow", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), + generateCSRF: happyCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: &errorReturningEncoder{}, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: happyGetRequestPathForOIDCUpstream, + wantStatus: http.StatusSeeOther, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInternalServerErrorQueryWithHint("Error encoding upstream state param.")), + wantBodyString: "", }, { - name: "error while encoding CSRF cookie value for new cookie using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), - generateCSRF: happyCSRFGenerator, - generatePKCE: happyPKCEGenerator, - generateNonce: happyNonceGenerator, - stateEncoder: happyStateEncoder, - cookieEncoder: &errorReturningEncoder{}, - method: http.MethodGet, - path: happyGetRequestPathForOIDCUpstream, - wantStatus: http.StatusInternalServerError, - wantContentType: plainContentType, - wantBodyString: "Internal Server Error: error encoding CSRF cookie\n", + name: "error while encoding CSRF cookie value for new cookie using OIDC upstream browser flow", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), + generateCSRF: happyCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: &errorReturningEncoder{}, + method: http.MethodGet, + path: happyGetRequestPathForOIDCUpstream, + wantStatus: http.StatusSeeOther, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInternalServerErrorQueryWithHint("Error encoding CSRF cookie.")), + wantBodyString: "", }, { - name: "error while generating CSRF token using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), - generateCSRF: sadCSRFGenerator, - generatePKCE: happyPKCEGenerator, - generateNonce: happyNonceGenerator, - stateEncoder: happyStateEncoder, - cookieEncoder: happyCookieEncoder, - method: http.MethodGet, - path: happyGetRequestPathForOIDCUpstream, - wantStatus: http.StatusInternalServerError, - wantContentType: plainContentType, - wantBodyString: "Internal Server Error: error generating CSRF token\n", + name: "error while generating CSRF token using OIDC upstream browser flow", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), + generateCSRF: sadCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: happyGetRequestPathForOIDCUpstream, + wantStatus: http.StatusSeeOther, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInternalServerErrorQueryWithHint("Server could not generate necessary values.")), + wantBodyString: "", }, { - name: "error while generating nonce using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), - generateCSRF: happyCSRFGenerator, - generatePKCE: happyPKCEGenerator, - generateNonce: sadNonceGenerator, - stateEncoder: happyStateEncoder, - cookieEncoder: happyCookieEncoder, - method: http.MethodGet, - path: happyGetRequestPathForOIDCUpstream, - wantStatus: http.StatusInternalServerError, - wantContentType: plainContentType, - wantBodyString: "Internal Server Error: error generating nonce param\n", + name: "error while generating nonce using OIDC upstream browser flow", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), + generateCSRF: happyCSRFGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: sadNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: happyGetRequestPathForOIDCUpstream, + wantStatus: http.StatusSeeOther, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInternalServerErrorQueryWithHint("Server could not generate necessary values.")), + wantBodyString: "", }, { - name: "error while generating PKCE using OIDC upstream browser flow", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), - generateCSRF: happyCSRFGenerator, - generatePKCE: sadPKCEGenerator, - generateNonce: happyNonceGenerator, - stateEncoder: happyStateEncoder, - cookieEncoder: happyCookieEncoder, - method: http.MethodGet, - path: happyGetRequestPathForOIDCUpstream, - wantStatus: http.StatusInternalServerError, - wantContentType: plainContentType, - wantBodyString: "Internal Server Error: error generating PKCE param\n", + name: "error while generating PKCE using OIDC upstream browser flow", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), + generateCSRF: happyCSRFGenerator, + generatePKCE: sadPKCEGenerator, + generateNonce: happyNonceGenerator, + stateEncoder: happyStateEncoder, + cookieEncoder: happyCookieEncoder, + method: http.MethodGet, + path: happyGetRequestPathForOIDCUpstream, + wantStatus: http.StatusSeeOther, + wantContentType: jsonContentType, + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeInternalServerErrorQueryWithHint("Server could not generate necessary values.")), + wantBodyString: "", }, { name: "no default upstream provider is configured and no specific IDP was requested in the request params", diff --git a/internal/httputil/responseutil/responseutil.go b/internal/httputil/responseutil/responseutil.go new file mode 100644 index 000000000..17ea981e6 --- /dev/null +++ b/internal/httputil/responseutil/responseutil.go @@ -0,0 +1,16 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package responseutil + +import ( + "fmt" + "net/http" +) + +func HTTPErrorf(w http.ResponseWriter, code int, errorFmt string, a ...any) { + http.Error(w, + fmt.Sprintf("%s: %s", http.StatusText(code), fmt.Sprintf(errorFmt, a...)), + code, + ) +}