Skip to content

Commit 6d9bef1

Browse files
authored
Fix/validate redirect uri (#502)
* fix: validate redirect_uri against AllowedOrigins to prevent open redirect token theft Six endpoints accepted user-controlled redirect_uri without validating against AllowedOrigins, allowing attackers to steal tokens by redirecting to malicious URLs. Added validators.IsValidOrigin() checks to: - ForgotPassword (GraphQL mutation) - MagicLinkLogin (GraphQL mutation) - SignUp (GraphQL mutation) - InviteMembers (GraphQL mutation) - OAuthLoginHandler (HTTP handler) - VerifyEmailHandler (HTTP handler, both query param and JWT claim fallback) * test: add redirect_uri validation tests for open-redirect prevention Tests verify that ForgotPassword, MagicLinkLogin, SignUp, and InviteMembers reject attacker-controlled redirect_uri values not matching AllowedOrigins, and accept valid ones. * test: fix redirect_uri validation tests to use correct AllowedOrigins format IsValidOrigin compares hostname:port (without protocol), so AllowedOrigins must be specified without the http:// prefix. Also use SQLite to allow tests to run without Docker/Postgres. * fix: rewrite IsValidOrigin to use net/url and normalize origins correctly The previous implementation compared allowed origins (e.g. "https://example.com") as raw regex against "hostname:port", so protocols in AllowedOrigins caused matches to always fail. Now both input URL and allowed origins are normalized via net/url.Parse to strip protocol/path before comparison. Also anchors the regex with ^...$ to prevent partial matches (e.g. "example.com" matching "notexample.com"). Adds comprehensive unit tests covering: exact domains, custom ports, standard ports (80/443), bare domains without protocol, subdomains, deep subdomains, wildcard subdomains, wildcard with port, multiple origins, attacker URLs, www variants, and live domain scenarios. * fix: update InviteMembers test to use allowed origin for redirect_uri The test was using "https://authorizer.dev/" which is not in AllowedOrigins, so it now correctly gets rejected by the new redirect_uri validation. Updated to use "http://localhost:3000/" which matches the test config's AllowedOrigins.
1 parent 73679fa commit 6d9bef1

11 files changed

Lines changed: 983 additions & 19 deletions

File tree

ROADMAP_V2.md

Lines changed: 547 additions & 0 deletions
Large diffs are not rendered by default.

internal/graphql/forgot_password.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/authorizerdev/authorizer/internal/storage/schemas"
1717
"github.com/authorizerdev/authorizer/internal/token"
1818
"github.com/authorizerdev/authorizer/internal/utils"
19+
"github.com/authorizerdev/authorizer/internal/validators"
1920
)
2021

