From 05cfb121ec3f214c0e45206c188f34bad4d4eb8c Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 4 Nov 2024 16:58:53 -0500 Subject: [PATCH] forwardauth: Skip copying missing response headers (#6608) --- .../forward_auth_authelia.caddyfiletest | 102 +++++++++++++- .../forward_auth_rename_headers.caddyfiletest | 132 ++++++++++++++++-- .../reverseproxy/forwardauth/caddyfile.go | 67 ++++++--- 3 files changed, 267 insertions(+), 34 deletions(-) diff --git a/caddytest/integration/caddyfile_adapt/forward_auth_authelia.caddyfiletest b/caddytest/integration/caddyfile_adapt/forward_auth_authelia.caddyfiletest index a0570328..240bdc62 100644 --- a/caddytest/integration/caddyfile_adapt/forward_auth_authelia.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/forward_auth_authelia.caddyfiletest @@ -1,6 +1,6 @@ app.example.com { forward_auth authelia:9091 { - uri /api/verify?rd=https://authelia.example.com + uri /api/authz/forward-auth copy_headers Remote-User Remote-Groups Remote-Name Remote-Email } @@ -39,6 +39,13 @@ app.example.com { ] }, "routes": [ + { + "handle": [ + { + "handler": "vars" + } + ] + }, { "handle": [ { @@ -47,19 +54,104 @@ app.example.com { "set": { "Remote-Email": [ "{http.reverse_proxy.header.Remote-Email}" - ], + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.Remote-Email}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { "Remote-Groups": [ "{http.reverse_proxy.header.Remote-Groups}" - ], + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.Remote-Groups}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { "Remote-Name": [ "{http.reverse_proxy.header.Remote-Name}" - ], + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.Remote-Name}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { "Remote-User": [ "{http.reverse_proxy.header.Remote-User}" ] } } } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.Remote-User}": [ + "" + ] + } + } + ] + } ] } ] @@ -80,7 +172,7 @@ app.example.com { }, "rewrite": { "method": "GET", - "uri": "/api/verify?rd=https://authelia.example.com" + "uri": "/api/authz/forward-auth" }, "upstreams": [ { diff --git a/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest b/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest index 65228174..c2be2ed4 100644 --- a/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest +++ b/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest @@ -28,6 +28,13 @@ forward_auth localhost:9000 { ] }, "routes": [ + { + "handle": [ + { + "handler": "vars" + } + ] + }, { "handle": [ { @@ -36,22 +43,131 @@ forward_auth localhost:9000 { "set": { "1": [ "{http.reverse_proxy.header.A}" - ], - "3": [ - "{http.reverse_proxy.header.C}" - ], - "5": [ - "{http.reverse_proxy.header.E}" - ], + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.A}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { "B": [ "{http.reverse_proxy.header.B}" - ], + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.B}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { + "3": [ + "{http.reverse_proxy.header.C}" + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.C}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { "D": [ "{http.reverse_proxy.header.D}" ] } } } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.D}": [ + "" + ] + } + } + ] + } + ] + }, + { + "handle": [ + { + "handler": "headers", + "request": { + "set": { + "5": [ + "{http.reverse_proxy.header.E}" + ] + } + } + } + ], + "match": [ + { + "not": [ + { + "vars": { + "{http.reverse_proxy.header.E}": [ + "" + ] + } + } + ] + } ] } ] diff --git a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go index 8350096a..347f6dfb 100644 --- a/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/forwardauth/caddyfile.go @@ -17,6 +17,7 @@ package forwardauth import ( "encoding/json" "net/http" + "sort" "strings" "github.com/caddyserver/caddy/v2" @@ -170,42 +171,66 @@ func parseCaddyfile(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, dispenser.Errf("the 'uri' subdirective is required") } - // set up handler for good responses; when a response - // has 2xx status, then we will copy some headers from - // the response onto the original request, and allow - // handling to continue down the middleware chain, - // by _not_ executing a terminal handler. + // Set up handler for good responses; when a response has 2xx status, + // then we will copy some headers from the response onto the original + // request, and allow handling to continue down the middleware chain, + // by _not_ executing a terminal handler. We must have at least one + // route in the response handler, even if it's no-op, so that the + // response handling logic in reverse_proxy doesn't skip this entry. goodResponseHandler := caddyhttp.ResponseHandler{ Match: &caddyhttp.ResponseMatcher{ StatusCode: []int{2}, }, - Routes: []caddyhttp.Route{}, - } - - handler := &headers.Handler{ - Request: &headers.HeaderOps{ - Set: http.Header{}, + Routes: []caddyhttp.Route{ + { + HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( + &caddyhttp.VarsMiddleware{}, + "handler", + "vars", + nil, + )}, + }, }, } - // the list of headers to copy may be empty, but that's okay; we - // need at least one handler in the routes for the response handling - // logic in reverse_proxy to not skip this entry as empty. - for from, to := range headersToCopy { - handler.Request.Set.Set(to, "{http.reverse_proxy.header."+http.CanonicalHeaderKey(from)+"}") + // Sort the headers so that the order in the JSON output is deterministic. + sortedHeadersToCopy := make([]string, 0, len(headersToCopy)) + for k := range headersToCopy { + sortedHeadersToCopy = append(sortedHeadersToCopy, k) } + sort.Strings(sortedHeadersToCopy) - goodResponseHandler.Routes = append( - goodResponseHandler.Routes, - caddyhttp.Route{ + // Set up handlers to copy headers from the auth response onto the + // original request. We use vars matchers to test that the placeholder + // values aren't empty, because the header handler would not replace + // placeholders which have no value. + copyHeaderRoutes := []caddyhttp.Route{} + for _, from := range sortedHeadersToCopy { + to := http.CanonicalHeaderKey(headersToCopy[from]) + placeholderName := "http.reverse_proxy.header." + http.CanonicalHeaderKey(from) + handler := &headers.Handler{ + Request: &headers.HeaderOps{ + Set: http.Header{ + to: []string{"{" + placeholderName + "}"}, + }, + }, + } + copyHeaderRoutes = append(copyHeaderRoutes, caddyhttp.Route{ + MatcherSetsRaw: []caddy.ModuleMap{{ + "not": h.JSON(caddyhttp.MatchNot{MatcherSetsRaw: []caddy.ModuleMap{{ + "vars": h.JSON(caddyhttp.VarsMatcher{"{" + placeholderName + "}": []string{""}}), + }}}), + }}, HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject( handler, "handler", "headers", nil, )}, - }, - ) + }) + } + + goodResponseHandler.Routes = append(goodResponseHandler.Routes, copyHeaderRoutes...) // note that when a response has any other status than 2xx, then we // use the reverse proxy's default behaviour of copying the response