diff --git a/Dockerfile b/Dockerfile index 7ddea4d4..be74db5f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # Copyright 2020 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -FROM golang:1.15.3 as build-env +FROM golang:1.15.5 as build-env WORKDIR /work # Get dependencies first so they can be cached as a layer @@ -13,6 +13,7 @@ RUN go mod download # Copy only the production source code to avoid cache misses when editing other files COPY generated ./generated COPY cmd ./cmd +COPY pkg ./pkg COPY internal ./internal COPY tools ./tools COPY hack ./hack diff --git a/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl index ea12b063..9be04701 100644 --- a/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl @@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct { // AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization // request flow with an OIDC identity provider. By default only the "openid" scope will be requested. // +optional - AdditionalScopes []string `json:"additionalScopes"` + AdditionalScopes []string `json:"additionalScopes,omitempty"` } // OIDCClaims provides a mapping from upstream claims into identities. @@ -82,7 +82,7 @@ type UpstreamOIDCProviderSpec struct { // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional - AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"` + AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig,omitempty"` // Claims provides the names of token claims that will be used when inspecting an identity from // this OIDC identity provider. diff --git a/cmd/pinniped/cmd/get_kubeconfig.go b/cmd/pinniped/cmd/get_kubeconfig.go index b7299c67..3f4b22b8 100644 --- a/cmd/pinniped/cmd/get_kubeconfig.go +++ b/cmd/pinniped/cmd/get_kubeconfig.go @@ -52,7 +52,7 @@ type getKubeConfigCommand struct { func newGetKubeConfigCommand() *getKubeConfigCommand { return &getKubeConfigCommand{ flags: getKubeConfigFlags{ - namespace: "pinniped", + namespace: "pinniped-concierge", }, getPathToSelf: os.Executable, kubeClientCreator: func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { diff --git a/cmd/pinniped/cmd/get_kubeconfig_test.go b/cmd/pinniped/cmd/get_kubeconfig_test.go index 099ab753..70488b3c 100644 --- a/cmd/pinniped/cmd/get_kubeconfig_test.go +++ b/cmd/pinniped/cmd/get_kubeconfig_test.go @@ -35,7 +35,7 @@ var ( -h, --help help for get-kubeconfig --kubeconfig string Path to the kubeconfig file --kubeconfig-context string Kubeconfig context override - --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped") + --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped-concierge") --token string Credential to include in the resulting kubeconfig output (Required) `) @@ -66,7 +66,7 @@ var ( -h, --help help for get-kubeconfig --kubeconfig string Path to the kubeconfig file --kubeconfig-context string Kubeconfig context override - --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped") + --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped-concierge") --token string Credential to include in the resulting kubeconfig output (Required) `) ) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index bf12e7be..1677c00f 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -18,8 +18,8 @@ import ( clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" "k8s.io/klog/v2/klogr" - "go.pinniped.dev/internal/oidcclient" - "go.pinniped.dev/internal/oidcclient/filesession" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/filesession" ) //nolint: gochecknoinits diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index a8bb03ed..3a61934d 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -12,7 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "go.pinniped.dev/internal/here" - "go.pinniped.dev/internal/oidcclient" + "go.pinniped.dev/pkg/oidcclient" ) func TestLoginOIDCCommand(t *testing.T) { diff --git a/cmd/pinniped/cmd/root.go b/cmd/pinniped/cmd/root.go index 58603081..8b5d01ae 100644 --- a/cmd/pinniped/cmd/root.go +++ b/cmd/pinniped/cmd/root.go @@ -4,7 +4,6 @@ package cmd import ( - "fmt" "os" "github.com/spf13/cobra" @@ -22,7 +21,6 @@ var rootCmd = &cobra.Command{ // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { if err := rootCmd.Execute(); err != nil { - fmt.Println(err) os.Exit(1) } } diff --git a/deploy/concierge/rbac.yaml b/deploy/concierge/rbac.yaml index 4f4c3305..7ebc75d1 100644 --- a/deploy/concierge/rbac.yaml +++ b/deploy/concierge/rbac.yaml @@ -24,6 +24,10 @@ rules: - apiGroups: [ policy ] resources: [ podsecuritypolicies ] verbs: [ use ] + - apiGroups: [ security.openshift.io ] + resources: [ securitycontextconstraints ] + verbs: [ use ] + resourceNames: [ nonroot ] --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/doc/demo.md b/doc/demo.md index 8589610f..3032bb3a 100644 --- a/doc/demo.md +++ b/doc/demo.md @@ -108,7 +108,7 @@ as the identity provider. | tee /tmp/local-user-authenticator-ca-base64-encoded ``` -1. Deploy Pinniped. +1. Deploy the Pinniped concierge. ```bash kubectl apply -f https://github.com/vmware-tanzu/pinniped/releases/download/$pinniped_version/install-pinniped-concierge.yaml @@ -121,7 +121,7 @@ as the identity provider. 1. Create a `WebhookAuthenticator` object to configure Pinniped to authenticate using local-user-authenticator. ```bash - cat < /tmp/pinniped-kubeconfig + pinniped get-kubeconfig --pinniped-namespace pinniped-concierge --token "pinny-the-seal:password123" --authenticator-type webhook --authenticator-name local-user-authenticator > /tmp/pinniped-kubeconfig ``` If you are using MacOS, you may get an error dialog that says @@ -162,7 +162,7 @@ as the identity provider. the `pinny-the-seal` user. ```bash - kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped + kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped-concierge ``` Because this user has no RBAC permissions on this cluster, the previous command @@ -179,7 +179,7 @@ as the identity provider. 1. Use the generated kubeconfig to issue arbitrary `kubectl` commands as the `pinny-the-seal` user. ```bash - kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped + kubectl --kubeconfig /tmp/pinniped-kubeconfig get pods -n pinniped-concierge ``` The user has permission to list pods, so the command succeeds this time. diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go index ea12b063..9be04701 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct { // AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization // request flow with an OIDC identity provider. By default only the "openid" scope will be requested. // +optional - AdditionalScopes []string `json:"additionalScopes"` + AdditionalScopes []string `json:"additionalScopes,omitempty"` } // OIDCClaims provides a mapping from upstream claims into identities. @@ -82,7 +82,7 @@ type UpstreamOIDCProviderSpec struct { // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional - AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"` + AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig,omitempty"` // Claims provides the names of token claims that will be used when inspecting an identity from // this OIDC identity provider. diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go index ea12b063..9be04701 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct { // AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization // request flow with an OIDC identity provider. By default only the "openid" scope will be requested. // +optional - AdditionalScopes []string `json:"additionalScopes"` + AdditionalScopes []string `json:"additionalScopes,omitempty"` } // OIDCClaims provides a mapping from upstream claims into identities. @@ -82,7 +82,7 @@ type UpstreamOIDCProviderSpec struct { // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional - AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"` + AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig,omitempty"` // Claims provides the names of token claims that will be used when inspecting an identity from // this OIDC identity provider. diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go index ea12b063..9be04701 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -41,7 +41,7 @@ type OIDCAuthorizationConfig struct { // AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization // request flow with an OIDC identity provider. By default only the "openid" scope will be requested. // +optional - AdditionalScopes []string `json:"additionalScopes"` + AdditionalScopes []string `json:"additionalScopes,omitempty"` } // OIDCClaims provides a mapping from upstream claims into identities. @@ -82,7 +82,7 @@ type UpstreamOIDCProviderSpec struct { // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional - AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"` + AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig,omitempty"` // Claims provides the names of token claims that will be used when inspecting an identity from // this OIDC identity provider. diff --git a/go.mod b/go.mod index fae73095..4277bd9b 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/golang/mock v1.4.4 github.com/golangci/golangci-lint v1.31.0 github.com/google/go-cmp v0.5.2 + github.com/google/gofuzz v1.1.0 github.com/gorilla/securecookie v1.1.1 github.com/ory/fosite v0.35.1 github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 diff --git a/hack/lib/tilt/Tiltfile b/hack/lib/tilt/Tiltfile index cc4d44d5..e657e967 100644 --- a/hack/lib/tilt/Tiltfile +++ b/hack/lib/tilt/Tiltfile @@ -19,23 +19,25 @@ local_resource( ) ##################################################################################################### -# Dex +# Test IDP (Dex + cert generation + squid proxy) # -# Render the Dex installation manifest using ytt. +# Render the IDP installation manifest using ytt. k8s_yaml(local(['ytt','--file', '../../../test/deploy/dex'])) # Tell tilt to watch all of those files for changes. watch_file('../../../test/deploy/dex') -# Collect all the deployed Dex resources under a "dex" resource tab. -k8s_resource( - workload='dex', # this is the deployment name - objects=[ - # these are the objects that would otherwise appear in the "uncategorized" tab in the tilt UI - 'dex:namespace', - 'dex-config:configmap', - ], -) +k8s_resource(objects=['dex:namespace'], new_name='dex-ns') +k8s_resource(workload='cert-issuer', resource_deps=['dex-ns'], objects=[ + 'cert-issuer:serviceaccount', + 'cert-issuer:role', + 'cert-issuer:rolebinding', +]) +k8s_resource(workload='proxy', resource_deps=['dex-ns']) +k8s_resource(workload='dex', resource_deps=['dex-ns', 'cert-issuer'], objects=[ + 'dex-config:configmap', +]) + ##################################################################################################### # Local-user-authenticator app @@ -186,6 +188,6 @@ k8s_resource( local_resource( 'test-env', 'TILT_MODE=yes ../../prepare-for-integration-tests.sh', - resource_deps=['local-user-auth', 'concierge', 'supervisor'], + resource_deps=['local-user-auth', 'concierge', 'supervisor', 'dex', 'proxy'], deps=['../../prepare-for-integration-tests.sh'], ) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 89b7f2f8..11e1fbf8 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -268,7 +268,7 @@ fi # # Download the test CA bundle that was generated in the Dex pod. # -test_ca_bundle_pem="$(kubectl exec -n dex deployment/dex -- cat /var/certs/ca.pem)" +test_ca_bundle_pem="$(kubectl get secrets -n dex certs -o go-template='{{index .data "ca.pem" | base64decode}}')" # # Create the environment file @@ -295,9 +295,16 @@ export PINNIPED_TEST_PROXY=http://127.0.0.1:12346 export PINNIPED_TEST_CLI_OIDC_ISSUER=https://dex.dex.svc.cluster.local/dex export PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE="${test_ca_bundle_pem}" export PINNIPED_TEST_CLI_OIDC_CLIENT_ID=pinniped-cli -export PINNIPED_TEST_CLI_OIDC_LOCALHOST_PORT=48095 +export PINNIPED_TEST_CLI_OIDC_CALLBACK_URL=http://127.0.0.1:48095/callback export PINNIPED_TEST_CLI_OIDC_USERNAME=pinny@example.com export PINNIPED_TEST_CLI_OIDC_PASSWORD=password +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER=https://dex.dex.svc.cluster.local/dex +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE="${test_ca_bundle_pem}" +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID=pinniped-supervisor +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET=pinniped-supervisor-secret +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL=https://127.0.0.1:12345/some/path/callback +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME=pinny@example.com +export PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD=password read -r -d '' PINNIPED_TEST_CLUSTER_CAPABILITY_YAML << PINNIPED_TEST_CLUSTER_CAPABILITY_YAML_EOF || true ${pinniped_cluster_capability_file_content} diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index bf8e0586..d01b7233 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -192,6 +192,13 @@ func getAggregatedAPIServerConfig( return nil, err } + // temporarily disable max inflight checks for mutating requests until we + // pick up a fix for https://github.com/kubernetes/kubernetes/issues/95300 + // we do not need to set MaxRequestsInFlight to 0 because we are constantly + // hammered by the kubelet for /healthz and the api server for discovery + // which keeps the non-mutating request watermark histograms up to date + serverConfig.Config.MaxMutatingRequestsInFlight = 0 + apiServerConfig := &apiserver.Config{ GenericConfig: serverConfig, ExtraConfig: apiserver.ExtraConfig{ diff --git a/internal/crud/crud.go b/internal/crud/crud.go new file mode 100644 index 00000000..82d32082 --- /dev/null +++ b/internal/crud/crud.go @@ -0,0 +1,165 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crud + +import ( + "bytes" + "context" + "encoding/base32" + "encoding/base64" + "encoding/json" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + + "go.pinniped.dev/internal/constable" +) + +//nolint:gosec // ignore lint warnings that these are credentials +const ( + secretNameFormat = "pinniped-storage-%s-%s" + secretLabelKey = "storage.pinniped.dev" + secretTypeFormat = "storage.pinniped.dev/%s" + secretVersion = "1" + secretDataKey = "pinniped-storage-data" + secretVersionKey = "pinniped-storage-version" + + ErrSecretTypeMismatch = constable.Error("secret storage data has incorrect type") + ErrSecretLabelMismatch = constable.Error("secret storage data has incorrect label") + ErrSecretVersionMismatch = constable.Error("secret storage data has incorrect version") // TODO do we need this? +) + +type Storage interface { + Create(ctx context.Context, signature string, data JSON) (resourceVersion string, err error) + Get(ctx context.Context, signature string, data JSON) (resourceVersion string, err error) + Update(ctx context.Context, signature, resourceVersion string, data JSON) (newResourceVersion string, err error) + Delete(ctx context.Context, signature string) error +} + +type JSON interface{} // document that we need valid JSON types + +func New(resource string, secrets corev1client.SecretInterface) Storage { + return &secretsStorage{ + resource: resource, + secretType: corev1.SecretType(fmt.Sprintf(secretTypeFormat, resource)), + secretVersion: []byte(secretVersion), + secrets: secrets, + } +} + +type secretsStorage struct { + resource string + secretType corev1.SecretType + secretVersion []byte + secrets corev1client.SecretInterface +} + +func (s *secretsStorage) Create(ctx context.Context, signature string, data JSON) (string, error) { + secret, err := s.toSecret(signature, "", data) + if err != nil { + return "", err + } + secret, err = s.secrets.Create(ctx, secret, metav1.CreateOptions{}) + if err != nil { + return "", fmt.Errorf("failed to create %s for signature %s: %w", s.resource, signature, err) + } + return secret.ResourceVersion, nil +} + +func (s *secretsStorage) Get(ctx context.Context, signature string, data JSON) (string, error) { + secret, err := s.secrets.Get(ctx, s.getName(signature), metav1.GetOptions{}) + if err != nil { + return "", fmt.Errorf("failed to get %s for signature %s: %w", s.resource, signature, err) + } + if err := s.validateSecret(secret); err != nil { + return "", err + } + if err := json.Unmarshal(secret.Data[secretDataKey], data); err != nil { + return "", fmt.Errorf("failed to decode %s for signature %s: %w", s.resource, signature, err) + } + return secret.ResourceVersion, nil +} + +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 { + return fmt.Errorf("%w: %s must equal %s", ErrSecretLabelMismatch, labelResource, s.resource) + } + if !bytes.Equal(secret.Data[secretVersionKey], s.secretVersion) { + return ErrSecretVersionMismatch // TODO should this be fatal or not? + } + return nil +} + +func (s *secretsStorage) Update(ctx context.Context, signature, resourceVersion string, data JSON) (string, error) { + secret, err := s.toSecret(signature, resourceVersion, data) + if err != nil { + return "", err + } + secret, err = s.secrets.Update(ctx, secret, metav1.UpdateOptions{}) + if err != nil { + return "", fmt.Errorf("failed to update %s for signature %s at resource version %s: %w", s.resource, signature, resourceVersion, err) + } + return secret.ResourceVersion, nil +} + +func (s *secretsStorage) Delete(ctx context.Context, signature string) error { + if err := s.secrets.Delete(ctx, s.getName(signature), metav1.DeleteOptions{}); err != nil { + return fmt.Errorf("failed to delete %s for signature %s: %w", s.resource, signature, err) + } + return nil +} + +//nolint: gochecknoglobals +var b32 = base32.StdEncoding.WithPadding(base32.NoPadding) + +func (s *secretsStorage) getName(signature string) string { + // try to decode base64 signatures to prevent double encoding of binary data + signatureBytes := maybeBase64Decode(signature) + // lower case base32 encoding insures that our secret name is valid per ValidateSecretName in k/k + signatureAsValidName := strings.ToLower(b32.EncodeToString(signatureBytes)) + return fmt.Sprintf(secretNameFormat, s.resource, signatureAsValidName) +} + +func (s *secretsStorage) toSecret(signature, resourceVersion string, data JSON) (*corev1.Secret, error) { + buf, err := json.Marshal(data) + if err != nil { + return nil, fmt.Errorf("failed to encode secret data for %s: %w", s.getName(signature), err) + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: s.getName(signature), + ResourceVersion: resourceVersion, + Labels: map[string]string{ + secretLabelKey: s.resource, // make it easier to find this stuff via kubectl + }, + OwnerReferences: nil, // TODO we should set this to make sure stuff gets clean up + }, + Data: map[string][]byte{ + secretDataKey: buf, + secretVersionKey: s.secretVersion, + }, + Type: s.secretType, + }, nil +} + +func maybeBase64Decode(signature string) []byte { + for _, encoding := range []*base64.Encoding{ + // ordered in most likely used by HMAC, JWT, etc signatures + base64.RawURLEncoding, + base64.URLEncoding, + base64.RawStdEncoding, + base64.StdEncoding, + } { + if signatureBytes, err := encoding.DecodeString(signature); err == nil { + return signatureBytes + } + } + return []byte(signature) +} diff --git a/internal/crud/crud_test.go b/internal/crud/crud_test.go new file mode 100644 index 00000000..93ee9818 --- /dev/null +++ b/internal/crud/crud_test.go @@ -0,0 +1,635 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package crud + +import ( + "context" + "errors" + "testing" + + "github.com/ory/fosite/compose" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" +) + +func TestStorage(t *testing.T) { + ctx := context.Background() + secretsGVR := schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "secrets", + } + + type testJSON struct { + Data string + } + + type mocker interface { + AddReactor(verb, resource string, reaction coretesting.ReactionFunc) + PrependReactor(verb, resource string, reaction coretesting.ReactionFunc) + Tracker() coretesting.ObjectTracker + } + + hmac := compose.NewOAuth2HMACStrategy(&compose.Config{}, []byte("super-secret-32-byte-for-testing"), nil) + // test data generation via: + // code, signature, err := hmac.GenerateAuthorizeCode(ctx, nil) + + validateSecretName := validation.NameIsDNSSubdomain // matches k/k + + const ( + namespace = "test-ns" + authorizationCode1 = "81qE408EKL-e99gcXo3UnXBz9W05yGm92_hBmvXeadM.R5h38Bmw7yOaWNy0ypB3feh9toM-3T2zlwMXQyeE9B0" + authorizationCode2 = "p7aIiOLy-btBBlCro5RWm1QABANKCiC0JmDPhUtfOY4.XXJsYsMWhnSMJi9TXJcPO6SDVO2R_QXImwroxxnQPA8" + authorizationCode3 = "skKp1RjGgIwZhT3vaB_k1F3cIj2yp7U8a7UD0xAaemU.5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I" + ) + + tests := []struct { + name string + resource string + mocks func(*testing.T, mocker) + run func(*testing.T, Storage) error + wantActions []coretesting.Action + wantSecrets []corev1.Secret + wantErr string + }{ + { + name: "get non-existent", + resource: "authorization-codes", + mocks: nil, + run: func(t *testing.T, storage Storage) error { + _, err := storage.Get(ctx, "not-exists", nil) + return err + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authorization-codes-t2fx46yyvs3a"), + }, + wantSecrets: nil, + wantErr: `failed to get authorization-codes for signature not-exists: secrets "pinniped-storage-authorization-codes-t2fx46yyvs3a" not found`, + }, + { + name: "delete non-existent", + resource: "tokens", + mocks: nil, + run: func(t *testing.T, storage Storage) error { + return storage.Delete(ctx, "not-a-token") + }, + wantActions: []coretesting.Action{ + coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-tokens-t2fx427lnci6s"), + }, + wantSecrets: nil, + wantErr: `failed to delete tokens for signature not-a-token: secrets "pinniped-storage-tokens-t2fx427lnci6s" not found`, + }, + { + name: "create and get", + resource: "access-tokens", + mocks: nil, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode1) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + data := &testJSON{Data: "create-and-get"} + rv1, err := storage.Create(ctx, signature, data) + require.Empty(t, rv1) // fake client does not set this + require.NoError(t, err) + + out := &testJSON{} + rv2, err := storage.Get(ctx, signature, out) + require.Empty(t, rv2) // fake client does not set this + require.NoError(t, err) + require.Equal(t, data, out) + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewCreateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "access-tokens", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"create-and-get"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/access-tokens", + }), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-access-tokens-i6mhp4azwdxshgsy3s2mvedxpxuh3nudh3ot3m4xamlugj4e6qoq", + Namespace: namespace, + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "access-tokens", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"create-and-get"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/access-tokens", + }, + }, + wantErr: "", + }, + { + name: "get existing", + resource: "pandas-are-best", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-pandas-are-best-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq", + Namespace: namespace, + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "pandas-are-best", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"snorlax"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/pandas-are-best", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode2) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + data := &testJSON{Data: "snorlax"} + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Empty(t, rv1) // fake client does not set this + require.NoError(t, err) + require.Equal(t, data, out) + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-pandas-are-best-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-pandas-are-best-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq", + Namespace: namespace, + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "pandas-are-best", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"snorlax"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/pandas-are-best", + }, + }, + wantErr: "", + }, + { + name: "update existing", + resource: "stores", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "35", + Labels: map[string]string{ + "storage.pinniped.dev": "stores", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"pants"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/stores", + }) + require.NoError(t, err) + + mock.PrependReactor("update", "secrets", func(action coretesting.Action) (handled bool, ret runtime.Object, err error) { + secret := action.(coretesting.UpdateAction).GetObject().(*corev1.Secret) + secret.ResourceVersion = "45" + return false, nil, nil // we mutated the secret in place but we do not "handle" it + }) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode3) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + data := &testJSON{Data: "pants"} + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Equal(t, "35", rv1) // set in mock above + require.NoError(t, err) + require.Equal(t, data, out) + + newData := &testJSON{Data: "shirts"} + rv2, err := storage.Update(ctx, signature, rv1, newData) + require.Equal(t, "45", rv2) // mock sets to a higher value on update + require.NoError(t, err) + + newOut := &testJSON{} + rv3, err := storage.Get(ctx, signature, newOut) + require.Equal(t, "45", rv3) // we should see new rv now + require.NoError(t, err) + require.Equal(t, newData, newOut) + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + ResourceVersion: "35", // update at initial RV + Labels: map[string]string{ + "storage.pinniped.dev": "stores", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"shirts"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/stores", + }), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-stores-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "45", // final list at new RV + Labels: map[string]string{ + "storage.pinniped.dev": "stores", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"shirts"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/stores", + }, + }, + wantErr: "", + }, + { + name: "delete existing", + resource: "seals", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-seals-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq", + Namespace: namespace, + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "seals", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"sad-seal"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/seals", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode2) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + return storage.Delete(ctx, signature) + }, + wantActions: []coretesting.Action{ + coretesting.NewDeleteAction(secretsGVR, namespace, "pinniped-storage-seals-lvzgyywdc2dhjdbgf5jvzfyphosigvhnsh6qlse3blumogoqhqhq"), + }, + wantSecrets: nil, + wantErr: "", + }, + { + name: "invalid exiting secret type", + resource: "candies", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies-not", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode3) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Empty(t, rv1) + require.Empty(t, out.Data) + require.True(t, errors.Is(err, ErrSecretTypeMismatch)) + require.EqualError(t, err, "secret storage data has incorrect type: storage.pinniped.dev/candies-not must equal storage.pinniped.dev/candies") + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies-not", + }, + }, + wantErr: "", + }, + { + name: "invalid exiting secret wrong label", + resource: "candies", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies-are-bad", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode3) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Empty(t, rv1) + require.Empty(t, out.Data) + require.True(t, errors.Is(err, ErrSecretLabelMismatch)) + require.EqualError(t, err, "secret storage data has incorrect label: candies-are-bad must equal candies") + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies-are-bad", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies", + }, + }, + wantErr: "", + }, + { + name: "invalid exiting secret wrong version", + resource: "candies", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("77"), + }, + Type: "storage.pinniped.dev/candies", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode3) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Empty(t, rv1) + require.Empty(t, out.Data) + require.True(t, errors.Is(err, ErrSecretVersionMismatch)) + require.EqualError(t, err, "secret storage data has incorrect version") + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"Data":"twizzlers"}`), + "pinniped-storage-version": []byte("77"), + }, + Type: "storage.pinniped.dev/candies", + }, + }, + wantErr: "", + }, + { + name: "invalid exiting secret not json", + resource: "candies", + mocks: func(t *testing.T, mock mocker) { + err := mock.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`}}bad data{{`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies", + }) + require.NoError(t, err) + }, + run: func(t *testing.T, storage Storage) error { + signature := hmac.AuthorizeCodeSignature(authorizationCode3) + require.NotEmpty(t, signature) + require.NotEmpty(t, validateSecretName(signature, false)) // signature is not valid secret name as-is + + out := &testJSON{} + rv1, err := storage.Get(ctx, signature, out) + require.Empty(t, rv1) + require.Empty(t, out.Data) + require.EqualError(t, err, "failed to decode candies for signature 5aUhdNmfWLW3yKX8Zfz5ztS5IiiWBgu36Gja-o2xl0I: invalid character '}' looking for beginning of value") + + return nil + }, + wantActions: []coretesting.Action{ + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba"), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-candies-4wssc5gzt5mlln6iux6gl7hzz3klsirisydaxn7indnpvdnrs5ba", + Namespace: namespace, + ResourceVersion: "55", + Labels: map[string]string{ + "storage.pinniped.dev": "candies", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`}}bad data{{`), + "pinniped-storage-version": []byte("1"), + }, + Type: "storage.pinniped.dev/candies", + }, + }, + wantErr: "", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + client := fake.NewSimpleClientset() + if tt.mocks != nil { + tt.mocks(t, client) + } + secrets := client.CoreV1().Secrets(namespace) + storage := New(tt.resource, secrets) + + err := tt.run(t, storage) + + require.Equal(t, tt.wantErr, errString(err)) + require.Equal(t, tt.wantActions, client.Actions()) + checkSecretActionNames(t, client.Actions()) + actualSecrets, err := secrets.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, tt.wantSecrets, actualSecrets.Items) + checkSecretListNames(t, actualSecrets.Items) + }) + } +} + +func checkSecretActionNames(t *testing.T, actions []coretesting.Action) { + t.Helper() + + for _, action := range actions { + name := getName(t, action) + assertValidName(t, name) + } +} + +func checkSecretListNames(t *testing.T, secrets []corev1.Secret) { + t.Helper() + + for _, secret := range secrets { + assertValidName(t, secret.Name) + } +} + +func assertValidName(t *testing.T, name string) { + t.Helper() + + validateSecretName := validation.NameIsDNSSubdomain // matches k/k + + require.NotEmpty(t, name) + require.Empty(t, validateSecretName(name, false)) + require.Empty(t, validateSecretName(name, true)) // I do not think we actually care about this case +} + +func getName(t *testing.T, action coretesting.Action) string { + t.Helper() + + if getter, ok := action.(interface { + GetName() string + }); ok { + return getter.GetName() + } + + if getter, ok := action.(interface { + GetObject() runtime.Object + }); ok { + accessor, err := meta.Accessor(getter.GetObject()) + require.NoError(t, err) + return accessor.GetName() + } + + t.Fatalf("failed to get name for action: %#v", action) + panic("unreachable") +} + +func errString(err error) string { + if err == nil { + return "" + } + + return err.Error() +} diff --git a/internal/fosite/authorizationcode/authorizationcode.go b/internal/fosite/authorizationcode/authorizationcode.go new file mode 100644 index 00000000..917c5cc0 --- /dev/null +++ b/internal/fosite/authorizationcode/authorizationcode.go @@ -0,0 +1,382 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package authorizationcode + +import ( + "context" + stderrors "errors" + "fmt" + + "github.com/ory/fosite" + "github.com/ory/fosite/handler/oauth2" + "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" +) + +const ( + ErrInvalidAuthorizeRequestType = constable.Error("authorization request must be of type fosite.AuthorizeRequest") + ErrInvalidAuthorizeRequestData = constable.Error("authorization request data must not be nil") + ErrInvalidAuthorizeRequestVersion = constable.Error("authorization request data has wrong version") + + authorizeCodeStorageVersion = "1" +) + +var _ oauth2.AuthorizeCodeStorage = &authorizeCodeStorage{} + +type authorizeCodeStorage struct { + storage crud.Storage +} + +type AuthorizeCodeSession struct { + Active bool `json:"active"` + Request *fosite.AuthorizeRequest `json:"request"` + Version string `json:"version"` +} + +func New(secrets corev1client.SecretInterface) oauth2.AuthorizeCodeStorage { + return &authorizeCodeStorage{storage: crud.New("authorization-codes", secrets)} +} + +func (a *authorizeCodeStorage) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error { + // this conversion assumes that we do not wrap the default type in any way + // i.e. we use the default fosite.OAuth2Provider.NewAuthorizeRequest implementation + // note that because this type is serialized and stored in Kube, we cannot easily change the implementation later + // TODO hydra uses the fosite.Request struct and ignores the extra fields in fosite.AuthorizeRequest + request, err := validateAndExtractAuthorizeRequest(requester) + if err != nil { + return err + } + + // TODO hydra stores specific fields from the requester + // request ID + // requestedAt + // OAuth client ID + // requested scopes, granted scopes + // requested audience, granted audience + // url encoded request form + // session as JSON bytes with (optional) encryption + // session subject + // consent challenge from session which is the identifier ("authorization challenge") + // of the consent authorization request. It is used to identify the session. + // signature for lookup in the DB + + _, err = a.storage.Create(ctx, signature, &AuthorizeCodeSession{Active: true, Request: request, Version: authorizeCodeStorageVersion}) + return err +} + +func (a *authorizeCodeStorage) GetAuthorizeCodeSession(ctx context.Context, signature string, _ fosite.Session) (fosite.Requester, error) { + // TODO hydra uses the incoming fosite.Session to provide the type needed to json.Unmarshal their session bytes + + // TODO hydra gets the client from its DB as a concrete type via client ID, + // the hydra memory client just validates that the client ID exists + + // TODO hydra uses the sha512.Sum384 hash of signature when using JWT as access token to reduce length + + session, _, err := a.getSession(ctx, signature) + + // we need to always pass both the request and error back + if session == nil { + return nil, err + } + + return session.Request, err +} + +func (a *authorizeCodeStorage) InvalidateAuthorizeCodeSession(ctx context.Context, signature string) error { + // TODO write garbage collector for these codes + + session, rv, err := a.getSession(ctx, signature) + if err != nil { + return err + } + + session.Active = false + if _, err := a.storage.Update(ctx, signature, rv, session); err != nil { + if errors.IsConflict(err) { + return &errSerializationFailureWithCause{cause: err} + } + return err + } + + return nil +} + +func (a *authorizeCodeStorage) getSession(ctx context.Context, signature string) (*AuthorizeCodeSession, string, error) { + session := NewValidEmptyAuthorizeCodeSession() + 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 authorization code session for %s: %w", signature, err) + } + + if version := session.Version; version != authorizeCodeStorageVersion { + return nil, "", fmt.Errorf("%w: authorization code session for %s has version %s instead of %s", + ErrInvalidAuthorizeRequestVersion, signature, version, authorizeCodeStorageVersion) + } + + if session.Request == nil { + return nil, "", fmt.Errorf("malformed authorization code session for %s: %w", signature, ErrInvalidAuthorizeRequestData) + } + + // we must return the session in this case to allow fosite to revoke the associated tokens + if !session.Active { + return session, rv, fmt.Errorf("authorization code session for %s has already been used: %w", signature, fosite.ErrInvalidatedAuthorizeCode) + } + + return session, rv, nil +} + +func NewValidEmptyAuthorizeCodeSession() *AuthorizeCodeSession { + return &AuthorizeCodeSession{ + Request: &fosite.AuthorizeRequest{ + Request: fosite.Request{ + Client: &fosite.DefaultOpenIDConnectClient{}, + Session: &openid.DefaultSession{}, + }, + }, + } +} + +func validateAndExtractAuthorizeRequest(requester fosite.Requester) (*fosite.AuthorizeRequest, error) { + request, ok1 := requester.(*fosite.AuthorizeRequest) + if !ok1 { + return nil, ErrInvalidAuthorizeRequestType + } + _, ok2 := request.Client.(*fosite.DefaultOpenIDConnectClient) + _, ok3 := request.Session.(*openid.DefaultSession) + + valid := ok2 && ok3 + if !valid { + return nil, ErrInvalidAuthorizeRequestType + } + + return request, nil +} + +var _ interface { + Is(error) bool + Unwrap() error + error +} = &errSerializationFailureWithCause{} + +type errSerializationFailureWithCause struct { + cause error +} + +func (e *errSerializationFailureWithCause) Is(err error) bool { + return stderrors.Is(fosite.ErrSerializationFailure, err) +} + +func (e *errSerializationFailureWithCause) Unwrap() error { + return e.cause +} + +func (e *errSerializationFailureWithCause) Error() string { + return fmt.Sprintf("%s: %s", fosite.ErrSerializationFailure, e.cause) +} + +// ExpectedAuthorizeCodeSessionJSONFromFuzzing is used for round tripping tests. +// It is exported to allow integration tests to use it. +const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ + "active": true, + "request": { + "responseTypes": [ + "¥Îʒ襧.ɕ7崛瀇莒AȒ[ɠ牐7#$ɭ", + ".5ȿELj9ûF済(D疻翋膗", + "螤Yɫüeɯ紤邥翔勋\\RBʒ;-" + ], + "redirectUri": { + "Scheme": "ħesƻU赒M喦_ģ", + "Opaque": "Ġ/_章Ņ缘T蝟NJ儱礹燃ɢ", + "User": {}, + "Host": "ȳ4螘Wo", + "Path": "}i{", + "RawPath": "5Dža丝eF0eė鱊hǒx蔼Q", + "ForceQuery": true, + "RawQuery": "熤1bbWV", + "Fragment": "ȋc剠鏯ɽÿ¸", + "RawFragment": "qƤ" + }, + "state": "@n,x竘Şǥ嗾稀'ã击漰怼禝穞梠Ǫs", + "handledResponseTypes": [ + "m\"e尚鬞ƻɼ抹d誉y鿜Ķ" + ], + "id": "ō澩ć|3U2Ǜl霨ǦǵpƉ", + "requestedAt": "1989-11-05T22:02:31.105295894Z", + "client": { + "id": "[:c顎疻紵D", + "client_secret": "mQ==", + "redirect_uris": [ + "恣S@T嵇LJV,Æ櫔袆鋹奘菲", + "ãƻʚ肈ą8O+a駣Ʉɼk瘸'鴵y" + ], + "grant_types": [ + ".湆ê\"唐", + "曎餄FxD溪躲珫ÈşɜȨû臓嬣\"ǃŤz" + ], + "response_types": [ + "Ņʘʟ車sʊ儓JǐŪɺǣy|耑ʄ" + ], + "scopes": [ + "Ą", + "萙Į(潶饏熞ĝƌĆ1", + "əȤ4Į筦p煖鵄$睱奐耡q" + ], + "audience": [ + "Ʃǣ鿫/Ò敫ƤV" + ], + "public": true, + "jwks_uri": "ȩđ[嬧鱒Ȁ彆媚杨嶒ĤG", + "jwks": { + "keys": [ + { + "kty": "OKP", + "crv": "Ed25519", + "x": "JmA-6KpjzqKu0lq9OiB6ORL4s2UzBFPsE1hm6vESeXM", + "x5u": { + "Scheme": "", + "Opaque": "", + "User": null, + "Host": "", + "Path": "", + "RawPath": "", + "ForceQuery": false, + "RawQuery": "", + "Fragment": "", + "RawFragment": "" + } + }, + { + "kty": "OKP", + "crv": "Ed25519", + "x": "LbRC1_3HEe5o7Japk9jFp3_7Ou7Gi2gpqrVrIi0eLDQ", + "x5u": { + "Scheme": "", + "Opaque": "", + "User": null, + "Host": "", + "Path": "", + "RawPath": "", + "ForceQuery": false, + "RawQuery": "", + "Fragment": "", + "RawFragment": "" + } + }, + { + "kty": "OKP", + "crv": "Ed25519", + "x": "Ovk4DF8Yn3mkULuTqnlGJxFnKGu9EL6Xcf2Nql9lK3c", + "x5u": { + "Scheme": "", + "Opaque": "", + "User": null, + "Host": "", + "Path": "", + "RawPath": "", + "ForceQuery": false, + "RawQuery": "", + "Fragment": "", + "RawFragment": "" + } + } + ] + }, + "token_endpoint_auth_method": "\u0026(K鵢Kj ŏ9Q韉Ķ%嶑輫ǘ(", + "request_uris": [ + ":", + "6ě#嫀^xz Ū胧r" + ], + "request_object_signing_alg": "^¡!犃ĹĐJí¿ō擫ų懫砰¿", + "token_endpoint_auth_signing_alg": "ƈŮå" + }, + "scopes": [ + "阃.Ù頀ʌGa皶竇瞍涘¹", + "ȽŮ切衖庀ŰŒ矠", + "楓)馻řĝǕ菸Tĕ1伞柲\u003c\"ʗȆ\\雤" + ], + "grantedScopes": [ + "ơ鮫R嫁ɍUƞ9+u!Ȱ", + "}Ă岜" + ], + "form": { + "旸Ť/Õ薝隧;綡,鼞纂=": [ + "[滮]憀", + "3\u003eÙœ蓄UK嗤眇疟Țƒ1v¸KĶ" + ] + }, + "session": { + "Claims": { + "JTI": "};Ų斻遟a衪荖舃", + "Issuer": "芠顋敀拲h蝺$!", + "Subject": "}j%(=ſ氆]垲莲顇", + "Audience": [ + "彑V\\廳蟕Țǡ蔯ʠ浵Ī龉磈螖畭5", + "渇Ȯʕc" + ], + "Nonce": "Ǖ=rlƆ褡{ǏS", + "ExpiresAt": "1975-11-17T14:21:34.205609651Z", + "IssuedAt": "2104-07-03T15:40:03.66710966Z", + "RequestedAt": "2031-05-18T05:14:19.449350555Z", + "AuthTime": "2018-01-27T07:55:06.056862114Z", + "AccessTokenHash": "鹰肁躧", + "AuthenticationContextClassReference": "}Ɇ", + "AuthenticationMethodsReference": "DQh:uȣ", + "CodeHash": "ɘȏıȒ諃龟", + "Extra": { + "a": { + "^i臏f恡ƨ彮": { + "DĘ敨ýÏʥZq7烱藌\\": null, + "V": { + "őŧQĝ微'X焌襱ǭɕņ殥!_n": false + } + }, + "Ż猁": [ + 1706822246 + ] + }, + "Ò椪)ɫqň2搞Ŀ高摠鲒鿮禗O": 1233332227 + } + }, + "Headers": { + "Extra": { + "?戋璖$9\u0026": { + "µcɕ餦ÑEǰ哤癨浦浏1R": [ + 3761201123 + ], + "頓ć§蚲6rǦ\u003cqċ": { + "Łʀ§ȏœɽDz斡冭ȸěaʜD捛?½ʀ+": null, + "ɒúIJ誠ƉyÖ.峷1藍殙菥趏": { + "jHȬȆ#)\u003cX": true + } + } + }, + "U": 1354158262 + } + }, + "ExpiresAt": { + "\"嘬ȹĹaó剺撱Ȱ": "1985-09-09T04:35:40.533197189Z", + "ʆ\u003e": "1998-08-07T05:37:11.759718906Z", + "柏ʒ鴙*鸆偡Ȓ肯Ûx": "2036-12-19T06:36:14.414805124Z" + }, + "Username": "qmʎaðƠ绗ʢ緦Hū", + "Subject": "屾Ê窢ɋ鄊qɠ谫ǯǵƕ牀1鞊\\ȹ)" + }, + "requestedAudience": [ + "鉍商OɄƣ圔,xĪɏV鵅砍" + ], + "grantedAudience": [ + "C笜嚯\u003cǐšɚĀĥʋ6鉅\\þc涎漄Ɨ腼" + ] + }, + "version": "1" +}` diff --git a/internal/fosite/authorizationcode/authorizationcode_test.go b/internal/fosite/authorizationcode/authorizationcode_test.go new file mode 100644 index 00000000..d97f2b0c --- /dev/null +++ b/internal/fosite/authorizationcode/authorizationcode_test.go @@ -0,0 +1,335 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package authorizationcode + +import ( + "context" + "crypto/ed25519" + "crypto/x509" + "encoding/json" + "math/rand" + "net/url" + "strings" + "testing" + "time" + + fuzz "github.com/google/gofuzz" + "github.com/ory/fosite" + "github.com/ory/fosite/handler/oauth2" + "github.com/ory/fosite/handler/openid" + "github.com/stretchr/testify/require" + "gopkg.in/square/go-jose.v2" + 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" +) + +func TestAuthorizeCodeStorage(t *testing.T) { + ctx := context.Background() + secretsGVR := schema.GroupVersionResource{ + Group: "", + Version: "v1", + 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.AuthorizeRequest{ + ResponseTypes: fosite.Arguments{"not-code"}, + RedirectURI: &url.URL{ + Scheme: "", + Opaque: "weee", + User: &url.Userinfo{}, + Host: "", + Path: "/callback", + RawPath: "", + ForceQuery: false, + RawQuery: "", + Fragment: "", + RawFragment: "", + }, + State: "stated", + HandledResponseTypes: fosite.Arguments{"not-type"}, + 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-authorization-codes-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authorization-codes", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"active":true,"request":{"responseTypes":["not-code"],"redirectUri":{"Scheme":"","Opaque":"weee","User":{},"Host":"","Path":"/callback","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"state":"stated","handledResponseTypes":["not-type"],"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/authorization-codes", + }), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authorization-codes-pwu5zs7lekbhnln2w4"), + coretesting.NewGetAction(secretsGVR, namespace, "pinniped-storage-authorization-codes-pwu5zs7lekbhnln2w4"), + coretesting.NewUpdateAction(secretsGVR, namespace, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authorization-codes-pwu5zs7lekbhnln2w4", + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authorization-codes", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"active":false,"request":{"responseTypes":["not-code"],"redirectUri":{"Scheme":"","Opaque":"weee","User":{},"Host":"","Path":"/callback","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"state":"stated","handledResponseTypes":["not-type"],"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/authorization-codes", + }), + }, + wantSecrets: []corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pinniped-storage-authorization-codes-pwu5zs7lekbhnln2w4", + Namespace: namespace, + ResourceVersion: "", + Labels: map[string]string{ + "storage.pinniped.dev": "authorization-codes", + }, + }, + Data: map[string][]byte{ + "pinniped-storage-data": []byte(`{"active":false,"request":{"responseTypes":["not-code"],"redirectUri":{"Scheme":"","Opaque":"weee","User":{},"Host":"","Path":"/callback","RawPath":"","ForceQuery":false,"RawQuery":"","Fragment":"","RawFragment":""},"state":"stated","handledResponseTypes":["not-type"],"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/authorization-codes", + }, + }, + wantErr: "", + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + client := fake.NewSimpleClientset() + if tt.mocks != nil { + tt.mocks(t, client) + } + secrets := client.CoreV1().Secrets(namespace) + storage := New(secrets) + + err := tt.run(t, storage) + + 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) + }) + } +} + +func errString(err error) string { + if err == nil { + return "" + } + + return err.Error() +} + +// TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession asserts that we can correctly round trip our authorize code session. +// It will detect any changes to fosite.AuthorizeRequest and guarantees that all interface types have concrete implementations. +func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { + validSession := NewValidEmptyAuthorizeCodeSession() + + // sanity check our valid session + extractedRequest, err := validateAndExtractAuthorizeRequest(validSession.Request) + require.NoError(t, err) + require.Equal(t, validSession.Request, extractedRequest) + + // checked above + defaultClient := validSession.Request.Request.Client.(*fosite.DefaultOpenIDConnectClient) + defaultSession := validSession.Request.Request.Session.(*openid.DefaultSession) + + // makes it easier to use a raw string + replacer := strings.NewReplacer("`", "a") + randString := func(c fuzz.Continue) string { + for { + s := c.RandString() + if len(s) == 0 { + continue // skip empty string + } + return replacer.Replace(s) + } + } + + // deterministic fuzzing of fosite.AuthorizeRequest + f := fuzz.New().RandSource(rand.NewSource(1)).NilChance(0).NumElements(1, 3).Funcs( + // these functions guarantee that these are the only interface types we need to fill out + // if fosite.AuthorizeRequest changes to add more, the fuzzer will panic + func(fc *fosite.Client, c fuzz.Continue) { + c.Fuzz(defaultClient) + *fc = defaultClient + }, + func(fs *fosite.Session, c fuzz.Continue) { + c.Fuzz(defaultSession) + *fs = defaultSession + }, + + // these types contain an interface{} that we need to handle + // this is safe because we explicitly provide the openid.DefaultSession concrete type + func(value *map[string]interface{}, c fuzz.Continue) { + // cover all the JSON data types just in case + *value = map[string]interface{}{ + randString(c): float64(c.Intn(1 << 32)), + randString(c): map[string]interface{}{ + randString(c): []interface{}{float64(c.Intn(1 << 32))}, + randString(c): map[string]interface{}{ + randString(c): nil, + randString(c): map[string]interface{}{ + randString(c): c.RandBool(), + }, + }, + }, + } + }, + // JWK contains an interface{} Key that we need to handle + // this is safe because JWK explicitly implements JSON marshalling and unmarshalling + func(jwk *jose.JSONWebKey, c fuzz.Continue) { + key, _, err := ed25519.GenerateKey(c) + require.NoError(t, err) + jwk.Key = key + + // set these fields to make the .Equal comparison work + jwk.Certificates = []*x509.Certificate{} + jwk.CertificatesURL = &url.URL{} + jwk.CertificateThumbprintSHA1 = []byte{} + jwk.CertificateThumbprintSHA256 = []byte{} + }, + + // set this to make the .Equal comparison work + // this is safe because Time explicitly implements JSON marshalling and unmarshalling + func(tp *time.Time, c fuzz.Continue) { + *tp = time.Unix(c.Int63n(1<<32), c.Int63n(1<<32)).UTC() + }, + + // make random strings that do not contain any ` characters + func(s *string, c fuzz.Continue) { + *s = randString(c) + }, + // handle string type alias + func(s *fosite.TokenType, c fuzz.Continue) { + *s = fosite.TokenType(randString(c)) + }, + // handle string type alias + func(s *fosite.Arguments, c fuzz.Continue) { + n := c.Intn(3) + 1 // 1 to 3 items + arguments := make(fosite.Arguments, n) + for i := range arguments { + arguments[i] = randString(c) + } + *s = arguments + }, + ) + + f.Fuzz(validSession) + + const name = "fuzz" // value is irrelevant + ctx := context.Background() + secrets := fake.NewSimpleClientset().CoreV1().Secrets(name) + storage := New(secrets) + + // issue a create using the fuzzed request to confirm that marshalling works + err = storage.CreateAuthorizeCodeSession(ctx, name, validSession.Request) + require.NoError(t, err) + + // retrieve a copy of the fuzzed request from storage to confirm that unmarshalling works + newRequest, err := storage.GetAuthorizeCodeSession(ctx, name, nil) + require.NoError(t, err) + + // the fuzzed request and the copy from storage should be exactly the same + require.Equal(t, validSession.Request, newRequest) + + secretList, err := secrets.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, secretList.Items, 1) + authorizeCodeSessionJSONFromStorage := string(secretList.Items[0].Data["pinniped-storage-data"]) + + // set these to match CreateAuthorizeCodeSession so that .JSONEq works + validSession.Active = true + validSession.Version = "1" + + validSessionJSONBytes, err := json.MarshalIndent(validSession, "", "\t") + require.NoError(t, err) + authorizeCodeSessionJSONFromFuzzing := string(validSessionJSONBytes) + + // the fuzzed session and storage session should have identical JSON + require.JSONEq(t, authorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromStorage) + + // while the fuzzer will panic if AuthorizeRequest changes in a way that cannot be fuzzed, + // if it adds a new field that can be fuzzed, this check will fail + // thus if AuthorizeRequest changes, we will detect it here (though we could possibly miss an omitempty field) + require.Equal(t, ExpectedAuthorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromFuzzing) +} diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 5634fc01..57bd548f 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -18,9 +18,9 @@ import ( "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/provider" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) func NewHandler( diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 519f5b7c..eca7472a 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -22,8 +22,8 @@ import ( "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/oidctestutil" "go.pinniped.dev/internal/oidc/provider" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) func TestAuthorizationEndpoint(t *testing.T) { diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index c51ad86a..c19f4990 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -22,10 +22,10 @@ import ( "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/oidctestutil" - "go.pinniped.dev/internal/oidcclient" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) const ( diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index ee69ee13..83445ac4 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -10,8 +10,8 @@ import ( "go.pinniped.dev/internal/oidc/csrftoken" "go.pinniped.dev/internal/oidc/provider" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) const ( diff --git a/internal/oidc/oidctestutil/oidc.go b/internal/oidc/oidctestutil/oidc.go index fc8c3092..a9b6acbd 100644 --- a/internal/oidc/oidctestutil/oidc.go +++ b/internal/oidc/oidctestutil/oidc.go @@ -8,9 +8,9 @@ import ( "net/url" "go.pinniped.dev/internal/oidc/provider" - "go.pinniped.dev/internal/oidcclient" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) // Test helpers for the OIDC package. diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 41ffb7d1..98c86bdb 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -8,9 +8,9 @@ import ( "net/url" "sync" - "go.pinniped.dev/internal/oidcclient" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) type UpstreamOIDCIdentityProviderI interface { diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index 7c6403b9..42c828c1 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -17,9 +17,9 @@ import ( "go.pinniped.dev/internal/oidc/discovery" "go.pinniped.dev/internal/oidc/jwks" "go.pinniped.dev/internal/oidc/provider" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" ) // Manager can manage multiple active OIDC providers. It acts as a request router for them. diff --git a/internal/oidcclient/filesession/cachefile.go b/pkg/oidcclient/filesession/cachefile.go similarity index 99% rename from internal/oidcclient/filesession/cachefile.go rename to pkg/oidcclient/filesession/cachefile.go index 14c055f3..3629ca5f 100644 --- a/internal/oidcclient/filesession/cachefile.go +++ b/pkg/oidcclient/filesession/cachefile.go @@ -16,7 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/yaml" - "go.pinniped.dev/internal/oidcclient" + "go.pinniped.dev/pkg/oidcclient" ) var ( diff --git a/internal/oidcclient/filesession/cachefile_test.go b/pkg/oidcclient/filesession/cachefile_test.go similarity index 99% rename from internal/oidcclient/filesession/cachefile_test.go rename to pkg/oidcclient/filesession/cachefile_test.go index d9094271..0ddcdf9b 100644 --- a/internal/oidcclient/filesession/cachefile_test.go +++ b/pkg/oidcclient/filesession/cachefile_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "go.pinniped.dev/internal/oidcclient" + "go.pinniped.dev/pkg/oidcclient" ) // validSession should be the same data as `testdata/valid.yaml`. diff --git a/internal/oidcclient/filesession/filesession.go b/pkg/oidcclient/filesession/filesession.go similarity index 99% rename from internal/oidcclient/filesession/filesession.go rename to pkg/oidcclient/filesession/filesession.go index 880d608e..47e0f761 100644 --- a/internal/oidcclient/filesession/filesession.go +++ b/pkg/oidcclient/filesession/filesession.go @@ -15,7 +15,7 @@ import ( "github.com/gofrs/flock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "go.pinniped.dev/internal/oidcclient" + "go.pinniped.dev/pkg/oidcclient" ) const ( diff --git a/internal/oidcclient/filesession/filesession_test.go b/pkg/oidcclient/filesession/filesession_test.go similarity index 99% rename from internal/oidcclient/filesession/filesession_test.go rename to pkg/oidcclient/filesession/filesession_test.go index 8dd07bc9..1b28e184 100644 --- a/internal/oidcclient/filesession/filesession_test.go +++ b/pkg/oidcclient/filesession/filesession_test.go @@ -15,7 +15,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "go.pinniped.dev/internal/oidcclient" + "go.pinniped.dev/pkg/oidcclient" ) func TestNew(t *testing.T) { diff --git a/internal/oidcclient/filesession/testdata/invalid.yaml b/pkg/oidcclient/filesession/testdata/invalid.yaml similarity index 100% rename from internal/oidcclient/filesession/testdata/invalid.yaml rename to pkg/oidcclient/filesession/testdata/invalid.yaml diff --git a/internal/oidcclient/filesession/testdata/valid.yaml b/pkg/oidcclient/filesession/testdata/valid.yaml similarity index 100% rename from internal/oidcclient/filesession/testdata/valid.yaml rename to pkg/oidcclient/filesession/testdata/valid.yaml diff --git a/internal/oidcclient/filesession/testdata/wrong-version.yaml b/pkg/oidcclient/filesession/testdata/wrong-version.yaml similarity index 100% rename from internal/oidcclient/filesession/testdata/wrong-version.yaml rename to pkg/oidcclient/filesession/testdata/wrong-version.yaml diff --git a/internal/oidcclient/login.go b/pkg/oidcclient/login.go similarity index 98% rename from internal/oidcclient/login.go rename to pkg/oidcclient/login.go index 6fda48c9..0898f944 100644 --- a/internal/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -20,9 +20,9 @@ import ( "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/httputil/securityheader" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" - "go.pinniped.dev/internal/oidcclient/state" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient/state" ) const ( diff --git a/internal/oidcclient/login_test.go b/pkg/oidcclient/login_test.go similarity index 99% rename from internal/oidcclient/login_test.go rename to pkg/oidcclient/login_test.go index 8cd4bd2a..2b13752b 100644 --- a/internal/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -23,10 +23,10 @@ import ( "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/mocks/mockkeyset" - "go.pinniped.dev/internal/oidcclient/nonce" - "go.pinniped.dev/internal/oidcclient/pkce" - "go.pinniped.dev/internal/oidcclient/state" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient/state" ) // mockSessionCache exists to avoid an import cycle if we generate mocks into another package. diff --git a/internal/oidcclient/nonce/nonce.go b/pkg/oidcclient/nonce/nonce.go similarity index 100% rename from internal/oidcclient/nonce/nonce.go rename to pkg/oidcclient/nonce/nonce.go diff --git a/internal/oidcclient/nonce/nonce_test.go b/pkg/oidcclient/nonce/nonce_test.go similarity index 100% rename from internal/oidcclient/nonce/nonce_test.go rename to pkg/oidcclient/nonce/nonce_test.go diff --git a/internal/oidcclient/pkce/pkce.go b/pkg/oidcclient/pkce/pkce.go similarity index 100% rename from internal/oidcclient/pkce/pkce.go rename to pkg/oidcclient/pkce/pkce.go diff --git a/internal/oidcclient/pkce/pkce_test.go b/pkg/oidcclient/pkce/pkce_test.go similarity index 100% rename from internal/oidcclient/pkce/pkce_test.go rename to pkg/oidcclient/pkce/pkce_test.go diff --git a/internal/oidcclient/state/state.go b/pkg/oidcclient/state/state.go similarity index 100% rename from internal/oidcclient/state/state.go rename to pkg/oidcclient/state/state.go diff --git a/internal/oidcclient/state/state_test.go b/pkg/oidcclient/state/state_test.go similarity index 100% rename from internal/oidcclient/state/state_test.go rename to pkg/oidcclient/state/state_test.go diff --git a/internal/oidcclient/types.go b/pkg/oidcclient/types.go similarity index 100% rename from internal/oidcclient/types.go rename to pkg/oidcclient/types.go diff --git a/test/deploy/dex/cert-issuer.yaml b/test/deploy/dex/cert-issuer.yaml new file mode 100644 index 00000000..86eecc72 --- /dev/null +++ b/test/deploy/dex/cert-issuer.yaml @@ -0,0 +1,101 @@ +#! Copyright 2020 the Pinniped contributors. All Rights Reserved. +#! SPDX-License-Identifier: Apache-2.0 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: cert-issuer + namespace: dex + labels: + app: cert-issuer +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: cert-issuer + namespace: dex + labels: + app: cert-issuer +rules: + - apiGroups: [""] + resources: [secrets] + verbs: [create, get, patch, update, watch, delete] +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: cert-issuer + namespace: dex + labels: + app: cert-issuer +subjects: + - kind: ServiceAccount + name: cert-issuer + namespace: dex +roleRef: + kind: Role + name: cert-issuer + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: cert-issuer + namespace: dex + labels: + app: cert-issuer +spec: + template: + spec: + serviceAccountName: cert-issuer + initContainers: + - name: generate-certs + image: cfssl/cfssl:1.5.0 + imagePullPolicy: IfNotPresent + command: ["/bin/bash"] + args: + - -c + - | + cd /var/certs + cfssl print-defaults config > /tmp/cfssl-default.json + echo '{"CN": "Pinniped Test","hosts": [],"key": {"algo": "ecdsa","size": 256},"names": [{}]}' > /tmp/csr.json + + echo "generating CA key..." + cfssl genkey \ + -config /tmp/cfssl-default.json \ + -initca /tmp/csr.json \ + | cfssljson -bare ca + + echo "generating Dex server certificate..." + cfssl gencert \ + -ca ca.pem -ca-key ca-key.pem \ + -config /tmp/cfssl-default.json \ + -profile www \ + -cn "dex.dex.svc.cluster.local" \ + -hostname "dex.dex.svc.cluster.local" \ + /tmp/csr.json \ + | cfssljson -bare dex + + chmod -R 777 /var/certs + + echo "generated certificates:" + ls -l /var/certs + volumeMounts: + - name: certs + mountPath: /var/certs + containers: + - name: save-certs + image: bitnami/kubectl + command: ["/bin/bash"] + args: + - -c + - | + kubectl get secrets -n dex certs -o jsonpath='created: {.metadata.creationTimestamp}' || \ + kubectl create secret generic certs --from-file=/var/certs + volumeMounts: + - name: certs + mountPath: /var/certs + volumes: + - name: certs + emptyDir: {} + restartPolicy: Never \ No newline at end of file diff --git a/test/deploy/dex/dex.yaml b/test/deploy/dex/dex.yaml index 6372d49a..bd078f24 100644 --- a/test/deploy/dex/dex.yaml +++ b/test/deploy/dex/dex.yaml @@ -24,6 +24,12 @@ staticClients: redirectURIs: - #@ "http://127.0.0.1:" + str(data.values.ports.cli) + "/callback" - #@ "http://[::1]:" + str(data.values.ports.cli) + "/callback" +- id: pinniped-supervisor + name: 'Pinniped Supervisor' + secret: pinniped-supervisor-secret + redirectURIs: + - #@ "http://127.0.0.1:" + str(data.values.ports.cli) + "/callback" + - #@ "http://[::1]:" + str(data.values.ports.cli) + "/callback" enablePasswordDB: true staticPasswords: - username: "pinny" @@ -69,36 +75,6 @@ spec: annotations: dexConfigHash: #@ sha256.sum(yaml.encode(dexConfig())) spec: - initContainers: - - name: generate-certs - image: cfssl/cfssl:1.5.0 - imagePullPolicy: IfNotPresent - command: ["/bin/bash"] - args: - - -c - - | - cd /var/certs - cfssl print-defaults config > /tmp/cfssl-default.json - echo '{"CN": "Pinniped Test","hosts": [],"key": {"algo": "ecdsa","size": 256},"names": [{}]}' > csr.json - - echo "generating CA key..." - cfssl genkey \ - -config /tmp/cfssl-default.json \ - -initca csr.json \ - | cfssljson -bare ca - - echo "generating Dex server certificate..." - cfssl gencert \ - -ca ca.pem -ca-key ca-key.pem \ - -config /tmp/cfssl-default.json \ - -profile www \ - -cn "dex.dex.svc.cluster.local" \ - -hostname "dex.dex.svc.cluster.local" \ - csr.json \ - | cfssljson -bare dex - volumeMounts: - - name: certs - mountPath: /var/certs containers: - name: dex image: quay.io/dexidp/dex:v2.10.0 @@ -121,7 +97,8 @@ spec: configMap: name: dex-config - name: certs - emptyDir: {} + secret: + secretName: certs --- apiVersion: v1 kind: Service diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 6a1c2a15..48965f76 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -11,11 +11,11 @@ import ( "fmt" "io" "io/ioutil" + "net/url" "os" "os/exec" "path/filepath" "regexp" - "strconv" "strings" "testing" "time" @@ -26,8 +26,8 @@ import ( "gopkg.in/square/go-jose.v2" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" - "go.pinniped.dev/internal/oidcclient" - "go.pinniped.dev/internal/oidcclient/filesession" + "go.pinniped.dev/pkg/oidcclient" + "go.pinniped.dev/pkg/oidcclient/filesession" "go.pinniped.dev/test/library" ) @@ -118,7 +118,7 @@ type loginProviderPatterns struct { func getLoginProvider(t *testing.T) *loginProviderPatterns { t.Helper() - issuer := library.IntegrationEnv(t).OIDCUpstream.Issuer + issuer := library.IntegrationEnv(t).CLITestUpstream.Issuer for _, p := range []loginProviderPatterns{ { Name: "Okta", @@ -270,13 +270,13 @@ func TestCLILoginOIDC(t *testing.T) { // Fill in the username and password and click "submit". t.Logf("logging into %s", loginProvider.Name) - require.NoError(t, page.First(loginProvider.UsernameSelector).Fill(env.OIDCUpstream.Username)) - require.NoError(t, page.First(loginProvider.PasswordSelector).Fill(env.OIDCUpstream.Password)) + require.NoError(t, page.First(loginProvider.UsernameSelector).Fill(env.CLITestUpstream.Username)) + require.NoError(t, page.First(loginProvider.PasswordSelector).Fill(env.CLITestUpstream.Password)) require.NoError(t, page.First(loginProvider.LoginButtonSelector).Click()) // Wait for the login to happen and us be redirected back to a localhost callback. t.Logf("waiting for redirect to localhost callback") - callbackURLPattern := regexp.MustCompile(`\Ahttp://127.0.0.1:` + strconv.Itoa(env.OIDCUpstream.LocalhostPort) + `/.+\z`) + callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(env.CLITestUpstream.CallbackURL) + `\?.+\z`) waitForURL(t, page, callbackURLPattern) // Wait for the "pre" element that gets rendered for a `text/plain` page, and @@ -313,9 +313,9 @@ func TestCLILoginOIDC(t *testing.T) { require.NoError(t, err) claims := map[string]interface{}{} require.NoError(t, json.Unmarshal(jws.UnsafePayloadWithoutVerification(), &claims)) - require.Equal(t, env.OIDCUpstream.Issuer, claims["iss"]) - require.Equal(t, env.OIDCUpstream.ClientID, claims["aud"]) - require.Equal(t, env.OIDCUpstream.Username, claims["email"]) + require.Equal(t, env.CLITestUpstream.Issuer, claims["iss"]) + require.Equal(t, env.CLITestUpstream.ClientID, claims["aud"]) + require.Equal(t, env.CLITestUpstream.Username, claims["email"]) require.NotEmpty(t, claims["nonce"]) // Run the CLI again with the same session cache and login parameters. @@ -334,10 +334,10 @@ func TestCLILoginOIDC(t *testing.T) { t.Logf("overwriting cache to remove valid ID token") cache := filesession.New(sessionCachePath) cacheKey := oidcclient.SessionCacheKey{ - Issuer: env.OIDCUpstream.Issuer, - ClientID: env.OIDCUpstream.ClientID, + Issuer: env.CLITestUpstream.Issuer, + ClientID: env.CLITestUpstream.ClientID, Scopes: []string{"email", "offline_access", "openid", "profile"}, - RedirectURI: fmt.Sprintf("http://localhost:%d/callback", env.OIDCUpstream.LocalhostPort), + RedirectURI: strings.ReplaceAll(env.CLITestUpstream.CallbackURL, "127.0.0.1", "localhost"), } cached := cache.GetToken(cacheKey) require.NotNil(t, cached) @@ -378,10 +378,24 @@ func waitForVisibleElements(t *testing.T, page *agouti.Page, selectors ...string } func waitForURL(t *testing.T, page *agouti.Page, pat *regexp.Regexp) { - require.Eventually(t, func() bool { - url, err := page.URL() - return err == nil && pat.MatchString(url) - }, 10*time.Second, 100*time.Millisecond) + var lastURL string + require.Eventuallyf(t, + func() bool { + url, err := page.URL() + if err == nil && pat.MatchString(url) { + return true + } + if url != lastURL { + t.Logf("saw URL %s", url) + lastURL = url + } + return false + }, + 10*time.Second, + 100*time.Millisecond, + "expected to browse to %s, but never got there", + pat, + ) } func readAndExpectEmpty(r io.Reader) (err error) { @@ -407,18 +421,20 @@ func spawnTestGoroutine(t *testing.T, f func() error) { func oidcLoginCommand(ctx context.Context, t *testing.T, pinnipedExe string, sessionCachePath string) *exec.Cmd { env := library.IntegrationEnv(t) + callbackURL, err := url.Parse(env.CLITestUpstream.CallbackURL) + require.NoError(t, err) cmd := exec.CommandContext(ctx, pinnipedExe, "login", "oidc", - "--issuer", env.OIDCUpstream.Issuer, - "--client-id", env.OIDCUpstream.ClientID, - "--listen-port", strconv.Itoa(env.OIDCUpstream.LocalhostPort), + "--issuer", env.CLITestUpstream.Issuer, + "--client-id", env.CLITestUpstream.ClientID, + "--listen-port", callbackURL.Port(), "--session-cache", sessionCachePath, "--skip-browser", ) // If there is a custom CA bundle, pass it via --ca-bundle and a temporary file. - if env.OIDCUpstream.CABundle != "" { + if env.CLITestUpstream.CABundle != "" { path := filepath.Join(t.TempDir(), "test-ca.pem") - require.NoError(t, ioutil.WriteFile(path, []byte(env.OIDCUpstream.CABundle), 0600)) + require.NoError(t, ioutil.WriteFile(path, []byte(env.CLITestUpstream.CABundle), 0600)) cmd.Args = append(cmd.Args, "--ca-bundle", path) } diff --git a/test/integration/storage_test.go b/test/integration/storage_test.go new file mode 100644 index 00000000..e2f3bdf2 --- /dev/null +++ b/test/integration/storage_test.go @@ -0,0 +1,105 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "encoding/json" + stderrors "errors" + "strings" + "testing" + "time" + + "github.com/ory/fosite" + "github.com/ory/fosite/compose" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "go.pinniped.dev/internal/fosite/authorizationcode" + "go.pinniped.dev/test/library" +) + +func TestAuthorizeCodeStorage(t *testing.T) { + env := library.IntegrationEnv(t) + client := library.NewClientset(t) + + const ( + // randomly generated HMAC authorization code (see below) + code = "TQ72B8YjdEOZyxridYbTLE-pzoK4hpdkZxym5j4EmSc.TKRTgQG41IBQ16FDKTthRdhXfLlNaErcMd9Fy47uXAw" + // name of the secret that will be created in Kube + name = "pinniped-storage-authorization-codes-jssfhaibxdkiaugxufbsso3bixmfo7fzjvuevxbr35c4xdxolqga" + ) + + hmac := compose.NewOAuth2HMACStrategy(&compose.Config{}, []byte("super-secret-32-byte-for-testing"), nil) + // test data generation via: + // code, signature, err := hmac.GenerateAuthorizeCode(ctx, nil) + signature := hmac.AuthorizeCodeSignature(code) + + secrets := client.CoreV1().Secrets(env.SupervisorNamespace) + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err := secrets.Delete(ctx, name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // get a session with most of the data filled out + session := authorizationcode.NewValidEmptyAuthorizeCodeSession() + err := json.Unmarshal([]byte(authorizationcode.ExpectedAuthorizeCodeSessionJSONFromFuzzing), session) + require.NoError(t, err) + + storage := authorizationcode.New(secrets) + + // the session for this signature should not exist yet + notFoundRequest, err := storage.GetAuthorizeCodeSession(ctx, signature, nil) + require.Error(t, err) + require.True(t, stderrors.Is(err, fosite.ErrNotFound)) + require.Nil(t, notFoundRequest) + + err = storage.CreateAuthorizeCodeSession(ctx, signature, session.Request) + require.NoError(t, err) + + // trying to create the session again fails because it already exists + err = storage.CreateAuthorizeCodeSession(ctx, signature, session.Request) + require.Error(t, err) + require.True(t, errors.IsAlreadyExists(err)) + + // check that the data stored in Kube matches what we put in + initialSecret, err := secrets.Get(ctx, name, metav1.GetOptions{}) + require.NoError(t, err) + require.JSONEq(t, authorizationcode.ExpectedAuthorizeCodeSessionJSONFromFuzzing, string(initialSecret.Data["pinniped-storage-data"])) + + // we should be able to get the session now and the request should be the same as what we put in + request, err := storage.GetAuthorizeCodeSession(ctx, signature, nil) + require.NoError(t, err) + require.Equal(t, session.Request, request) + + // simulate the authorization code being exchanged + err = storage.InvalidateAuthorizeCodeSession(ctx, signature) + require.NoError(t, err) + + // trying to use the code session more than once should fail + // getting an invalidated session should return an error and the request + invalidatedRequest, err := storage.GetAuthorizeCodeSession(ctx, signature, nil) + require.Error(t, err) + require.True(t, stderrors.Is(err, fosite.ErrInvalidatedAuthorizeCode)) + require.Equal(t, session.Request, invalidatedRequest) + + // trying to use the code session more than once should fail + err = storage.InvalidateAuthorizeCodeSession(ctx, signature) + require.Error(t, err) + require.True(t, stderrors.Is(err, fosite.ErrInvalidatedAuthorizeCode)) + + // the data stored in Kube should be exactly the same but it should be marked as used + invalidatedSecret, err := secrets.Get(ctx, name, metav1.GetOptions{}) + require.NoError(t, err) + expectedInvalidatedJSON := strings.Replace(authorizationcode.ExpectedAuthorizeCodeSessionJSONFromFuzzing, + `"active": true,`, `"active": false,`, 1) + require.JSONEq(t, expectedInvalidatedJSON, string(invalidatedSecret.Data["pinniped-storage-data"])) +} diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go new file mode 100644 index 00000000..ac7c3d02 --- /dev/null +++ b/test/integration/supervisor_login_test.go @@ -0,0 +1,191 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package integration + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/base64" + "fmt" + "net/http" + "net/url" + "testing" + "time" + + "github.com/coreos/go-oidc" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/pkg/oidcclient/nonce" + "go.pinniped.dev/pkg/oidcclient/pkce" + "go.pinniped.dev/pkg/oidcclient/state" + "go.pinniped.dev/test/library" +) + +func TestSupervisorLogin(t *testing.T) { + env := library.IntegrationEnv(t) + client := library.NewSupervisorClientset(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + // Create downstream OIDC provider (i.e., update supervisor with OIDC provider). + scheme := "http" + addr := env.SupervisorHTTPAddress + caBundle := "" + path := "/some/path" + issuer := fmt.Sprintf("https://%s%s", addr, path) + _, _ = requireCreatingOIDCProviderCausesDiscoveryEndpointsToAppear( + ctx, + t, + scheme, + addr, + caBundle, + issuer, + client, + ) + + // Create HTTP client. + httpClient := newHTTPClient(t, caBundle, nil) + httpClient.CheckRedirect = func(_ *http.Request, _ []*http.Request) error { + // Don't follow any redirects right now, since we simply want to validate that our auth endpoint + // redirects us. + return http.ErrUseLastResponse + } + + // Declare the downstream auth endpoint url we will use. + downstreamAuthURL := makeDownstreamAuthURL(t, scheme, addr, path) + + // Make request to auth endpoint - should fail, since we have no upstreams. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downstreamAuthURL, nil) + require.NoError(t, err) + rsp, err := httpClient.Do(req) + require.NoError(t, err) + defer rsp.Body.Close() + require.Equal(t, http.StatusUnprocessableEntity, rsp.StatusCode) + + // Create upstream OIDC provider. + spec := idpv1alpha1.UpstreamOIDCProviderSpec{ + Issuer: env.SupervisorTestUpstream.Issuer, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)), + }, + Client: idpv1alpha1.OIDCClient{ + SecretName: makeTestClientCredsSecret(t, env.SupervisorTestUpstream.ClientID, env.SupervisorTestUpstream.ClientSecret).Name, + }, + } + upstream := makeTestUpstream(t, spec, idpv1alpha1.PhaseReady) + + upstreamRedirectURI := fmt.Sprintf("https://%s/some/path/callback/%s", env.SupervisorHTTPAddress, upstream.Name) + + // Make request to authorize endpoint - should pass, since we now have an upstream. + req, err = http.NewRequestWithContext(ctx, http.MethodGet, downstreamAuthURL, nil) + require.NoError(t, err) + rsp, err = httpClient.Do(req) + require.NoError(t, err) + defer rsp.Body.Close() + require.Equal(t, http.StatusFound, rsp.StatusCode) + requireValidRedirectLocation( + ctx, + t, + upstream.Spec.Issuer, + env.SupervisorTestUpstream.ClientID, + upstreamRedirectURI, + rsp.Header.Get("Location"), + ) +} + +func makeDownstreamAuthURL(t *testing.T, scheme, addr, path string) string { + t.Helper() + downstreamOAuth2Config := oauth2.Config{ + // This is the hardcoded public client that the supervisor supports. + ClientID: "pinniped-cli", + Endpoint: oauth2.Endpoint{ + AuthURL: fmt.Sprintf("%s://%s%s/oauth2/authorize", scheme, addr, path), + }, + // This is the hardcoded downstream redirect URI that the supervisor supports. + RedirectURL: "http://127.0.0.1/callback", + Scopes: []string{"openid"}, + } + state, nonce, pkce := generateAuthRequestParams(t) + return downstreamOAuth2Config.AuthCodeURL( + state.String(), + nonce.Param(), + pkce.Challenge(), + pkce.Method(), + ) +} + +func generateAuthRequestParams(t *testing.T) (state.State, nonce.Nonce, pkce.Code) { + t.Helper() + state, err := state.Generate() + require.NoError(t, err) + nonce, err := nonce.Generate() + require.NoError(t, err) + pkce, err := pkce.Generate() + require.NoError(t, err) + return state, nonce, pkce +} + +func requireValidRedirectLocation( + ctx context.Context, + t *testing.T, + issuer, clientID, redirectURI, actualLocation string, +) { + t.Helper() + env := library.IntegrationEnv(t) + + // Do OIDC discovery on our test issuer to get auth endpoint. + transport := http.Transport{} + if env.Proxy != "" { + transport.Proxy = func(_ *http.Request) (*url.URL, error) { + return url.Parse(env.Proxy) + } + } + if env.SupervisorTestUpstream.CABundle != "" { + transport.TLSClientConfig = &tls.Config{RootCAs: x509.NewCertPool()} + transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(env.SupervisorTestUpstream.CABundle)) + } + + ctx = oidc.ClientContext(ctx, &http.Client{Transport: &transport}) + upstreamProvider, err := oidc.NewProvider(ctx, issuer) + require.NoError(t, err) + + // Parse expected upstream auth URL. + expectedLocationURL, err := url.Parse( + (&oauth2.Config{ + ClientID: clientID, + Endpoint: upstreamProvider.Endpoint(), + RedirectURL: redirectURI, + Scopes: []string{"openid"}, + }).AuthCodeURL("", oauth2.AccessTypeOffline), + ) + require.NoError(t, err) + + // Parse actual upstream auth URL. + actualLocationURL, err := url.Parse(actualLocation) + require.NoError(t, err) + + // First make some assertions on the query values. Note that we will not be able to know what + // certain query values are since they may be random (e.g., state, pkce, nonce). + expectedLocationQuery := expectedLocationURL.Query() + actualLocationQuery := actualLocationURL.Query() + require.NotEmpty(t, actualLocationQuery.Get("state")) + actualLocationQuery.Del("state") + require.NotEmpty(t, actualLocationQuery.Get("code_challenge")) + actualLocationQuery.Del("code_challenge") + require.NotEmpty(t, actualLocationQuery.Get("code_challenge_method")) + actualLocationQuery.Del("code_challenge_method") + require.NotEmpty(t, actualLocationQuery.Get("nonce")) + actualLocationQuery.Del("nonce") + require.Equal(t, expectedLocationQuery, actualLocationQuery) + + // Zero-out query values, since we made specific assertions about those above, and assert that the + // URL's are equal otherwise. + expectedLocationURL.RawQuery = "" + actualLocationURL.RawQuery = "" + require.Equal(t, expectedLocationURL, actualLocationURL) +} diff --git a/test/integration/supervisor_upstream_test.go b/test/integration/supervisor_upstream_test.go index c96ae7af..e38f2b17 100644 --- a/test/integration/supervisor_upstream_test.go +++ b/test/integration/supervisor_upstream_test.go @@ -24,9 +24,6 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) { t.Parallel() spec := v1alpha1.UpstreamOIDCProviderSpec{ Issuer: "https://127.0.0.1:444444/issuer", - AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{ - AdditionalScopes: []string{"email", "profile"}, - }, Client: v1alpha1.OIDCClient{ SecretName: "does-not-exist", }, @@ -51,9 +48,9 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) { t.Run("valid", func(t *testing.T) { t.Parallel() spec := v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: env.OIDCUpstream.Issuer, + Issuer: env.SupervisorTestUpstream.Issuer, TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.OIDCUpstream.CABundle)), + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)), }, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{ AdditionalScopes: []string{"email", "profile"}, diff --git a/test/library/env.go b/test/library/env.go index ce7e5f94..e6f3c1da 100644 --- a/test/library/env.go +++ b/test/library/env.go @@ -6,7 +6,6 @@ package library import ( "io/ioutil" "os" - "strconv" "strings" "testing" @@ -46,14 +45,18 @@ type TestEnv struct { ExpectedGroups []string `json:"expectedGroups"` } `json:"testUser"` - OIDCUpstream struct { - Issuer string `json:"issuer"` - CABundle string `json:"caBundle" ` - ClientID string `json:"clientID"` - LocalhostPort int `json:"localhostPort"` - Username string `json:"username"` - Password string `json:"password"` - } `json:"oidcUpstream"` + CLITestUpstream TestOIDCUpstream `json:"cliOIDCUpstream"` + SupervisorTestUpstream TestOIDCUpstream `json:"supervisorOIDCUpstream"` +} + +type TestOIDCUpstream struct { + Issuer string `json:"issuer"` + CABundle string `json:"caBundle" ` + ClientID string `json:"clientID"` + ClientSecret string `json:"clientSecret"` + CallbackURL string `json:"callback"` + Username string `json:"username"` + Password string `json:"password"` } // IntegrationEnv gets the integration test environment from OS environment variables. This @@ -130,12 +133,24 @@ func loadEnvVars(t *testing.T, result *TestEnv) { require.NotEmpty(t, result.SupervisorCustomLabels, "PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS cannot be empty") result.Proxy = os.Getenv("PINNIPED_TEST_PROXY") - result.OIDCUpstream.Issuer = needEnv(t, "PINNIPED_TEST_CLI_OIDC_ISSUER") - result.OIDCUpstream.CABundle = os.Getenv("PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE") - result.OIDCUpstream.ClientID = needEnv(t, "PINNIPED_TEST_CLI_OIDC_CLIENT_ID") - result.OIDCUpstream.LocalhostPort, _ = strconv.Atoi(needEnv(t, "PINNIPED_TEST_CLI_OIDC_LOCALHOST_PORT")) - result.OIDCUpstream.Username = needEnv(t, "PINNIPED_TEST_CLI_OIDC_USERNAME") - result.OIDCUpstream.Password = needEnv(t, "PINNIPED_TEST_CLI_OIDC_PASSWORD") + result.CLITestUpstream = TestOIDCUpstream{ + Issuer: needEnv(t, "PINNIPED_TEST_CLI_OIDC_ISSUER"), + CABundle: os.Getenv("PINNIPED_TEST_CLI_OIDC_ISSUER_CA_BUNDLE"), + ClientID: needEnv(t, "PINNIPED_TEST_CLI_OIDC_CLIENT_ID"), + CallbackURL: needEnv(t, "PINNIPED_TEST_CLI_OIDC_CALLBACK_URL"), + Username: needEnv(t, "PINNIPED_TEST_CLI_OIDC_USERNAME"), + Password: needEnv(t, "PINNIPED_TEST_CLI_OIDC_PASSWORD"), + } + + result.SupervisorTestUpstream = TestOIDCUpstream{ + Issuer: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER"), + CABundle: os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE"), + ClientID: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID"), + ClientSecret: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_SECRET"), + CallbackURL: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CALLBACK_URL"), + Username: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME"), + Password: needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_PASSWORD"), + } } func (e *TestEnv) HasCapability(cap Capability) bool {