From 303f2e1208c04ed57ab271f58dd9799e49d7c5e7 Mon Sep 17 00:00:00 2001 From: fidoriel <49869342+fidoriel@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:22:10 +0100 Subject: [PATCH 1/2] Also consider nginx original header on decision api --- api/decision.go | 23 ++++++++++++++++++----- api/decision_test.go | 31 +++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/api/decision.go b/api/decision.go index d1cfbb37dd..7dc75f07b4 100644 --- a/api/decision.go +++ b/api/decision.go @@ -5,6 +5,7 @@ package api import ( "net/http" + "net/url" "strings" "github.com/ory/oathkeeper/pipeline/authn" @@ -21,6 +22,8 @@ const ( xForwardedProto = "X-Forwarded-Proto" xForwardedHost = "X-Forwarded-Host" xForwardedUri = "X-Forwarded-Uri" + xOriginalMethod = "X-Original-Method" + xOriginalUrl = "X-Original-Url" ) type decisionHandlerRegistry interface { @@ -41,11 +44,21 @@ func NewJudgeHandler(r decisionHandlerRegistry) *DecisionHandler { func (h *DecisionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { if len(r.URL.Path) >= len(DecisionPath) && r.URL.Path[:len(DecisionPath)] == DecisionPath { - r.Method = x.OrDefaultString(r.Header.Get(xForwardedMethod), r.Method) - r.URL.Scheme = x.OrDefaultString(r.Header.Get(xForwardedProto), - x.IfThenElseString(r.TLS != nil, "https", "http")) - r.URL.Host = x.OrDefaultString(r.Header.Get(xForwardedHost), r.Host) - r.URL.Path = x.OrDefaultString(strings.SplitN(r.Header.Get(xForwardedUri), "?", 2)[0], r.URL.Path[len(DecisionPath):]) + r.Method = x.OrDefaultString(r.Header.Get(xForwardedMethod), x.OrDefaultString(r.Header.Get(xOriginalMethod), r.Method)) + + originalURL := r.Header.Get(xOriginalUrl) + if originalURL != "" { + if parsedURL, err := url.Parse(originalURL); err == nil { + r.URL.Scheme = parsedURL.Scheme + r.URL.Host = parsedURL.Host + r.URL.Path = parsedURL.Path + } + } else { + r.URL.Scheme = x.OrDefaultString(r.Header.Get(xForwardedProto), + x.IfThenElseString(r.TLS != nil, "https", "http")) + r.URL.Host = x.OrDefaultString(r.Header.Get(xForwardedHost), r.Host) + r.URL.Path = x.OrDefaultString(strings.SplitN(r.Header.Get(xForwardedUri), "?", 2)[0], r.URL.Path[len(DecisionPath):]) + } h.decisions(w, r) } else { diff --git a/api/decision_test.go b/api/decision_test.go index d4c6d07236..376315b650 100644 --- a/api/decision_test.go +++ b/api/decision_test.go @@ -396,7 +396,7 @@ func TestDecisionAPIHeaderUsage(t *testing.T) { }, }, { - name: "all arguments are taken from the headers", + name: "all arguments are taken from the `forwarded` headers", expectedUrl: &url.URL{Scheme: "https", Host: "test.dev", Path: "/bar"}, expectedMethod: "POST", transform: func(req *http.Request) { @@ -407,7 +407,16 @@ func TestDecisionAPIHeaderUsage(t *testing.T) { }, }, { - name: "argument from the headers doesn't get url encoded", + name: "all arguments are taken from the `original` headers", + expectedUrl: &url.URL{Scheme: "https", Host: "test.dev", Path: "/bar"}, + expectedMethod: "POST", + transform: func(req *http.Request) { + req.Header.Add("X-Original-Method", "POST") + req.Header.Add("X-Original-Url", "https://test.dev/bar") + }, + }, + { + name: "argument from the `forwarded` headers doesn't get url encoded", expectedUrl: &url.URL{Scheme: "https", Host: "test.dev", Path: "/bar"}, expectedMethod: "POST", transform: func(req *http.Request) { @@ -417,6 +426,15 @@ func TestDecisionAPIHeaderUsage(t *testing.T) { req.Header.Add("X-Forwarded-Uri", "/bar?this=is&a=test") }, }, + { + name: "argument from the `original` headers doesn't get url encoded", + expectedUrl: &url.URL{Scheme: "https", Host: "test.dev", Path: "/bar"}, + expectedMethod: "POST", + transform: func(req *http.Request) { + req.Header.Add("X-Original-Method", "POST") + req.Header.Add("X-Original-Url", "https://test.dev/bar?this=is&a=test") + }, + }, { name: "only scheme is taken from the headers", expectedUrl: &url.URL{Scheme: "https", Host: defaultUrl.Host, Path: defaultUrl.Path}, @@ -425,6 +443,15 @@ func TestDecisionAPIHeaderUsage(t *testing.T) { req.Header.Add("X-Forwarded-Proto", "https") }, }, + { + name: "scheme is taken from the `forwarded` headers and url from `original` headers", + expectedUrl: &url.URL{Scheme: "https", Host: defaultUrl.Host, Path: defaultUrl.Path}, + expectedMethod: defaultMethod, + transform: func(req *http.Request) { + req.Header.Add("X-Forwarded-Proto", "https") + req.Header.Add("X-Original-Url", "https://test.dev/bar") + }, + }, { name: "only method is taken from the headers", expectedUrl: defaultUrl, From 10ecfa93250c848b6addca6b86ebd7e540523883 Mon Sep 17 00:00:00 2001 From: fidoriel <49869342+fidoriel@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:45:57 +0100 Subject: [PATCH 2/2] fix test --- api/decision_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/decision_test.go b/api/decision_test.go index 376315b650..b65c659155 100644 --- a/api/decision_test.go +++ b/api/decision_test.go @@ -444,11 +444,11 @@ func TestDecisionAPIHeaderUsage(t *testing.T) { }, }, { - name: "scheme is taken from the `forwarded` headers and url from `original` headers", + name: "schemethodme is taken from the `forwarded` headers and url from `original` headers", expectedUrl: &url.URL{Scheme: "https", Host: defaultUrl.Host, Path: defaultUrl.Path}, expectedMethod: defaultMethod, transform: func(req *http.Request) { - req.Header.Add("X-Forwarded-Proto", "https") + req.Header.Add("X-Forwarded-Method", "POST") req.Header.Add("X-Original-Url", "https://test.dev/bar") }, },