1d74a51 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Use X-Forwarded-{Proto,Host,Uri} on redirect as last resort (#957)

1d74a51İlteriş Eroğlu6 years ago

CHANGELOG.md+2

@@ -6,6 +6,7 @@

7- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Keycloak will now use `--profile-url` if set for the userinfo endpoint

8 instead of `--validate-url`. `--validate-url` will still work for backwards compatibility.

9+- [#957](https://github.com/oauth2-proxy/oauth2-proxy/pull/957) To use X-Forwarded-{Proto,Host,Uri} on redirect detection, `--reverse-proxy` must be `true`.

9- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) `--user-id-claim` option is deprecated and replaced by `--oidc-email-claim`

10- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) `--user-id-claim` option is deprecated and replaced by `--oidc-email-claim`

10- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Gitlab projects needs a Gitlab application with the extra `read_api` enabled

11- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Gitlab projects needs a Gitlab application with the extra `read_api` enabled

11- [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) `/oauth2/auth` `allowed_groups` querystring parameter can be paired with the `allowed-groups` configuration option.

12- [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) `/oauth2/auth` `allowed_groups` querystring parameter can be paired with the `allowed-groups` configuration option.

@@ -59,6 +60,7 @@

59## Changes since v6.1.1

60## Changes since v6.1.1

61- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Migrate Keycloak to EnrichSession & support multiple groups for authorization (@NickMeves)

62- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Migrate Keycloak to EnrichSession & support multiple groups for authorization (@NickMeves)

63+- [#957](https://github.com/oauth2-proxy/oauth2-proxy/pull/957) Use X-Forwarded-{Proto,Host,Uri} on redirect as last resort (@linuxgemini)

62- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh)

64- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh)

63- [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed)

65- [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed)

64- [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves)

66- [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves)

docs/docs/configuration/overview.md+68−1

@@ -106,7 +106,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/

109+| `--reverse-proxy` | bool | are we running behind a reverse proxy, controls whether headers like X-Real-IP are accepted and allows X-Forwarded-{Proto,Host,Uri} headers to be used on redirect selection | false |

@@ -354,6 +354,73 @@ It is recommended to use `--session-store-type=redis` when expecting large sessi

354

355You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=".

356

357+## Configuring for use with the Traefik (v2) `ForwardAuth` middleware

358+

359+**This option requires `--reverse-proxy` option to be set.**

360+

361+The [Traefik v2 `ForwardAuth` middleware](https://doc.traefik.io/traefik/middlewares/forwardauth/) allows Traefik to authenticate requests via the oauth2-proxy's `/oauth2/auth` endpoint on every request, which only returns a 202 Accepted response or a 401 Unauthorized response without proxying the whole request through. For example, on Dynamic File (YAML) Configuration:

362+

363+```yaml

364+http:

365+ routers:

366+ a-service:

367+ rule: "Host(`a-service.example.com`)"

368+ service: a-service-backend

369+ middlewares:

370+ - oauth-errors

371+ - oauth-auth

372+ tls:

373+ certResolver: default

374+ domains:

375+ - main: "example.com"

376+ sans:

377+ - "*.example.com"

378+ oauth:

379+ rule: "Host(`a-service.example.com`, `oauth.example.com`) && PathPrefix(`/oauth2/`)"

380+ middlewares:

381+ - auth-headers

382+ service: oauth-backend

383+ tls:

384+ certResolver: default

385+ domains:

386+ - main: "example.com"

387+ sans:

388+ - "*.example.com"

389+

390+ services:

391+ a-service-backend:

392+ loadBalancer:

393+ servers:

394+ - url: http://172.16.0.2:7555

395+ oauth-backend:

396+ loadBalancer:

397+ servers:

398+ - url: http://172.16.0.1:4180

399+

400+ middlewares:

401+ auth-headers:

402+ headers:

403+ sslRedirect: true

404+ stsSeconds: 315360000

405+ browserXssFilter: true

406+ contentTypeNosniff: true

407+ forceSTSHeader: true

408+ sslHost: example.com

409+ stsIncludeSubdomains: true

410+ stsPreload: true

411+ frameDeny: true

412+ oauth-auth:

413+ forwardAuth:

414+ address: https://oauth.example.com/oauth2/auth

415+ trustForwardHeader: true

416+ oauth-errors:

417+ errors:

418+ status:

419+ - "401-403"

420+ service: oauth-backend

421+ query: "/oauth2/sign_in"

422+```

423+

357:::note

424:::note

358If you set up your OAuth2 provider to rotate your client secret, you can use the `client-secret-file` option to reload the secret when it is updated.

425If you set up your OAuth2 provider to rotate your client secret, you can use the `client-secret-file` option to reload the secret when it is updated.

359:::

426:::

oauthproxy.go+28−1

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

98 SetAuthorization bool

99 PassAuthorization bool

100 PreferEmailToUser bool

101+ ReverseProxy bool

101 skipAuthPreflight bool

102 skipAuthPreflight bool

102 skipJwtBearerTokens bool

103 skipJwtBearerTokens bool

103 templates *template.Template

104 templates *template.Template

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

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

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

201

202

202 ProxyPrefix: opts.ProxyPrefix,

203 ProxyPrefix: opts.ProxyPrefix,

204+ ReverseProxy: opts.ReverseProxy,

203 provider: opts.GetProvider(),

205 provider: opts.GetProvider(),

204 providerNameOverride: opts.ProviderName,

206 providerNameOverride: opts.ProviderName,

205 sessionStore: sessionStore,

207 sessionStore: sessionStore,

@@ -578,10 +580,18 @@ func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error)

