29b2479 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Use X-Forwarded-Host consistently

commit29b24793e3df4c157b3e31c1d9a101b471295152 authorNick Meves <nick.meves@greenhouse.io> dateFri 21 Aug 2020, 19:50 (6 years ago) parentbd5fab4

8 files changed+61−22

compare with master

unifiedsplit

CHANGELOG.md+2 oauthproxy.go+4−3 pkg/cookies/cookies.go+4−12 pkg/logger/logger.go+4−2 pkg/middleware/redirect_to_https.go+4−4 pkg/middleware/redirect_to_https_test.go+11 pkg/util/util.go+10 pkg/util/util_test.go+22−1

CHANGELOG.md+2

@@ -8,6 +8,8 @@

99## Changes since v6.1.0

1010

11+- [#729](https://github.com/oauth2-proxy/oauth2-proxy/pull/729) Use X-Forwarded-Host consistently when set (@NickMeves)

12+

1113# v6.1.0

1214

1315## Release Highlights

oauthproxy.go+4−3

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

2828 "github.com/oauth2-proxy/oauth2-proxy/pkg/middleware"

2929 "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions"

3030 "github.com/oauth2-proxy/oauth2-proxy/pkg/upstream"

31+ "github.com/oauth2-proxy/oauth2-proxy/pkg/util"

3132 "github.com/oauth2-proxy/oauth2-proxy/providers"

3233)

3334

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

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

333334

334335 if cookieDomain != "" {

335− domain := cookies.GetRequestHost(req)

336+ domain := util.GetRequestHost(req)

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

337338 domain = h

338339 }

@@ -747,7 +748,7 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

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

748749 return

749750 }

750− redirectURI := p.GetRedirectURI(req.Host)

751+ redirectURI := p.GetRedirectURI(util.GetRequestHost(req))

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

752753}

753754

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

770771 return

771772 }

772773

773− session, err := p.redeemCode(req.Context(), req.Host, req.Form.Get("code"))

774+ session, err := p.redeemCode(req.Context(), util.GetRequestHost(req), req.Form.Get("code"))

