Some refactoring of shared code between OIDC and LDAP browser flows

Signed-off-by: Margo Crawford <margaretc@vmware.com>

internal/oidc/auth/auth_handler.go

@@ -172,62 +172,23 @@ func handleAuthRequestForLDAPUpstreamBrowserFlow(
 	upstreamStateEncoder oidc.Encoder,
 	cookieCodec oidc.Codec,
 ) error {
-	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false)
+	encodedStateParamValue, _, _, err := handleBrowserAuthRequest(
-	if !created {
+		r,
-		return nil
+		w,
-	}
+		oauthHelper,
-
+		generateCSRF,
-	now := time.Now()
+		generateNonce,
-	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{
+		generatePKCE,
-		Fosite: &openid.DefaultSession{
-			Claims: &jwt.IDTokenClaims{
-				// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
-				Subject:     "none",
-				AuthTime:    now,
-				RequestedAt: now,
-			},
-		},
-	})
-	if err != nil {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, err, false)
-	}
-
-	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
-	if err != nil {
-		plog.Error("authorize generate error", err)
-		return err
-	}
-	csrfFromCookie := readCSRFCookie(r, cookieCodec)
-	if csrfFromCookie != "" {
-		csrfValue = csrfFromCookie
-	}
-
-	encodedStateParamValue, err := upstreamStateParam(
-		authorizeRequester,
 		ldapUpstream.GetName(),
-		string(idpType),
+		idpType,
-		nonceValue,
+		cookieCodec,
-		csrfValue,
-		pkceValue,
 		upstreamStateEncoder,
 	)
 	if err != nil {
-		plog.Error("authorize upstream state param error", err)
 		return err
 	}
-
+	if encodedStateParamValue == "" {
-	promptParam := r.Form.Get(promptParamName)
+		return nil
-	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
-	}
-
-	if csrfFromCookie == "" {
-		// We did not receive an incoming CSRF cookie, so write a new one.
-		err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec)
-		if err != nil {
-			plog.Error("error setting CSRF cookie", err)
-			return err
-		}
 	}
 
 	loginURL, err := url.Parse(downstreamIssuer + "/login")
@@ -312,34 +273,23 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
 	upstreamStateEncoder oidc.Encoder,
 	cookieCodec oidc.Codec,
 ) error {
-	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false)
+	encodedStateParamValue, pkceValue, nonceValue, err := handleBrowserAuthRequest(
-	if !created {
+		r,
-		return nil
+		w,
-	}
+		oauthHelper,
-
+		generateCSRF,
-	now := time.Now()
+		generateNonce,
-	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{
+		generatePKCE,
-		Fosite: &openid.DefaultSession{
+		oidcUpstream.GetName(),
-			Claims: &jwt.IDTokenClaims{
+		psession.ProviderTypeOIDC,
-				// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
+		cookieCodec,
-				Subject:     "none",
+		upstreamStateEncoder,
-				AuthTime:    now,
+	)
-				RequestedAt: now,
-			},
-		},
-	})
 	if err != nil {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, err, false)
-	}
-
-	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
-	if err != nil {
-		plog.Error("authorize generate error", err)
 		return err
 	}
-	csrfFromCookie := readCSRFCookie(r, cookieCodec)
+	if encodedStateParamValue == "" {
-	if csrfFromCookie != "" {
+		return nil
-		csrfValue = csrfFromCookie
 	}
 
 	upstreamOAuthConfig := oauth2.Config{
@@ -351,44 +301,16 @@ func handleAuthRequestForOIDCUpstreamAuthcodeGrant(
 		Scopes:      oidcUpstream.GetScopes(),
 	}
 
-	encodedStateParamValue, err := upstreamStateParam(
-		authorizeRequester,
-		oidcUpstream.GetName(),
-		string(psession.ProviderTypeOIDC),
-		nonceValue,
-		csrfValue,
-		pkceValue,
-		upstreamStateEncoder,
-	)
-	if err != nil {
-		plog.Error("authorize upstream state param error", err)
-		return err
-	}
-
 	authCodeOptions := []oauth2.AuthCodeOption{
 		nonceValue.Param(),
 		pkceValue.Challenge(),
 		pkceValue.Method(),
 	}
 
-	promptParam := r.Form.Get(promptParamName)
-	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
-		return writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
-	}
-
 	for key, val := range oidcUpstream.GetAdditionalAuthcodeParams() {
 		authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(key, val))
 	}
 
