Refactor error handling in authorize endpoint (changes some responses)

- Simplify the error handling in the authorize endpoint by making the
  private helper functions return fosite-style errors, and having
  one place that writes those errors to the response.
- Some types of errors were previously returned as regular http-style
  errors. Those have all been converted to be returned as oauth-style
  errors (which can be redirects to the client), except for http method
  not found errors. This is a change in behavior from the client's point
  of view, but only when those unexpected errors happen. These types of
  errors are more consistent with RFC6749 section 4.1.2.1.
- Avoids using the httperr package for error handling.
- Create a struct for the handler as a first step toward making smaller
  functions with fewer parameters.

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{

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",

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,
+	)
+}