diff --git a/example.env b/example.env index 39c207ea34..0ece529070 100644 --- a/example.env +++ b/example.env @@ -77,6 +77,9 @@ GOTRUE_EXTERNAL_IOS_BUNDLE_ID="com.supabase.auth" # Whitelist redirect to URLs here, a comma separated list of URIs (e.g. "https://foo.example.com,https://*.foo.example.com,https://bar.example.com") GOTRUE_URI_ALLOW_LIST="http://localhost:3000" +# Enable distinct redirect URIs per OAuth provider for RFC 9700 Mix-Up attack defense (e.g. /callback/google) +GOTRUE_EXTERNAL_USE_DISTINCT_REDIRECT_URIS="false" + # Apple OAuth config GOTRUE_EXTERNAL_APPLE_ENABLED="false" GOTRUE_EXTERNAL_APPLE_CLIENT_ID="" diff --git a/internal/api/api.go b/internal/api/api.go index e90dc55da5..d0b6b60d7c 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -213,6 +213,9 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne r.Get("/", api.ExternalProviderCallback) r.Post("/", api.ExternalProviderCallback) + + r.Get("/{provider}", api.ExternalProviderCallback) + r.Post("/{provider}", api.ExternalProviderCallback) }) r.Route("/", func(r *router) { diff --git a/internal/api/external.go b/internal/api/external.go index 41195d1dbc..874bfbfeb4 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -10,6 +10,7 @@ import ( "time" "github.com/fatih/structs" + "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" "github.com/sirupsen/logrus" "github.com/supabase/auth/internal/api/apierrors" @@ -168,12 +169,24 @@ func (a *API) handleOAuthCallback(r *http.Request) (*OAuthProviderData, error) { func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() + + providerType, emailOptional := getExternalProviderType(ctx) + if pathProvider := chi.URLParam(r, "provider"); pathProvider != "" { + if !strings.EqualFold(pathProvider, providerType) { + return apierrors.NewBadRequestError( + apierrors.ErrorCodeBadOAuthCallback, + "OAuth mix-up detected: callback provider '%s' does not match flow state provider '%s'", + pathProvider, + providerType, + ) + } + } + db := a.db.WithContext(ctx) var grantParams models.GrantParams grantParams.FillGrantParams(r) - providerType, emailOptional := getExternalProviderType(ctx) data, err := a.handleOAuthCallback(r) if err != nil { return err @@ -618,81 +631,58 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide return a.loadCustomProvider(ctx, db, name, scopes) } + pConfig = a.getProviderConfig(name) + switch name { case AppleProvider: - pConfig = config.External.Apple p, err = provider.NewAppleProvider(ctx, pConfig, a.oidcCache) case AzureProvider: - pConfig = config.External.Azure p, err = provider.NewAzureProvider(pConfig, scopes, a.oidcCache) case BitbucketProvider: - pConfig = config.External.Bitbucket p, err = provider.NewBitbucketProvider(pConfig) case DiscordProvider: - pConfig = config.External.Discord p, err = provider.NewDiscordProvider(pConfig, scopes) case FacebookProvider: - pConfig = config.External.Facebook p, err = provider.NewFacebookProvider(pConfig, scopes) case FigmaProvider: - pConfig = config.External.Figma p, err = provider.NewFigmaProvider(pConfig, scopes) case FlyProvider: - pConfig = config.External.Fly p, err = provider.NewFlyProvider(pConfig, scopes) case GitHubProvider: - pConfig = config.External.Github p, err = provider.NewGithubProvider(pConfig, scopes) case GitLabProvider: - pConfig = config.External.Gitlab p, err = provider.NewGitlabProvider(pConfig, scopes) case GoogleProvider: - pConfig = config.External.Google p, err = provider.NewGoogleProvider(ctx, pConfig, scopes, a.oidcCache) case KakaoProvider: - pConfig = config.External.Kakao p, err = provider.NewKakaoProvider(pConfig, scopes) case KeycloakProvider: - pConfig = config.External.Keycloak p, err = provider.NewKeycloakProvider(pConfig, scopes) case LinkedInProvider: - pConfig = config.External.Linkedin p, err = provider.NewLinkedinProvider(pConfig, scopes) case LinkedInOIDCProvider: - pConfig = config.External.LinkedinOIDC p, err = provider.NewLinkedinOIDCProvider(ctx, pConfig, scopes, a.oidcCache) case NotionProvider: - pConfig = config.External.Notion p, err = provider.NewNotionProvider(pConfig) case SnapchatProvider: - pConfig = config.External.Snapchat p, err = provider.NewSnapchatProvider(pConfig, scopes) case SpotifyProvider: - pConfig = config.External.Spotify p, err = provider.NewSpotifyProvider(pConfig, scopes) case SlackProvider: - pConfig = config.External.Slack p, err = provider.NewSlackProvider(pConfig, scopes) case SlackOIDCProvider: - pConfig = config.External.SlackOIDC p, err = provider.NewSlackOIDCProvider(pConfig, scopes) case TwitchProvider: - pConfig = config.External.Twitch p, err = provider.NewTwitchProvider(pConfig, scopes) case TwitterProvider: - pConfig = config.External.Twitter p, err = provider.NewTwitterProvider(pConfig, scopes) case XProvider: - pConfig = config.External.X p, err = provider.NewXProvider(pConfig, scopes) case VercelMarketplaceProvider: - pConfig = config.External.VercelMarketplace p, err = provider.NewVercelMarketplaceProvider(ctx, pConfig, scopes, a.oidcCache) case WorkOSProvider: - pConfig = config.External.WorkOS p, err = provider.NewWorkOSProvider(pConfig) case ZoomProvider: - pConfig = config.External.Zoom p, err = provider.NewZoomProvider(pConfig) default: return nil, pConfig, fmt.Errorf("Provider %s could not be found", name) @@ -701,6 +691,71 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide return p, pConfig, err } +func (a *API) getProviderConfig(name string) conf.OAuthProviderConfiguration { + config := a.config + var pConfig conf.OAuthProviderConfiguration + switch name { + case AppleProvider: + pConfig = config.External.Apple + case AzureProvider: + pConfig = config.External.Azure + case BitbucketProvider: + pConfig = config.External.Bitbucket + case DiscordProvider: + pConfig = config.External.Discord + case FacebookProvider: + pConfig = config.External.Facebook + case FigmaProvider: + pConfig = config.External.Figma + case FlyProvider: + pConfig = config.External.Fly + case GitHubProvider: + pConfig = config.External.Github + case GitLabProvider: + pConfig = config.External.Gitlab + case GoogleProvider: + pConfig = config.External.Google + case KakaoProvider: + pConfig = config.External.Kakao + case NotionProvider: + pConfig = config.External.Notion + case KeycloakProvider: + pConfig = config.External.Keycloak + case LinkedInProvider: + pConfig = config.External.Linkedin + case LinkedInOIDCProvider: + pConfig = config.External.LinkedinOIDC + case SnapchatProvider: + pConfig = config.External.Snapchat + case SpotifyProvider: + pConfig = config.External.Spotify + case SlackProvider: + pConfig = config.External.Slack + case SlackOIDCProvider: + pConfig = config.External.SlackOIDC + case TwitchProvider: + pConfig = config.External.Twitch + case TwitterProvider: + pConfig = config.External.Twitter + case XProvider: + pConfig = config.External.X + case VercelMarketplaceProvider: + pConfig = config.External.VercelMarketplace + case WorkOSProvider: + pConfig = config.External.WorkOS + case ZoomProvider: + pConfig = config.External.Zoom + } + + if config.External.UseDistinctRedirectURIs && pConfig.RedirectURI != "" { + if !strings.HasSuffix(strings.ToLower(pConfig.RedirectURI), "/"+strings.ToLower(name)) { + pConfig.RedirectURI = strings.TrimRight(pConfig.RedirectURI, "/") + "/" + strings.ToLower(name) + } + } + + return pConfig +} + // loadCustomProvider loads a custom OAuth or OIDC provider from the database // identifier should be the full provider name with 'custom:' prefix (e.g., 'custom:github-enterprise') func (a *API) loadCustomProvider(ctx context.Context, db *storage.Connection, identifier string, scopes string) (provider.Provider, conf.OAuthProviderConfiguration, error) { @@ -712,6 +767,9 @@ func (a *API) loadCustomProvider(ctx context.Context, db *storage.Connection, id externalURL = config.CustomOAuth.ExternalURL } redirectURL := strings.TrimRight(externalURL, "/") + "/callback" + if config.External.UseDistinctRedirectURIs { + redirectURL = strings.TrimRight(externalURL, "/") + "/callback/" + identifier + } // Parse scopes (space-separated per RFC 6749) var scopeList []string diff --git a/internal/api/external_test.go b/internal/api/external_test.go index 62fdaf9d6f..6a8002dcc5 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -1,17 +1,21 @@ package api import ( + "context" "crypto/sha256" "encoding/base64" "fmt" "net/http" "net/http/httptest" "net/url" + "strings" "testing" + "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/api/provider" "github.com/supabase/auth/internal/conf" "github.com/supabase/auth/internal/models" @@ -555,3 +559,132 @@ func (ts *ExternalTestSuite) TestPKCEFlowStateReuseRejected() { ts.Contains(errorQuery.Get("error_description"), "already been used", "second callback with same state should be rejected as already used") } + +// TestOAuthMixUpDetection verifies that a callback with a mismatched provider in the path +// (e.g. /callback/github when state was for google) is rejected as a mix-up attack. +func (ts *ExternalTestSuite) TestOAuthMixUpDetection() { + codeVerifier := "testtesttesttesttesttesttesttesttesttesttesttesttesttest" + hashedCodeVerifier := sha256.Sum256([]byte(codeVerifier)) + codeChallenge := base64.RawURLEncoding.EncodeToString(hashedCodeVerifier[:]) + + // Initiate flow for Google + w := performPKCEAuthorizationRequest(ts, "google", codeChallenge, "s256") + ts.Require().Equal(http.StatusFound, w.Code) + u, err := url.Parse(w.Header().Get("Location")) + ts.Require().NoError(err) + state := u.Query().Get("state") + ts.Require().NotEmpty(state) + + // Callback sent to /callback/github instead of /callback/google + callbackURL := fmt.Sprintf("http://localhost/callback/github?code=authcode&state=%s", state) + req := httptest.NewRequest(http.MethodGet, callbackURL, nil) + w = httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + + // Should redirect to site URL with error describing mix-up + redirectURL, err := url.Parse(w.Header().Get("Location")) + ts.Require().NoError(err) + errorQuery, err := url.ParseQuery(redirectURL.RawQuery) + ts.Require().NoError(err) + ts.Contains(errorQuery.Get("error_description"), "mix-up detected", + "callback with mismatched provider path should be rejected as OAuth mix-up") +} + +// TestUseDistinctRedirectURIs verifies that enabling GOTRUE_EXTERNAL_USE_DISTINCT_REDIRECT_URIS +// appends the provider name to the redirect URI. +func (ts *ExternalTestSuite) TestUseDistinctRedirectURIs() { + ts.Config.External.UseDistinctRedirectURIs = true + defer func() { + ts.Config.External.UseDistinctRedirectURIs = false + }() + + provider.ResetGoogleProvider() + + req := httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=google", nil) + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + ts.Require().Equal(http.StatusFound, w.Code) + u, err := url.Parse(w.Header().Get("Location")) + ts.Require().NoError(err) + q := u.Query() + + expectedRedirect := ts.Config.External.Google.RedirectURI + "/google" + ts.Equal(expectedRedirect, q.Get("redirect_uri")) +} + +func TestUseDistinctRedirectURIsConfig(t *testing.T) { + api := &API{ + config: &conf.GlobalConfiguration{ + External: conf.ProviderConfiguration{ + UseDistinctRedirectURIs: true, + Google: conf.OAuthProviderConfiguration{ + RedirectURI: "http://localhost:9999/callback", + }, + Github: conf.OAuthProviderConfiguration{ + RedirectURI: "http://localhost:9999/callback", + }, + }, + }, + } + + googleConfig := api.getProviderConfig("google") + require.Equal(t, "http://localhost:9999/callback/google", googleConfig.RedirectURI) + + githubConfig := api.getProviderConfig("github") + require.Equal(t, "http://localhost:9999/callback/github", githubConfig.RedirectURI) + + // Disabled by default / when false + api.config.External.UseDistinctRedirectURIs = false + disabledConfig := api.getProviderConfig("google") + require.Equal(t, "http://localhost:9999/callback", disabledConfig.RedirectURI) +} + +func TestOAuthMixUpCallbackCheck(t *testing.T) { + api := &API{ + config: &conf.GlobalConfiguration{}, + } + + // Create request with chi route context where provider param in URL path is "github" + req := httptest.NewRequest(http.MethodGet, "/callback/github", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("provider", "github") + ctx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx) + + // Set flow state provider in context as "google" (mismatch) + ctx = withExternalProviderType(ctx, "google", false) + req = req.WithContext(ctx) + + w := httptest.NewRecorder() + err := api.internalExternalProviderCallback(w, req) + require.Error(t, err) + + httpErr, ok := err.(*apierrors.HTTPError) + require.True(t, ok) + require.Equal(t, http.StatusBadRequest, httpErr.HTTPStatus) + require.Contains(t, httpErr.Message, "OAuth mix-up detected") + + // Matching provider: URL path parameter "google" matches flow state provider "google" + rctxMatching := chi.NewRouteContext() + rctxMatching.URLParams.Add("provider", "google") + ctxMatching := context.WithValue(httptest.NewRequest(http.MethodGet, "/callback/google", nil).Context(), chi.RouteCtxKey, rctxMatching) + ctxMatching = withExternalProviderType(ctxMatching, "google", false) + reqMatching := httptest.NewRequest(http.MethodGet, "/callback/google", nil).WithContext(ctxMatching) + + // Verify that matching provider path passes the mix-up check (so it reaches DB/callback logic instead of returning mix-up error) + pathProvider := chi.URLParam(reqMatching, "provider") + providerType, _ := getExternalProviderType(reqMatching.Context()) + require.True(t, strings.EqualFold(pathProvider, providerType)) +} + +func TestLegacyCallbackBackwardCompatibility(t *testing.T) { + // Request to generic /callback without provider in URL path + req := httptest.NewRequest(http.MethodGet, "/callback", nil) + rctx := chi.NewRouteContext() + ctx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx) + ctx = withExternalProviderType(ctx, "google", false) + req = req.WithContext(ctx) + + // Ensure pathProvider is empty and does not trigger mix-up error + pathProvider := chi.URLParam(req, "provider") + require.Empty(t, pathProvider) +} diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 6772bf1963..68cd93ebe8 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -583,6 +583,7 @@ type ProviderConfiguration struct { RedirectURL string `json:"redirect_url"` AllowedIdTokenIssuers []string `json:"allowed_id_token_issuers" split_words:"true"` FlowStateExpiryDuration time.Duration `json:"flow_state_expiry_duration" split_words:"true"` + UseDistinctRedirectURIs bool `json:"use_distinct_redirect_uris" split_words:"true" default:"false"` // OIDCProviderCacheTTL controls how long OIDC discovery documents are cached. OIDCProviderCacheTTL time.Duration `json:"oidc_provider_cache_ttl" split_words:"true" default:"1h"`