git.iko.soy/typeable/oauth2-proxy
on master help sign in
Move Logging to Middleware Package (#1070)

* Use a specialized ResponseWriter in middleware * Track User & Upstream in RequestScope * Wrap responses in our custom ResponseWriter * Add tests for logging middleware * Inject upstream metadata into request scope * Use custom ResponseWriter only in logging middleware * Assume RequestScope is never nil

commit602dac78526336068c03f20b6037daa8f0e3003d authorNick Meves <nick.meves@greenhouse.io> dateSat 6 Mar 2021, 09:27 (6 years ago) parent220b370
16 files changed+337−266
compare with master
unifiedsplit
CHANGELOG.md+1 logging_handler.go−108 logging_handler_test.go−116 oauthproxy.go+8−2 pkg/apis/middleware/scope.go+3 pkg/middleware/middleware_suite_test.go+12 pkg/middleware/request_logger.go+110 pkg/middleware/request_logger_test.go+121 pkg/upstream/file.go+7−1 pkg/upstream/file_test.go+6−1 pkg/upstream/http.go+7−1 pkg/upstream/http_test.go+21−15 pkg/upstream/proxy_test.go+23−18 pkg/upstream/static.go+12−2 pkg/upstream/static_test.go+6−1 pkg/upstream/upstream_suite_test.go−1
CHANGELOG.md+1
@@ -8,6 +8,7 @@
88
99## Changes since v7.0.1
1010
11+- [#1070](https://github.com/oauth2-proxy/oauth2-proxy/pull/1070) Refactor logging middleware to middleware package (@NickMeves)
1112- [#1064](https://github.com/oauth2-proxy/oauth2-proxy/pull/1064) Add support for setting groups on session when using basic auth (@stefansedich)
1213- [#1056](https://github.com/oauth2-proxy/oauth2-proxy/pull/1056) Add option for custom logos on the sign in page (@JoelSpeed)
1314- [#1054](https://github.com/oauth2-proxy/oauth2-proxy/pull/1054) Update to Go 1.16 (@JoelSpeed)
logging_handler.godeleted−108
@@ -1,108 +0,0 @@
1−// largely adapted from https://github.com/gorilla/handlers/blob/master/handlers.go
2−// to add logging of request duration as last value (and drop referrer)
3−
4−package main
5−
6−import (
7− "bufio"
8− "errors"
9− "net"
10− "net/http"
11− "time"
12−
13− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
14−)
15−
16−// responseLogger is wrapper of http.ResponseWriter that keeps track of its HTTP status
17−// code and body size
18−type responseLogger struct {
19− w http.ResponseWriter
20− status int
21− size int
22− upstream string
23− authInfo string
24−}
25−
26−// Header returns the ResponseWriter's Header
27−func (l *responseLogger) Header() http.Header {
28− return l.w.Header()
29−}
30−
31−// Support Websocket
32−func (l *responseLogger) Hijack() (rwc net.Conn, buf *bufio.ReadWriter, err error) {
33− if hj, ok := l.w.(http.Hijacker); ok {
34− return hj.Hijack()
35− }
36− return nil, nil, errors.New("http.Hijacker is not available on writer")
37−}
38−
39−// ExtractGAPMetadata extracts and removes GAP headers from the ResponseWriter's
40−// Header
41−func (l *responseLogger) ExtractGAPMetadata() {
42− upstream := l.w.Header().Get("GAP-Upstream-Address")
43− if upstream != "" {
44− l.upstream = upstream
45− l.w.Header().Del("GAP-Upstream-Address")
46− }
47− authInfo := l.w.Header().Get("GAP-Auth")
48− if authInfo != "" {
49− l.authInfo = authInfo
50− l.w.Header().Del("GAP-Auth")
51− }
52−}
53−
54−// Write writes the response using the ResponseWriter
55−func (l *responseLogger) Write(b []byte) (int, error) {
56− if l.status == 0 {
57− // The status will be StatusOK if WriteHeader has not been called yet
58− l.status = http.StatusOK
59− }
60− l.ExtractGAPMetadata()
61− size, err := l.w.Write(b)
62− l.size += size
63− return size, err
64−}
65−
66−// WriteHeader writes the status code for the Response
67−func (l *responseLogger) WriteHeader(s int) {
68− l.ExtractGAPMetadata()
69− l.w.WriteHeader(s)
70− l.status = s
71−}
72−
73−// Status returns the response status code
74−func (l *responseLogger) Status() int {
75− return l.status
76−}
77−
78−// Size returns the response size
79−func (l *responseLogger) Size() int {
80− return l.size
81−}
82−
83−// Flush sends any buffered data to the client
84−func (l *responseLogger) Flush() {
85− if flusher, ok := l.w.(http.Flusher); ok {
86− flusher.Flush()
87− }
88−}
89−
90−// loggingHandler is the http.Handler implementation for LoggingHandler
91−type loggingHandler struct {
92− handler http.Handler
93−}
94−
95−// LoggingHandler provides an http.Handler which logs requests to the HTTP server
96−func LoggingHandler(h http.Handler) http.Handler {
97− return loggingHandler{
98− handler: h,
99− }
100−}
101−
102−func (h loggingHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
103− t := time.Now()
104− url := *req.URL
105− responseLogger := &responseLogger{w: w}
106− h.handler.ServeHTTP(responseLogger, req)
107− logger.PrintReq(responseLogger.authInfo, responseLogger.upstream, req, url, t, responseLogger.Status(), responseLogger.Size())
108−}
logging_handler_test.godeleted−116
@@ -1,116 +0,0 @@
1−package main
2−
3−import (
4− "bytes"
5− "net/http"
6− "net/http/httptest"
7− "testing"
8−
9− "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
10− "github.com/stretchr/testify/assert"
11−)
12−
13−const RequestLoggingFormatWithoutTime = "{{.Client}} - {{.Username}} [TIMELESS] {{.Host}} {{.RequestMethod}} {{.Upstream}} {{.RequestURI}} {{.Protocol}} {{.UserAgent}} {{.StatusCode}} {{.ResponseSize}} {{.RequestDuration}}"
14−
15−func TestLoggingHandler_ServeHTTP(t *testing.T) {
16− tests := []struct {
17− Format string
18− ExpectedLogMessage string
19− Path string
20− ExcludePaths []string
21− }{
22− {
23− Format: RequestLoggingFormatWithoutTime,
24− ExpectedLogMessage: "127.0.0.1 - - [TIMELESS] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n",
25− Path: "/foo/bar",
26− ExcludePaths: []string{},
27− },
28− {
29− Format: RequestLoggingFormatWithoutTime,
30− ExpectedLogMessage: "127.0.0.1 - - [TIMELESS] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n",
31− Path: "/foo/bar",
32− ExcludePaths: []string{},
33− },
34− {
35− Format: RequestLoggingFormatWithoutTime,
36− ExpectedLogMessage: "127.0.0.1 - - [TIMELESS] test-server GET - \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n",
37− Path: "/foo/bar",
38− ExcludePaths: []string{"/ping"},
39− },
40− {
41− Format: RequestLoggingFormatWithoutTime,
42− ExpectedLogMessage: "",
43− Path: "/foo/bar",
44− ExcludePaths: []string{"/foo/bar"},
45− },
46− {
47− Format: RequestLoggingFormatWithoutTime,
48− ExpectedLogMessage: "127.0.0.1 - - [TIMELESS] test-server GET - \"/ping\" HTTP/1.1 \"\" 200 4 0.000\n",
49− Path: "/ping",
50− ExcludePaths: []string{},
51− },
52− {
53− Format: RequestLoggingFormatWithoutTime,
54− ExpectedLogMessage: "",
55− Path: "/ping",
56− ExcludePaths: []string{"/ping"},
57− },
58− {
59− Format: RequestLoggingFormatWithoutTime,
60− ExpectedLogMessage: "",
61− Path: "/ping",
62− ExcludePaths: []string{"/foo/bar", "/ping"},
63− },
64− {
65− Format: "{{.RequestMethod}}",
66− ExpectedLogMessage: "GET\n",
67− Path: "/foo/bar",
68− ExcludePaths: []string{""},
69− },
70− {
71− Format: "{{.RequestMethod}}",
72− ExpectedLogMessage: "GET\n",
73− Path: "/foo/bar",
74− ExcludePaths: []string{"/ping"},
75− },
76− {
77− Format: "{{.RequestMethod}}",
78− ExpectedLogMessage: "GET\n",
79− Path: "/ping",
80− ExcludePaths: []string{""},
81− },
82− {
83− Format: "{{.RequestMethod}}",
84− ExpectedLogMessage: "",
85− Path: "/ping",
86− ExcludePaths: []string{"/ping"},
87− },
88− }
89−
90− for _, test := range tests {
91− buf := bytes.NewBuffer(nil)
92− handler := func(w http.ResponseWriter, req *http.Request) {
93− _, ok := w.(http.Hijacker)
94− if !ok {
95− t.Error("http.Hijacker is not available")
96− }
97−
98− _, err := w.Write([]byte("test"))
99− assert.NoError(t, err)
100− }
101−
102− logger.SetOutput(buf)
103− logger.SetReqTemplate(test.Format)
104− logger.SetExcludePaths(test.ExcludePaths)
105− h := LoggingHandler(http.HandlerFunc(handler))
106−
107− r, _ := http.NewRequest("GET", test.Path, nil)
108− r.RemoteAddr = "127.0.0.1"
109− r.Host = "test-server"
110−
111− h.ServeHTTP(httptest.NewRecorder(), r)
112−
113− actual := buf.String()
114− assert.Equal(t, test.ExpectedLogMessage, actual)
115− }
116−}
oauthproxy.go+8−2
@@ -250,9 +250,15 @@ func buildPreAuthChain(opts *options.Options) (alice.Chain, error) {
250250 // To silence logging of health checks, register the health check handler before
251251 // the logging handler
252252 if opts.Logging.SilencePing {
253− chain = chain.Append(middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents), LoggingHandler)
253+ chain = chain.Append(
254+ middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents),
255+ middleware.NewRequestLogger(),
256+ )
254257 } else {
255− chain = chain.Append(LoggingHandler, middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents))
258+ chain = chain.Append(
259+ middleware.NewRequestLogger(),
260+ middleware.NewHealthCheck(healthCheckPaths, healthCheckUserAgents),
261+ )
256262 }
257263
258264 chain = chain.Append(middleware.NewRequestMetricsWithDefaultRegistry())
pkg/apis/middleware/scope.go+3
@@ -34,6 +34,9 @@ type RequestScope struct {
3434 // SessionRevalidated indicates whether the session has been revalidated since
3535 // it was loaded or not.
3636 SessionRevalidated bool
37+
38+ // Upstream tracks which upstream was used for this request
39+ Upstream string
3740}
3841
3942// GetRequestScope returns the current request scope from the given request
pkg/middleware/middleware_suite_test.go+12
@@ -4,6 +4,7 @@ import (
44 "net/http"
55 "testing"
66
7+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
78 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
89 . "github.com/onsi/ginkgo"
910 . "github.com/onsi/gomega"
@@ -19,6 +20,17 @@ func TestMiddlewareSuite(t *testing.T) {
1920
2021func testHandler() http.Handler {
2122 return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
23+ rw.WriteHeader(200)
24+ rw.Write([]byte("test"))
25+ })
26+}
27+
28+func testUpstreamHandler(upstream string) http.Handler {
29+ return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
30+ scope := middlewareapi.GetRequestScope(req)
31+ scope.Upstream = upstream
32+
33+ rw.WriteHeader(200)
2234 rw.Write([]byte("test"))
2335 })
2436}
pkg/middleware/request_logger.gonew file+110
@@ -0,0 +1,110 @@
1+package middleware
2+
3+import (
4+ "bufio"
5+ "errors"
6+ "net"
7+ "net/http"
8+ "time"
9+
10+ "github.com/justinas/alice"
11+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
12+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
13+)
14+
15+// NewRequestLogger returns middleware which logs requests
16+// It uses a custom ResponseWriter to track status code & response size details
17+func NewRequestLogger() alice.Constructor {
18+ return requestLogger
19+}
20+
21+func requestLogger(next http.Handler) http.Handler {
22+ return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
23+ startTime := time.Now()
24+ url := *req.URL
25+
26+ responseLogger := &loggingResponse{ResponseWriter: rw}
27+ next.ServeHTTP(responseLogger, req)
28+
29+ scope := middlewareapi.GetRequestScope(req)
30+ // If scope is nil, this will panic.
31+ // A scope should always be injected before this handler is called.
32+ logger.PrintReq(
33+ getUser(scope),
34+ scope.Upstream,
35+ req,
36+ url,
37+ startTime,
38+ responseLogger.Status(),
39+ responseLogger.Size(),
40+ )
41+ })
42+}
43+
44+func getUser(scope *middlewareapi.RequestScope) string {
45+ session := scope.Session
46+ if session != nil {
47+ if session.Email != "" {
48+ return session.Email
49+ }
50+ return session.User
51+ }
52+ return ""
53+}
54+
55+// loggingResponse is a custom http.ResponseWriter that allows tracking certain
56+// details for request logging.
57+type loggingResponse struct {
58+ http.ResponseWriter
59+
60+ status int
61+ size int
62+}
63+
64+// Write writes the response using the ResponseWriter
65+func (r *loggingResponse) Write(b []byte) (int, error) {
66+ if r.status == 0 {
67+ // The status will be StatusOK if WriteHeader has not been called yet
68+ r.status = http.StatusOK
69+ }
70+ size, err := r.ResponseWriter.Write(b)
71+ r.size += size
72+ return size, err
73+}
74+
75+// WriteHeader writes the status code for the Response
76+func (r *loggingResponse) WriteHeader(s int) {
77+ r.ResponseWriter.WriteHeader(s)
78+ r.status = s
79+}
80+
81+// Hijack implements the `http.Hijacker` interface that actual ResponseWriters
82+// implement to support websockets
83+func (r *loggingResponse) Hijack() (net.Conn, *bufio.ReadWriter, error) {
84+ if hj, ok := r.ResponseWriter.(http.Hijacker); ok {
85+ return hj.Hijack()
86+ }
87+ return nil, nil, errors.New("http.Hijacker is not available on writer")
88+}
89+
90+// Flush sends any buffered data to the client. Implements the `http.Flusher`
91+// interface
92+func (r *loggingResponse) Flush() {
93+ if flusher, ok := r.ResponseWriter.(http.Flusher); ok {
94+ if r.status == 0 {
95+ // The status will be StatusOK if WriteHeader has not been called yet
96+ r.status = http.StatusOK
97+ }
98+ flusher.Flush()
99+ }
100+}
101+
102+// Status returns the response status code
103+func (r *loggingResponse) Status() int {
104+ return r.status
105+}
106+
107+// Size returns the response size
108+func (r *loggingResponse) Size() int {
109+ return r.size
110+}
pkg/middleware/request_logger_test.gonew file+121
@@ -0,0 +1,121 @@
1+package middleware
2+
3+import (
4+ "bytes"
5+ "net/http"
6+ "net/http/httptest"
7+
8+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
9+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
10+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
11+ . "github.com/onsi/ginkgo"
12+ . "github.com/onsi/ginkgo/extensions/table"
13+ . "github.com/onsi/gomega"
14+)
15+
16+const RequestLoggingFormatWithoutTime = "{{.Client}} - {{.Username}} [TIMELESS] {{.Host}} {{.RequestMethod}} {{.Upstream}} {{.RequestURI}} {{.Protocol}} {{.UserAgent}} {{.StatusCode}} {{.ResponseSize}} {{.RequestDuration}}"
17+
18+var _ = Describe("Request logger suite", func() {
19+ type requestLoggerTableInput struct {
20+ Format string
21+ ExpectedLogMessage string
22+ Path string
23+ ExcludePaths []string
24+ Upstream string
25+ Session *sessions.SessionState
26+ }
27+
28+ DescribeTable("when service a request",
29+ func(in *requestLoggerTableInput) {
30+ buf := bytes.NewBuffer(nil)
31+ logger.SetOutput(buf)
32+ logger.SetReqTemplate(in.Format)
33+ logger.SetExcludePaths(in.ExcludePaths)
34+
35+ req, err := http.NewRequest("GET", in.Path, nil)
36+ Expect(err).ToNot(HaveOccurred())
37+ req.RemoteAddr = "127.0.0.1"
38+ req.Host = "test-server"
39+
40+ scope := &middlewareapi.RequestScope{Session: in.Session}
41+ req = middlewareapi.AddRequestScope(req, scope)
42+
43+ handler := NewRequestLogger()(testUpstreamHandler(in.Upstream))
44+ handler.ServeHTTP(httptest.NewRecorder(), req)
45+
46+ Expect(buf.String()).To(Equal(in.ExpectedLogMessage))
47+ },
48+ Entry("standard request", &requestLoggerTableInput{
49+ Format: RequestLoggingFormatWithoutTime,
50+ ExpectedLogMessage: "127.0.0.1 - standard.user [TIMELESS] test-server GET standard \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n",
51+ Path: "/foo/bar",
52+ ExcludePaths: []string{},
53+ Upstream: "standard",
54+ Session: &sessions.SessionState{User: "standard.user"},
55+ }),
56+ Entry("with unrelated path excluded", &requestLoggerTableInput{
57+ Format: RequestLoggingFormatWithoutTime,
58+ ExpectedLogMessage: "127.0.0.1 - unrelated.exclusion [TIMELESS] test-server GET unrelated \"/foo/bar\" HTTP/1.1 \"\" 200 4 0.000\n",
59+ Path: "/foo/bar",
60+ ExcludePaths: []string{"/ping"},
61+ Upstream: "unrelated",
62+ Session: &sessions.SessionState{User: "unrelated.exclusion"},
63+ }),
64+ Entry("with path as the sole exclusion", &requestLoggerTableInput{
65+ Format: RequestLoggingFormatWithoutTime,
66+ ExpectedLogMessage: "",
67+ Path: "/foo/bar",
68+ ExcludePaths: []string{"/foo/bar"},
69+ }),
70+ Entry("ping path", &requestLoggerTableInput{
71+ Format: RequestLoggingFormatWithoutTime,
72+ ExpectedLogMessage: "127.0.0.1 - mr.ping [TIMELESS] test-server GET - \"/ping\" HTTP/1.1 \"\" 200 4 0.000\n",
73+ Path: "/ping",
74+ ExcludePaths: []string{},
75+ Upstream: "",
76+ Session: &sessions.SessionState{User: "mr.ping"},
77+ }),
78+ Entry("ping path but excluded", &requestLoggerTableInput{
79+ Format: RequestLoggingFormatWithoutTime,
80+ ExpectedLogMessage: "",
81+ Path: "/ping",
82+ ExcludePaths: []string{"/ping"},
83+ Upstream: "",
84+ Session: &sessions.SessionState{User: "mr.ping"},
85+ }),
86+ Entry("ping path and excluded in list", &requestLoggerTableInput{
87+ Format: RequestLoggingFormatWithoutTime,
88+ ExpectedLogMessage: "",
89+ Path: "/ping",
90+ ExcludePaths: []string{"/foo/bar", "/ping"},
91+ }),
92+ Entry("custom format", &requestLoggerTableInput{
93+ Format: "{{.RequestMethod}} {{.Username}} {{.Upstream}}",
94+ ExpectedLogMessage: "GET custom.format custom\n",
95+ Path: "/foo/bar",
96+ ExcludePaths: []string{""},
97+ Upstream: "custom",
98+ Session: &sessions.SessionState{User: "custom.format"},
99+ }),
100+ Entry("custom format with unrelated exclusion", &requestLoggerTableInput{
101+ Format: "{{.RequestMethod}} {{.Username}} {{.Upstream}}",
102+ ExpectedLogMessage: "GET custom.format custom\n",
103+ Path: "/foo/bar",
104+ ExcludePaths: []string{"/ping"},
105+ Upstream: "custom",
106+ Session: &sessions.SessionState{User: "custom.format"},
107+ }),
108+ Entry("custom format ping path", &requestLoggerTableInput{
109+ Format: "{{.RequestMethod}}",
110+ ExpectedLogMessage: "GET\n",
111+ Path: "/ping",
112+ ExcludePaths: []string{""},
113+ }),
114+ Entry("custom format ping path excluded", &requestLoggerTableInput{
115+ Format: "{{.RequestMethod}}",
116+ ExpectedLogMessage: "",
117+ Path: "/ping",
118+ ExcludePaths: []string{"/ping"},
119+ }),
120+ )
121+})
pkg/upstream/file.go+7−1
@@ -4,6 +4,8 @@ import (
44 "net/http"
55 "runtime"
66 "strings"
7+
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
79)
810
911const fileScheme = "file"
@@ -37,6 +39,10 @@ type fileServer struct {
3739// ServeHTTP proxies requests to the upstream provider while signing the
3840// request headers
3941func (u *fileServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
40− rw.Header().Set("GAP-Upstream-Address", u.upstream)
42+ scope := middleware.GetRequestScope(req)
43+ // If scope is nil, this will panic.
44+ // A scope should always be injected before this handler is called.
45+ scope.Upstream = u.upstream
46+
4147 u.handler.ServeHTTP(rw, req)
4248}
pkg/upstream/file_test.go+6−1
@@ -7,6 +7,7 @@ import (
77 "net/http/httptest"
88 "os"
99
10+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1011 . "github.com/onsi/ginkgo"
1112 . "github.com/onsi/ginkgo/extensions/table"
1213 . "github.com/onsi/gomega"
@@ -42,10 +43,14 @@ var _ = Describe("File Server Suite", func() {
4243 DescribeTable("fileServer ServeHTTP",
4344 func(requestPath string, expectedResponseCode int, expectedBody string) {
4445 req := httptest.NewRequest("", requestPath, nil)
46+ req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
47+
4548 rw := httptest.NewRecorder()
4649 handler.ServeHTTP(rw, req)
4750
48− Expect(rw.Header().Get("GAP-Upstream-Address")).To(Equal(id))
51+ scope := middlewareapi.GetRequestScope(req)
52+ Expect(scope.Upstream).To(Equal(id))
53+
4954 Expect(rw.Code).To(Equal(expectedResponseCode))
5055 Expect(rw.Body.String()).To(Equal(expectedBody))
5156 },
pkg/upstream/http.go+7−1
@@ -8,6 +8,7 @@ import (
88 "strings"
99
1010 "github.com/mbland/hmacauth"
11+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1112 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
1213 "github.com/yhat/wsutil"
1314)
@@ -76,7 +77,12 @@ type httpUpstreamProxy struct {
7677// ServeHTTP proxies requests to the upstream provider while signing the
7778// request headers
7879func (h *httpUpstreamProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
79− rw.Header().Set("GAP-Upstream-Address", h.upstream)
80+ scope := middleware.GetRequestScope(req)
81+ // If scope is nil, this will panic.
82+ // A scope should always be injected before this handler is called.
83+ scope.Upstream = h.upstream
84+
85+ // TODO (@NickMeves) - Deprecate GAP-Signature & remove GAP-Auth
8086 if h.auth != nil {
8187 req.Header.Set("GAP-Auth", rw.Header().Get("GAP-Auth"))
8288 h.auth.SignRequest(req)
pkg/upstream/http_test.go+21−15
@@ -13,7 +13,9 @@ import (
1313 "strings"
1414 "time"
1515
16+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1617 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
18+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/middleware"
1719 . "github.com/onsi/ginkgo"
1820 . "github.com/onsi/ginkgo/extensions/table"
1921 . "github.com/onsi/gomega"
@@ -36,6 +38,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
3638 signatureData *options.SignatureData
3739 existingHeaders map[string]string
3840 expectedResponse testHTTPResponse
41+ expectedUpstream string
3942 errorHandler ProxyErrorHandler
4043 }
4144
@@ -50,6 +53,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
5053 req.Header.Add(key, value)
5154 }
5255
56+ req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
5357 rw := httptest.NewRecorder()
5458
5559 flush := options.Duration(1 * time.Second)
@@ -71,6 +75,9 @@ var _ = Describe("HTTP Upstream Suite", func() {
7175
7276 Expect(rw.Code).To(Equal(in.expectedResponse.code))
7377
78+ scope := middlewareapi.GetRequestScope(req)
79+ Expect(scope.Upstream).To(Equal(in.expectedUpstream))
80+
7481 // Delete extra headers that aren't relevant to tests
7582 testSanitizeResponseHeader(rw.Header())
7683 Expect(rw.Header()).To(Equal(in.expectedResponse.header))
@@ -97,7 +104,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
97104 expectedResponse: testHTTPResponse{
98105 code: 200,
99106 header: map[string][]string{
100− gapUpstream: {"default"},
101107 contentType: {applicationJSON},
102108 },
103109 request: testHTTPRequest{
@@ -109,6 +115,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
109115 RequestURI: "http://example.localhost/foo",
110116 },
111117 },
118+ expectedUpstream: "default",
112119 }),
113120 Entry("request a path with encoded slashes", &httpUpstreamTableInput{
114121 id: "encodedSlashes",
@@ -120,7 +127,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
120127 expectedResponse: testHTTPResponse{
121128 code: 200,
122129 header: map[string][]string{
123− gapUpstream: {"encodedSlashes"},
124130 contentType: {applicationJSON},
125131 },
126132 request: testHTTPRequest{
@@ -132,6 +138,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
132138 RequestURI: "http://example.localhost/foo%2fbar/?baz=1",
133139 },
134140 },
141+ expectedUpstream: "encodedSlashes",
135142 }),
136143 Entry("when the request has a body", &httpUpstreamTableInput{
137144 id: "requestWithBody",
@@ -143,7 +150,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
143150 expectedResponse: testHTTPResponse{
144151 code: 200,
145152 header: map[string][]string{
146− gapUpstream: {"requestWithBody"},
147153 contentType: {applicationJSON},
148154 },
149155 request: testHTTPRequest{
@@ -157,6 +163,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
157163 RequestURI: "http://example.localhost/withBody",
158164 },
159165 },
166+ expectedUpstream: "requestWithBody",
160167 }),
161168 Entry("when the upstream is unavailable", &httpUpstreamTableInput{
162169 id: "unavailableUpstream",
@@ -166,12 +173,11 @@ var _ = Describe("HTTP Upstream Suite", func() {
166173 body: []byte{},
167174 errorHandler: nil,
168175 expectedResponse: testHTTPResponse{
169− code: 502,
170− header: map[string][]string{
171− gapUpstream: {"unavailableUpstream"},
172− },
176+ code: 502,
177+ header: map[string][]string{},
173178 request: testHTTPRequest{},
174179 },
180+ expectedUpstream: "unavailableUpstream",
175181 }),
176182 Entry("when the upstream is unavailable and an error handler is set", &httpUpstreamTableInput{
177183 id: "withErrorHandler",
@@ -184,13 +190,12 @@ var _ = Describe("HTTP Upstream Suite", func() {
184190 rw.Write([]byte("error"))
185191 },
186192 expectedResponse: testHTTPResponse{
187− code: 502,
188− header: map[string][]string{
189− gapUpstream: {"withErrorHandler"},
190− },
193+ code: 502,
194+ header: map[string][]string{},
191195 raw: "error",
192196 request: testHTTPRequest{},
193197 },
198+ expectedUpstream: "withErrorHandler",
194199 }),
195200 Entry("with a signature", &httpUpstreamTableInput{
196201 id: "withSignature",
@@ -207,7 +212,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
207212 code: 200,
208213 header: map[string][]string{
209214 contentType: {applicationJSON},
210− gapUpstream: {"withSignature"},
211215 },
212216 request: testHTTPRequest{
213217 Method: "GET",
@@ -221,6 +225,7 @@ var _ = Describe("HTTP Upstream Suite", func() {
221225 RequestURI: "http://example.localhost/withSignature",
222226 },
223227 },
228+ expectedUpstream: "withSignature",
224229 }),
225230 Entry("with existing headers", &httpUpstreamTableInput{
226231 id: "existingHeaders",
@@ -236,7 +241,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
236241 expectedResponse: testHTTPResponse{
237242 code: 200,
238243 header: map[string][]string{
239− gapUpstream: {"existingHeaders"},
240244 contentType: {applicationJSON},
241245 },
242246 request: testHTTPRequest{
@@ -251,11 +255,13 @@ var _ = Describe("HTTP Upstream Suite", func() {
251255 RequestURI: "http://example.localhost/existingHeaders",
252256 },
253257 },
258+ expectedUpstream: "existingHeaders",
254259 }),
255260 )
256261
257262 It("ServeHTTP, when not passing a host header", func() {
258263 req := httptest.NewRequest("", "http://example.localhost/foo", nil)
264+ req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
259265 rw := httptest.NewRecorder()
260266
261267 flush := options.Duration(1 * time.Second)
@@ -383,7 +389,8 @@ var _ = Describe("HTTP Upstream Suite", func() {
383389 Expect(err).ToNot(HaveOccurred())
384390
385391 handler := newHTTPUpstreamProxy(upstream, u, nil, nil)
386− proxyServer = httptest.NewServer(handler)
392+
393+ proxyServer = httptest.NewServer(middleware.NewScope(false)(handler))
387394 })
388395
389396 AfterEach(func() {
@@ -414,7 +421,6 @@ var _ = Describe("HTTP Upstream Suite", func() {
414421 response, err := http.Get(fmt.Sprintf("http://%s", proxyServer.Listener.Addr().String()))
415422 Expect(err).ToNot(HaveOccurred())
416423 Expect(response.StatusCode).To(Equal(200))
417− Expect(response.Header.Get(gapUpstream)).To(Equal("websocketProxy"))
418424 })
419425 })
420426})
pkg/upstream/proxy_test.go+23−18
@@ -7,6 +7,7 @@ import (
77 "net/http"
88 "net/http/httptest"
99
10+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
1011 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
1112 . "github.com/onsi/ginkgo"
1213 . "github.com/onsi/ginkgo/extensions/table"
@@ -64,17 +65,24 @@ var _ = Describe("Proxy Suite", func() {
6465 type proxyTableInput struct {
6566 target string
6667 response testHTTPResponse
68+ upstream string
6769 }
6870
69− DescribeTable("Proxy ServerHTTP",
71+ DescribeTable("Proxy ServeHTTP",
7072 func(in *proxyTableInput) {
71− req := httptest.NewRequest("", in.target, nil)
73+ req := middlewareapi.AddRequestScope(
74+ httptest.NewRequest("", in.target, nil),
75+ &middlewareapi.RequestScope{},
76+ )
7277 rw := httptest.NewRecorder()
7378 // Don't mock the remote Address
7479 req.RemoteAddr = ""
7580
7681 upstreamServer.ServeHTTP(rw, req)
7782
83+ scope := middlewareapi.GetRequestScope(req)
84+ Expect(scope.Upstream).To(Equal(in.upstream))
85+
7886 Expect(rw.Code).To(Equal(in.response.code))
7987
8088 // Delete extra headers that aren't relevant to tests
@@ -99,7 +107,6 @@ var _ = Describe("Proxy Suite", func() {
99107 response: testHTTPResponse{
100108 code: 200,
101109 header: map[string][]string{
102− gapUpstream: {"http-backend"},
103110 contentType: {applicationJSON},
104111 },
105112 request: testHTTPRequest{
@@ -114,6 +121,7 @@ var _ = Describe("Proxy Suite", func() {
114121 RequestURI: "http://example.localhost/http/1234",
115122 },
116123 },
124+ upstream: "http-backend",
117125 }),
118126 Entry("with a request to the File backend", &proxyTableInput{
119127 target: "http://example.localhost/files/foo",
@@ -121,31 +129,29 @@ var _ = Describe("Proxy Suite", func() {
121129 code: 200,
122130 header: map[string][]string{
123131 contentType: {textPlainUTF8},
124− gapUpstream: {"file-backend"},
125132 },
126133 raw: "foo",
127134 },
135+ upstream: "file-backend",
128136 }),
129137 Entry("with a request to the Static backend", &proxyTableInput{
130138 target: "http://example.localhost/static/bar",
131139 response: testHTTPResponse{
132− code: 200,
133− header: map[string][]string{
134− gapUpstream: {"static-backend"},
135− },
136− raw: "Authenticated",
140+ code: 200,
141+ header: map[string][]string{},
142+ raw: "Authenticated",
137143 },
144+ upstream: "static-backend",
138145 }),
139146 Entry("with a request to the bad HTTP backend", &proxyTableInput{
140147 target: "http://example.localhost/bad-http/bad",
141148 response: testHTTPResponse{
142− code: 502,
143− header: map[string][]string{
144− gapUpstream: {"bad-http-backend"},
145− },
149+ code: 502,
150+ header: map[string][]string{},
146151 // This tests the error handler
147152 raw: "Proxy Error",
148153 },
154+ upstream: "bad-http-backend",
149155 }),
150156 Entry("with a request to the to an unregistered path", &proxyTableInput{
151157 target: "http://example.localhost/unregistered",
@@ -161,12 +167,11 @@ var _ = Describe("Proxy Suite", func() {
161167 Entry("with a request to the to backend registered to a single path", &proxyTableInput{
162168 target: "http://example.localhost/single-path",
163169 response: testHTTPResponse{
164− code: 200,
165− header: map[string][]string{
166− gapUpstream: {"single-path-backend"},
167− },
168− raw: "Authenticated",
170+ code: 200,
171+ header: map[string][]string{},
172+ raw: "Authenticated",
169173 },
174+ upstream: "single-path-backend",
170175 }),
171176 Entry("with a request to the to a subpath of a backend registered to a single path", &proxyTableInput{
172177 target: "http://example.localhost/single-path/unregistered",
pkg/upstream/static.go+12−2
@@ -3,6 +3,9 @@ package upstream
33import (
44 "fmt"
55 "net/http"
6+
7+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
8+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
69)
710
811const defaultStaticResponseCode = 200
@@ -24,9 +27,16 @@ type staticResponseHandler struct {
2427
2528// ServeHTTP serves a static response.
2629func (s *staticResponseHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
27− rw.Header().Set("GAP-Upstream-Address", s.upstream)
30+ scope := middleware.GetRequestScope(req)
31+ // If scope is nil, this will panic.
32+ // A scope should always be injected before this handler is called.
33+ scope.Upstream = s.upstream
34+
2835 rw.WriteHeader(s.code)
29− fmt.Fprintf(rw, "Authenticated")
36+ _, err := fmt.Fprintf(rw, "Authenticated")
37+ if err != nil {
38+ logger.Errorf("Error writing static response: %v", err)
39+ }
3040}
3141
3242// derefStaticCode returns the derefenced value, or the default if the value is nil
pkg/upstream/static_test.go+6−1
@@ -6,6 +6,7 @@ import (
66 "net/http"
77 "net/http/httptest"
88
9+ middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
910 . "github.com/onsi/ginkgo"
1011 . "github.com/onsi/ginkgo/extensions/table"
1112 . "github.com/onsi/gomega"
@@ -40,10 +41,14 @@ var _ = Describe("Static Response Suite", func() {
4041 handler := newStaticResponseHandler(id, code)
4142
4243 req := httptest.NewRequest("", in.requestPath, nil)
44+ req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
45+
4346 rw := httptest.NewRecorder()
4447 handler.ServeHTTP(rw, req)
4548
46− Expect(rw.Header().Get("GAP-Upstream-Address")).To(Equal(id))
49+ scope := middlewareapi.GetRequestScope(req)
50+ Expect(scope.Upstream).To(Equal(id))
51+
4752 Expect(rw.Code).To(Equal(in.expectedCode))
4853 Expect(rw.Body.String()).To(Equal(in.expectedBody))
4954 },
pkg/upstream/upstream_suite_test.go−1
@@ -59,7 +59,6 @@ const (
5959 acceptEncoding = "Accept-Encoding"
6060 applicationJSON = "application/json"
6161 textPlainUTF8 = "text/plain; charset=utf-8"
62− gapUpstream = "Gap-Upstream-Address"
6362 gapAuth = "Gap-Auth"
6463 gapSignature = "Gap-Signature"
6564)