Merge pull request #101 from stlaz/repeat_fetch_2

openshift-merge-robot · web-flow · commit d98a037cebf9 · 2019-02-15T09:44:27.000+01:00
openshift provider: refetch oauth endpoints on login
diff --git a/oauthproxy.go b/oauthproxy.go
@@ -318,7 +318,12 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *providers.SessionState, e
 		return nil, errors.New("missing code")
 	}
 	redirectURI := p.GetRedirectURI(host)
-	s, err = p.provider.Redeem(redirectURI, code)
+
+	redeemURL := p.provider.GetRedeemURL()
+	if redeemURL == nil {
+		return s, fmt.Errorf("unable to discover the redeeem endpoint")
+	}
+	s, err = p.provider.Redeem(redeemURL, redirectURI, code)
 	if err != nil {
 		return
 	}
@@ -606,8 +611,17 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
 		p.ErrorPage(rw, 500, "Internal Error", err.Error())
 		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)
-	http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), 302)
+	loginURL := p.provider.GetLoginURL()
+	if loginURL == nil {
+		p.ErrorPage(rw, 500, "Internal Error", fmt.Sprintf("could not get login endpoint"))
+		return
+	}
+	http.Redirect(rw, req, p.provider.GetLoginRedirectURL(*loginURL, redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), 302)
 }
 
 func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
