Merge branch 'master' into fix-typos

plugin/pkg/auth/authenticator/token/oidc/oidc.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2015 The Kubernetes Authors All rights reserved.
+Copyright 2015 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -14,17 +14,28 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// oidc implements the authenticator.Token interface using the OpenID Connect protocol.
+/*
+oidc implements the authenticator.Token interface using the OpenID Connect protocol.
+
+	config := oidc.OIDCOptions{
+		IssuerURL:     "https://accounts.google.com",
+		ClientID:      os.Getenv("GOOGLE_CLIENT_ID"),
+		UsernameClaim: "email",
+	}
+	tokenAuthenticator, err := oidc.New(config)
+*/
 package oidc
 
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
-	"time"
+	"sync"
+	"sync/atomic"
 
 	"github.com/coreos/go-oidc/jose"
 	"github.com/coreos/go-oidc/oidc"
@@ -32,43 +43,61 @@ import (
 	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/util/crypto"
 	"k8s.io/kubernetes/pkg/util/net"
-)
-
-const (
-	DefaultRetries = 5
-	DefaultBackoff = time.Second * 3
+	"k8s.io/kubernetes/pkg/util/runtime"
 )
 
 type OIDCOptions struct {
-	IssuerURL     string
-	ClientID      string
-	CAFile        string
-	UsernameClaim string
-	GroupsClaim   string
+	// IssuerURL is the URL the provider signs ID Tokens as. This will be the "iss"
+	// field of all tokens produced by the provider and is used for configuration
+	// discovery.
+	//
+	// The URL is usually the provider's URL without a path, for example
+	// "https://accounts.google.com" or "https://login.salesforce.com".
+	//
+	// The provider must implement configuration discovery.
+	// See: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig
+	IssuerURL string
 
-	// 0 disables retry
-	MaxRetries   int
-	RetryBackoff time.Duration
+	// ClientID the JWT must be issued for, the "sub" field. This plugin only trusts a single
+	// client to ensure the plugin can be used with public providers.
+	//
+	// The plugin supports the "authorized party" OpenID Connect claim, which allows
+	// specialized providers to issue tokens to a client for a different client.
+	// See: https://openid.net/specs/openid-connect-core-1_0.html#IDToken
+	ClientID string
+
+	// Path to a PEM encoded root certificate of the provider.
+	CAFile string
+
+	// UsernameClaim is the JWT field to use as the user's username.
+	UsernameClaim string
+
+	// GroupsClaim, if specified, causes the OIDCAuthenticator to try to populate the user's
+	// groups with a ID Token field. If the GrouppClaim field is present in a ID Token the value
+	// must be a list of strings.
+	GroupsClaim string
 }
 
 type OIDCAuthenticator struct {
-	clientConfig     oidc.ClientConfig
-	client           *oidc.Client
-	usernameClaim    string
-	groupsClaim      string
-	stopSyncProvider chan struct{}
-	maxRetries       int
-	retryBackoff     time.Duration
+	issuerURL string
+
+	trustedClientID string
+
+	usernameClaim string
+	groupsClaim   string
+
+	httpClient *http.Client
+
+	// Contains an *oidc.Client. Do not access directly. Use client() method.
+	oidcClient atomic.Value
+
+	// Guards the close method and is used to lock during initialization and closing.
+	mu    sync.Mutex
+	close func() // May be nil
 }
 
