From 89745435a15b53153a15bcbd7b05c6b88980641d Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Mon, 13 Apr 2026 13:10:51 +0200 Subject: [PATCH 1/3] chore: rfc9700 strict apply RFC9700 calls for strict URL validation, not just hostname: This means the authorization server MUST ensure that the two URIs are equal; see Section 6.2.1 of [RFC3986], Simple String Comparison, for details. The only exception is native apps using a localhost URI: In this case, the authorization server MUST allow variable port numbers as described in Section 7.3 of [RFC8252]. --- internal/utilities/request.go | 6 ++- internal/utilities/request_test.go | 78 ++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/internal/utilities/request.go b/internal/utilities/request.go index bd38c73819..ea84f4c656 100644 --- a/internal/utilities/request.go +++ b/internal/utilities/request.go @@ -101,7 +101,11 @@ func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bo // As long as the referrer came from the site, we will redirect back there if berr == nil && rerr == nil && base.Hostname() == refurl.Hostname() { - return true + // ensure schema and port haven't changed + // most browsers should be checking insecure protocol switching but be double check + if base.Scheme == refurl.Scheme && base.Port() == refurl.Port() { + return true + } } if rerr != nil { diff --git a/internal/utilities/request_test.go b/internal/utilities/request_test.go index 91ae97fac6..20497a01b8 100644 --- a/internal/utilities/request_test.go +++ b/internal/utilities/request_test.go @@ -10,6 +10,69 @@ import ( "github.com/supabase/auth/internal/sbff" ) +func TestIsRedirectURLValidSameOrigin(t *tst.T) { + cases := []struct { + desc string + siteURL string + redirectURL string + want bool + }{ + { + desc: "exact match", + siteURL: "https://example.com", + redirectURL: "https://example.com/path", + want: true, + }, + { + desc: "scheme downgrade https→http rejected", + siteURL: "https://example.com", + redirectURL: "http://example.com/path", + want: false, + }, + { + desc: "scheme upgrade http→https rejected", + siteURL: "http://example.com", + redirectURL: "https://example.com/path", + want: false, + }, + { + desc: "different port rejected", + siteURL: "https://example.com", + redirectURL: "https://example.com:8443/path", + want: false, + }, + { + desc: "explicit port matches SiteURL explicit port", + siteURL: "https://example.com:9000", + redirectURL: "https://example.com:9000/path", + want: true, + }, + { + desc: "no port vs explicit port rejected", + siteURL: "https://example.com:9000", + redirectURL: "https://example.com/path", + want: false, + }, + { + desc: "different explicit ports rejected", + siteURL: "https://example.com:9000", + redirectURL: "https://example.com:9001/path", + want: false, + }, + } + + for _, c := range cases { + t.Run(c.desc, func(t *tst.T) { + config := conf.GlobalConfiguration{ + SiteURL: c.siteURL, + JWT: conf.JWTConfiguration{Secret: "testsecret"}, + } + require.NoError(t, config.ApplyDefaults()) + require.Equal(t, c.want, IsRedirectURLValid(&config, c.redirectURL)) + }) + } +} + func TestGetIPAddressWithSBFF(t *tst.T) { testCases := []struct { name string @@ -217,6 +280,21 @@ func TestGetReferrer(t *tst.T) { redirectURL: "http://[0:0:0:0:0:0:0:1]:12345/path", expected: "http://[0:0:0:0:0:0:0:1]:12345/path", }, + { + desc: "same origin allowed", + redirectURL: "https://example.com/dashboard", + expected: "https://example.com/dashboard", + }, + { + desc: "same hostname but http scheme rejected (scheme downgrade)", + redirectURL: "http://example.com/dashboard", + expected: config.SiteURL, + }, + { + desc: "same hostname and scheme but explicit non-default port rejected", + redirectURL: "https://example.com:8443/dashboard", + expected: config.SiteURL, + }, } for _, c := range cases { From 396f007b9a44912c202f7b0d4306ffb51debef2b Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Mon, 13 Apr 2026 13:16:51 +0200 Subject: [PATCH 2/3] chore: add localhost exception --- internal/utilities/request.go | 12 ++++++++---- internal/utilities/request_test.go | 31 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/internal/utilities/request.go b/internal/utilities/request.go index ea84f4c656..cd5e7d8b9a 100644 --- a/internal/utilities/request.go +++ b/internal/utilities/request.go @@ -101,10 +101,14 @@ func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bo // As long as the referrer came from the site, we will redirect back there if berr == nil && rerr == nil && base.Hostname() == refurl.Hostname() { - // ensure schema and port haven't changed - // most browsers should be checking insecure protocol switching but be double check - if base.Scheme == refurl.Scheme && base.Port() == refurl.Port() { - return true + // ensure scheme hasn't changed; most browsers also check this but double check here + if base.Scheme == refurl.Scheme { + // Per RFC 8252 Section 7.3, native apps using a localhost redirect URI + // MUST be allowed to use variable port numbers, so skip the port check + // for loopback addresses. + if base.Port() == refurl.Port() || isLocalhost(refurl.Hostname()) { + return true + } } } diff --git a/internal/utilities/request_test.go b/internal/utilities/request_test.go index 20497a01b8..b7b825fbbf 100644 --- a/internal/utilities/request_test.go +++ b/internal/utilities/request_test.go @@ -59,6 +59,37 @@ func TestIsRedirectURLValidSameOrigin(t *tst.T) { redirectURL: "https://example.com:9001/path", want: false, }, + // RFC 8252 Section 7.3: variable ports must be allowed for localhost + { + desc: "localhost with different port allowed (RFC 8252 Section 7.3)", + siteURL: "http://localhost:3000", + redirectURL: "http://localhost:8080/callback", + want: true, + }, + { + desc: "127.0.0.1 with different port allowed (RFC 8252 Section 7.3)", + siteURL: "http://127.0.0.1:3000", + redirectURL: "http://127.0.0.1:8080/callback", + want: true, + }, + { + desc: "localhost without port in redirect allowed (RFC 8252 Section 7.3)", + siteURL: "http://localhost:3000", + redirectURL: "http://localhost/callback", + want: true, + }, + { + desc: "localhost scheme downgrade still rejected despite RFC 8252", + siteURL: "https://localhost:3000", + redirectURL: "http://localhost:8080/callback", + want: false, + }, + { + desc: "non-localhost variable port still rejected", + siteURL: "https://example.com:9000", + redirectURL: "https://example.com:9001/path", + want: false, + }, } for _, c := range cases { From 66f0d51961b28e19ced85d5eb827dfe21aeb779a Mon Sep 17 00:00:00 2001 From: Etienne Stalmans Date: Mon, 13 Apr 2026 13:28:34 +0200 Subject: [PATCH 3/3] chore: update test for oauthserver --- internal/api/oauthserver/authorize_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/api/oauthserver/authorize_test.go b/internal/api/oauthserver/authorize_test.go index 3cfa2aeaca..bb2d7fb954 100644 --- a/internal/api/oauthserver/authorize_test.go +++ b/internal/api/oauthserver/authorize_test.go @@ -136,13 +136,15 @@ func TestValidateRequestOriginEdgeCases(t *testing.T) { tokenService := tokens.NewService(globalConfig, hooksMgr) server := NewServer(globalConfig, conn, tokenService) - t.Run("Origin with different port should be allowed (hostname matching)", func(t *testing.T) { + t.Run("Origin with different port on non-localhost should be rejected", func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/test", nil) req.Header.Set("Origin", "https://example.com:8080") - // Should pass because hostname matches (IsRedirectURLValid allows different ports) + // Must be rejected: port mismatch on a non-loopback host. + // RFC 8252 Section 7.3 variable-port exception only applies to localhost. err := server.validateRequestOrigin(req) - assert.NoError(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unauthorized request origin") }) t.Run("Case sensitivity in Origin header", func(t *testing.T) {