diff --git a/internal/graphql/forgot_password.go b/internal/graphql/forgot_password.go index dbd25e8f..2cd53f37 100644 --- a/internal/graphql/forgot_password.go +++ b/internal/graphql/forgot_password.go @@ -77,7 +77,7 @@ func (g *graphqlProvider) ForgotPassword(ctx context.Context, params *model.Forg // give higher preference to params redirect uri if strings.TrimSpace(refs.StringValue(params.RedirectURI)) != "" { redirectURI = refs.StringValue(params.RedirectURI) - if !validators.IsValidOrigin(redirectURI, g.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURI, g.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") return nil, fmt.Errorf("invalid redirect URI") } diff --git a/internal/graphql/invite_members.go b/internal/graphql/invite_members.go index 1f4dfd35..7fcb185a 100644 --- a/internal/graphql/invite_members.go +++ b/internal/graphql/invite_members.go @@ -90,7 +90,7 @@ func (g *graphqlProvider) InviteMembers(ctx context.Context, params *model.Invit redirectURL := appURL if params.RedirectURI != nil { redirectURL = *params.RedirectURI - if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") return nil, fmt.Errorf("invalid redirect URI") } diff --git a/internal/graphql/magic_link_login.go b/internal/graphql/magic_link_login.go index 1c431f0b..5a8d2622 100644 --- a/internal/graphql/magic_link_login.go +++ b/internal/graphql/magic_link_login.go @@ -150,7 +150,7 @@ func (g *graphqlProvider) MagicLinkLogin(ctx context.Context, params *model.Magi redirectURL := parsers.GetAppURL(gc) if params.RedirectURI != nil { redirectURL = *params.RedirectURI - if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") return nil, fmt.Errorf("invalid redirect URI") } diff --git a/internal/graphql/signup.go b/internal/graphql/signup.go index 0f15f92f..c1d8d165 100644 --- a/internal/graphql/signup.go +++ b/internal/graphql/signup.go @@ -211,7 +211,7 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques redirectURL := parsers.GetAppURL(gc) if params.RedirectURI != nil { redirectURL = *params.RedirectURI - if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") return nil, fmt.Errorf("invalid redirect URI") } diff --git a/internal/http_handlers/app.go b/internal/http_handlers/app.go index 4362e0fc..4c1bd793 100644 --- a/internal/http_handlers/app.go +++ b/internal/http_handlers/app.go @@ -43,7 +43,7 @@ func (h *httpProvider) AppHandler() gin.HandlerFunc { redirectURI = hostname + "/app" } else { // validate redirect url with allowed origins - if !validators.IsValidOrigin(redirectURI, h.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect url") c.JSON(400, gin.H{"error": "invalid redirect url"}) return diff --git a/internal/http_handlers/authorize.go b/internal/http_handlers/authorize.go index 20342be8..45decc7c 100644 --- a/internal/http_handlers/authorize.go +++ b/internal/http_handlers/authorize.go @@ -44,6 +44,7 @@ import ( "github.com/authorizerdev/authorizer/internal/cookie" "github.com/authorizerdev/authorizer/internal/parsers" "github.com/authorizerdev/authorizer/internal/token" + "github.com/authorizerdev/authorizer/internal/validators" ) // Check the flow for generating and verifying codes: https://developer.okta.com/blog/2019/08/22/okta-authjs-pkce#:~:text=PKCE%20works%20by%20having%20the,is%20called%20the%20Code%20Challenge. @@ -90,6 +91,16 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc { if redirectURI == "" { redirectURI = "/app" + } else { + hostname := parsers.GetHost(gc) + if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) { + log.Debug().Msg("Invalid redirect URI") + gc.JSON(http.StatusBadRequest, gin.H{ + "error": "invalid_request", + "error_description": "invalid redirect_uri", + }) + return + } } if responseType == "" { diff --git a/internal/http_handlers/logout.go b/internal/http_handlers/logout.go index 4df663ad..d2c3df1c 100644 --- a/internal/http_handlers/logout.go +++ b/internal/http_handlers/logout.go @@ -11,8 +11,10 @@ import ( "github.com/authorizerdev/authorizer/internal/constants" "github.com/authorizerdev/authorizer/internal/cookie" "github.com/authorizerdev/authorizer/internal/crypto" + "github.com/authorizerdev/authorizer/internal/parsers" "github.com/authorizerdev/authorizer/internal/token" "github.com/authorizerdev/authorizer/internal/utils" + "github.com/authorizerdev/authorizer/internal/validators" ) // Handler to logout user @@ -69,6 +71,14 @@ func (h *httpProvider) LogoutHandler() gin.HandlerFunc { }) if redirectURL != "" { + hostname := parsers.GetHost(gc) + if !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) { + log.Debug().Msg("Invalid redirect URI") + gc.JSON(http.StatusBadRequest, gin.H{ + "error": "invalid redirect uri", + }) + return + } gc.Redirect(http.StatusFound, redirectURL) } else { gc.JSON(http.StatusOK, gin.H{ diff --git a/internal/http_handlers/oauth_login.go b/internal/http_handlers/oauth_login.go index 2d772e09..487573d9 100644 --- a/internal/http_handlers/oauth_login.go +++ b/internal/http_handlers/oauth_login.go @@ -7,6 +7,7 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" + "github.com/authorizerdev/authorizer/internal/parsers" "github.com/authorizerdev/authorizer/internal/validators" ) @@ -31,7 +32,8 @@ func (h *httpProvider) OAuthLoginHandler() gin.HandlerFunc { return } - if !validators.IsValidOrigin(redirectURI, h.Config.AllowedOrigins) { + hostname := parsers.GetHost(c) + if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") c.JSON(400, gin.H{ "error": "invalid redirect uri", diff --git a/internal/http_handlers/verify_email.go b/internal/http_handlers/verify_email.go index 6811ce41..6e4cdd39 100644 --- a/internal/http_handlers/verify_email.go +++ b/internal/http_handlers/verify_email.go @@ -24,8 +24,9 @@ import ( func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { log := h.Log.With().Str("func", "VerifyEmailHandler").Logger() return func(c *gin.Context) { + hostname := parsers.GetHost(c) redirectURL := strings.TrimSpace(c.Query("redirect_uri")) - if redirectURL != "" && !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) { + if redirectURL != "" && !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI") c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"}) return @@ -49,7 +50,6 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { } // verify if token exists in db - hostname := parsers.GetHost(c) claim, err := h.TokenProvider.ParseJWTToken(tokenInQuery) if err != nil { log.Debug().Err(err).Msg("Error parsing jwt token") @@ -199,7 +199,7 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc { if redirectURL == "" { redirectURL = claim["redirect_uri"].(string) } - if !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) { + if !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) { log.Debug().Msg("Invalid redirect URI in token claim") c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"}) return diff --git a/internal/integration_tests/redirect_uri_validation_test.go b/internal/integration_tests/redirect_uri_validation_test.go index fc1153c0..3dfc830c 100644 --- a/internal/integration_tests/redirect_uri_validation_test.go +++ b/internal/integration_tests/redirect_uri_validation_test.go @@ -1,65 +1,30 @@ package integration_tests import ( - "fmt" "testing" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/authorizerdev/authorizer/internal/constants" - "github.com/authorizerdev/authorizer/internal/crypto" + "github.com/authorizerdev/authorizer/internal/config" "github.com/authorizerdev/authorizer/internal/graph/model" "github.com/authorizerdev/authorizer/internal/refs" ) -// TestRedirectURIValidation verifies that all endpoints accepting redirect_uri -// validate it against AllowedOrigins to prevent open-redirect token theft. -func TestRedirectURIValidation(t *testing.T) { - cfg := getTestConfig() - cfg.AllowedOrigins = []string{"http://localhost:3000"} - cfg.EnableMagicLinkLogin = true - cfg.EnableEmailVerification = true - cfg.IsEmailServiceEnabled = true - cfg.IsSMSServiceEnabled = true - cfg.SMTPHost = "localhost" - cfg.SMTPPort = 1025 - cfg.SMTPSenderEmail = "test@authorizer.dev" - cfg.SMTPSenderName = "Test" - cfg.SMTPLocalName = "Test" - cfg.SMTPSkipTLSVerification = true - - ts := initTestSetup(t, cfg) - _, ctx := createContext(ts) - - attackerURL := "https://attacker.com/steal" - validURL := "http://localhost:3000/callback" - - t.Run("ForgotPassword should reject invalid redirect_uri", func(t *testing.T) { - // First create a user - email := "fp_redirect_test_" + uuid.New().String() + "@authorizer.dev" - password := "Password@123" - signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ - Email: &email, - Password: password, - ConfirmPassword: password, - }) - require.NoError(t, err) - require.NotNil(t, signupRes) +// TestRedirectURIRejectsAttacker verifies that forgot_password rejects +// attacker-controlled redirect_uri values with explicit AllowedOrigins. +func TestRedirectURIRejectsAttacker(t *testing.T) { + runForEachDB(t, func(t *testing.T, cfg *config.Config) { + cfg.AllowedOrigins = []string{"http://localhost:3000"} + cfg.EnableBasicAuthentication = true + cfg.EnableEmailVerification = false + cfg.EnableSignup = true - // Attacker-controlled redirect_uri should be rejected - res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ - Email: refs.NewStringRef(email), - RedirectURI: refs.NewStringRef(attackerURL), - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Contains(t, err.Error(), "invalid redirect URI") - }) + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) - t.Run("ForgotPassword should accept valid redirect_uri", func(t *testing.T) { - email := "fp_valid_redirect_" + uuid.New().String() + "@authorizer.dev" + email := "redirect_test_" + uuid.New().String() + "@authorizer.dev" password := "Password@123" signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ Email: &email, @@ -69,98 +34,96 @@ func TestRedirectURIValidation(t *testing.T) { require.NoError(t, err) require.NotNil(t, signupRes) - res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ - Email: refs.NewStringRef(email), - RedirectURI: refs.NewStringRef(validURL), + t.Run("rejects attacker redirect_uri", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef("https://attacker.com/steal"), + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Contains(t, err.Error(), "invalid redirect URI") }) - assert.NoError(t, err) - assert.NotNil(t, res) - }) - t.Run("MagicLinkLogin should reject invalid redirect_uri", func(t *testing.T) { - email := "ml_redirect_test_" + uuid.New().String() + "@authorizer.dev" - res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{ - Email: email, - RedirectURI: &attackerURL, + t.Run("accepts valid redirect_uri", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef("http://localhost:3000/reset"), + }) + assert.NoError(t, err) + assert.NotNil(t, res) }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Contains(t, err.Error(), "invalid redirect URI") }) +} - t.Run("MagicLinkLogin should accept valid redirect_uri", func(t *testing.T) { - email := "ml_valid_redirect_" + uuid.New().String() + "@authorizer.dev" - res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{ - Email: email, - RedirectURI: &validURL, - }) - assert.NoError(t, err) - assert.NotNil(t, res) - }) +// TestRedirectURIWildcardOrigins is a regression test for the open redirect +// vulnerability (issue #540). When allowed_origins=["*"] (the default config), +// attacker-controlled redirect_uri values must still be rejected. +func TestRedirectURIWildcardOrigins(t *testing.T) { + runForEachDB(t, func(t *testing.T, cfg *config.Config) { + cfg.AllowedOrigins = []string{"*"} + cfg.EnableBasicAuthentication = true + cfg.EnableEmailVerification = false + cfg.EnableSignup = true + ts := initTestSetup(t, cfg) + _, ctx := createContext(ts) - t.Run("Signup should reject invalid redirect_uri", func(t *testing.T) { - email := "signup_redirect_test_" + uuid.New().String() + "@authorizer.dev" + email := "wildcard_redirect_" + uuid.New().String() + "@authorizer.dev" password := "Password@123" - res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ - Email: &email, - Password: password, - ConfirmPassword: password, - RedirectURI: &attackerURL, - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Contains(t, err.Error(), "invalid redirect URI") - }) - t.Run("Signup should accept valid redirect_uri", func(t *testing.T) { - email := "signup_valid_redirect_" + uuid.New().String() + "@authorizer.dev" - password := "Password@123" - res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ + signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{ Email: &email, Password: password, ConfirmPassword: password, - RedirectURI: &validURL, }) - assert.NoError(t, err) - assert.NotNil(t, res) - }) - - t.Run("InviteMembers should reject invalid redirect_uri", func(t *testing.T) { - cfg.IsEmailServiceEnabled = true - cfg.EnableBasicAuthentication = true - cfg.EnableMagicLinkLogin = true - - req, _ := createContext(ts) - h, err := crypto.EncryptPassword(cfg.AdminSecret) require.NoError(t, err) - req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h)) + require.NotNil(t, signupRes) - emailTo := "invite_redirect_test_" + uuid.New().String() + "@authorizer.dev" - res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{ - Emails: []string{emailTo}, - RedirectURI: &attackerURL, + t.Run("rejects attacker redirect_uri with wildcard origins", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef("https://attacker.com/capture"), + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Contains(t, err.Error(), "invalid redirect URI") }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Contains(t, err.Error(), "invalid redirect URI") - }) - t.Run("InviteMembers should accept valid redirect_uri", func(t *testing.T) { - cfg.IsEmailServiceEnabled = true - cfg.EnableBasicAuthentication = true - cfg.EnableMagicLinkLogin = true + t.Run("allows self-origin redirect_uri with wildcard origins", func(t *testing.T) { + selfURI := "http://" + ts.HttpServer.Listener.Addr().String() + "/app/reset-password" + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef(selfURI), + }) + assert.NoError(t, err) + assert.NotNil(t, res) + assert.NotEmpty(t, res.Message) + }) - req, _ := createContext(ts) - h, err := crypto.EncryptPassword(cfg.AdminSecret) - require.NoError(t, err) - req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h)) + t.Run("works without redirect_uri (uses default)", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + }) + assert.NoError(t, err) + assert.NotNil(t, res) + assert.NotEmpty(t, res.Message) + }) + + t.Run("rejects javascript scheme", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef("javascript:alert(1)"), + }) + assert.Error(t, err) + assert.Nil(t, res) + }) - emailTo := "invite_valid_redirect_" + uuid.New().String() + "@authorizer.dev" - res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{ - Emails: []string{emailTo}, - RedirectURI: &validURL, + t.Run("rejects data scheme", func(t *testing.T) { + res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{ + Email: refs.NewStringRef(email), + RedirectURI: refs.NewStringRef("data:text/html,