WIP towards using k8s fosite storage in the supervisor's callback endpoint

- Note that this WIP commit includes a failing unit test, which will
  be addressed in the next commit

Signed-off-by: Ryan Richard <richardry@vmware.com>

internal/oidc/callback/callback_handler_test.go

@@ -17,8 +17,11 @@ import (
 	"github.com/gorilla/securecookie"
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
-	"github.com/ory/fosite/storage"
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/fake"
+	kubetesting "k8s.io/client-go/testing"
 
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/oidctestutil"
@@ -419,14 +422,12 @@ func TestCallbackEndpoint(t *testing.T) {
 		test := test
 
 		t.Run(test.name, func(t *testing.T) {
-			// Configure fosite the same way that the production code would, except use in-memory storage.
+			client := fake.NewSimpleClientset()
+			secrets := client.CoreV1().Secrets("some-namespace")
+
+			// Configure fosite the same way that the production code would.
 			// Inject this into our test subject at the last second so we get a fresh storage for every test.
-			oauthStore := &storage.MemoryStore{
-				Clients:        map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()},
-				AuthorizeCodes: map[string]storage.StoreAuthorizeCode{},
-				PKCES:          map[string]fosite.Requester{},
-				IDSessions:     map[string]fosite.Requester{},
-			}
+			oauthStore := oidc.NewKubeStorage(secrets)
 			hmacSecret := []byte("some secret - must have at least 32 bytes")
 			require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes")
 			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecret)
@@ -471,6 +472,21 @@ func TestCallbackEndpoint(t *testing.T) {
 				authcodeDataAndSignature := strings.Split(capturedAuthCode, ".")
 				require.Len(t, authcodeDataAndSignature, 2)
 
+				// Several Secrets should have been created
+				expectedNumberOfCreatedSecrets := 2
+				if test.wantGrantedOpenidScope {
+					expectedNumberOfCreatedSecrets++
+				}
+				require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets)
+
+				// One authcode should have been stored.
+				actualAction := client.Actions()[0].(kubetesting.CreateActionImpl)
+				require.Equal(t, "create", actualAction.GetVerb())
+				require.Equal(t, schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, actualAction.GetResource())
+				actualSecret := actualAction.GetObject().(*corev1.Secret)
+				require.True(t, strings.HasPrefix(actualSecret.Name, "pinniped-storage-authcode-"))
+				require.Empty(t, actualSecret.Namespace) // because the secrets client is already scoped to a namespace
+
 				storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage(
 					t,
 					oauthStore,
@@ -481,6 +497,14 @@ func TestCallbackEndpoint(t *testing.T) {
 					test.wantDownstreamRequestedScopes,
 				)
 
+				// One PKCE should have been stored.
+				actualAction = client.Actions()[1].(kubetesting.CreateActionImpl)
+				require.Equal(t, "create", actualAction.GetVerb())
+				require.Equal(t, schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, actualAction.GetResource())
+				actualSecret = actualAction.GetObject().(*corev1.Secret)
+				require.True(t, strings.HasPrefix(actualSecret.Name, "pinniped-storage-pkce-"))
+				require.Empty(t, actualSecret.Namespace) // because the secrets client is already scoped to a namespace
+
 				validatePKCEStorage(
 					t,
 					oauthStore,
@@ -491,15 +515,24 @@ func TestCallbackEndpoint(t *testing.T) {
 					test.wantDownstreamPKCEChallengeMethod,
 				)
 
-				validateIDSessionStorage(
-					t,
-					oauthStore,
-					capturedAuthCode, // IDSession store key is full authcode
-					storedRequestFromAuthcode,
-					storedSessionFromAuthcode,
-					test.wantGrantedOpenidScope,
-					test.wantDownstreamNonce,
-				)
+				// One IDSession should have been stored, if the downstream actually requested the "openid" scope
+				if test.wantGrantedOpenidScope {
+					actualAction = client.Actions()[2].(kubetesting.CreateActionImpl)
+					require.Equal(t, "create", actualAction.GetVerb())
+					require.Equal(t, schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, actualAction.GetResource())
+					actualSecret = actualAction.GetObject().(*corev1.Secret)
+					require.True(t, strings.HasPrefix(actualSecret.Name, "pinniped-storage-idsession-"))
+					require.Empty(t, actualSecret.Namespace) // because the secrets client is already scoped to a namespace
+
+					validateIDSessionStorage(
+						t,
+						oauthStore,
+						capturedAuthCode, // IDSession store key is full authcode
+						storedRequestFromAuthcode,
+						storedSessionFromAuthcode,
+						test.wantDownstreamNonce,
+					)
+				}
 			}
 		})
 	}
