From e85940ed179f8be36f2333d13f3f1297bb7d9178 Mon Sep 17 00:00:00 2001 From: Bobby Rullo Date: Wed, 11 May 2016 16:49:22 -0700 Subject: [PATCH] add tests for newOIDCAuthProvider --- .../authenticator/token/oidc/oidc_test.go | 29 +---- .../token/oidc/testing/provider.go | 22 ++++ plugin/pkg/client/auth/oidc/oidc.go | 42 ++++--- plugin/pkg/client/auth/oidc/oidc_test.go | 116 +++++++++++++++++- 4 files changed, 166 insertions(+), 43 deletions(-) diff --git a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go b/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go index 5450052a800..f0dab980aa8 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go +++ b/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go @@ -19,7 +19,6 @@ package oidc import ( "fmt" "net/http/httptest" - "net/url" "os" "path" "reflect" @@ -34,14 +33,6 @@ import ( oidctesting "k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/oidc/testing" ) -func mustParseURL(t *testing.T, s string) *url.URL { - u, err := url.Parse(s) - if err != nil { - t.Fatalf("Failed to parse url: %v", err) - } - return u -} - func generateToken(t *testing.T, op *oidctesting.OIDCProvider, iss, sub, aud string, usernameClaim, value, groupsClaim string, groups []string, iat, exp time.Time) string { signer := op.PrivKey.Signer() claims := oidc.NewClaims(iss, sub, aud, iat, exp) @@ -98,7 +89,7 @@ func TestOIDCDiscoveryNoKeyEndpoint(t *testing.T) { defer srv.Close() op.PCFG = oidc.ProviderConfig{ - Issuer: mustParseURL(t, srv.URL), // An invalid ProviderConfig. Keys endpoint is required. + Issuer: oidctesting.MustParseURL(srv.URL), // An invalid ProviderConfig. Keys endpoint is required. } _, err = New(OIDCOptions{srv.URL, "client-foo", cert, "sub", "", 0, 0}) @@ -114,8 +105,8 @@ func TestOIDCDiscoverySecureConnection(t *testing.T) { defer srv.Close() op.PCFG = oidc.ProviderConfig{ - Issuer: mustParseURL(t, srv.URL), - KeysEndpoint: mustParseURL(t, srv.URL+"/keys"), + Issuer: oidctesting.MustParseURL(srv.URL), + KeysEndpoint: oidctesting.MustParseURL(srv.URL + "/keys"), } expectErr := fmt.Errorf("'oidc-issuer-url' (%q) has invalid scheme (%q), require 'https'", srv.URL, "http") @@ -147,8 +138,8 @@ func TestOIDCDiscoverySecureConnection(t *testing.T) { defer tlsSrv.Close() op.PCFG = oidc.ProviderConfig{ - Issuer: mustParseURL(t, tlsSrv.URL), - KeysEndpoint: mustParseURL(t, tlsSrv.URL+"/keys"), + Issuer: oidctesting.MustParseURL(tlsSrv.URL), + KeysEndpoint: oidctesting.MustParseURL(tlsSrv.URL + "/keys"), } // Create a client using cert2, should fail. @@ -179,15 +170,7 @@ func TestOIDCAuthentication(t *testing.T) { defer srv.Close() // A provider config with all required fields. - op.PCFG = oidc.ProviderConfig{ - Issuer: mustParseURL(t, srv.URL), - AuthEndpoint: mustParseURL(t, srv.URL+"/auth"), - TokenEndpoint: mustParseURL(t, srv.URL+"/token"), - KeysEndpoint: mustParseURL(t, srv.URL+"/keys"), - ResponseTypesSupported: []string{"code"}, - SubjectTypesSupported: []string{"public"}, - IDTokenSigningAlgValues: []string{"RS256"}, - } + op.AddMinimalProviderConfig(srv) tests := []struct { userClaim string diff --git a/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go b/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go index 5396b1dde3a..ee2735d539a 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go +++ b/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go @@ -31,6 +31,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "testing" @@ -79,6 +80,19 @@ func (op *OIDCProvider) ServeTLSWithKeyPair(cert, key string) (*httptest.Server, return srv, nil } +func (op *OIDCProvider) AddMinimalProviderConfig(srv *httptest.Server) { + op.PCFG = oidc.ProviderConfig{ + Issuer: MustParseURL(srv.URL), + AuthEndpoint: MustParseURL(srv.URL + "/auth"), + TokenEndpoint: MustParseURL(srv.URL + "/token"), + KeysEndpoint: MustParseURL(srv.URL + "/keys"), + ResponseTypesSupported: []string{"code"}, + SubjectTypesSupported: []string{"public"}, + IDTokenSigningAlgValues: []string{"RS256"}, + } + +} + func (op *OIDCProvider) handleConfig(w http.ResponseWriter, req *http.Request) { b, err := json.Marshal(&op.PCFG) if err != nil { @@ -108,6 +122,14 @@ func (op *OIDCProvider) handleKeys(w http.ResponseWriter, req *http.Request) { w.Write(b) } +func MustParseURL(s string) *url.URL { + u, err := url.Parse(s) + if err != nil { + panic(fmt.Errorf("Failed to parse url: %v", err)) + } + return u +} + // generateSelfSignedCert generates a self-signed cert/key pairs and writes to the certPath/keyPath. // This method is mostly identical to crypto.GenerateSelfSignedCert except for the 'IsCA' and 'KeyUsage' // in the certificate template. (Maybe we can merge these two methods). diff --git a/plugin/pkg/client/auth/oidc/oidc.go b/plugin/pkg/client/auth/oidc/oidc.go index c752ba2b4a8..bf37d569b8e 100644 --- a/plugin/pkg/client/auth/oidc/oidc.go +++ b/plugin/pkg/client/auth/oidc/oidc.go @@ -34,12 +34,12 @@ import ( const ( cfgIssuerUrl = "idp-issuer-url" - cfgClientId = "client-id" + cfgClientID = "client-id" cfgClientSecret = "client-secret" cfgCertificateAuthority = "idp-certificate-authority" cfgCertificateAuthorityData = "idp-certificate-authority-data" cfgExtraScopes = "extra-scopes" - cfgIdToken = "id-token" + cfgIDToken = "id-token" cfgRefreshToken = "refresh-token" ) @@ -50,6 +50,21 @@ func init() { } func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.AuthProviderConfigPersister) (restclient.AuthProvider, error) { + issuer := cfg[cfgIssuerUrl] + if issuer == "" { + return nil, fmt.Errorf("Must provide %s", cfgIssuerUrl) + } + + clientID := cfg[cfgClientID] + if clientID == "" { + return nil, fmt.Errorf("Must provide %s", cfgClientID) + } + + clientSecret := cfg[cfgClientSecret] + if clientSecret == "" { + return nil, fmt.Errorf("Must provide %s", cfgClientSecret) + } + var certAuthData []byte var err error if cfg[cfgCertificateAuthorityData] != "" { @@ -72,11 +87,6 @@ func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.A } hc := &http.Client{Transport: trans} - issuer, ok := cfg[cfgIssuerUrl] - if !ok || issuer == "" { - return nil, errors.New("Must provide idp-issuer-url") - } - providerCfg, err := oidc.FetchProviderConfig(hc, strings.TrimSuffix(issuer, "/")) if err != nil { return nil, fmt.Errorf("error fetching provider config: %v", err) @@ -86,8 +96,8 @@ func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.A oidcCfg := oidc.ClientConfig{ HTTPClient: hc, Credentials: oidc.ClientCredentials{ - ID: cfg[cfgClientId], - Secret: cfg[cfgClientSecret], + ID: clientID, + Secret: clientSecret, }, ProviderConfig: providerCfg, Scope: scopes, @@ -99,15 +109,15 @@ func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.A } var initialIDToken jose.JWT - if cfg[cfgIdToken] != "" { - initialIDToken, err = jose.ParseJWT(cfg[cfgIdToken]) + if cfg[cfgIDToken] != "" { + initialIDToken, err = jose.ParseJWT(cfg[cfgIDToken]) if err != nil { return nil, err } } return &oidcAuthProvider{ - intialIDToken: initialIDToken, + initialIDToken: initialIDToken, refresher: &idTokenRefresher{ client: client, cfg: cfg, @@ -117,8 +127,8 @@ func newOIDCAuthProvider(_ string, cfg map[string]string, persister restclient.A } type oidcAuthProvider struct { - refresher *idTokenRefresher - intialIDToken jose.JWT + refresher *idTokenRefresher + initialIDToken jose.JWT } func (g *oidcAuthProvider) WrapTransport(rt http.RoundTripper) http.RoundTripper { @@ -126,7 +136,7 @@ func (g *oidcAuthProvider) WrapTransport(rt http.RoundTripper) http.RoundTripper TokenRefresher: g.refresher, RoundTripper: rt, } - at.SetJWT(g.intialIDToken) + at.SetJWT(g.initialIDToken) return at } @@ -185,7 +195,7 @@ func (r *idTokenRefresher) Refresh() (jose.JWT, error) { if tokens.RefreshToken != "" && tokens.RefreshToken != rt { r.cfg[cfgRefreshToken] = tokens.RefreshToken } - r.cfg[cfgIdToken] = jwt.Encode() + r.cfg[cfgIDToken] = jwt.Encode() err = r.persister.Persist(r.cfg) if err != nil { diff --git a/plugin/pkg/client/auth/oidc/oidc_test.go b/plugin/pkg/client/auth/oidc/oidc_test.go index cba7395ebc0..767781e63b2 100644 --- a/plugin/pkg/client/auth/oidc/oidc_test.go +++ b/plugin/pkg/client/auth/oidc/oidc_test.go @@ -1,14 +1,62 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package oidc import ( + "encoding/base64" + "io/ioutil" + "os" + "path" "testing" - "k8s.io/kubernetes/pkg/util/diff" - "github.com/coreos/go-oidc/jose" + + "k8s.io/kubernetes/pkg/util/diff" + oidctesting "k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/oidc/testing" ) func TestNewOIDCAuthProvider(t *testing.T) { + cert := path.Join(os.TempDir(), "oidc-cert") + key := path.Join(os.TempDir(), "oidc-key") + + defer os.Remove(cert) + defer os.Remove(key) + + oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert, key) + op := oidctesting.NewOIDCProvider(t) + srv, err := op.ServeTLSWithKeyPair(cert, key) + op.AddMinimalProviderConfig(srv) + if err != nil { + t.Fatalf("Cannot start server %v", err) + } + defer srv.Close() + + certData, err := ioutil.ReadFile(cert) + if err != nil { + t.Fatalf("Could not read cert bytes %v", err) + } + + jwt, err := jose.NewSignedJWT(jose.Claims(map[string]interface{}{ + "test": "jwt", + }), op.PrivKey.Signer()) + if err != nil { + t.Fatalf("Could not create signed JWT %v", err) + } + tests := []struct { cfg map[string]string @@ -16,10 +64,70 @@ func TestNewOIDCAuthProvider(t *testing.T) { wantInitialIDToken jose.JWT }{ { + // A Valid configuration cfg: map[string]string{ - cfgIssuerUrl: "auth.example.com", + cfgIssuerUrl: srv.URL, + cfgCertificateAuthority: cert, + cfgClientID: "client-id", + cfgClientSecret: "client-secret", }, }, + { + // A Valid configuration with an Initial JWT + cfg: map[string]string{ + cfgIssuerUrl: srv.URL, + cfgCertificateAuthority: cert, + cfgClientID: "client-id", + cfgClientSecret: "client-secret", + cfgIDToken: jwt.Encode(), + }, + wantInitialIDToken: *jwt, + }, + { + // Valid config, but using cfgCertificateAuthorityData + cfg: map[string]string{ + cfgIssuerUrl: srv.URL, + cfgCertificateAuthorityData: base64.StdEncoding.EncodeToString(certData), + cfgClientID: "client-id", + cfgClientSecret: "client-secret", + }, + }, + { + // Missing client id + cfg: map[string]string{ + cfgIssuerUrl: srv.URL, + cfgCertificateAuthority: cert, + cfgClientSecret: "client-secret", + }, + wantErr: true, + }, + { + // Missing client secret + cfg: map[string]string{ + cfgIssuerUrl: srv.URL, + cfgCertificateAuthority: cert, + cfgClientID: "client-id", + }, + wantErr: true, + }, + { + // Missing issuer url. + cfg: map[string]string{ + cfgCertificateAuthority: cert, + cfgClientID: "client-id", + cfgClientSecret: "secret", + }, + wantErr: true, + }, + { + // No TLS config + cfg: map[string]string{ + cfgIssuerUrl: srv.URL, + cfgClientID: "client-id", + cfgClientSecret: "secret", + }, + wantErr: true, + }, } for i, tt := range tests { @@ -27,8 +135,8 @@ func TestNewOIDCAuthProvider(t *testing.T) { if tt.wantErr { if err == nil { t.Errorf("case %d: want non-nil err", i) - continue } + continue } if err != nil {