diff --git a/pkg/apiserver/authenticator/authn.go b/pkg/apiserver/authenticator/authn.go index 44c1eb93aa6..a2d1a716705 100644 --- a/pkg/apiserver/authenticator/authn.go +++ b/pkg/apiserver/authenticator/authn.go @@ -138,7 +138,15 @@ func newAuthenticatorFromTokenFile(tokenAuthFile string) (authenticator.Request, // newAuthenticatorFromOIDCIssuerURL returns an authenticator.Request or an error. func newAuthenticatorFromOIDCIssuerURL(issuerURL, clientID, caFile, usernameClaim, groupsClaim string) (authenticator.Request, error) { - tokenAuthenticator, err := oidc.New(issuerURL, clientID, caFile, usernameClaim, groupsClaim) + tokenAuthenticator, err := oidc.New(oidc.OIDCOptions{ + IssuerURL: issuerURL, + ClientID: clientID, + CAFile: caFile, + UsernameClaim: usernameClaim, + GroupsClaim: groupsClaim, + MaxRetries: oidc.DefaultRetries, + RetryBackoff: oidc.DefaultBackoff, + }) if err != nil { return nil, err } diff --git a/plugin/pkg/auth/authenticator/token/oidc/oidc.go b/plugin/pkg/auth/authenticator/token/oidc/oidc.go index 30a6b72192e..52ea1de4aec 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/oidc.go +++ b/plugin/pkg/auth/authenticator/token/oidc/oidc.go @@ -34,38 +34,52 @@ import ( "k8s.io/kubernetes/pkg/util/net" ) -var ( - maxRetries = 5 - retryBackoff = time.Second * 3 +const ( + DefaultRetries = 5 + DefaultBackoff = time.Second * 3 ) +type OIDCOptions struct { + IssuerURL string + ClientID string + CAFile string + UsernameClaim string + GroupsClaim string + + // 0 disables retry + MaxRetries int + RetryBackoff time.Duration +} + type OIDCAuthenticator struct { clientConfig oidc.ClientConfig client *oidc.Client usernameClaim string groupsClaim string stopSyncProvider chan struct{} + maxRetries int + retryBackoff time.Duration } // 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. -func New(issuerURL, clientID, caFile, usernameClaim, groupsClaim string) (*OIDCAuthenticator, error) { +func New(opts OIDCOptions) (*OIDCAuthenticator, error) { var cfg oidc.ProviderConfig var err error var roots *x509.CertPool - url, err := url.Parse(issuerURL) + url, err := url.Parse(opts.IssuerURL) if err != nil { return nil, err } if url.Scheme != "https" { - return nil, fmt.Errorf("'oidc-issuer-url' (%q) has invalid scheme (%q), require 'https'", issuerURL, url.Scheme) + return nil, fmt.Errorf("'oidc-issuer-url' (%q) has invalid scheme (%q), require 'https'", opts.IssuerURL, url.Scheme) } - if caFile != "" { - roots, err = crypto.CertPoolFromFile(caFile) + if opts.CAFile != "" { + roots, err = crypto.CertPoolFromFile(opts.CAFile) if err != nil { glog.Errorf("Failed to read the CA file: %v", err) } @@ -84,12 +98,21 @@ func New(issuerURL, clientID, caFile, usernameClaim, groupsClaim string) (*OIDCA hc := &http.Client{} hc.Transport = tr + maxRetries := opts.MaxRetries + if maxRetries < 0 { + maxRetries = DefaultRetries + } + retryBackoff := opts.RetryBackoff + if retryBackoff < 0 { + retryBackoff = DefaultBackoff + } + for i := 0; i <= maxRetries; i++ { if i == maxRetries { return nil, fmt.Errorf("failed to fetch provider config after %v retries", maxRetries) } - cfg, err = oidc.FetchProviderConfig(hc, strings.TrimSuffix(issuerURL, "/")) + cfg, err = oidc.FetchProviderConfig(hc, strings.TrimSuffix(opts.IssuerURL, "/")) if err == nil { break } @@ -97,11 +120,11 @@ func New(issuerURL, clientID, caFile, usernameClaim, groupsClaim string) (*OIDCA time.Sleep(retryBackoff) } - glog.Infof("Fetched provider config from %s: %#v", issuerURL, cfg) + glog.Infof("Fetched provider config from %s: %#v", opts.IssuerURL, cfg) ccfg := oidc.ClientConfig{ HTTPClient: hc, - Credentials: oidc.ClientCredentials{ID: clientID}, + Credentials: oidc.ClientCredentials{ID: opts.ClientID}, ProviderConfig: cfg, } @@ -113,9 +136,17 @@ func New(issuerURL, clientID, caFile, usernameClaim, groupsClaim string) (*OIDCA // 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 mininum // and maximum threshold. - stop := client.SyncProviderConfig(issuerURL) + stop := client.SyncProviderConfig(opts.IssuerURL) - return &OIDCAuthenticator{ccfg, client, usernameClaim, groupsClaim, stop}, nil + return &OIDCAuthenticator{ + ccfg, + client, + opts.UsernameClaim, + opts.GroupsClaim, + stop, + maxRetries, + retryBackoff, + }, nil } // AuthenticateToken decodes and verifies a JWT using the OIDC client, if the verification succeeds, diff --git a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go b/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go index bd5fc8bcbf1..f886ca7b572 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go +++ b/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go @@ -200,11 +200,8 @@ func generateSelfSignedCert(t *testing.T, host, certPath, keyPath string) { } func TestOIDCDiscoveryTimeout(t *testing.T) { - maxRetries = 3 - retryBackoff = time.Second - expectErr := fmt.Errorf("failed to fetch provider config after 3 retries") - - _, err := New("https://foo/bar", "client-foo", "", "sub", "") + 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) } @@ -212,7 +209,7 @@ func TestOIDCDiscoveryTimeout(t *testing.T) { func TestOIDCDiscoveryNoKeyEndpoint(t *testing.T) { var err error - expectErr := fmt.Errorf("failed to fetch provider config after 3 retries") + 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") @@ -237,16 +234,13 @@ func TestOIDCDiscoveryNoKeyEndpoint(t *testing.T) { Issuer: mustParseURL(t, srv.URL), // An invalid ProviderConfig. Keys endpoint is required. } - _, err = New(srv.URL, "client-foo", cert, "sub", "") + _, 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) { - maxRetries = 3 - retryBackoff = time.Second - // Verify that plain HTTP issuer URL is forbidden. op := newOIDCProvider(t) srv := httptest.NewServer(op.mux) @@ -260,7 +254,7 @@ func TestOIDCDiscoverySecureConnection(t *testing.T) { expectErr := fmt.Errorf("'oidc-issuer-url' (%q) has invalid scheme (%q), require 'https'", srv.URL, "http") - _, err := New(srv.URL, "client-foo", "", "sub", "") + _, err := New(OIDCOptions{srv.URL, "client-foo", "", "sub", "", 0, 0}) if !reflect.DeepEqual(err, expectErr) { t.Errorf("Expecting %v, but got %v", expectErr, err) } @@ -296,7 +290,7 @@ func TestOIDCDiscoverySecureConnection(t *testing.T) { } // Create a client using cert2, should fail. - _, err = New(tlsSrv.URL, "client-foo", cert2, "sub", "") + _, err = New(OIDCOptions{tlsSrv.URL, "client-foo", cert2, "sub", "", 0, 0}) if err == nil { t.Fatalf("Expecting error, but got nothing") } @@ -417,7 +411,7 @@ func TestOIDCAuthentication(t *testing.T) { } for i, tt := range tests { - client, err := New(srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim) + client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim, 1, 100 * time.Millisecond}) if err != nil { t.Errorf("Unexpected error: %v", err) continue