@@ -674,7 +707,7 @@ func shallowCopyAndModifyQuery(query url.Values, modifications map[string]string
 
 func validateAuthcodeStorage(
 	t *testing.T,
-	oauthStore *storage.MemoryStore,
+	oauthStore *oidc.KubeStorage,
 	storeKey string,
 	wantGrantedOpenidScope bool,
 	wantDownstreamIDTokenSubject string,
@@ -683,9 +716,6 @@ func validateAuthcodeStorage(
 ) (*fosite.Request, *openid.DefaultSession) {
 	t.Helper()
 
-	// One authcode should have been stored.
-	require.Len(t, oauthStore.AuthorizeCodes, 1)
-
 	// Get the authcode session back from storage so we can require that it was stored correctly.
 	storedAuthorizeRequestFromAuthcode, err := oauthStore.GetAuthorizeCodeSession(context.Background(), storeKey, nil)
 	require.NoError(t, err)
@@ -725,7 +755,7 @@ func validateAuthcodeStorage(
 	require.Equal(t, wantDownstreamIDTokenSubject, actualClaims.Subject)
 	if wantDownstreamIDTokenGroups != nil {
 		require.Len(t, actualClaims.Extra, 1)
-		require.Equal(t, wantDownstreamIDTokenGroups, actualClaims.Extra["groups"])
+		require.ElementsMatch(t, wantDownstreamIDTokenGroups, actualClaims.Extra["groups"])
 	} else {
 		require.Empty(t, actualClaims.Extra)
 		require.NotContains(t, actualClaims.Extra, "groups")
@@ -760,7 +790,7 @@ func validateAuthcodeStorage(
 
 func validatePKCEStorage(
 	t *testing.T,
-	oauthStore *storage.MemoryStore,
+	oauthStore *oidc.KubeStorage,
 	storeKey string,
 	storedRequestFromAuthcode *fosite.Request,
 	storedSessionFromAuthcode *openid.DefaultSession,
@@ -768,8 +798,6 @@ func validatePKCEStorage(
 ) {
 	t.Helper()
 
-	// One PKCE should have been stored.
-	require.Len(t, oauthStore.PKCES, 1)
 	storedAuthorizeRequestFromPKCE, err := oauthStore.GetPKCERequestSession(context.Background(), storeKey, nil)
 	require.NoError(t, err)
 
@@ -787,33 +815,26 @@ func validatePKCEStorage(
 
 func validateIDSessionStorage(
 	t *testing.T,
-	oauthStore *storage.MemoryStore,
+	oauthStore *oidc.KubeStorage,
 	storeKey string,
 	storedRequestFromAuthcode *fosite.Request,
 	storedSessionFromAuthcode *openid.DefaultSession,
-	wantGrantedOpenidScope bool,
 	wantDownstreamNonce string,
 ) {
 	t.Helper()
 
-	// One IDSession should have been stored, if the downstream actually requested the "openid" scope..
-	if wantGrantedOpenidScope {
-		require.Len(t, oauthStore.IDSessions, 1)
-		storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil)
-		require.NoError(t, err)
+	storedAuthorizeRequestFromIDSession, err := oauthStore.GetOpenIDConnectSession(context.Background(), storeKey, nil)
+	require.NoError(t, err)
 
-		// Check that storage returned the expected concrete data types.
-		storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession)
+	// Check that storage returned the expected concrete data types.
+	storedRequestFromIDSession, storedSessionFromIDSession := castStoredAuthorizeRequest(t, storedAuthorizeRequestFromIDSession)
 
-		// The stored IDSession request should be the same as the stored authcode request.
-		require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID)
-		require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession)
+	// The stored IDSession request should be the same as the stored authcode request.
+	require.Equal(t, storedRequestFromAuthcode.ID, storedRequestFromIDSession.ID)
+	require.Equal(t, storedSessionFromAuthcode, storedSessionFromIDSession)
 
-		// The stored IDSession request should also contain the nonce that the downstream sent us.
-		require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce"))
-	} else {
-		require.Len(t, oauthStore.IDSessions, 0)
-	}
+	// The stored IDSession request should also contain the nonce that the downstream sent us.
+	require.Equal(t, wantDownstreamNonce, storedRequestFromIDSession.Form.Get("nonce"))
 }
 
 func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requester) (*fosite.Request, *openid.DefaultSession) {

internal/oidc/kube_storage.go

@@ -0,0 +1,110 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package oidc
+
+import (
+	"context"
+	"time"
+
+	"github.com/ory/fosite"
+	"github.com/ory/fosite/handler/oauth2"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
+
+	"go.pinniped.dev/internal/constable"
+	"go.pinniped.dev/internal/fositestorage/authorizationcode"
+)
+
+const errKubeStorageNotImplemented = constable.Error("KubeStorage does not implement this method. It should not have been called.")
+
+type KubeStorage struct {
+	authorizationCodeStorage oauth2.AuthorizeCodeStorage
+}
+
+func NewKubeStorage(secrets corev1client.SecretInterface) *KubeStorage {
+	return &KubeStorage{authorizationCodeStorage: authorizationcode.New(secrets)}
+}
+
+func (KubeStorage) RevokeRefreshToken(_ context.Context, _ string) error {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) RevokeAccessToken(_ context.Context, _ string) error {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) CreateRefreshTokenSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
+	return nil
+}
+
+func (KubeStorage) GetRefreshTokenSession(_ context.Context, _ string, _ fosite.Session) (request fosite.Requester, err error) {
+	return nil, errKubeStorageNotImplemented
+}
+
+func (KubeStorage) DeleteRefreshTokenSession(_ context.Context, _ string) (err error) {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) CreateAccessTokenSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
+	return nil
+}
+
+func (KubeStorage) GetAccessTokenSession(_ context.Context, _ string, _ fosite.Session) (request fosite.Requester, err error) {
+	return nil, errKubeStorageNotImplemented
+}
+
+func (KubeStorage) DeleteAccessTokenSession(_ context.Context, _ string) (err error) {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) CreateOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) error {
+	return nil
+}
+
+func (KubeStorage) GetOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) (fosite.Requester, error) {
+	return nil, errKubeStorageNotImplemented
+}
+
+func (KubeStorage) DeleteOpenIDConnectSession(_ context.Context, _ string) error {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) GetPKCERequestSession(_ context.Context, _ string, _ fosite.Session) (fosite.Requester, error) {
+	return nil, errKubeStorageNotImplemented
+}
+
+func (KubeStorage) CreatePKCERequestSession(_ context.Context, _ string, _ fosite.Requester) error {
+	return nil
+}
+
+func (KubeStorage) DeletePKCERequestSession(_ context.Context, _ string) error {
+	return errKubeStorageNotImplemented
+}
+
+func (k KubeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, r fosite.Requester) (err error) {
+	return k.authorizationCodeStorage.CreateAuthorizeCodeSession(ctx, signature, r)
+}
+
+func (k KubeStorage) GetAuthorizeCodeSession(ctx context.Context, signature string, s fosite.Session) (request fosite.Requester, err error) {
+	return k.authorizationCodeStorage.GetAuthorizeCodeSession(ctx, signature, s)
+}
+
+func (k KubeStorage) InvalidateAuthorizeCodeSession(ctx context.Context, signature string) (err error) {
+	return k.authorizationCodeStorage.InvalidateAuthorizeCodeSession(ctx, signature)
+}
+
+func (KubeStorage) GetClient(_ context.Context, id string) (fosite.Client, error) {
+	client := PinnipedCLIOIDCClient()
+	if client.ID == id {
+		return client, nil
+	}
+	return nil, fosite.ErrNotFound
+}
+
+func (KubeStorage) ClientAssertionJWTValid(_ context.Context, _ string) error {
+	return errKubeStorageNotImplemented
+}
+
+func (KubeStorage) SetClientAssertionJWT(_ context.Context, _ string, _ time.Time) error {
+	return errKubeStorageNotImplemented
+}

internal/oidc/nullstorage.go

@@ -12,16 +12,16 @@ import (
 	"go.pinniped.dev/internal/constable"
 )
 
-const errNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.")
+const errNullStorageNotImplemented = constable.Error("NullStorage does not implement this method. It should not have been called.")
 
 type NullStorage struct{}
 
 func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) RevokeAccessToken(_ context.Context, _ string) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) CreateRefreshTokenSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
