Merge pull request #689 from vmware-tanzu/trivial_refactor

Extract some trivial helpers for identical code usages

internal/oidc/auth/auth_handler.go

@@ -21,6 +21,7 @@ import (
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/csrftoken"
+	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
@@ -109,18 +110,11 @@ func handleAuthRequestForLDAPUpstream(
 		return nil
 	}
 
-	now := time.Now().UTC()
+	openIDSession := downstreamsession.MakeDownstreamSession(
-	openIDSession := &openid.DefaultSession{
+		downstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse),
-		Claims: &jwt.IDTokenClaims{
+		authenticateResponse.User.GetName(),
-			Subject:     downstreamSubjectFromUpstreamLDAP(ldapUpstream, authenticateResponse),
+		authenticateResponse.User.GetGroups(),
-			RequestedAt: now,
+	)
-			AuthTime:    now,
-		},
-	}
-	openIDSession.Claims.Extra = map[string]interface{}{
-		oidc.DownstreamUsernameClaim: authenticateResponse.User.GetName(),
-		oidc.DownstreamGroupsClaim:   authenticateResponse.User.GetGroups(),
-	}
 
 	authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
 	if err != nil {
@@ -130,6 +124,7 @@ func handleAuthRequestForLDAPUpstream(
 	}
 
 	oauthHelper.WriteAuthorizeResponse(w, authorizeRequester, authorizeResponder)
+
 	return nil
 }
 
@@ -236,18 +231,14 @@ func newAuthorizeRequest(r *http.Request, w http.ResponseWriter, oauthHelper fos
 		oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
 		return nil, false
 	}
-	grantScopes(authorizeRequester)
-	return authorizeRequester, true
-}
 
-func grantScopes(authorizeRequester fosite.AuthorizeRequester) {
+	// Automatically grant the openid, offline_access, and pinniped:request-audience 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.
-	oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
 	// 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.
-	oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
+	downstreamsession.GrantScopesIfRequested(authorizeRequester)
-	// Grant the pinniped:request-audience scope if requested.
+
-	oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
+	return authorizeRequester, true
 }
 
 func readCSRFCookie(r *http.Request, codec oidc.Decoder) csrftoken.CSRFToken {

internal/oidc/callback/callback_handler.go

@@ -9,17 +9,14 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"time"
 
-	coreosoidc "github.com/coreos/go-oidc/v3/oidc"
 	"github.com/ory/fosite"
-	"github.com/ory/fosite/handler/openid"
-	"github.com/ory/fosite/token/jwt"
 
 	"go.pinniped.dev/internal/httputil/httperr"
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/csrftoken"
+	"go.pinniped.dev/internal/oidc/downstreamsession"
 	"go.pinniped.dev/internal/oidc/provider"
 	"go.pinniped.dev/internal/plog"
 )
@@ -65,9 +62,7 @@ func NewHandler(
 		}
 
 		// Automatically grant the openid, offline_access, and pinniped:request-audience scopes, but only if they were requested.
-		oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID)
+		downstreamsession.GrantScopesIfRequested(authorizeRequester)
-		oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOfflineAccess)
-		oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
 
 		token, err := upstreamIDPConfig.ExchangeAuthcodeAndValidateTokens(
 			r.Context(),
@@ -91,7 +86,8 @@ func NewHandler(
 			return err
 		}
 
-		openIDSession := makeDownstreamSession(subject, username, groups)
+		openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups)
+
 		authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession)
 		if err != nil {
 			plog.WarningErr("error while generating and saving authcode", err, "upstreamName", upstreamIDPConfig.GetName())
@@ -347,22 +343,3 @@ func extractGroups(groupsAsInterface interface{}) ([]string, bool) {
 
 	return groupsAsStrings, true
 }
-
-func makeDownstreamSession(subject string, username string, groups []string) *openid.DefaultSession {
-	now := time.Now().UTC()
-	openIDSession := &openid.DefaultSession{
-		Claims: &jwt.IDTokenClaims{
-			Subject:     subject,
-			RequestedAt: now,
-			AuthTime:    now,
-		},
-	}
-	if groups == nil {
-		groups = []string{}
-	}
-	openIDSession.Claims.Extra = map[string]interface{}{
-		oidc.DownstreamUsernameClaim: username,
-		oidc.DownstreamGroupsClaim:   groups,
-	}
-	return openIDSession
-}

internal/oidc/downstreamsession/downstream_session.go

@@ -0,0 +1,43 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package downstreamsession provides some shared helpers for creating downstream OIDC sessions.
+package downstreamsession
+
+import (
+	"time"
+
+	oidc2 "github.com/coreos/go-oidc/v3/oidc"
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/handler/openid"
+	"github.com/ory/fosite/token/jwt"
+
+	"go.pinniped.dev/internal/oidc"
+)
+
+// MakeDownstreamSession creates a downstream OIDC session.
+func MakeDownstreamSession(subject string, username string, groups []string) *openid.DefaultSession {
+	now := time.Now().UTC()
+	openIDSession := &openid.DefaultSession{
+		Claims: &jwt.IDTokenClaims{
+			Subject:     subject,
+			RequestedAt: now,
+			AuthTime:    now,
+		},
+	}
+	if groups == nil {
+		groups = []string{}
+	}
+	openIDSession.Claims.Extra = map[string]interface{}{
+		oidc.DownstreamUsernameClaim: username,
+		oidc.DownstreamGroupsClaim:   groups,
+	}
+	return openIDSession
+}
+
+// GrantScopesIfRequested auto-grants the scopes for which we do not require end-user approval, if they were requested.
+func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester) {
+	oidc.GrantScopeIfRequested(authorizeRequester, oidc2.ScopeOpenID)
+	oidc.GrantScopeIfRequested(authorizeRequester, oidc2.ScopeOfflineAccess)
+	oidc.GrantScopeIfRequested(authorizeRequester, "pinniped:request-audience")
+}