774775 if err != nil {

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

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

pkg/cookies/cookies.go+4−12

@@ -9,13 +9,14 @@ import (

1010 "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"

1111 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"

12+ "github.com/oauth2-proxy/oauth2-proxy/pkg/util"

1213)

1314

1415// MakeCookie constructs a cookie from the given parameters,

1516// discovering the domain from the request if not specified.

1617func MakeCookie(req *http.Request, name string, value string, path string, domain string, httpOnly bool, secure bool, expiration time.Duration, now time.Time, sameSite http.SameSite) *http.Cookie {

1718 if domain != "" {

18− host := req.Host

19+ host := util.GetRequestHost(req)

1920 if h, _, err := net.SplitHostPort(host); err == nil {

2021 host = h

2122 }

@@ -47,7 +48,7 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, cookieO

4748 // If nothing matches, create the cookie with the shortest domain

4849 defaultDomain := ""

4950 if len(cookieOpts.Domains) > 0 {

50− logger.Errorf("Warning: request host %q did not match any of the specific cookie domains of %q", GetRequestHost(req), strings.Join(cookieOpts.Domains, ","))

51+ logger.Errorf("Warning: request host %q did not match any of the specific cookie domains of %q", util.GetRequestHost(req), strings.Join(cookieOpts.Domains, ","))

5152 defaultDomain = cookieOpts.Domains[len(cookieOpts.Domains)-1]

5253 }

5354 return MakeCookie(req, name, value, cookieOpts.Path, defaultDomain, cookieOpts.HTTPOnly, cookieOpts.Secure, expiration, now, ParseSameSite(cookieOpts.SameSite))

@@ -56,7 +57,7 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, cookieO

5657// GetCookieDomain returns the correct cookie domain given a list of domains

5758// by checking the X-Fowarded-Host and host header of an an http request

5859func GetCookieDomain(req *http.Request, cookieDomains []string) string {

59− host := GetRequestHost(req)

60+ host := util.GetRequestHost(req)

6061 for _, domain := range cookieDomains {

6162 if strings.HasSuffix(host, domain) {

6263 return domain

@@ -65,15 +66,6 @@ func GetCookieDomain(req *http.Request, cookieDomains []string) string {

6566 return ""

6667}

6768

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

69−func GetRequestHost(req *http.Request) string {

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

71− if host == "" {

72− host = req.Host

73− }

74− return host

75−}

76−

7769// Parse a valid http.SameSite value from a user supplied string for use of making cookies.

7870func ParseSameSite(v string) http.SameSite {

7971 switch v {

pkg/logger/logger.go+4−2

@@ -11,6 +11,8 @@ import (

1111 "sync"

1212 "text/template"

1313 "time"

14+

15+ "github.com/oauth2-proxy/oauth2-proxy/pkg/util"

1416)

1517

1618// AuthStatus defines the different types of auth logging that occur

@@ -195,7 +197,7 @@ func (l *Logger) PrintAuthf(username string, req *http.Request, status AuthStatu

195197

196198 err := l.authTemplate.Execute(l.writer, authLogMessageData{

197199 Client: client,

198− Host: req.Host,

200+ Host: util.GetRequestHost(req),

199201 Protocol: req.Proto,

200202 RequestMethod: req.Method,

201203 Timestamp: FormatTimestamp(now),

@@ -249,7 +251,7 @@ func (l *Logger) PrintReq(username, upstream string, req *http.Request, url url.

249251

250252 err := l.reqTemplate.Execute(l.writer, reqLogMessageData{

251253 Client: client,

252− Host: req.Host,

254+ Host: util.GetRequestHost(req),

253255 Protocol: req.Proto,

254256 RequestDuration: fmt.Sprintf("%0.3f", duration),

255257 RequestMethod: req.Method,

pkg/middleware/redirect_to_https.go+4−4

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

77 "strings"

99 "github.com/justinas/alice"

10+ "github.com/oauth2-proxy/oauth2-proxy/pkg/util"

1011)

1112

1213const httpsScheme = "https"

@@ -38,10 +39,9 @@ func redirectToHTTPS(httpsPort string, next http.Handler) http.Handler {

3839 // Set the scheme to HTTPS

3940 targetURL.Scheme = httpsScheme

4041

41− // Set the req.Host when the targetURL still does not have one

42− if targetURL.Host == "" {

43− targetURL.Host = req.Host

44− }

42+ // Set the Host in case the targetURL still does not have one

43+ // or it isn't X-Forwarded-Host aware

44+ targetURL.Host = util.GetRequestHost(req)

4545

4646 // Overwrite the port if the original request was to a non-standard port

4747 if targetURL.Port() != "" {

pkg/middleware/redirect_to_https_test.go+11

@@ -164,5 +164,16 @@ var _ = Describe("RedirectToHTTPS suite", func() {

164164 expectedBody: permanentRedirectBody("https://example.com/"),

165165 expectedLocation: "https://example.com/",

166166 }),

167+ Entry("without TLS with an X-Forwarded-Host header", &requestTableInput{

168+ requestString: "http://internal.example.com",

169+ useTLS: false,

170+ headers: map[string]string{

171+ "X-Forwarded-Proto": "HTTP",

172+ "X-Forwarded-Host": "external.example.com",

173+ },

174+ expectedStatus: 308,

175+ expectedBody: permanentRedirectBody("https://external.example.com"),

176+ expectedLocation: "https://external.example.com",

177+ }),

167178 )

168179})

pkg/util/util.go+10

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

44 "crypto/x509"

55 "fmt"

66 "io/ioutil"

7+ "net/http"

78)

910func GetCertPool(paths []string) (*x509.CertPool, error) {

@@ -23,3 +24,12 @@ func GetCertPool(paths []string) (*x509.CertPool, error) {

2324 }

2425 return pool, nil

2526}

27+

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

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

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

31+ if host == "" {

32+ host = req.Host

33+ }

34+ return host

35+}

pkg/util/util_test.go+22−1

@@ -4,9 +4,11 @@ import (

44 "crypto/x509/pkix"

55 "encoding/asn1"

66 "io/ioutil"

7+ "net/http/httptest"

78 "os"

89 "testing"

910

11+ . "github.com/onsi/gomega"

1012 "github.com/stretchr/testify/assert"

1113)

1214

@@ -70,7 +72,13 @@ func TestGetCertPool_NoRoots(t *testing.T) {

7072func TestGetCertPool(t *testing.T) {

7173 tempDir, err := ioutil.TempDir("", "certtest")

7274 assert.NoError(t, err)

73− defer os.RemoveAll(tempDir)

75+ defer func(path string) {

76+ rerr := os.RemoveAll(path)

77+ if rerr != nil {

78+ panic(rerr)

79+ }

80+ }(tempDir)

81+

7482 certFile1 := makeTestCertFile(t, testCA1, tempDir)

7583 certFile2 := makeTestCertFile(t, testCA2, tempDir)

7684

@@ -89,3 +97,16 @@ func TestGetCertPool(t *testing.T) {

8997 expectedSubjects := []string{testCA1Subj, testCA2Subj}

9098 assert.Equal(t, expectedSubjects, got)

9199}

100+

101+func TestGetRequestHost(t *testing.T) {

102+ g := NewWithT(t)

103+

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

105+ host := GetRequestHost(req)

106+ g.Expect(host).To(Equal("example.com"))

107+

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

109+ proxyReq.Header.Add("X-Forwarded-Host", "external.example.com")

110+ extHost := GetRequestHost(proxyReq)

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

112+}