@@ -29,11 +29,11 @@ func (NullStorage) CreateRefreshTokenSession(_ context.Context, _ string, _ fosi
 }
 
 func (NullStorage) GetRefreshTokenSession(_ context.Context, _ string, _ fosite.Session) (request fosite.Requester, err error) {
-	return nil, errNotImplemented
+	return nil, errNullStorageNotImplemented
 }
 
 func (NullStorage) DeleteRefreshTokenSession(_ context.Context, _ string) (err error) {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) CreateAccessTokenSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
@@ -41,11 +41,11 @@ func (NullStorage) CreateAccessTokenSession(_ context.Context, _ string, _ fosit
 }
 
 func (NullStorage) GetAccessTokenSession(_ context.Context, _ string, _ fosite.Session) (request fosite.Requester, err error) {
-	return nil, errNotImplemented
+	return nil, errNullStorageNotImplemented
 }
 
 func (NullStorage) DeleteAccessTokenSession(_ context.Context, _ string) (err error) {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) CreateOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) error {
@@ -53,15 +53,15 @@ func (NullStorage) CreateOpenIDConnectSession(_ context.Context, _ string, _ fos
 }
 
 func (NullStorage) GetOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) (fosite.Requester, error) {
-	return nil, errNotImplemented
+	return nil, errNullStorageNotImplemented
 }
 
 func (NullStorage) DeleteOpenIDConnectSession(_ context.Context, _ string) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) GetPKCERequestSession(_ context.Context, _ string, _ fosite.Session) (fosite.Requester, error) {
-	return nil, errNotImplemented
+	return nil, errNullStorageNotImplemented
 }
 
 func (NullStorage) CreatePKCERequestSession(_ context.Context, _ string, _ fosite.Requester) error {
@@ -69,7 +69,7 @@ func (NullStorage) CreatePKCERequestSession(_ context.Context, _ string, _ fosit
 }
 
 func (NullStorage) DeletePKCERequestSession(_ context.Context, _ string) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) CreateAuthorizeCodeSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
@@ -77,11 +77,11 @@ func (NullStorage) CreateAuthorizeCodeSession(_ context.Context, _ string, _ fos
 }
 
 func (NullStorage) GetAuthorizeCodeSession(_ context.Context, _ string, _ fosite.Session) (request fosite.Requester, err error) {
-	return nil, errNotImplemented
+	return nil, errNullStorageNotImplemented
 }
 
 func (NullStorage) InvalidateAuthorizeCodeSession(_ context.Context, _ string) (err error) {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) GetClient(_ context.Context, id string) (fosite.Client, error) {
@@ -93,9 +93,9 @@ func (NullStorage) GetClient(_ context.Context, id string) (fosite.Client, error
 }
 
 func (NullStorage) ClientAssertionJWTValid(_ context.Context, _ string) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }
 
 func (NullStorage) SetClientAssertionJWT(_ context.Context, _ string, _ time.Time) error {
-	return errNotImplemented
+	return errNullStorageNotImplemented
 }