From df0e715bb7c8c9484c1884f6dabe104552543490 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 12 May 2021 09:05:13 -0700 Subject: [PATCH 1/4] Add integration test that waits for access token expiry --- test/integration/e2e_test.go | 50 +++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index b70a5053..f624b01e 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -6,6 +6,7 @@ import ( "bufio" "bytes" "context" + "encoding/base32" "encoding/base64" "errors" "fmt" @@ -25,11 +26,13 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" authv1alpha "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient" @@ -42,7 +45,7 @@ import ( func TestE2EFullIntegration(t *testing.T) { env := library.IntegrationEnv(t) - ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute) + ctx, cancelFunc := context.WithTimeout(context.Background(), 15*time.Minute) defer cancelFunc() // Build pinniped CLI. @@ -261,6 +264,8 @@ func TestE2EFullIntegration(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) // Run kubectl again, which should work with no browser interaction. + t.Logf("Waiting 6 minutes to allow access token to expire") + time.Sleep(6 * time.Minute) kubectlCmd2 := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath) kubectlCmd2.Env = append(os.Environ(), env.ProxyEnv()...) start = time.Now() @@ -284,6 +289,38 @@ func TestE2EFullIntegration(t *testing.T) { }) require.NotNil(t, token) + // check that the access token is new (since it's just been refreshed) and has close to two minutes left. + testutil.RequireTimeInDelta(t, start.Add(2*time.Minute), token.AccessToken.Expiry.Time, 15*time.Second) + + kubeClient := library.NewKubernetesClientset(t).CoreV1() + + // get the access token secret that matches the signature from the cache + accessTokenSignature := strings.Split(token.AccessToken.Token, ".")[1] + accessSecretName := getSecretNameFromSignature(t, accessTokenSignature, "access-token") + accessTokenSecret, err := kubeClient.Secrets(env.SupervisorNamespace).Get(ctx, accessSecretName, metav1.GetOptions{}) + require.NoError(t, err) + + // Check that the access token garbage-collect-after value is 9 hours from now + accessTokenGCTimeString := accessTokenSecret.Annotations["storage.pinniped.dev/garbage-collect-after"] + accessTokenGCTime, err := time.Parse(crud.SecretLifetimeAnnotationDateFormat, accessTokenGCTimeString) + require.NoError(t, err) + require.True(t, accessTokenGCTime.After(time.Now().Add(9*time.Hour))) + + // get the refresh token secret that matches the signature from the cache + refreshTokenSignature := strings.Split(token.RefreshToken.Token, ".")[1] + refreshSecretName := getSecretNameFromSignature(t, refreshTokenSignature, "refresh-token") + refreshTokenSecret, err := kubeClient.Secrets(env.SupervisorNamespace).Get(ctx, refreshSecretName, metav1.GetOptions{}) + require.NoError(t, err) + + // Check that the refresh token garbage-collect-after value is 9 hours + refreshTokenGCTimeString := refreshTokenSecret.Annotations["storage.pinniped.dev/garbage-collect-after"] + refreshTokenGCTime, err := time.Parse(crud.SecretLifetimeAnnotationDateFormat, refreshTokenGCTimeString) + require.NoError(t, err) + require.True(t, refreshTokenGCTime.After(time.Now().Add(9*time.Hour))) + + // the access token and the refresh token should be garbage collected at essentially the same time + testutil.RequireTimeInDelta(t, accessTokenGCTime, refreshTokenGCTime, 1*time.Minute) + idTokenClaims := token.IDToken.Claims require.Equal(t, env.SupervisorTestUpstream.Username, idTokenClaims[oidc.DownstreamUsernameClaim]) @@ -338,3 +375,14 @@ status: append(env.SupervisorTestUpstream.ExpectedGroups, "system:authenticated"), ) } + +func getSecretNameFromSignature(t *testing.T, signature string, typeLabel string) string { + t.Helper() + // try to decode base64 signatures to prevent double encoding of binary data + signatureBytes, err := base64.RawURLEncoding.DecodeString(signature) + require.NoError(t, err) + // lower case base32 encoding insures that our secret name is valid per ValidateSecretName in k/k + var b32 = base32.StdEncoding.WithPadding(base32.NoPadding) + signatureAsValidName := strings.ToLower(b32.EncodeToString(signatureBytes)) + return fmt.Sprintf("pinniped-storage-%s-%s", typeLabel, signatureAsValidName) +} From 874f938fc7ca25f7adcbe5bdb9f500663ed3e4b1 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 12 May 2021 13:55:54 -0700 Subject: [PATCH 2/4] unit test for garbage collection time for refresh and access tokens --- internal/oidc/token/token_handler_test.go | 30 +++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 3f47e987..0521bbfa 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -9,8 +9,10 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/sha256" + "encoding/base32" "encoding/base64" "encoding/json" + "fmt" "io" "io/ioutil" "net/http" @@ -1201,7 +1203,7 @@ func requireTokenEndpointBehavior( wantRefreshToken := contains(test.wantSuccessBodyFields, "refresh_token") requireInvalidAuthCodeStorage(t, authCode, oauthStore) - requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) + requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, secrets) requireInvalidPKCEStorage(t, authCode, oauthStore) requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) @@ -1215,7 +1217,7 @@ func requireTokenEndpointBehavior( requireValidIDToken(t, parsedResponseBody, jwtSigningKey, wantAtHashClaimInIDToken, wantNonceValueInIDToken, parsedResponseBody["access_token"].(string)) } if wantRefreshToken { - requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) + requireValidRefreshTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, secrets) } testutil.RequireNumberOfSecretsMatchingLabelSelector(t, secrets, labels.Set{crud.SecretLabelKey: authorizationcode.TypeLabelValue}, 1) @@ -1450,6 +1452,7 @@ func requireValidRefreshTokenStorage( storage oauth2.CoreStorage, wantRequestedScopes []string, wantGrantedScopes []string, + secrets v1.SecretInterface, ) { t.Helper() @@ -1471,6 +1474,8 @@ func requireValidRefreshTokenStorage( wantGrantedScopes, true, ) + + requireGarbageCollectTimeInDelta(t, refreshTokenString, "refresh-token", secrets, time.Now().Add(9*time.Hour).Add(2*time.Minute), 1*time.Minute) } func requireValidAccessTokenStorage( @@ -1479,6 +1484,7 @@ func requireValidAccessTokenStorage( storage oauth2.CoreStorage, wantRequestedScopes []string, wantGrantedScopes []string, + secrets v1.SecretInterface, ) { t.Helper() @@ -1519,6 +1525,8 @@ func requireValidAccessTokenStorage( wantGrantedScopes, true, ) + + requireGarbageCollectTimeInDelta(t, accessTokenString, "access-token", secrets, time.Now().Add(9*time.Hour).Add(2*time.Minute), 1*time.Minute) } func requireInvalidAccessTokenStorage( @@ -1681,6 +1689,24 @@ func requireValidStoredRequest( require.Empty(t, session.Subject) } +func requireGarbageCollectTimeInDelta(t *testing.T, tokenString string, typeLabel string, secrets v1.SecretInterface, wantExpirationTime time.Time, deltaTime time.Duration) { + t.Helper() + signature := getFositeDataSignature(t, tokenString) + signatureBytes, err := base64.RawURLEncoding.DecodeString(signature) + require.NoError(t, err) + // lower case base32 encoding insures that our secret name is valid per ValidateSecretName in k/k + var b32 = base32.StdEncoding.WithPadding(base32.NoPadding) + signatureAsValidName := strings.ToLower(b32.EncodeToString(signatureBytes)) + secretName := fmt.Sprintf("pinniped-storage-%s-%s", typeLabel, signatureAsValidName) + secret, err := secrets.Get(context.Background(), secretName, metav1.GetOptions{}) + require.NoError(t, err) + refreshTokenGCTimeString := secret.Annotations["storage.pinniped.dev/garbage-collect-after"] + refreshTokenGCTime, err := time.Parse(crud.SecretLifetimeAnnotationDateFormat, refreshTokenGCTimeString) + require.NoError(t, err) + + testutil.RequireTimeInDelta(t, refreshTokenGCTime, wantExpirationTime, deltaTime) +} + func requireValidIDToken( t *testing.T, body map[string]interface{}, From b391d5ae0291bf157ce2866451d9e341f9f9dc47 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 12 May 2021 14:22:14 -0700 Subject: [PATCH 3/4] Also check that the authcode storage is around for a while --- internal/oidc/token/token_handler_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 0521bbfa..b13a64cb 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -577,7 +577,7 @@ func TestTokenEndpointWhenAuthcodeIsUsedTwice(t *testing.T) { require.JSONEq(t, fositeReusedAuthCodeErrorBody, reusedAuthcodeResponse.Body.String()) // This was previously invalidated by the first request, so it remains invalidated - requireInvalidAuthCodeStorage(t, authCode, oauthStore) + requireInvalidAuthCodeStorage(t, authCode, oauthStore, secrets) // Has now invalidated the access token that was previously handed out by the first request requireInvalidAccessTokenStorage(t, parsedResponseBody, oauthStore) // This was previously invalidated by the first request, so it remains invalidated @@ -1202,7 +1202,7 @@ func requireTokenEndpointBehavior( wantIDToken := contains(test.wantSuccessBodyFields, "id_token") wantRefreshToken := contains(test.wantSuccessBodyFields, "refresh_token") - requireInvalidAuthCodeStorage(t, authCode, oauthStore) + requireInvalidAuthCodeStorage(t, authCode, oauthStore, secrets) requireValidAccessTokenStorage(t, parsedResponseBody, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes, secrets) requireInvalidPKCEStorage(t, authCode, oauthStore) requireValidOIDCStorage(t, parsedResponseBody, authCode, oauthStore, test.wantRequestedScopes, test.wantGrantedScopes) @@ -1438,12 +1438,15 @@ func requireInvalidAuthCodeStorage( t *testing.T, code string, storage oauth2.CoreStorage, + secrets v1.SecretInterface, ) { t.Helper() // Make sure we have invalidated this auth code. _, err := storage.GetAuthorizeCodeSession(context.Background(), getFositeDataSignature(t, code), nil) require.True(t, errors.Is(err, fosite.ErrInvalidatedAuthorizeCode)) + // make sure that its still around in storage so if someone tries to use it again we invalidate everything + requireGarbageCollectTimeInDelta(t, code, "authcode", secrets, time.Now().Add(9*time.Hour).Add(10*time.Minute), 30*time.Second) } func requireValidRefreshTokenStorage( From 6479015caf8e4119825561285fea3fb7365bb7cb Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 13 May 2021 10:23:44 -0700 Subject: [PATCH 4/4] Remove timeout so this test doesnt take forever --- test/integration/e2e_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index f624b01e..be1220d6 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -264,8 +264,6 @@ func TestE2EFullIntegration(t *testing.T) { t.Logf("first kubectl command took %s", time.Since(start).String()) // Run kubectl again, which should work with no browser interaction. - t.Logf("Waiting 6 minutes to allow access token to expire") - time.Sleep(6 * time.Minute) kubectlCmd2 := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath) kubectlCmd2.Env = append(os.Environ(), env.ProxyEnv()...) start = time.Now()