-	if csrfFromCookie == "" {
-		// We did not receive an incoming CSRF cookie, so write a new one.
-		err := addCSRFSetCookieHeader(w, csrfValue, cookieCodec)
-		if err != nil {
-			plog.Error("error setting CSRF cookie", err)
-			return err
-		}
-	}
-
 	http.Redirect(w, r,
 		upstreamOAuthConfig.AuthCodeURL(
 			encodedStateParamValue,
@@ -549,6 +471,81 @@ func chooseUpstreamIDP(idpLister oidc.UpstreamIdentityProvidersLister) (provider
 	}
 }
 
+// handleBrowserAuthRequest performs the shared validations and setup between browser based auth requests
+// regardless of IDP type-- LDAP, Active Directory and OIDC.
+// It generates the state param, sets the CSRF cookie, and validates the prompt param.
+func handleBrowserAuthRequest(
+	r *http.Request,
+	w http.ResponseWriter,
+	oauthHelper fosite.OAuth2Provider,
+	generateCSRF func() (csrftoken.CSRFToken, error),
+	generateNonce func() (nonce.Nonce, error),
+	generatePKCE func() (pkce.Code, error),
+	upstreamName string,
+	idpType psession.ProviderType,
+	cookieCodec oidc.Codec,
+	upstreamStateEncoder oidc.Encoder,
+) (string, pkce.Code, nonce.Nonce, error) {
+	authorizeRequester, created := newAuthorizeRequest(r, w, oauthHelper, false)
+	if !created {
+		return "", "", "", nil
+	}
+
+	now := time.Now()
+	_, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &psession.PinnipedSession{
+		Fosite: &openid.DefaultSession{
+			Claims: &jwt.IDTokenClaims{
+				// Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations.
+				Subject:     "none",
+				AuthTime:    now,
+				RequestedAt: now,
+			},
+		},
+	})
+	if err != nil {
+		return "", "", "", writeAuthorizeError(w, oauthHelper, authorizeRequester, err, false)
+	}
+
+	csrfValue, nonceValue, pkceValue, err := generateValues(generateCSRF, generateNonce, generatePKCE)
+	if err != nil {
+		plog.Error("authorize generate error", err)
+		return "", "", "", err
+	}
+	csrfFromCookie := readCSRFCookie(r, cookieCodec)
+	if csrfFromCookie != "" {
+		csrfValue = csrfFromCookie
+	}
+
+	encodedStateParamValue, err := upstreamStateParam(
+		authorizeRequester,
+		upstreamName,
+		string(idpType),
+		nonceValue,
+		csrfValue,
+		pkceValue,
+		upstreamStateEncoder,
+	)
+	if err != nil {
+		plog.Error("authorize upstream state param error", err)
+		return "", "", "", err
+	}
+
+	promptParam := r.Form.Get(promptParamName)
+	if promptParam == promptParamNone && oidc.ScopeWasRequested(authorizeRequester, coreosoidc.ScopeOpenID) {
+		return "", "", "", writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrLoginRequired, false)
+	}
+
+	if csrfFromCookie == "" {
+		// We did not receive an incoming CSRF cookie, so write a new one.
+		err = addCSRFSetCookieHeader(w, csrfValue, cookieCodec)
+		if err != nil {
+			plog.Error("error setting CSRF cookie", err)
+			return "", "", "", err
+		}
+	}
+	return encodedStateParamValue, pkceValue, nonceValue, nil
+}
+
 func generateValues(
 	generateCSRF func() (csrftoken.CSRFToken, error),
 	generateNonce func() (nonce.Nonce, error),