Skip to content

Commit

Permalink
Use constants for X-Forwarded headers (#522)
Browse files Browse the repository at this point in the history 
* Replace X-Forwarded header names with constants
  • Loading branch information
@p53
p53 authored Nov 22, 2024
1 parent 7ff5ae3 commit 45150cf
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 60 deletions.
22 changes: 11 additions & 11 deletions e2e/e2e_uma_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,10 +400,10 @@ var _ = Describe("UMA Code Flow, NOPROXY authorization with method scope", func(
var err error
rClient := resty.New()
rClient.SetHeaders(map[string]string{
"X-Forwarded-Proto": "http",
"X-Forwarded-Host": strings.Split(proxyAddress, "//")[1],
"X-Forwarded-URI": umaMethodAllowedPath,
"X-Forwarded-Method": "GET",
constant.HeaderXForwardedProto: "http",
constant.HeaderXForwardedHost: strings.Split(proxyAddress, "//")[1],
constant.HeaderXForwardedURI: umaMethodAllowedPath,
constant.HeaderXForwardedMethod: "GET",
})
resp := codeFlowLogin(rClient, proxyAddress, http.StatusOK, testUser, testPass)
Expect(resp.Header().Get(constant.AuthorizationHeader)).ToNot(BeEmpty())
Expand All @@ -422,10 +422,10 @@ var _ = Describe("UMA Code Flow, NOPROXY authorization with method scope", func(
It("should be forbidden", func(_ context.Context) {
rClient := resty.New()
rClient.SetHeaders(map[string]string{
"X-Forwarded-Proto": "http",
"X-Forwarded-Host": strings.Split(proxyAddress, "//")[1],
"X-Forwarded-URI": umaMethodAllowedPath,
"X-Forwarded-Method": "POST",
constant.HeaderXForwardedProto: "http",
constant.HeaderXForwardedHost: strings.Split(proxyAddress, "//")[1],
constant.HeaderXForwardedURI: umaMethodAllowedPath,
constant.HeaderXForwardedMethod: "POST",
})
resp := codeFlowLogin(rClient, proxyAddress, http.StatusForbidden, testUser, testPass)
Expect(resp.Header().Get(constant.AuthorizationHeader)).To(BeEmpty())
Expand All @@ -436,9 +436,9 @@ var _ = Describe("UMA Code Flow, NOPROXY authorization with method scope", func(
It("should be forbidden", func(_ context.Context) {
rClient := resty.New()
rClient.SetHeaders(map[string]string{
"X-Forwarded-Proto": "http",
"X-Forwarded-Host": strings.Split(proxyAddress, "//")[1],
"X-Forwarded-URI": umaMethodAllowedPath,
constant.HeaderXForwardedProto: "http",
constant.HeaderXForwardedHost: strings.Split(proxyAddress, "//")[1],
constant.HeaderXForwardedURI: umaMethodAllowedPath,
})
resp := codeFlowLogin(rClient, proxyAddress, http.StatusForbidden, testUser, testPass)
Expect(resp.Header().Get(constant.AuthorizationHeader)).To(BeEmpty())
Expand Down
11 changes: 7 additions & 4 deletions pkg/constant/constant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,13 @@ const (

_ contextKey = iota
ContextScopeName
HeaderXForwardedFor = "X-Forwarded-For"
HeaderXForwardedHost = "X-Forwarded-Host"
HeaderXRealIP = "X-Real-IP"
HeaderXHMAC = "X-HMAC-SHA256"
HeaderXForwardedFor = "X-Forwarded-For"
HeaderXForwardedHost = "X-Forwarded-Host"
HeaderXRealIP = "X-Real-IP"
HeaderXForwardedProto = "X-Forwarded-Proto"
HeaderXForwardedURI = "X-Forwarded-URI"
HeaderXForwardedMethod = "X-Forwarded-Method"
HeaderXHMAC = "X-HMAC-SHA256"

DurationType = "time.Duration"

Expand Down
4 changes: 2 additions & 2 deletions pkg/keycloak/proxy/middleware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func authorizationMiddleware(
if enableUmaMethodScope {
methSc := constant.UmaMethodScope + req.Method
if noProxy {
xForwardedMethod := req.Header.Get("X-Forwarded-Method")
xForwardedMethod := req.Header.Get(constant.HeaderXForwardedMethod)
if xForwardedMethod == "" {
scope.Logger.Error(apperrors.ErrForwardAuthMissingHeaders.Error())
accessForbidden(wrt, req)
Expand All @@ -107,7 +107,7 @@ func authorizationMiddleware(

authzPath := req.URL.Path
if noProxy {
authzPath = req.Header.Get("X-Forwarded-Uri")
authzPath = req.Header.Get(constant.HeaderXForwardedURI)
if authzPath == "" {
scope.Logger.Error(apperrors.ErrForwardAuthMissingHeaders.Error())
accessForbidden(wrt, req)
Expand Down
2 changes: 1 addition & 1 deletion pkg/proxy/cookie/cookies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (cm *Manager) DropStateParameterCookie(req *http.Request, wrt http.Response
requestURI := req.URL.RequestURI()

if cm.NoProxy && !cm.NoRedirects {
xReqURI := req.Header.Get("X-Forwarded-Uri")
xReqURI := req.Header.Get(constant.HeaderXForwardedURI)
requestURI = xReqURI
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/proxy/core/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ func RedirectToAuthorization(
url := utils.WithOAuthURI(baseURI, oAuthURI)(constant.AuthorizationURL + authQuery)

if noProxy && !noRedirects {
xForwardedHost := req.Header.Get("X-Forwarded-Host")
xProto := req.Header.Get("X-Forwarded-Proto")
xForwardedHost := req.Header.Get(constant.HeaderXForwardedHost)
xProto := req.Header.Get(constant.HeaderXForwardedProto)

if xForwardedHost == "" || xProto == "" {
logger.Error(apperrors.ErrForwardAuthMissingHeaders.Error())
Expand Down
4 changes: 2 additions & 2 deletions pkg/proxy/handlers/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ func GetRedirectionURL(
var host string

if noProxy && !noRedirects {
scheme = req.Header.Get("X-Forwarded-Proto")
host = req.Header.Get("X-Forwarded-Host")
scheme = req.Header.Get(constant.HeaderXForwardedProto)
host = req.Header.Get(constant.HeaderXForwardedHost)
} else {
// need to determine the scheme, cx.Request.URL.Scheme doesn't have it, best way is to default
// and then check for TLS
Expand Down
6 changes: 3 additions & 3 deletions pkg/proxy/middleware/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func ProxyMiddleware(
}

// @step: add the proxy forwarding headers
req.Header.Set("X-Real-IP", utils.RealIP(req))
req.Header.Set(constant.HeaderXRealIP, utils.RealIP(req))
if xff := req.Header.Get(constant.HeaderXForwardedFor); xff == "" {
req.Header.Set(constant.HeaderXForwardedFor, utils.RealIP(req))
}
Expand Down Expand Up @@ -379,11 +379,11 @@ func ForwardAuthMiddleware(logger *zap.Logger, oAuthURI string) func(http.Handle

return http.HandlerFunc(func(wrt http.ResponseWriter, req *http.Request) {
if !strings.Contains(req.URL.Path, oAuthURI) { // this condition is here only because of tests to work
if forwardedPath := req.Header.Get("X-Forwarded-Uri"); forwardedPath != "" {
if forwardedPath := req.Header.Get(constant.HeaderXForwardedURI); forwardedPath != "" {
req.URL.Path = forwardedPath
req.URL.RawPath = forwardedPath
}
if forwardedMethod := req.Header.Get("X-Forwarded-Method"); forwardedMethod != "" {
if forwardedMethod := req.Header.Get(constant.HeaderXForwardedMethod); forwardedMethod != "" {
req.Method = forwardedMethod
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/proxy/middleware/security.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func SecurityMiddleware(
ContentSecurityPolicy: contentSecurityPolicy,
ContentTypeNosniff: contentTypeNosniff,
FrameDeny: frameDeny,
SSLProxyHeaders: map[string]string{"X-Forwarded-Proto": "https"},
SSLProxyHeaders: map[string]string{constant.HeaderXForwardedProto: "https"},
SSLRedirect: sslRedirect,
})

Expand Down
4 changes: 2 additions & 2 deletions pkg/testsuite/fake_proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,8 @@ func makeTestCodeFlowLogin(location string, xforwarded bool) (*http.Response, []
}

if xforwarded {
req.Header.Add("X-Forwarded-Host", uri.Host)
req.Header.Add("X-Forwarded-Proto", uri.Scheme)
req.Header.Add(constant.HeaderXForwardedHost, uri.Host)
req.Header.Add(constant.HeaderXForwardedProto, uri.Scheme)
}

if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/testsuite/middleware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestMetricsMiddleware(t *testing.T) {
{
URI: uri,
Headers: map[string]string{
"X-Forwarded-For": "10.0.0.1",
constant.HeaderXForwardedFor: "10.0.0.1",
},
ExpectedCode: http.StatusForbidden,
},
Expand Down Expand Up @@ -2487,19 +2487,19 @@ func TestLogRealIP(t *testing.T) {
ExpectedIP: "127.0.0.1",
},
{
Headers: map[string]string{"X-Forwarded-For": "192.168.1.1"},
Headers: map[string]string{constant.HeaderXForwardedFor: "192.168.1.1"},
ExpectedIP: "192.168.1.1",
},
{
Headers: map[string]string{"X-Forwarded-For": "192.168.1.1, 192.168.1.2"},
Headers: map[string]string{constant.HeaderXForwardedFor: "192.168.1.1, 192.168.1.2"},
ExpectedIP: "192.168.1.1",
},
{
Headers: map[string]string{"X-Real-Ip": "10.0.0.1"},
Headers: map[string]string{constant.HeaderXRealIP: "10.0.0.1"},
ExpectedIP: "10.0.0.1",
},
{
Headers: map[string]string{"X-Forwarded-For": "192.168.1.1", "X-Real-Ip": "10.0.0.1"},
Headers: map[string]string{constant.HeaderXForwardedFor: "192.168.1.1", constant.HeaderXRealIP: "10.0.0.1"},
ExpectedIP: "192.168.1.1",
},
}
Expand Down
77 changes: 57 additions & 20 deletions pkg/testsuite/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1352,8 +1352,8 @@ func TestNoProxy(t *testing.T) {
},
ExpectedLocation: "https://thiswillbereplaced/oauth",
Headers: map[string]string{
"X-Forwarded-Host": "thiswillbereplaced",
"X-Forwarded-Proto": "https",
constant.HeaderXForwardedHost: "thiswillbereplaced",
constant.HeaderXForwardedProto: "https",
},
},
},
Expand Down Expand Up @@ -1426,8 +1426,8 @@ func TestNoProxy(t *testing.T) {
assert.Equal(t, "", body)
},
Headers: map[string]string{
"X-Forwarded-Uri": "/private",
"X-Forwarded-Method": "POST",
constant.HeaderXForwardedURI: "/private",
constant.HeaderXForwardedMethod: "POST",
},
},
{
Expand All @@ -1441,8 +1441,8 @@ func TestNoProxy(t *testing.T) {
assert.Equal(t, "", body)
},
Headers: map[string]string{
"X-Forwarded-Uri": "/private",
"X-Forwarded-Method": "DELETE",
constant.HeaderXForwardedURI: "/private",
constant.HeaderXForwardedMethod: "DELETE",
},
},
},
Expand Down Expand Up @@ -1493,7 +1493,7 @@ func TestProxyProtocol(t *testing.T) {
HasToken: true,
ExpectedProxy: true,
ExpectedProxyHeaders: map[string]string{
"X-Forwarded-For": "127.0.0.1",
constant.HeaderXForwardedFor: "127.0.0.1",
},
ExpectedCode: http.StatusOK,
},
Expand All @@ -1503,7 +1503,7 @@ func TestProxyProtocol(t *testing.T) {
ProxyProtocol: "189.10.10.1",
ExpectedProxy: true,
ExpectedProxyHeaders: map[string]string{
"X-Forwarded-For": "189.10.10.1",
constant.HeaderXForwardedFor: "189.10.10.1",
},
ExpectedCode: http.StatusOK,
},
Expand All @@ -1527,8 +1527,8 @@ func TestXForwarded(t *testing.T) {
HasToken: true,
ExpectedProxy: true,
ExpectedProxyHeaders: map[string]string{
"X-Forwarded-For": "127.0.0.1",
"X-Real-IP": "127.0.0.1",
constant.HeaderXForwardedFor: "127.0.0.1",
constant.HeaderXRealIP: "127.0.0.1",
},
ExpectedCode: http.StatusOK,
},
Expand All @@ -1544,11 +1544,11 @@ func TestXForwarded(t *testing.T) {
HasToken: true,
ExpectedProxy: true,
Headers: map[string]string{
"X-Forwarded-For": "189.10.10.1",
constant.HeaderXForwardedFor: "189.10.10.1",
},
ExpectedProxyHeaders: map[string]string{
"X-Forwarded-For": "189.10.10.1",
"X-Real-IP": "189.10.10.1",
constant.HeaderXForwardedFor: "189.10.10.1",
constant.HeaderXRealIP: "189.10.10.1",
},
ExpectedCode: http.StatusOK,
},
Expand All @@ -1564,11 +1564,48 @@ func TestXForwarded(t *testing.T) {
HasToken: true,
ExpectedProxy: true,
Headers: map[string]string{
"X-Real-IP": "189.10.10.1",
constant.HeaderXRealIP: "189.10.10.1",
},
ExpectedProxyHeaders: map[string]string{
"X-Forwarded-For": "189.10.10.1",
"X-Real-IP": "189.10.10.1",
constant.HeaderXForwardedFor: "189.10.10.1",
constant.HeaderXRealIP: "189.10.10.1",
},
ExpectedCode: http.StatusOK,
},
},
},
{
Name: "TestEmptyXForwardedHost",
ProxySettings: func(_ *config.Config) {
},
ExecutionSettings: []fakeRequest{
{
URI: FakeAuthAllURL + FakeTestURL,
HasToken: true,
ExpectedProxy: true,
ExpectedProxyHeadersValidator: map[string]func(*testing.T, *config.Config, string){
constant.HeaderXForwardedHost: func(t *testing.T, _ *config.Config, value string) {
assert.Contains(t, value, "127.0.0.1")
},
},
ExpectedCode: http.StatusOK,
},
},
},
{
Name: "TestXForwardedHostPresent",
ProxySettings: func(_ *config.Config) {
},
ExecutionSettings: []fakeRequest{
{
URI: FakeAuthAllURL + FakeTestURL,
HasToken: true,
ExpectedProxy: true,
Headers: map[string]string{
constant.HeaderXForwardedHost: "189.10.10.1",
},
ExpectedProxyHeaders: map[string]string{
constant.HeaderXForwardedHost: "189.10.10.1",
},
ExpectedCode: http.StatusOK,
},
Expand Down Expand Up @@ -1662,10 +1699,10 @@ func TestTokenEncryption(t *testing.T) {
ExpectedProxy: true,
Redirects: true,
ExpectedProxyHeaders: map[string]string{
"X-Auth-Email": "[email protected]",
"X-Auth-Userid": "rjayawardene",
"X-Auth-Username": "rjayawardene",
"X-Forwarded-For": "127.0.0.1",
"X-Auth-Email": "[email protected]",
"X-Auth-Userid": "rjayawardene",
"X-Auth-Username": "rjayawardene",
constant.HeaderXForwardedFor: "127.0.0.1",
},
ExpectedCode: http.StatusOK,
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ func GetRequestHostURL(req *http.Request) string {
}

redirect := fmt.Sprintf("%s://%s",
DefaultTo(req.Header.Get("X-Forwarded-Proto"), scheme),
DefaultTo(req.Header.Get("X-Forwarded-Host"), req.Host))
DefaultTo(req.Header.Get(constant.HeaderXForwardedProto), scheme),
DefaultTo(req.Header.Get(constant.HeaderXForwardedHost), req.Host))

return redirect
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/utils/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ func TestGetRequestHostURL(t *testing.T) {
}{
{
Expected: "http://www.test.com",
Headers: map[string]string{"X-Forwarded-Host": "www.test.com"},
Headers: map[string]string{constant.HeaderXForwardedHost: "www.test.com"},
},
{
Expected: "http://",
},
{
Expected: "http://www.override.com",
Headers: map[string]string{"X-Forwarded-Host": "www.override.com"},
Headers: map[string]string{constant.HeaderXForwardedHost: "www.override.com"},
Hostname: "www.test.com",
},
{
Expand All @@ -114,15 +114,15 @@ func TestGetRequestHostURL(t *testing.T) {
},
{
Expected: "https://www.override.com",
Headers: map[string]string{"X-Forwarded-Host": "www.override.com"},
Headers: map[string]string{constant.HeaderXForwardedHost: "www.override.com"},
Hostname: "www.test.com",
TLS: &tls.ConnectionState{},
},
{
Expected: "https://www.override.com",
Headers: map[string]string{
"X-Forwarded-Host": "www.override.com",
"X-Forwarded-Proto": "https"},
constant.HeaderXForwardedHost: "www.override.com",
constant.HeaderXForwardedProto: "https"},
Hostname: "www.override.com",
},
}
Expand Down

0 comments on commit 45150cf

Please sign in to comment.