2122
// ForgotPassword is a for forgot password mutation
@@ -75,6 +76,10 @@ func (g *graphqlProvider) ForgotPassword(ctx context.Context, params *model.Forg
7576
// give higher preference to params redirect uri
7677
if strings.TrimSpace(refs.StringValue(params.RedirectURI)) != "" {
7778
redirectURI = refs.StringValue(params.RedirectURI)
79+
if !validators.IsValidOrigin(redirectURI, g.Config.AllowedOrigins) {
80+
log.Debug().Msg("Invalid redirect URI")
81+
return nil, fmt.Errorf("invalid redirect URI")
82+
}
7883
} else {
7984
redirectURI = g.Config.ResetPasswordURL
8085
if redirectURI == "" {

internal/graphql/invite_members.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ func (g *graphqlProvider) InviteMembers(ctx context.Context, params *model.Invit
8989
redirectURL := appURL
9090
if params.RedirectURI != nil {
9191
redirectURL = *params.RedirectURI
92+
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
93+
log.Debug().Msg("Invalid redirect URI")
94+
return nil, fmt.Errorf("invalid redirect URI")
95+
}
9296
}
9397

9498
_, nonceHash, err := utils.GenerateNonce()

internal/graphql/magic_link_login.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ func (g *graphqlProvider) MagicLinkLogin(ctx context.Context, params *model.Magi
149149
redirectURL := parsers.GetAppURL(gc)
150150
if params.RedirectURI != nil {
151151
redirectURL = *params.RedirectURI
152+
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
153+
log.Debug().Msg("Invalid redirect URI")
154+
return nil, fmt.Errorf("invalid redirect URI")
155+
}
152156
}
153157

154158
if strings.Contains(redirectURL, "?") {

internal/graphql/signup.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,10 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
210210
redirectURL := parsers.GetAppURL(gc)
211211
if params.RedirectURI != nil {
212212
redirectURL = *params.RedirectURI
213+
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
214+
log.Debug().Msg("Invalid redirect URI")
215+
return nil, fmt.Errorf("invalid redirect URI")
216+
}
213217
}
214218
verificationToken, err := g.TokenProvider.CreateVerificationToken(&token.AuthTokenConfig{
215219
Nonce: nonceHash,

internal/http_handlers/oauth_login.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ func (h *httpProvider) OAuthLoginHandler() gin.HandlerFunc {
3131
return
3232
}
3333

34+
if !validators.IsValidOrigin(redirectURI, h.Config.AllowedOrigins) {
35+
log.Debug().Msg("Invalid redirect URI")
36+
c.JSON(400, gin.H{
37+
"error": "invalid redirect uri",
38+
})
39+
return
40+
}
41+
3442
if state == "" {
3543
log.Debug().Msg("state is missing, creating new state")
3644
state = uuid.New().String()

internal/http_handlers/verify_email.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc {
2525
log := h.Log.With().Str("func", "VerifyEmailHandler").Logger()
2626
return func(c *gin.Context) {
2727
redirectURL := strings.TrimSpace(c.Query("redirect_uri"))
28+
if redirectURL != "" && !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) {
29+
log.Debug().Msg("Invalid redirect URI")
30+
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"})
31+
return
32+
}
2833
errorRes := gin.H{
2934
"error": "token is required",
3035
}
@@ -194,6 +199,11 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc {
194199
if redirectURL == "" {
195200
redirectURL = claim["redirect_uri"].(string)
196201
}
202+
if !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) {
203+
log.Debug().Msg("Invalid redirect URI in token claim")
204+
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"})
205+
return
206+
}
197207

198208
if strings.Contains(redirectURL, "?") {
199209
redirectURL = redirectURL + "&" + params

internal/integration_tests/invite_members_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestInviteMembersUser(t *testing.T) {
2222
email := "invite_user_test_" + uuid.New().String() + "@authorizer.dev"
2323
emailTo := "test_user_invitation_" + uuid.New().String() + "@authorizer.dev"
2424
password := "Password@123"
25-
url := "https://authorizer.dev/"
25+
url := "http://localhost:3000/"
2626
// Signup the user
2727
signupReq := &model.SignUpRequest{
2828
Email: &email,
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
package integration_tests
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/authorizerdev/authorizer/internal/constants"
12+
"github.com/authorizerdev/authorizer/internal/crypto"
13+
"github.com/authorizerdev/authorizer/internal/graph/model"
14+
"github.com/authorizerdev/authorizer/internal/refs"
15+
)
16+
17+
// TestRedirectURIValidation verifies that all endpoints accepting redirect_uri
18+
// validate it against AllowedOrigins to prevent open-redirect token theft.
19+
func TestRedirectURIValidation(t *testing.T) {
20+
cfg := getTestConfig()
21+
// Use SQLite so tests can run without Postgres/Docker
22+
cfg.DatabaseType = constants.DbTypeSqlite
23+
cfg.DatabaseURL = "authorizer_redirect_test.db"
24+
cfg.AllowedOrigins = []string{"http://localhost:3000"}
25+
cfg.EnableMagicLinkLogin = true
26+
cfg.EnableEmailVerification = true
27+
cfg.IsEmailServiceEnabled = true
28+
cfg.IsSMSServiceEnabled = true
29+
cfg.SMTPHost = "localhost"
30+
cfg.SMTPPort = 1025
31+
cfg.SMTPSenderEmail = "test@authorizer.dev"
32+
cfg.SMTPSenderName = "Test"
33+
cfg.SMTPLocalName = "Test"
34+
cfg.SMTPSkipTLSVerification = true
35+
36+
ts := initTestSetup(t, cfg)
37+
_, ctx := createContext(ts)
38+
39+
attackerURL := "https://attacker.com/steal"
40+
validURL := "http://localhost:3000/callback"
41+
42+
t.Run("ForgotPassword should reject invalid redirect_uri", func(t *testing.T) {
43+
// First create a user
44+
email := "fp_redirect_test_" + uuid.New().String() + "@authorizer.dev"
45+
password := "Password@123"
46+
signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
47+
Email: &email,
48+
Password: password,
49+
ConfirmPassword: password,
50+
})
51+
require.NoError(t, err)
52+
require.NotNil(t, signupRes)
53+
54+
// Attacker-controlled redirect_uri should be rejected
55+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
56+
Email: refs.NewStringRef(email),
57+
RedirectURI: refs.NewStringRef(attackerURL),
58+
})
59+
assert.Error(t, err)
60+
assert.Nil(t, res)
61+
assert.Contains(t, err.Error(), "invalid redirect URI")
62+
})
63+
64+
t.Run("ForgotPassword should accept valid redirect_uri", func(t *testing.T) {
65+
email := "fp_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
66+
password := "Password@123"
67+
signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
68+
Email: &email,
69+
Password: password,
70+
ConfirmPassword: password,
71+
})
72+
require.NoError(t, err)
73+
require.NotNil(t, signupRes)
74+
75+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
76+
Email: refs.NewStringRef(email),
77+
RedirectURI: refs.NewStringRef(validURL),
78+
})
79+
assert.NoError(t, err)
80+
assert.NotNil(t, res)
81+
})
82+
83+
t.Run("MagicLinkLogin should reject invalid redirect_uri", func(t *testing.T) {
84+
email := "ml_redirect_test_" + uuid.New().String() + "@authorizer.dev"
85+
res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{
86+
Email: email,
87+
RedirectURI: &attackerURL,
88+
})
89+
assert.Error(t, err)
90+
assert.Nil(t, res)
91+
assert.Contains(t, err.Error(), "invalid redirect URI")
92+
})
93+
94+
t.Run("MagicLinkLogin should accept valid redirect_uri", func(t *testing.T) {
95+
email := "ml_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
96+
res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{
97+
Email: email,
98+
RedirectURI: &validURL,
99+
})
100+
assert.NoError(t, err)
101+
assert.NotNil(t, res)
102+
})
103+
104+
t.Run("Signup should reject invalid redirect_uri", func(t *testing.T) {
105+
email := "signup_redirect_test_" + uuid.New().String() + "@authorizer.dev"
106+
password := "Password@123"
107+
res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
108+
Email: &email,
109+
Password: password,
110+
ConfirmPassword: password,
111+
RedirectURI: &attackerURL,
112+
})
113+
assert.Error(t, err)
114+
assert.Nil(t, res)
115+
assert.Contains(t, err.Error(), "invalid redirect URI")
116+
})
117+
118+
t.Run("Signup should accept valid redirect_uri", func(t *testing.T) {
119+
email := "signup_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
120+
password := "Password@123"
121+
res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
122+
Email: &email,
123+
Password: password,
124+
ConfirmPassword: password,
125+
RedirectURI: &validURL,
126+
})
127+
assert.NoError(t, err)
128+
assert.NotNil(t, res)
129+
})
130+
131+
t.Run("InviteMembers should reject invalid redirect_uri", func(t *testing.T) {
132+
cfg.IsEmailServiceEnabled = true
133+
cfg.EnableBasicAuthentication = true
134+
cfg.EnableMagicLinkLogin = true
135+
136+
req, _ := createContext(ts)
137+
h, err := crypto.EncryptPassword(cfg.AdminSecret)
138+
require.NoError(t, err)
139+
req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h))
140+
141+
emailTo := "invite_redirect_test_" + uuid.New().String() + "@authorizer.dev"
142+
res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{
143+
Emails: []string{emailTo},
144+
RedirectURI: &attackerURL,
145+
})
146+
assert.Error(t, err)
147+
assert.Nil(t, res)
148+
assert.Contains(t, err.Error(), "invalid redirect URI")
149+
})
150+
151+
t.Run("InviteMembers should accept valid redirect_uri", func(t *testing.T) {
152+
cfg.IsEmailServiceEnabled = true
153+
cfg.EnableBasicAuthentication = true
154+
cfg.EnableMagicLinkLogin = true
155+
156+
req, _ := createContext(ts)
157+
h, err := crypto.EncryptPassword(cfg.AdminSecret)
158+
require.NoError(t, err)
159+
req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h))
160+
161+
emailTo := "invite_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
162+
res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{
163+
Emails: []string{emailTo},
164+
RedirectURI: &validURL,
165+
})
166+
assert.NoError(t, err)
167+
assert.NotNil(t, res)
168+
})
169+
}

