git.iko.soy/typeable/oauth2-proxy
on master help sign in
Figure out final app redirect URL with proxy aware request utils
commit73fc7706bca4c4be241006c7f0ac2bbcaa160ec0 authorNick Meves <nick.meves@greenhouse.io> dateSat 2 Jan 2021, 13:46 (6 years ago) parentf054682
2 files changed+111−59
oauthproxy.go+102−48 oauthproxy_test.go+9−11
oauthproxy.go+102−48
@@ -24,9 +24,9 @@ import (
2424 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
2525 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
2626 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/middleware"
27+ requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"
2728 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions"
2829 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"
29− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"
3030 "github.com/oauth2-proxy/oauth2-proxy/v7/providers"
3131)
3232
@@ -98,7 +98,6 @@ type OAuthProxy struct {
9898 SetAuthorization bool
9999 PassAuthorization bool
100100 PreferEmailToUser bool
101− ReverseProxy bool
102101 skipAuthPreflight bool
103102 skipJwtBearerTokens bool
104103 templates *template.Template
@@ -201,7 +200,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
201200 UserInfoPath: fmt.Sprintf("%s/userinfo", opts.ProxyPrefix),
202201
203202 ProxyPrefix: opts.ProxyPrefix,
204− ReverseProxy: opts.ReverseProxy,
205203 provider: opts.GetProvider(),
206204 providerNameOverride: opts.ProviderName,
207205 sessionStore: sessionStore,
@@ -231,7 +229,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
231229// the OAuth2 Proxy authentication logic kicks in.
232230// For example forcing HTTPS or health checks.
233231func buildPreAuthChain(opts *options.Options) (alice.Chain, error) {
234− chain := alice.New(middleware.NewScope(opts))
232+ chain := alice.New(middleware.NewScope(opts.ReverseProxy))
235233
236234 if opts.ForceHTTPS {
237235 _, httpsPort, err := net.SplitHostPort(opts.HTTPSAddress)
@@ -368,9 +366,9 @@ func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {
368366 return routes, nil
369367}
370368
371−// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will
369+// GetOAuthRedirectURI returns the redirectURL that the upstream OAuth Provider will
372370// redirect clients to once authenticated
373−func (p *OAuthProxy) GetRedirectURI(host string) string {
371+func (p *OAuthProxy) GetOAuthRedirectURI(host string) string {
374372 // default to the request Host if not set
375373 if p.redirectURL.Host != "" {
376374 return p.redirectURL.String()
@@ -391,7 +389,7 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessio
391389 if code == "" {
392390 return nil, providers.ErrMissingCode
393391 }
394− redirectURI := p.GetRedirectURI(host)
392+ redirectURI := p.GetOAuthRedirectURI(host)
395393 s, err := p.provider.Redeem(ctx, redirectURI, code)
396394 if err != nil {
397395 return nil, err
@@ -420,7 +418,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex
420418 cookieDomain := cookies.GetCookieDomain(req, p.CookieDomains)
421419
422420 if cookieDomain != "" {
423− domain := util.GetRequestHost(req)
421+ domain := requestutil.GetRequestHost(req)
424422 if h, _, err := net.SplitHostPort(domain); err == nil {
425423 domain = h
426424 }
@@ -509,7 +507,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
509507 }
510508 rw.WriteHeader(code)
511509
512− redirectURL, err := p.GetRedirect(req)
510+ redirectURL, err := p.GetAppRedirect(req)
513511 if err != nil {
514512 logger.Errorf("Error obtaining redirect: %v", err)
515513 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())
@@ -568,46 +566,108 @@ func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool) {
568566 return "", false
569567}
570568
571−// GetRedirect reads the query parameter to get the URL to redirect clients to
569+// GetAppRedirect determines the full URL or URI path to redirect clients to
572570// once authenticated with the OAuthProxy
573−func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error) {
574− err = req.ParseForm()
571+// Strategy priority (first legal result is used):
572+// - `rd` querysting parameter
573+// - `X-Auth-Request-Redirect` header
574+// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)
575+// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)
576+// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)
577+// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)
578+// - `/`
579+func (p *OAuthProxy) GetAppRedirect(req *http.Request) (string, error) {
580+ err := req.ParseForm()
575581 if err != nil {
576− return
582+ return "", err
583+ }
584+
585+ // These redirect getter functions are strategies ordered by priority
586+ // for figuring out the redirect URL.
587+ type redirectGetter func(req *http.Request) string
588+ for _, rdGetter := range []redirectGetter{
589+ p.getRdQuerystringRedirect,
590+ p.getXAuthRequestRedirect,
591+ p.getXForwardedHeadersRedirect,
592+ p.getURIRedirect,
593+ } {
594+ if redirect := rdGetter(req); redirect != "" {
595+ return redirect, nil
596+ }
577597 }
578598
579− redirect = req.Header.Get("X-Auth-Request-Redirect")
580− if req.Form.Get("rd") != "" {
581− redirect = req.Form.Get("rd")
582− }
583− // Quirk: On reverse proxies that doesn't have support for
584− // "X-Auth-Request-Redirect" header or dynamic header/query string
585− // manipulation (like Traefik v1 and v2), we can try if the header
586− // X-Forwarded-Host exists or not.
587− if redirect == "" && isForwardedRequest(req, p.ReverseProxy) {
588− redirect = p.getRedirectFromForwardHeaders(req)
589− }
590− if !p.IsValidRedirect(redirect) {
591− // Use RequestURI to preserve ?query
592− redirect = req.URL.RequestURI()
599+ return "/", nil
600+}
593601
594− if strings.HasPrefix(redirect, fmt.Sprintf("%s/", p.ProxyPrefix)) {
595− redirect = "/"
596− }
602+func isForwardedRequest(req *http.Request) bool {
603+ return requestutil.IsProxied(req) &&
604+ req.Host != requestutil.GetRequestHost(req)
605+}
606+
607+func (p *OAuthProxy) hasProxyPrefix(path string) bool {
608+ return strings.HasPrefix(path, fmt.Sprintf("%s/", p.ProxyPrefix))
609+}
610+
611+// getRdQuerystringRedirect handles this GetAppRedirect strategy:
612+// - `rd` querysting parameter
613+func (p *OAuthProxy) getRdQuerystringRedirect(req *http.Request) string {
614+ redirect := req.Form.Get("rd")
615+ if p.IsValidRedirect(redirect) {
616+ return redirect
597617 }
618+ return ""
619+}
598620
599− return
621+// getXAuthRequestRedirect handles this GetAppRedirect strategy:
622+// - `X-Auth-Request-Redirect` Header
623+func (p *OAuthProxy) getXAuthRequestRedirect(req *http.Request) string {
624+ redirect := req.Header.Get("X-Auth-Request-Redirect")
625+ if p.IsValidRedirect(redirect) {
626+ return redirect
627+ }
628+ return ""
600629}
601630
602−// getRedirectFromForwardHeaders returns the redirect URL based on X-Forwarded-{Proto,Host,Uri} headers
603−func (p *OAuthProxy) getRedirectFromForwardHeaders(req *http.Request) string {
604− uri := util.GetRequestURI(req)
631+// getXForwardedHeadersRedirect handles these GetAppRedirect strategies:
632+// - `X-Forwarded-(Proto|Host|Uri)` headers (when ReverseProxy mode is enabled)
633+// - `X-Forwarded-(Proto|Host)` if `Uri` has the ProxyPath (i.e. /oauth2/*)
634+func (p *OAuthProxy) getXForwardedHeadersRedirect(req *http.Request) string {
635+ if !isForwardedRequest(req) {
636+ return ""
637+ }
605638
606− if strings.HasPrefix(uri, fmt.Sprintf("%s/", p.ProxyPrefix)) {
639+ uri := requestutil.GetRequestURI(req)
640+ if p.hasProxyPrefix(uri) {
607641 uri = "/"
608642 }
609643
610− return fmt.Sprintf("%s://%s%s", util.GetRequestProto(req), util.GetRequestHost(req), uri)
644+ redirect := fmt.Sprintf(
645+ "%s://%s%s",
646+ requestutil.GetRequestProto(req),
647+ requestutil.GetRequestHost(req),
648+ uri,
649+ )
650+
651+ if p.IsValidRedirect(redirect) {
652+ return redirect
653+ }
654+ return ""
655+}
656+
657+// getURIRedirect handles these GetAppRedirect strategies:
658+// - `X-Forwarded-Uri` direct URI path (when ReverseProxy mode is enabled)
659+// - `req.URL.RequestURI` if not under the ProxyPath (i.e. /oauth2/*)
660+// - `/`
661+func (p *OAuthProxy) getURIRedirect(req *http.Request) string {
662+ redirect := requestutil.GetRequestURI(req)
663+ if !p.IsValidRedirect(redirect) {
664+ redirect = req.URL.RequestURI()
665+ }
666+
667+ if p.hasProxyPrefix(redirect) {
668+ return "/"
669+ }
670+ return redirect
611671}
612672
613673// splitHostPort separates host and port. If the port is not valid, it returns
@@ -707,12 +767,6 @@ func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {
707767 return false
708768}
709769
710−// isForwardedRequest is used to check if X-Forwarded-Host header exists or not
711−func isForwardedRequest(req *http.Request, reverseProxy bool) bool {
712− isForwarded := req.Host != util.GetRequestHost(req)
713− return isForwarded && reverseProxy
714−}
715−
716770// See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching?hl=en
717771var noCacheHeaders = map[string]string{
718772 "Expires": time.Unix(0, 0).Format(time.RFC1123),
@@ -781,7 +835,7 @@ func (p *OAuthProxy) serveHTTP(rw http.ResponseWriter, req *http.Request) {
781835
782836// SignIn serves a page prompting users to sign in
783837func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {
784− redirect, err := p.GetRedirect(req)
838+ redirect, err := p.GetAppRedirect(req)
785839 if err != nil {
786840 logger.Errorf("Error obtaining redirect: %v", err)
787841 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())
@@ -839,7 +893,7 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) {
839893
840894// SignOut sends a response to clear the authentication cookie
841895func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {
842− redirect, err := p.GetRedirect(req)
896+ redirect, err := p.GetAppRedirect(req)
843897 if err != nil {
844898 logger.Errorf("Error obtaining redirect: %v", err)
845899 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())
@@ -864,13 +918,13 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
864918 return
865919 }
866920 p.SetCSRFCookie(rw, req, nonce)
867− redirect, err := p.GetRedirect(req)
921+ redirect, err := p.GetAppRedirect(req)
868922 if err != nil {
869923 logger.Errorf("Error obtaining redirect: %v", err)
870924 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())
871925 return
872926 }
873− redirectURI := p.GetRedirectURI(util.GetRequestHost(req))
927+ redirectURI := p.GetOAuthRedirectURI(requestutil.GetRequestHost(req))
874928 http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), http.StatusFound)
875929}
876930
@@ -893,7 +947,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
893947 return
894948 }
895949
896− session, err := p.redeemCode(req.Context(), util.GetRequestHost(req), req.Form.Get("code"))
950+ session, err := p.redeemCode(req.Context(), requestutil.GetRequestHost(req), req.Form.Get("code"))
897951 if err != nil {
898952 logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)
899953 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error")
@@ -1024,7 +1078,7 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R
10241078 var session *sessionsapi.SessionState
10251079
10261080 getSession := p.sessionChain.Then(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
1027− session = middleware.GetRequestScope(req).Session
1081+ session = middlewareapi.GetRequestScope(req).Session
10281082 }))
10291083 getSession.ServeHTTP(rw, req)
10301084
oauthproxy_test.go+9−11
@@ -19,6 +19,7 @@ import (
1919
2020 "github.com/coreos/go-oidc"
2121 "github.com/mbland/hmacauth"
22+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
2223 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
2324 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
2425 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
@@ -1750,8 +1751,7 @@ func TestRequestSignature(t *testing.T) {
17501751
17511752func TestGetRedirect(t *testing.T) {
17521753 opts := baseTestOptions()
1753− opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com")
1754− opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com:8443")
1754+ opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com", ".example.com:8443")
17551755 err := validation.Validate(opts)
17561756 assert.NoError(t, err)
17571757 require.NotEmpty(t, opts.ProxyPrefix)
@@ -1854,9 +1854,6 @@ func TestGetRedirect(t *testing.T) {
18541854 url: "https://oauth.example.com/foo/bar",
18551855 headers: map[string]string{
18561856 "X-Auth-Request-Redirect": "https://a-service.example.com/foo/bar",
1857− "X-Forwarded-Proto": "",
1858− "X-Forwarded-Host": "",
1859− "X-Forwarded-Uri": "",
18601857 },
18611858 reverseProxy: true,
18621859 expectedRedirect: "https://a-service.example.com/foo/bar",
@@ -1884,10 +1881,9 @@ func TestGetRedirect(t *testing.T) {
18841881 name: "proxied request with rd query string and some headers set redirects to proxied URL on rd query string",
18851882 url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbaz",
18861883 headers: map[string]string{
1887− "X-Auth-Request-Redirect": "",
1888− "X-Forwarded-Proto": "https",
1889− "X-Forwarded-Host": "another-service.example.com",
1890− "X-Forwarded-Uri": "/seasons/greetings",
1884+ "X-Forwarded-Proto": "https",
1885+ "X-Forwarded-Host": "another-service.example.com",
1886+ "X-Forwarded-Uri": "/seasons/greetings",
18911887 },
18921888 reverseProxy: true,
18931889 expectedRedirect: "https://a-service.example.com/foo/baz",
@@ -1901,8 +1897,10 @@ func TestGetRedirect(t *testing.T) {
19011897 req.Header.Add(header, value)
19021898 }
19031899 }
1904− proxy.ReverseProxy = tt.reverseProxy
1905− redirect, err := proxy.GetRedirect(req)
1900+ req = middleware.AddRequestScope(req, &middleware.RequestScope{
1901+ ReverseProxy: tt.reverseProxy,
1902+ })
1903+ redirect, err := proxy.GetAppRedirect(req)
19061904
19071905 assert.NoError(t, err)
19081906 assert.Equal(t, tt.expectedRedirect, redirect)