578 if req.Form.Get("rd") != "" {

580 if req.Form.Get("rd") != "" {

579 redirect = req.Form.Get("rd")

581 redirect = req.Form.Get("rd")

580 }

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

581 if !p.IsValidRedirect(redirect) {

590 if !p.IsValidRedirect(redirect) {

582 // Use RequestURI to preserve ?query

591 // Use RequestURI to preserve ?query

583 redirect = req.URL.RequestURI()

592 redirect = req.URL.RequestURI()

593+

584− if strings.HasPrefix(redirect, p.ProxyPrefix) {

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

585 redirect = "/"

595 redirect = "/"

586 }

596 }

587 }

597 }

@@ -589,6 +599,17 @@ func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error)

589 return

599 return

590}

600}

591

601

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)

605+

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

607+ uri = "/"

608+ }

609+

610+ return fmt.Sprintf("%s://%s%s", util.GetRequestProto(req), util.GetRequestHost(req), uri)

611+}

612+

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

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

593// the entire input as host, and it doesn't check the validity of the host.

614// the entire input as host, and it doesn't check the validity of the host.

594// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.

615// Unlike net.SplitHostPort, but per RFC 3986, it requires ports to be numeric.

@@ -686,6 +707,12 @@ func (p *OAuthProxy) isAllowedRoute(req *http.Request) bool {

686 return false

707 return false

687}

708}

688

709

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+

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

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

