fix: allow custom URI schemes in OAuth Dynamic Client Registration

internal/api/oauthserver/service.go

@@ -9,17 +9,19 @@ import (
 	"fmt"
 	"net/url"
 	"slices"
+	"strings"
 	"time"
 
 	"github.com/gofrs/uuid"
 	"github.com/pkg/errors"
 	"github.com/supabase/auth/internal/api/apierrors"
+	"github.com/supabase/auth/internal/conf"
 	"github.com/supabase/auth/internal/models"
 	"github.com/supabase/auth/internal/utilities"
 )
 
 // validateRedirectURIList validates a list of redirect URIs
-func validateRedirectURIList(redirectURIs []string, required bool) error {
+func (s *Server) validateRedirectURIList(redirectURIs []string, required bool) error {
 	if required && len(redirectURIs) == 0 {
 		return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "redirect_uris is required")
 	}
@@ -33,7 +35,7 @@ func validateRedirectURIList(redirectURIs []string, required bool) error {
 	}
 
 	for _, uri := range redirectURIs {
-		if err := validateRedirectURI(uri); err != nil {
+		if err := s.validateRedirectURI(uri); err != nil {
 			return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "invalid redirect_uri '%s': %v", uri, err)
 		}
 	}
@@ -117,9 +119,9 @@ type OAuthServerClientRegisterParams struct {
 }
 
 // validate validates the OAuth client registration parameters
-func (p *OAuthServerClientRegisterParams) validate() error {
+func (p *OAuthServerClientRegisterParams) validate(s *Server) error {
 	// Validate redirect URIs (required for registration)
-	if err := validateRedirectURIList(p.RedirectURIs, true); err != nil {
+	if err := s.validateRedirectURIList(p.RedirectURIs, true); err != nil {
 		return err
 	}
 
@@ -170,8 +172,8 @@ func (p *OAuthServerClientRegisterParams) validate() error {
 	return nil
 }
 
-// validateRedirectURI validates OAuth 2.1 redirect URIs
-func validateRedirectURI(uri string) error {
+// validateRedirectURI validates OAuth 2.1 redirect URIs with enhanced security checks
+func (s *Server) validateRedirectURI(uri string) error {
 	if uri == "" {
 		return fmt.Errorf("redirect URI cannot be empty")
 	}
@@ -181,30 +183,89 @@ func validateRedirectURI(uri string) error {
 		return fmt.Errorf("invalid URL format")
 	}
 
-	// Must have scheme and host
-	if parsedURL.Scheme == "" || parsedURL.Host == "" {
-		return fmt.Errorf("must have scheme and host")
-	}
-
-	// Check scheme requirements
-	if parsedURL.Scheme == "http" {
-		// HTTP only allowed for localhost
-		host := parsedURL.Hostname()
-		if host != "localhost" && host != "127.0.0.1" {
-			return fmt.Errorf("HTTP scheme only allowed for localhost")
-		}
-	} else if parsedURL.Scheme != "https" {
-		return fmt.Errorf("scheme must be HTTPS or HTTP (localhost only)")
+	// Must have scheme
+	if parsedURL.Scheme == "" {
+		return fmt.Errorf("must have scheme")
 	}
 
 	// Must not have fragment
 	if parsedURL.Fragment != "" {
 		return fmt.Errorf("fragment not allowed in redirect URI")
 	}
 
+	// Block dangerous schemes that could be used for attacks
+	scheme := strings.ToLower(parsedURL.Scheme)
+	dangerousSchemes := []string{"javascript", "data", "file", "vbscript", "about"}
+	for _, dangerous := range dangerousSchemes {
+		if scheme == dangerous {
+			return fmt.Errorf("scheme '%s' is not allowed for security reasons", scheme)
+		}
+	}
+
+	// For OAuth client registration, we need stricter validation than general redirects
+	// Check if it's a standard web scheme (http/https)
+	isStandardScheme := scheme == "http" || scheme == "https"
+
+	if isStandardScheme {
+		// For HTTP/HTTPS, use standard validation
+		if scheme == "http" {
+			// HTTP only allowed for localhost
+			host := parsedURL.Hostname()
+			if host != "localhost" && host != "127.0.0.1" && !strings.HasPrefix(host, "127.") {
+				return fmt.Errorf("HTTP scheme only allowed for localhost")
+			}
+		}
+		// HTTPS is always allowed if in the allow list
+		if !utilities.IsRedirectURLValid(s.config, uri) {
+			return fmt.Errorf("redirect URI not allowed by configuration")
+		}
+	} else {
+		// For custom schemes (cursor://, exp://, etc.), require explicit allow list match
+		// This prevents the hostname-matching bypass in IsRedirectURLValid
+		if !isCustomSchemeAllowed(s.config, uri) {
+			return fmt.Errorf("custom scheme '%s' not allowed by configuration", scheme)
+		}
+	}
+
 	return nil
 }
 
+// isCustomSchemeAllowed checks if a custom URI scheme is explicitly allowed in the configuration
+// This provides stricter validation for non-HTTP(S) schemes used in OAuth client registration
+func isCustomSchemeAllowed(config *conf.GlobalConfiguration, uri string) bool {
+	if uri == "" {
+		return false
+	}
+
+	parsedURL, err := url.Parse(uri)
+	if err != nil {
+		return false
+	}
+
+	scheme := strings.ToLower(parsedURL.Scheme)
+
+	// Only check against allow list for custom schemes
+	// This bypasses the hostname-matching shortcut in IsRedirectURLValid
+	for pattern, glob := range config.URIAllowListMap {
+		// Only match patterns that start with the same custom scheme
+		patternURL, err := url.Parse(pattern)
+		if err != nil {
+			continue
+		}
+
+		patternScheme := strings.ToLower(patternURL.Scheme)
+		if patternScheme == scheme {
+			// Match without fragment
+			matchAgainst, _, _ := strings.Cut(uri, "#")
+			if glob.Match(matchAgainst) {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // generateClientSecret generates a secure random client secret
 func generateClientSecret() string {
 	b := make([]byte, 32)
@@ -235,7 +296,7 @@ func ValidateClientSecret(providedSecret, storedHash string) bool {
 // registerOAuthServerClient creates a new OAuth server client with generated credentials
 func (s *Server) registerOAuthServerClient(ctx context.Context, params *OAuthServerClientRegisterParams) (*models.OAuthServerClient, string, error) {
 	// Validate all parameters
-	if err := params.validate(); err != nil {
+	if err := params.validate(s); err != nil {
 		return nil, "", err
 	}
 
@@ -362,10 +423,10 @@ func (p *OAuthServerClientUpdateParams) isEmpty() bool {
 }
 
 // validate validates the OAuth client update parameters
-func (p *OAuthServerClientUpdateParams) validate() error {
+func (p *OAuthServerClientUpdateParams) validate(s *Server) error {
 	// Validate redirect URIs if provided
 	if p.RedirectURIs != nil {
-		if err := validateRedirectURIList(*p.RedirectURIs, false); err != nil {
+		if err := s.validateRedirectURIList(*p.RedirectURIs, false); err != nil {
 			return err
 		}
 	}
@@ -404,7 +465,7 @@ func (p *OAuthServerClientUpdateParams) validate() error {
 // updateOAuthServerClient updates an existing OAuth client
 func (s *Server) updateOAuthServerClient(ctx context.Context, clientID uuid.UUID, params *OAuthServerClientUpdateParams) (*models.OAuthServerClient, error) {
 	// Validate all parameters
-	if err := params.validate(); err != nil {
+	if err := params.validate(s); err != nil {
 		return nil, err
 	}
 

internal/api/oauthserver/service_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/gobwas/glob"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
@@ -59,6 +60,15 @@ func (ts *OAuthServiceTestSuite) SetupTest() {
 	// Enable OAuth server and dynamic client registration for tests
 	ts.Config.OAuthServer.Enabled = true
 	ts.Config.OAuthServer.AllowDynamicRegistration = true
+
+	// Add test URIs to allow list for testing
+	ts.Config.URIAllowList = append(ts.Config.URIAllowList, "https://example.com/**", "https://app.example.com/**", "http://localhost:3000/**")
+	// Rebuild the allow list map
+	ts.Config.URIAllowListMap = make(map[string]glob.Glob)
+	for _, uri := range ts.Config.URIAllowList {
+		g := glob.MustCompile(uri, '.', '/')
+		ts.Config.URIAllowListMap[uri] = g
+	}
 }
 
 // Helper function to create test OAuth client
@@ -276,34 +286,46 @@ func (ts *OAuthServiceTestSuite) TestRedirectURIValidation() {
 			errorMsg:    "redirect URI cannot be empty",
 		},
 		{
-			name:        "Invalid scheme",
+			name:        "Invalid URI with fragment",
+			uri:         "https://example.com/callback#fragment",
+			shouldError: true,
+			errorMsg:    "fragment not allowed in redirect URI",
+		},
+		{
+			name:        "Invalid URI format",
+			uri:         "not-a-uri",
+			shouldError: true,
+			errorMsg:    "must have scheme",
+		},
+		{
+			name:        "URI not in allow list",
 			uri:         "ftp://example.com/callback",
 			shouldError: true,
-			errorMsg:    "scheme must be HTTPS or HTTP (localhost only)",
+			errorMsg:    "not allowed by configuration",
 		},
 		{
-			name:        "Invalid HTTP non-localhost",
-			uri:         "http://example.com/callback",
+			name:        "Dangerous javascript scheme blocked",
+			uri:         "javascript:alert('xss')",
 			shouldError: true,
-			errorMsg:    "HTTP scheme only allowed for localhost",
+			errorMsg:    "not allowed for security reasons",
 		},
 		{
-			name:        "Invalid URI with fragment",
-			uri:         "https://example.com/callback#fragment",
+			name:        "Dangerous data scheme blocked",
+			uri:         "data:text/html,<script>alert('xss')</script>",
 			shouldError: true,
-			errorMsg:    "fragment not allowed in redirect URI",
+			errorMsg:    "not allowed for security reasons",
 		},
 		{
-			name:        "Invalid URI format",
-			uri:         "not-a-uri",
+			name:        "Dangerous file scheme blocked",
+			uri:         "file:///etc/passwd",
 			shouldError: true,
-			errorMsg:    "must have scheme and host",
+			errorMsg:    "not allowed for security reasons",
 		},
 	}
 
 	for _, tc := range testCases {
 		ts.T().Run(tc.name, func(t *testing.T) {
-			err := validateRedirectURI(tc.uri)
+			err := ts.Server.validateRedirectURI(tc.uri)
 			if tc.shouldError {
 				assert.Error(t, err)
 				if tc.errorMsg != "" {
@@ -336,3 +358,91 @@ func (ts *OAuthServiceTestSuite) TestGrantTypeDefaults() {
 	assert.Contains(ts.T(), grantTypes, "refresh_token")
 	assert.Len(ts.T(), grantTypes, 2)
 }
+
+func (ts *OAuthServiceTestSuite) TestCustomURISchemes() {
+	// Test custom URI schemes when they're in the allow list
+	// This tests the fix for issue #2285
+
+	// Save original allow list
+	originalAllowList := ts.Config.URIAllowList
+	originalAllowListMap := ts.Config.URIAllowListMap
+	defer func() {
+		ts.Config.URIAllowList = originalAllowList
+		ts.Config.URIAllowListMap = originalAllowListMap
+	}()
+
+	// Configure allow list with custom schemes (keep existing + add custom)
+	ts.Config.URIAllowList = append([]string{}, originalAllowList...)
+	ts.Config.URIAllowList = append(ts.Config.URIAllowList, "cursor://**", "com.example.app://**", "exp://**")
+	// Rebuild the allow list map
+	ts.Config.URIAllowListMap = make(map[string]glob.Glob)
+	for _, uri := range ts.Config.URIAllowList {
+		g := glob.MustCompile(uri, '.', '/')
+		ts.Config.URIAllowListMap[uri] = g
+	}
+
+	ctx := context.Background()
+
+	// Test 1: cursor:// scheme (for Cursor IDE)
+	params := &OAuthServerClientRegisterParams{
+		ClientName:       "Cursor IDE",
+		RedirectURIs:     []string{"cursor://anysphere.cursor-mcp/callback"},
+		RegistrationType: "dynamic",
+	}
+
+	client, secret, err := ts.Server.registerOAuthServerClient(ctx, params)
+	require.NoError(ts.T(), err, "Should allow cursor:// scheme when in allow list")
+	require.NotNil(ts.T(), client)
+	require.NotEmpty(ts.T(), secret)
+	assert.Equal(ts.T(), "Cursor IDE", *client.ClientName)
+	assert.Equal(ts.T(), []string{"cursor://anysphere.cursor-mcp/callback"}, client.GetRedirectURIs())
+
+	// Test 2: Mobile app scheme (com.example.app://)
+	params = &OAuthServerClientRegisterParams{
+		ClientName:       "Mobile App",
+		RedirectURIs:     []string{"com.example.app://sign-in/v2"},
+		RegistrationType: "dynamic",
+	}
+
+	client, secret, err = ts.Server.registerOAuthServerClient(ctx, params)
+	require.NoError(ts.T(), err, "Should allow com.example.app:// scheme when in allow list")
+	require.NotNil(ts.T(), client)
+	require.NotEmpty(ts.T(), secret)
+	assert.Equal(ts.T(), "Mobile App", *client.ClientName)
+
+	// Test 3: Expo scheme (exp://)
+	params = &OAuthServerClientRegisterParams{
+		ClientName:       "Expo App",
+		RedirectURIs:     []string{"exp://192.168.1.1:19000/--/auth/callback"},
+		RegistrationType: "dynamic",
+	}
+
+	client, secret, err = ts.Server.registerOAuthServerClient(ctx, params)
+	require.NoError(ts.T(), err, "Should allow exp:// scheme when in allow list")
+	require.NotNil(ts.T(), client)
+	require.NotEmpty(ts.T(), secret)
+
+	// Test 4: Unauthorized custom scheme should fail
+	params = &OAuthServerClientRegisterParams{
+		ClientName:       "Malicious App",
+		RedirectURIs:     []string{"malicious://attack"},
+		RegistrationType: "dynamic",
+	}
+
+	_, _, err = ts.Server.registerOAuthServerClient(ctx, params)
+	assert.Error(ts.T(), err, "Should reject custom scheme not in allow list")
+	assert.Contains(ts.T(), err.Error(), "redirect URI not allowed by configuration")
+
+	// Test 5: Mix of custom and standard schemes
+	params = &OAuthServerClientRegisterParams{
+		ClientName:       "Multi-Platform App",
+		RedirectURIs:     []string{"https://example.com/callback", "cursor://app/callback"},
+		RegistrationType: "dynamic",
+	}
+
+	client, secret, err = ts.Server.registerOAuthServerClient(ctx, params)
+	require.NoError(ts.T(), err, "Should allow mix of standard and custom schemes")
+	require.NotNil(ts.T(), client)
+	require.NotEmpty(ts.T(), secret)
+	assert.Len(ts.T(), client.GetRedirectURIs(), 2)
+}