diff --git a/internal/controller/proxy_controller.go b/internal/controller/proxy_controller.go index 32000c2b..c239e29f 100644 --- a/internal/controller/proxy_controller.go +++ b/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, - Proto: proto, - Path: uri, - Method: method, - Type: ForwardAuth, + Host: host, + Proto: proto, + PathRaw: uri, + Method: method, + 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, - Proto: proto, - Path: path, - Method: method, - Type: AuthRequest, + Host: host, + Proto: proto, + PathRaw: url.RequestURI(), + Method: method, + Type: AuthRequest, }, nil } @@ -469,11 +469,11 @@ func (controller *ProxyController) getExtAuthzContext(c *gin.Context) (ProxyCont method := c.Request.Method return ProxyContext{ - Host: host, - Proto: proto, - Path: path, - Method: method, - Type: ExtAuthz, + Host: host, + Proto: proto, + PathRaw: path, + Method: method, + 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 - upath, err := url.Parse(ctx.Path) + // Parse the raw path to populate the cleaned path used for ACLs + upath, err := url.Parse(ctx.PathRaw) if err != nil { return ProxyContext{}, fmt.Errorf("failed to parse request path: %v", err) diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index 1f484fe5..0e6a75e1 100644 --- a/internal/controller/proxy_controller_test.go +++ b/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", "/?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/?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/?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/?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{},