-// New creates a new OpenID Connect client with the given issuerURL and clientID.
-// NOTE(yifan): For now we assume the server provides the "jwks_uri" so we don't
-// need to manager the key sets by ourselves.
+// New creates a token authenticator which validates OpenID Connect ID Tokens.
 func New(opts OIDCOptions) (*OIDCAuthenticator, error) {
-	var cfg oidc.ProviderConfig
-	var err error
-	var roots *x509.CertPool
-
 	url, err := url.Parse(opts.IssuerURL)
 	if err != nil {
 		return nil, err
@@ -78,14 +107,18 @@ func New(opts OIDCOptions) (*OIDCAuthenticator, error) {
 		return nil, fmt.Errorf("'oidc-issuer-url' (%q) has invalid scheme (%q), require 'https'", opts.IssuerURL, url.Scheme)
 	}
 
+	if opts.UsernameClaim == "" {
+		return nil, errors.New("no username claim provided")
+	}
+
+	var roots *x509.CertPool
 	if opts.CAFile != "" {
 		roots, err = crypto.CertPoolFromFile(opts.CAFile)
 		if err != nil {
-			glog.Errorf("Failed to read the CA file: %v", err)
+			return nil, fmt.Errorf("Failed to read the CA file: %v", err)
 		}
-	}
-	if roots == nil {
-		glog.Info("No x509 certificates provided, will use host's root CA set")
+	} else {
+		glog.Info("OIDC: No x509 certificates provided, will use host's root CA set")
 	}
 
 	// Copied from http.DefaultTransport.
@@ -95,61 +128,86 @@ func New(opts OIDCOptions) (*OIDCAuthenticator, error) {
 		TLSClientConfig: &tls.Config{RootCAs: roots},
 	})
 
-	hc := &http.Client{}
-	hc.Transport = tr
-
-	maxRetries := opts.MaxRetries
-	if maxRetries < 0 {
-		maxRetries = DefaultRetries
-	}
-	retryBackoff := opts.RetryBackoff
-	if retryBackoff < 0 {
-		retryBackoff = DefaultBackoff
+	authenticator := &OIDCAuthenticator{
+		issuerURL:       opts.IssuerURL,
+		trustedClientID: opts.ClientID,
+		usernameClaim:   opts.UsernameClaim,
+		groupsClaim:     opts.GroupsClaim,
+		httpClient:      &http.Client{Transport: tr},
 	}
 
-	for i := 0; i <= maxRetries; i++ {
-		if i == maxRetries {
-			return nil, fmt.Errorf("failed to fetch provider config after %v retries", maxRetries)
-		}
+	// Attempt to initialize the authenticator asynchronously.
+	//
+	// Ignore errors instead of returning it since the OpenID Connect provider might not be
+	// available yet, for instance if it's running on the cluster and needs the API server
+	// to come up first. Errors will be logged within the client() method.
+	go func() {
+		defer runtime.HandleCrash()
+		authenticator.client()
+	}()
 
-		cfg, err = oidc.FetchProviderConfig(hc, strings.TrimSuffix(opts.IssuerURL, "/"))
-		if err == nil {
-			break
-		}
-		glog.Errorf("Failed to fetch provider config, trying again in %v: %v", retryBackoff, err)
-		time.Sleep(retryBackoff)
+	return authenticator, nil
+}
+
+// Close stops all goroutines used by the authenticator.
+func (a *OIDCAuthenticator) Close() {
+	a.mu.Lock()
+	defer a.mu.Unlock()
+
+	if a.close != nil {
+		a.close()
+	}
+	return
+}
+
+func (a *OIDCAuthenticator) client() (*oidc.Client, error) {
+	// Fast check to see if client has already been initialized.
+	if client := a.oidcClient.Load(); client != nil {
+		return client.(*oidc.Client), nil
 	}
 
-	glog.Infof("Fetched provider config from %s: %#v", opts.IssuerURL, cfg)
-
-	ccfg := oidc.ClientConfig{
-		HTTPClient:     hc,
-		Credentials:    oidc.ClientCredentials{ID: opts.ClientID},
-		ProviderConfig: cfg,
+	// Acquire lock, then recheck initialization.
+	a.mu.Lock()
+	defer a.mu.Unlock()
+	if client := a.oidcClient.Load(); client != nil {
+		return client.(*oidc.Client), nil
 	}
 
-	client, err := oidc.NewClient(ccfg)
+	// Try to initialize client.
+	providerConfig, err := oidc.FetchProviderConfig(a.httpClient, strings.TrimSuffix(a.issuerURL, "/"))
 	if err != nil {
-		return nil, err
+		glog.Errorf("oidc authenticator: failed to fetch provider discovery data: %v", err)
+		return nil, fmt.Errorf("fetch provider config: %v", err)
+	}
+
+	clientConfig := oidc.ClientConfig{
+		HTTPClient:     a.httpClient,
+		Credentials:    oidc.ClientCredentials{ID: a.trustedClientID},
+		ProviderConfig: providerConfig,
+	}
+
+	client, err := oidc.NewClient(clientConfig)
+	if err != nil {
+		glog.Errorf("oidc authenticator: failed to create client: %v", err)
+		return nil, fmt.Errorf("create client: %v", err)
 	}
 
 	// SyncProviderConfig will start a goroutine to periodically synchronize the provider config.
 	// The synchronization interval is set by the expiration length of the config, and has a minimum
 	// and maximum threshold.
-	stop := client.SyncProviderConfig(opts.IssuerURL)
-
-	return &OIDCAuthenticator{
-		ccfg,
-		client,
-		opts.UsernameClaim,
-		opts.GroupsClaim,
-		stop,
-		maxRetries,
-		retryBackoff,
-	}, nil
+	stop := client.SyncProviderConfig(a.issuerURL)
+	a.oidcClient.Store(client)
+	a.close = func() {
+		// This assumes the stop is an unbuffered channel.
+		// So instead of closing the channel, we send am empty struct here.
+		// This guarantees that when this function returns, there is no flying requests,
+		// because a send to an unbuffered channel happens after the receive from the channel.
+		stop <- struct{}{}
+	}
+	return client, nil
 }
 
-// AuthenticateToken decodes and verifies a JWT using the OIDC client, if the verification succeeds,
+// AuthenticateToken decodes and verifies a ID Token using the OIDC client, if the verification succeeds,
 // then it will extract the user info from the JWT claims.
 func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, error) {
 	jwt, err := jose.ParseJWT(value)
@@ -157,7 +215,11 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er
 		return nil, false, err
 	}
 
-	if err := a.client.VerifyJWT(jwt); err != nil {
+	client, err := a.client()
+	if err != nil {
+		return nil, false, err
+	}
+	if err := client.VerifyJWT(jwt); err != nil {
 		return nil, false, err
 	}
 
@@ -181,7 +243,7 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er
 		username = claim
 	default:
 		// For all other cases, use issuerURL + claim as the user name.
-		username = fmt.Sprintf("%s#%s", a.clientConfig.ProviderConfig.Issuer, claim)
+		username = fmt.Sprintf("%s#%s", a.issuerURL, claim)
 	}
 
 	// TODO(yifan): Add UID, also populate the issuer to upper layer.
@@ -199,12 +261,3 @@ func (a *OIDCAuthenticator) AuthenticateToken(value string) (user.Info, bool, er
 	}
 	return info, true, nil
 }
-
-// Close closes the OIDC authenticator, this will close the provider sync goroutine.
-func (a *OIDCAuthenticator) Close() {
-	// This assumes the s.stopSyncProvider is an unbuffered channel.
-	// So instead of closing the channel, we send am empty struct here.
-	// This guarantees that when this function returns, there is no flying requests,
-	// because a send to an unbuffered channel happens after the receive from the channel.
-	a.stopSyncProvider <- struct{}{}
-}

plugin/pkg/auth/authenticator/token/oidc/oidc_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2015 The Kubernetes Authors All rights reserved.
+Copyright 2015 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -18,10 +18,10 @@ package oidc
 
 import (
 	"fmt"
-	"net/http/httptest"
 	"os"
 	"path"
 	"reflect"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -61,61 +61,7 @@ func generateExpiredToken(t *testing.T, op *oidctesting.OIDCProvider, iss, sub,
 	return generateToken(t, op, iss, sub, aud, usernameClaim, value, groupsClaim, groups, time.Now().Add(-2*time.Hour), time.Now().Add(-1*time.Hour))
 }
 
-func TestOIDCDiscoveryTimeout(t *testing.T) {
-	expectErr := fmt.Errorf("failed to fetch provider config after 1 retries")
-	_, err := New(OIDCOptions{"https://127.0.0.1:9999/bar", "client-foo", "", "sub", "", 1, 100 * time.Millisecond})
-	if !reflect.DeepEqual(err, expectErr) {
-		t.Errorf("Expecting %v, but got %v", expectErr, err)
-	}
-}
-
-func TestOIDCDiscoveryNoKeyEndpoint(t *testing.T) {
-	var err error
-	expectErr := fmt.Errorf("failed to fetch provider config after 0 retries")
-
-	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)
-	if err != nil {
-		t.Fatalf("Cannot start server %v", err)
-	}
-	defer srv.Close()
-
-	op.PCFG = oidc.ProviderConfig{
-		Issuer: oidctesting.MustParseURL(srv.URL), // An invalid ProviderConfig. Keys endpoint is required.
-	}
-
-	_, err = New(OIDCOptions{srv.URL, "client-foo", cert, "sub", "", 0, 0})
-	if !reflect.DeepEqual(err, expectErr) {
-		t.Errorf("Expecting %v, but got %v", expectErr, err)
-	}
-}
-
-func TestOIDCDiscoverySecureConnection(t *testing.T) {
-	// Verify that plain HTTP issuer URL is forbidden.
-	op := oidctesting.NewOIDCProvider(t)
-	srv := httptest.NewServer(op.Mux)
-	defer srv.Close()
-
-	op.PCFG = oidc.ProviderConfig{
-		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")
-
-	_, err := New(OIDCOptions{srv.URL, "client-foo", "", "sub", "", 0, 0})
-	if !reflect.DeepEqual(err, expectErr) {
-		t.Errorf("Expecting %v, but got %v", expectErr, err)
-	}
-
+func TestTLSConfig(t *testing.T) {
 	// Verify the cert/key pair works.
 	cert1 := path.Join(os.TempDir(), "oidc-cert-1")
 	key1 := path.Join(os.TempDir(), "oidc-key-1")
@@ -130,24 +76,105 @@ func TestOIDCDiscoverySecureConnection(t *testing.T) {
 	oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert1, key1)
 	oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert2, key2)
 
-	// Create a TLS server using cert/key pair 1.
-	tlsSrv, err := op.ServeTLSWithKeyPair(cert1, key1)
-	if err != nil {
-		t.Fatalf("Cannot start server: %v", err)
-	}
-	defer tlsSrv.Close()
+	tests := []struct {
+		testCase string
 
-	op.PCFG = oidc.ProviderConfig{
-		Issuer:       oidctesting.MustParseURL(tlsSrv.URL),
-		KeysEndpoint: oidctesting.MustParseURL(tlsSrv.URL + "/keys"),
+		serverCertFile string
+		serverKeyFile  string
+
+		trustedCertFile string
+
+		wantErr bool
+	}{
+		{
+			testCase:       "provider using untrusted custom cert",
+			serverCertFile: cert1,
+			serverKeyFile:  key1,
+			wantErr:        true,
+		},
+		{
+			testCase:        "provider using untrusted cert",
+			serverCertFile:  cert1,
+			serverKeyFile:   key1,
+			trustedCertFile: cert2,
+			wantErr:         true,
+		},
+		{
+			testCase:        "provider using trusted cert",
+			serverCertFile:  cert1,
+			serverKeyFile:   key1,
+			trustedCertFile: cert1,
+			wantErr:         false,
+		},
 	}
 
-	// Create a client using cert2, should fail.
-	_, err = New(OIDCOptions{tlsSrv.URL, "client-foo", cert2, "sub", "", 0, 0})
-	if err == nil {
-		t.Fatalf("Expecting error, but got nothing")
-	}
+	for _, tc := range tests {
+		func() {
+			op := oidctesting.NewOIDCProvider(t)
+			srv, err := op.ServeTLSWithKeyPair(tc.serverCertFile, tc.serverKeyFile)
+			if err != nil {
+				t.Errorf("%s: %v", tc.testCase, err)
+				return
+			}
+			defer srv.Close()
+			op.AddMinimalProviderConfig(srv)
 
+			issuer := srv.URL
+			clientID := "client-foo"
+
+			options := OIDCOptions{
+				IssuerURL:     srv.URL,
+				ClientID:      clientID,
+				CAFile:        tc.trustedCertFile,
+				UsernameClaim: "email",
+				GroupsClaim:   "groups",
+			}
+
+			authenticator, err := New(options)
+			if err != nil {
+				t.Errorf("%s: failed to initialize authenticator: %v", tc.testCase, err)
+				return
+			}
+			defer authenticator.Close()
+
+			email := "user-1@example.com"
+			groups := []string{"group1", "group2"}
+			sort.Strings(groups)
+
+			token := generateGoodToken(t, op, issuer, "user-1", clientID, "email", email, "groups", groups)
+
+			// Because this authenticator behaves differently for subsequent requests, run these
+			// tests multiple times (but expect the same result).
+			for i := 1; i < 4; i++ {
+
+				user, ok, err := authenticator.AuthenticateToken(token)
+				if err != nil {
+					if !tc.wantErr {
+						t.Errorf("%s (req #%d): failed to authenticate token: %v", tc.testCase, i, err)
+					}
+					continue
+				}
+
+				if tc.wantErr {
+					t.Errorf("%s (req #%d): expected error authenticating", tc.testCase, i)
+					continue
+				}
+				if !ok {
+					t.Errorf("%s (req #%d): did not get user or error", tc.testCase, i)
+					continue
+				}
+
+				if gotUsername := user.GetName(); email != gotUsername {
+					t.Errorf("%s (req #%d): GetName() expected=%q got %q", tc.testCase, i, email, gotUsername)
+				}
+				gotGroups := user.GetGroups()
+				sort.Strings(gotGroups)
+				if !reflect.DeepEqual(gotGroups, groups) {
+					t.Errorf("%s (req #%d): GetGroups() expected=%q got %q", tc.testCase, i, groups, gotGroups)
+				}
+			}
+		}()
+	}
 }
 
 func TestOIDCAuthentication(t *testing.T) {
@@ -252,7 +279,7 @@ func TestOIDCAuthentication(t *testing.T) {
 	}
 
 	for i, tt := range tests {
-		client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim, 1, 100 * time.Millisecond})
+		client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim})
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 			continue

plugin/pkg/auth/authenticator/token/oidc/testing/provider.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

plugin/pkg/auth/authenticator/token/tokenfile/tokenfile.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 The Kubernetes Authors All rights reserved.
+Copyright 2014 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

plugin/pkg/auth/authenticator/token/tokenfile/tokenfile_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 The Kubernetes Authors All rights reserved.
+Copyright 2014 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

plugin/pkg/auth/authenticator/token/tokentest/tokentest.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2014 The Kubernetes Authors All rights reserved.
+Copyright 2014 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

plugin/pkg/auth/authenticator/token/webhook/certs_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

plugin/pkg/auth/authenticator/token/webhook/webhook.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -18,12 +18,14 @@ limitations under the License.
 package webhook
 
 import (
+	"fmt"
 	"time"
 
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apis/authentication.k8s.io/v1beta1"
 	"k8s.io/kubernetes/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/user"
+	"k8s.io/kubernetes/pkg/client/restclient"
 	"k8s.io/kubernetes/pkg/util/cache"
 	"k8s.io/kubernetes/plugin/pkg/webhook"
 
@@ -34,6 +36,8 @@ var (
 	groupVersions = []unversioned.GroupVersion{v1beta1.SchemeGroupVersion}
 )
 
+const retryBackoff = 500 * time.Millisecond
+
 // Ensure WebhookTokenAuthenticator implements the authenticator.Token interface.
 var _ authenticator.Token = (*WebhookTokenAuthenticator)(nil)
 
@@ -45,7 +49,12 @@ type WebhookTokenAuthenticator struct {
 
 // New creates a new WebhookTokenAuthenticator from the provided kubeconfig file.
 func New(kubeConfigFile string, ttl time.Duration) (*WebhookTokenAuthenticator, error) {
-	gw, err := webhook.NewGenericWebhook(kubeConfigFile, groupVersions)
+	return newWithBackoff(kubeConfigFile, ttl, retryBackoff)
+}
+
+// newWithBackoff allows tests to skip the sleep.
+func newWithBackoff(kubeConfigFile string, ttl, initialBackoff time.Duration) (*WebhookTokenAuthenticator, error) {
+	gw, err := webhook.NewGenericWebhook(kubeConfigFile, groupVersions, initialBackoff)
 	if err != nil {
 		return nil, err
 	}
@@ -60,15 +69,21 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(token string) (user.Info,
 	if entry, ok := w.responseCache.Get(r.Spec); ok {
 		r.Status = entry.(v1beta1.TokenReviewStatus)
 	} else {
-		result := w.RestClient.Post().Body(r).Do()
+		result := w.WithExponentialBackoff(func() restclient.Result {
+			return w.RestClient.Post().Body(r).Do()
+		})
 		if err := result.Error(); err != nil {
 			return nil, false, err
 		}
+		var statusCode int
+		if result.StatusCode(&statusCode); statusCode < 200 || statusCode >= 300 {
+			return nil, false, fmt.Errorf("Error contacting webhook: %d", statusCode)
+		}
 		spec := r.Spec
 		if err := result.Into(r); err != nil {
 			return nil, false, err
 		}
-		go w.responseCache.Add(spec, r.Status, w.ttl)
+		w.responseCache.Add(spec, r.Status, w.ttl)
 	}
 	if !r.Status.Authenticated {
 		return nil, false, nil

plugin/pkg/auth/authenticator/token/webhook/webhook_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors All rights reserved.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@ import (
 	"os"
 	"reflect"
 	"testing"
+	"time"
 
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/apis/authentication.k8s.io/v1beta1"
@@ -39,6 +40,7 @@ type Service interface {
 	// Review looks at the TokenReviewSpec and provides an authentication
 	// response in the TokenReviewStatus.
 	Review(*v1beta1.TokenReview)
+	HTTPStatusCode() int
 }
 
 // NewTestServer wraps a Service as an httptest.Server.
@@ -68,6 +70,10 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
 			http.Error(w, fmt.Sprintf("failed to decode body: %v", err), http.StatusBadRequest)
 			return
 		}
+		if s.HTTPStatusCode() < 200 || s.HTTPStatusCode() >= 300 {
+			http.Error(w, "HTTP Error", s.HTTPStatusCode())
+			return
+		}
 		s.Review(&review)
 		type userInfo struct {
 			Username string   `json:"username"`
@@ -104,7 +110,8 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
 
 // A service that can be set to say yes or no to authentication requests.
 type mockService struct {
-	allow bool
+	allow      bool
+	statusCode int
 }
 
 func (m *mockService) Review(r *v1beta1.TokenReview) {
@@ -113,12 +120,13 @@ func (m *mockService) Review(r *v1beta1.TokenReview) {
 		r.Status.User.Username = "realHooman@email.com"
 	}
 }
-func (m *mockService) Allow() { m.allow = true }
-func (m *mockService) Deny()  { m.allow = false }
+func (m *mockService) Allow()              { m.allow = true }
+func (m *mockService) Deny()               { m.allow = false }
+func (m *mockService) HTTPStatusCode() int { return m.statusCode }
 
 // newTokenAuthenticator creates a temporary kubeconfig file from the provided
 // arguments and attempts to load a new WebhookTokenAuthenticator from it.
-func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte) (*WebhookTokenAuthenticator, error) {
+func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration) (*WebhookTokenAuthenticator, error) {
 	tempfile, err := ioutil.TempFile("", "")
 	if err != nil {
 		return nil, err
@@ -140,7 +148,7 @@ func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte) (
 	if err := json.NewEncoder(tempfile).Encode(config); err != nil {
 		return nil, err
 	}
-	return New(p, 0)
+	return newWithBackoff(p, cacheTime, 0)
 }
 
 func TestTLSConfig(t *testing.T) {
@@ -187,6 +195,7 @@ func TestTLSConfig(t *testing.T) {
 		// Use a closure so defer statements trigger between loop iterations.
 		func() {
 			service := new(mockService)
+			service.statusCode = 200
 
 			server, err := NewTestServer(service, tt.serverCert, tt.serverKey, tt.serverCA)
 			if err != nil {
@@ -195,7 +204,7 @@ func TestTLSConfig(t *testing.T) {
 			}
 			defer server.Close()
 
-			wh, err := newTokenAuthenticator(server.URL, tt.clientCert, tt.clientKey, tt.clientCA)
+			wh, err := newTokenAuthenticator(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, 0)
 			if err != nil {
 				t.Errorf("%s: failed to create client: %v", tt.test, err)
 				return
@@ -239,6 +248,8 @@ func (rec *recorderService) Review(r *v1beta1.TokenReview) {
 	r.Status = rec.response
 }
 
+func (rec *recorderService) HTTPStatusCode() int { return 200 }
+
 func TestWebhookTokenAuthenticator(t *testing.T) {
 	serv := &recorderService{}
 
@@ -248,7 +259,7 @@ func TestWebhookTokenAuthenticator(t *testing.T) {
 	}
 	defer s.Close()
 
-	wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert)
+	wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 0)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -350,3 +361,52 @@ func (a *authenticationUserInfo) GetExtra() map[string][]string { return a.Extra
 // Ensure v1beta1.UserInfo contains the fields necessary to implement the
 // user.Info interface.
 var _ user.Info = (*authenticationUserInfo)(nil)
+
+// TestWebhookCache verifies that error responses from the server are not
+// cached, but successful responses are.
+func TestWebhookCache(t *testing.T) {
+	serv := new(mockService)
+	s, err := NewTestServer(serv, serverCert, serverKey, caCert)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+
+	// Create an authenticator that caches successful responses "forever" (100 days).
+	wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 2400*time.Hour)
+	if err != nil {
+		t.Fatal(err)
+	}
+	token := "t0k3n"
+	serv.allow = true
+	serv.statusCode = 500
+	if _, _, err := wh.AuthenticateToken(token); err == nil {
+		t.Errorf("Webhook returned HTTP 500, but authorizer reported success.")
+	}
+	serv.statusCode = 404
+	if _, _, err := wh.AuthenticateToken(token); err == nil {
+		t.Errorf("Webhook returned HTTP 404, but authorizer reported success.")
+	}
+	serv.statusCode = 200
+	if _, _, err := wh.AuthenticateToken(token); err != nil {
+		t.Errorf("Webhook returned HTTP 200, but authorizer reported unauthorized.")
+	}
+	serv.statusCode = 500
+	if _, _, err := wh.AuthenticateToken(token); err != nil {
+		t.Errorf("Webhook should have successful response cached, but authorizer reported unauthorized.")
+	}
+	// For a different request, webhook should be called again.
+	token = "an0th3r_t0k3n"
+	serv.statusCode = 500
+	if _, _, err := wh.AuthenticateToken(token); err == nil {
+		t.Errorf("Webhook returned HTTP 500, but authorizer reported success.")
+	}
+	serv.statusCode = 200
+	if _, _, err := wh.AuthenticateToken(token); err != nil {
+		t.Errorf("Webhook returned HTTP 200, but authorizer reported unauthorized.")
+	}
+	serv.statusCode = 500
+	if _, _, err := wh.AuthenticateToken(token); err != nil {
+		t.Errorf("Webhook should have successful response cached, but authorizer reported unauthorized.")
+	}
+}