fix: validate OAuth redirect URI against allowlist when strict

api/external.go

@@ -363,11 +363,66 @@ func getErrorQueryString(err error, errorID string, log logrus.FieldLogger) *url
 func (a *API) getExternalRedirectURL(r *http.Request) string {
 	ctx := r.Context()
 	config := a.getConfig(ctx)
+
+	candidates := []string{}
 	if config.External.RedirectURL != "" {
-		return config.External.RedirectURL
+		candidates = append(candidates, config.External.RedirectURL)
 	}
 	if er := getExternalReferrer(ctx); er != "" {
-		return er
+		candidates = append(candidates, er)
+	}
+	candidates = append(candidates, config.SiteURL)
+
+	if !config.Security.Strict {
+		return candidates[0]
+	}
+
+	allowed := config.Security.AllowedRedirectURIs
+	if len(allowed) == 0 {
+		allowed = []string{config.SiteURL}
+	}
+
+	for _, candidate := range candidates {
+		if isAllowedRedirectURI(candidate, allowed) {
+			return candidate
+		}
+	}
+	// Nothing matched the allowlist. Returning config.SiteURL here would
+	// leak OAuth tokens to a non-allowlisted destination when the operator
+	// configured AllowedRedirectURIs without including SiteURL. Prefer the
+	// operator's primary allowlisted entry instead.
+	return allowed[0]
+}
+
+// isAllowedRedirectURI matches candidate against allowed by exact scheme,
+// case-insensitive host, and path-segment prefix. An allowlist entry with an
+// empty path or path "/" matches any path on that host. A path-scoped entry
+// matches the exact path or a sub-path, but not a path that merely shares a
+// string prefix (entry "/auth" matches "/auth" and "/auth/cb" but not
+// "/authorize"). Subdomains do NOT match.
+func isAllowedRedirectURI(candidate string, allowed []string) bool {
+	cu, err := url.Parse(candidate)
+	if err != nil || cu.Scheme == "" || cu.Host == "" {
+		return false
+	}
+	for _, entry := range allowed {
+		au, err := url.Parse(entry)
+		if err != nil || au.Scheme == "" || au.Host == "" {
+			continue
+		}
+		if cu.Scheme != au.Scheme {
+			continue
+		}
+		if !strings.EqualFold(cu.Host, au.Host) {
+			continue
+		}
+		allowedPath := strings.TrimSuffix(au.Path, "/")
+		if allowedPath == "" {
+			return true
+		}
+		if cu.Path == allowedPath || strings.HasPrefix(cu.Path, allowedPath+"/") {
+			return true
+		}
 	}
-	return config.SiteURL
+	return false
 }

api/external_test.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -9,10 +10,63 @@ import (
 	"github.com/gofrs/uuid"
 	"github.com/netlify/gotrue/conf"
 	"github.com/netlify/gotrue/models"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
+func TestIsAllowedRedirectURI(t *testing.T) {
+	cases := []struct {
+		name      string
+		candidate string
+		allowed   []string
+		want      bool
+	}{
+		{"exact host", "https://app.example.com/cb", []string{"https://app.example.com"}, true},
+		{"empty path on entry matches any candidate path", "https://app.example.com/auth/cb?x=1", []string{"https://app.example.com"}, true},
+		{"path prefix", "https://app.example.com/auth/cb", []string{"https://app.example.com/auth"}, true},
+		{"path exact", "https://app.example.com/auth", []string{"https://app.example.com/auth"}, true},
+		{"path prefix mismatch", "https://app.example.com/other/cb", []string{"https://app.example.com/auth"}, false},
+		{"adjacent path prefix not allowed", "https://app.example.com/authorize", []string{"https://app.example.com/auth"}, false},
+		{"trailing slash entry", "https://app.example.com/auth/cb", []string{"https://app.example.com/auth/"}, true},
+		{"scheme mismatch", "http://app.example.com/cb", []string{"https://app.example.com"}, false},
+		{"host mismatch", "https://evil.com/cb", []string{"https://app.example.com"}, false},
+		{"subdomain not allowed", "https://evil.app.example.com/cb", []string{"https://app.example.com"}, false},
+		{"empty candidate", "", []string{"https://app.example.com"}, false},
+		{"candidate without scheme", "/relative/path", []string{"https://app.example.com"}, false},
+		{"case-insensitive host", "https://APP.EXAMPLE.COM/cb", []string{"https://app.example.com"}, true},
+		{"matches second entry", "https://preview.example.com/cb", []string{"https://app.example.com", "https://preview.example.com"}, true},
+		{"empty allowlist", "https://app.example.com/cb", nil, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, isAllowedRedirectURI(tc.candidate, tc.allowed))
+		})
+	}
+}
+
+// TestGetExternalRedirectURL_StrictFallbackUsesAllowlist guards against the
+// subtle hole where strict mode would still leak a redirect to SiteURL if the
+// operator configured AllowedRedirectURIs without including it. With nothing
+// matching the allowlist, the result must be an allowlisted entry, not the
+// raw SiteURL.
+func TestGetExternalRedirectURL_StrictFallbackUsesAllowlist(t *testing.T) {
+	api := &API{config: &conf.GlobalConfiguration{}}
+	config := &conf.Configuration{
+		SiteURL: "https://app.example.com",
+		Security: conf.SecurityConfiguration{
+			Strict:              true,
+			AllowedRedirectURIs: []string{"https://allowed.example.com"},
+		},
+	}
+	config.ApplyDefaults()
+	ctx, err := WithInstanceConfig(context.Background(), config, uuid.Nil)
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodGet, "/callback", nil).WithContext(ctx)
+	assert.Equal(t, "https://allowed.example.com", api.getExternalRedirectURL(req))
+}
+
 type ExternalTestSuite struct {
 	suite.Suite
 	API        *API

conf/configuration.go

@@ -125,6 +125,13 @@ type MailerConfiguration struct {
 // follow-up changes.
 type SecurityConfiguration struct {
 	Strict bool `json:"strict"`
+
+	// AllowedRedirectURIs is the allowlist for OAuth redirect URIs when Strict.
+	// Entries match by exact scheme and case-insensitive host; the path is
+	// matched as a segment prefix, so "https://app.example.com/auth" allows
+	// "/auth" and "/auth/cb" but not "/authorize". Subdomains are not implied.
+	// Empty list means only the SiteURL host is accepted.
+	AllowedRedirectURIs []string `json:"allowed_redirect_uris" envconfig:"ALLOWED_REDIRECT_URIS"`
 }
 
 // Configuration holds all the per-instance configuration.

conf/configuration_test.go

@@ -79,6 +79,15 @@ func TestSecurityConfigurationDefaults(t *testing.T) {
 		require.NoError(t, err)
 		assert.True(t, c.Security.Strict)
 	})
+
+	t.Run("loads allowed redirect URIs from env", func(t *testing.T) {
+		baseEnv()
+		os.Setenv("GOTRUE_SECURITY_STRICT", "true")
+		os.Setenv("GOTRUE_SECURITY_ALLOWED_REDIRECT_URIS", "https://app.example.com,https://app.example.com/cb")
+		c, err := LoadConfig("")
+		require.NoError(t, err)
+		assert.Equal(t, []string{"https://app.example.com", "https://app.example.com/cb"}, c.Security.AllowedRedirectURIs)
+	})
 }
 
 func TestNewInstancesSecureByDefault(t *testing.T) {