fix: preserve query parameters for login (#1068)

internal/controller/proxy_controller.go

@@ -47,6 +47,7 @@ type ProxyContext struct {
 	Host      string
 	Proto     string
 	Path      string
+	PathRaw   string
 	Method    string
 	Type      AuthModuleType
 	IsBrowser bool
@@ -281,7 +282,7 @@ func (controller *ProxyController) proxyHandler(c *gin.Context) {
 	}
 
 	queries, err := query.Values(RedirectQuery{
-		RedirectURI: fmt.Sprintf("%s://%s%s", proxyCtx.Proto, proxyCtx.Host, proxyCtx.Path),
+		RedirectURI: fmt.Sprintf("%s://%s%s", proxyCtx.Proto, proxyCtx.Host, proxyCtx.PathRaw),
 		LoginFor:    FrontendLoginForApp,
 	})
 
@@ -402,11 +403,11 @@ func (controller *ProxyController) getForwardAuthContext(c *gin.Context) (ProxyC
 	method := c.Request.Method
 
 	return ProxyContext{
-		Host:   host,
+		Host:    host,
-		Proto:  proto,
+		Proto:   proto,
-		Path:   uri,
+		PathRaw: uri,
-		Method: method,
+		Method:  method,
-		Type:   ForwardAuth,
+		Type:    ForwardAuth,
 	}, nil
 }
 
@@ -435,15 +436,14 @@ func (controller *ProxyController) getAuthRequestContext(c *gin.Context) (ProxyC
 		return ProxyContext{}, errors.New("proto not found")
 	}
 
-	path := url.Path
 	method := c.Request.Method
 
 	return ProxyContext{
-		Host:   host,
+		Host:    host,
-		Proto:  proto,
+		Proto:   proto,
-		Path:   path,
+		PathRaw: url.RequestURI(),
-		Method: method,
+		Method:  method,
-		Type:   AuthRequest,
+		Type:    AuthRequest,
 	}, nil
 }
 
@@ -469,11 +469,11 @@ func (controller *ProxyController) getExtAuthzContext(c *gin.Context) (ProxyCont
 	method := c.Request.Method
 
 	return ProxyContext{
-		Host:   host,
+		Host:    host,
-		Proto:  proto,
+		Proto:   proto,
-		Path:   path,
+		PathRaw: path,
-		Method: method,
+		Method:  method,
-		Type:   ExtAuthz,
+		Type:    ExtAuthz,
 	}, nil
 }
 
@@ -552,8 +552,8 @@ func (controller *ProxyController) getProxyContext(c *gin.Context) (ProxyContext
 		return ProxyContext{}, err
 	}
 
-	// remove any query params from the request path
+	// Parse the raw path to populate the cleaned path used for ACLs
-	upath, err := url.Parse(ctx.Path)
+	upath, err := url.Parse(ctx.PathRaw)
 
 	if err != nil {
 		return ProxyContext{}, fmt.Errorf("failed to parse request path: %v", err)

internal/controller/proxy_controller_test.go

@@ -95,6 +95,38 @@ func TestProxyController(t *testing.T) {
 				assert.Contains(t, location, "https://tinyauth.example.com/login")
 			},
 		},
+		{
+			description: "Forward auth login redirect should preserve query parameters",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
+				req.Header.Set("x-forwarded-host", "test.example.com")
+				req.Header.Set("x-forwarded-proto", "https")
+				req.Header.Set("x-forwarded-uri", "/search?foo=bar")
+				req.Header.Set("user-agent", browserUserAgent)
+				router.ServeHTTP(recorder, req)
+
+				assert.Equal(t, http.StatusFound, recorder.Code)
+				location := recorder.Header().Get("Location")
+				assert.Contains(t, location, url.QueryEscape("https://test.example.com/search?foo=bar"))
+				assert.Contains(t, location, "login_for=app")
+				assert.Contains(t, location, "https://tinyauth.example.com/login")
+			},
+		},
+		{
+			description: "Auth request (nginx) login redirect should preserve query parameters",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("GET", "/api/auth/nginx", nil)
+				req.Header.Set("x-original-url", "https://test.example.com/search?foo=bar")
+				router.ServeHTTP(recorder, req)
+				assert.Equal(t, http.StatusUnauthorized, recorder.Code)
+				location := recorder.Header().Get("x-tinyauth-location")
+				assert.Contains(t, location, url.QueryEscape("https://test.example.com/search?foo=bar"))
+				assert.Contains(t, location, "login_for=app")
+				assert.Contains(t, location, "https://tinyauth.example.com/login")
+			},
+		},
 		{
 			description: "Auth request (nginx) should be detected and used",
 			middlewares: []gin.HandlerFunc{},
@@ -126,6 +158,22 @@ func TestProxyController(t *testing.T) {
 				assert.Contains(t, location, "https://tinyauth.example.com/login")
 			},
 		},
+		{
+			description: "Ext authz (envoy) login redirect should preserve query parameters",
+			middlewares: []gin.HandlerFunc{},
+			run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
+				req := httptest.NewRequest("HEAD", "/api/auth/envoy?path=%2Fhello%3Ffoo%3Dbar", nil)
+				req.Host = "test.example.com"
+				req.Header.Set("x-forwarded-proto", "https")
+				req.Header.Set("user-agent", browserUserAgent)
+				router.ServeHTTP(recorder, req)
+				assert.Equal(t, http.StatusFound, recorder.Code)
+				location := recorder.Header().Get("Location")
+				assert.Contains(t, location, url.QueryEscape("https://test.example.com/hello?foo=bar"))
+				assert.Contains(t, location, "login_for=app")
+				assert.Contains(t, location, "https://tinyauth.example.com/login")
+			},
+		},
 		{
 			description: "Forward auth with caddy should be detected and used",
 			middlewares: []gin.HandlerFunc{},