690var noCacheHeaders = map[string]string{

717var noCacheHeaders = map[string]string{

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

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

oauthproxy_test.go+120

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

1750

1751func TestGetRedirect(t *testing.T) {

1752 opts := baseTestOptions()

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

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

1753 err := validation.Validate(opts)

1755 err := validation.Validate(opts)

1754 assert.NoError(t, err)

1756 assert.NoError(t, err)

1755 require.NotEmpty(t, opts.ProxyPrefix)

1757 require.NotEmpty(t, opts.ProxyPrefix)

@@ -1761,27 +1763,145 @@ func TestGetRedirect(t *testing.T) {

1761 tests := []struct {

1763 tests := []struct {

1762 name string

1764 name string

1763 url string

1765 url string

1766+ headers map[string]string

1767+ reverseProxy bool

1764 expectedRedirect string

1768 expectedRedirect string

1765 }{

1769 }{

1766 {

1770 {

1767 name: "request outside of ProxyPrefix redirects to original URL",

1771 name: "request outside of ProxyPrefix redirects to original URL",

1768 url: "/foo/bar",

1772 url: "/foo/bar",

1773+ headers: nil,

1774+ reverseProxy: false,

1769 expectedRedirect: "/foo/bar",

1775 expectedRedirect: "/foo/bar",

1770 },

1776 },

1771 {

1777 {

1772 name: "request with query preserves query",

1778 name: "request with query preserves query",

1773 url: "/foo?bar",

1779 url: "/foo?bar",

1780+ headers: nil,

1781+ reverseProxy: false,

1774 expectedRedirect: "/foo?bar",

1782 expectedRedirect: "/foo?bar",

1775 },

1783 },

1776 {

1784 {

1777 name: "request under ProxyPrefix redirects to root",

1785 name: "request under ProxyPrefix redirects to root",

1778 url: proxy.ProxyPrefix + "/foo/bar",

1786 url: proxy.ProxyPrefix + "/foo/bar",

1787+ headers: nil,

1788+ reverseProxy: false,

1779 expectedRedirect: "/",

1789 expectedRedirect: "/",

1780 },

1790 },

1791+ {

1792+ name: "proxied request outside of ProxyPrefix redirects to proxied URL",

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

1794+ headers: map[string]string{

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

1796+ "X-Forwarded-Host": "a-service.example.com",

1797+ "X-Forwarded-Uri": "/foo/bar",

1798+ },

1799+ reverseProxy: true,

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

1801+ },

1802+ {

1803+ name: "non-proxied request with spoofed proxy headers wouldn't redirect",

1804+ url: "https://oauth.example.com/foo?bar",

1805+ headers: map[string]string{

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

1807+ "X-Forwarded-Host": "a-service.example.com",

1808+ "X-Forwarded-Uri": "/foo/bar",

1809+ },

1810+ reverseProxy: false,

1811+ expectedRedirect: "/foo?bar",

1812+ },

1813+ {

1814+ name: "proxied request under ProxyPrefix redirects to root",

1815+ url: "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",

1816+ headers: map[string]string{

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

1818+ "X-Forwarded-Host": "a-service.example.com",

1819+ "X-Forwarded-Uri": proxy.ProxyPrefix + "/foo/bar",

1820+ },

1821+ reverseProxy: true,

1822+ expectedRedirect: "https://a-service.example.com/",

1823+ },

1824+ {

1825+ name: "proxied request with port under ProxyPrefix redirects to root",

1826+ url: "https://oauth.example.com" + proxy.ProxyPrefix + "/foo/bar",

1827+ headers: map[string]string{

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

1829+ "X-Forwarded-Host": "a-service.example.com:8443",

1830+ "X-Forwarded-Uri": proxy.ProxyPrefix + "/foo/bar",

1831+ },

1832+ reverseProxy: true,

1833+ expectedRedirect: "https://a-service.example.com:8443/",

1834+ },

1835+ {

1836+ name: "proxied request with missing uri header would still redirect to desired redirect",

1837+ url: "https://oauth.example.com/foo?bar",

1838+ headers: map[string]string{

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

1840+ "X-Forwarded-Host": "a-service.example.com",

1841+ },

1842+ reverseProxy: true,

1843+ expectedRedirect: "https://a-service.example.com/foo?bar",

1844+ },

1845+ {

1846+ name: "request with headers proxy not being set (and reverse proxy enabled) would still redirect to desired redirect",

1847+ url: "https://oauth.example.com/foo?bar",

1848+ headers: nil,

1849+ reverseProxy: true,

1850+ expectedRedirect: "/foo?bar",

1851+ },

1852+ {

1853+ name: "proxied request with X-Auth-Request-Redirect being set outside of ProxyPrefix redirects to proxied URL",

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

1861+ reverseProxy: true,

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

1863+ },

1864+ {

1865+ name: "proxied request with rd query string redirects to proxied URL",

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

1867+ headers: nil,

1868+ reverseProxy: false,

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

1870+ },

1871+ {

1872+ name: "proxied request with rd query string and all headers set (and reverse proxy not enabled) redirects to proxied URL on rd query string",

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

1874+ headers: map[string]string{

1875+ "X-Auth-Request-Redirect": "https://a-service.example.com/foo/baz",

1876+ "X-Forwarded-Proto": "http",

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

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

1879+ },

1880+ reverseProxy: false,

1881+ expectedRedirect: "https://a-service.example.com/foo/jazz",

1882+ },

1883+ {

1884+ 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",

1886+ 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",

1891+ },

1892+ reverseProxy: true,

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

1894+ },

1781 }

1895 }

1782 for _, tt := range tests {

1896 for _, tt := range tests {

1783 t.Run(tt.name, func(t *testing.T) {

1897 t.Run(tt.name, func(t *testing.T) {

1784 req, _ := http.NewRequest("GET", tt.url, nil)

1898 req, _ := http.NewRequest("GET", tt.url, nil)

1899+ for header, value := range tt.headers {

1900+ if value != "" {

1901+ req.Header.Add(header, value)

1902+ }

1903+ }

1904+ proxy.ReverseProxy = tt.reverseProxy

1785 redirect, err := proxy.GetRedirect(req)

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

1786

1906

1787 assert.NoError(t, err)

1907 assert.NoError(t, err)

pkg/util/util.go+19

@@ -25,6 +25,15 @@ func GetCertPool(paths []string) (*x509.CertPool, error) {

25 return pool, nil

26}

28+// GetRequestProto return the request host header or X-Forwarded-Proto if present

29+func GetRequestProto(req *http.Request) string {

30+ proto := req.Header.Get("X-Forwarded-Proto")

31+ if proto == "" {

32+ proto = req.URL.Scheme

33+ }

34+ return proto

35+}

36+

28// GetRequestHost return the request host header or X-Forwarded-Host if present

37// GetRequestHost return the request host header or X-Forwarded-Host if present

29func GetRequestHost(req *http.Request) string {

38func GetRequestHost(req *http.Request) string {

30 host := req.Header.Get("X-Forwarded-Host")

39 host := req.Header.Get("X-Forwarded-Host")

@@ -33,3 +42,13 @@ func GetRequestHost(req *http.Request) string {

33 }

42 }

34 return host

43 return host

35}

44}

45+

46+// GetRequestURI return the request host header or X-Forwarded-Uri if present

47+func GetRequestURI(req *http.Request) string {

48+ uri := req.Header.Get("X-Forwarded-Uri")

49+ if uri == "" {

50+ // Use RequestURI to preserve ?query

51+ uri = req.URL.RequestURI()

52+ }

53+ return uri

54+}

pkg/util/util_test.go+26

@@ -110,3 +110,29 @@ func TestGetRequestHost(t *testing.T) {

110 extHost := GetRequestHost(proxyReq)

111 g.Expect(extHost).To(Equal("external.example.com"))

112}

113+

114+func TestGetRequestProto(t *testing.T) {

115+ g := NewWithT(t)

116+

117+ req := httptest.NewRequest("GET", "https://example.com", nil)

118+ proto := GetRequestProto(req)

119+ g.Expect(proto).To(Equal("https"))

120+

121+ proxyReq := httptest.NewRequest("GET", "https://internal.example.com", nil)

122+ proxyReq.Header.Add("X-Forwarded-Proto", "http")

123+ extProto := GetRequestProto(proxyReq)

124+ g.Expect(extProto).To(Equal("http"))

125+}

126+

127+func TestGetRequestURI(t *testing.T) {

128+ g := NewWithT(t)

129+

130+ req := httptest.NewRequest("GET", "https://example.com/ping", nil)

131+ uri := GetRequestURI(req)

132+ g.Expect(uri).To(Equal("/ping"))

133+

134+ proxyReq := httptest.NewRequest("GET", "http://internal.example.com/bong", nil)

135+ proxyReq.Header.Add("X-Forwarded-Uri", "/ping")

136+ extURI := GetRequestURI(proxyReq)

137+ g.Expect(extURI).To(Equal("/ping"))

138+}