Expose the Supervisor OIDC authorization endpoint to the public

cmd/pinniped-supervisor/main.go

@@ -181,9 +181,10 @@ func run(serverInstallationNamespace string, cfg *supervisor.Config) error {
 
 	dynamicJWKSProvider := jwks.NewDynamicJWKSProvider()
 	dynamicTLSCertProvider := provider.NewDynamicTLSCertProvider()
+	dynamicUpstreamIDPProvider := provider.NewDynamicUpstreamIDPProvider()
 
 	// OIDC endpoints will be served by the oidProvidersManager, and any non-OIDC paths will fallback to the healthMux.
-	oidProvidersManager := manager.NewManager(healthMux, dynamicJWKSProvider)
+	oidProvidersManager := manager.NewManager(healthMux, dynamicJWKSProvider, dynamicUpstreamIDPProvider)
 
 	startControllers(
 		ctx,

internal/oidc/auth/auth_handler.go

@@ -44,23 +44,27 @@ func NewHandler(
 
 		authorizeRequester, err := oauthHelper.NewAuthorizeRequest(r.Context(), r)
 		if err != nil {
+			klog.InfoS("authorize request error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues())
 			oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
 			return nil
 		}
 
 		upstreamIDP, err := chooseUpstreamIDP(idpListGetter)
 		if err != nil {
+			klog.InfoS("authorize request upstream selection error", "err", err)
 			return err
 		}
 
 		_, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{})
 		if err != nil {
+			klog.InfoS("authorize response error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues())
 			oauthHelper.WriteAuthorizeError(w, authorizeRequester, err)
 			return nil
 		}
 
 		stateValue, nonceValue, pkceValue, err := generateParams(generateState, generateNonce, generatePKCE)
 		if err != nil {
+			klog.InfoS("authorize generate error", "err", err)
 			return err
 		}
 

internal/oidc/oidc.go

@@ -29,8 +29,7 @@ func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient {
 	}
 }
 
-// Note that Fosite requires the HMAC secret to be 32 bytes.
+func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLengthAtLeast32 []byte) fosite.OAuth2Provider {
-func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLength32 []byte) fosite.OAuth2Provider {
 	oauthConfig := &compose.Config{
 		EnforcePKCEForPublicClients: true,
 	}
@@ -39,7 +38,8 @@ func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLength32 []byte) fos
 		oauthConfig,
 		oauthStore,
 		&compose.CommonStrategy{
-			CoreStrategy: compose.NewOAuth2HMACStrategy(oauthConfig, hmacSecretOfLength32, nil),
+			// Note that Fosite requires the HMAC secret to be at least 32 bytes.
+			CoreStrategy: compose.NewOAuth2HMACStrategy(oauthConfig, hmacSecretOfLengthAtLeast32, nil),
 		},
 		nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets.
 		compose.OAuth2AuthorizeExplicitFactory,

internal/oidc/provider/manager/manager.go

@@ -8,12 +8,18 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/storage"
 	"k8s.io/klog/v2"
 
 	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/auth"
 	"go.pinniped.dev/internal/oidc/discovery"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/oidc/provider"
+	"go.pinniped.dev/internal/oidcclient/nonce"
+	"go.pinniped.dev/internal/oidcclient/pkce"
+	"go.pinniped.dev/internal/oidcclient/state"
 )
 
 // Manager can manage multiple active OIDC providers. It acts as a request router for them.
@@ -25,16 +31,19 @@ type Manager struct {
 	providerHandlers    map[string]http.Handler  // map of all routes for all providers
 	nextHandler         http.Handler             // the next handler in a chain, called when this manager didn't know how to handle a request
 	dynamicJWKSProvider jwks.DynamicJWKSProvider // in-memory cache of per-issuer JWKS data
+	idpListGetter       auth.IDPListGetter       // in-memory cache of upstream IDPs
 }
 
 // NewManager returns an empty Manager.
 // nextHandler will be invoked for any requests that could not be handled by this manager's providers.
 // dynamicJWKSProvider will be used as an in-memory cache for per-issuer JWKS data.
-func NewManager(nextHandler http.Handler, dynamicJWKSProvider jwks.DynamicJWKSProvider) *Manager {
+// idpListGetter will be used as an in-memory cache of currently configured upstream IDPs.
+func NewManager(nextHandler http.Handler, dynamicJWKSProvider jwks.DynamicJWKSProvider, idpListGetter auth.IDPListGetter) *Manager {
 	return &Manager{
 		providerHandlers:    make(map[string]http.Handler),
 		nextHandler:         nextHandler,
 		dynamicJWKSProvider: dynamicJWKSProvider,
+		idpListGetter:       idpListGetter,
 	}
 }
 
@@ -60,6 +69,17 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) {
 		jwksURL := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath() + oidc.JWKSEndpointPath
 		m.providerHandlers[jwksURL] = jwks.NewHandler(incomingProvider.Issuer(), m.dynamicJWKSProvider)
 
+		// Each OIDC provider gets its own storage.
+		oauthStore := &storage.MemoryStore{
+			Clients:        map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()},
+			AuthorizeCodes: map[string]storage.StoreAuthorizeCode{},
+			PKCES:          map[string]fosite.Requester{},
+		}
+		oauthHelper := oidc.FositeOauth2Helper(oauthStore, []byte("some secret - must have at least 32 bytes")) // TODO replace this secret
+
+		authURL := strings.ToLower(incomingProvider.IssuerHost()) + "/" + incomingProvider.IssuerPath() + oidc.AuthorizationEndpointPath
+		m.providerHandlers[authURL] = auth.NewHandler(incomingProvider.Issuer(), m.idpListGetter, oauthHelper, state.Generate, pkce.Generate, nonce.Generate)
+
 		klog.InfoS("oidc provider manager added or updated issuer", "issuer", incomingProvider.Issuer())
 	}
 }

