From faee39fbf832b93553ee0bc87681813252081fe1 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 10 Dec 2015 10:00:46 -0800 Subject: [PATCH] Godeps: update coreos/go-oidc to include OIDC race condition fixes Update github.com/coreos/go-oidc/... to include coreos/go-oidc#17 which fixes a race condition in the OIDC connector. --- Godeps/Godeps.json | 10 +-- .../github.com/coreos/go-oidc/oidc/client.go | 67 ++++++++++++------- .../coreos/go-oidc/oidc/provider.go | 7 +- 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index c8606375..b1556a00 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -21,23 +21,23 @@ }, { "ImportPath": "github.com/coreos/go-oidc/http", - "Rev": "faf70c34f9c411f234eb96d23c518c087cd96d79" + "Rev": "024cdeee09d02fb439eb55bc422e582ac115615b" }, { "ImportPath": "github.com/coreos/go-oidc/jose", - "Rev": "faf70c34f9c411f234eb96d23c518c087cd96d79" + "Rev": "024cdeee09d02fb439eb55bc422e582ac115615b" }, { "ImportPath": "github.com/coreos/go-oidc/key", - "Rev": "faf70c34f9c411f234eb96d23c518c087cd96d79" + "Rev": "024cdeee09d02fb439eb55bc422e582ac115615b" }, { "ImportPath": "github.com/coreos/go-oidc/oauth2", - "Rev": "faf70c34f9c411f234eb96d23c518c087cd96d79" + "Rev": "024cdeee09d02fb439eb55bc422e582ac115615b" }, { "ImportPath": "github.com/coreos/go-oidc/oidc", - "Rev": "faf70c34f9c411f234eb96d23c518c087cd96d79" + "Rev": "024cdeee09d02fb439eb55bc422e582ac115615b" }, { "ImportPath": "github.com/coreos/pkg/capnslog", diff --git a/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/client.go b/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/client.go index 85a7af21..76330237 100644 --- a/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/client.go +++ b/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/client.go @@ -78,7 +78,7 @@ func NewClient(cfg ClientConfig) (*Client, error) { httpClient: cfg.HTTPClient, scope: cfg.Scope, redirectURL: ru.String(), - providerConfig: cfg.ProviderConfig, + providerConfig: newProviderConfigRepo(cfg.ProviderConfig), keySet: cfg.KeySet, } @@ -96,7 +96,7 @@ func NewClient(cfg ClientConfig) (*Client, error) { type Client struct { httpClient phttp.Client - providerConfig ProviderConfig + providerConfig *providerConfigRepo credentials ClientCredentials redirectURL string scope []string @@ -106,14 +106,39 @@ type Client struct { lastKeySetSync time.Time } +type providerConfigRepo struct { + mu sync.RWMutex + config ProviderConfig // do not access directly, use Get() +} + +func newProviderConfigRepo(pc ProviderConfig) *providerConfigRepo { + return &providerConfigRepo{sync.RWMutex{}, pc} +} + +// returns an error to implement ProviderConfigSetter +func (r *providerConfigRepo) Set(cfg ProviderConfig) error { + r.mu.Lock() + defer r.mu.Unlock() + r.config = cfg + return nil +} + +func (r *providerConfigRepo) Get() ProviderConfig { + r.mu.RLock() + defer r.mu.RUnlock() + return r.config +} + func (c *Client) Healthy() error { now := time.Now().UTC() - if c.providerConfig.Empty() { + cfg := c.providerConfig.Get() + + if cfg.Empty() { return errors.New("oidc client provider config empty") } - if !c.providerConfig.ExpiresAt.IsZero() && c.providerConfig.ExpiresAt.Before(now) { + if !cfg.ExpiresAt.IsZero() && cfg.ExpiresAt.Before(now) { return errors.New("oidc client provider config expired") } @@ -121,7 +146,8 @@ func (c *Client) Healthy() error { } func (c *Client) OAuthClient() (*oauth2.Client, error) { - authMethod, err := c.chooseAuthMethod() + cfg := c.providerConfig.Get() + authMethod, err := chooseAuthMethod(cfg) if err != nil { return nil, err } @@ -129,8 +155,8 @@ func (c *Client) OAuthClient() (*oauth2.Client, error) { ocfg := oauth2.Config{ Credentials: oauth2.ClientCredentials(c.credentials), RedirectURL: c.redirectURL, - AuthURL: c.providerConfig.AuthEndpoint, - TokenURL: c.providerConfig.TokenEndpoint, + AuthURL: cfg.AuthEndpoint, + TokenURL: cfg.TokenEndpoint, Scope: c.scope, AuthMethod: authMethod, } @@ -138,12 +164,12 @@ func (c *Client) OAuthClient() (*oauth2.Client, error) { return oauth2.NewClient(c.httpClient, ocfg) } -func (c *Client) chooseAuthMethod() (string, error) { - if len(c.providerConfig.TokenEndpointAuthMethodsSupported) == 0 { +func chooseAuthMethod(cfg ProviderConfig) (string, error) { + if len(cfg.TokenEndpointAuthMethodsSupported) == 0 { return oauth2.AuthMethodClientSecretBasic, nil } - for _, authMethod := range c.providerConfig.TokenEndpointAuthMethodsSupported { + for _, authMethod := range cfg.TokenEndpointAuthMethodsSupported { if _, ok := supportedAuthMethods[authMethod]; ok { return authMethod, nil } @@ -153,9 +179,8 @@ func (c *Client) chooseAuthMethod() (string, error) { } func (c *Client) SyncProviderConfig(discoveryURL string) chan struct{} { - rp := &providerConfigRepo{c} r := NewHTTPProviderConfigGetter(c.httpClient, discoveryURL) - return NewProviderConfigSyncer(r, rp).Run() + return NewProviderConfigSyncer(r, c.providerConfig).Run() } func (c *Client) maybeSyncKeys() error { @@ -178,7 +203,8 @@ func (c *Client) maybeSyncKeys() error { return nil } - r := NewRemotePublicKeyRepo(c.httpClient, c.providerConfig.KeysEndpoint) + cfg := c.providerConfig.Get() + r := NewRemotePublicKeyRepo(c.httpClient, cfg.KeysEndpoint) w := &clientKeyRepo{client: c} _, err := key.Sync(r, w) c.lastKeySetSync = time.Now().UTC() @@ -186,15 +212,6 @@ func (c *Client) maybeSyncKeys() error { return err } -type providerConfigRepo struct { - client *Client -} - -func (r *providerConfigRepo) Set(cfg ProviderConfig) error { - r.client.providerConfig = cfg - return nil -} - type clientKeyRepo struct { client *Client } @@ -209,7 +226,9 @@ func (r *clientKeyRepo) Set(ks key.KeySet) error { } func (c *Client) ClientCredsToken(scope []string) (jose.JWT, error) { - if !c.providerConfig.SupportsGrantType(oauth2.GrantTypeClientCreds) { + cfg := c.providerConfig.Get() + + if !cfg.SupportsGrantType(oauth2.GrantTypeClientCreds) { return jose.JWT{}, fmt.Errorf("%v grant type is not supported", oauth2.GrantTypeClientCreds) } @@ -280,7 +299,7 @@ func (c *Client) VerifyJWT(jwt jose.JWT) error { } v := NewJWTVerifier( - c.providerConfig.Issuer, + c.providerConfig.Get().Issuer, c.credentials.ID, c.maybeSyncKeys, keysFunc) diff --git a/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/provider.go b/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/provider.go index bfdeef3f..f911e39e 100644 --- a/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/provider.go +++ b/Godeps/_workspace/src/github.com/coreos/go-oidc/oidc/provider.go @@ -25,6 +25,9 @@ const ( discoveryConfigPath = "/.well-known/openid-configuration" ) +// internally configurable for tests +var minimumProviderConfigSyncInterval = MinimumProviderConfigSyncInterval + type ProviderConfig struct { Issuer string `json:"issuer"` AuthEndpoint string `json:"authorization_endpoint"` @@ -172,8 +175,8 @@ func nextSyncAfter(exp time.Time, clock clockwork.Clock) time.Duration { t := exp.Sub(clock.Now()) / 2 if t > MaximumProviderConfigSyncInterval { t = MaximumProviderConfigSyncInterval - } else if t < MinimumProviderConfigSyncInterval { - t = MinimumProviderConfigSyncInterval + } else if t < minimumProviderConfigSyncInterval { + t = minimumProviderConfigSyncInterval } return t