diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e82c2..f3de8731 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.3 +- [#3505](https://github.com/oauth2-proxy/oauth2-proxy/pull/3505) fix: respect `skip-auth-preflight` in forward auth / API mode via `X-Forwarded-Method` + # V7.15.3 ## Release Highlights diff --git a/oauthproxy.go b/oauthproxy.go index f8dc5471..832fea4f 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -587,7 +587,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code i // IsAllowedRequest is used to check if auth should be skipped for this request func (p *OAuthProxy) IsAllowedRequest(req *http.Request) bool { - isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == http.MethodOptions + isPreflightRequestAllowed := p.skipAuthPreflight && requestutil.GetRequestMethod(req) == http.MethodOptions return isPreflightRequestAllowed || p.isAllowedRoute(req) || p.isTrustedIP(req) } diff --git a/pkg/requests/util/util.go b/pkg/requests/util/util.go index 568ebcc6..2bb9211a 100644 --- a/pkg/requests/util/util.go +++ b/pkg/requests/util/util.go @@ -9,9 +9,10 @@ import ( ) const ( - XForwardedProto = "X-Forwarded-Proto" - XForwardedHost = "X-Forwarded-Host" - XForwardedURI = "X-Forwarded-Uri" + XForwardedProto = "X-Forwarded-Proto" + XForwardedHost = "X-Forwarded-Host" + XForwardedURI = "X-Forwarded-Uri" + XForwardedMethod = "X-Forwarded-Method" ) // GetRequestProto returns the request scheme or X-Forwarded-Proto if present @@ -45,6 +46,16 @@ func GetRequestURI(req *http.Request) string { return uri } +// GetRequestMethod returns the request method or X-Forwarded-Method if present +// and the request came from a trusted reverse proxy. +func GetRequestMethod(req *http.Request) string { + method := req.Header.Get(XForwardedMethod) + if !CanTrustForwardedHeaders(req) || method == "" { + return req.Method + } + return method +} + // GetRequestPath returns the request URI or X-Forwarded-Uri if present and the // request came from a trusted reverse proxy but always strips the query // parameters and fragment suffixes and only returns the pure path. diff --git a/pkg/requests/util/util_test.go b/pkg/requests/util/util_test.go index c4185b35..529dd32a 100644 --- a/pkg/requests/util/util_test.go +++ b/pkg/requests/util/util_test.go @@ -200,6 +200,42 @@ var _ = Describe("Util Suite", func() { }) }) + Context("GetRequestMethod", func() { + Context("trusted forwarded headers are disabled", func() { + BeforeEach(func() { + req = middleware.AddRequestScope(req, &middleware.RequestScope{}) + }) + + It("returns the request method", func() { + Expect(util.GetRequestMethod(req)).To(Equal(http.MethodGet)) + }) + + It("ignores X-Forwarded-Method and returns the request method", func() { + req.Header.Add("X-Forwarded-Method", http.MethodOptions) + Expect(util.GetRequestMethod(req)).To(Equal(http.MethodGet)) + }) + }) + + Context("trusted forwarded headers are enabled", func() { + BeforeEach(func() { + req.RemoteAddr = "127.0.0.1:4180" + req = middleware.AddRequestScope(req, &middleware.RequestScope{ + ReverseProxy: true, + TrustedProxies: trustedProxies, + }) + }) + + It("returns the request method if X-Forwarded-Method is not present", func() { + Expect(util.GetRequestMethod(req)).To(Equal(http.MethodGet)) + }) + + It("returns the X-Forwarded-Method when present", func() { + req.Header.Add("X-Forwarded-Method", http.MethodOptions) + Expect(util.GetRequestMethod(req)).To(Equal(http.MethodOptions)) + }) + }) + }) + Context("CanTrustForwardedHeaders", func() { It("returns false when no scope is present", func() { Expect(util.CanTrustForwardedHeaders(req)).To(BeFalse())