From d60c184424f60b751e317249551cfe33890470e7 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 1 Dec 2020 17:18:32 -0800 Subject: [PATCH] Add pkce and openidconnect storage - Also refactor authorizationcode_test Signed-off-by: Ryan Richard --- .../authorizationcode/authorizationcode.go | 4 +- .../authorizationcode_test.go | 309 ++++++++++-------- .../openidconnect/openidconnect.go | 124 +++++++ .../openidconnect/openidconnect_test.go | 209 ++++++++++++ .../oidc/callback/callback_handler_test.go | 43 ++- internal/oidc/kube_storage.go | 22 +- test/library/iotest.go | 1 + 7 files changed, 546 insertions(+), 166 deletions(-) create mode 100644 internal/fositestorage/openidconnect/openidconnect.go create mode 100644 internal/fositestorage/openidconnect/openidconnect_test.go diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index 0c522985..8aca618a 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -20,7 +20,7 @@ import ( ) const ( - ErrInvalidAuthorizeRequestData = constable.Error("authorization request data must not be nil") + ErrInvalidAuthorizeRequestData = constable.Error("authorization request data must be present") ErrInvalidAuthorizeRequestVersion = constable.Error("authorization request data has wrong version") authorizeCodeStorageVersion = "1" @@ -119,7 +119,7 @@ func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string) ErrInvalidAuthorizeRequestVersion, signature, version, authorizeCodeStorageVersion) } - if session.Request == nil { + if session.Request.ID == "" { return nil, "", fmt.Errorf("malformed authorization code session for %s: %w", signature, ErrInvalidAuthorizeRequestData) } diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index 9d03995a..38f3e1a9 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -16,8 +16,8 @@ import ( fuzz "github.com/google/gofuzz" "github.com/ory/fosite" - "github.com/ory/fosite/handler/oauth2" "github.com/ory/fosite/handler/openid" + "github.com/pkg/errors" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" corev1 "k8s.io/api/core/v1" @@ -29,7 +29,9 @@ import ( "go.pinniped.dev/internal/fositestorage" ) -func TestAuthorizeCodeStorage(t *testing.T) { +const namespace = "test-ns" + +func TestAuthorizationCodeStorage(t *testing.T) { ctx := context.Background() secretsGVR := schema.GroupVersionResource{ Group: "", @@ -37,151 +39,186 @@ func TestAuthorizeCodeStorage(t *testing.T) { Resource: "secrets", } - const namespace = "test-ns" - - type mocker interface { - AddReactor(verb, resource string, reaction coretesting.ReactionFunc) - PrependReactor(verb, resource string, reaction coretesting.ReactionFunc) - Tracker() coretesting.ObjectTracker - } - - tests := []struct { - name string - mocks func(*testing.T, mocker) - run func(*testing.T, oauth2.AuthorizeCodeStorage) error - wantActions []coretesting.Action - wantSecrets []corev1.Secret - wantErr string - }{ - { - name: "create, get, invalidate standard flow", - mocks: nil, - run: func(t *testing.T, storage oauth2.AuthorizeCodeStorage) error { - 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: "", - }, - RequestedScope: nil, - GrantedScope: nil, - Form: url.Values{"key": []string{"val"}}, - Session: &openid.DefaultSession{ - Claims: nil, - Headers: nil, - ExpiresAt: nil, - Username: "snorlax", - Subject: "panda", - }, - RequestedAudience: nil, - GrantedAudience: nil, - } - err := storage.CreateAuthorizeCodeSession(ctx, "fancy-signature", request) - require.NoError(t, err) - - newRequest, err := storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) - require.NoError(t, err) - require.Equal(t, request, newRequest) - - return storage.InvalidateAuthorizeCodeSession(ctx, "fancy-signature") - }, - wantActions: []coretesting.Action{ - coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", - ResourceVersion: "", - Labels: map[string]string{ - "storage.pinniped.dev": "authcode", - }, - }, - Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), - "pinniped-storage-version": []byte("1"), - }, - Type: "storage.pinniped.dev/authcode", - }), - coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authcode-pwu5zs7lekbhnln2w4"), - coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authcode-pwu5zs7lekbhnln2w4"), - coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", - ResourceVersion: "", - Labels: map[string]string{ - "storage.pinniped.dev": "authcode", - }, - }, - Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), - "pinniped-storage-version": []byte("1"), - }, - Type: "storage.pinniped.dev/authcode", - }), - }, - wantSecrets: []corev1.Secret{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", - Namespace: namespace, - ResourceVersion: "", - Labels: map[string]string{ - "storage.pinniped.dev": "authcode", - }, - }, - Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), - "pinniped-storage-version": []byte("1"), - }, - Type: "storage.pinniped.dev/authcode", + wantActions := []coretesting.Action{ + coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authcode", }, }, - wantErr: "", + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"active":true,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/authcode", + }), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authcode-pwu5zs7lekbhnln2w4"), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authcode-pwu5zs7lekbhnln2w4"), + coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authcode", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"active":false,"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/authcode", + }), + } + + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + 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: "", }, + RequestedScope: nil, + GrantedScope: nil, + Form: url.Values{"key": []string{"val"}}, + Session: &openid.DefaultSession{ + Claims: nil, + Headers: nil, + ExpiresAt: nil, + Username: "snorlax", + Subject: "panda", + }, + RequestedAudience: nil, + GrantedAudience: nil, } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + err := storage.CreateAuthorizeCodeSession(ctx, "fancy-signature", request) + require.NoError(t, err) - client := fake.NewSimpleClientset() - if tt.mocks != nil { - tt.mocks(t, client) - } - secrets := client.CoreV1().Secrets(namespace) - storage := New(secrets) + newRequest, err := storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) + require.NoError(t, err) + require.Equal(t, request, newRequest) - err := tt.run(t, storage) + err = storage.InvalidateAuthorizeCodeSession(ctx, "fancy-signature") + require.NoError(t, err) - require.Equal(t, tt.wantErr, errString(err)) - require.Equal(t, tt.wantActions, client.Actions()) - - actualSecrets, err := secrets.List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.Equal(t, tt.wantSecrets, actualSecrets.Items) - }) - } + require.Equal(t, wantActions, client.Actions()) } -func errString(err error) string { - if err == nil { - return "" +func TestGetNotFound(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + _, notFoundErr := storage.GetAuthorizeCodeSession(ctx, "non-existent-signature", nil) + require.EqualError(t, notFoundErr, "not_found") + require.True(t, errors.Is(notFoundErr, fosite.ErrNotFound)) +} + +func TestWrongVersion(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authcode", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"not-the-right-version", "active": true}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/authcode", + } + _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + + _, err = storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) + + require.EqualError(t, err, "authorization request data has wrong version: authorization code session for fancy-signature has version not-the-right-version instead of 1") +} + +func TestNilSessionRequest(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authcode-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authcode", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"nonsense-key": "nonsense-value", "version":"1", "active": true}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/authcode", } - return err.Error() + _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + + _, err = storage.GetAuthorizeCodeSession(ctx, "fancy-signature", nil) + require.EqualError(t, err, "malformed authorization code session for fancy-signature: authorization request data must be present") +} + +func TestCreateWithNilRequester(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + err := storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", nil) + require.EqualError(t, err, "requester must be of type fosite.Request") +} + +func TestCreateWithWrongRequesterDataTypes(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + request := &fosite.Request{ + Session: nil, + Client: &fosite.DefaultOpenIDConnectClient{}, + } + err := storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", request) + require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") + + request = &fosite.Request{ + Session: &openid.DefaultSession{}, + Client: nil, + } + err = storage.CreateAuthorizeCodeSession(ctx, "signature-doesnt-matter", request) + require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") } // TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession asserts that we can correctly round trip our authorize code session. diff --git a/internal/fositestorage/openidconnect/openidconnect.go b/internal/fositestorage/openidconnect/openidconnect.go new file mode 100644 index 00000000..797d21a8 --- /dev/null +++ b/internal/fositestorage/openidconnect/openidconnect.go @@ -0,0 +1,124 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package openidconnect + +import ( + "context" + "fmt" + "strings" + + "github.com/ory/fosite" + "github.com/ory/fosite/handler/openid" + "k8s.io/apimachinery/pkg/api/errors" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + + "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/crud" + "go.pinniped.dev/internal/fositestorage" +) + +const ( + ErrInvalidOIDCRequestVersion = constable.Error("oidc request data has wrong version") + ErrInvalidOIDCRequestData = constable.Error("oidc request data must be present") + ErrMalformedAuthorizationCode = constable.Error("malformed authorization code") + + oidcStorageVersion = "1" +) + +var _ openid.OpenIDConnectRequestStorage = &openIDConnectRequestStorage{} + +type openIDConnectRequestStorage struct { + storage crud.Storage +} + +type session struct { + Request *fosite.Request `json:"request"` + Version string `json:"version"` +} + +func New(secrets corev1client.SecretInterface) openid.OpenIDConnectRequestStorage { + return &openIDConnectRequestStorage{storage: crud.New("oidc", secrets)} +} + +func (a *openIDConnectRequestStorage) CreateOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) error { + signature, err := getSignature(authcode) + if err != nil { + return err + } + + request, err := fositestorage.ValidateAndExtractAuthorizeRequest(requester) + if err != nil { + return err + } + + _, err = a.storage.Create(ctx, signature, &session{Request: request, Version: oidcStorageVersion}) + return err +} + +func (a *openIDConnectRequestStorage) GetOpenIDConnectSession(ctx context.Context, authcode string, _ fosite.Requester) (fosite.Requester, error) { + signature, err := getSignature(authcode) + if err != nil { + return nil, err + } + + session, _, err := a.getSession(ctx, signature) + + if err != nil { + return nil, err + } + + return session.Request, err +} + +func (a *openIDConnectRequestStorage) DeleteOpenIDConnectSession(ctx context.Context, authcode string) error { + signature, err := getSignature(authcode) + if err != nil { + return err + } + + return a.storage.Delete(ctx, signature) +} + +func (a *openIDConnectRequestStorage) getSession(ctx context.Context, signature string) (*session, string, error) { + session := newValidEmptyOIDCSession() + rv, err := a.storage.Get(ctx, signature, session) + + if errors.IsNotFound(err) { + return nil, "", fosite.ErrNotFound.WithCause(err).WithDebug(err.Error()) + } + + if err != nil { + return nil, "", fmt.Errorf("failed to get oidc session for %s: %w", signature, err) + } + + if version := session.Version; version != oidcStorageVersion { + return nil, "", fmt.Errorf("%w: oidc session for %s has version %s instead of %s", + ErrInvalidOIDCRequestVersion, signature, version, oidcStorageVersion) + } + + if session.Request.ID == "" { + return nil, "", fmt.Errorf("malformed oidc session for %s: %w", signature, ErrInvalidOIDCRequestData) + } + + return session, rv, nil +} + +func newValidEmptyOIDCSession() *session { + return &session{ + Request: &fosite.Request{ + Client: &fosite.DefaultOpenIDConnectClient{}, + Session: &openid.DefaultSession{}, + }, + } +} + +func getSignature(authorizationCode string) (string, error) { + split := strings.Split(authorizationCode, ".") + + if len(split) != 2 { + return "", ErrMalformedAuthorizationCode + } + + return split[1], nil +} diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go new file mode 100644 index 00000000..976828ed --- /dev/null +++ b/internal/fositestorage/openidconnect/openidconnect_test.go @@ -0,0 +1,209 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package openidconnect + +import ( + "context" + "net/url" + "testing" + "time" + + "github.com/ory/fosite" + "github.com/ory/fosite/handler/openid" + "github.com/pkg/errors" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" +) + +const namespace = "test-ns" + +func TestOpenIdConnectStorage(t *testing.T) { + ctx := context.Background() + secretsGVR := schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "secrets", + } + + wantActions := []coretesting.Action{ + coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-oidc-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "oidc", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"1"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/oidc", + }), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-oidc-pwu5zs7lekbhnln2w4"), + coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-oidc-pwu5zs7lekbhnln2w4"), + } + + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + 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: "", + }, + RequestedScope: nil, + GrantedScope: nil, + Form: url.Values{"key": []string{"val"}}, + Session: &openid.DefaultSession{ + Claims: nil, + Headers: nil, + ExpiresAt: nil, + Username: "snorlax", + Subject: "panda", + }, + RequestedAudience: nil, + GrantedAudience: nil, + } + err := storage.CreateOpenIDConnectSession(ctx, "fancy-code.fancy-signature", request) + require.NoError(t, err) + + newRequest, err := storage.GetOpenIDConnectSession(ctx, "fancy-code.fancy-signature", nil) + require.NoError(t, err) + require.Equal(t, request, newRequest) + + err = storage.DeleteOpenIDConnectSession(ctx, "fancy-code.fancy-signature") + require.NoError(t, err) + + require.Equal(t, wantActions, client.Actions()) +} + +func TestGetNotFound(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + _, notFoundErr := storage.GetOpenIDConnectSession(ctx, "authcode.non-existent-signature", nil) + require.EqualError(t, notFoundErr, "not_found") + require.True(t, errors.Is(notFoundErr, fosite.ErrNotFound)) +} + +func TestWrongVersion(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-oidc-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "oidc", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"requestedAudience":null,"grantedAudience":null},"version":"not-the-right-version"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/oidc", + } + _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + + _, err = storage.GetOpenIDConnectSession(ctx, "fancy-code.fancy-signature", nil) + + require.EqualError(t, err, "oidc request data has wrong version: oidc session for fancy-signature has version not-the-right-version instead of 1") +} + +func TestNilSessionRequest(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-oidc-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "oidc", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"nonsense-key": "nonsense-value","version":"1"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/oidc", + } + + _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + + _, err = storage.GetOpenIDConnectSession(ctx, "fancy-code.fancy-signature", nil) + require.EqualError(t, err, "malformed oidc session for fancy-signature: oidc request data must be present") +} + +func TestCreateWithNilRequester(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + err := storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", nil) + require.EqualError(t, err, "requester must be of type fosite.Request") +} + +func TestCreateWithWrongRequesterDataTypes(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + request := &fosite.Request{ + Session: nil, + Client: &fosite.DefaultOpenIDConnectClient{}, + } + err := storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", request) + require.EqualError(t, err, "requester's session must be of type openid.DefaultSession") + + request = &fosite.Request{ + Session: &openid.DefaultSession{}, + Client: nil, + } + err = storage.CreateOpenIDConnectSession(ctx, "authcode.signature-doesnt-matter", request) + require.EqualError(t, err, "requester's client must be of type fosite.DefaultOpenIDConnectClient") +} + +func TestAuthcodeHasNoDot(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + err := storage.CreateOpenIDConnectSession(ctx, "all-one-part", nil) + require.EqualError(t, err, "malformed authorization code") +} diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index 6c12394a..e73eb759 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -479,13 +479,18 @@ 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. - 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 + requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-authcode-") storedRequestFromAuthcode, storedSessionFromAuthcode := validateAuthcodeStorage( t, @@ -498,12 +503,7 @@ func TestCallbackEndpoint(t *testing.T) { ) // 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 + requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-pkce-") validatePKCEStorage( t, @@ -517,12 +517,7 @@ func TestCallbackEndpoint(t *testing.T) { // 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 + requireAnyStringHasPrefix(t, actualSecretNames, "pinniped-storage-oidc") validateIDSessionStorage( t, @@ -847,3 +842,15 @@ 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) +} diff --git a/internal/oidc/kube_storage.go b/internal/oidc/kube_storage.go index 388cd771..405a6ade 100644 --- a/internal/oidc/kube_storage.go +++ b/internal/oidc/kube_storage.go @@ -7,16 +7,16 @@ import ( "context" "time" - fositepkce "github.com/ory/fosite/handler/pkce" - - "go.pinniped.dev/internal/fositestorage/pkce" - "github.com/ory/fosite" "github.com/ory/fosite/handler/oauth2" + "github.com/ory/fosite/handler/openid" + 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/authorizationcode" + "go.pinniped.dev/internal/fositestorage/openidconnect" + "go.pinniped.dev/internal/fositestorage/pkce" ) const errKubeStorageNotImplemented = constable.Error("KubeStorage does not implement this method. It should not have been called.") @@ -24,12 +24,14 @@ const errKubeStorageNotImplemented = constable.Error("KubeStorage does not imple type KubeStorage struct { authorizationCodeStorage oauth2.AuthorizeCodeStorage pkceStorage fositepkce.PKCERequestStorage + oidcStorage openid.OpenIDConnectRequestStorage } func NewKubeStorage(secrets corev1client.SecretInterface) *KubeStorage { return &KubeStorage{ authorizationCodeStorage: authorizationcode.New(secrets), pkceStorage: pkce.New(secrets), + oidcStorage: openidconnect.New(secrets), } } @@ -65,16 +67,16 @@ func (KubeStorage) DeleteAccessTokenSession(_ context.Context, _ string) (err er return errKubeStorageNotImplemented } -func (KubeStorage) CreateOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) error { - return nil +func (k KubeStorage) CreateOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) error { + return k.oidcStorage.CreateOpenIDConnectSession(ctx, authcode, requester) } -func (KubeStorage) GetOpenIDConnectSession(_ context.Context, _ string, _ fosite.Requester) (fosite.Requester, error) { - return nil, errKubeStorageNotImplemented +func (k KubeStorage) GetOpenIDConnectSession(ctx context.Context, authcode string, requester fosite.Requester) (fosite.Requester, error) { + return k.oidcStorage.GetOpenIDConnectSession(ctx, authcode, requester) } -func (KubeStorage) DeleteOpenIDConnectSession(_ context.Context, _ string) error { - return errKubeStorageNotImplemented +func (k KubeStorage) DeleteOpenIDConnectSession(ctx context.Context, authcode string) error { + return k.oidcStorage.DeleteOpenIDConnectSession(ctx, authcode) } func (k KubeStorage) GetPKCERequestSession(ctx context.Context, signature string, session fosite.Session) (fosite.Requester, error) { diff --git a/test/library/iotest.go b/test/library/iotest.go index daf2ed4e..dcb0e695 100644 --- a/test/library/iotest.go +++ b/test/library/iotest.go @@ -33,6 +33,7 @@ func (l *testlogReader) Read(p []byte) (n int, err error) { return } +//nolint: gochecknoglobals var tokenLike = regexp.MustCompile(`(?mi)[a-zA-Z0-9._-]{30,}|[a-zA-Z0-9]{20,}`) func maskTokens(in []byte) string {