Make assertions about how many secrets were stored by fosite in tests

In both callback_handler_test.go and token_handler_test.go

Signed-off-by: Aram Price <pricear@vmware.com>

internal/crud/crud.go

@@ -22,8 +22,9 @@ import (
 
 //nolint:gosec // ignore lint warnings that these are credentials
 const (
+	SecretLabelKey = "storage.pinniped.dev/type"
+
 	secretNameFormat = "pinniped-storage-%s-%s"
-	secretLabelKey   = "storage.pinniped.dev/type"
 	secretTypeFormat = "storage.pinniped.dev/%s"
 	secretVersion    = "1"
 	secretDataKey    = "pinniped-storage-data"
@@ -90,7 +91,7 @@ func (s *secretsStorage) validateSecret(secret *corev1.Secret) error {
 	if secret.Type != s.secretType {
 		return fmt.Errorf("%w: %s must equal %s", ErrSecretTypeMismatch, secret.Type, s.secretType)
 	}
-	if labelResource := secret.Labels[secretLabelKey]; labelResource != s.resource {
+	if labelResource := secret.Labels[SecretLabelKey]; labelResource != s.resource {
 		return fmt.Errorf("%w: %s must equal %s", ErrSecretLabelMismatch, labelResource, s.resource)
 	}
 	if !bytes.Equal(secret.Data[secretVersionKey], s.secretVersion) {
@@ -121,7 +122,7 @@ func (s *secretsStorage) Delete(ctx context.Context, signature string) error {
 func (s *secretsStorage) DeleteByLabel(ctx context.Context, labelName string, labelValue string) error {
 	list, err := s.secrets.List(ctx, metav1.ListOptions{
 		LabelSelector: labels.Set{
-			secretLabelKey: s.resource,
+			SecretLabelKey: s.resource,
 			labelName:      labelValue,
 		}.String(),
 	})
@@ -158,7 +159,7 @@ func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON,
 	}
 
 	labels := map[string]string{
-		secretLabelKey: s.resource, // make it easier to find this stuff via kubectl
+		SecretLabelKey: s.resource, // make it easier to find this stuff via kubectl
 	}
 	for labelName, labelValue := range additionalLabels {
 		labels[labelName] = labelValue

internal/fositestorage/accesstoken/accesstoken.go

@@ -19,6 +19,8 @@ import (
 )
 
 const (
+	TypeLabelValue = "access-token"
+
 	ErrInvalidAccessTokenRequestVersion = constable.Error("access token request data has wrong version")
 	ErrInvalidAccessTokenRequestData    = constable.Error("access token request data must be present")
 
@@ -42,7 +44,7 @@ type session struct {
 }
 
 func New(secrets corev1client.SecretInterface) RevocationStorage {
-	return &accessTokenStorage{storage: crud.New("access-token", secrets)}
+	return &accessTokenStorage{storage: crud.New(TypeLabelValue, secrets)}
 }
 
 func (a *accessTokenStorage) RevokeAccessToken(ctx context.Context, requestID string) error {

internal/fositestorage/authorizationcode/authorizationcode.go

@@ -20,6 +20,8 @@ import (
 )
 
 const (
+	TypeLabelValue = "authcode"
+
 	ErrInvalidAuthorizeRequestData    = constable.Error("authorization request data must be present")
 	ErrInvalidAuthorizeRequestVersion = constable.Error("authorization request data has wrong version")
 
@@ -39,7 +41,7 @@ type AuthorizeCodeSession struct {
 }
 
 func New(secrets corev1client.SecretInterface) oauth2.AuthorizeCodeStorage {
-	return &authorizeCodeStorage{storage: crud.New("authcode", secrets)}
+	return &authorizeCodeStorage{storage: crud.New(TypeLabelValue, secrets)}
 }
 
 func (a *authorizeCodeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error {

internal/fositestorage/openidconnect/openidconnect.go

@@ -19,6 +19,8 @@ import (
 )
 
 const (
+	TypeLabelValue = "oidc"
+
 	ErrInvalidOIDCRequestVersion  = constable.Error("oidc request data has wrong version")
 	ErrInvalidOIDCRequestData     = constable.Error("oidc request data must be present")
 	ErrMalformedAuthorizationCode = constable.Error("malformed authorization code")
@@ -38,7 +40,7 @@ type session struct {
 }
 
 func New(secrets corev1client.SecretInterface) openid.OpenIDConnectRequestStorage {
-	return &openIDConnectRequestStorage{storage: crud.New("oidc", secrets)}
+	return &openIDConnectRequestStorage{storage: crud.New(TypeLabelValue, secrets)}
 }
 
 func (a *openIDConnectRequestStorage) CreateOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) error {

internal/fositestorage/pkce/pkce.go

@@ -19,6 +19,8 @@ import (
 )
 
 const (
+	TypeLabelValue = "pkce"
+
 	ErrInvalidPKCERequestVersion = constable.Error("pkce request data has wrong version")
 	ErrInvalidPKCERequestData    = constable.Error("pkce request data must be present")
 
@@ -37,7 +39,7 @@ type session struct {
 }
 
 func New(secrets corev1client.SecretInterface) pkce.PKCERequestStorage {
-	return &pkceStorage{storage: crud.New("pkce", secrets)}
+	return &pkceStorage{storage: crud.New(TypeLabelValue, secrets)}
 }
 
 func (a *pkceStorage) CreatePKCERequestSession(ctx context.Context, signature string, requester fosite.Requester) error {

internal/fositestorage/refreshtoken/refreshtoken.go

@@ -19,6 +19,8 @@ import (
 )
 
 const (
+	TypeLabelValue = "refresh-token"
+
 	ErrInvalidRefreshTokenRequestVersion = constable.Error("refresh token request data has wrong version")
 	ErrInvalidRefreshTokenRequestData    = constable.Error("refresh token request data must be present")
 
@@ -42,7 +44,7 @@ type session struct {
 }
 
 func New(secrets corev1client.SecretInterface) RevocationStorage {
-	return &refreshTokenStorage{storage: crud.New("refresh-token", secrets)}
+	return &refreshTokenStorage{storage: crud.New(TypeLabelValue, secrets)}
 }
 
 func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID string) error {

internal/oidc/callback/callback_handler_test.go

@@ -18,18 +18,20 @@ import (
 	"github.com/ory/fosite"
 	"github.com/ory/fosite/handler/openid"
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/fake"
-	kubetesting "k8s.io/client-go/testing"
 
+	"go.pinniped.dev/internal/crud"
+	"go.pinniped.dev/internal/fositestorage/authorizationcode"
+	"go.pinniped.dev/internal/fositestorage/openidconnect"
+	"go.pinniped.dev/internal/fositestorage/pkce"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
 	"go.pinniped.dev/internal/oidc/oidctestutil"
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/pkg/oidcclient/nonce"
 	"go.pinniped.dev/pkg/oidcclient/oidctypes"
-	"go.pinniped.dev/pkg/oidcclient/pkce"
+	oidcpkce "go.pinniped.dev/pkg/oidcclient/pkce"
 )
 
 const (
@@ -106,7 +108,7 @@ func TestCallbackEndpoint(t *testing.T) {
 
 	happyExchangeAndValidateTokensArgs := &oidctestutil.ExchangeAuthcodeAndValidateTokenArgs{
 		Authcode:             happyUpstreamAuthcode,
-		PKCECodeVerifier:     pkce.Code(happyDownstreamPKCE),
+		PKCECodeVerifier:     oidcpkce.Code(happyDownstreamPKCE),
 		ExpectedIDTokenNonce: nonce.Nonce(happyDownstreamNonce),
 		RedirectURI:          happyUpstreamRedirectURI,
 	}
@@ -484,18 +486,8 @@ func TestCallbackEndpoint(t *testing.T) {
 				}
 				require.Len(t, client.Actions(), expectedNumberOfCreatedSecrets)
 
-				actualSecretNames := []string{}
-				for i := range client.Actions() {
-					actualAction := client.Actions()[i].(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.Empty(t, actualSecret.Namespace) // because the secrets client is already scoped to a namespace
-					actualSecretNames = append(actualSecretNames, actualSecret.Name)
-				}
-
 				// One authcode should have been stored.
-				requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-authcode-")
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
 
 				storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage(
 					t,
@@ -508,7 +500,7 @@ func TestCallbackEndpoint(t *testing.T) {
 				)
 
 				// One PKCE should have been stored.
-				requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-pkce-")
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: pkce.TypeLabelValue}, 1)
 
 				validatePKCEStorage(
 					t,
@@ -522,7 +514,7 @@ func TestCallbackEndpoint(t *testing.T) {
 
 				// One IDSession should have been stored, if the downstream actually requested the "openid" scope
 				if test.wantGrantedOpenidScope {
-					requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-oidc")
+					testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
 
 					validateIDSessionStorage(
 						t,
@@ -684,7 +676,7 @@ func (u *upstreamOIDCIdentityProviderBuilder) Build() oidctestutil.TestUpstreamO
 		UsernameClaim: u.usernameClaim,
 		GroupsClaim:   u.groupsClaim,
 		Scopes:        []string{"scope1", "scope2"},
-		ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (oidctypes.Token, map[string]interface{}, error) {
+		ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier oidcpkce.Code, expectedIDTokenNonce nonce.Nonce) (oidctypes.Token, map[string]interface{}, error) {
 			return oidctypes.Token{}, u.idToken, u.authcodeExchangeErr
 		},
 	}
@@ -847,15 +839,3 @@ func castStoredAuthorizeRequest(t *testing.T, storedAuthorizeRequest fosite.Requ
 
 	return storedRequest, storedSession
 }
-
-func requireAnyStringHasPrefix(t *testing.T, stringList []string, prefix string) {
-	t.Helper()
-
-	containsPrefix := false
-	for i := range stringList {
-		if strings.HasPrefix(stringList[i], prefix) {
-			containsPrefix = true
-		}
-	}
-	require.Truef(t, containsPrefix, "list %v did not contain any strings with prefix %s", stringList, prefix)
-}

internal/oidc/token/token_handler_test.go

@@ -26,8 +26,15 @@ import (
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/square/go-jose.v2"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/kubernetes/fake"
 
+	"go.pinniped.dev/internal/crud"
+	"go.pinniped.dev/internal/fositestorage/accesstoken"
+	"go.pinniped.dev/internal/fositestorage/authorizationcode"
+	"go.pinniped.dev/internal/fositestorage/openidconnect"
+	storagepkce "go.pinniped.dev/internal/fositestorage/pkce"
+	"go.pinniped.dev/internal/fositestorage/refreshtoken"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/internal/oidc"
 	"go.pinniped.dev/internal/oidc/jwks"
@@ -394,7 +401,14 @@ func TestTokenEndpoint(t *testing.T) {
 			}
 			subject := NewHandler(oauthHelper)
 
-			// TODO add assertions about how many of each storage type exist at this point as a pre-condition
+			testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+			testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 1)
+			if strings.Contains(authRequest.Form.Get("scope"), "openid") {
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3)
+			} else {
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2)
+			}
 
 			req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyBody(authCode).ReadCloser())
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
@@ -421,11 +435,18 @@ func TestTokenEndpoint(t *testing.T) {
 				requireInvalidPKCEStorage(t, code, oauthStore)
 				requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
 
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 1)
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0)
+				testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0)
+
 				if wantOpenidScope {
 					requireValidIDToken(t, m, jwtSigningKey)
+					testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+					testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3)
+				} else {
+					testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2)
 				}
-
-				// TODO add assertions about how many of each storage type are remaining at this point
 			} else {
 				require.JSONEq(t, test.wantExactBody, rsp.Body.String())
 			}
@@ -442,7 +463,10 @@ func TestTokenEndpoint(t *testing.T) {
 		oauthHelper, authCode, jwtSigningKey := makeHappyOauthHelper(t, authRequest, oauthStore)
 		subject := NewHandler(oauthHelper)
 
-		// TODO add assertions about how many of each storage type exist at this point as a pre-condition
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3)
 
 		req := httptest.NewRequest("POST", "/path/shouldn't/matter", happyBody(authCode).ReadCloser())
 		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
@@ -469,7 +493,12 @@ func TestTokenEndpoint(t *testing.T) {
 		requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
 		requireValidIDToken(t, m, jwtSigningKey)
 
-		// TODO add assertions about how many of each storage type are remaining at this point
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 3)
 
 		// Second call - should be unsuccessful since auth code was already used.
 		//
@@ -488,7 +517,12 @@ func TestTokenEndpoint(t *testing.T) {
 		requireInvalidPKCEStorage(t, code, oauthStore)
 		requireValidOIDCStorage(t, m, code, oauthStore, wantOpenidScope)
 
-		// TODO add assertions about how many of each storage type are remaining at this point
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: accesstoken.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: refreshtoken.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: storagepkce.TypeLabelValue}, 0)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: openidconnect.TypeLabelValue}, 1)
+		testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{}, 2)
 	})
 }
 

internal/testutil/assertions.go

@@ -4,11 +4,15 @@
 package testutil
 
 import (
+	"context"
 	"mime"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
+	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
 )
 
 func RequireTimeInDelta(t *testing.T, t1 time.Time, t2 time.Time, delta time.Duration) {
@@ -39,3 +43,12 @@ func RequireEqualContentType(t *testing.T, actual string, expected string) {
 	require.Equal(t, actualContentType, expectedContentType)
 	require.Equal(t, actualContentTypeParams, expectedContentTypeParams)
 }
+
+func RequireNumberOfSecretsMatchingLabelSelector(t *testing.T, secrets v1.SecretInterface, labelSet labels.Set, expectedNumberOfSecrets int) {
+	t.Helper()
+	storedAuthcodeSecrets, err := secrets.List(context.Background(), v12.ListOptions{
+		LabelSelector: labelSet.String(),
+	})
+	require.NoError(t, err)
+	require.Len(t, storedAuthcodeSecrets.Items, expectedNumberOfSecrets)
+}