From a7e7c9848e4aa4c57d218c3c10e6ddb85d028be9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:33:34 +0000 Subject: [PATCH 1/3] Initial plan From d65b159b483f8eb8613f5260eb13892489daea91 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:49:42 +0000 Subject: [PATCH 2/3] Restrict redirect hosts in login flow Co-authored-by: jeroenrinzema <3440116+jeroenrinzema@users.noreply.github.com> --- internal/config/config.go | 13 +- .../http/controllers/v1/management/auth.go | 68 ++++++++-- .../controllers/v1/management/auth_test.go | 117 ++++++++++++++++++ internal/wasm/test/action/go.sum | 8 ++ internal/wasm/test/provider/go.sum | 8 ++ modules/actions/webhook/go.sum | 8 ++ modules/providers/logger/go.sum | 8 ++ modules/providers/twilio/go.sum | 8 ++ 8 files changed, 225 insertions(+), 13 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index deed677d..2e85afbf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -29,12 +29,13 @@ type Node struct { } type Auth struct { - Driver string `env:"DRIVER"` - JWTSecret string `env:"JWT_SECRET"` - JWKS claim.JWKS `env:"JWKS_URL"` - TokenLife time.Duration `env:"TOKEN_LIFE" envDefault:"24h"` - Basic BasicAuth `envPrefix:"BASIC_"` - Clerk ClerkAuth `envPrefix:"CLERK_"` + Driver string `env:"DRIVER"` + JWTSecret string `env:"JWT_SECRET"` + JWKS claim.JWKS `env:"JWKS_URL"` + TokenLife time.Duration `env:"TOKEN_LIFE" envDefault:"24h"` + AllowedRedirectHosts []string `env:"ALLOWED_REDIRECT_HOSTS" envSeparator:","` + Basic BasicAuth `envPrefix:"BASIC_"` + Clerk ClerkAuth `envPrefix:"CLERK_"` } type BasicAuth struct { diff --git a/internal/http/controllers/v1/management/auth.go b/internal/http/controllers/v1/management/auth.go index 795a58a5..a79def38 100644 --- a/internal/http/controllers/v1/management/auth.go +++ b/internal/http/controllers/v1/management/auth.go @@ -1,19 +1,27 @@ package v1 import ( + "bytes" + "encoding/json" + "errors" + "io" "net/http" + "net/url" + "strings" "github.com/jmoiron/sqlx" "github.com/lunogram/platform/internal/config" "github.com/lunogram/platform/internal/http/auth/providers" "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" - "github.com/lunogram/platform/internal/http/json" + httpjson "github.com/lunogram/platform/internal/http/json" "github.com/lunogram/platform/internal/http/problem" "github.com/lunogram/platform/internal/rbac" "github.com/lunogram/platform/internal/store/management" "go.uber.org/zap" ) +var ErrInvalidRedirect = errors.New("redirect host not allowed") + func NewAuthController(logger *zap.Logger, db *sqlx.DB, cfg config.Node, engine *rbac.Engine) (*AuthController, error) { stores := management.NewState(db) @@ -23,18 +31,20 @@ func NewAuthController(logger *zap.Logger, db *sqlx.DB, cfg config.Node, engine } return &AuthController{ - logger: logger, - provider: provider, + logger: logger, + provider: provider, + allowedRedirectHosts: cfg.Auth.AllowedRedirectHosts, }, nil } type AuthController struct { - logger *zap.Logger - provider providers.Provider + logger *zap.Logger + provider providers.Provider + allowedRedirectHosts []string } func (c *AuthController) GetAuthMethods(w http.ResponseWriter, r *http.Request) { - json.Write(w, http.StatusOK, []string{c.provider.Driver()}) + httpjson.Write(w, http.StatusOK, []string{c.provider.Driver()}) } func (c *AuthController) AuthCallback(w http.ResponseWriter, r *http.Request, driver oapi.AuthCallbackParamsDriver) { @@ -43,8 +53,25 @@ func (c *AuthController) AuthCallback(w http.ResponseWriter, r *http.Request, dr return } + // Read the body so we can validate the redirect parameter, then restore it + // for the provider's Authenticate method which also reads from r.Body. + body, err := io.ReadAll(r.Body) + if err != nil { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("failed to read request body"))) + return + } + r.Body = io.NopCloser(bytes.NewReader(body)) + + var req oapi.AuthCallbackRequest + if jsonErr := json.Unmarshal(body, &req); jsonErr == nil && req.Redirect != nil { + if validateErr := c.validateRedirect(*req.Redirect); validateErr != nil { + oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("invalid redirect URL"))) + return + } + } + ctx := r.Context() - _, err := c.provider.Authenticate(ctx, w, r) + _, err = c.provider.Authenticate(ctx, w, r) if err != nil { c.logger.Error("auth validation failed", zap.String("driver", string(driver)), zap.Error(err)) c.writeAuthError(w, err) @@ -54,6 +81,33 @@ func (c *AuthController) AuthCallback(w http.ResponseWriter, r *http.Request, dr w.WriteHeader(http.StatusOK) } +// validateRedirect checks that the redirect URL is safe to redirect to. +// Relative URLs (starting with "/" but not "//") are always allowed. +// Absolute URLs are only allowed if their hostname appears in the +// configured AllowedRedirectHosts list. +func (c *AuthController) validateRedirect(redirect string) error { + // Allow relative URLs that start with "/" but not "//" (protocol-relative). + // Protocol-relative URLs such as "//evil.com/path" are treated as absolute + // by browsers and would redirect to an external host. + if strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") { + return nil + } + + parsed, err := url.Parse(redirect) + if err != nil { + return ErrInvalidRedirect + } + + host := parsed.Hostname() + for _, allowed := range c.allowedRedirectHosts { + if host == allowed { + return nil + } + } + + return ErrInvalidRedirect +} + func (c *AuthController) AuthWebhook(w http.ResponseWriter, r *http.Request, driver oapi.AuthWebhookParamsDriver) { if string(driver) != c.provider.Driver() { oapi.WriteProblem(w, problem.ErrNotFound(problem.Describe("auth driver not found"))) diff --git a/internal/http/controllers/v1/management/auth_test.go b/internal/http/controllers/v1/management/auth_test.go index 5b4ab890..a3f35b31 100644 --- a/internal/http/controllers/v1/management/auth_test.go +++ b/internal/http/controllers/v1/management/auth_test.go @@ -3,6 +3,7 @@ package v1 import ( "encoding/json" "net/http/httptest" + "strings" "testing" "github.com/lunogram/platform/internal/config" @@ -140,3 +141,119 @@ func TestAuthWebhookWithInvalidDriver(t *testing.T) { }) } } + +func TestValidateRedirect(t *testing.T) { + t.Parallel() + + type test struct { + redirect string + allowedRedirectHosts []string + wantErr bool + } + + tests := map[string]test{ + "relative path is always allowed": { + redirect: "/dashboard", + allowedRedirectHosts: nil, + wantErr: false, + }, + "relative root is always allowed": { + redirect: "/", + allowedRedirectHosts: nil, + wantErr: false, + }, + "protocol-relative url is rejected": { + redirect: "//evil.com/steal", + allowedRedirectHosts: nil, + wantErr: true, + }, + "absolute url with allowed host": { + redirect: "https://app.example.com/dashboard", + allowedRedirectHosts: []string{"app.example.com"}, + wantErr: false, + }, + "absolute url with disallowed host": { + redirect: "https://evil.example.com/steal", + allowedRedirectHosts: []string{"app.example.com"}, + wantErr: true, + }, + "absolute url with no allowed hosts configured": { + redirect: "https://app.example.com/dashboard", + allowedRedirectHosts: nil, + wantErr: true, + }, + "absolute url host must match exactly": { + redirect: "https://notapp.example.com/path", + allowedRedirectHosts: []string{"app.example.com"}, + wantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + c := &AuthController{ + allowedRedirectHosts: test.allowedRedirectHosts, + } + err := c.validateRedirect(test.redirect) + if test.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestAuthCallbackRejectsDisallowedRedirect(t *testing.T) { + t.Parallel() + + logger := zaptest.NewLogger(t) + mgmt, _, _ := teststore.RunPostgreSQL(t) + + cfg := config.Node{ + Auth: config.Auth{ + Driver: "clerk", + Clerk: config.ClerkAuth{ + SecretKey: "sk_test_xxx", + }, + AllowedRedirectHosts: []string{"app.example.com"}, + }, + } + + controller, err := NewAuthController(logger, mgmt, cfg, nil) + require.NoError(t, err) + + type test struct { + redirect string + code int + } + + tests := map[string]test{ + "disallowed absolute redirect returns 400": { + redirect: "https://evil.com/steal", + code: 400, + }, + "allowed absolute redirect passes redirect validation": { + redirect: "https://app.example.com/dashboard", + code: 401, + }, + "relative redirect always passes": { + redirect: "/dashboard", + code: 401, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + body := strings.NewReader(`{"redirect":"` + test.redirect + `"}`) + res := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/v1/auth/callback/clerk", body) + req.Header.Set("Content-Type", "application/json") + controller.AuthCallback(res, req, oapi.AuthCallbackParamsDriverClerk) + + require.Equal(t, test.code, res.Code, res.Body.String()) + }) + } +} diff --git a/internal/wasm/test/action/go.sum b/internal/wasm/test/action/go.sum index c15d3829..643851fc 100644 --- a/internal/wasm/test/action/go.sum +++ b/internal/wasm/test/action/go.sum @@ -1,2 +1,10 @@ +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/extism/go-pdk v1.1.3 h1:hfViMPWrqjN6u67cIYRALZTZLk/enSPpNKa+rZ9X2SQ= github.com/extism/go-pdk v1.1.3/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/wasm/test/provider/go.sum b/internal/wasm/test/provider/go.sum index c15d3829..643851fc 100644 --- a/internal/wasm/test/provider/go.sum +++ b/internal/wasm/test/provider/go.sum @@ -1,2 +1,10 @@ +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/extism/go-pdk v1.1.3 h1:hfViMPWrqjN6u67cIYRALZTZLk/enSPpNKa+rZ9X2SQ= github.com/extism/go-pdk v1.1.3/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/modules/actions/webhook/go.sum b/modules/actions/webhook/go.sum index c15d3829..643851fc 100644 --- a/modules/actions/webhook/go.sum +++ b/modules/actions/webhook/go.sum @@ -1,2 +1,10 @@ +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/extism/go-pdk v1.1.3 h1:hfViMPWrqjN6u67cIYRALZTZLk/enSPpNKa+rZ9X2SQ= github.com/extism/go-pdk v1.1.3/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/modules/providers/logger/go.sum b/modules/providers/logger/go.sum index c15d3829..643851fc 100644 --- a/modules/providers/logger/go.sum +++ b/modules/providers/logger/go.sum @@ -1,2 +1,10 @@ +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/extism/go-pdk v1.1.3 h1:hfViMPWrqjN6u67cIYRALZTZLk/enSPpNKa+rZ9X2SQ= github.com/extism/go-pdk v1.1.3/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/modules/providers/twilio/go.sum b/modules/providers/twilio/go.sum index c15d3829..643851fc 100644 --- a/modules/providers/twilio/go.sum +++ b/modules/providers/twilio/go.sum @@ -1,2 +1,10 @@ +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= +github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/extism/go-pdk v1.1.3 h1:hfViMPWrqjN6u67cIYRALZTZLk/enSPpNKa+rZ9X2SQ= github.com/extism/go-pdk v1.1.3/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From c86e4df20a34f00c9418e39e242f6a702f9727d2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Mar 2026 12:11:24 +0000 Subject: [PATCH 3/3] Use internal/http/json package and decode body once in AuthCallback Co-authored-by: jeroenrinzema <3440116+jeroenrinzema@users.noreply.github.com> --- .../http/controllers/v1/management/auth.go | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/http/controllers/v1/management/auth.go b/internal/http/controllers/v1/management/auth.go index a79def38..87b78ba0 100644 --- a/internal/http/controllers/v1/management/auth.go +++ b/internal/http/controllers/v1/management/auth.go @@ -2,7 +2,6 @@ package v1 import ( "bytes" - "encoding/json" "errors" "io" "net/http" @@ -13,7 +12,7 @@ import ( "github.com/lunogram/platform/internal/config" "github.com/lunogram/platform/internal/http/auth/providers" "github.com/lunogram/platform/internal/http/controllers/v1/management/oapi" - httpjson "github.com/lunogram/platform/internal/http/json" + "github.com/lunogram/platform/internal/http/json" "github.com/lunogram/platform/internal/http/problem" "github.com/lunogram/platform/internal/rbac" "github.com/lunogram/platform/internal/store/management" @@ -44,7 +43,7 @@ type AuthController struct { } func (c *AuthController) GetAuthMethods(w http.ResponseWriter, r *http.Request) { - httpjson.Write(w, http.StatusOK, []string{c.provider.Driver()}) + json.Write(w, http.StatusOK, []string{c.provider.Driver()}) } func (c *AuthController) AuthCallback(w http.ResponseWriter, r *http.Request, driver oapi.AuthCallbackParamsDriver) { @@ -53,23 +52,29 @@ func (c *AuthController) AuthCallback(w http.ResponseWriter, r *http.Request, dr return } - // Read the body so we can validate the redirect parameter, then restore it - // for the provider's Authenticate method which also reads from r.Body. - body, err := io.ReadAll(r.Body) - if err != nil { + // Decode the body once to validate the redirect parameter, then re-encode + // it so the provider can read it without consuming the body a second time. + var req oapi.AuthCallbackRequest + if err := json.Decode(r.Body, &req); err != nil { oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("failed to read request body"))) return } - r.Body = io.NopCloser(bytes.NewReader(body)) - var req oapi.AuthCallbackRequest - if jsonErr := json.Unmarshal(body, &req); jsonErr == nil && req.Redirect != nil { - if validateErr := c.validateRedirect(*req.Redirect); validateErr != nil { + if req.Redirect != nil { + if err := c.validateRedirect(*req.Redirect); err != nil { oapi.WriteProblem(w, problem.ErrBadRequest(problem.Describe("invalid redirect URL"))) return } } + encoded, err := json.Marshal(req) + if err != nil { + oapi.WriteProblem(w, problem.ErrInternal(problem.Describe("authentication failed"))) + return + } + + r.Body = io.NopCloser(bytes.NewReader(encoded)) + ctx := r.Context() _, err = c.provider.Authenticate(ctx, w, r) if err != nil {