internal/oidc/provider/manager/manager_test.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"strings"
 	"testing"
 
@@ -32,12 +33,15 @@ func TestManager(t *testing.T) {
 			dynamicJWKSProvider      jwks.DynamicJWKSProvider
 		)
 
-		issuer1 := "https://example.com/some/path"
+		const (
-		issuer1DifferentCaseHostname := "https://eXamPle.coM/some/path"
+			issuer1                      = "https://example.com/some/path"
-		issuer1KeyID := "issuer1-key"
+			issuer1DifferentCaseHostname = "https://eXamPle.coM/some/path"
-		issuer2 := "https://example.com/some/path/more/deeply/nested/path" // note that this is a sub-path of the other issuer url
+			issuer1KeyID                 = "issuer1-key"
-		issuer2DifferentCaseHostname := "https://exAmPlE.Com/some/path/more/deeply/nested/path"
+			issuer2                      = "https://example.com/some/path/more/deeply/nested/path" // note that this is a sub-path of the other issuer url
-		issuer2KeyID := "issuer2-key"
+			issuer2DifferentCaseHostname = "https://exAmPlE.Com/some/path/more/deeply/nested/path"
+			issuer2KeyID                 = "issuer2-key"
+			upstreamIDPAuthorizationURL  = "https://test-upstream.com/auth"
+		)
 
 		newGetRequest := func(url string) *http.Request {
 			return httptest.NewRequest(http.MethodGet, url, nil)
@@ -50,17 +54,33 @@ func TestManager(t *testing.T) {
 
 			r.False(fallbackHandlerWasCalled)
 
+			// Minimal check to ensure that the right discovery endpoint was called
 			r.Equal(http.StatusOK, recorder.Code)
 			responseBody, err := ioutil.ReadAll(recorder.Body)
 			r.NoError(err)
 			parsedDiscoveryResult := discovery.Metadata{}
 			err = json.Unmarshal(responseBody, &parsedDiscoveryResult)
 			r.NoError(err)
-
-			// Minimal check to ensure that the right discovery endpoint was called
 			r.Equal(expectedIssuerInResponse, parsedDiscoveryResult.Issuer)
 		}
 
+		requireAuthorizationRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedRedirectLocationPrefix string) {
+			recorder := httptest.NewRecorder()
+
+			subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.AuthorizationEndpointPath+requestURLSuffix))
+
+			r.False(fallbackHandlerWasCalled)
+
+			// Minimal check to ensure that the right endpoint was called
+			r.Equal(http.StatusFound, recorder.Code)
+			actualLocation := recorder.Header().Get("Location")
+			r.True(
+				strings.HasPrefix(actualLocation, expectedRedirectLocationPrefix),
+				"actual location %s did not start with expected prefix %s",
+				actualLocation, expectedRedirectLocationPrefix,
+			)
+		}
+
 		requireJWKSRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedJWKKeyID string) {
 			recorder := httptest.NewRecorder()
 
@@ -68,14 +88,13 @@ func TestManager(t *testing.T) {
 
 			r.False(fallbackHandlerWasCalled)
 
+			// Minimal check to ensure that the right JWKS endpoint was called
 			r.Equal(http.StatusOK, recorder.Code)
 			responseBody, err := ioutil.ReadAll(recorder.Body)
 			r.NoError(err)
 			parsedJWKSResult := jose.JSONWebKeySet{}
 			err = json.Unmarshal(responseBody, &parsedJWKSResult)
 			r.NoError(err)
-
-			// Minimal check to ensure that the right JWKS endpoint was called
 			r.Equal(expectedJWKKeyID, parsedJWKSResult.Keys[0].KeyID)
 		}
 
@@ -85,7 +104,20 @@ func TestManager(t *testing.T) {
 				fallbackHandlerWasCalled = true
 			}
 			dynamicJWKSProvider = jwks.NewDynamicJWKSProvider()
-			subject = NewManager(nextHandler, dynamicJWKSProvider)
+
+			parsedUpstreamIDPAuthorizationURL, err := url.Parse(upstreamIDPAuthorizationURL)
+			r.NoError(err)
+			idpListGetter := provider.NewDynamicUpstreamIDPProvider()
+			idpListGetter.SetIDPList([]provider.UpstreamOIDCIdentityProvider{
+				{
+					Name:             "test-idp",
+					ClientID:         "test-client-id",
+					AuthorizationURL: *parsedUpstreamIDPAuthorizationURL,
+					Scopes:           []string{"test-scope"},
+				},
+			})
+
+			subject = NewManager(nextHandler, dynamicJWKSProvider, idpListGetter)
 		})
 
 		when("given no providers via SetProviders()", func() {
@@ -113,6 +145,45 @@ func TestManager(t *testing.T) {
 			return k
 		}
 
+		requireRoutesMatchingRequestsToAppropriateProvider := func() {
+			requireDiscoveryRequestToBeHandled(issuer1, "", issuer1)
+			requireDiscoveryRequestToBeHandled(issuer2, "", issuer2)
+			requireDiscoveryRequestToBeHandled(issuer2, "?some=query", issuer2)
+
+			// Hostnames are case-insensitive, so test that we can handle that.
+			requireDiscoveryRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1)
+			requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2)
+			requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2)
+
+			requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID)
+			requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID)
+			requireJWKSRequestToBeHandled(issuer2, "?some=query", issuer2KeyID)
+
+			// Hostnames are case-insensitive, so test that we can handle that.
+			requireJWKSRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1KeyID)
+			requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2KeyID)
+			requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2KeyID)
+
+			authRedirectURI := "http://127.0.0.1/callback"
+			authRequestParams := "?" + url.Values{
+				"response_type":         []string{"code"},
+				"scope":                 []string{"openid profile email"},
+				"client_id":             []string{"pinniped-cli"},
+				"state":                 []string{"some-state-value"},
+				"nonce":                 []string{"some-nonce-value"},
+				"code_challenge":        []string{"some-challenge"},
+				"code_challenge_method": []string{"S256"},
+				"redirect_uri":          []string{authRedirectURI},
+			}.Encode()
+
+			requireAuthorizationRequestToBeHandled(issuer1, authRequestParams, upstreamIDPAuthorizationURL)
+			requireAuthorizationRequestToBeHandled(issuer2, authRequestParams, upstreamIDPAuthorizationURL)
+
+			// Hostnames are case-insensitive, so test that we can handle that.
+			requireAuthorizationRequestToBeHandled(issuer1DifferentCaseHostname, authRequestParams, upstreamIDPAuthorizationURL)
+			requireAuthorizationRequestToBeHandled(issuer2DifferentCaseHostname, authRequestParams, upstreamIDPAuthorizationURL)
+		}
+
 		when("given some valid providers via SetProviders()", func() {
 			it.Before(func() {
 				p1, err := provider.NewOIDCProvider(issuer1)
@@ -129,8 +200,8 @@ func TestManager(t *testing.T) {
 
 			it("sends all non-matching host requests to the nextHandler", func() {
 				r.False(fallbackHandlerWasCalled)
-				url := strings.ReplaceAll(issuer1+oidc.WellKnownEndpointPath, "example.com", "wrong-host.com")
+				wrongHostURL := strings.ReplaceAll(issuer1+oidc.WellKnownEndpointPath, "example.com", "wrong-host.com")
-				subject.ServeHTTP(httptest.NewRecorder(), newGetRequest(url))
+				subject.ServeHTTP(httptest.NewRecorder(), newGetRequest(wrongHostURL))
 				r.True(fallbackHandlerWasCalled)
 			})
 
@@ -147,23 +218,7 @@ func TestManager(t *testing.T) {
 			})
 
 			it("routes matching requests to the appropriate provider", func() {
-				requireDiscoveryRequestToBeHandled(issuer1, "", issuer1)
+				requireRoutesMatchingRequestsToAppropriateProvider()
-				requireDiscoveryRequestToBeHandled(issuer2, "", issuer2)
-				requireDiscoveryRequestToBeHandled(issuer2, "?some=query", issuer2)
-
-				// Hostnames are case-insensitive, so test that we can handle that.
-				requireDiscoveryRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1)
-				requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2)
-				requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2)
-
-				requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID)
-				requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID)
-				requireJWKSRequestToBeHandled(issuer2, "?some=query", issuer2KeyID)
-
-				// Hostnames are case-insensitive, so test that we can handle that.
-				requireJWKSRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1KeyID)
-				requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2KeyID)
-				requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2KeyID)
 			})
 		})
 
@@ -182,23 +237,7 @@ func TestManager(t *testing.T) {
 			})
 
 			it("still routes matching requests to the appropriate provider", func() {
-				requireDiscoveryRequestToBeHandled(issuer1, "", issuer1)
+				requireRoutesMatchingRequestsToAppropriateProvider()
-				requireDiscoveryRequestToBeHandled(issuer2, "", issuer2)
-				requireDiscoveryRequestToBeHandled(issuer2, "?some=query", issuer2)
-
-				// Hostnames are case-insensitive, so test that we can handle that.
-				requireDiscoveryRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1)
-				requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2)
-				requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2)
-
-				requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID)
-				requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID)
-				requireJWKSRequestToBeHandled(issuer2, "?some=query", issuer2KeyID)
-
-				// Hostnames are case-insensitive, so test that we can handle that.
-				requireJWKSRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1KeyID)
-				requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2KeyID)
-				requireJWKSRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2KeyID)
 			})
 		})
 	})