diff --git a/internal/controller/proxy_controller.go b/internal/controller/proxy_controller.go index cc398d1d..3637a331 100644 --- a/internal/controller/proxy_controller.go +++ b/internal/controller/proxy_controller.go @@ -390,6 +390,12 @@ func (controller *ProxyController) getForwardAuthContext(c *gin.Context) (ProxyC return ProxyContext{}, errors.New("x-forwarded-uri not found") } + parsedURI, err := url.ParseRequestURI(uri) + + if err != nil { + return ProxyContext{}, fmt.Errorf("invalid x-forwarded-uri: %w", err) + } + proto, ok := controller.getHeader(c, "x-forwarded-proto") if !ok { @@ -403,7 +409,7 @@ func (controller *ProxyController) getForwardAuthContext(c *gin.Context) (ProxyC return ProxyContext{ Host: host, Proto: proto, - Path: uri, + Path: parsedURI.Path, Method: method, Type: ForwardAuth, }, nil diff --git a/internal/controller/proxy_controller_test.go b/internal/controller/proxy_controller_test.go index 87148109..f297f27f 100644 --- a/internal/controller/proxy_controller_test.go +++ b/internal/controller/proxy_controller_test.go @@ -287,6 +287,66 @@ func TestProxyController(t *testing.T) { assert.Equal(t, http.StatusOK, recorder.Code) }, }, + { + description: "Ensure path allow ACL does not match forwarded URI query string", + 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", "path-allow.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/admin?path=/allowed") + router.ServeHTTP(recorder, req) + assert.Equal(t, http.StatusUnauthorized, recorder.Code) + }, + }, + { + description: "Ensure path allow ACL does not match path substrings", + 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", "path-allow.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/admin/allowed") + router.ServeHTTP(recorder, req) + assert.Equal(t, http.StatusUnauthorized, recorder.Code) + }, + }, + { + description: "Ensure path block ACL works on forward auth", + 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", "path-block.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/blocked") + router.ServeHTTP(recorder, req) + assert.Equal(t, http.StatusUnauthorized, recorder.Code) + }, + }, + { + description: "Ensure path block ACL does not match forwarded URI query string", + 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", "path-block.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/admin?path=/blocked") + router.ServeHTTP(recorder, req) + assert.Equal(t, http.StatusOK, recorder.Code) + }, + }, + { + description: "Ensure path block ACL does not match path substrings", + 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", "path-block.example.com") + req.Header.Set("x-forwarded-proto", "https") + req.Header.Set("x-forwarded-uri", "/admin/blocked") + router.ServeHTTP(recorder, req) + assert.Equal(t, http.StatusOK, recorder.Code) + }, + }, { description: "Ensure path allow ACL works on nginx auth request", middlewares: []gin.HandlerFunc{}, diff --git a/internal/service/access_controls_rules.go b/internal/service/access_controls_rules.go index 318894d2..be6dc8b8 100644 --- a/internal/service/access_controls_rules.go +++ b/internal/service/access_controls_rules.go @@ -2,7 +2,6 @@ package service import ( "errors" - "regexp" "strings" "github.com/tinyauthapp/tinyauth/internal/model" @@ -180,33 +179,45 @@ type AuthEnabledRule struct { Log *logger.Logger } +func matchPathRule(paths, path string) bool { + for _, configuredPath := range strings.Split(paths, ",") { + configuredPath = strings.TrimSpace(configuredPath) + + if configuredPath == "/" { + return true + } + + configuredPath = strings.TrimRight(configuredPath, "/") + + if configuredPath == "" { + continue + } + + if path == configuredPath || strings.HasPrefix(path, configuredPath+"/") { + return true + } + } + + return false +} + func (rule *AuthEnabledRule) Evaluate(ctx *ACLContext) Effect { if ctx.ACLs == nil { return EffectDeny } if ctx.ACLs.Path.Block != "" { - regex, err := regexp.Compile(ctx.ACLs.Path.Block) + match := matchPathRule(ctx.ACLs.Path.Block, ctx.Path) - if err != nil { - rule.Log.App.Error().Err(err).Msg("Failed to compile block regex") - return EffectDeny - } - - if !regex.MatchString(ctx.Path) { + if !match { return EffectAllow } } if ctx.ACLs.Path.Allow != "" { - regex, err := regexp.Compile(ctx.ACLs.Path.Allow) + match := matchPathRule(ctx.ACLs.Path.Allow, ctx.Path) - if err != nil { - rule.Log.App.Error().Err(err).Msg("Failed to compile allow regex") - return EffectDeny - } - - if regex.MatchString(ctx.Path) { + if match { return EffectAllow } } diff --git a/internal/service/access_controls_rules_test.go b/internal/service/access_controls_rules_test.go index 7cb0f8ba..39009b71 100644 --- a/internal/service/access_controls_rules_test.go +++ b/internal/service/access_controls_rules_test.go @@ -527,52 +527,52 @@ func TestAuthEnabledRule(t *testing.T) { expected: EffectDeny, }, { - name: "allows when path does not match block regex", + name: "allows when path does not match block path", ctx: &ACLContext{ ACLs: &model.App{ - Path: model.AppPath{Block: "^/admin"}, + Path: model.AppPath{Block: "/admin"}, }, Path: "/public", }, expected: EffectAllow, }, { - name: "denies when path matches block regex and no allow regex", + name: "denies when path matches block path", ctx: &ACLContext{ ACLs: &model.App{ - Path: model.AppPath{Block: "^/admin"}, + Path: model.AppPath{Block: "/admin"}, }, Path: "/admin/users", }, expected: EffectDeny, }, { - name: "allows when path matches allow regex", + name: "allows when path matches allow path", ctx: &ACLContext{ ACLs: &model.App{ - Path: model.AppPath{Allow: "^/public"}, + Path: model.AppPath{Allow: "/public"}, }, Path: "/public/index", }, expected: EffectAllow, }, { - name: "denies when path does not match allow regex", + name: "denies when path does not match allow path", ctx: &ACLContext{ ACLs: &model.App{ - Path: model.AppPath{Allow: "^/public"}, + Path: model.AppPath{Allow: "/public"}, }, Path: "/private", }, expected: EffectDeny, }, { - name: "allows when blocked path is also explicitly allowed", + name: "allows when blocked path is explicitly allowed", ctx: &ACLContext{ ACLs: &model.App{ Path: model.AppPath{ - Block: "^/admin", - Allow: "^/admin/public", + Block: "/admin", + Allow: "/admin/public", }, }, Path: "/admin/public/page", @@ -580,20 +580,10 @@ func TestAuthEnabledRule(t *testing.T) { expected: EffectAllow, }, { - name: "denies when block regex fails to compile", + name: "denies when root is blocked", ctx: &ACLContext{ ACLs: &model.App{ - Path: model.AppPath{Block: "[invalid"}, - }, - Path: "/anything", - }, - expected: EffectDeny, - }, - { - name: "denies when allow regex fails to compile", - ctx: &ACLContext{ - ACLs: &model.App{ - Path: model.AppPath{Allow: "[invalid"}, + Path: model.AppPath{Block: "/"}, }, Path: "/anything", }, diff --git a/internal/test/test.go b/internal/test/test.go index 20275adc..cb407689 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -61,6 +61,14 @@ func CreateTestConfigs(t *testing.T) (model.Config, model.RuntimeConfig) { Allow: "/allowed", }, }, + "app_path_block": { + Config: model.AppConfig{ + Domain: "path-block.example.com", + }, + Path: model.AppPath{ + Block: "/blocked", + }, + }, "app_user_allow": { Config: model.AppConfig{ Domain: "user-allow.example.com",