Skip to content

Commit 3d5ab54

Browse files
scottmckendryscottmckendry
committed
fix(acls): auth bypass via forward auth
1 parent c7077a7 commit 3d5ab54

5 files changed

Lines changed: 114 additions & 39 deletions

File tree

‎internal/controller/proxy_controller.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,12 @@ func (controller *ProxyController) getForwardAuthContext(c *gin.Context) (ProxyC
390390
return ProxyContext{}, errors.New("x-forwarded-uri not found")
391391
}
392392

393+
parsedURI, err := url.ParseRequestURI(uri)
394+
395+
if err != nil {
396+
return ProxyContext{}, fmt.Errorf("invalid x-forwarded-uri: %w", err)
397+
}
398+
393399
proto, ok := controller.getHeader(c, "x-forwarded-proto")
394400

395401
if !ok {
@@ -403,7 +409,7 @@ func (controller *ProxyController) getForwardAuthContext(c *gin.Context) (ProxyC
403409
return ProxyContext{
404410
Host: host,
405411
Proto: proto,
406-
Path: uri,
412+
Path: parsedURI.Path,
407413
Method: method,
408414
Type: ForwardAuth,
409415
}, nil

‎internal/controller/proxy_controller_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,66 @@ func TestProxyController(t *testing.T) {
287287
assert.Equal(t, http.StatusOK, recorder.Code)
288288
},
289289
},
290+
{
291+
description: "Ensure path allow ACL does not match forwarded URI query string",
292+
middlewares: []gin.HandlerFunc{},
293+
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
294+
req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
295+
req.Header.Set("x-forwarded-host", "path-allow.example.com")
296+
req.Header.Set("x-forwarded-proto", "https")
297+
req.Header.Set("x-forwarded-uri", "/admin?path=/allowed")
298+
router.ServeHTTP(recorder, req)
299+
assert.Equal(t, http.StatusUnauthorized, recorder.Code)
300+
},
301+
},
302+
{
303+
description: "Ensure path allow ACL does not match path substrings",
304+
middlewares: []gin.HandlerFunc{},
305+
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
306+
req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
307+
req.Header.Set("x-forwarded-host", "path-allow.example.com")
308+
req.Header.Set("x-forwarded-proto", "https")
309+
req.Header.Set("x-forwarded-uri", "/admin/allowed")
310+
router.ServeHTTP(recorder, req)
311+
assert.Equal(t, http.StatusUnauthorized, recorder.Code)
312+
},
313+
},
314+
{
315+
description: "Ensure path block ACL works on forward auth",
316+
middlewares: []gin.HandlerFunc{},
317+
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
318+
req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
319+
req.Header.Set("x-forwarded-host", "path-block.example.com")
320+
req.Header.Set("x-forwarded-proto", "https")
321+
req.Header.Set("x-forwarded-uri", "/blocked")
322+
router.ServeHTTP(recorder, req)
323+
assert.Equal(t, http.StatusUnauthorized, recorder.Code)
324+
},
325+
},
326+
{
327+
description: "Ensure path block ACL does not match forwarded URI query string",
328+
middlewares: []gin.HandlerFunc{},
329+
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
330+
req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
331+
req.Header.Set("x-forwarded-host", "path-block.example.com")
332+
req.Header.Set("x-forwarded-proto", "https")
333+
req.Header.Set("x-forwarded-uri", "/admin?path=/blocked")
334+
router.ServeHTTP(recorder, req)
335+
assert.Equal(t, http.StatusOK, recorder.Code)
336+
},
337+
},
338+
{
339+
description: "Ensure path block ACL does not match path substrings",
340+
middlewares: []gin.HandlerFunc{},
341+
run: func(t *testing.T, router *gin.Engine, recorder *httptest.ResponseRecorder) {
342+
req := httptest.NewRequest("GET", "/api/auth/traefik", nil)
343+
req.Header.Set("x-forwarded-host", "path-block.example.com")
344+
req.Header.Set("x-forwarded-proto", "https")
345+
req.Header.Set("x-forwarded-uri", "/admin/blocked")
346+
router.ServeHTTP(recorder, req)
347+
assert.Equal(t, http.StatusOK, recorder.Code)
348+
},
349+
},
290350
{
291351
description: "Ensure path allow ACL works on nginx auth request",
292352
middlewares: []gin.HandlerFunc{},

‎internal/service/access_controls_rules.go‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package service
22

33
import (
44
"errors"
5-
"regexp"
65
"strings"
76

87
"github.com/tinyauthapp/tinyauth/internal/model"
@@ -180,33 +179,45 @@ type AuthEnabledRule struct {
180179
Log *logger.Logger
181180
}
182181

182+
func matchPathRule(paths, path string) bool {
183+
for _, configuredPath := range strings.Split(paths, ",") {
184+
configuredPath = strings.TrimSpace(configuredPath)
185+
186+
if configuredPath == "/" {
187+
return true
188+
}
189+
190+
configuredPath = strings.TrimRight(configuredPath, "/")
191+
192+
if configuredPath == "" {
193+
continue
194+
}
195+
196+
if path == configuredPath || strings.HasPrefix(path, configuredPath+"/") {
197+
return true
198+
}
199+
}
200+
201+
return false
202+
}
203+
183204
func (rule *AuthEnabledRule) Evaluate(ctx *ACLContext) Effect {
184205
if ctx.ACLs == nil {
185206
return EffectDeny
186207
}
187208

188209
if ctx.ACLs.Path.Block != "" {
189-
regex, err := regexp.Compile(ctx.ACLs.Path.Block)
190-
191-
if err != nil {
192-
rule.Log.App.Error().Err(err).Msg("Failed to compile block regex")
193-
return EffectDeny
194-
}
210+
match := matchPathRule(ctx.ACLs.Path.Block, ctx.Path)
195211

196-
if !regex.MatchString(ctx.Path) {
212+
if !match {
197213
return EffectAllow
198214
}
199215
}
200216

201217
if ctx.ACLs.Path.Allow != "" {
202-
regex, err := regexp.Compile(ctx.ACLs.Path.Allow)
218+
match := matchPathRule(ctx.ACLs.Path.Allow, ctx.Path)
203219

204-
if err != nil {
205-
rule.Log.App.Error().Err(err).Msg("Failed to compile allow regex")
206-
return EffectDeny
207-
}
208-
209-
if regex.MatchString(ctx.Path) {
220+
if match {
210221
return EffectAllow
211222
}
212223
}

‎internal/service/access_controls_rules_test.go‎

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -527,73 +527,63 @@ func TestAuthEnabledRule(t *testing.T) {
527527
expected: EffectDeny,
528528
},
529529
{
530-
name: "allows when path does not match block regex",
530+
name: "allows when path does not match block path",
531531
ctx: &ACLContext{
532532
ACLs: &model.App{
533-
Path: model.AppPath{Block: "^/admin"},
533+
Path: model.AppPath{Block: "/admin"},
534534
},
535535
Path: "/public",
536536
},
537537
expected: EffectAllow,
538538
},
539539
{
540-
name: "denies when path matches block regex and no allow regex",
540+
name: "denies when path matches block path",
541541
ctx: &ACLContext{
542542
ACLs: &model.App{
543-
Path: model.AppPath{Block: "^/admin"},
543+
Path: model.AppPath{Block: "/admin"},
544544
},
545545
Path: "/admin/users",
546546
},
547547
expected: EffectDeny,
548548
},
549549
{
550-
name: "allows when path matches allow regex",
550+
name: "allows when path matches allow path",
551551
ctx: &ACLContext{
552552
ACLs: &model.App{
553-
Path: model.AppPath{Allow: "^/public"},
553+
Path: model.AppPath{Allow: "/public"},
554554
},
555555
Path: "/public/index",
556556
},
557557
expected: EffectAllow,
558558
},
559559
{
560-
name: "denies when path does not match allow regex",
560+
name: "denies when path does not match allow path",
561561
ctx: &ACLContext{
562562
ACLs: &model.App{
563-
Path: model.AppPath{Allow: "^/public"},
563+
Path: model.AppPath{Allow: "/public"},
564564
},
565565
Path: "/private",
566566
},
567567
expected: EffectDeny,
568568
},
569569
{
570-
name: "allows when blocked path is also explicitly allowed",
570+
name: "allows when blocked path is explicitly allowed",
571571
ctx: &ACLContext{
572572
ACLs: &model.App{
573573
Path: model.AppPath{
574-
Block: "^/admin",
575-
Allow: "^/admin/public",
574+
Block: "/admin",
575+
Allow: "/admin/public",
576576
},
577577
},
578578
Path: "/admin/public/page",
579579
},
580580
expected: EffectAllow,
581581
},
582582
{
583-
name: "denies when block regex fails to compile",
583+
name: "denies when root is blocked",
584584
ctx: &ACLContext{
585585
ACLs: &model.App{
586-
Path: model.AppPath{Block: "[invalid"},
587-
},
588-
Path: "/anything",
589-
},
590-
expected: EffectDeny,
591-
},
592-
{
593-
name: "denies when allow regex fails to compile",
594-
ctx: &ACLContext{
595-
ACLs: &model.App{
596-
Path: model.AppPath{Allow: "[invalid"},
586+
Path: model.AppPath{Block: "/"},
597587
},
598588
Path: "/anything",
599589
},

‎internal/test/test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ func CreateTestConfigs(t *testing.T) (model.Config, model.RuntimeConfig) {
6161
Allow: "/allowed",
6262
},
6363
},
64+
"app_path_block": {
65+
Config: model.AppConfig{
66+
Domain: "path-block.example.com",
67+
},
68+
Path: model.AppPath{
69+
Block: "/blocked",
70+
},
71+
},
6472
"app_user_allow": {
6573
Config: model.AppConfig{
6674
Domain: "user-allow.example.com",

0 commit comments

Comments
 (0)