From a4f9c932143bd2dda2820688f233ddcf8330f312 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 26 Feb 2024 16:16:02 +0100 Subject: [PATCH 1/5] Extend bypass middleware with support of wildcard paths --- .../server/http/middleware/bypass/bypass.go | 42 +++++++++++++++++-- .../http/middleware/bypass/bypass_test.go | 30 ++++++++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/management/server/http/middleware/bypass/bypass.go b/management/server/http/middleware/bypass/bypass.go index 2f2652eb61d..8ff30963ff8 100644 --- a/management/server/http/middleware/bypass/bypass.go +++ b/management/server/http/middleware/bypass/bypass.go @@ -2,7 +2,10 @@ package bypass import ( "net/http" + "path" "sync" + + log "github.com/sirupsen/logrus" ) var byPassMutex sync.RWMutex @@ -11,10 +14,16 @@ var byPassMutex sync.RWMutex var bypassPaths = make(map[string]struct{}) // AddBypassPath adds an exact path to the list of paths that bypass middleware. -func AddBypassPath(path string) { +// Paths can include wildcards, such as /api/*. Paths are matched using path.Match. +// Returns an error if the path has invalid pattern. +func AddBypassPath(path string) error { byPassMutex.Lock() defer byPassMutex.Unlock() + if err := validatePath(path); err != nil { + return err + } bypassPaths[path] = struct{}{} + return nil } // RemovePath removes a path from the list of paths that bypass middleware. @@ -24,16 +33,41 @@ func RemovePath(path string) { delete(bypassPaths, path) } +// GetList returns a list of all bypass paths. +func GetList() []string { + byPassMutex.RLock() + defer byPassMutex.RUnlock() + + list := make([]string, 0, len(bypassPaths)) + for k := range bypassPaths { + list = append(list, k) + } + + return list +} + // ShouldBypass checks if the request path is one of the auth bypass paths and returns true if the middleware should be bypassed. // This can be used to bypass authz/authn middlewares for certain paths, such as webhooks that implement their own authentication. func ShouldBypass(requestPath string, h http.Handler, w http.ResponseWriter, r *http.Request) bool { byPassMutex.RLock() defer byPassMutex.RUnlock() - if _, ok := bypassPaths[requestPath]; ok { - h.ServeHTTP(w, r) - return true + for bypassPath := range bypassPaths { + matched, err := path.Match(bypassPath, requestPath) + if err != nil { + log.Errorf("Error matching path %s with %s from %s: %v", bypassPath, requestPath, GetList(), err) + continue + } + if matched { + h.ServeHTTP(w, r) + return true + } } return false } + +func validatePath(p string) error { + _, err := path.Match(p, "") + return err +} diff --git a/management/server/http/middleware/bypass/bypass_test.go b/management/server/http/middleware/bypass/bypass_test.go index efcfe1c3d88..aba2d32c4e5 100644 --- a/management/server/http/middleware/bypass/bypass_test.go +++ b/management/server/http/middleware/bypass/bypass_test.go @@ -11,6 +11,19 @@ import ( "github.com/netbirdio/netbird/management/server/http/middleware/bypass" ) +func TestGetList(t *testing.T) { + bypassPaths := []string{"/path1", "/path2", "/path3"} + + for _, path := range bypassPaths { + err := bypass.AddBypassPath(path) + require.NoError(t, err, "Adding bypass path should not fail") + } + + list := bypass.GetList() + + assert.ElementsMatch(t, bypassPaths, list, "Bypass path list did not match expected paths") +} + func TestAuthBypass(t *testing.T) { dummyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -31,6 +44,13 @@ func TestAuthBypass(t *testing.T) { expectBypass: true, expectHTTPCode: http.StatusOK, }, + { + name: "Wildcard path added to bypass", + pathToAdd: "/bypass/*", + testPath: "/bypass/extra", + expectBypass: true, + expectHTTPCode: http.StatusOK, + }, { name: "Path not added to bypass", testPath: "/no-bypass", @@ -59,6 +79,13 @@ func TestAuthBypass(t *testing.T) { expectBypass: false, expectHTTPCode: http.StatusOK, }, + { + name: "Wildrarded subpath does not match bypass", + pathToAdd: "/webhook/*", + testPath: "/webhook/extra/path", + expectBypass: false, + expectHTTPCode: http.StatusOK, + }, { name: "Similar path does not match bypass", pathToAdd: "/webhook", @@ -78,7 +105,8 @@ func TestAuthBypass(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { if tc.pathToAdd != "" { - bypass.AddBypassPath(tc.pathToAdd) + err := bypass.AddBypassPath(tc.pathToAdd) + require.NoError(t, err, "Adding bypass path should not fail") defer bypass.RemovePath(tc.pathToAdd) } From 47bf21d624319bc1b153ee537efac29846872743 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 26 Feb 2024 16:25:00 +0100 Subject: [PATCH 2/5] Fix linter --- management/server/http/middleware/auth_middleware_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/management/server/http/middleware/auth_middleware_test.go b/management/server/http/middleware/auth_middleware_test.go index 3e3e8419446..588bcaf02e3 100644 --- a/management/server/http/middleware/auth_middleware_test.go +++ b/management/server/http/middleware/auth_middleware_test.go @@ -177,7 +177,10 @@ func TestAuthMiddleware_Handler(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { if tc.shouldBypassAuth { - bypass.AddBypassPath(tc.path) + err := bypass.AddBypassPath(tc.path) + if err != nil { + t.Fatalf("failed to add bypass path: %v", err) + } } req := httptest.NewRequest("GET", "http://testing"+tc.path, nil) From 8c2fde064cd9fe3c5c2e0a3dacf8638d2bb4bb9d Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 26 Feb 2024 16:52:09 +0100 Subject: [PATCH 3/5] Fix typo --- management/server/http/middleware/bypass/bypass_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/middleware/bypass/bypass_test.go b/management/server/http/middleware/bypass/bypass_test.go index aba2d32c4e5..c65e6fa1f24 100644 --- a/management/server/http/middleware/bypass/bypass_test.go +++ b/management/server/http/middleware/bypass/bypass_test.go @@ -80,7 +80,7 @@ func TestAuthBypass(t *testing.T) { expectHTTPCode: http.StatusOK, }, { - name: "Wildrarded subpath does not match bypass", + name: "Wildcard subpath does not match bypass", pathToAdd: "/webhook/*", testPath: "/webhook/extra/path", expectBypass: false, From bb1bfca31afc58bebe3678f0051b54c03e554c7d Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 26 Feb 2024 17:33:30 +0100 Subject: [PATCH 4/5] Update management/server/http/middleware/bypass/bypass.go Co-authored-by: Viktor Liu --- management/server/http/middleware/bypass/bypass.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/server/http/middleware/bypass/bypass.go b/management/server/http/middleware/bypass/bypass.go index 8ff30963ff8..f6a45e30b31 100644 --- a/management/server/http/middleware/bypass/bypass.go +++ b/management/server/http/middleware/bypass/bypass.go @@ -20,7 +20,7 @@ func AddBypassPath(path string) error { byPassMutex.Lock() defer byPassMutex.Unlock() if err := validatePath(path); err != nil { - return err + return fmt.Errorf("validate: %w", err) } bypassPaths[path] = struct{}{} return nil From 1f7ba2e70371fd5fcfff6e3de044e5035d4101dc Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 26 Feb 2024 17:36:20 +0100 Subject: [PATCH 5/5] Add missed import --- management/server/http/middleware/bypass/bypass.go | 1 + 1 file changed, 1 insertion(+) diff --git a/management/server/http/middleware/bypass/bypass.go b/management/server/http/middleware/bypass/bypass.go index f6a45e30b31..87b41c6fc51 100644 --- a/management/server/http/middleware/bypass/bypass.go +++ b/management/server/http/middleware/bypass/bypass.go @@ -1,6 +1,7 @@ package bypass import ( + "fmt" "net/http" "path" "sync"