git.iko.soy/typeable/oauth2-proxy
on master help sign in
Refactor organization of scope aware request utils

Reorganized the structure of the Request Utils due to their widespread use resulting in circular imports issues (mostly because of middleware & logger).

commit6fb3274ca3649d8e5b263e0742879002608f1455 authorNick Meves <nick.meves@greenhouse.io> dateSat 2 Jan 2021, 13:16 (6 years ago) parentb625de9
20 files changed+357−185
pkg/apis/middleware/middleware_suite_test.go+19 pkg/apis/middleware/scope.go+25 pkg/apis/middleware/scope_test.go+56 pkg/cookies/cookies.go+4−4 pkg/logger/logger.go+3−3 pkg/middleware/basic_session.go+2−1 pkg/middleware/basic_session_test.go+2−4 pkg/middleware/headers.go+3−2 pkg/middleware/headers_test.go+2−5 pkg/middleware/jwt_session.go+1−1 pkg/middleware/jwt_session_test.go+2−3 pkg/middleware/scope.go+4−23 pkg/middleware/scope_test.go+32−57 pkg/middleware/stored_session.go+2−1 pkg/middleware/stored_session_test.go+2−3 pkg/requests/util/util.go+48 pkg/requests/util/util_suite_test.go+19 pkg/requests/util/util_test.go+131 pkg/util/util.go−37 pkg/util/util_test.go−41
pkg/apis/middleware/middleware_suite_test.gonew file+19
@@ -0,0 +1,19 @@
1+package middleware_test
2+
3+import (
4+ "testing"
5+
6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
7+ . "github.com/onsi/ginkgo"
8+ . "github.com/onsi/gomega"
9+)
10+
11+// TestMiddlewareSuite and related tests are in a *_test package
12+// to prevent circular imports with the `logger` package which uses
13+// this functionality
14+func TestMiddlewareSuite(t *testing.T) {
15+ logger.SetOutput(GinkgoWriter)
16+
17+ RegisterFailHandler(Fail)
18+ RunSpecs(t, "Middleware API")
19+}
pkg/apis/middleware/scope.go+25
@@ -1,9 +1,18 @@
11package middleware
22
33import (
4+ "context"
5+ "net/http"
6+
47 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
58)
69
10+type scopeKey string
11+
12+// RequestScopeKey uses a typed string to reduce likelihood of clashing
13+// with other context keys
14+const RequestScopeKey scopeKey = "request-scope"
15+
716// RequestScope contains information regarding the request that is being made.
817// The RequestScope is used to pass information between different middlewares
918// within the chain.
@@ -26,3 +35,19 @@ type RequestScope struct {
2635 // it was loaded or not.
2736 SessionRevalidated bool
2837}
38+
39+// GetRequestScope returns the current request scope from the given request
40+func GetRequestScope(req *http.Request) *RequestScope {
41+ scope := req.Context().Value(RequestScopeKey)
42+ if scope == nil {
43+ return nil
44+ }
45+
46+ return scope.(*RequestScope)
47+}
48+
49+// AddRequestScope adds a RequestScope to a request
50+func AddRequestScope(req *http.Request, scope *RequestScope) *http.Request {
51+ ctx := context.WithValue(req.Context(), RequestScopeKey, scope)
52+ return req.WithContext(ctx)
53+}
pkg/apis/middleware/scope_test.gonew file+56
@@ -0,0 +1,56 @@
1+package middleware_test
2+
3+import (
4+ "net/http"
5+
6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
7+ . "github.com/onsi/ginkgo"
8+ . "github.com/onsi/gomega"
9+)
10+
11+var _ = Describe("Scope Suite", func() {
12+ Context("GetRequestScope", func() {
13+ var request *http.Request
14+
15+ BeforeEach(func() {
16+ var err error
17+ request, err = http.NewRequest("", "http://127.0.0.1/", nil)
18+ Expect(err).ToNot(HaveOccurred())
19+ })
20+
21+ Context("with a scope", func() {
22+ var scope *middleware.RequestScope
23+
24+ BeforeEach(func() {
25+ scope = &middleware.RequestScope{}
26+ request = middleware.AddRequestScope(request, scope)
27+ })
28+
29+ It("returns the scope", func() {
30+ s := middleware.GetRequestScope(request)
31+ Expect(s).ToNot(BeNil())
32+ Expect(s).To(Equal(scope))
33+ })
34+
35+ Context("if the scope is then modified", func() {
36+ BeforeEach(func() {
37+ Expect(scope.SaveSession).To(BeFalse())
38+ scope.SaveSession = true
39+ })
40+
41+ It("returns the updated session", func() {
42+ s := middleware.GetRequestScope(request)
43+ Expect(s).ToNot(BeNil())
44+ Expect(s).To(Equal(scope))
45+ Expect(s.SaveSession).To(BeTrue())
46+ })
47+ })
48+ })
49+
50+ Context("without a scope", func() {
51+ It("returns nil", func() {
52+ Expect(middleware.GetRequestScope(request)).To(BeNil())
53+ })
54+ })
55+ })
56+})
pkg/cookies/cookies.go+4−4
@@ -9,14 +9,14 @@ import (
99
1010 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
1111 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
12− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"
12+ requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"
1313)
1414
1515// MakeCookie constructs a cookie from the given parameters,
1616// discovering the domain from the request if not specified.
1717func 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 {
1818 if domain != "" {
19− host := util.GetRequestHost(req)
19+ host := requestutil.GetRequestHost(req)
2020 if h, _, err := net.SplitHostPort(host); err == nil {
2121 host = h
2222 }
@@ -48,7 +48,7 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, cookieO
4848 // If nothing matches, create the cookie with the shortest domain
4949 defaultDomain := ""
5050 if len(cookieOpts.Domains) > 0 {
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, ","))
51+ logger.Errorf("Warning: request host %q did not match any of the specific cookie domains of %q", requestutil.GetRequestHost(req), strings.Join(cookieOpts.Domains, ","))
5252 defaultDomain = cookieOpts.Domains[len(cookieOpts.Domains)-1]
5353 }
5454 return MakeCookie(req, name, value, cookieOpts.Path, defaultDomain, cookieOpts.HTTPOnly, cookieOpts.Secure, expiration, now, ParseSameSite(cookieOpts.SameSite))
@@ -57,7 +57,7 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, cookieO
5757// GetCookieDomain returns the correct cookie domain given a list of domains
5858// by checking the X-Fowarded-Host and host header of an an http request
5959func GetCookieDomain(req *http.Request, cookieDomains []string) string {
60− host := util.GetRequestHost(req)
60+ host := requestutil.GetRequestHost(req)
6161 for _, domain := range cookieDomains {
6262 if strings.HasSuffix(host, domain) {
6363 return domain
pkg/logger/logger.go+3−3
@@ -12,7 +12,7 @@ import (
1212 "text/template"
1313 "time"
1414
15− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"
15+ requestutil "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"
1616)
1717
1818// AuthStatus defines the different types of auth logging that occur
@@ -197,7 +197,7 @@ func (l *Logger) PrintAuthf(username string, req *http.Request, status AuthStatu
197197
198198 err := l.authTemplate.Execute(l.writer, authLogMessageData{
199199 Client: client,
200− Host: util.GetRequestHost(req),
200+ Host: requestutil.GetRequestHost(req),
201201 Protocol: req.Proto,
202202 RequestMethod: req.Method,
203203 Timestamp: FormatTimestamp(now),
@@ -251,7 +251,7 @@ func (l *Logger) PrintReq(username, upstream string, req *http.Request, url url.
251251
252252 err := l.reqTemplate.Execute(l.writer, reqLogMessageData{
253253 Client: client,
254− Host: util.GetRequestHost(req),
254+ Host: requestutil.GetRequestHost(req),
255255 Protocol: req.Proto,
256256 RequestDuration: fmt.Sprintf("%0.3f", duration),
257257 RequestMethod: req.Method,
pkg/middleware/basic_session.go+2−1
@@ -5,6 +5,7 @@ import (
55 "net/http"
66
77 "github.com/justinas/alice"
8+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
89 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
910 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"
1011 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
@@ -23,7 +24,7 @@ func NewBasicAuthSessionLoader(validator basic.Validator) alice.Constructor {
2324// If a session was loaded by a previous handler, it will not be replaced.
2425func loadBasicAuthSession(validator basic.Validator, next http.Handler) http.Handler {
2526 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
26− scope := GetRequestScope(req)
27+ scope := middlewareapi.GetRequestScope(req)
2728 // If scope is nil, this will panic.
2829 // A scope should always be injected before this handler is called.
2930 if scope.Session != nil {
pkg/middleware/basic_session_test.go+2−4
@@ -1,7 +1,6 @@
11package middleware
22
33import (
4− "context"
54 "fmt"
65 "net/http"
76 "net/http/httptest"
@@ -40,8 +39,7 @@ var _ = Describe("Basic Auth Session Suite", func() {
4039 // Set up the request with the authorization header and a request scope
4140 req := httptest.NewRequest("", "/", nil)
4241 req.Header.Set("Authorization", in.authorizationHeader)
43− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
44− req = req.WithContext(contextWithScope)
42+ req = middlewareapi.AddRequestScope(req, scope)
4543
4644 rw := httptest.NewRecorder()
4745
@@ -57,7 +55,7 @@ var _ = Describe("Basic Auth Session Suite", func() {
5755 // from the scope
5856 var gotSession *sessionsapi.SessionState
5957 handler := NewBasicAuthSessionLoader(validator)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
60− gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
58+ gotSession = middlewareapi.GetRequestScope(r).Session
6159 }))
6260 handler.ServeHTTP(rw, req)
6361
pkg/middleware/headers.go+3−2
@@ -5,6 +5,7 @@ import (
55 "net/http"
66
77 "github.com/justinas/alice"
8+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
89 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
910 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/header"
1011)
@@ -61,7 +62,7 @@ func newRequestHeaderInjector(headers []options.Header) (alice.Constructor, erro
6162
6263func injectRequestHeaders(injector header.Injector, next http.Handler) http.Handler {
6364 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
64− scope := GetRequestScope(req)
65+ scope := middlewareapi.GetRequestScope(req)
6566
6667 // If scope is nil, this will panic.
6768 // A scope should always be injected before this handler is called.
@@ -92,7 +93,7 @@ func newResponseHeaderInjector(headers []options.Header) (alice.Constructor, err
9293
9394func injectResponseHeaders(injector header.Injector, next http.Handler) http.Handler {
9495 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
95− scope := GetRequestScope(req)
96+ scope := middlewareapi.GetRequestScope(req)
9697
9798 // If scope is nil, this will panic.
9899 // A scope should always be injected before this handler is called.
pkg/middleware/headers_test.go+2−5
@@ -1,7 +1,6 @@
11package middleware
22
33import (
4− "context"
54 "encoding/base64"
65 "net/http"
76 "net/http/httptest"
@@ -31,8 +30,7 @@ var _ = Describe("Headers Suite", func() {
3130
3231 // Set up the request with a request scope
3332 req := httptest.NewRequest("", "/", nil)
34− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
35− req = req.WithContext(contextWithScope)
33+ req = middlewareapi.AddRequestScope(req, scope)
3634 req.Header = in.initialHeaders.Clone()
3735
3836 rw := httptest.NewRecorder()
@@ -218,8 +216,7 @@ var _ = Describe("Headers Suite", func() {
218216
219217 // Set up the request with a request scope
220218 req := httptest.NewRequest("", "/", nil)
221− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
222− req = req.WithContext(contextWithScope)
219+ req = middlewareapi.AddRequestScope(req, scope)
223220
224221 rw := httptest.NewRecorder()
225222 for key, values := range in.initialHeaders {
pkg/middleware/jwt_session.go+1−1
@@ -37,7 +37,7 @@ type jwtSessionLoader struct {
3737// If a session was loaded by a previous handler, it will not be replaced.
3838func (j *jwtSessionLoader) loadSession(next http.Handler) http.Handler {
3939 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
40− scope := GetRequestScope(req)
40+ scope := middlewareapi.GetRequestScope(req)
4141 // If scope is nil, this will panic.
4242 // A scope should always be injected before this handler is called.
4343 if scope.Session != nil {
pkg/middleware/jwt_session_test.go+2−3
@@ -103,8 +103,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=`
103103 // Set up the request with the authorization header and a request scope
104104 req := httptest.NewRequest("", "/", nil)
105105 req.Header.Set("Authorization", in.authorizationHeader)
106− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
107− req = req.WithContext(contextWithScope)
106+ req = middlewareapi.AddRequestScope(req, scope)
108107
109108 rw := httptest.NewRecorder()
110109
@@ -116,7 +115,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=`
116115 // from the scope
117116 var gotSession *sessionsapi.SessionState
118117 handler := NewJwtSessionLoader(sessionLoaders)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
119− gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
118+ gotSession = middlewareapi.GetRequestScope(r).Session
120119 }))
121120 handler.ServeHTTP(rw, req)
122121
pkg/middleware/scope.go+4−23
@@ -1,39 +1,20 @@
11package middleware
22
33import (
4− "context"
54 "net/http"
65
76 "github.com/justinas/alice"
87 middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
9− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
108)
119
12−type scopeKey string
13−
14−// requestScopeKey uses a typed string to reduce likelihood of clashing
15−// with other context keys
16−const requestScopeKey scopeKey = "request-scope"
17−
18−func NewScope(opts *options.Options) alice.Constructor {
10+func NewScope(reverseProxy bool) alice.Constructor {
1911 return func(next http.Handler) http.Handler {
2012 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
2113 scope := &middlewareapi.RequestScope{
22− ReverseProxy: opts.ReverseProxy,
14+ ReverseProxy: reverseProxy,
2315 }
24− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
25− requestWithScope := req.WithContext(contextWithScope)
26− next.ServeHTTP(rw, requestWithScope)
16+ req = middlewareapi.AddRequestScope(req, scope)
17+ next.ServeHTTP(rw, req)
2718 })
2819 }
2920}
30−
31−// GetRequestScope returns the current request scope from the given request
32−func GetRequestScope(req *http.Request) *middlewareapi.RequestScope {
33− scope := req.Context().Value(requestScopeKey)
34− if scope == nil {
35− return nil
36− }
37−
38− return scope.(*middlewareapi.RequestScope)
39−}
pkg/middleware/scope_test.go+32−57
@@ -1,7 +1,6 @@
11package middleware
22
33import (
4− "context"
54 "net/http"
65 "net/http/httptest"
76
@@ -21,73 +20,49 @@ var _ = Describe("Scope Suite", func() {
2120 Expect(err).ToNot(HaveOccurred())
2221
2322 rw = httptest.NewRecorder()
24−
25− handler := NewScope()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26− nextRequest = r
27− w.WriteHeader(200)
28− }))
29− handler.ServeHTTP(rw, request)
30− })
31−
32− It("does not add a scope to the original request", func() {
33− Expect(request.Context().Value(requestScopeKey)).To(BeNil())
34− })
35−
36− It("cannot load a scope from the original request using GetRequestScope", func() {
37− Expect(GetRequestScope(request)).To(BeNil())
38− })
39−
40− It("adds a scope to the request for the next handler", func() {
41− Expect(nextRequest.Context().Value(requestScopeKey)).ToNot(BeNil())
42− })
43−
44− It("can load a scope from the next handler's request using GetRequestScope", func() {
45− Expect(GetRequestScope(nextRequest)).ToNot(BeNil())
46− })
47− })
48−
49− Context("GetRequestScope", func() {
50− var request *http.Request
51−
52− BeforeEach(func() {
53− var err error
54− request, err = http.NewRequest("", "http://127.0.0.1/", nil)
55− Expect(err).ToNot(HaveOccurred())
5623 })
5724
58− Context("with a scope", func() {
59− var scope *middlewareapi.RequestScope
60−
25+ Context("ReverseProxy is false", func() {
6126 BeforeEach(func() {
62− scope = &middlewareapi.RequestScope{}
63− contextWithScope := context.WithValue(request.Context(), requestScopeKey, scope)
64− request = request.WithContext(contextWithScope)
27+ handler := NewScope(false)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
28+ nextRequest = r
29+ w.WriteHeader(200)
30+ }))
31+ handler.ServeHTTP(rw, request)
6532 })
6633
67− It("returns the scope", func() {
68− s := GetRequestScope(request)
69− Expect(s).ToNot(BeNil())
70− Expect(s).To(Equal(scope))
34+ It("does not add a scope to the original request", func() {
35+ Expect(request.Context().Value(middlewareapi.RequestScopeKey)).To(BeNil())
7136 })
7237
73− Context("if the scope is then modified", func() {
74− BeforeEach(func() {
75− Expect(scope.SaveSession).To(BeFalse())
76− scope.SaveSession = true
77− })
38+ It("cannot load a scope from the original request using GetRequestScope", func() {
39+ Expect(middlewareapi.GetRequestScope(request)).To(BeNil())
40+ })
41+
42+ It("adds a scope to the request for the next handler", func() {
43+ Expect(nextRequest.Context().Value(middlewareapi.RequestScopeKey)).ToNot(BeNil())
44+ })
7845
79− It("returns the updated session", func() {
80− s := GetRequestScope(request)
81− Expect(s).ToNot(BeNil())
82− Expect(s).To(Equal(scope))
83− Expect(s.SaveSession).To(BeTrue())
84− })
46+ It("can load a scope from the next handler's request using GetRequestScope", func() {
47+ scope := middlewareapi.GetRequestScope(nextRequest)
48+ Expect(scope).ToNot(BeNil())
49+ Expect(scope.ReverseProxy).To(BeFalse())
8550 })
8651 })
8752
88− Context("without a scope", func() {
89− It("returns nil", func() {
90− Expect(GetRequestScope(request)).To(BeNil())
53+ Context("ReverseProxy is true", func() {
54+ BeforeEach(func() {
55+ handler := NewScope(true)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
56+ nextRequest = r
57+ w.WriteHeader(200)
58+ }))
59+ handler.ServeHTTP(rw, request)
60+ })
61+
62+ It("return a scope where the ReverseProxy field is true", func() {
63+ scope := middlewareapi.GetRequestScope(nextRequest)
64+ Expect(scope).ToNot(BeNil())
65+ Expect(scope.ReverseProxy).To(BeTrue())
9166 })
9267 })
9368 })
pkg/middleware/stored_session.go+2−1
@@ -8,6 +8,7 @@ import (
88 "time"
99
1010 "github.com/justinas/alice"
11+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1112 sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
1213 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
1314)
@@ -59,7 +60,7 @@ type storedSessionLoader struct {
5960// If a session was loader by a previous handler, it will not be replaced.
6061func (s *storedSessionLoader) loadSession(next http.Handler) http.Handler {
6162 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
62− scope := GetRequestScope(req)
63+ scope := middlewareapi.GetRequestScope(req)
6364 // If scope is nil, this will panic.
6465 // A scope should always be injected before this handler is called.
6566 if scope.Session != nil {
pkg/middleware/stored_session_test.go+2−3
@@ -104,8 +104,7 @@ var _ = Describe("Stored Session Suite", func() {
104104 // Set up the request with the request headesr and a request scope
105105 req := httptest.NewRequest("", "/", nil)
106106 req.Header = in.requestHeaders
107− contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
108− req = req.WithContext(contextWithScope)
107+ req = middlewareapi.AddRequestScope(req, scope)
109108
110109 rw := httptest.NewRecorder()
111110
@@ -120,7 +119,7 @@ var _ = Describe("Stored Session Suite", func() {
120119 // from the scope
121120 var gotSession *sessionsapi.SessionState
122121 handler := NewStoredSessionLoader(opts)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
123− gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
122+ gotSession = middlewareapi.GetRequestScope(r).Session
124123 }))
125124 handler.ServeHTTP(rw, req)
126125
pkg/requests/util/util.gonew file+48
@@ -0,0 +1,48 @@
1+package util
2+
3+import (
4+ "net/http"
5+
6+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
7+)
8+
9+// GetRequestProto returns the request scheme or X-Forwarded-Proto if present
10+// and the request is proxied.
11+func GetRequestProto(req *http.Request) string {
12+ proto := req.Header.Get("X-Forwarded-Proto")
13+ if !IsProxied(req) || proto == "" {
14+ proto = req.URL.Scheme
15+ }
16+ return proto
17+}
18+
19+// GetRequestHost returns the request host header or X-Forwarded-Host if
20+// present and the request is proxied.
21+func GetRequestHost(req *http.Request) string {
22+ host := req.Header.Get("X-Forwarded-Host")
23+ if !IsProxied(req) || host == "" {
24+ host = req.Host
25+ }
26+ return host
27+}
28+
29+// GetRequestURI return the request URI or X-Forwarded-Uri if present and the
30+// request is proxied.
31+func GetRequestURI(req *http.Request) string {
32+ uri := req.Header.Get("X-Forwarded-Uri")
33+ if !IsProxied(req) || uri == "" {
34+ // Use RequestURI to preserve ?query
35+ uri = req.URL.RequestURI()
36+ }
37+ return uri
38+}
39+
40+// IsProxied determines if a request was from a proxy based on the RequestScope
41+// ReverseProxy tracker.
42+func IsProxied(req *http.Request) bool {
43+ scope := middlewareapi.GetRequestScope(req)
44+ if scope == nil {
45+ return false
46+ }
47+ return scope.ReverseProxy
48+}
pkg/requests/util/util_suite_test.gonew file+19
@@ -0,0 +1,19 @@
1+package util_test
2+
3+import (
4+ "testing"
5+
6+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
7+ . "github.com/onsi/ginkgo"
8+ . "github.com/onsi/gomega"
9+)
10+
11+// TestRequestUtilSuite and related tests are in a *_test package
12+// to prevent circular imports with the `logger` package which uses
13+// this functionality
14+func TestRequestUtilSuite(t *testing.T) {
15+ logger.SetOutput(GinkgoWriter)
16+
17+ RegisterFailHandler(Fail)
18+ RunSpecs(t, "Request Utils")
19+}
pkg/requests/util/util_test.gonew file+131
@@ -0,0 +1,131 @@
1+package util_test
2+
3+import (
4+ "fmt"
5+ "net/http"
6+ "net/http/httptest"
7+
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
9+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests/util"
10+ . "github.com/onsi/ginkgo"
11+ . "github.com/onsi/gomega"
12+)
13+
14+var _ = Describe("Util Suite", func() {
15+ const (
16+ proto = "http"
17+ host = "www.oauth2proxy.test"
18+ uri = "/test/endpoint"
19+ )
20+ var req *http.Request
21+
22+ BeforeEach(func() {
23+ req = httptest.NewRequest(
24+ http.MethodGet,
25+ fmt.Sprintf("%s://%s%s", proto, host, uri),
26+ nil,
27+ )
28+ })
29+
30+ Context("GetRequestHost", func() {
31+ Context("IsProxied is false", func() {
32+ BeforeEach(func() {
33+ req = middleware.AddRequestScope(req, &middleware.RequestScope{})
34+ })
35+
36+ It("returns the host", func() {
37+ Expect(util.GetRequestHost(req)).To(Equal(host))
38+ })
39+
40+ It("ignores X-Forwarded-Host and returns the host", func() {
41+ req.Header.Add("X-Forwarded-Host", "external.oauth2proxy.text")
42+ Expect(util.GetRequestHost(req)).To(Equal(host))
43+ })
44+ })
45+
46+ Context("IsProxied is true", func() {
47+ BeforeEach(func() {
48+ req = middleware.AddRequestScope(req, &middleware.RequestScope{
49+ ReverseProxy: true,
50+ })
51+ })
52+
53+ It("returns the host if X-Forwarded-Host is not present", func() {
54+ Expect(util.GetRequestHost(req)).To(Equal(host))
55+ })
56+
57+ It("returns the X-Forwarded-Host when present", func() {
58+ req.Header.Add("X-Forwarded-Host", "external.oauth2proxy.text")
59+ Expect(util.GetRequestHost(req)).To(Equal("external.oauth2proxy.text"))
60+ })
61+ })
62+ })
63+
64+ Context("GetRequestProto", func() {
65+ Context("IsProxied is false", func() {
66+ BeforeEach(func() {
67+ req = middleware.AddRequestScope(req, &middleware.RequestScope{})
68+ })
69+
70+ It("returns the scheme", func() {
71+ Expect(util.GetRequestProto(req)).To(Equal(proto))
72+ })
73+
74+ It("ignores X-Forwarded-Proto and returns the scheme", func() {
75+ req.Header.Add("X-Forwarded-Proto", "https")
76+ Expect(util.GetRequestProto(req)).To(Equal(proto))
77+ })
78+ })
79+
80+ Context("IsProxied is true", func() {
81+ BeforeEach(func() {
82+ req = middleware.AddRequestScope(req, &middleware.RequestScope{
83+ ReverseProxy: true,
84+ })
85+ })
86+
87+ It("returns the scheme if X-Forwarded-Proto is not present", func() {
88+ Expect(util.GetRequestProto(req)).To(Equal(proto))
89+ })
90+
91+ It("returns the X-Forwarded-Proto when present", func() {
92+ req.Header.Add("X-Forwarded-Proto", "https")
93+ Expect(util.GetRequestProto(req)).To(Equal("https"))
94+ })
95+ })
96+ })
97+
98+ Context("GetRequestURI", func() {
99+ Context("IsProxied is false", func() {
100+ BeforeEach(func() {
101+ req = middleware.AddRequestScope(req, &middleware.RequestScope{})
102+ })
103+
104+ It("returns the URI", func() {
105+ Expect(util.GetRequestURI(req)).To(Equal(uri))
106+ })
107+
108+ It("ignores X-Forwarded-Uri and returns the URI", func() {
109+ req.Header.Add("X-Forwarded-Uri", "/some/other/path")
110+ Expect(util.GetRequestURI(req)).To(Equal(uri))
111+ })
112+ })
113+
114+ Context("IsProxied is true", func() {
115+ BeforeEach(func() {
116+ req = middleware.AddRequestScope(req, &middleware.RequestScope{
117+ ReverseProxy: true,
118+ })
119+ })
120+
121+ It("returns the URI if X-Forwarded-Uri is not present", func() {
122+ Expect(util.GetRequestURI(req)).To(Equal(uri))
123+ })
124+
125+ It("returns the X-Forwarded-Uri when present", func() {
126+ req.Header.Add("X-Forwarded-Uri", "/some/other/path")
127+ Expect(util.GetRequestURI(req)).To(Equal("/some/other/path"))
128+ })
129+ })
130+ })
131+})
pkg/util/util.go−37
@@ -4,9 +4,6 @@ import (
44 "crypto/x509"
55 "fmt"
66 "io/ioutil"
7− "net/http"
8−
9− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/middleware"
107)
118
129func GetCertPool(paths []string) (*x509.CertPool, error) {
@@ -26,37 +23,3 @@ func GetCertPool(paths []string) (*x509.CertPool, error) {
2623 }
2724 return pool, nil
2825}
29−
30−// GetRequestProto return the request host header or X-Forwarded-Proto if present
31−func GetRequestProto(req *http.Request) string {
32− proto := req.Header.Get("X-Forwarded-Proto")
33− if !isProxied(req) || proto == "" {
34− proto = req.URL.Scheme
35− }
36− return proto
37−}
38−
39−// GetRequestHost return the request host header or X-Forwarded-Host if present
40−// and reverse proxy mode is enabled.
41−func GetRequestHost(req *http.Request) string {
42− host := req.Header.Get("X-Forwarded-Host")
43− if !isProxied(req) || host == "" {
44− host = req.Host
45− }
46− return host
47−}
48−
49−// GetRequestURI return the request host header or X-Forwarded-Uri if present
50−func GetRequestURI(req *http.Request) string {
51− uri := req.Header.Get("X-Forwarded-Uri")
52− if !isProxied(req) || uri == "" {
53− // Use RequestURI to preserve ?query
54− uri = req.URL.RequestURI()
55− }
56− return uri
57−}
58−
59−func isProxied(req *http.Request) bool {
60− scope := middleware.GetRequestScope(req)
61− return scope.ReverseProxy
62−}
pkg/util/util_test.go−41
@@ -4,11 +4,9 @@ import (
44 "crypto/x509/pkix"
55 "encoding/asn1"
66 "io/ioutil"
7− "net/http/httptest"
87 "os"
98 "testing"
109
11− . "github.com/onsi/gomega"
1210 "github.com/stretchr/testify/assert"
1311)
1412
@@ -97,42 +95,3 @@ func TestGetCertPool(t *testing.T) {
9795 expectedSubjects := []string{testCA1Subj, testCA2Subj}
9896 assert.Equal(t, expectedSubjects, got)
9997}
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−}
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−}