providers: don't cache login and redeem URLs

The code which was caching login and redeem URLs is actually
working in parallel which may cause nil dereferences as the cache
is being wiped out. Don't cache these urls at all.

Author: stlaz

oauthproxy.go

@@ -319,9 +319,9 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *providers.SessionState, e
 	}
 	redirectURI := p.GetRedirectURI(host)
 
-	redeemURL := p.provider.GetRedeemURL()
-	if redeemURL == nil {
-		return s, fmt.Errorf("unable to discover the redeeem endpoint")
+	redeemURL, err := p.provider.GetRedeemURL()
+	if err != nil {
+		return s, err
 	}
 	s, err = p.provider.Redeem(redeemURL, redirectURI, code)
 	if err != nil {
@@ -612,13 +612,10 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	// clear cached endpoints, get fresh ones in case /.well-known/oauth-authorization-server changed its values
-	p.provider.ClearEndpointsCache()
-
 	redirectURI := p.GetRedirectURI(req.Host)
-	loginURL := p.provider.GetLoginURL()
-	if loginURL == nil {
-		p.ErrorPage(rw, 500, "Internal Error", fmt.Sprintf("could not get login endpoint"))
+	loginURL, err := p.provider.GetLoginURL()
+	if err != nil {
+		p.ErrorPage(rw, 500, "Internal Error", err.Error())
 		return
 	}
 	http.Redirect(rw, req, p.provider.GetLoginRedirectURL(*loginURL, redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), 302)

oauthproxy_test.go

@@ -185,12 +185,12 @@ func NewTestProvider(provider_url *url.URL, email_address string) *TestProvider
 	return &TestProvider{
 		ProviderData: &providers.ProviderData{
 			ProviderName: "Test Provider",
-			LoginURL: &url.URL{
+			ConfigLoginURL: &url.URL{
 				Scheme: "http",
 				Host:   provider_url.Host,
 				Path:   "/oauth/authorize",
 			},
-			RedeemURL: &url.URL{
+			ConfigRedeemURL: &url.URL{
 				Scheme: "http",
 				Host:   provider_url.Host,
 				Path:   "/oauth/token",

options.go

@@ -336,8 +336,8 @@ func (o *Options) validateProvider(provider providers.Provider) []string {
 		p.ClientID = data.ClientID
 		p.ClientSecret = data.ClientSecret
 		p.ApprovalPrompt = data.ApprovalPrompt
-		p.LoginURL = data.LoginURL
-		p.RedeemURL = data.RedeemURL
+		p.ConfigLoginURL = data.ConfigLoginURL
+		p.ConfigRedeemURL = data.ConfigRedeemURL
 		p.ProfileURL = data.ProfileURL
 		p.ValidateURL = data.ValidateURL
 	}

providers/openshift/provider.go

@@ -108,11 +108,6 @@ func (p *OpenShiftProvider) LoadDefaults(serviceAccount string, caPaths []string
 		}
 	}
 
-	// attempt to discover endpoints
-	if err := discoverOpenShiftOAuth(defaults, p.Client); err != nil {
-		log.Printf("Unable to discover default cluster OAuth info: %v", err)
-		return defaults, nil
-	}
 	// provide default URLs
 	defaults.ValidateURL = getKubeAPIURLWithPath("/apis/user.openshift.io/v1/users/~")
 
@@ -510,67 +505,54 @@ func (p *OpenShiftProvider) Redeem(redeemURL *url.URL, redirectURL, code string)
 	return
 }
 
-func (p *OpenShiftProvider) GetLoginURL() *url.URL {
+func (p *OpenShiftProvider) GetLoginURL() (*url.URL, error) {
 	if !emptyURL(p.ConfigLoginURL) {
-		return p.ConfigLoginURL
+		return p.ConfigLoginURL, nil
 	}
 
-	if emptyURL(p.LoginURL) {
-		// clear the endpoints so that we get all the newly discovered endpoints for all
-		p.ClearEndpointsCache()
-		discoverOpenShiftOAuth(p.ProviderData, p.Client)
-	}
-	return p.LoginURL
+	loginURL, _, err := discoverOpenShiftOAuth(p.Client)
+	return loginURL, err
 }
 
-func (p *OpenShiftProvider) GetRedeemURL() *url.URL {
+func (p *OpenShiftProvider) GetRedeemURL() (*url.URL, error) {
 	if !emptyURL(p.ConfigRedeemURL) {
-		return p.ConfigRedeemURL
+		return p.ConfigRedeemURL, nil
 	}
 
-	if emptyURL(p.RedeemURL) {
-		// clear the endpoints so that we get all the newly discovered endpoints
-		p.ClearEndpointsCache()
-		discoverOpenShiftOAuth(p.ProviderData, p.Client)
-	}
-	return p.RedeemURL
+	_, redeemURL, err := discoverOpenShiftOAuth(p.Client)
+	return redeemURL, err
 }
 
-// discoverOpenshiftOAuth sets the LoginURL and RedeemURL of the supplied ProviderData if they are unset or empty
-func discoverOpenShiftOAuth(provider *providers.ProviderData, client *http.Client) error {
+// discoverOpenshiftOAuth returns the urls of the login and code redeem endpoitns
+// it receives from the /.well-known/oauth-authorization-server endpoint
+func discoverOpenShiftOAuth(client *http.Client) (*url.URL, *url.URL, error) {
 	wellKnownAuthorization := getKubeAPIURLWithPath("/.well-known/oauth-authorization-server")
 	log.Printf("Performing OAuth discovery against %s", wellKnownAuthorization)
 	req, err := http.NewRequest("GET", wellKnownAuthorization.String(), nil)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
 	json, err := request(client, req)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
-	if emptyURL(provider.LoginURL) {
-		if value, err := json.Get("authorization_endpoint").String(); err == nil && len(value) > 0 {
-			if u, err := url.Parse(value); err == nil {
-				provider.LoginURL = u
-			} else {
-				log.Printf("Unable to parse 'authorization_endpoint' from %s: %v", wellKnownAuthorization, err)
-			}
-		} else {
-			log.Printf("No 'authorization_endpoint' provided by %s: %v", wellKnownAuthorization, err)
+
+	var loginURL, redeemURL *url.URL
+	if value, err := json.Get("authorization_endpoint").String(); err == nil && len(value) > 0 {
+		if loginURL, err = url.Parse(value); err != nil {
+			return nil, nil, fmt.Errorf("Unable to parse 'authorization_endpoint' from %s: %v", wellKnownAuthorization, err)
 		}
+	} else {
+		return nil, nil, fmt.Errorf("No 'authorization_endpoint' provided by %s: %v", wellKnownAuthorization, err)
 	}
-	if emptyURL(provider.RedeemURL) {
-		if value, err := json.Get("token_endpoint").String(); err == nil && len(value) > 0 {
-			if u, err := url.Parse(value); err == nil {
-				provider.RedeemURL = u
-			} else {
-				log.Printf("Unable to parse 'token_endpoint' from %s: %v", wellKnownAuthorization, err)
-			}
-		} else {
-			log.Printf("No 'token_endpoint' provided by %s: %v", wellKnownAuthorization, err)
+	if value, err := json.Get("token_endpoint").String(); err == nil && len(value) > 0 {
+		if redeemURL, err = url.Parse(value); err != nil {
+			return nil, nil, fmt.Errorf("Unable to parse 'token_endpoint' from %s: %v", wellKnownAuthorization, err)
 		}
+	} else {
+		return nil, nil, fmt.Errorf("No 'token_endpoint' provided by %s: %v", wellKnownAuthorization, err)
 	}
-	return nil
+	return loginURL, redeemURL, nil
 }
 
 // Copied to override http.Client so that CA can be set

providers/provider_data.go

@@ -8,12 +8,8 @@ type ProviderData struct {
 	ProviderName string
 	ClientID     string
 	ClientSecret string
-	// LoginURL, RedeemURL are cached in runtime, only set/unset
-	// them in cache-related methods
-	LoginURL  *url.URL
-	RedeemURL *url.URL
-	// Config* attributes are attributes that are set in options and override
-	// the cached attributes above
+	// Config* attributes are set in the options, if set, these endpoints won't
+	// be refetched
 	ConfigLoginURL    *url.URL
 	ConfigRedeemURL   *url.URL
 	ValidateURL       *url.URL

providers/provider_default.go

@@ -127,22 +127,18 @@ func (p *ProviderData) RefreshSessionIfNeeded(s *SessionState) (bool, error) {
 	return false, nil
 }
 
-func (p *ProviderData) GetLoginURL() *url.URL {
+func (p *ProviderData) GetLoginURL() (*url.URL, error) {
 	if !(p.ConfigLoginURL == nil || p.ConfigLoginURL.String() == "") {
-		return p.ConfigLoginURL
+		return p.ConfigLoginURL, nil
 	}
 
-	return p.LoginURL
+	return nil, fmt.Errorf("no login endpoint was configured")
 }
 
-func (p *ProviderData) GetRedeemURL() *url.URL {
+func (p *ProviderData) GetRedeemURL() (*url.URL, error) {
 	if !(p.ConfigRedeemURL == nil || p.ConfigRedeemURL.String() == "") {
-		return p.ConfigRedeemURL
+		return p.ConfigRedeemURL, nil
 	}
-	return p.RedeemURL
-}
 
-func (p *ProviderData) ClearEndpointsCache() {
-	p.LoginURL = nil
-	p.RedeemURL = nil
+	return nil, fmt.Errorf("no redeem endpoint was configured")
 }

providers/providers.go

@@ -21,9 +21,8 @@ type Provider interface {
 	SessionFromCookie(string, *cookie.Cipher) (*SessionState, error)
 	CookieForSession(*SessionState, *cookie.Cipher) (string, error)
 	ValidateRequest(*http.Request) (*SessionState, error)
-	GetLoginURL() *url.URL
-	GetRedeemURL() *url.URL
-	ClearEndpointsCache()
+	GetLoginURL() (*url.URL, error)
+	GetRedeemURL() (*url.URL, error)
 }
 
 // ErrPermissionDenied may be returned from Redeem() to indicate the user is not allowed to login.