Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/graphql/forgot_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/graphql/invite_members.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/graphql/magic_link_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/graphql/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/http_handlers/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions internal/http_handlers/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 == "" {
Expand Down
10 changes: 10 additions & 0 deletions internal/http_handlers/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 3 additions & 1 deletion internal/http_handlers/oauth_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions internal/http_handlers/verify_email.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
205 changes: 84 additions & 121 deletions internal/integration_tests/redirect_uri_validation_test.go
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,<h1>evil</h1>"),
})
assert.Error(t, err)
assert.Nil(t, res)
})
assert.NoError(t, err)
assert.NotNil(t, res)
})
}
Loading