forwardauth: Skip copying missing response headers (#6608)

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 a05703280..240bdc62f 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
 	}
 
@@ -42,24 +42,116 @@ app.example.com {
 																{
 																	"handle": [
 																		{
+																			"handler": "vars"
+																		}
+																	]
+																},
+																{
+																	"handle": [
+																		{
 																			"handler": "headers",
 																			"request": {
 																				"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 65228174d..c2be2ed43 100644
--- a/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest
+++ b/caddytest/integration/caddyfile_adapt/forward_auth_rename_headers.caddyfiletest
@@ -31,27 +31,143 @@ forward_auth localhost:9000 {
 												{
 													"handle": [
 														{
+															"handler": "vars"
+														}
+													]
+												},
+												{
+													"handle": [
+														{
 															"handler": "headers",
 															"request": {
 																"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 8350096ae..347f6dfbf 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