From d64acbb5a9e20a0100f4f1c3153285f956bb4be8 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 30 Nov 2020 14:54:11 -0600 Subject: [PATCH] Add upstreamoidc.ProviderConfig type implementing provider.UpstreamOIDCIdentityProviderI. Signed-off-by: Matt Moyer --- .../upstreamwatcher/upstreamwatcher.go | 25 +- .../upstreamwatcher/upstreamwatcher_test.go | 71 ++++-- .../provider/dynamic_upstream_idp_provider.go | 50 +--- internal/upstreamoidc/upstreamoidc.go | 106 +++++++++ internal/upstreamoidc/upstreamoidc_test.go | 223 ++++++++++++++++++ 5 files changed, 391 insertions(+), 84 deletions(-) create mode 100644 internal/upstreamoidc/upstreamoidc.go create mode 100644 internal/upstreamoidc/upstreamoidc_test.go diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go index 9c8952e9..4955646b 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go @@ -17,6 +17,7 @@ import ( "github.com/coreos/go-oidc" "github.com/go-logr/logr" + "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -30,6 +31,7 @@ import ( pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/upstreamoidc" ) const ( @@ -150,10 +152,14 @@ func (c *controller) Sync(ctx controllerlib.Context) error { // validateUpstream validates the provided v1alpha1.UpstreamOIDCProvider and returns the validated configuration as a // provider.UpstreamOIDCIdentityProvider. As a side effect, it also updates the status of the v1alpha1.UpstreamOIDCProvider. -func (c *controller) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.UpstreamOIDCProvider) *provider.UpstreamOIDCIdentityProvider { - result := provider.UpstreamOIDCIdentityProvider{ - Name: upstream.Name, - Scopes: computeScopes(upstream.Spec.AuthorizationConfig.AdditionalScopes), +func (c *controller) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.UpstreamOIDCProvider) *upstreamoidc.ProviderConfig { + result := upstreamoidc.ProviderConfig{ + Name: upstream.Name, + Config: &oauth2.Config{ + Scopes: computeScopes(upstream.Spec.AuthorizationConfig.AdditionalScopes), + }, + UsernameClaim: upstream.Spec.Claims.Username, + GroupsClaim: upstream.Spec.Claims.Groups, } conditions := []*v1alpha1.Condition{ c.validateSecret(upstream, &result), @@ -180,7 +186,7 @@ func (c *controller) validateUpstream(ctx controllerlib.Context, upstream *v1alp } // validateSecret validates the .spec.client.secretName field and returns the appropriate ClientCredentialsValid condition. -func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition { +func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, result *upstreamoidc.ProviderConfig) *v1alpha1.Condition { secretName := upstream.Spec.Client.SecretName // Fetch the Secret from informer cache. @@ -217,7 +223,7 @@ func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, res } // If everything is valid, update the result and set the condition to true. - result.ClientID = string(clientID) + result.Config.ClientID = string(clientID) return &v1alpha1.Condition{ Type: typeClientCredsValid, Status: v1alpha1.ConditionTrue, @@ -227,7 +233,7 @@ func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, res } // validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition. -func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition { +func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, result *upstreamoidc.ProviderConfig) *v1alpha1.Condition { // Get the provider (from cache if possible). discoveredProvider := c.validatorCache.getProvider(&upstream.Spec) @@ -258,8 +264,6 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst c.validatorCache.putProvider(&upstream.Spec, discoveredProvider) } - // TODO also parse the token endpoint from the discovery info and put it onto the `result` - // Parse out and validate the discovered authorize endpoint. authURL, err := url.Parse(discoveredProvider.Endpoint().AuthURL) if err != nil { @@ -280,7 +284,8 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst } // If everything is valid, update the result and set the condition to true. - result.AuthorizationURL = *authURL + result.Config.Endpoint = discoveredProvider.Endpoint() + result.Provider = discoveredProvider return &v1alpha1.Condition{ Type: typeOIDCDiscoverySucceeded, Status: v1alpha1.ConditionTrue, diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go index 4891ae8a..3ecfa91a 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go @@ -24,9 +24,11 @@ import ( pinnipedfake "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned/fake" pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/testlogger" + "go.pinniped.dev/internal/upstreamoidc" ) func TestController(t *testing.T) { @@ -49,6 +51,8 @@ func TestController(t *testing.T) { testClientID = "test-oidc-client-id" testClientSecret = "test-oidc-client-secret" testValidSecretData = map[string][]byte{"clientID": []byte(testClientID), "clientSecret": []byte(testClientSecret)} + testGroupsClaim = "test-groups-claim" + testUsernameClaim = "test-username-claim" ) tests := []struct { name string @@ -56,7 +60,7 @@ func TestController(t *testing.T) { inputSecrets []runtime.Object wantErr string wantLogs []string - wantResultingCache []provider.UpstreamOIDCIdentityProvider + wantResultingCache []provider.UpstreamOIDCIdentityProviderI wantResultingUpstreams []v1alpha1.UpstreamOIDCProvider }{ { @@ -80,7 +84,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="secret \"test-client-secret\" not found" "name"="test-name" "namespace"="test-namespace" "reason"="SecretNotFound" "type"="ClientCredentialsValid"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -126,7 +130,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="referenced Secret \"test-client-secret\" has wrong type \"some-other-type\" (should be \"secrets.pinniped.dev/oidc-client\")" "name"="test-name" "namespace"="test-namespace" "reason"="SecretWrongType" "type"="ClientCredentialsValid"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -171,7 +175,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="referenced Secret \"test-client-secret\" is missing required keys [\"clientID\" \"clientSecret\"]" "name"="test-name" "namespace"="test-namespace" "reason"="SecretMissingKeys" "type"="ClientCredentialsValid"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -219,7 +223,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -267,7 +271,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: no certificates found" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: no certificates found" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -312,7 +316,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"invalid-url\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to perform OIDC discovery against \"invalid-url\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -358,7 +362,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to parse authorization endpoint URL: parse \"%\": invalid URL escape \"%\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -404,7 +408,7 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "reason"="InvalidResponse" "status"="False" "type"="OIDCDiscoverySucceeded"`, `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="authorization endpoint URL scheme must be \"https\", not \"http\"" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidResponse" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{}, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -437,6 +441,7 @@ func TestController(t *testing.T) { TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")}, + Claims: v1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim}, }, Status: v1alpha1.UpstreamOIDCProviderStatus{ Phase: "Error", @@ -455,12 +460,16 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{{ - Name: testName, - ClientID: testClientID, - AuthorizationURL: *testIssuerAuthorizeURL, - Scopes: append(testExpectedScopes, "xyz"), - }}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{ + &oidctestutil.TestUpstreamOIDCIdentityProvider{ + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: append(testExpectedScopes, "xyz"), + UsernameClaim: testUsernameClaim, + GroupsClaim: testGroupsClaim, + }, + }, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -481,6 +490,7 @@ func TestController(t *testing.T) { TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, + Claims: v1alpha1.OIDCClaims{Groups: testGroupsClaim, Username: testUsernameClaim}, }, Status: v1alpha1.UpstreamOIDCProviderStatus{ Phase: "Ready", @@ -499,12 +509,16 @@ func TestController(t *testing.T) { `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="discovered issuer configuration" "reason"="Success" "status"="True" "type"="OIDCDiscoverySucceeded"`, }, - wantResultingCache: []provider.UpstreamOIDCIdentityProvider{{ - Name: testName, - ClientID: testClientID, - AuthorizationURL: *testIssuerAuthorizeURL, - Scopes: testExpectedScopes, - }}, + wantResultingCache: []provider.UpstreamOIDCIdentityProviderI{ + &oidctestutil.TestUpstreamOIDCIdentityProvider{ + Name: testName, + ClientID: testClientID, + AuthorizationURL: *testIssuerAuthorizeURL, + Scopes: testExpectedScopes, + UsernameClaim: testUsernameClaim, + GroupsClaim: testGroupsClaim, + }, + }, wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, Status: v1alpha1.UpstreamOIDCProviderStatus{ @@ -527,9 +541,9 @@ func TestController(t *testing.T) { kubeInformers := informers.NewSharedInformerFactory(fakeKubeClient, 0) testLog := testlogger.New(t) cache := provider.NewDynamicUpstreamIDPProvider() - initialProviderList := make([]provider.UpstreamOIDCIdentityProviderI, 1) - initialProviderList[0] = &provider.UpstreamOIDCIdentityProvider{Name: "initial-entry"} - cache.SetIDPList(initialProviderList) + cache.SetIDPList([]provider.UpstreamOIDCIdentityProviderI{ + &upstreamoidc.ProviderConfig{Name: "initial-entry"}, + }) controller := New( cache, @@ -557,8 +571,13 @@ func TestController(t *testing.T) { actualIDPList := cache.GetIDPList() require.Equal(t, len(tt.wantResultingCache), len(actualIDPList)) for i := range actualIDPList { - actualIDP := actualIDPList[i].(*provider.UpstreamOIDCIdentityProvider) - require.Equal(t, tt.wantResultingCache[i], *actualIDP) + actualIDP := actualIDPList[i].(*upstreamoidc.ProviderConfig) + require.Equal(t, tt.wantResultingCache[i].GetName(), actualIDP.GetName()) + require.Equal(t, tt.wantResultingCache[i].GetClientID(), actualIDP.GetClientID()) + require.Equal(t, tt.wantResultingCache[i].GetAuthorizationURL().String(), actualIDP.GetAuthorizationURL().String()) + require.Equal(t, tt.wantResultingCache[i].GetUsernameClaim(), actualIDP.GetUsernameClaim()) + require.Equal(t, tt.wantResultingCache[i].GetGroupsClaim(), actualIDP.GetGroupsClaim()) + require.ElementsMatch(t, tt.wantResultingCache[i].GetScopes(), actualIDP.GetScopes()) } actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().UpstreamOIDCProviders(testNamespace).List(ctx, metav1.ListOptions{}) diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 98c86bdb..50ba17ca 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -33,12 +33,8 @@ type UpstreamOIDCIdentityProviderI interface { // ID Token groups claim name. May return empty string, in which case we won't try to read groups from the upstream provider. GetGroupsClaim() string - AuthcodeExchanger -} - -// Performs upstream OIDC authorization code exchange and token validation. -// Returns the validated raw tokens as well as the parsed claims of the ID token. -type AuthcodeExchanger interface { + // Performs upstream OIDC authorization code exchange and token validation. + // Returns the validated raw tokens as well as the parsed claims of the ID token. ExchangeAuthcodeAndValidateTokens( ctx context.Context, authcode string, @@ -47,48 +43,6 @@ type AuthcodeExchanger interface { ) (tokens oidcclient.Token, parsedIDTokenClaims map[string]interface{}, err error) } -type UpstreamOIDCIdentityProvider struct { - Name string - ClientID string - AuthorizationURL url.URL - UsernameClaim string - GroupsClaim string - Scopes []string -} - -func (u *UpstreamOIDCIdentityProvider) GetName() string { - return u.Name -} - -func (u *UpstreamOIDCIdentityProvider) GetClientID() string { - return u.ClientID -} - -func (u *UpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL { - return &u.AuthorizationURL -} - -func (u *UpstreamOIDCIdentityProvider) GetScopes() []string { - return u.Scopes -} - -func (u *UpstreamOIDCIdentityProvider) GetUsernameClaim() string { - return u.UsernameClaim -} - -func (u *UpstreamOIDCIdentityProvider) GetGroupsClaim() string { - return u.GroupsClaim -} - -func (u *UpstreamOIDCIdentityProvider) ExchangeAuthcodeAndValidateTokens( - ctx context.Context, - authcode string, - pkceCodeVerifier pkce.Code, - expectedIDTokenNonce nonce.Nonce, -) (oidcclient.Token, map[string]interface{}, error) { - panic("TODO implement me") // TODO -} - type DynamicUpstreamIDPProvider interface { SetIDPList(oidcIDPs []UpstreamOIDCIdentityProviderI) GetIDPList() []UpstreamOIDCIdentityProviderI diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go new file mode 100644 index 00000000..13a800b0 --- /dev/null +++ b/internal/upstreamoidc/upstreamoidc.go @@ -0,0 +1,106 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package upstreamoidc implements an abstraction of upstream OIDC provider interactions. +package upstreamoidc + +import ( + "context" + "net/http" + "net/url" + + "github.com/coreos/go-oidc" + "golang.org/x/oauth2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "go.pinniped.dev/internal/httputil/httperr" + "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" +) + +// ProviderConfig holds the active configuration of an upstream OIDC provider. +type ProviderConfig struct { + Name string + UsernameClaim string + GroupsClaim string + Config *oauth2.Config + Provider interface { + Verifier(*oidc.Config) *oidc.IDTokenVerifier + } +} + +// *ProviderConfig should implement provider.UpstreamOIDCIdentityProviderI. +var _ provider.UpstreamOIDCIdentityProviderI = (*ProviderConfig)(nil) + +func (p *ProviderConfig) GetName() string { + return p.Name +} + +func (p *ProviderConfig) GetClientID() string { + return p.Config.ClientID +} + +func (p *ProviderConfig) GetAuthorizationURL() *url.URL { + result, _ := url.Parse(p.Config.Endpoint.AuthURL) + return result +} + +func (p *ProviderConfig) GetScopes() []string { + return p.Config.Scopes +} + +func (p *ProviderConfig) GetUsernameClaim() string { + return p.UsernameClaim +} + +func (p *ProviderConfig) GetGroupsClaim() string { + return p.GroupsClaim +} + +func (p *ProviderConfig) ExchangeAuthcodeAndValidateTokens(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (oidcclient.Token, map[string]interface{}, error) { + tok, err := p.Config.Exchange(ctx, authcode, pkceCodeVerifier.Verifier()) + if err != nil { + return oidcclient.Token{}, nil, err + } + + idTok, hasIDTok := tok.Extra("id_token").(string) + if !hasIDTok { + return oidcclient.Token{}, nil, httperr.New(http.StatusBadRequest, "received response missing ID token") + } + validated, err := p.Provider.Verifier(&oidc.Config{ClientID: p.GetClientID()}).Verify(ctx, idTok) + if err != nil { + return oidcclient.Token{}, nil, httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) + } + if validated.AccessTokenHash != "" { + if err := validated.VerifyAccessToken(tok.AccessToken); err != nil { + return oidcclient.Token{}, nil, httperr.Wrap(http.StatusBadRequest, "received invalid ID token", err) + } + } + if expectedIDTokenNonce != "" { + if err := expectedIDTokenNonce.Validate(validated); err != nil { + return oidcclient.Token{}, nil, httperr.Wrap(http.StatusBadRequest, "received ID token with invalid nonce", err) + } + } + + var validatedClaims map[string]interface{} + if err := validated.Claims(&validatedClaims); err != nil { + return oidcclient.Token{}, nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal claims", err) + } + + return oidcclient.Token{ + AccessToken: &oidcclient.AccessToken{ + Token: tok.AccessToken, + Type: tok.TokenType, + Expiry: metav1.NewTime(tok.Expiry), + }, + RefreshToken: &oidcclient.RefreshToken{ + Token: tok.RefreshToken, + }, + IDToken: &oidcclient.IDToken{ + Token: idTok, + Expiry: metav1.NewTime(validated.Expiry), + }, + }, validatedClaims, nil +} diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go new file mode 100644 index 00000000..d3cf77ac --- /dev/null +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -0,0 +1,223 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package upstreamoidc + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/coreos/go-oidc" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + "gopkg.in/square/go-jose.v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "go.pinniped.dev/internal/mocks/mockkeyset" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/nonce" +) + +func TestProviderConfig(t *testing.T) { + t.Run("getters get", func(t *testing.T) { + p := ProviderConfig{ + Name: "test-name", + UsernameClaim: "test-username-claim", + GroupsClaim: "test-groups-claim", + Config: &oauth2.Config{ + ClientID: "test-client-id", + Endpoint: oauth2.Endpoint{AuthURL: "https://example.com"}, + Scopes: []string{"scope1", "scope2"}, + }, + } + require.Equal(t, "test-name", p.GetName()) + require.Equal(t, "test-client-id", p.GetClientID()) + require.Equal(t, "https://example.com", p.GetAuthorizationURL().String()) + require.ElementsMatch(t, []string{"scope1", "scope2"}, p.GetScopes()) + require.Equal(t, "test-username-claim", p.GetUsernameClaim()) + require.Equal(t, "test-groups-claim", p.GetGroupsClaim()) + }) + + const ( + // Test JWTs generated with https://smallstep.com/docs/cli/crypto/jwt/: + + // step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"at_hash": "invalid-at-hash"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub test-user --subtle --kid="test-kid" --jti="test-jti" + invalidAccessTokenHashIDToken = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdF9oYXNoIjoiaW52YWxpZC1hdC1oYXNoIiwiYXVkIjoidGVzdC1jbGllbnQtaWQiLCJpYXQiOjE2MDIyODM3OTEsImp0aSI6InRlc3QtanRpIiwibmJmIjoxNjAyMjgzNzkxLCJzdWIiOiJ0ZXN0LXVzZXIifQ.jryXr4jiwcf79wBLaHpjdclEYHoUFGhvTu95QyA6Hnk9NQ0x1vsWYurtj7a8uKydNPryC_HNZi9QTAE_tRIJjycseog3695-5y4B4EZlqL-a94rdOtffuF2O_lnPbKvoja9EKNrp0kLBCftFRHhLAEwuP0N9E5padZwPpIGK0yE_JqljnYgCySvzsQu7tasR38yaULny13h3mtp2WRHPG5DrLyuBuF8Z01hSgRi5hGcVpgzTwBgV5-eMaSUCUo-ZDkqUsLQI6dVlaikCSKYZRb53HeexH0tB_R9PJJHY7mIr-rS76kkQEx9pLuVnheIH9Oc6zbdYWg-zWMijopA8Pg" //nolint: gosec + + // step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"nonce": "invalid-nonce"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub test-user --subtle --kid="test-kid" --jti="test-jti" + invalidNonceIDToken = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ0ZXN0LWNsaWVudC1pZCIsImlhdCI6MTYwMjI4Mzc0MSwianRpIjoidGVzdC1qdGkiLCJuYmYiOjE2MDIyODM3NDEsIm5vbmNlIjoiaW52YWxpZC1ub25jZSIsInN1YiI6InRlc3QtdXNlciJ9.PRpq-7j5djaIAkraL-8t8ad9Xm4hM8RW67gyD1VIe0BecWeBFxsTuh3SZVKM9zmcwTgjudsyn8kQOwipDa49IN4PV8FcJA_uUJZi2wiqGJUSTG2K5I89doV_7e0RM1ZYIDDW1G2heKJNW7MbKkX7iEPr7u4MyEzswcPcupbyDA-CQFeL95vgwawoqa6yO94ympTbozqiNfj6Xyw_nHtThQnstjWsJZ9s2mUgppZezZv4HZYTQ7c3e_bzwhWgCzh2CSDJn9_Ra_n_4GcVkpHbsHTP35dFsnf0vactPx6CAu6A1-Apk-BruCktpZ3B4Ercf1UnUOHdGqzQKJtqvB03xQ" //nolint: gosec + + // step crypto keypair key.pub key.priv --kty RSA --no-password --insecure --force && echo '{"foo": "bar", "bat": "baz"}' | step crypto jwt sign --key key.priv --aud test-client-id --sub test-user --subtle --kid="test-kid" --jti="test-jti" + validIDToken = "eyJhbGciOiJSUzI1NiIsImtpZCI6InRlc3Qta2lkIiwidHlwIjoiSldUIn0.eyJhdWQiOiJ0ZXN0LWNsaWVudC1pZCIsImJhdCI6ImJheiIsImZvbyI6ImJhciIsImlhdCI6MTYwNjc2ODU5MywianRpIjoidGVzdC1qdGkiLCJuYmYiOjE2MDY3Njg1OTMsInN1YiI6InRlc3QtdXNlciJ9.DuqVZ7pGhHqKz7gNr4j2W1s1N8YrSltktH4wW19L4oD1OE2-O72jAnNj5xdjilsa8l7h9ox-5sMF0Tkh3BdRlHQK9dEtNm9tW-JreUnWJ3LCqUs-LZp4NG7edvq2sH_1Bn7O2_NQV51s8Pl04F60CndjQ4NM-6WkqDQTKyY6vJXU7idvM-6TM2HJZK-Na88cOJ9KIK37tL5DhcbsHVF47Dq8uPZ0KbjNQjJLAIi_1GeQBgc6yJhDUwRY4Xu6S0dtTHA6xTI8oSXoamt4bkViEHfJBp97LZQiNz8mku5pVc0aNwP1p4hMHxRHhLXrJjbh-Hx4YFjxtOnIq9t1mHlD4A" //nolint: gosec + ) + + tests := []struct { + name string + authCode string + expectNonce nonce.Nonce + returnIDTok string + wantErr string + wantToken oidcclient.Token + wantClaims map[string]interface{} + }{ + { + name: "exchange fails with network error", + authCode: "invalid-auth-code", + wantErr: "oauth2: cannot fetch token: 403 Forbidden\nResponse: invalid authorization code\n", + }, + { + name: "missing ID token", + authCode: "valid", + wantErr: "received response missing ID token", + }, + { + name: "invalid ID token", + authCode: "valid", + returnIDTok: "invalid-jwt", + wantErr: "received invalid ID token: oidc: malformed jwt: square/go-jose: compact JWS format must have three parts", + }, + { + name: "invalid access token hash", + authCode: "valid", + returnIDTok: invalidAccessTokenHashIDToken, + wantErr: "received invalid ID token: access token hash does not match value in ID token", + }, + { + name: "invalid nonce", + authCode: "valid", + expectNonce: "test-nonce", + returnIDTok: invalidNonceIDToken, + wantErr: `received ID token with invalid nonce: invalid nonce (expected "test-nonce", got "invalid-nonce")`, + }, + { + name: "invalid nonce but not checked", + authCode: "valid", + expectNonce: "", + returnIDTok: invalidNonceIDToken, + wantToken: oidcclient.Token{ + AccessToken: &oidcclient.AccessToken{ + Token: "test-access-token", + Expiry: metav1.Time{}, + }, + RefreshToken: &oidcclient.RefreshToken{ + Token: "test-refresh-token", + }, + IDToken: &oidcclient.IDToken{ + Token: invalidNonceIDToken, + Expiry: metav1.Time{}, + }, + }, + }, + { + name: "valid", + authCode: "valid", + returnIDTok: validIDToken, + wantToken: oidcclient.Token{ + AccessToken: &oidcclient.AccessToken{ + Token: "test-access-token", + Expiry: metav1.Time{}, + }, + RefreshToken: &oidcclient.RefreshToken{ + Token: "test-refresh-token", + }, + IDToken: &oidcclient.IDToken{ + Token: validIDToken, + Expiry: metav1.Time{}, + }, + }, + wantClaims: map[string]interface{}{ + "foo": "bar", + "bat": "baz", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + tokenServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.NoError(t, r.ParseForm()) + require.Equal(t, "test-client-id", r.Form.Get("client_id")) + require.Equal(t, "test-pkce", r.Form.Get("code_verifier")) + require.Equal(t, "authorization_code", r.Form.Get("grant_type")) + require.NotEmpty(t, r.Form.Get("code")) + if r.Form.Get("code") != "valid" { + http.Error(w, "invalid authorization code", http.StatusForbidden) + return + } + var response struct { + oauth2.Token + IDToken string `json:"id_token,omitempty"` + } + response.AccessToken = "test-access-token" + response.RefreshToken = "test-refresh-token" + response.Expiry = time.Now().Add(time.Hour) + response.IDToken = tt.returnIDTok + w.Header().Set("content-type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(&response)) + })) + t.Cleanup(tokenServer.Close) + + p := ProviderConfig{ + Name: "test-name", + UsernameClaim: "test-username-claim", + GroupsClaim: "test-groups-claim", + Config: &oauth2.Config{ + ClientID: "test-client-id", + Endpoint: oauth2.Endpoint{ + AuthURL: "https://example.com", + TokenURL: tokenServer.URL, + AuthStyle: oauth2.AuthStyleInParams, + }, + Scopes: []string{"scope1", "scope2"}, + }, + Provider: &mockProvider{}, + } + + ctx := context.Background() + + tok, claims, err := p.ExchangeAuthcodeAndValidateTokens(ctx, tt.authCode, "test-pkce", tt.expectNonce) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + require.Equal(t, oidcclient.Token{}, tok) + require.Nil(t, claims) + return + } + require.NoError(t, err) + require.Equal(t, tt.wantToken, tok) + + for k, v := range tt.wantClaims { + require.Equal(t, v, claims[k]) + } + }) + } +} + +// mockVerifier returns an *oidc.IDTokenVerifier that validates any correctly serialized JWT without doing much else. +func mockVerifier() *oidc.IDTokenVerifier { + mockKeySet := mockkeyset.NewMockKeySet(gomock.NewController(nil)) + mockKeySet.EXPECT().VerifySignature(gomock.Any(), gomock.Any()). + AnyTimes(). + DoAndReturn(func(ctx context.Context, jwt string) ([]byte, error) { + jws, err := jose.ParseSigned(jwt) + if err != nil { + return nil, err + } + return jws.UnsafePayloadWithoutVerification(), nil + }) + + return oidc.NewVerifier("", mockKeySet, &oidc.Config{ + SkipIssuerCheck: true, + SkipExpiryCheck: true, + SkipClientIDCheck: true, + }) +} + +type mockProvider struct{} + +func (m *mockProvider) Verifier(_ *oidc.Config) *oidc.IDTokenVerifier { return mockVerifier() }