From 551249fb69efa182f3405b8a4512a4e6a1cf326b Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 15 Jun 2021 11:27:30 -0500 Subject: [PATCH] Use a custom type for our static CLI client (smaller change). Before this change, we used the `fosite.DefaultOpenIDConnectClient{}` struct, which implements the `fosite.Client` and `fosite.OpenIDConnectClient` interfaces. For a future change, we also need to implement some additional optional interfaces, so we can no longer use the provided default types. Instead, we now use a custom `clientregistry.Client{}` struct, which implements all the requisite interfaces and can be extended to handle the new functionality (in a future change). There is also a new `clientregistry.StaticRegistry{}` struct, which implements the `fosite.ClientManager` and looks up our single static client. We could potentially extend this in the future with a registry backed by Kubernetes API, for example. This should be 100% refactor, with no user-observable change. Signed-off-by: Matt Moyer --- .../fositestorage/accesstoken/accesstoken.go | 5 +- .../accesstoken/accesstoken_test.go | 61 ++++++------ .../authorizationcode/authorizationcode.go | 5 +- .../authorizationcode_test.go | 45 +++++---- internal/fositestorage/fositestorage.go | 7 +- .../openidconnect/openidconnect.go | 5 +- .../openidconnect/openidconnect_test.go | 42 ++++---- internal/fositestorage/pkce/pkce.go | 5 +- internal/fositestorage/pkce/pkce_test.go | 42 ++++---- .../refreshtoken/refreshtoken.go | 5 +- .../refreshtoken/refreshtoken_test.go | 58 ++++++----- .../oidc/clientregistry/clientregistry.go | 94 ++++++++++++++++++ .../clientregistry/clientregistry_test.go | 95 +++++++++++++++++++ internal/oidc/kube_storage.go | 29 ++---- internal/oidc/nullstorage.go | 22 +---- internal/oidc/nullstorage_test.go | 37 -------- internal/oidc/oidc.go | 14 --- 17 files changed, 356 insertions(+), 215 deletions(-) create mode 100644 internal/oidc/clientregistry/clientregistry.go create mode 100644 internal/oidc/clientregistry/clientregistry_test.go delete mode 100644 internal/oidc/nullstorage_test.go diff --git a/internal/fositestorage/accesstoken/accesstoken.go b/internal/fositestorage/accesstoken/accesstoken.go index 28a4b39f..b4427543 100644 --- a/internal/fositestorage/accesstoken/accesstoken.go +++ b/internal/fositestorage/accesstoken/accesstoken.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 accesstoken @@ -17,6 +17,7 @@ import ( "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( @@ -108,7 +109,7 @@ func (a *accessTokenStorage) getSession(ctx context.Context, signature string) ( func newValidEmptyAccessTokenSession() *session { return &session{ Request: &fosite.Request{ - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, }, } diff --git a/internal/fositestorage/accesstoken/accesstoken_test.go b/internal/fositestorage/accesstoken/accesstoken_test.go index 614b1e0f..3c5b22df 100644 --- a/internal/fositestorage/accesstoken/accesstoken_test.go +++ b/internal/fositestorage/accesstoken/accesstoken_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 accesstoken @@ -20,6 +20,8 @@ import ( "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" coretesting "k8s.io/client-go/testing" + + "go.pinniped.dev/internal/oidc/clientregistry" ) const namespace = "test-ns" @@ -63,24 +65,25 @@ func TestAccessTokenStorage(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Secret: nil, - RedirectURIs: nil, - GrantTypes: nil, - ResponseTypes: nil, - Scopes: nil, - Audience: nil, - Public: true, - }, - JSONWebKeysURI: "where", - JSONWebKeys: nil, - TokenEndpointAuthMethod: "something", - RequestURIs: nil, - RequestObjectSigningAlgorithm: "", - TokenEndpointAuthSigningAlgorithm: "", - }, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Secret: nil, + RedirectURIs: nil, + GrantTypes: nil, + ResponseTypes: nil, + Scopes: nil, + Audience: nil, + Public: true, + }, + JSONWebKeysURI: "where", + JSONWebKeys: nil, + TokenEndpointAuthMethod: "something", + RequestURIs: nil, + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: "", + }}, RequestedScope: nil, GrantedScope: nil, Form: url.Values{"key": []string{"val"}}, @@ -138,13 +141,15 @@ func TestAccessTokenStorageRevocation(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Public: true, + }, + JSONWebKeysURI: "where", + TokenEndpointAuthMethod: "something", }, - JSONWebKeysURI: "where", - TokenEndpointAuthMethod: "something", }, Form: url.Values{"key": []string{"val"}}, Session: &openid.DefaultSession{ @@ -238,7 +243,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { request := &fosite.Request{ Session: nil, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateAccessTokenSession(ctx, "signature-doesnt-matter", request) require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") @@ -248,7 +253,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { Client: nil, } err = storage.CreateAccessTokenSession(ctx, "signature-doesnt-matter", request) - require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") + require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } func TestCreateWithoutRequesterID(t *testing.T) { @@ -257,7 +262,7 @@ func TestCreateWithoutRequesterID(t *testing.T) { request := &fosite.Request{ ID: "", // empty ID Session: &openid.DefaultSession{}, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateAccessTokenSession(ctx, "signature-doesnt-matter", request) require.NoError(t, err) diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index ef870d0f..f66c193c 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.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 authorizationcode @@ -18,6 +18,7 @@ import ( "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( @@ -137,7 +138,7 @@ func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string) func NewValidEmptyAuthorizeCodeSession() *AuthorizeCodeSession { return &AuthorizeCodeSession{ Request: &fosite.Request{ - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, }, } diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index d88bf6d5..b93481c9 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_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 authorizationcode @@ -33,6 +33,7 @@ import ( kubetesting "k8s.io/client-go/testing" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const namespace = "test-ns" @@ -92,23 +93,25 @@ func TestAuthorizationCodeStorage(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Secret: nil, - RedirectURIs: nil, - GrantTypes: nil, - ResponseTypes: nil, - Scopes: nil, - Audience: nil, - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Secret: nil, + RedirectURIs: nil, + GrantTypes: nil, + ResponseTypes: nil, + Scopes: nil, + Audience: nil, + Public: true, + }, + JSONWebKeysURI: "where", + JSONWebKeys: nil, + TokenEndpointAuthMethod: "something", + RequestURIs: nil, + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: "", }, - JSONWebKeysURI: "where", - JSONWebKeys: nil, - TokenEndpointAuthMethod: "something", - RequestURIs: nil, - RequestObjectSigningAlgorithm: "", - TokenEndpointAuthSigningAlgorithm: "", }, RequestedScope: nil, GrantedScope: nil, @@ -169,7 +172,7 @@ func TestInvalidateWhenConflictOnUpdateHappens(t *testing.T) { request := &fosite.Request{ ID: "some-request-id", - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, } err := storage.CreateAuthorizeCodeSession(ctx, "fancy-signature", request) @@ -240,7 +243,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { request := &fosite.Request{ Session: nil, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", request) require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") @@ -250,7 +253,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { Client: nil, } err = storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", request) - require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") + require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, oauth2.AuthorizeCodeStorage) { @@ -270,7 +273,7 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { require.Equal(t, validSession.Request, extractedRequest) // checked above - defaultClient := validSession.Request.Client.(*fosite.DefaultOpenIDConnectClient) + defaultClient := validSession.Request.Client.(*clientregistry.Client) defaultSession := validSession.Request.Session.(*openid.DefaultSession) // makes it easier to use a raw string diff --git a/internal/fositestorage/fositestorage.go b/internal/fositestorage/fositestorage.go index d95cda5d..bf1c20e9 100644 --- a/internal/fositestorage/fositestorage.go +++ b/internal/fositestorage/fositestorage.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 fositestorage @@ -8,11 +8,12 @@ import ( "github.com/ory/fosite/handler/openid" "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( ErrInvalidRequestType = constable.Error("requester must be of type fosite.Request") - ErrInvalidClientType = constable.Error("requester's client must be of type fosite.DefaultOpenIDConnectClient") + ErrInvalidClientType = constable.Error("requester's client must be of type clientregistry.Client") ErrInvalidSessionType = constable.Error("requester's session must be of type openid.DefaultSession") StorageRequestIDLabelName = "storage.pinniped.dev/request-id" //nolint:gosec // this is not a credential ) @@ -22,7 +23,7 @@ func ValidateAndExtractAuthorizeRequest(requester fosite.Requester) (*fosite.Req if !ok1 { return nil, ErrInvalidRequestType } - _, ok2 := request.Client.(*fosite.DefaultOpenIDConnectClient) + _, ok2 := request.Client.(*clientregistry.Client) if !ok2 { return nil, ErrInvalidClientType } diff --git a/internal/fositestorage/openidconnect/openidconnect.go b/internal/fositestorage/openidconnect/openidconnect.go index ab324d7e..f747a667 100644 --- a/internal/fositestorage/openidconnect/openidconnect.go +++ b/internal/fositestorage/openidconnect/openidconnect.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 openidconnect @@ -17,6 +17,7 @@ import ( "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( @@ -110,7 +111,7 @@ func (a *openIDConnectRequestStorage) getSession(ctx context.Context, signature func newValidEmptyOIDCSession() *session { return &session{ Request: &fosite.Request{ - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, }, } diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go index e727e36b..6328ffa2 100644 --- a/internal/fositestorage/openidconnect/openidconnect_test.go +++ b/internal/fositestorage/openidconnect/openidconnect_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 openidconnect @@ -20,6 +20,8 @@ import ( "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" coretesting "k8s.io/client-go/testing" + + "go.pinniped.dev/internal/oidc/clientregistry" ) const namespace = "test-ns" @@ -62,23 +64,25 @@ func TestOpenIdConnectStorage(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Secret: nil, - RedirectURIs: nil, - GrantTypes: nil, - ResponseTypes: nil, - Scopes: nil, - Audience: nil, - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Secret: nil, + RedirectURIs: nil, + GrantTypes: nil, + ResponseTypes: nil, + Scopes: nil, + Audience: nil, + Public: true, + }, + JSONWebKeysURI: "where", + JSONWebKeys: nil, + TokenEndpointAuthMethod: "something", + RequestURIs: nil, + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: "", }, - JSONWebKeysURI: "where", - JSONWebKeys: nil, - TokenEndpointAuthMethod: "something", - RequestURIs: nil, - RequestObjectSigningAlgorithm: "", - TokenEndpointAuthSigningAlgorithm: "", }, RequestedScope: nil, GrantedScope: nil, @@ -176,7 +180,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { request := &fosite.Request{ Session: nil, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", request) require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") @@ -186,7 +190,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { Client: nil, } err = storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", request) - require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") + require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } func TestAuthcodeHasNoDot(t *testing.T) { diff --git a/internal/fositestorage/pkce/pkce.go b/internal/fositestorage/pkce/pkce.go index 7767d5ac..424a7855 100644 --- a/internal/fositestorage/pkce/pkce.go +++ b/internal/fositestorage/pkce/pkce.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 pkce @@ -17,6 +17,7 @@ import ( "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( @@ -94,7 +95,7 @@ func (a *pkceStorage) getSession(ctx context.Context, signature string) (*sessio func newValidEmptyPKCESession() *session { return &session{ Request: &fosite.Request{ - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, }, } diff --git a/internal/fositestorage/pkce/pkce_test.go b/internal/fositestorage/pkce/pkce_test.go index aa9c3faa..671797eb 100644 --- a/internal/fositestorage/pkce/pkce_test.go +++ b/internal/fositestorage/pkce/pkce_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 pkce @@ -21,6 +21,8 @@ import ( "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" coretesting "k8s.io/client-go/testing" + + "go.pinniped.dev/internal/oidc/clientregistry" ) const namespace = "test-ns" @@ -63,23 +65,25 @@ func TestPKCEStorage(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Secret: nil, - RedirectURIs: nil, - GrantTypes: nil, - ResponseTypes: nil, - Scopes: nil, - Audience: nil, - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Secret: nil, + RedirectURIs: nil, + GrantTypes: nil, + ResponseTypes: nil, + Scopes: nil, + Audience: nil, + Public: true, + }, + JSONWebKeysURI: "where", + JSONWebKeys: nil, + TokenEndpointAuthMethod: "something", + RequestURIs: nil, + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: "", }, - JSONWebKeysURI: "where", - JSONWebKeys: nil, - TokenEndpointAuthMethod: "something", - RequestURIs: nil, - RequestObjectSigningAlgorithm: "", - TokenEndpointAuthSigningAlgorithm: "", }, RequestedScope: nil, GrantedScope: nil, @@ -183,7 +187,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { request := &fosite.Request{ Session: nil, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreatePKCERequestSession(ctx, "signature-doesnt-matter", request) require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") @@ -193,7 +197,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { Client: nil, } err = storage.CreatePKCERequestSession(ctx, "signature-doesnt-matter", request) - require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") + require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } func makeTestSubject() (context.Context, *fake.Clientset, corev1client.SecretInterface, pkce.PKCERequestStorage) { diff --git a/internal/fositestorage/refreshtoken/refreshtoken.go b/internal/fositestorage/refreshtoken/refreshtoken.go index c6fcfd9c..f53d9bf7 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken.go +++ b/internal/fositestorage/refreshtoken/refreshtoken.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 refreshtoken @@ -17,6 +17,7 @@ import ( "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/fositestorage" + "go.pinniped.dev/internal/oidc/clientregistry" ) const ( @@ -108,7 +109,7 @@ func (a *refreshTokenStorage) getSession(ctx context.Context, signature string) func newValidEmptyRefreshTokenSession() *session { return &session{ Request: &fosite.Request{ - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, Session: &openid.DefaultSession{}, }, } diff --git a/internal/fositestorage/refreshtoken/refreshtoken_test.go b/internal/fositestorage/refreshtoken/refreshtoken_test.go index a776b408..10ee75bf 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken_test.go +++ b/internal/fositestorage/refreshtoken/refreshtoken_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 refreshtoken @@ -20,6 +20,8 @@ import ( "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" coretesting "k8s.io/client-go/testing" + + "go.pinniped.dev/internal/oidc/clientregistry" ) const namespace = "test-ns" @@ -62,23 +64,25 @@ func TestRefreshTokenStorage(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Secret: nil, - RedirectURIs: nil, - GrantTypes: nil, - ResponseTypes: nil, - Scopes: nil, - Audience: nil, - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Secret: nil, + RedirectURIs: nil, + GrantTypes: nil, + ResponseTypes: nil, + Scopes: nil, + Audience: nil, + Public: true, + }, + JSONWebKeysURI: "where", + JSONWebKeys: nil, + TokenEndpointAuthMethod: "something", + RequestURIs: nil, + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: "", }, - JSONWebKeysURI: "where", - JSONWebKeys: nil, - TokenEndpointAuthMethod: "something", - RequestURIs: nil, - RequestObjectSigningAlgorithm: "", - TokenEndpointAuthSigningAlgorithm: "", }, RequestedScope: nil, GrantedScope: nil, @@ -137,13 +141,15 @@ func TestRefreshTokenStorageRevocation(t *testing.T) { request := &fosite.Request{ ID: "abcd-1", RequestedAt: time.Time{}, - Client: &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinny", - Public: true, + Client: &clientregistry.Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinny", + Public: true, + }, + JSONWebKeysURI: "where", + TokenEndpointAuthMethod: "something", }, - JSONWebKeysURI: "where", - TokenEndpointAuthMethod: "something", }, Form: url.Values{"key": []string{"val"}}, Session: &openid.DefaultSession{ @@ -237,7 +243,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { request := &fosite.Request{ Session: nil, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request) require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") @@ -247,7 +253,7 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) { Client: nil, } err = storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request) - require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") + require.EqualError(t, err, "requester's client must be of type clientregistry.Client") } func TestCreateWithoutRequesterID(t *testing.T) { @@ -256,7 +262,7 @@ func TestCreateWithoutRequesterID(t *testing.T) { request := &fosite.Request{ ID: "", // empty ID Session: &openid.DefaultSession{}, - Client: &fosite.DefaultOpenIDConnectClient{}, + Client: &clientregistry.Client{}, } err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request) require.NoError(t, err) diff --git a/internal/oidc/clientregistry/clientregistry.go b/internal/oidc/clientregistry/clientregistry.go new file mode 100644 index 00000000..f60cc07d --- /dev/null +++ b/internal/oidc/clientregistry/clientregistry.go @@ -0,0 +1,94 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package clientregistry defines Pinniped's OAuth2/OIDC clients. +package clientregistry + +import ( + "context" + "fmt" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/ory/fosite" +) + +// Client represents a Pinniped OAuth/OIDC client. +type Client struct { + fosite.DefaultOpenIDConnectClient +} + +// It implements both the base and OIDC client interfaces of Fosite. +var ( + _ fosite.Client = (*Client)(nil) + _ fosite.OpenIDConnectClient = (*Client)(nil) +) + +// StaticClientManager is a fosite.ClientManager with statically-defined clients. +type StaticClientManager struct{} + +var _ fosite.ClientManager = (*StaticClientManager)(nil) + +// GetClient returns a static client specified by the given ID. +// +// It returns a fosite.ErrNotFound if an unknown client is specified. +func (StaticClientManager) GetClient(_ context.Context, id string) (fosite.Client, error) { + switch id { + case "pinniped-cli": + return PinnipedCLI(), nil + default: + return nil, fosite.ErrNotFound.WithDescription("no such client") + } +} + +// ClientAssertionJWTValid returns an error if the JTI is +// known or the DB check failed and nil if the JTI is not known. +// +// This functionality is not supported by the StaticClientManager. +func (StaticClientManager) ClientAssertionJWTValid(ctx context.Context, jti string) error { + return fmt.Errorf("not implemented") +} + +// SetClientAssertionJWT marks a JTI as known for the given +// expiry time. Before inserting the new JTI, it will clean +// up any existing JTIs that have expired as those tokens can +// not be replayed due to the expiry. +// +// This functionality is not supported by the StaticClientManager. +func (StaticClientManager) SetClientAssertionJWT(ctx context.Context, jti string, exp time.Time) error { + return fmt.Errorf("not implemented") +} + +// PinnipedCLI returns the static Client corresponding to the Pinniped CLI. +func PinnipedCLI() *Client { + return &Client{ + DefaultOpenIDConnectClient: fosite.DefaultOpenIDConnectClient{ + DefaultClient: &fosite.DefaultClient{ + ID: "pinniped-cli", + Secret: nil, + RedirectURIs: []string{"http://127.0.0.1/callback"}, + GrantTypes: fosite.Arguments{ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:token-exchange", + }, + ResponseTypes: []string{"code"}, + Scopes: fosite.Arguments{ + oidc.ScopeOpenID, + oidc.ScopeOfflineAccess, + "profile", + "email", + "pinniped:request-audience", + }, + Audience: nil, + Public: true, + }, + RequestURIs: nil, + JSONWebKeys: nil, + JSONWebKeysURI: "", + RequestObjectSigningAlgorithm: "", + TokenEndpointAuthSigningAlgorithm: oidc.RS256, + TokenEndpointAuthMethod: "none", + }, + } +} diff --git a/internal/oidc/clientregistry/clientregistry_test.go b/internal/oidc/clientregistry/clientregistry_test.go new file mode 100644 index 00000000..0da67fcc --- /dev/null +++ b/internal/oidc/clientregistry/clientregistry_test.go @@ -0,0 +1,95 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package clientregistry + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/ory/fosite" + "github.com/stretchr/testify/require" +) + +func TestStaticRegistry(t *testing.T) { + ctx := context.Background() + + t.Run("unimplemented methods", func(t *testing.T) { + registry := StaticClientManager{} + require.EqualError(t, registry.ClientAssertionJWTValid(ctx, "some-token-id"), "not implemented") + require.EqualError(t, registry.SetClientAssertionJWT(ctx, "some-token-id", time.Now()), "not implemented") + }) + + t.Run("not found", func(t *testing.T) { + registry := StaticClientManager{} + got, err := registry.GetClient(ctx, "does-not-exist") + require.Error(t, err) + require.Nil(t, got) + rfcErr := fosite.ErrorToRFC6749Error(err) + require.NotNil(t, rfcErr) + require.Equal(t, rfcErr.CodeField, 404) + require.Equal(t, rfcErr.GetDescription(), "no such client") + }) + + t.Run("pinniped CLI", func(t *testing.T) { + registry := StaticClientManager{} + got, err := registry.GetClient(ctx, "pinniped-cli") + require.NoError(t, err) + require.NotNil(t, got) + require.IsType(t, &Client{}, got) + }) +} + +func TestPinnipedCLI(t *testing.T) { + c := PinnipedCLI() + require.Equal(t, "pinniped-cli", c.GetID()) + require.Nil(t, c.GetHashedSecret()) + require.Equal(t, []string{"http://127.0.0.1/callback"}, c.GetRedirectURIs()) + require.Equal(t, fosite.Arguments{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, c.GetGrantTypes()) + require.Equal(t, fosite.Arguments{"code"}, c.GetResponseTypes()) + require.Equal(t, fosite.Arguments{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience"}, c.GetScopes()) + require.True(t, c.IsPublic()) + require.Nil(t, c.GetAudience()) + require.Nil(t, c.GetRequestURIs()) + require.Nil(t, c.GetJSONWebKeys()) + require.Equal(t, "", c.GetJSONWebKeysURI()) + require.Equal(t, "", c.GetRequestObjectSigningAlgorithm()) + require.Equal(t, "none", c.GetTokenEndpointAuthMethod()) + require.Equal(t, "RS256", c.GetTokenEndpointAuthSigningAlgorithm()) + + marshaled, err := json.Marshal(c) + require.NoError(t, err) + require.JSONEq(t, ` + { + "id": "pinniped-cli", + "redirect_uris": [ + "http://127.0.0.1/callback" + ], + "grant_types": [ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:token-exchange" + ], + "response_types": [ + "code" + ], + "scopes": [ + "openid", + "offline_access", + "profile", + "email", + "pinniped:request-audience" + ], + "audience": null, + "public": true, + "jwks_uri": "", + "jwks": null, + "token_endpoint_auth_method": "none", + "request_uris": null, + "request_object_signing_alg": "", + "token_endpoint_auth_signing_alg": "RS256" + }`, string(marshaled)) +} diff --git a/internal/oidc/kube_storage.go b/internal/oidc/kube_storage.go index f775c22b..12c94d5f 100644 --- a/internal/oidc/kube_storage.go +++ b/internal/oidc/kube_storage.go @@ -13,18 +13,17 @@ import ( fositepkce "github.com/ory/fosite/handler/pkce" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/fositestorage/accesstoken" "go.pinniped.dev/internal/fositestorage/authorizationcode" "go.pinniped.dev/internal/fositestorage/openidconnect" "go.pinniped.dev/internal/fositestorage/pkce" "go.pinniped.dev/internal/fositestorage/refreshtoken" "go.pinniped.dev/internal/fositestoragei" + "go.pinniped.dev/internal/oidc/clientregistry" ) -const errKubeStorageNotImplemented = constable.Error("KubeStorage does not implement this method. It should not have been called.") - type KubeStorage struct { + clientManager fosite.ClientManager authorizationCodeStorage oauth2.AuthorizeCodeStorage pkceStorage fositepkce.PKCERequestStorage oidcStorage openid.OpenIDConnectRequestStorage @@ -37,6 +36,7 @@ var _ fositestoragei.AllFositeStorage = &KubeStorage{} func NewKubeStorage(secrets corev1client.SecretInterface, timeoutsConfiguration TimeoutsConfiguration) *KubeStorage { nowFunc := time.Now return &KubeStorage{ + clientManager: &clientregistry.StaticClientManager{}, authorizationCodeStorage: authorizationcode.New(secrets, nowFunc, timeoutsConfiguration.AuthorizationCodeSessionStorageLifetime), pkceStorage: pkce.New(secrets, nowFunc, timeoutsConfiguration.PKCESessionStorageLifetime), oidcStorage: openidconnect.New(secrets, nowFunc, timeoutsConfiguration.OIDCSessionStorageLifetime), @@ -183,26 +183,15 @@ func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) e // // OAuth client definitions: // -// For the time being, we only allow a single pre-defined client, so we do not need to interact with any underlying -// storage mechanism to fetch them. -// -func (KubeStorage) GetClient(_ context.Context, id string) (fosite.Client, error) { - client := PinnipedCLIOIDCClient() - if client.ID == id { - return client, nil - } - return nil, fosite.ErrNotFound +func (k KubeStorage) GetClient(ctx context.Context, id string) (fosite.Client, error) { + return k.clientManager.GetClient(ctx, id) } -// -// Unused interface methods. -// - -func (KubeStorage) ClientAssertionJWTValid(_ context.Context, _ string) error { - return errKubeStorageNotImplemented +func (k KubeStorage) ClientAssertionJWTValid(ctx context.Context, jti string) error { + return k.clientManager.ClientAssertionJWTValid(ctx, jti) } -func (KubeStorage) SetClientAssertionJWT(_ context.Context, _ string, _ time.Time) error { - return errKubeStorageNotImplemented +func (k KubeStorage) SetClientAssertionJWT(ctx context.Context, jti string, exp time.Time) error { + return k.clientManager.SetClientAssertionJWT(ctx, jti, exp) } diff --git a/internal/oidc/nullstorage.go b/internal/oidc/nullstorage.go index 121e3c3a..c782b848 100644 --- a/internal/oidc/nullstorage.go +++ b/internal/oidc/nullstorage.go @@ -5,17 +5,19 @@ package oidc import ( "context" - "time" "github.com/ory/fosite" "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/fositestoragei" + "go.pinniped.dev/internal/oidc/clientregistry" ) const errNullStorageNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.") -type NullStorage struct{} +type NullStorage struct { + clientregistry.StaticClientManager +} var _ fositestoragei.AllFositeStorage = &NullStorage{} @@ -86,19 +88,3 @@ func (NullStorage) GetAuthorizeCodeSession(_ context.Context, _ string, _ fosite func (NullStorage) InvalidateAuthorizeCodeSession(_ context.Context, _ string) (err error) { return errNullStorageNotImplemented } - -func (NullStorage) GetClient(_ context.Context, id string) (fosite.Client, error) { - client := PinnipedCLIOIDCClient() - if client.ID == id { - return client, nil - } - return nil, fosite.ErrNotFound -} - -func (NullStorage) ClientAssertionJWTValid(_ context.Context, _ string) error { - return errNullStorageNotImplemented -} - -func (NullStorage) SetClientAssertionJWT(_ context.Context, _ string, _ time.Time) error { - return errNullStorageNotImplemented -} diff --git a/internal/oidc/nullstorage_test.go b/internal/oidc/nullstorage_test.go deleted file mode 100644 index 74f1f8a1..00000000 --- a/internal/oidc/nullstorage_test.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package oidc - -import ( - "context" - "testing" - - "github.com/ory/fosite" - "github.com/stretchr/testify/require" -) - -func TestNullStorage_GetClient(t *testing.T) { - storage := NullStorage{} - - client, err := storage.GetClient(context.Background(), "some-other-client") - require.Equal(t, fosite.ErrNotFound, err) - require.Zero(t, client) - - client, err = storage.GetClient(context.Background(), "pinniped-cli") - require.NoError(t, err) - require.Equal(t, - &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinniped-cli", - Public: true, - RedirectURIs: []string{"http://127.0.0.1/callback"}, - ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, - Scopes: []string{"openid", "offline_access", "profile", "email", "pinniped:request-audience"}, - }, - TokenEndpointAuthMethod: "none", - }, - client, - ) -} diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 78319329..d92a0f1b 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -98,20 +98,6 @@ type UpstreamStateParamData struct { FormatVersion string `json:"v"` } -func PinnipedCLIOIDCClient() *fosite.DefaultOpenIDConnectClient { - return &fosite.DefaultOpenIDConnectClient{ - DefaultClient: &fosite.DefaultClient{ - ID: "pinniped-cli", - Public: true, - RedirectURIs: []string{"http://127.0.0.1/callback"}, - ResponseTypes: []string{"code"}, - GrantTypes: []string{"authorization_code", "refresh_token", "urn:ietf:params:oauth:grant-type:token-exchange"}, - Scopes: []string{coreosoidc.ScopeOpenID, coreosoidc.ScopeOfflineAccess, "profile", "email", "pinniped:request-audience"}, - }, - TokenEndpointAuthMethod: "none", - } -} - type TimeoutsConfiguration struct { // The length of time that our state param that we encrypt and pass to the upstream OIDC IDP should be considered // valid. If a state param generated by the authorize endpoint is sent to the callback endpoint after this much