From dd2133458e15606a784674b28aad9e7dbc7f342c Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 16 Nov 2020 11:54:13 -0600 Subject: [PATCH 1/5] Add --ca-bundle flag to "pinniped login oidc" command. Signed-off-by: Matt Moyer --- .golangci.yaml | 2 +- cmd/pinniped/cmd/login_oidc.go | 28 ++++++++++++++++++++++++++++ cmd/pinniped/cmd/login_oidc_test.go | 1 + internal/oidcclient/login.go | 12 ++++++++++++ internal/oidcclient/login_test.go | 1 + 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index ec1e153c..9578fd7e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -60,7 +60,7 @@ issues: linters-settings: funlen: - lines: 125 + lines: 150 statements: 50 goheader: template: |- diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 3754f06a..bf12e7be 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -4,7 +4,12 @@ package cmd import ( + "crypto/tls" + "crypto/x509" "encoding/json" + "fmt" + "io/ioutil" + "net/http" "os" "path/filepath" @@ -36,6 +41,7 @@ func oidcLoginCommand(loginFunc func(issuer string, clientID string, opts ...oid scopes []string skipBrowser bool sessionCachePath string + caBundlePaths []string debugSessionCache bool ) cmd.Flags().StringVar(&issuer, "issuer", "", "OpenID Connect issuer URL.") @@ -44,6 +50,7 @@ func oidcLoginCommand(loginFunc func(issuer string, clientID string, opts ...oid cmd.Flags().StringSliceVar(&scopes, "scopes", []string{"offline_access", "openid", "email", "profile"}, "OIDC scopes to request during login.") cmd.Flags().BoolVar(&skipBrowser, "skip-browser", false, "Skip opening the browser (just print the URL).") cmd.Flags().StringVar(&sessionCachePath, "session-cache", filepath.Join(mustGetConfigDir(), "sessions.yaml"), "Path to session cache file.") + cmd.Flags().StringSliceVar(&caBundlePaths, "ca-bundle", nil, "Path to TLS certificate authority bundle (PEM format, optional, can be repeated).") cmd.Flags().BoolVar(&debugSessionCache, "debug-session-cache", false, "Print debug logs related to the session cache.") mustMarkHidden(&cmd, "debug-session-cache") mustMarkRequired(&cmd, "issuer", "client-id") @@ -80,6 +87,27 @@ func oidcLoginCommand(loginFunc func(issuer string, clientID string, opts ...oid })) } + if len(caBundlePaths) > 0 { + pool := x509.NewCertPool() + for _, p := range caBundlePaths { + pem, err := ioutil.ReadFile(p) + if err != nil { + return fmt.Errorf("could not read --ca-bundle: %w", err) + } + pool.AppendCertsFromPEM(pem) + } + tlsConfig := tls.Config{ + RootCAs: pool, + MinVersion: tls.VersionTLS12, + } + opts = append(opts, oidcclient.WithClient(&http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tlsConfig, + }, + })) + } + tok, err := loginFunc(issuer, clientID, opts...) if err != nil { return err diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 637a7af4..a8bb03ed 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -40,6 +40,7 @@ func TestLoginOIDCCommand(t *testing.T) { oidc --issuer ISSUER --client-id CLIENT_ID [flags] Flags: + --ca-bundle strings Path to TLS certificate authority bundle (PEM format, optional, can be repeated). --client-id string OpenID Connect client ID. -h, --help help for oidc --issuer string OpenID Connect issuer URL. diff --git a/internal/oidcclient/login.go b/internal/oidcclient/login.go index 5ab3b5ff..6fda48c9 100644 --- a/internal/oidcclient/login.go +++ b/internal/oidcclient/login.go @@ -44,6 +44,8 @@ type handlerState struct { scopes []string cache SessionCache + httpClient *http.Client + // Parameters of the localhost listener. listenAddr string callbackPath string @@ -122,6 +124,14 @@ func WithSessionCache(cache SessionCache) Option { } } +// WithClient sets the HTTP client used to make CLI-to-provider requests. +func WithClient(httpClient *http.Client) Option { + return func(h *handlerState) error { + h.httpClient = httpClient + return nil + } +} + // nopCache is a SessionCache that doesn't actually do anything. type nopCache struct{} @@ -144,6 +154,7 @@ func Login(issuer string, clientID string, opts ...Option) (*Token, error) { callbackPath: "/callback", ctx: context.Background(), callbacks: make(chan callbackResult), + httpClient: http.DefaultClient, // Default implementations of external dependencies (to be mocked in tests). generateState: state.Generate, @@ -163,6 +174,7 @@ func Login(issuer string, clientID string, opts ...Option) (*Token, error) { // Always set a long, but non-infinite timeout for this operation. ctx, cancel := context.WithTimeout(h.ctx, 10*time.Minute) defer cancel() + ctx = oidc.ClientContext(ctx, h.httpClient) h.ctx = ctx // Initialize login parameters. diff --git a/internal/oidcclient/login_test.go b/internal/oidcclient/login_test.go index ea12737f..35574ba0 100644 --- a/internal/oidcclient/login_test.go +++ b/internal/oidcclient/login_test.go @@ -416,6 +416,7 @@ func TestLogin(t *testing.T) { require.Equal(t, []*Token{&testToken}, cache.sawPutTokens) }) require.NoError(t, WithSessionCache(cache)(h)) + require.NoError(t, WithClient(&http.Client{Timeout: 10 * time.Second})(h)) h.openURL = func(actualURL string) error { parsedActualURL, err := url.Parse(actualURL) From b17ac6ec0b7f09ecd1be1904c1a7a52da82a68a7 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 16 Nov 2020 14:04:08 -0600 Subject: [PATCH 2/5] Update integration tests to run Dex over HTTPS. Signed-off-by: Matt Moyer --- hack/prepare-for-integration-tests.sh | 8 ++- test/deploy/dex/dex.yaml | 72 ++++++++++++++++++++------- test/integration/cli_test.go | 14 +++++- test/library/env.go | 2 + 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 25c4ad0f..89b7f2f8 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -265,6 +265,11 @@ if ! tilt_mode; then popd >/dev/null 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)" + # # Create the environment file # @@ -287,7 +292,8 @@ export PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS='${supervisor_custom_labels}' export PINNIPED_TEST_SUPERVISOR_HTTP_ADDRESS="127.0.0.1:12345" export PINNIPED_TEST_SUPERVISOR_HTTPS_ADDRESS="localhost:12344" export PINNIPED_TEST_PROXY=http://127.0.0.1:12346 -export PINNIPED_TEST_CLI_OIDC_ISSUER=http://dex.dex.svc.cluster.local/dex +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_USERNAME=pinny@example.com diff --git a/test/deploy/dex/dex.yaml b/test/deploy/dex/dex.yaml index 447a561f..6372d49a 100644 --- a/test/deploy/dex/dex.yaml +++ b/test/deploy/dex/dex.yaml @@ -6,13 +6,15 @@ #@ load("@ytt:yaml", "yaml") #@ def dexConfig(): -issuer: http://dex.dex.svc.cluster.local/dex +issuer: https://dex.dex.svc.cluster.local/dex storage: type: sqlite3 config: file: ":memory:" web: - http: 0.0.0.0:80 + https: 0.0.0.0:443 + tlsCert: /var/certs/dex.pem + tlsKey: /var/certs/dex-key.pem oauth2: skipApprovalScreen: true staticClients: @@ -67,24 +69,59 @@ 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 - imagePullPolicy: IfNotPresent - command: - - /usr/local/bin/dex - - serve - - /etc/dex/cfg/config.yaml - ports: - - name: http - containerPort: 80 - volumeMounts: - - name: config - mountPath: /etc/dex/cfg + - name: dex + image: quay.io/dexidp/dex:v2.10.0 + imagePullPolicy: IfNotPresent + command: + - /usr/local/bin/dex + - serve + - /etc/dex/cfg/config.yaml + ports: + - name: https + containerPort: 443 + volumeMounts: + - name: dex-config + mountPath: /etc/dex/cfg + - name: certs + mountPath: /var/certs + readOnly: true volumes: - - name: config + - name: dex-config configMap: name: dex-config + - name: certs + emptyDir: {} --- apiVersion: v1 kind: Service @@ -98,4 +135,5 @@ spec: selector: app: dex ports: - - port: 80 + - port: 443 + name: https diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 50980e5e..6a1c2a15 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -130,8 +130,8 @@ func getLoginProvider(t *testing.T) *loginProviderPatterns { }, { Name: "Dex", - IssuerPattern: regexp.MustCompile(`\Ahttp://dex\.dex\.svc\.cluster\.local/dex.*\z`), - LoginPagePattern: regexp.MustCompile(`\Ahttp://dex\.dex\.svc\.cluster\.local/dex/auth/local.+\z`), + IssuerPattern: regexp.MustCompile(`\Ahttps://dex\.dex\.svc\.cluster\.local/dex.*\z`), + LoginPagePattern: regexp.MustCompile(`\Ahttps://dex\.dex\.svc\.cluster\.local/dex/auth/local.+\z`), UsernameSelector: "input#login", PasswordSelector: "input#password", LoginButtonSelector: "button#submit-login", @@ -170,6 +170,7 @@ func TestCLILoginOIDC(t *testing.T) { agouti.Desired(caps), agouti.ChromeOptions("args", []string{ "--no-sandbox", + "--ignore-certificate-errors", "--headless", // Comment out this line to see the tests happen in a visible browser window. }), // Uncomment this to see stdout/stderr from chromedriver. @@ -413,6 +414,15 @@ func oidcLoginCommand(ctx context.Context, t *testing.T, pinnipedExe string, ses "--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 != "" { + path := filepath.Join(t.TempDir(), "test-ca.pem") + require.NoError(t, ioutil.WriteFile(path, []byte(env.OIDCUpstream.CABundle), 0600)) + cmd.Args = append(cmd.Args, "--ca-bundle", path) + } + + // If there is a custom proxy, set it using standard environment variables. if env.Proxy != "" { cmd.Env = append(os.Environ(), "http_proxy="+env.Proxy, diff --git a/test/library/env.go b/test/library/env.go index 14c2ada1..ce7e5f94 100644 --- a/test/library/env.go +++ b/test/library/env.go @@ -48,6 +48,7 @@ type TestEnv struct { OIDCUpstream struct { Issuer string `json:"issuer"` + CABundle string `json:"caBundle" ` ClientID string `json:"clientID"` LocalhostPort int `json:"localhostPort"` Username string `json:"username"` @@ -130,6 +131,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) { 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") From e867fb82b98158658013bdaa7bcfb0ae08f59c24 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 16 Nov 2020 14:42:43 -0600 Subject: [PATCH 3/5] Add `spec.tls` field to UpstreamOIDCProvider API. This allows for a custom CA bundle to be used when connecting to the upstream issuer. Signed-off-by: Matt Moyer --- .../supervisor/idp/v1alpha1/types_tls.go.tmpl | 11 ++++++++++ .../types_upstreamoidcprovider.go.tmpl | 4 ++++ ...or.pinniped.dev_upstreamoidcproviders.yaml | 9 ++++++++ generated/1.17/README.adoc | 18 ++++++++++++++++ .../apis/supervisor/idp/v1alpha1/types_tls.go | 11 ++++++++++ .../v1alpha1/types_upstreamoidcprovider.go | 4 ++++ .../idp/v1alpha1/zz_generated.deepcopy.go | 21 +++++++++++++++++++ ...or.pinniped.dev_upstreamoidcproviders.yaml | 9 ++++++++ generated/1.18/README.adoc | 18 ++++++++++++++++ .../apis/supervisor/idp/v1alpha1/types_tls.go | 11 ++++++++++ .../v1alpha1/types_upstreamoidcprovider.go | 4 ++++ .../idp/v1alpha1/zz_generated.deepcopy.go | 21 +++++++++++++++++++ ...or.pinniped.dev_upstreamoidcproviders.yaml | 9 ++++++++ generated/1.19/README.adoc | 18 ++++++++++++++++ .../apis/supervisor/idp/v1alpha1/types_tls.go | 11 ++++++++++ .../v1alpha1/types_upstreamoidcprovider.go | 4 ++++ .../idp/v1alpha1/zz_generated.deepcopy.go | 21 +++++++++++++++++++ ...or.pinniped.dev_upstreamoidcproviders.yaml | 9 ++++++++ 18 files changed, 213 insertions(+) create mode 100644 apis/supervisor/idp/v1alpha1/types_tls.go.tmpl create mode 100644 generated/1.17/apis/supervisor/idp/v1alpha1/types_tls.go create mode 100644 generated/1.18/apis/supervisor/idp/v1alpha1/types_tls.go create mode 100644 generated/1.19/apis/supervisor/idp/v1alpha1/types_tls.go diff --git a/apis/supervisor/idp/v1alpha1/types_tls.go.tmpl b/apis/supervisor/idp/v1alpha1/types_tls.go.tmpl new file mode 100644 index 00000000..fa4db315 --- /dev/null +++ b/apis/supervisor/idp/v1alpha1/types_tls.go.tmpl @@ -0,0 +1,11 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +// Configuration for TLS parameters related to identity provider integration. +type TLSSpec struct { + // X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. + // +optional + CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"` +} diff --git a/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl index cc8ca0fa..ea12b063 100644 --- a/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl @@ -75,6 +75,10 @@ type UpstreamOIDCProviderSpec struct { // +kubebuilder:validation:Pattern=`^https://` Issuer string `json:"issuer"` + // TLS configuration for discovery/JWKS requests to the issuer. + // +optional + TLS *TLSSpec `json:"tls,omitempty"` + // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml index 451a4474..780fe6fe 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml +++ b/deploy/supervisor/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml @@ -98,6 +98,15 @@ spec: minLength: 1 pattern: ^https:// type: string + tls: + description: TLS configuration for discovery/JWKS requests to the + issuer. + properties: + certificateAuthorityData: + description: X.509 Certificate Authority (base64-encoded PEM bundle). + If omitted, a default set of system roots will be trusted. + type: string + type: object required: - client - issuer diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index 1854d94a..a81aa6e9 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -373,6 +373,23 @@ OIDCClient contains information about an OIDC client (e.g., client ID and client |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-tlsspec"] +==== TLSSpec + +Configuration for TLS parameters related to identity provider integration. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-upstreamoidcproviderspec[$$UpstreamOIDCProviderSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`certificateAuthorityData`* __string__ | X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-upstreamoidcprovider"] ==== UpstreamOIDCProvider @@ -409,6 +426,7 @@ Spec for configuring an OIDC identity provider. |=== | Field | Description | *`issuer`* __string__ | Issuer is the issuer URL of this OIDC identity provider, i.e., where to fetch /.well-known/openid-configuration. +| *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for discovery/JWKS requests to the issuer. | *`authorizationConfig`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-oidcauthorizationconfig[$$OIDCAuthorizationConfig$$]__ | AuthorizationConfig holds information about how to form the OAuth2 authorization request parameters to be used with this OIDC identity provider. | *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-oidcclaims[$$OIDCClaims$$]__ | Claims provides the names of token claims that will be used when inspecting an identity from this OIDC identity provider. | *`client`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-oidcclient[$$OIDCClient$$]__ | OIDCClient contains OIDC client information to be used used with this OIDC identity provider. diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_tls.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_tls.go new file mode 100644 index 00000000..fa4db315 --- /dev/null +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_tls.go @@ -0,0 +1,11 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +// Configuration for TLS parameters related to identity provider integration. +type TLSSpec struct { + // X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. + // +optional + CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"` +} 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 cc8ca0fa..ea12b063 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -75,6 +75,10 @@ type UpstreamOIDCProviderSpec struct { // +kubebuilder:validation:Pattern=`^https://` Issuer string `json:"issuer"` + // TLS configuration for discovery/JWKS requests to the issuer. + // +optional + TLS *TLSSpec `json:"tls,omitempty"` + // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go b/generated/1.17/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go index 07cbb8b6..9eeade4e 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go @@ -81,6 +81,22 @@ func (in *OIDCClient) DeepCopy() *OIDCClient { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSSpec. +func (in *TLSSpec) DeepCopy() *TLSSpec { + if in == nil { + return nil + } + out := new(TLSSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProvider) DeepCopyInto(out *UpstreamOIDCProvider) { *out = *in @@ -145,6 +161,11 @@ func (in *UpstreamOIDCProviderList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProviderSpec) DeepCopyInto(out *UpstreamOIDCProviderSpec) { *out = *in + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(TLSSpec) + **out = **in + } in.AuthorizationConfig.DeepCopyInto(&out.AuthorizationConfig) out.Claims = in.Claims out.Client = in.Client diff --git a/generated/1.17/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml b/generated/1.17/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml index 451a4474..780fe6fe 100644 --- a/generated/1.17/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml +++ b/generated/1.17/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml @@ -98,6 +98,15 @@ spec: minLength: 1 pattern: ^https:// type: string + tls: + description: TLS configuration for discovery/JWKS requests to the + issuer. + properties: + certificateAuthorityData: + description: X.509 Certificate Authority (base64-encoded PEM bundle). + If omitted, a default set of system roots will be trusted. + type: string + type: object required: - client - issuer diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index bb11a577..a2a20313 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -373,6 +373,23 @@ OIDCClient contains information about an OIDC client (e.g., client ID and client |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-tlsspec"] +==== TLSSpec + +Configuration for TLS parameters related to identity provider integration. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-upstreamoidcproviderspec[$$UpstreamOIDCProviderSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`certificateAuthorityData`* __string__ | X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-upstreamoidcprovider"] ==== UpstreamOIDCProvider @@ -409,6 +426,7 @@ Spec for configuring an OIDC identity provider. |=== | Field | Description | *`issuer`* __string__ | Issuer is the issuer URL of this OIDC identity provider, i.e., where to fetch /.well-known/openid-configuration. +| *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for discovery/JWKS requests to the issuer. | *`authorizationConfig`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-oidcauthorizationconfig[$$OIDCAuthorizationConfig$$]__ | AuthorizationConfig holds information about how to form the OAuth2 authorization request parameters to be used with this OIDC identity provider. | *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-oidcclaims[$$OIDCClaims$$]__ | Claims provides the names of token claims that will be used when inspecting an identity from this OIDC identity provider. | *`client`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-oidcclient[$$OIDCClient$$]__ | OIDCClient contains OIDC client information to be used used with this OIDC identity provider. diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_tls.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_tls.go new file mode 100644 index 00000000..fa4db315 --- /dev/null +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_tls.go @@ -0,0 +1,11 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +// Configuration for TLS parameters related to identity provider integration. +type TLSSpec struct { + // X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. + // +optional + CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"` +} 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 cc8ca0fa..ea12b063 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -75,6 +75,10 @@ type UpstreamOIDCProviderSpec struct { // +kubebuilder:validation:Pattern=`^https://` Issuer string `json:"issuer"` + // TLS configuration for discovery/JWKS requests to the issuer. + // +optional + TLS *TLSSpec `json:"tls,omitempty"` + // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go b/generated/1.18/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go index 07cbb8b6..9eeade4e 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go @@ -81,6 +81,22 @@ func (in *OIDCClient) DeepCopy() *OIDCClient { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSSpec. +func (in *TLSSpec) DeepCopy() *TLSSpec { + if in == nil { + return nil + } + out := new(TLSSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProvider) DeepCopyInto(out *UpstreamOIDCProvider) { *out = *in @@ -145,6 +161,11 @@ func (in *UpstreamOIDCProviderList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProviderSpec) DeepCopyInto(out *UpstreamOIDCProviderSpec) { *out = *in + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(TLSSpec) + **out = **in + } in.AuthorizationConfig.DeepCopyInto(&out.AuthorizationConfig) out.Claims = in.Claims out.Client = in.Client diff --git a/generated/1.18/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml b/generated/1.18/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml index 451a4474..780fe6fe 100644 --- a/generated/1.18/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml +++ b/generated/1.18/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml @@ -98,6 +98,15 @@ spec: minLength: 1 pattern: ^https:// type: string + tls: + description: TLS configuration for discovery/JWKS requests to the + issuer. + properties: + certificateAuthorityData: + description: X.509 Certificate Authority (base64-encoded PEM bundle). + If omitted, a default set of system roots will be trusted. + type: string + type: object required: - client - issuer diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index 500bf9ea..e36be991 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -373,6 +373,23 @@ OIDCClient contains information about an OIDC client (e.g., client ID and client |=== +[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-tlsspec"] +==== TLSSpec + +Configuration for TLS parameters related to identity provider integration. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-upstreamoidcproviderspec[$$UpstreamOIDCProviderSpec$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`certificateAuthorityData`* __string__ | X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-upstreamoidcprovider"] ==== UpstreamOIDCProvider @@ -409,6 +426,7 @@ Spec for configuring an OIDC identity provider. |=== | Field | Description | *`issuer`* __string__ | Issuer is the issuer URL of this OIDC identity provider, i.e., where to fetch /.well-known/openid-configuration. +| *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for discovery/JWKS requests to the issuer. | *`authorizationConfig`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-oidcauthorizationconfig[$$OIDCAuthorizationConfig$$]__ | AuthorizationConfig holds information about how to form the OAuth2 authorization request parameters to be used with this OIDC identity provider. | *`claims`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-oidcclaims[$$OIDCClaims$$]__ | Claims provides the names of token claims that will be used when inspecting an identity from this OIDC identity provider. | *`client`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-oidcclient[$$OIDCClient$$]__ | OIDCClient contains OIDC client information to be used used with this OIDC identity provider. diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_tls.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_tls.go new file mode 100644 index 00000000..fa4db315 --- /dev/null +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_tls.go @@ -0,0 +1,11 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +// Configuration for TLS parameters related to identity provider integration. +type TLSSpec struct { + // X.509 Certificate Authority (base64-encoded PEM bundle). If omitted, a default set of system roots will be trusted. + // +optional + CertificateAuthorityData string `json:"certificateAuthorityData,omitempty"` +} 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 cc8ca0fa..ea12b063 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go @@ -75,6 +75,10 @@ type UpstreamOIDCProviderSpec struct { // +kubebuilder:validation:Pattern=`^https://` Issuer string `json:"issuer"` + // TLS configuration for discovery/JWKS requests to the issuer. + // +optional + TLS *TLSSpec `json:"tls,omitempty"` + // AuthorizationConfig holds information about how to form the OAuth2 authorization request // parameters to be used with this OIDC identity provider. // +optional diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go b/generated/1.19/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go index 07cbb8b6..9eeade4e 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/zz_generated.deepcopy.go @@ -81,6 +81,22 @@ func (in *OIDCClient) DeepCopy() *OIDCClient { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSSpec. +func (in *TLSSpec) DeepCopy() *TLSSpec { + if in == nil { + return nil + } + out := new(TLSSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProvider) DeepCopyInto(out *UpstreamOIDCProvider) { *out = *in @@ -145,6 +161,11 @@ func (in *UpstreamOIDCProviderList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamOIDCProviderSpec) DeepCopyInto(out *UpstreamOIDCProviderSpec) { *out = *in + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(TLSSpec) + **out = **in + } in.AuthorizationConfig.DeepCopyInto(&out.AuthorizationConfig) out.Claims = in.Claims out.Client = in.Client diff --git a/generated/1.19/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml b/generated/1.19/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml index 451a4474..780fe6fe 100644 --- a/generated/1.19/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml +++ b/generated/1.19/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml @@ -98,6 +98,15 @@ spec: minLength: 1 pattern: ^https:// type: string + tls: + description: TLS configuration for discovery/JWKS requests to the + issuer. + properties: + certificateAuthorityData: + description: X.509 Certificate Authority (base64-encoded PEM bundle). + If omitted, a default set of system roots will be trusted. + type: string + type: object required: - client - issuer From ee978fdde8cc257501600527413f1106759f60ae Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 16 Nov 2020 18:15:58 -0600 Subject: [PATCH 4/5] Add controller support for spec.tls field. Signed-off-by: Matt Moyer --- .../upstreamwatcher/upstreamwatcher.go | 81 +++++++++-- .../upstreamwatcher/upstreamwatcher_test.go | 136 +++++++++++++++--- 2 files changed, 191 insertions(+), 26 deletions(-) diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go index 144d2944..bc3db3bf 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go @@ -6,7 +6,11 @@ package upstreamwatcher import ( "context" + "crypto/tls" + "crypto/x509" + "encoding/base64" "fmt" + "net/http" "net/url" "sort" "time" @@ -48,10 +52,12 @@ const ( reasonMissingKeys = "SecretMissingKeys" reasonSuccess = "Success" reasonUnreachable = "Unreachable" + reasonInvalidTLSConfig = "InvalidTLSConfig" reasonInvalidResponse = "InvalidResponse" // Errors that are generated by our reconcile process. - errFailureStatus = constable.Error("UpstreamOIDCProvider has a failing condition") + errFailureStatus = constable.Error("UpstreamOIDCProvider has a failing condition") + errNoCertificates = constable.Error("no certificates found") ) // IDPCache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations. @@ -59,13 +65,39 @@ type IDPCache interface { SetIDPList([]provider.UpstreamOIDCIdentityProvider) } +// lruValidatorCache caches the *oidc.Provider associated with a particular issuer/TLS configuration. +type lruValidatorCache struct{ cache *cache.Expiring } + +func (c *lruValidatorCache) getProvider(spec *v1alpha1.UpstreamOIDCProviderSpec) *oidc.Provider { + if result, ok := c.cache.Get(c.cacheKey(spec)); ok { + return result.(*oidc.Provider) + } + return nil +} + +func (c *lruValidatorCache) putProvider(spec *v1alpha1.UpstreamOIDCProviderSpec, provider *oidc.Provider) { + c.cache.Set(c.cacheKey(spec), provider, validatorCacheTTL) +} + +func (c *lruValidatorCache) cacheKey(spec *v1alpha1.UpstreamOIDCProviderSpec) interface{} { + var key struct{ issuer, caBundle string } + key.issuer = spec.Issuer + if spec.TLS != nil { + key.caBundle = spec.TLS.CertificateAuthorityData + } + return key +} + type controller struct { cache IDPCache log logr.Logger client pinnipedclientset.Interface providers idpinformers.UpstreamOIDCProviderInformer secrets corev1informers.SecretInformer - validatorCache *cache.Expiring + validatorCache interface { + getProvider(spec *v1alpha1.UpstreamOIDCProviderSpec) *oidc.Provider + putProvider(spec *v1alpha1.UpstreamOIDCProviderSpec, provider *oidc.Provider) + } } // New instantiates a new controllerlib.Controller which will populate the provided IDPCache. @@ -82,7 +114,7 @@ func New( client: client, providers: providers, secrets: secrets, - validatorCache: cache.NewExpiring(), + validatorCache: &lruValidatorCache{cache: cache.NewExpiring()}, } filter := pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()) return controllerlib.New( @@ -197,15 +229,22 @@ func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, res // validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition. func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition { // Get the provider (from cache if possible). - var discoveredProvider *oidc.Provider - if cached, ok := c.validatorCache.Get(upstream.Spec.Issuer); ok { - discoveredProvider = cached.(*oidc.Provider) - } + discoveredProvider := c.validatorCache.getProvider(&upstream.Spec) // If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache. if discoveredProvider == nil { - var err error - discoveredProvider, err = oidc.NewProvider(ctx, upstream.Spec.Issuer) + tlsConfig, err := getTLSConfig(upstream) + if err != nil { + return &v1alpha1.Condition{ + Type: typeOIDCDiscoverySucceeded, + Status: v1alpha1.ConditionFalse, + Reason: reasonInvalidTLSConfig, + Message: err.Error(), + } + } + httpClient := &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} + + discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer) if err != nil { return &v1alpha1.Condition{ Type: typeOIDCDiscoverySucceeded, @@ -216,7 +255,7 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst } // Update the cache with the newly discovered value. - c.validatorCache.Set(upstream.Spec.Issuer, discoveredProvider, validatorCacheTTL) + c.validatorCache.putProvider(&upstream.Spec, discoveredProvider) } // Parse out and validate the discovered authorize endpoint. @@ -248,6 +287,28 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst } } +func getTLSConfig(upstream *v1alpha1.UpstreamOIDCProvider) (*tls.Config, error) { + result := tls.Config{ + MinVersion: tls.VersionTLS12, + } + + if upstream.Spec.TLS == nil || upstream.Spec.TLS.CertificateAuthorityData == "" { + return &result, nil + } + + bundle, err := base64.StdEncoding.DecodeString(upstream.Spec.TLS.CertificateAuthorityData) + if err != nil { + return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", err) + } + + result.RootCAs = x509.NewCertPool() + if !result.RootCAs.AppendCertsFromPEM(bundle) { + return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", errNoCertificates) + } + + return &result, nil +} + func (c *controller) updateStatus(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, conditions []*v1alpha1.Condition) { log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go index 2e91a523..949effaf 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go @@ -5,9 +5,9 @@ package upstreamwatcher import ( "context" + "encoding/base64" "encoding/json" "net/http" - "net/http/httptest" "net/url" "strings" "testing" @@ -25,6 +25,7 @@ import ( pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/oidc/provider" + "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/testlogger" ) @@ -34,7 +35,8 @@ func TestController(t *testing.T) { earlier := metav1.NewTime(now.Add(-1 * time.Hour).UTC()) // Start another test server that answers discovery successfully. - testIssuer := newTestIssuer(t) + testIssuerCA, testIssuerURL := newTestIssuer(t) + testIssuerCABase64 := base64.StdEncoding.EncodeToString([]byte(testIssuerCA)) testIssuerAuthorizeURL, err := url.Parse("https://example.com/authorize") require.NoError(t, err) @@ -65,7 +67,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL, + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -106,7 +109,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL, + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -151,7 +155,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL, + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -190,6 +195,102 @@ func TestController(t *testing.T) { }, }}, }, + { + name: "TLS CA bundle is invalid base64", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: "invalid-base64", + }, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded client credentials", + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidTLSConfig", + Message: `spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7`, + }, + }, + }, + }}, + }, + { + name: "TLS CA bundle does not have any certificates", + inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"}, + Spec: v1alpha1.UpstreamOIDCProviderSpec{ + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("not-a-pem-ca-bundle")), + }, + Client: v1alpha1.OIDCClient{SecretName: testSecretName}, + AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")}, + }, + }}, + inputSecrets: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName}, + Type: "secrets.pinniped.dev/oidc-client", + Data: testValidSecretData, + }}, + wantErr: controllerlib.ErrSyntheticRequeue.Error(), + wantLogs: []string{ + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, + `upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: no certificates found" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: no certificates found" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`, + }, + wantResultingCache: []provider.UpstreamOIDCIdentityProvider{}, + wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, + Status: v1alpha1.UpstreamOIDCProviderStatus{ + Phase: "Error", + Conditions: []v1alpha1.Condition{ + { + Type: "ClientCredentialsValid", + Status: "True", + LastTransitionTime: now, + Reason: "Success", + Message: "loaded client credentials", + }, + { + Type: "OIDCDiscoverySucceeded", + Status: "False", + LastTransitionTime: now, + Reason: "InvalidTLSConfig", + Message: `spec.certificateAuthorityData is invalid: no certificates found`, + }, + }, + }, + }}, + }, { name: "issuer is invalid URL", inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ @@ -240,7 +341,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL + "/invalid", + Issuer: testIssuerURL + "/invalid", + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -285,7 +387,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL + "/insecure", + Issuer: testIssuerURL + "/insecure", + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -330,7 +433,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL, + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")}, }, @@ -373,7 +477,8 @@ func TestController(t *testing.T) { inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{ ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, Spec: v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: testIssuer.URL, + Issuer: testIssuerURL, + TLS: &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64}, Client: v1alpha1.OIDCClient{SecretName: testSecretName}, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes}, }, @@ -486,10 +591,9 @@ func normalizeUpstreams(upstreams []v1alpha1.UpstreamOIDCProvider, now metav1.Ti return result } -func newTestIssuer(t *testing.T) *httptest.Server { +func newTestIssuer(t *testing.T) (string, string) { mux := http.NewServeMux() - testServer := httptest.NewServer(mux) - t.Cleanup(testServer.Close) + caBundlePEM, testURL := testutil.TLSTestServer(t, mux.ServeHTTP) type providerJSON struct { Issuer string `json:"issuer"` @@ -502,7 +606,7 @@ func newTestIssuer(t *testing.T) *httptest.Server { mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testServer.URL, + Issuer: testURL, AuthURL: "https://example.com/authorize", }) }) @@ -511,7 +615,7 @@ func newTestIssuer(t *testing.T) *httptest.Server { mux.HandleFunc("/invalid/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testServer.URL + "/invalid", + Issuer: testURL + "/invalid", AuthURL: "%", }) }) @@ -520,10 +624,10 @@ func newTestIssuer(t *testing.T) *httptest.Server { mux.HandleFunc("/insecure/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") _ = json.NewEncoder(w).Encode(&providerJSON{ - Issuer: testServer.URL + "/insecure", + Issuer: testURL + "/insecure", AuthURL: "http://example.com/authorize", }) }) - return testServer + return caBundlePEM, testURL } From b31deff0fbd39cc8fdb450692d32f9c623c49800 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 16 Nov 2020 18:16:16 -0600 Subject: [PATCH 5/5] Update integration tests to use HTTPS Dex for UpstreamOIDCProvider testing. Signed-off-by: Matt Moyer --- test/integration/supervisor_upstream_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/integration/supervisor_upstream_test.go b/test/integration/supervisor_upstream_test.go index 40ef0e90..c96ae7af 100644 --- a/test/integration/supervisor_upstream_test.go +++ b/test/integration/supervisor_upstream_test.go @@ -5,6 +5,7 @@ package integration import ( "context" + "encoding/base64" "testing" "time" @@ -17,7 +18,7 @@ import ( ) func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) { - library.SkipUnlessIntegration(t) + env := library.IntegrationEnv(t) t.Run("invalid missing secret and bad issuer", func(t *testing.T) { t.Parallel() @@ -50,7 +51,10 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) { t.Run("valid", func(t *testing.T) { t.Parallel() spec := v1alpha1.UpstreamOIDCProviderSpec{ - Issuer: "https://accounts.google.com", // Use Google as an example of a valid OIDC issuer for now. + Issuer: env.OIDCUpstream.Issuer, + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.OIDCUpstream.CABundle)), + }, AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{ AdditionalScopes: []string{"email", "profile"}, },