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

pkg/middleware/basic_session.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 
 	"github.com/justinas/alice"
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
@@ -23,7 +24,7 @@ func NewBasicAuthSessionLoader(validator basic.Validator) alice.Constructor {
 // If a session was loaded by a previous handler, it will not be replaced.
 func loadBasicAuthSession(validator basic.Validator, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		scope := GetRequestScope(req)
+		scope := middlewareapi.GetRequestScope(req)
 		// If scope is nil, this will panic.
 		// A scope should always be injected before this handler is called.
 		if scope.Session != nil {

pkg/middleware/basic_session_test.go

@@ -1,7 +1,6 @@
 package middleware
 
 import (
-	"context"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -40,8 +39,7 @@ var _ = Describe("Basic Auth Session Suite", func() {
 				// Set up the request with the authorization header and a request scope
 				req := httptest.NewRequest("", "/", nil)
 				req.Header.Set("Authorization", in.authorizationHeader)
-				contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-				req = req.WithContext(contextWithScope)
+				req = middlewareapi.AddRequestScope(req, scope)
 
 				rw := httptest.NewRecorder()
 
@@ -57,7 +55,7 @@ var _ = Describe("Basic Auth Session Suite", func() {
 				// from the scope
 				var gotSession *sessionsapi.SessionState
 				handler := NewBasicAuthSessionLoader(validator)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-					gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
+					gotSession = middlewareapi.GetRequestScope(r).Session
 				}))
 				handler.ServeHTTP(rw, req)
 

pkg/middleware/headers.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 
 	"github.com/justinas/alice"
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/header"
 )
@@ -61,7 +62,7 @@ func newRequestHeaderInjector(headers []options.Header) (alice.Constructor, erro
 
 func injectRequestHeaders(injector header.Injector, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		scope := GetRequestScope(req)
+		scope := middlewareapi.GetRequestScope(req)
 
 		// If scope is nil, this will panic.
 		// A scope should always be injected before this handler is called.
@@ -92,7 +93,7 @@ func newResponseHeaderInjector(headers []options.Header) (alice.Constructor, err
 
 func injectResponseHeaders(injector header.Injector, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		scope := GetRequestScope(req)
+		scope := middlewareapi.GetRequestScope(req)
 
 		// If scope is nil, this will panic.
 		// A scope should always be injected before this handler is called.

pkg/middleware/headers_test.go

@@ -1,7 +1,6 @@
 package middleware
 
 import (
-	"context"
 	"encoding/base64"
 	"net/http"
 	"net/http/httptest"
@@ -31,8 +30,7 @@ var _ = Describe("Headers Suite", func() {
 
 			// Set up the request with a request scope
 			req := httptest.NewRequest("", "/", nil)
-			contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-			req = req.WithContext(contextWithScope)
+			req = middlewareapi.AddRequestScope(req, scope)
 			req.Header = in.initialHeaders.Clone()
 
 			rw := httptest.NewRecorder()
@@ -218,8 +216,7 @@ var _ = Describe("Headers Suite", func() {
 
 			// Set up the request with a request scope
 			req := httptest.NewRequest("", "/", nil)
-			contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-			req = req.WithContext(contextWithScope)
+			req = middlewareapi.AddRequestScope(req, scope)
 
 			rw := httptest.NewRecorder()
 			for key, values := range in.initialHeaders {

pkg/middleware/jwt_session.go

@@ -37,7 +37,7 @@ type jwtSessionLoader struct {
 // If a session was loaded by a previous handler, it will not be replaced.
 func (j *jwtSessionLoader) loadSession(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		scope := GetRequestScope(req)
+		scope := middlewareapi.GetRequestScope(req)
 		// If scope is nil, this will panic.
 		// A scope should always be injected before this handler is called.
 		if scope.Session != nil {

pkg/middleware/jwt_session_test.go

@@ -103,8 +103,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=`
 				// Set up the request with the authorization header and a request scope
 				req := httptest.NewRequest("", "/", nil)
 				req.Header.Set("Authorization", in.authorizationHeader)
-				contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-				req = req.WithContext(contextWithScope)
+				req = middlewareapi.AddRequestScope(req, scope)
 
 				rw := httptest.NewRecorder()
 
@@ -116,7 +115,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=`
 				// from the scope
 				var gotSession *sessionsapi.SessionState
 				handler := NewJwtSessionLoader(sessionLoaders)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-					gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
+					gotSession = middlewareapi.GetRequestScope(r).Session
 				}))
 				handler.ServeHTTP(rw, req)
 

pkg/middleware/scope.go

@@ -1,39 +1,20 @@
 package middleware
 
 import (
-	"context"
 	"net/http"
 
 	"github.com/justinas/alice"
 	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
-	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 )
 
-type scopeKey string
-
-// requestScopeKey uses a typed string to reduce likelihood of clashing
-// with other context keys
-const requestScopeKey scopeKey = "request-scope"
-
-func NewScope(opts *options.Options) alice.Constructor {
+func NewScope(reverseProxy bool) alice.Constructor {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			scope := &middlewareapi.RequestScope{
-				ReverseProxy: opts.ReverseProxy,
+				ReverseProxy: reverseProxy,
 			}
-			contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-			requestWithScope := req.WithContext(contextWithScope)
-			next.ServeHTTP(rw, requestWithScope)
+			req = middlewareapi.AddRequestScope(req, scope)
+			next.ServeHTTP(rw, req)
 		})
 	}
 }
-
-// GetRequestScope returns the current request scope from the given request
-func GetRequestScope(req *http.Request) *middlewareapi.RequestScope {
-	scope := req.Context().Value(requestScopeKey)
-	if scope == nil {
-		return nil
-	}
-
-	return scope.(*middlewareapi.RequestScope)
-}

pkg/middleware/scope_test.go

@@ -1,7 +1,6 @@
 package middleware
 
 import (
-	"context"
 	"net/http"
 	"net/http/httptest"
 
@@ -21,73 +20,49 @@ var _ = Describe("Scope Suite", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			rw = httptest.NewRecorder()
-
-			handler := NewScope()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				nextRequest = r
-				w.WriteHeader(200)
-			}))
-			handler.ServeHTTP(rw, request)
 		})
 
-		It("does not add a scope to the original request", func() {
-			Expect(request.Context().Value(requestScopeKey)).To(BeNil())
-		})
-
-		It("cannot load a scope from the original request using GetRequestScope", func() {
-			Expect(GetRequestScope(request)).To(BeNil())
-		})
-
-		It("adds a scope to the request for the next handler", func() {
-			Expect(nextRequest.Context().Value(requestScopeKey)).ToNot(BeNil())
-		})
-
-		It("can load a scope from the next handler's request using GetRequestScope", func() {
-			Expect(GetRequestScope(nextRequest)).ToNot(BeNil())
-		})
-	})
-
-	Context("GetRequestScope", func() {
-		var request *http.Request
-
-		BeforeEach(func() {
-			var err error
-			request, err = http.NewRequest("", "http://127.0.0.1/", nil)
-			Expect(err).ToNot(HaveOccurred())
-		})
-
-		Context("with a scope", func() {
-			var scope *middlewareapi.RequestScope
-
+		Context("ReverseProxy is false", func() {
 			BeforeEach(func() {
-				scope = &middlewareapi.RequestScope{}
-				contextWithScope := context.WithValue(request.Context(), requestScopeKey, scope)
-				request = request.WithContext(contextWithScope)
+				handler := NewScope(false)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					nextRequest = r
+					w.WriteHeader(200)
+				}))
+				handler.ServeHTTP(rw, request)
 			})
 
-			It("returns the scope", func() {
-				s := GetRequestScope(request)
-				Expect(s).ToNot(BeNil())
-				Expect(s).To(Equal(scope))
+			It("does not add a scope to the original request", func() {
+				Expect(request.Context().Value(middlewareapi.RequestScopeKey)).To(BeNil())
 			})
 
-			Context("if the scope is then modified", func() {
-				BeforeEach(func() {
-					Expect(scope.SaveSession).To(BeFalse())
-					scope.SaveSession = true
-				})
+			It("cannot load a scope from the original request using GetRequestScope", func() {
+				Expect(middlewareapi.GetRequestScope(request)).To(BeNil())
+			})
 
-				It("returns the updated session", func() {
-					s := GetRequestScope(request)
-					Expect(s).ToNot(BeNil())
-					Expect(s).To(Equal(scope))
-					Expect(s.SaveSession).To(BeTrue())
-				})
+			It("adds a scope to the request for the next handler", func() {
+				Expect(nextRequest.Context().Value(middlewareapi.RequestScopeKey)).ToNot(BeNil())
+			})
+
+			It("can load a scope from the next handler's request using GetRequestScope", func() {
+				scope := middlewareapi.GetRequestScope(nextRequest)
+				Expect(scope).ToNot(BeNil())
+				Expect(scope.ReverseProxy).To(BeFalse())
 			})
 		})
 
-		Context("without a scope", func() {
-			It("returns nil", func() {
-				Expect(GetRequestScope(request)).To(BeNil())
+		Context("ReverseProxy is true", func() {
+			BeforeEach(func() {
+				handler := NewScope(true)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					nextRequest = r
+					w.WriteHeader(200)
+				}))
+				handler.ServeHTTP(rw, request)
+			})
+
+			It("return a scope where the ReverseProxy field is true", func() {
+				scope := middlewareapi.GetRequestScope(nextRequest)
+				Expect(scope).ToNot(BeNil())
+				Expect(scope.ReverseProxy).To(BeTrue())
 			})
 		})
 	})

pkg/middleware/stored_session.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/justinas/alice"
+	middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
 	sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
 )
@@ -59,7 +60,7 @@ type storedSessionLoader struct {
 // If a session was loader by a previous handler, it will not be replaced.
 func (s *storedSessionLoader) loadSession(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		scope := GetRequestScope(req)
+		scope := middlewareapi.GetRequestScope(req)
 		// If scope is nil, this will panic.
 		// A scope should always be injected before this handler is called.
 		if scope.Session != nil {

pkg/middleware/stored_session_test.go

@@ -104,8 +104,7 @@ var _ = Describe("Stored Session Suite", func() {
 				// Set up the request with the request headesr and a request scope
 				req := httptest.NewRequest("", "/", nil)
 				req.Header = in.requestHeaders
-				contextWithScope := context.WithValue(req.Context(), requestScopeKey, scope)
-				req = req.WithContext(contextWithScope)
+				req = middlewareapi.AddRequestScope(req, scope)
 
 				rw := httptest.NewRecorder()
 
@@ -120,7 +119,7 @@ var _ = Describe("Stored Session Suite", func() {
 				// from the scope
 				var gotSession *sessionsapi.SessionState
 				handler := NewStoredSessionLoader(opts)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-					gotSession = r.Context().Value(requestScopeKey).(*middlewareapi.RequestScope).Session
+					gotSession = middlewareapi.GetRequestScope(r).Session
 				}))
 				handler.ServeHTTP(rw, req)