From 49219982b20909d9d6e49b82d54dd3034b8f956a Mon Sep 17 00:00:00 2001 From: Ilya Amelevich Date: Sat, 1 Apr 2023 23:16:16 +0200 Subject: [PATCH 1/5] feat: add `SetSkipper` method to `Plugin`, that let change skipper in proxy middleware --- plugin.go | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/plugin.go b/plugin.go index 1e96893..32f55ac 100644 --- a/plugin.go +++ b/plugin.go @@ -12,6 +12,11 @@ import ( "github.com/pocketbase/pocketbase/core" ) +// DefaultSkipper skip proxy middleware for requests, where path starts with /_/ or /api/. +func DefaultSkipper(c echo.Context) bool { + return strings.HasPrefix(c.Request().URL.Path, "/_/") || strings.HasPrefix(c.Request().URL.Path, "/api/") +} + // Options defines optional struct to customize the default plugin behavior. type Options struct { // Enabled defines if proxy should be enabled. @@ -35,6 +40,9 @@ type Plugin struct { // parsedUrl from options.Url parsedUrl *url.URL + + // Skipper function for proxy middleware + skipper middleware.Skipper } // Validate plugin options. Return error if some option is invalid. @@ -67,22 +75,34 @@ func (p *Plugin) Validate() error { return nil } +/* +SetSkipper set skipper function that should return true if that route shouldn't be proxied. + +If not set, the DefaultSkipper is used: + +If set - you should also control the middleware behavior for /_/ and /api/ routes. + +Example: + + plugin.SetSkipper(func(c echo.Context) bool { + return c.Request().URL.Path == "/my-super-secret-route" + }) +*/ +func (p *Plugin) SetSkipper(skipper middleware.Skipper) { + p.skipper = skipper +} + func (p *Plugin) enableProxy(e *core.ServeEvent) error { if p.options.Enabled { - // Skip PocketBase routes - skipperFunc := func(c echo.Context) bool { - return strings.HasPrefix(c.Request().URL.Path, "/_/") || strings.HasPrefix(c.Request().URL.Path, "/api/") - } - if p.options.ProxyLogsEnabled { e.Router.Use(middleware.LoggerWithConfig(middleware.LoggerConfig{ - Skipper: skipperFunc, + Skipper: p.skipper, })) } else { log.Println("Proxy logs are disabled") } e.Router.Use(middleware.ProxyWithConfig(middleware.ProxyConfig{ - Skipper: skipperFunc, + Skipper: p.skipper, Balancer: middleware.NewRoundRobinBalancer([]*middleware.ProxyTarget{ { URL: p.parsedUrl, @@ -114,7 +134,10 @@ func MustRegister(app core.App, options *Options) *Plugin { // Register registers plugin. func Register(app core.App, options *Options) (*Plugin, error) { - p := &Plugin{app: app} + p := &Plugin{ + app: app, + skipper: DefaultSkipper, + } // Set default options if options != nil { From ce9d90ac2375d74328a6eb966a1537eeec9485f4 Mon Sep 17 00:00:00 2001 From: Ilya Amelevich Date: Sat, 1 Apr 2023 23:23:50 +0200 Subject: [PATCH 2/5] test: add `SetSkipper` test --- plugin_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/plugin_test.go b/plugin_test.go index d5b05d8..1033525 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -4,6 +4,7 @@ import ( "net/http" "testing" + "github.com/labstack/echo/v5" "github.com/pocketbase/pocketbase" "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tests" @@ -192,6 +193,30 @@ func TestPlugin_MustRegister(t *testing.T) { Url: "http://localhost:1234", }), }, + { + Name: "/my-super-api-path request should not be proxied when enabled with custom skipper", + Method: http.MethodPost, + Url: "/my-super-api-path", + ExpectedStatus: 404, + ExpectedContent: []string{`"data":{}`}, + TestAppFactory: func() (*tests.TestApp, error) { + testApp, err := tests.NewTestApp() + if err != nil { + return nil, err + } + + p := MustRegister(testApp, &Options{ + Enabled: true, + Url: "http://localhost:1234", + }) + + p.SetSkipper(func(c echo.Context) bool { + return c.Request().URL.Path == "/my-super-api-path" + }) + + return testApp, nil + }, + }, } for _, scenario := range scenarios { From cbc265e10b30555a57fb8974558900e07cde2bc8 Mon Sep 17 00:00:00 2001 From: Ilya Amelevich Date: Sat, 1 Apr 2023 23:27:00 +0200 Subject: [PATCH 3/5] fix: not return error from `enableProxy` --- plugin.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/plugin.go b/plugin.go index 32f55ac..bf53df3 100644 --- a/plugin.go +++ b/plugin.go @@ -92,7 +92,7 @@ func (p *Plugin) SetSkipper(skipper middleware.Skipper) { p.skipper = skipper } -func (p *Plugin) enableProxy(e *core.ServeEvent) error { +func (p *Plugin) enableProxy(e *core.ServeEvent) { if p.options.Enabled { if p.options.ProxyLogsEnabled { e.Router.Use(middleware.LoggerWithConfig(middleware.LoggerConfig{ @@ -120,7 +120,6 @@ func (p *Plugin) enableProxy(e *core.ServeEvent) error { color.CyanString("%s", p.parsedUrl.String()), ) } - return nil } // MustRegister is a helper function that registers plugin and panics if error occurred. @@ -152,9 +151,7 @@ func Register(app core.App, options *Options) (*Plugin, error) { } app.OnBeforeServe().Add(func(e *core.ServeEvent) error { - if err := p.enableProxy(e); err != nil { - return err - } + p.enableProxy(e) return nil }) From 05be8579962294311cf75f5607e9bb90aed1c9e8 Mon Sep 17 00:00:00 2001 From: Ilya Amelevich Date: Sat, 1 Apr 2023 23:27:37 +0200 Subject: [PATCH 4/5] test: add test case for `ProxyLogsEnabled: true` --- plugin_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugin_test.go b/plugin_test.go index 1033525..7024760 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -152,6 +152,18 @@ func TestPlugin_MustRegister(t *testing.T) { Url: "http://localhost:1234", }), }, + { + Name: "/ request should be proxied when enabled and ProxyLogsEnabled", + Method: http.MethodPost, + Url: "/", + ExpectedStatus: 200, + ExpectedContent: []string{`OK from /`}, + TestAppFactory: setupTestApp(&Options{ + Enabled: true, + Url: "http://localhost:1234", + ProxyLogsEnabled: true, + }), + }, { Name: "/ shouldn be proxied when options nil", Method: http.MethodPost, From 15c9c8f3cc15a9eb128b43441392a7251d60ab22 Mon Sep 17 00:00:00 2001 From: Ilya Amelevich Date: Sat, 1 Apr 2023 23:32:23 +0200 Subject: [PATCH 5/5] test: add `Register` validation fail test --- plugin_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugin_test.go b/plugin_test.go index 7024760..f44f653 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -115,6 +115,13 @@ func TestPlugin_Validate(t *testing.T) { } } +func TestPlugin_Register(t *testing.T) { + _, err := Register(nil, nil) + if err == nil { + t.Errorf("Register() should fail when app is nil") + } +} + func TestPlugin_MustRegister(t *testing.T) { // setup the test ApiScenario app instance setupTestApp := func(options *Options) func() (*tests.TestApp, error) {