From 4bd83add354fee8800894018d6c646d3c7679c69 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 28 Apr 2021 13:14:21 -0700 Subject: [PATCH] Add Supervisor upstream IDP discovery on the server-side --- internal/oidc/discovery/discovery_handler.go | 74 ++++++++++++---- .../oidc/discovery/discovery_handler_test.go | 86 +++++++++++++++++-- internal/oidc/provider/manager/manager.go | 2 +- .../oidc/provider/manager/manager_test.go | 23 +++-- test/integration/supervisor_discovery_test.go | 3 +- 5 files changed, 149 insertions(+), 39 deletions(-) diff --git a/internal/oidc/discovery/discovery_handler.go b/internal/oidc/discovery/discovery_handler.go index b45c1042b..dabdda025 100644 --- a/internal/oidc/discovery/discovery_handler.go +++ b/internal/oidc/discovery/discovery_handler.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package discovery provides a handler for the OIDC discovery endpoint. @@ -8,10 +8,16 @@ import ( "bytes" "encoding/json" "net/http" + "sort" "go.pinniped.dev/internal/oidc" ) +const ( + idpDiscoveryTypeLDAP = "ldap" + idpDiscoveryTypeOIDC = "oidc" +) + // Metadata holds all fields (that we care about) from the OpenID Provider Metadata section in the // OpenID Connect Discovery specification: // https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3. @@ -37,33 +43,28 @@ type Metadata struct { ClaimsSupported []string `json:"claims_supported"` // ^^^ Optional ^^^ + + // vvv Custom vvv + + IDPs []IdentityProviderMetadata `json:"pinniped_idps"` + + // ^^^ Custom ^^^ +} + +type IdentityProviderMetadata struct { + Name string `json:"name"` + Type string `json:"type"` } // NewHandler returns an http.Handler that serves an OIDC discovery endpoint. -func NewHandler(issuerURL string) http.Handler { - oidcConfig := Metadata{ - Issuer: issuerURL, - AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath, - TokenEndpoint: issuerURL + oidc.TokenEndpointPath, - JWKSURI: issuerURL + oidc.JWKSEndpointPath, - ResponseTypesSupported: []string{"code"}, - SubjectTypesSupported: []string{"public"}, - IDTokenSigningAlgValuesSupported: []string{"ES256"}, - TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, - ScopesSupported: []string{"openid", "offline"}, - ClaimsSupported: []string{"groups"}, - } - - var b bytes.Buffer - encodeErr := json.NewEncoder(&b).Encode(&oidcConfig) - encodedMetadata := b.Bytes() - +func NewHandler(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvidersLister) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { http.Error(w, `Method not allowed (try GET)`, http.StatusMethodNotAllowed) return } + encodedMetadata, encodeErr := metadata(issuerURL, upstreamIDPs) if encodeErr != nil { http.Error(w, encodeErr.Error(), http.StatusInternalServerError) return @@ -76,3 +77,38 @@ func NewHandler(issuerURL string) http.Handler { } }) } + +func metadata(issuerURL string, upstreamIDPs oidc.UpstreamIdentityProvidersLister) ([]byte, error) { + oidcConfig := Metadata{ + Issuer: issuerURL, + AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath, + TokenEndpoint: issuerURL + oidc.TokenEndpointPath, + JWKSURI: issuerURL + oidc.JWKSEndpointPath, + ResponseTypesSupported: []string{"code"}, + SubjectTypesSupported: []string{"public"}, + IDTokenSigningAlgValuesSupported: []string{"ES256"}, + TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, + ScopesSupported: []string{"openid", "offline"}, + ClaimsSupported: []string{"groups"}, + IDPs: []IdentityProviderMetadata{}, + } + + // The cache of IDPs could change at any time, so always recalculate the list. + for _, provider := range upstreamIDPs.GetLDAPIdentityProviders() { + oidcConfig.IDPs = append(oidcConfig.IDPs, IdentityProviderMetadata{Name: provider.GetName(), Type: idpDiscoveryTypeLDAP}) + } + for _, provider := range upstreamIDPs.GetOIDCIdentityProviders() { + oidcConfig.IDPs = append(oidcConfig.IDPs, IdentityProviderMetadata{Name: provider.GetName(), Type: idpDiscoveryTypeOIDC}) + } + + // Nobody like an API that changes the results unnecessarily. :) + sort.SliceStable(oidcConfig.IDPs, func(i, j int) bool { + return oidcConfig.IDPs[i].Name < oidcConfig.IDPs[j].Name + }) + + var b bytes.Buffer + encodeErr := json.NewEncoder(&b).Encode(&oidcConfig) + encodedMetadata := b.Bytes() + + return encodedMetadata, encodeErr +} diff --git a/internal/oidc/discovery/discovery_handler_test.go b/internal/oidc/discovery/discovery_handler_test.go index f15c9a0c2..7236e5445 100644 --- a/internal/oidc/discovery/discovery_handler_test.go +++ b/internal/oidc/discovery/discovery_handler_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package discovery @@ -9,6 +9,10 @@ import ( "net/http/httptest" "testing" + "go.pinniped.dev/internal/oidc/provider" + + "go.pinniped.dev/internal/testutil/oidctestutil" + "github.com/stretchr/testify/require" "go.pinniped.dev/internal/oidc" @@ -22,10 +26,11 @@ func TestDiscovery(t *testing.T) { method string path string - wantStatus int - wantContentType string - wantBodyJSON interface{} - wantBodyString string + wantStatus int + wantContentType string + wantFirstResponseBodyJSON interface{} + wantSecondResponseBodyJSON interface{} + wantBodyString string }{ { name: "happy path", @@ -34,7 +39,7 @@ func TestDiscovery(t *testing.T) { path: "/some/path" + oidc.WellKnownEndpointPath, wantStatus: http.StatusOK, wantContentType: "application/json", - wantBodyJSON: &Metadata{ + wantFirstResponseBodyJSON: &Metadata{ Issuer: "https://some-issuer.com/some/path", AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize", TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token", @@ -45,6 +50,32 @@ func TestDiscovery(t *testing.T) { TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, ScopesSupported: []string{"openid", "offline"}, ClaimsSupported: []string{"groups"}, + IDPs: []IdentityProviderMetadata{ + {Name: "a-some-ldap-idp", Type: "ldap"}, + {Name: "a-some-oidc-idp", Type: "oidc"}, + {Name: "x-some-idp", Type: "ldap"}, + {Name: "x-some-idp", Type: "oidc"}, + {Name: "z-some-ldap-idp", Type: "ldap"}, + {Name: "z-some-oidc-idp", Type: "oidc"}, + }, + }, + wantSecondResponseBodyJSON: &Metadata{ + Issuer: "https://some-issuer.com/some/path", + AuthorizationEndpoint: "https://some-issuer.com/some/path/oauth2/authorize", + TokenEndpoint: "https://some-issuer.com/some/path/oauth2/token", + JWKSURI: "https://some-issuer.com/some/path/jwks.json", + ResponseTypesSupported: []string{"code"}, + SubjectTypesSupported: []string{"public"}, + IDTokenSigningAlgValuesSupported: []string{"ES256"}, + TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, + ScopesSupported: []string{"openid", "offline"}, + ClaimsSupported: []string{"groups"}, + IDPs: []IdentityProviderMetadata{ + {Name: "some-other-ldap-idp-1", Type: "ldap"}, + {Name: "some-other-ldap-idp-2", Type: "ldap"}, + {Name: "some-other-oidc-idp-1", Type: "oidc"}, + {Name: "some-other-oidc-idp-2", Type: "oidc"}, + }, }, }, { @@ -60,7 +91,16 @@ func TestDiscovery(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - handler := NewHandler(test.issuer) + idpLister := oidctestutil.NewUpstreamIDPListerBuilder(). + WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "z-some-oidc-idp"}). + WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "x-some-idp"}). + WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "a-some-ldap-idp"}). + WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "a-some-oidc-idp"}). + WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "z-some-ldap-idp"}). + WithLDAP(&oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "x-some-idp"}). + Build() + + handler := NewHandler(test.issuer, idpLister) req := httptest.NewRequest(test.method, test.path, nil) rsp := httptest.NewRecorder() handler.ServeHTTP(rsp, req) @@ -69,8 +109,36 @@ func TestDiscovery(t *testing.T) { require.Equal(t, test.wantContentType, rsp.Header().Get("Content-Type")) - if test.wantBodyJSON != nil { - wantJSON, err := json.Marshal(test.wantBodyJSON) + if test.wantFirstResponseBodyJSON != nil { + wantJSON, err := json.Marshal(test.wantFirstResponseBodyJSON) + require.NoError(t, err) + require.JSONEq(t, string(wantJSON), rsp.Body.String()) + } + + if test.wantBodyString != "" { + require.Equal(t, test.wantBodyString, rsp.Body.String()) + } + + // Change the list of IDPs in the cache. + idpLister.SetLDAPIdentityProviders([]provider.UpstreamLDAPIdentityProviderI{ + &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-1"}, + &oidctestutil.TestUpstreamLDAPIdentityProvider{Name: "some-other-ldap-idp-2"}, + }) + idpLister.SetOIDCIdentityProviders([]provider.UpstreamOIDCIdentityProviderI{ + &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-1"}, + &oidctestutil.TestUpstreamOIDCIdentityProvider{Name: "some-other-oidc-idp-2"}, + }) + + // Make the same request to the same handler instance again, and expect different results. + rsp = httptest.NewRecorder() + handler.ServeHTTP(rsp, req) + + require.Equal(t, test.wantStatus, rsp.Code) + + require.Equal(t, test.wantContentType, rsp.Header().Get("Content-Type")) + + if test.wantFirstResponseBodyJSON != nil { + wantJSON, err := json.Marshal(test.wantSecondResponseBodyJSON) require.NoError(t, err) require.JSONEq(t, string(wantJSON), rsp.Body.String()) } diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index f1192edbf..e17a47377 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -102,7 +102,7 @@ func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIs wrapGetter(incomingProvider.Issuer(), m.secretCache.GetStateEncoderBlockKey), ) - m.providerHandlers[(issuerHostWithPath + oidc.WellKnownEndpointPath)] = discovery.NewHandler(issuer) + m.providerHandlers[(issuerHostWithPath + oidc.WellKnownEndpointPath)] = discovery.NewHandler(issuer, m.upstreamIDPs) m.providerHandlers[(issuerHostWithPath + oidc.JWKSEndpointPath)] = jwks.NewHandler(issuer, m.dynamicJWKSProvider) diff --git a/internal/oidc/provider/manager/manager_test.go b/internal/oidc/provider/manager/manager_test.go index 04f30fa0d..f42fe11c1 100644 --- a/internal/oidc/provider/manager/manager_test.go +++ b/internal/oidc/provider/manager/manager_test.go @@ -52,6 +52,8 @@ func TestManager(t *testing.T) { issuer2DifferentCaseHostname = "https://exAmPlE.Com/some/path/more/deeply/nested/path" issuer2KeyID = "issuer2-key" upstreamIDPAuthorizationURL = "https://test-upstream.com/auth" + upstreamIDPName = "test-idp" + upstreamIDPType = "oidc" downstreamClientID = "pinniped-cli" downstreamRedirectURL = "http://127.0.0.1:12345/callback" @@ -68,7 +70,7 @@ func TestManager(t *testing.T) { return req } - requireDiscoveryRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedIssuerInResponse string) { + requireDiscoveryRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedIssuer, expectedIDPName, expectedIDPType string) { recorder := httptest.NewRecorder() subject.ServeHTTP(recorder, newGetRequest(requestIssuer+oidc.WellKnownEndpointPath+requestURLSuffix)) @@ -82,7 +84,10 @@ func TestManager(t *testing.T) { parsedDiscoveryResult := discovery.Metadata{} err = json.Unmarshal(responseBody, &parsedDiscoveryResult) r.NoError(err) - r.Equal(expectedIssuerInResponse, parsedDiscoveryResult.Issuer) + r.Equal(expectedIssuer, parsedDiscoveryResult.Issuer) + r.Len(parsedDiscoveryResult.IDPs, 1) + r.Equal(expectedIDPName, parsedDiscoveryResult.IDPs[0].Name) + r.Equal(expectedIDPType, parsedDiscoveryResult.IDPs[0].Type) } requireAuthorizationRequestToBeHandled := func(requestIssuer, requestURLSuffix, expectedRedirectLocationPrefix string) (string, string) { @@ -222,7 +227,7 @@ func TestManager(t *testing.T) { parsedUpstreamIDPAuthorizationURL, err := url.Parse(upstreamIDPAuthorizationURL) r.NoError(err) idpLister := oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(&oidctestutil.TestUpstreamOIDCIdentityProvider{ - Name: "test-idp", + Name: upstreamIDPName, ClientID: "test-client-id", AuthorizationURL: *parsedUpstreamIDPAuthorizationURL, Scopes: []string{"test-scope"}, @@ -284,14 +289,14 @@ func TestManager(t *testing.T) { } requireRoutesMatchingRequestsToAppropriateProvider := func() { - requireDiscoveryRequestToBeHandled(issuer1, "", issuer1) - requireDiscoveryRequestToBeHandled(issuer2, "", issuer2) - requireDiscoveryRequestToBeHandled(issuer2, "?some=query", issuer2) + requireDiscoveryRequestToBeHandled(issuer1, "", issuer1, upstreamIDPName, upstreamIDPType) + requireDiscoveryRequestToBeHandled(issuer2, "", issuer2, upstreamIDPName, upstreamIDPType) + requireDiscoveryRequestToBeHandled(issuer2, "?some=query", issuer2, upstreamIDPName, upstreamIDPType) // Hostnames are case-insensitive, so test that we can handle that. - requireDiscoveryRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1) - requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2) - requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2) + requireDiscoveryRequestToBeHandled(issuer1DifferentCaseHostname, "", issuer1, upstreamIDPName, upstreamIDPType) + requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "", issuer2, upstreamIDPName, upstreamIDPType) + requireDiscoveryRequestToBeHandled(issuer2DifferentCaseHostname, "?some=query", issuer2, upstreamIDPName, upstreamIDPType) issuer1JWKS := requireJWKSRequestToBeHandled(issuer1, "", issuer1KeyID) issuer2JWKS := requireJWKSRequestToBeHandled(issuer2, "", issuer2KeyID) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 2bcfd03d5..07b58de00 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -483,7 +483,8 @@ func requireWellKnownEndpointIsWorking(t *testing.T, supervisorScheme, superviso "response_types_supported": ["code"], "claims_supported": ["groups"], "subject_types_supported": ["public"], - "id_token_signing_alg_values_supported": ["ES256"] + "id_token_signing_alg_values_supported": ["ES256"], + "pinniped_idps": [] }`) expectedJSON := fmt.Sprintf(expectedResultTemplate, issuerName, issuerName, issuerName, issuerName)