diff --git a/options.go b/options.go
@@ -153,12 +153,6 @@ func (o *Options) Validate(p providers.Provider) error {
 		if len(o.Scope) == 0 {
 			o.Scope = defaults.Scope
 		}
-		if len(o.LoginURL) == 0 && defaults.LoginURL != nil {
-			o.LoginURL = defaults.LoginURL.String()
-		}
-		if len(o.RedeemURL) == 0 && defaults.RedeemURL != nil {
-			o.RedeemURL = defaults.RedeemURL.String()
-		}
 		if len(o.ValidateURL) == 0 && defaults.ValidateURL != nil {
 			o.ValidateURL = defaults.ValidateURL.String()
 		}
@@ -318,8 +312,9 @@ func (o *Options) validateProvider(provider providers.Provider) []string {
 		ClientSecret:   o.ClientSecret,
 		ApprovalPrompt: o.ApprovalPrompt,
 	}
-	data.LoginURL, msgs = parseURL(o.LoginURL, "login", msgs)
-	data.RedeemURL, msgs = parseURL(o.RedeemURL, "redeem", msgs)
+	data.ConfigLoginURL, msgs = parseURL(o.LoginURL, "login", msgs)
+	data.ConfigRedeemURL, msgs = parseURL(o.RedeemURL, "redeem", msgs)
+
 	data.ProfileURL, msgs = parseURL(o.ProfileURL, "profile", msgs)
 	data.ValidateURL, msgs = parseURL(o.ValidateURL, "validate", msgs)
 	if len(msgs) != 0 {
diff --git a/providers/openshift/provider.go b/providers/openshift/provider.go
@@ -441,7 +441,7 @@ func (p *OpenShiftProvider) ReviewUser(name, accessToken, host string) error {
 }
 
 // Copied up only to set a different client CA
-func (p *OpenShiftProvider) Redeem(redirectURL, code string) (s *providers.SessionState, err error) {
+func (p *OpenShiftProvider) Redeem(redeemURL *url.URL, redirectURL, code string) (s *providers.SessionState, err error) {
 	if code == "" {
 		err = errors.New("missing code")
 		return
@@ -462,7 +462,7 @@ func (p *OpenShiftProvider) Redeem(redirectURL, code string) (s *providers.Sessi
 	}
 
 	var req *http.Request
-	req, err = http.NewRequest("POST", p.RedeemURL.String(), bytes.NewBufferString(params.Encode()))
+	req, err = http.NewRequest("POST", redeemURL.String(), bytes.NewBufferString(params.Encode()))
 	if err != nil {
 		return
 	}
@@ -481,7 +481,7 @@ func (p *OpenShiftProvider) Redeem(redirectURL, code string) (s *providers.Sessi
 	}
 
 	if resp.StatusCode != 200 {
-		err = fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RedeemURL.String(), body)
+		err = fmt.Errorf("got %d from %q %s", resp.StatusCode, redeemURL.String(), body)
 		return
 	}
 
@@ -510,6 +510,33 @@ func (p *OpenShiftProvider) Redeem(redirectURL, code string) (s *providers.Sessi
 	return
 }
 
+func (p *OpenShiftProvider) GetLoginURL() *url.URL {
+	if !emptyURL(p.ConfigLoginURL) {
+		return p.ConfigLoginURL
+	}
+
+	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
+}
+
+func (p *OpenShiftProvider) GetRedeemURL() *url.URL {
+	if !emptyURL(p.ConfigRedeemURL) {
+		return p.ConfigRedeemURL
+	}
+
+	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
+}
+
+// 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 {
 	wellKnownAuthorization := getKubeAPIURLWithPath("/.well-known/oauth-authorization-server")
 	log.Printf("Performing OAuth discovery against %s", wellKnownAuthorization)
diff --git a/providers/provider_data.go b/providers/provider_data.go
@@ -5,14 +5,20 @@ import (
 )
 
 type ProviderData struct {
-	ProviderName      string
-	ClientID          string
-	ClientSecret      string
-	LoginURL          *url.URL
-	RedeemURL         *url.URL
+	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
+	ConfigLoginURL    *url.URL
+	ConfigRedeemURL   *url.URL
+	ValidateURL       *url.URL
 	ProfileURL        *url.URL
 	ProtectedResource *url.URL
-	ValidateURL       *url.URL
 	Scope             string
 	ApprovalPrompt    string
 }
diff --git a/providers/provider_default.go b/providers/provider_default.go
@@ -12,7 +12,7 @@ import (
 	"github.com/openshift/oauth-proxy/cookie"
 )
 
-func (p *ProviderData) Redeem(redirectURL, code string) (s *SessionState, err error) {
+func (p *ProviderData) Redeem(redeemURL *url.URL, redirectURL, code string) (s *SessionState, err error) {
 	if code == "" {
 		err = errors.New("missing code")
 		return
@@ -29,7 +29,7 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *SessionState, err er
 	}
 
 	var req *http.Request
-	req, err = http.NewRequest("POST", p.RedeemURL.String(), bytes.NewBufferString(params.Encode()))
+	req, err = http.NewRequest("POST", redeemURL.String(), bytes.NewBufferString(params.Encode()))
 	if err != nil {
 		return
 	}
@@ -48,7 +48,7 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *SessionState, err er
 	}
 
 	if resp.StatusCode != 200 {
-		err = fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RedeemURL.String(), body)
+		err = fmt.Errorf("got %d from %q %s", resp.StatusCode, redeemURL.String(), body)
 		return
 	}
 
@@ -77,19 +77,17 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *SessionState, err er
 	return
 }
 
-// GetLoginURL with typical oauth parameters
-func (p *ProviderData) GetLoginURL(redirectURI, state string) string {
-	var a url.URL
-	a = *p.LoginURL
-	params, _ := url.ParseQuery(a.RawQuery)
+// GetLoginRedirectURL with typical oauth parameters
+func (p *ProviderData) GetLoginRedirectURL(loginURL url.URL, redirectURI, state string) string {
+	params, _ := url.ParseQuery(loginURL.RawQuery)
 	params.Set("redirect_uri", redirectURI)
 	params.Set("approval_prompt", p.ApprovalPrompt)
 	params.Add("scope", p.Scope)
 	params.Set("client_id", p.ClientID)
 	params.Set("response_type", "code")
 	params.Add("state", state)
-	a.RawQuery = params.Encode()
-	return a.String()
+	loginURL.RawQuery = params.Encode()
+	return loginURL.String()
 }
 
 // CookieForSession serializes a session state for storage in a cookie
@@ -106,7 +104,7 @@ func (p *ProviderData) GetEmailAddress(s *SessionState) (string, error) {
 	return "", errors.New("not implemented")
 }
 
-func (p *ProviderData) ReviewUser(name, accessToken, host string) (error) {
+func (p *ProviderData) ReviewUser(name, accessToken, host string) error {
 	return nil
 }
 
@@ -128,3 +126,23 @@ func (p *ProviderData) ValidateSessionState(s *SessionState) bool {
 func (p *ProviderData) RefreshSessionIfNeeded(s *SessionState) (bool, error) {
 	return false, nil
 }
+
+func (p *ProviderData) GetLoginURL() *url.URL {
+	if !(p.ConfigLoginURL == nil || p.ConfigLoginURL.String() == "") {
+		return p.ConfigLoginURL
+	}
+
+	return p.LoginURL
+}
+
+func (p *ProviderData) GetRedeemURL() *url.URL {
+	if !(p.ConfigRedeemURL == nil || p.ConfigRedeemURL.String() == "") {
+		return p.ConfigRedeemURL
+	}
+	return p.RedeemURL
+}
+
+func (p *ProviderData) ClearEndpointsCache() {
+	p.LoginURL = nil
+	p.RedeemURL = nil
+}
diff --git a/providers/providers.go b/providers/providers.go
@@ -3,23 +3,27 @@ package providers
 import (
 	"errors"
 	"net/http"
+	"net/url"
 
 	"github.com/openshift/oauth-proxy/cookie"
 )
 
 type Provider interface {
 	Data() *ProviderData
 
-	ReviewUser(name, accessToken, host string) (error)
+	ReviewUser(name, accessToken, host string) error
 	GetEmailAddress(*SessionState) (string, error)
-	Redeem(string, string) (*SessionState, error)
+	Redeem(*url.URL, string, string) (*SessionState, error)
 	ValidateGroup(string) bool
 	ValidateSessionState(*SessionState) bool
-	GetLoginURL(redirectURI, finalRedirect string) string
+	GetLoginRedirectURL(loginURL url.URL, redirectURI, state string) string
 	RefreshSessionIfNeeded(*SessionState) (bool, error)
 	SessionFromCookie(string, *cookie.Cipher) (*SessionState, error)
 	CookieForSession(*SessionState, *cookie.Cipher) (string, error)
 	ValidateRequest(*http.Request) (*SessionState, error)
+	GetLoginURL() *url.URL
+	GetRedeemURL() *url.URL
+	ClearEndpointsCache()
 }
 
 // ErrPermissionDenied may be returned from Redeem() to indicate the user is not allowed to login.
