73fc770 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

Figure out final app redirect URL with proxy aware request utils

73fc770Nick Meves6 years ago

oauthproxy.go+102−48

compare with master

unifiedsplit

@@ -24,9 +24,9 @@ import (

24 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"

25 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

26 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/middleware"

27+ requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"

27 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions"

28 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions"

28 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"

29 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/upstream"

29− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"

30 "github.com/oauth2-proxy/oauth2-proxy/v7/providers"

31)

@@ -98,7 +98,6 @@ type OAuthProxy struct {

98 SetAuthorization bool

99 PassAuthorization bool

100 PreferEmailToUser bool

101− ReverseProxy bool

102 skipAuthPreflight bool

101 skipAuthPreflight bool

103 skipJwtBearerTokens bool

102 skipJwtBearerTokens bool

104 templates *template.Template

103 templates *template.Template

@@ -201,7 +200,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

201 UserInfoPath: fmt.Sprintf("%s/userinfo", opts.ProxyPrefix),

200 UserInfoPath: fmt.Sprintf("%s/userinfo", opts.ProxyPrefix),

202

201

203 ProxyPrefix: opts.ProxyPrefix,

202 ProxyPrefix: opts.ProxyPrefix,

204− ReverseProxy: opts.ReverseProxy,

205 provider: opts.GetProvider(),

203 provider: opts.GetProvider(),

206 providerNameOverride: opts.ProviderName,

204 providerNameOverride: opts.ProviderName,

207 sessionStore: sessionStore,

205 sessionStore: sessionStore,

@@ -231,7 +229,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr

231// the OAuth2 Proxy authentication logic kicks in.

229// the OAuth2 Proxy authentication logic kicks in.

232// For example forcing HTTPS or health checks.

230// For example forcing HTTPS or health checks.

233func buildPreAuthChain(opts *options.Options) (alice.Chain, error) {

231func buildPreAuthChain(opts *options.Options) (alice.Chain, error) {

234− chain := alice.New(middleware.NewScope(opts))

232+ chain := alice.New(middleware.NewScope(opts.ReverseProxy))

235

233

236 if opts.ForceHTTPS {

234 if opts.ForceHTTPS {

237 _, httpsPort, err := net.SplitHostPort(opts.HTTPSAddress)

235 _, httpsPort, err := net.SplitHostPort(opts.HTTPSAddress)

@@ -368,9 +366,9 @@ func buildRoutesAllowlist(opts *options.Options) ([]allowedRoute, error) {

368 return routes, nil

366 return routes, nil

369}

367}

370

368

371−// GetRedirectURI returns the redirectURL that the upstream OAuth Provider will

369+// GetOAuthRedirectURI returns the redirectURL that the upstream OAuth Provider will

372// redirect clients to once authenticated

370// redirect clients to once authenticated

373−func (p *OAuthProxy) GetRedirectURI(host string) string {

371+func (p *OAuthProxy) GetOAuthRedirectURI(host string) string {

374 // default to the request Host if not set

372 // default to the request Host if not set

375 if p.redirectURL.Host != "" {

373 if p.redirectURL.Host != "" {

376 return p.redirectURL.String()

374 return p.redirectURL.String()

@@ -391,7 +389,7 @@ func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (*sessio

391 if code == "" {

389 if code == "" {

392 return nil, providers.ErrMissingCode

390 return nil, providers.ErrMissingCode

393 }

391 }

394− redirectURI := p.GetRedirectURI(host)

392+ redirectURI := p.GetOAuthRedirectURI(host)

395 s, err := p.provider.Redeem(ctx, redirectURI, code)

393 s, err := p.provider.Redeem(ctx, redirectURI, code)

396 if err != nil {

394 if err != nil {

397 return nil, err

395 return nil, err

@@ -420,7 +418,7 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex

420 cookieDomain := cookies.GetCookieDomain(req, p.CookieDomains)

418 cookieDomain := cookies.GetCookieDomain(req, p.CookieDomains)

421

419

422 if cookieDomain != "" {

420 if cookieDomain != "" {

423− domain := util.GetRequestHost(req)

421+ domain := requestutil.GetRequestHost(req)

424 if h, _, err := net.SplitHostPort(domain); err == nil {

422 if h, _, err := net.SplitHostPort(domain); err == nil {

425 domain = h

423 domain = h

426 }

424 }

@@ -509,7 +507,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code

509 }

507 }

510 rw.WriteHeader(code)

508 rw.WriteHeader(code)

511

509

512− redirectURL, err := p.GetRedirect(req)

510+ redirectURL, err := p.GetAppRedirect(req)

513 if err != nil {

511 if err != nil {

514 logger.Errorf("Error obtaining redirect: %v", err)

512 logger.Errorf("Error obtaining redirect: %v", err)

515 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

513 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

@@ -568,46 +566,108 @@ func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool) {

568 return "", false

566 return "", false

569}

567}

570

568

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

572// once authenticated with the OAuthProxy

570// once authenticated with the OAuthProxy

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+// - `/`

573−func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error) {

579+func (p *OAuthProxy) GetAppRedirect(req *http.Request) (string, error) {

574− err = req.ParseForm()

580+ err := req.ParseForm()

575 if err != nil {

581 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+ }

577 }

597 }

578

598

579− redirect = req.Header.Get("X-Auth-Request-Redirect")

599+ return "/", nil

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− }

600+}

590− if !p.IsValidRedirect(redirect) {

591− // Use RequestURI to preserve ?query

592− redirect = req.URL.RequestURI()

593

601

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 {

594− if strings.HasPrefix(redirect, fmt.Sprintf("%s/", p.ProxyPrefix)) {

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 {

595− redirect = "/"

614+ redirect := req.Form.Get("rd")

596− }

615+ if p.IsValidRedirect(redirect) {

616+ return redirect

597 }

617 }

618+ return ""

619+}

598

620

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+ }

599− return

628+ return ""

600}

629}

601

630

631+// getXForwardedHeadersRedirect handles these GetAppRedirect strategies:

602−// getRedirectFromForwardHeaders returns the redirect URL based on X-Forwarded-{Proto,Host,Uri} headers

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/*)

603−func (p *OAuthProxy) getRedirectFromForwardHeaders(req *http.Request) string {

634+func (p *OAuthProxy) getXForwardedHeadersRedirect(req *http.Request) string {

604− uri := util.GetRequestURI(req)

635+ if !isForwardedRequest(req) {

636+ return ""

637+ }

605

638

606− if strings.HasPrefix(uri, fmt.Sprintf("%s/", p.ProxyPrefix)) {

639+ uri := requestutil.GetRequestURI(req)

640+ if p.hasProxyPrefix(uri) {

607 uri = "/"

641 uri = "/"

608 }

642 }

609

643

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

611}

671}

612

672

613// splitHostPort separates host and port. If the port is not valid, it returns

673// 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 {

707 return false

767 return false

708}

768}

709

769

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−

716// See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching?hl=en

770// See https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching?hl=en

717var noCacheHeaders = map[string]string{

771var noCacheHeaders = map[string]string{

718 "Expires": time.Unix(0, 0).Format(time.RFC1123),

772 "Expires": time.Unix(0, 0).Format(time.RFC1123),

@@ -781,7 +835,7 @@ func (p *OAuthProxy) serveHTTP(rw http.ResponseWriter, req *http.Request) {

781

835

782// SignIn serves a page prompting users to sign in

836// SignIn serves a page prompting users to sign in

783func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

837func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

784− redirect, err := p.GetRedirect(req)

838+ redirect, err := p.GetAppRedirect(req)

785 if err != nil {

839 if err != nil {

786 logger.Errorf("Error obtaining redirect: %v", err)

840 logger.Errorf("Error obtaining redirect: %v", err)

787 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

841 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

@@ -839,7 +893,7 @@ func (p *OAuthProxy) UserInfo(rw http.ResponseWriter, req *http.Request) {

839

893

840// SignOut sends a response to clear the authentication cookie

894// SignOut sends a response to clear the authentication cookie

841func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

895func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

842− redirect, err := p.GetRedirect(req)

896+ redirect, err := p.GetAppRedirect(req)

843 if err != nil {

897 if err != nil {

844 logger.Errorf("Error obtaining redirect: %v", err)

898 logger.Errorf("Error obtaining redirect: %v", err)

845 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

899 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

@@ -864,13 +918,13 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

864 return

918 return

865 }

919 }

866 p.SetCSRFCookie(rw, req, nonce)

920 p.SetCSRFCookie(rw, req, nonce)

867− redirect, err := p.GetRedirect(req)

921+ redirect, err := p.GetAppRedirect(req)

868 if err != nil {

922 if err != nil {

869 logger.Errorf("Error obtaining redirect: %v", err)

923 logger.Errorf("Error obtaining redirect: %v", err)

870 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

924 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", err.Error())

871 return

925 return

872 }

926 }

873− redirectURI := p.GetRedirectURI(util.GetRequestHost(req))

927+ redirectURI := p.GetOAuthRedirectURI(requestutil.GetRequestHost(req))

874 http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), http.StatusFound)

928 http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), http.StatusFound)

875}

929}

876

930

@@ -893,7 +947,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

893 return

947 return

894 }

948 }

895

949

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"))

897 if err != nil {

951 if err != nil {

898 logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)

952 logger.Errorf("Error redeeming code during OAuth2 callback: %v", err)

899 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error")

953 p.ErrorPage(rw, http.StatusInternalServerError, "Internal Server Error", "Internal Error")

@@ -1024,7 +1078,7 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R

1024 var session *sessionsapi.SessionState

1078 var session *sessionsapi.SessionState

1025

1079

1026 getSession := p.sessionChain.Then(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {

1080 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

1028 }))

1082 }))

1029 getSession.ServeHTTP(rw, req)

1083 getSession.ServeHTTP(rw, req)

1030

1084

oauthproxy_test.go+9−11

@@ -19,6 +19,7 @@ import (

20 "github.com/coreos/go-oidc"

21 "github.com/mbland/hmacauth"

22+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"

22 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

23 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

23 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

24 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

24 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

25 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

@@ -1750,8 +1751,7 @@ func TestRequestSignature(t *testing.T) {

1750

1751

1751func TestGetRedirect(t *testing.T) {

1752func TestGetRedirect(t *testing.T) {

1752 opts := baseTestOptions()

1753 opts := baseTestOptions()

1753− opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com")

1754+ opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com", ".example.com:8443")

1754− opts.WhitelistDomains = append(opts.WhitelistDomains, ".example.com:8443")

1755 err := validation.Validate(opts)

1756 assert.NoError(t, err)

1757 require.NotEmpty(t, opts.ProxyPrefix)

@@ -1854,9 +1854,6 @@ func TestGetRedirect(t *testing.T) {

1854 url: "https://oauth.example.com/foo/bar",

1855 headers: map[string]string{

1856 "X-Auth-Request-Redirect": "https://a-service.example.com/foo/bar",

1857− "X-Forwarded-Proto": "",

1858− "X-Forwarded-Host": "",

1859− "X-Forwarded-Uri": "",

1860 },

1857 },

1861 reverseProxy: true,

1858 reverseProxy: true,

1862 expectedRedirect: "https://a-service.example.com/foo/bar",

1859 expectedRedirect: "https://a-service.example.com/foo/bar",

@@ -1884,10 +1881,9 @@ func TestGetRedirect(t *testing.T) {

1884 name: "proxied request with rd query string and some headers set redirects to proxied URL on rd query string",

1881 name: "proxied request with rd query string and some headers set redirects to proxied URL on rd query string",

1885 url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbaz",

1882 url: "https://oauth.example.com/foo/bar?rd=https%3A%2F%2Fa%2Dservice%2Eexample%2Ecom%2Ffoo%2Fbaz",

1886 headers: map[string]string{

1883 headers: map[string]string{

1887− "X-Auth-Request-Redirect": "",

1884+ "X-Forwarded-Proto": "https",

1888− "X-Forwarded-Proto": "https",

1889− "X-Forwarded-Host": "another-service.example.com",

1885+ "X-Forwarded-Host": "another-service.example.com",

1890− "X-Forwarded-Uri": "/seasons/greetings",

1886+ "X-Forwarded-Uri": "/seasons/greetings",

1891 },

1887 },

1892 reverseProxy: true,

1888 reverseProxy: true,

1893 expectedRedirect: "https://a-service.example.com/foo/baz",

1889 expectedRedirect: "https://a-service.example.com/foo/baz",

@@ -1901,8 +1897,10 @@ func TestGetRedirect(t *testing.T) {

1901 req.Header.Add(header, value)

1897 req.Header.Add(header, value)

1902 }

1898 }

1903 }

1899 }

1900+ req = middleware.AddRequestScope(req, &middleware.RequestScope{

1904− proxy.ReverseProxy = tt.reverseProxy

1901+ ReverseProxy: tt.reverseProxy,

1902+ })

1905− redirect, err := proxy.GetRedirect(req)

1903+ redirect, err := proxy.GetAppRedirect(req)

1906

1904

1907 assert.NoError(t, err)

1905 assert.NoError(t, err)

1908 assert.Equal(t, tt.expectedRedirect, redirect)

1906 assert.Equal(t, tt.expectedRedirect, redirect)