diff --git a/internal/controller/proxy_controller.go b/internal/controller/proxy_controller.go index 7cdcb74b..62a06efc 100644 --- a/internal/controller/proxy_controller.go +++ b/internal/controller/proxy_controller.go @@ -7,7 +7,9 @@ import ( "net/http" "net/url" "path" + "reflect" "regexp" + "slices" "strings" "github.com/tinyauthapp/tinyauth/internal/model" @@ -26,6 +28,7 @@ const ( AuthRequest AuthModuleType = iota ExtAuthz ForwardAuth + AuthModuleUnknown ) type ProxyType int @@ -466,6 +469,10 @@ func (controller *ProxyController) getExtAuthzContext(c *gin.Context) (ProxyCont } // The path is attached to the end of the /api/auth/envoy?path= string so we just strip it out + if !strings.HasPrefix(c.Request.RequestURI, "/api/auth/envoy?path=") { + return ProxyContext{}, errors.New("path not found") + } + path := strings.TrimPrefix(c.Request.RequestURI, "/api/auth/envoy?path=") if strings.TrimSpace(path) == "" { @@ -529,37 +536,10 @@ func (controller *ProxyController) getContextFromAuthModule(c *gin.Context, modu return ProxyContext{}, fmt.Errorf("unsupported auth module: %v", module) } -func (controller *ProxyController) authModuleIdentifiersPresent(c *gin.Context, module AuthModuleType) bool { - switch module { - case ForwardAuth: - _, host := controller.getHeader(c, "x-forwarded-host") - _, uri := controller.getHeader(c, "x-forwarded-uri") - return host || uri - case AuthRequest: - _, ok := controller.getHeader(c, "x-original-url") - return ok - case ExtAuthz: - return strings.TrimSpace(c.Query("path")) != "" - default: - return false - } -} - -func (controller *ProxyController) ensureNoMultipleAuthModules(c *gin.Context, authModules []AuthModuleType) error { - present := 0 - - for _, module := range authModules { - if controller.authModuleIdentifiersPresent(c, module) { - present++ - } - } - - if present > 1 { - controller.log.App.Warn().Msg("Request carries headers for multiple auth modules, possible spoofing attempt, denying") - return fmt.Errorf("conflicting auth module headers") - } - - return nil +func (controller *ProxyController) compareProxyContext(ctx1, ctx2 ProxyContext) bool { + ctx1.Type = AuthModuleUnknown + ctx2.Type = AuthModuleUnknown + return reflect.DeepEqual(ctx1, ctx2) } func (controller *ProxyController) getProxyContext(c *gin.Context) (ProxyContext, error) { @@ -584,13 +564,7 @@ func (controller *ProxyController) getProxyContext(c *gin.Context) (ProxyContext return ProxyContext{}, fmt.Errorf("no auth modules supported for proxy: %v", req.Proxy) } - err = controller.ensureNoMultipleAuthModules(c, controller.determineAuthModules(proxy, true)) - - if err != nil { - return ProxyContext{}, err - } - - var ctx *ProxyContext + var ctxSlice []ProxyContext for _, module := range authModules { controller.log.App.Debug().Msgf("Trying to get context from auth module %v", module) @@ -600,14 +574,22 @@ func (controller *ProxyController) getProxyContext(c *gin.Context) (ProxyContext continue } controller.log.App.Debug().Msgf("Successfully got context from auth module %v", module) - ctx = &authModuleCtx - break + ctxSlice = append(ctxSlice, authModuleCtx) } - if ctx == nil { + if len(ctxSlice) == 0 { return ProxyContext{}, fmt.Errorf("failed to get context from any auth module") } + if len(ctxSlice) > 1 { + if len(slices.CompactFunc(ctxSlice, controller.compareProxyContext)) > 1 { + controller.log.App.Warn().Msg("Request carries headers for multiple auth modules, possible spoofing attempt, denying") + return ProxyContext{}, fmt.Errorf("conflicting auth module headers") + } + } + + ctx := ctxSlice[0] + // Parse the raw path to populate the cleaned path used for ACLs upath, err := url.Parse(ctx.PathRaw) @@ -633,5 +615,5 @@ func (controller *ProxyController) getProxyContext(c *gin.Context) (ProxyContext ctx.IsBrowser = isBrowser ctx.ProxyType = proxy - return *ctx, nil + return ctx, nil } diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index ef573663..7153e595 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -867,10 +867,10 @@ func TestProxyController(t *testing.T) { }, }, { - description: "Forward auth and auth request headers should fail for nginx", + description: "Forward auth and different auth request headers should fail for nginx", run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { req := httptest.NewRequest("GET", "/api/auth/nginx", nil) - req.Header.Set("x-forwarded-host", "foo.example.com") + req.Header.Set("x-forwarded-host", "foobar.example.com") req.Header.Set("x-forwarded-proto", "https") req.Header.Set("x-forwarded-uri", "/foo?bar=foo") req.Header.Set("x-original-url", "https://foo.example.com/foo?bar=foo") @@ -880,13 +880,54 @@ func TestProxyController(t *testing.T) { }, }, { - description: "Forward auth and ext authz headers should fail for envoy", + description: "Forward auth and different ext authz headers should fail for envoy", run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { req := httptest.NewRequest("HEAD", "/api/auth/envoy?path=/hello", nil) req.Host = "foo.example.com" + req.Header.Set("x-forwarded-host", "foobar.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/foo?bar=foo") + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusBadRequest, recorder.Code) + }, + }, + { + description: "Forward auth and same auth request headers should succeed for nginx", + middlewares: []gin.HandlerFunc{ + simpleCtx, + }, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + req := httptest.NewRequest("GET", "/api/auth/nginx", nil) req.Header.Set("x-forwarded-host", "foo.example.com") req.Header.Set("x-forwarded-proto", "https") req.Header.Set("x-forwarded-uri", "/foo?bar=foo") + req.Header.Set("x-original-url", "https://foo.example.com/foo?bar=foo") + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusOK, recorder.Code) + }, + }, + { + description: "Forward auth and ext authz headers should succeed for envoy of they match", + middlewares: []gin.HandlerFunc{ + simpleCtx, + }, + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + req := httptest.NewRequest("HEAD", "/api/auth/envoy?path=/foo?bar=foo", nil) + req.Host = "foo.example.com" + req.Header.Set("x-forwarded-host", "foo.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/foo?bar=foo") + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusOK, recorder.Code) + }, + }, + { + description: "Proxy without any modules matching should fail", + run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) { + req := httptest.NewRequest("GET", "/api/auth/traefik", nil) router.ServeHTTP(recorder, req) assert.Equal(t, http.StatusBadRequest, recorder.Code)