internal/validators/url.go

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,33 @@
11
package validators
22

33
import (
4+
"net/url"
45
"regexp"
56
"strings"
6-
7-
"github.com/authorizerdev/authorizer/internal/parsers"
87
)
98

9+
// normalizeOrigin extracts hostname:port from a URL or origin string.
10+
// Standard ports (80/443) and absent ports are omitted so that
11+
// "https://example.com" and "https://example.com:443" both normalise to "example.com".
12+
func normalizeOrigin(raw string) string {
13+
raw = strings.TrimSpace(raw)
14+
if !strings.HasPrefix(raw, "http://") && !strings.HasPrefix(raw, "https://") {
15+
raw = "https://" + raw
16+
}
17+
u, err := url.Parse(raw)
18+
if err != nil {
19+
return raw
20+
}
21+
host := u.Hostname()
22+
port := u.Port()
23+
if port == "" || port == "443" || port == "80" {
24+
return host
25+
}
26+
return host + ":" + port
27+
}
28+
1029
// IsValidOrigin validates origin based on ALLOWED_ORIGINS
11-
func IsValidOrigin(url string, allowedOriginsConfig []string) bool {
30+
func IsValidOrigin(inputURL string, allowedOriginsConfig []string) bool {
1231
allowedOrigins := allowedOriginsConfig
1332
if len(allowedOrigins) == 0 {
1433
allowedOrigins = []string{"*"}
@@ -17,31 +36,30 @@ func IsValidOrigin(url string, allowedOriginsConfig []string) bool {
1736
return true
1837
}
1938

20-
hasValidURL := false
21-
hostName, port := parsers.GetHostParts(url)
22-
currentOrigin := hostName + ":" + port
39+
currentOrigin := normalizeOrigin(inputURL)
2340

2441
for _, origin := range allowedOrigins {
25-
replacedString := origin
26-
// if has regex whitelisted domains
42+
// Normalize the allowed origin the same way as the input URL
43+
pattern := normalizeOrigin(origin)
44+
45+
// if has wildcard domains, convert to regex
2746
if strings.Contains(origin, "*") {
28-
replacedString = strings.ReplaceAll(origin, ".", "\\.")
29-
replacedString = strings.ReplaceAll(replacedString, "*", ".*")
47+
pattern = strings.ReplaceAll(pattern, ".", "\\.")
48+
pattern = strings.ReplaceAll(pattern, "*", ".*")
3049

31-
if strings.HasPrefix(replacedString, ".*") {
32-
replacedString += "\\b"
50+
if strings.HasPrefix(pattern, ".*") {
51+
pattern += "\\b"
3352
}
3453

35-
if strings.HasSuffix(replacedString, ".*") {
36-
replacedString = "\\b" + replacedString
54+
if strings.HasSuffix(pattern, ".*") {
55+
pattern = "\\b" + pattern
3756
}
3857
}
3958

40-
if matched, _ := regexp.MatchString(replacedString, currentOrigin); matched {
41-
hasValidURL = true
42-
break
59+
if matched, _ := regexp.MatchString("^"+pattern+"$", currentOrigin); matched {
60+
return true
4361
}
4462
}
4563

46-
return hasValidURL
64+
return false
4765
}

0 commit comments

Comments
 (0)