From ee8b9032981d599cc0e489875a5b0833c0e46a63 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Mon, 4 Aug 2025 14:18:10 +0200 Subject: [PATCH 01/10] feat: implements email-less accounts with oauth --- internal/api/apierrors/errorcode.go | 2 +- internal/api/context.go | 58 +++++++------ internal/api/external.go | 123 ++++++++++++++++++---------- internal/api/external_oauth.go | 15 ++-- internal/api/provider/oidc.go | 7 +- internal/api/provider/snapchat.go | 42 ++++++++-- internal/api/samlacs.go | 2 +- internal/api/token_oidc.go | 47 ++++++++--- internal/api/token_oidc_test.go | 3 +- internal/api/web3.go | 4 +- internal/conf/configuration.go | 13 +-- 11 files changed, 206 insertions(+), 110 deletions(-) diff --git a/internal/api/apierrors/errorcode.go b/internal/api/apierrors/errorcode.go index 6406028bc..00797d027 100644 --- a/internal/api/apierrors/errorcode.go +++ b/internal/api/apierrors/errorcode.go @@ -95,6 +95,6 @@ const ( ErrorCodeEmailAddressInvalid ErrorCode = "email_address_invalid" ErrorCodeWeb3ProviderDisabled ErrorCode = "web3_provider_disabled" ErrorCodeWeb3UnsupportedChain ErrorCode = "web3_unsupported_chain" - ErrorCodeOAuthDynamicClientRegistrationDisabled ErrorCode = "oauth_dynamic_client_registration_disabled" + ErrorCodeEmailAddressNotProvided ErrorCode = "email_address_not_provided" ) diff --git a/internal/api/context.go b/internal/api/context.go index 3047f3dd6..01a60149f 100644 --- a/internal/api/context.go +++ b/internal/api/context.go @@ -15,22 +15,24 @@ func (c contextKey) String() string { } const ( - tokenKey = contextKey("jwt") - inviteTokenKey = contextKey("invite_token") - signatureKey = contextKey("signature") - externalProviderTypeKey = contextKey("external_provider_type") - userKey = contextKey("user") - targetUserKey = contextKey("target_user") - factorKey = contextKey("factor") - sessionKey = contextKey("session") - externalReferrerKey = contextKey("external_referrer") - functionHooksKey = contextKey("function_hooks") - adminUserKey = contextKey("admin_user") - oauthTokenKey = contextKey("oauth_token") // for OAuth1.0, also known as request token - oauthVerifierKey = contextKey("oauth_verifier") - ssoProviderKey = contextKey("sso_provider") - externalHostKey = contextKey("external_host") - flowStateKey = contextKey("flow_state_id") + externalProviderTypeKey = contextKey("external_provider_type") + externalProviderAllowNoEmailKey = contextKey("external_provider_allow_no_email") + + tokenKey = contextKey("jwt") + inviteTokenKey = contextKey("invite_token") + signatureKey = contextKey("signature") + userKey = contextKey("user") + targetUserKey = contextKey("target_user") + factorKey = contextKey("factor") + sessionKey = contextKey("session") + externalReferrerKey = contextKey("external_referrer") + functionHooksKey = contextKey("function_hooks") + adminUserKey = contextKey("admin_user") + oauthTokenKey = contextKey("oauth_token") // for OAuth1.0, also known as request token + oauthVerifierKey = contextKey("oauth_verifier") + ssoProviderKey = contextKey("sso_provider") + externalHostKey = contextKey("external_host") + flowStateKey = contextKey("flow_state_id") ) // withToken adds the JWT token to the context. @@ -152,18 +154,26 @@ func getInviteToken(ctx context.Context) string { } // withExternalProviderType adds the provided request ID to the context. -func withExternalProviderType(ctx context.Context, id string) context.Context { - return context.WithValue(ctx, externalProviderTypeKey, id) +func withExternalProviderType(ctx context.Context, id string, allowNoEmail bool) context.Context { + return context.WithValue(context.WithValue(ctx, externalProviderTypeKey, id), externalProviderAllowNoEmailKey, allowNoEmail) } -// getExternalProviderType reads the request ID from the context. -func getExternalProviderType(ctx context.Context) string { - obj := ctx.Value(externalProviderTypeKey) - if obj == nil { - return "" +// getExternalProviderType returns the provider type and whether user data without email address should be allowed. +func getExternalProviderType(ctx context.Context) (string, bool) { + idValue := ctx.Value(externalProviderTypeKey) + allowNoEmailValue := ctx.Value(externalProviderAllowNoEmailKey) + + id, okID := idValue.(string) + if !okID { + return "", false } - return obj.(string) + allowNoEmail, okAllowNoEmail := allowNoEmailValue.(bool) + if !okAllowNoEmail { + return "", false + } + + return id, allowNoEmail } func withExternalReferrer(ctx context.Context, token string) context.Context { diff --git a/internal/api/external.go b/internal/api/external.go index a1c7b22fd..4094ecf68 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -31,6 +31,7 @@ type ExternalProviderClaims struct { Referrer string `json:"referrer,omitempty"` FlowStateID string `json:"flow_state_id"` LinkingTargetID string `json:"linking_target_id,omitempty"` + AllowNoEmail bool `json:"allow_no_email,omitempty"` } // ExternalProviderRedirect redirects the request to the oauth provider @@ -55,7 +56,7 @@ func (a *API) GetExternalProviderRedirectURL(w http.ResponseWriter, r *http.Requ codeChallenge := query.Get("code_challenge") codeChallengeMethod := query.Get("code_challenge_method") - p, err := a.Provider(ctx, providerType, scopes) + p, pConfig, err := a.Provider(ctx, providerType, scopes) if err != nil { return "", apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unsupported provider: %+v", err).WithInternalError(err) } @@ -96,10 +97,11 @@ func (a *API) GetExternalProviderRedirectURL(w http.ResponseWriter, r *http.Requ SiteURL: config.SiteURL, InstanceID: uuid.Nil.String(), }, - Provider: providerType, - InviteToken: inviteToken, - Referrer: redirectURL, - FlowStateID: flowStateID, + Provider: providerType, + InviteToken: inviteToken, + Referrer: redirectURL, + FlowStateID: flowStateID, + AllowNoEmail: pConfig.AllowNoEmail, } if linkingTargetUser != nil { @@ -144,7 +146,7 @@ func (a *API) ExternalProviderCallback(w http.ResponseWriter, r *http.Request) e func (a *API) handleOAuthCallback(r *http.Request) (*OAuthProviderData, error) { ctx := r.Context() - providerType := getExternalProviderType(ctx) + providerType, _ := getExternalProviderType(ctx) var oAuthResponseData *OAuthProviderData var err error @@ -168,16 +170,18 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re var grantParams models.GrantParams grantParams.FillGrantParams(r) - providerType := getExternalProviderType(ctx) + providerType, allowNoEmail := getExternalProviderType(ctx) data, err := a.handleOAuthCallback(r) if err != nil { return err } userData := data.userData - if len(userData.Emails) <= 0 { + + if len(userData.Emails) == 0 && !allowNoEmail { return apierrors.NewInternalServerError("Error getting user email from external provider") } + userData.Metadata.EmailVerified = false for _, email := range userData.Emails { if email.Primary { @@ -226,7 +230,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re return terr } } else { - if user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType); terr != nil { + if user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, allowNoEmail); terr != nil { return terr } } @@ -285,7 +289,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re return nil } -func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.Request, userData *provider.UserProvidedData, providerType string) (*models.User, error) { +func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.Request, userData *provider.UserProvidedData, providerType string, allowNoEmail bool) (*models.User, error) { ctx := r.Context() aud := a.requestAud(ctx, r) config := a.config @@ -378,10 +382,13 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeUserBanned, "User is banned") } - // TODO(hf): Expand this boolean with all providers that may not have emails (like X/Twitter, Discord). - hasEmails := providerType != "web3" // intentionally not using len(userData.Emails) != 0 for better backward compatibility control + hasEmails := providerType != "web3" && !(allowNoEmail && len(userData.Emails) == 0) if hasEmails && !user.IsConfirmed() { + if len(userData.Emails) == 0 && decision.CandidateEmail.Email == "" { + return nil, apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailAddressNotProvided, fmt.Sprintf("An email address is required to create an account or sign in with %v, please set up an email address for your profile in %v and try again", providerType, providerType)) + } + // The user may have other unconfirmed email + password // combination, phone or oauth identities. These identities // need to be removed when a new oauth identity is being added @@ -400,10 +407,6 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, apierrors.NewInternalServerError("Error updating user").WithInternalError(terr) } } else { - // Some providers, like web3 don't have email data. - // Treat these as if a confirmation email has been - // sent, although the user will be created without an - // email address. emailConfirmationSent := false if decision.CandidateEmail.Email != "" { if terr = a.sendConfirmation(r, tx, user, models.ImplicitFlow); terr != nil { @@ -411,11 +414,13 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. } emailConfirmationSent = true } + if !config.Mailer.AllowUnverifiedEmailSignIns { if emailConfirmationSent { return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. A confirmation email has been sent to your %v email", providerType, providerType))) } - return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. Verify the email with %v in order to sign in", providerType, providerType))) + + return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. Verify the email with %v and try to sign in again", providerType, providerType))) } } } else { @@ -564,67 +569,97 @@ func (a *API) loadExternalState(ctx context.Context, r *http.Request) (context.C } ctx = withTargetUser(ctx, u) } - ctx = withExternalProviderType(ctx, claims.Provider) + ctx = withExternalProviderType(ctx, claims.Provider, claims.AllowNoEmail) return withSignature(ctx, state), nil } // Provider returns a Provider interface for the given name. -func (a *API) Provider(ctx context.Context, name string, scopes string) (provider.Provider, error) { +func (a *API) Provider(ctx context.Context, name string, scopes string) (provider.Provider, conf.OAuthProviderConfiguration, error) { config := a.config name = strings.ToLower(name) + var err error + var p provider.Provider + var pConfig conf.OAuthProviderConfiguration + switch name { case "apple": - return provider.NewAppleProvider(ctx, config.External.Apple) + pConfig = config.External.Apple + p, err = provider.NewAppleProvider(ctx, pConfig) case "azure": - return provider.NewAzureProvider(config.External.Azure, scopes) + pConfig = config.External.Azure + p, err = provider.NewAzureProvider(pConfig, scopes) case "bitbucket": - return provider.NewBitbucketProvider(config.External.Bitbucket) + pConfig = config.External.Bitbucket + p, err = provider.NewBitbucketProvider(pConfig) case "discord": - return provider.NewDiscordProvider(config.External.Discord, scopes) + pConfig = config.External.Discord + p, err = provider.NewDiscordProvider(pConfig, scopes) case "facebook": - return provider.NewFacebookProvider(config.External.Facebook, scopes) + pConfig = config.External.Facebook + p, err = provider.NewFacebookProvider(pConfig, scopes) case "figma": - return provider.NewFigmaProvider(config.External.Figma, scopes) + pConfig = config.External.Figma + p, err = provider.NewFigmaProvider(pConfig, scopes) case "fly": - return provider.NewFlyProvider(config.External.Fly, scopes) + pConfig = config.External.Fly + p, err = provider.NewFlyProvider(pConfig, scopes) case "github": - return provider.NewGithubProvider(config.External.Github, scopes) + pConfig = config.External.Github + p, err = provider.NewGithubProvider(pConfig, scopes) case "gitlab": - return provider.NewGitlabProvider(config.External.Gitlab, scopes) + pConfig = config.External.Gitlab + p, err = provider.NewGitlabProvider(pConfig, scopes) case "google": - return provider.NewGoogleProvider(ctx, config.External.Google, scopes) + pConfig = config.External.Google + p, err = provider.NewGoogleProvider(ctx, pConfig, scopes) case "kakao": - return provider.NewKakaoProvider(config.External.Kakao, scopes) + pConfig = config.External.Kakao + p, err = provider.NewKakaoProvider(pConfig, scopes) case "keycloak": - return provider.NewKeycloakProvider(config.External.Keycloak, scopes) + pConfig = config.External.Keycloak + p, err = provider.NewKeycloakProvider(pConfig, scopes) case "linkedin": - return provider.NewLinkedinProvider(config.External.Linkedin, scopes) + pConfig = config.External.Linkedin + p, err = provider.NewLinkedinProvider(pConfig, scopes) case "linkedin_oidc": - return provider.NewLinkedinOIDCProvider(config.External.LinkedinOIDC, scopes) + pConfig = config.External.LinkedinOIDC + p, err = provider.NewLinkedinOIDCProvider(pConfig, scopes) case "notion": - return provider.NewNotionProvider(config.External.Notion) + pConfig = config.External.Notion + p, err = provider.NewNotionProvider(pConfig) case "snapchat": - return provider.NewSnapchatProvider(config.External.Snapchat, scopes) + pConfig = config.External.Snapchat + p, err = provider.NewSnapchatProvider(pConfig, scopes) case "spotify": - return provider.NewSpotifyProvider(config.External.Spotify, scopes) + pConfig = config.External.Spotify + p, err = provider.NewSpotifyProvider(pConfig, scopes) case "slack": - return provider.NewSlackProvider(config.External.Slack, scopes) + pConfig = config.External.Slack + p, err = provider.NewSlackProvider(pConfig, scopes) case "slack_oidc": - return provider.NewSlackOIDCProvider(config.External.SlackOIDC, scopes) + pConfig = config.External.SlackOIDC + p, err = provider.NewSlackOIDCProvider(pConfig, scopes) case "twitch": - return provider.NewTwitchProvider(config.External.Twitch, scopes) + pConfig = config.External.Twitch + p, err = provider.NewTwitchProvider(pConfig, scopes) case "twitter": - return provider.NewTwitterProvider(config.External.Twitter, scopes) + pConfig = config.External.Twitter + p, err = provider.NewTwitterProvider(pConfig, scopes) case "vercel_marketplace": - return provider.NewVercelMarketplaceProvider(config.External.VercelMarketplace, scopes) + pConfig = config.External.VercelMarketplace + p, err = provider.NewVercelMarketplaceProvider(pConfig, scopes) case "workos": - return provider.NewWorkOSProvider(config.External.WorkOS) + pConfig = config.External.WorkOS + p, err = provider.NewWorkOSProvider(pConfig) case "zoom": - return provider.NewZoomProvider(config.External.Zoom) + pConfig = config.External.Zoom + p, err = provider.NewZoomProvider(pConfig) default: - return nil, fmt.Errorf("Provider %s could not be found", name) + return nil, pConfig, fmt.Errorf("Provider %s could not be found", name) } + + return p, pConfig, err } func redirectErrors(handler apiHandler, w http.ResponseWriter, r *http.Request, u *url.URL) { diff --git a/internal/api/external_oauth.go b/internal/api/external_oauth.go index 68befa5f8..f52ea53ac 100644 --- a/internal/api/external_oauth.go +++ b/internal/api/external_oauth.go @@ -10,6 +10,7 @@ import ( "github.com/sirupsen/logrus" "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/observability" "github.com/supabase/auth/internal/utilities" ) @@ -69,7 +70,7 @@ func (a *API) oAuthCallback(ctx context.Context, r *http.Request, providerType s return nil, apierrors.NewBadRequestError(apierrors.ErrorCodeBadOAuthCallback, "OAuth callback with missing authorization code missing") } - oAuthProvider, err := a.OAuthProvider(ctx, providerType) + oAuthProvider, _, err := a.OAuthProvider(ctx, providerType) if err != nil { return nil, apierrors.NewBadRequestError(apierrors.ErrorCodeOAuthProviderNotSupported, "Unsupported provider: %+v", err).WithInternalError(err) } @@ -111,7 +112,7 @@ func (a *API) oAuthCallback(ctx context.Context, r *http.Request, providerType s } func (a *API) oAuth1Callback(ctx context.Context, providerType string) (*OAuthProviderData, error) { - oAuthProvider, err := a.OAuthProvider(ctx, providerType) + oAuthProvider, _, err := a.OAuthProvider(ctx, providerType) if err != nil { return nil, apierrors.NewBadRequestError(apierrors.ErrorCodeOAuthProviderNotSupported, "Unsupported provider: %+v", err).WithInternalError(err) } @@ -141,16 +142,16 @@ func (a *API) oAuth1Callback(ctx context.Context, providerType string) (*OAuthPr } // OAuthProvider returns the corresponding oauth provider as an OAuthProvider interface -func (a *API) OAuthProvider(ctx context.Context, name string) (provider.OAuthProvider, error) { - providerCandidate, err := a.Provider(ctx, name, "") +func (a *API) OAuthProvider(ctx context.Context, name string) (provider.OAuthProvider, conf.OAuthProviderConfiguration, error) { + providerCandidate, pConfig, err := a.Provider(ctx, name, "") if err != nil { - return nil, err + return nil, pConfig, err } switch p := providerCandidate.(type) { case provider.OAuthProvider: - return p, nil + return p, pConfig, nil default: - return nil, fmt.Errorf("Provider %v cannot be used for OAuth", name) + return nil, pConfig, fmt.Errorf("Provider %v cannot be used for OAuth", name) } } diff --git a/internal/api/provider/oidc.go b/internal/api/provider/oidc.go index 5046cab6b..2c7ab1b84 100644 --- a/internal/api/provider/oidc.go +++ b/internal/api/provider/oidc.go @@ -2,7 +2,6 @@ package provider import ( "context" - "fmt" "strconv" "strings" "time" @@ -64,6 +63,8 @@ func ParseIDToken(ctx context.Context, provider *oidc.Provider, config *oidc.Con case IssuerFacebook: // Handle only Facebook Limited Login JWT, NOT Facebook Access Token token, data, err = parseFacebookIDToken(token) + case IssuerSnapchat: + token, data, err = parseSnapchatIDToken(token) default: if IsAzureIssuer(token.Issuer) { token, data, err = parseAzureIDToken(token) @@ -450,9 +451,5 @@ func parseGenericIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, }) } - if len(data.Emails) <= 0 { - return nil, nil, fmt.Errorf("provider: Generic OIDC ID token from issuer %q must contain an email address", token.Issuer) - } - return token, &data, nil } diff --git a/internal/api/provider/snapchat.go b/internal/api/provider/snapchat.go index 7070542ff..a97cc06ef 100644 --- a/internal/api/provider/snapchat.go +++ b/internal/api/provider/snapchat.go @@ -2,14 +2,17 @@ package provider import ( "context" + "encoding/json" + "fmt" "net/url" "strings" + "github.com/coreos/go-oidc/v3/oidc" "github.com/supabase/auth/internal/conf" "golang.org/x/oauth2" ) -const IssuerSnapchat = "https://accounts.snapchat.com" +const IssuerSnapchat = "https://accounts.snapchat.com/accounts/oauth2/token" const ( defaultSnapchatAuthBase = "accounts.snapchat.com" @@ -93,12 +96,7 @@ func (p snapchatProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (* data := &UserProvidedData{} - // Snapchat doesn't provide email by default, additional scopes needed - data.Emails = []Email{{ - Email: strings.ToLower(u.Data.Me.ExternalID) + "@snapchat.id", // TODO: Create a pseudo-email using the external ID - Verified: true, - Primary: true, - }} + // Snapchat doesn't provide email address! data.Metadata = &Claims{ Issuer: IssuerSnapchat, @@ -115,3 +113,33 @@ func (p snapchatProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (* return data, nil } + +func parseSnapchatIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { + var data UserProvidedData + + if err := token.Claims(&data.Metadata); err != nil { + return nil, nil, err + } + + m, _ := json.Marshal(data.Metadata) + fmt.Printf("@@@@@@@@@@@@ %s\n", m) + + if data.Metadata.Email != "" { + data.Emails = append(data.Emails, Email{ + Email: data.Metadata.Email, + Verified: true, + Primary: true, + }) + + data.Metadata.EmailVerified = true + } else { + data.Emails = append(data.Emails, Email{ + // TODO(hf): Remove this once Supabase allowed to receive email address from developer accounts + Email: data.Metadata.Subject + "@snapchat.id", + Verified: true, + Primary: true, + }) + } + + return token, &data, nil +} diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index df840f063..e15c79674 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -305,7 +305,7 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { var user *models.User // accounts potentially created via SAML can contain non-unique email addresses in the auth.users table - if user, terr = a.createAccountFromExternalIdentity(tx, r, &userProvidedData, providerType); terr != nil { + if user, terr = a.createAccountFromExternalIdentity(tx, r, &userProvidedData, providerType, false); terr != nil { return terr } if flowState != nil { diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index ef4b5d590..8369b2644 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -26,7 +26,7 @@ type IdTokenGrantParams struct { Issuer string `json:"issuer"` } -func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.GlobalConfiguration, r *http.Request) (*oidc.Provider, bool, string, []string, error) { +func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.GlobalConfiguration, r *http.Request) (*oidc.Provider, bool, string, []string, bool, error) { log := observability.GetLogEntry(r).Entry var cfg *conf.OAuthProviderConfiguration @@ -42,13 +42,13 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa if issuer == "" { detectedIssuer, err := provider.DetectAppleIDTokenIssuer(ctx, p.IdToken) if err != nil { - return nil, false, "", nil, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Apple provider").WithInternalError(err) + return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Apple provider").WithInternalError(err) } if provider.IsAppleIssuer(detectedIssuer) { issuer = detectedIssuer } else { - return nil, false, "", nil, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Detected ID token issuer is not an Apple ID token issuer") + return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Detected ID token issuer is not an Apple ID token issuer") } } acceptableClientIDs = append(acceptableClientIDs, config.External.Apple.ClientID...) @@ -68,7 +68,7 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa if issuer == "" || !provider.IsAzureIssuer(issuer) { detectedIssuer, err := provider.DetectAzureIDTokenIssuer(ctx, p.IdToken) if err != nil { - return nil, false, "", nil, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Azure provider").WithInternalError(err) + return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Azure provider").WithInternalError(err) } issuer = detectedIssuer } @@ -102,6 +102,12 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa issuer = provider.IssuerVercelMarketplace acceptableClientIDs = append(acceptableClientIDs, config.External.VercelMarketplace.ClientID...) + case p.Provider == "snapchat" || p.Issuer == provider.IssuerSnapchat: + cfg = &config.External.Snapchat + providerType = "snapchat" + issuer = provider.IssuerSnapchat + acceptableClientIDs = append(acceptableClientIDs, config.External.Snapchat.ClientID...) + default: log.WithField("issuer", p.Issuer).WithField("client_id", p.ClientID).Warn("Use of POST /token with arbitrary issuer and client_id is deprecated for security reasons. Please switch to using the API with provider only!") @@ -117,7 +123,7 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa } if !allowed { - return nil, false, "", nil, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, fmt.Sprintf("Custom OIDC provider %q not allowed", p.Provider)) + return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, fmt.Sprintf("Custom OIDC provider %q not allowed", p.Provider)) } cfg = &conf.OAuthProviderConfiguration{ @@ -127,7 +133,7 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa } if !cfg.Enabled { - return nil, false, "", nil, apierrors.NewBadRequestError(apierrors.ErrorCodeProviderDisabled, fmt.Sprintf("Provider (issuer %q) is not enabled", issuer)) + return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeProviderDisabled, fmt.Sprintf("Provider (issuer %q) is not enabled", issuer)) } oidcCtx := ctx @@ -135,12 +141,29 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa oidcCtx = oidc.InsecureIssuerURLContext(ctx, issuer) } - oidcProvider, err := oidc.NewProvider(oidcCtx, issuer) - if err != nil { - return nil, false, "", nil, err + var oidcProvider *oidc.Provider + var err error + + if providerType == "snapchat" { + // TODO: Remove this once Snapchat confirm discovery URL + snapProviderConfig := oidc.ProviderConfig{ + IssuerURL: "https://accounts.snapchat.com/accounts/oauth2/token", + AuthURL: "https://accounts.snapchat.com/accounts/oauth2/auth", + TokenURL: "https://accounts.snapchat.com/accounts/oauth2/token", + UserInfoURL: "https://accounts.snapchat.com/accounts/oauth2/userinfo", + JWKSURL: "https://accounts.snapchat.com/oauth2/.well-known/jwks.json", + Algorithms: []string{"ES256"}, + } + + oidcProvider = snapProviderConfig.NewProvider(oidcCtx) + } else { + oidcProvider, err = oidc.NewProvider(oidcCtx, issuer) + if err != nil { + return nil, false, "", nil, cfg.AllowNoEmail, err + } } - return oidcProvider, cfg.SkipNonceCheck, providerType, acceptableClientIDs, nil + return oidcProvider, cfg.SkipNonceCheck, providerType, acceptableClientIDs, cfg.AllowNoEmail, nil } // IdTokenGrant implements the id_token grant type flow @@ -163,7 +186,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R return apierrors.NewOAuthError("invalid request", "provider or client_id and issuer required") } - oidcProvider, skipNonceCheck, providerType, acceptableClientIDs, err := params.getProvider(ctx, config, r) + oidcProvider, skipNonceCheck, providerType, acceptableClientIDs, allowNoEmail, err := params.getProvider(ctx, config, r) if err != nil { return err } @@ -261,7 +284,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R var user *models.User var terr error - user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType) + user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, allowNoEmail) if terr != nil { return terr } diff --git a/internal/api/token_oidc_test.go b/internal/api/token_oidc_test.go index 1eab99ebd..bef57d1e3 100644 --- a/internal/api/token_oidc_test.go +++ b/internal/api/token_oidc_test.go @@ -60,10 +60,11 @@ func (ts *TokenOIDCTestSuite) TestGetProvider() { ts.Config.External.AllowedIdTokenIssuers = []string{server.URL} req := httptest.NewRequest(http.MethodPost, "http://localhost", nil) - oidcProvider, skipNonceCheck, providerType, acceptableClientIds, err := params.getProvider(context.Background(), ts.Config, req) + oidcProvider, skipNonceCheck, providerType, acceptableClientIds, allowNoEmail, err := params.getProvider(context.Background(), ts.Config, req) require.NoError(ts.T(), err) require.NotNil(ts.T(), oidcProvider) require.False(ts.T(), skipNonceCheck) + require.False(ts.T(), allowNoEmail) require.Equal(ts.T(), params.Provider, providerType) require.NotEmpty(ts.T(), acceptableClientIds) } diff --git a/internal/api/web3.go b/internal/api/web3.go index 7fc11c95d..8b1d1881b 100644 --- a/internal/api/web3.go +++ b/internal/api/web3.go @@ -144,7 +144,7 @@ func (a *API) web3GrantSolana(ctx context.Context, w http.ResponseWriter, r *htt } err = db.Transaction(func(tx *storage.Connection) error { - user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, providerType) + user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, providerType, true) if terr != nil { return terr } @@ -280,7 +280,7 @@ func (a *API) web3GrantEthereum(ctx context.Context, w http.ResponseWriter, r *h } err = db.Transaction(func(tx *storage.Connection) error { - user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, providerType) + user, terr := a.createAccountFromExternalIdentity(tx, r, &userData, providerType, true) if terr != nil { return terr } diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 8aff15f91..d3fd6962f 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -59,12 +59,13 @@ func (t *Time) UnmarshalText(text []byte) error { // OAuthProviderConfiguration holds all config related to external account providers. type OAuthProviderConfiguration struct { - ClientID []string `json:"client_id" split_words:"true"` - Secret string `json:"secret"` - RedirectURI string `json:"redirect_uri" split_words:"true"` - URL string `json:"url"` - ApiURL string `json:"api_url" split_words:"true"` - Enabled bool `json:"enabled"` + ClientID []string `json:"client_id" split_words:"true"` + Secret string `json:"secret"` + RedirectURI string `json:"redirect_uri" split_words:"true"` + URL string `json:"url"` + ApiURL string `json:"api_url" split_words:"true"` + Enabled bool `json:"enabled"` + AllowNoEmail bool `json:"allow_no_email" split_words:"true"` // SkipNonceCheck bypasses nonce verification during OIDC token validation. // Note: Nonce verification helps prevent replay attacks; only disable when necessary. SkipNonceCheck bool `json:"skip_nonce_check" split_words:"true"` From 66b022a63da578be78cbe27028b9aff418b8cf3c Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Mon, 4 Aug 2025 16:00:53 +0200 Subject: [PATCH 02/10] use EmailOptional instead --- internal/api/external.go | 14 +++++++------- internal/api/token_oidc.go | 25 ++++--------------------- internal/conf/configuration.go | 14 +++++++------- 3 files changed, 18 insertions(+), 35 deletions(-) diff --git a/internal/api/external.go b/internal/api/external.go index 4094ecf68..6d36e5f7a 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -31,7 +31,7 @@ type ExternalProviderClaims struct { Referrer string `json:"referrer,omitempty"` FlowStateID string `json:"flow_state_id"` LinkingTargetID string `json:"linking_target_id,omitempty"` - AllowNoEmail bool `json:"allow_no_email,omitempty"` + EmailOptional bool `json:"email_optional,omitempty"` } // ExternalProviderRedirect redirects the request to the oauth provider @@ -97,11 +97,11 @@ func (a *API) GetExternalProviderRedirectURL(w http.ResponseWriter, r *http.Requ SiteURL: config.SiteURL, InstanceID: uuid.Nil.String(), }, - Provider: providerType, - InviteToken: inviteToken, - Referrer: redirectURL, - FlowStateID: flowStateID, - AllowNoEmail: pConfig.AllowNoEmail, + Provider: providerType, + InviteToken: inviteToken, + Referrer: redirectURL, + FlowStateID: flowStateID, + EmailOptional: pConfig.EmailOptional, } if linkingTargetUser != nil { @@ -569,7 +569,7 @@ func (a *API) loadExternalState(ctx context.Context, r *http.Request) (context.C } ctx = withTargetUser(ctx, u) } - ctx = withExternalProviderType(ctx, claims.Provider, claims.AllowNoEmail) + ctx = withExternalProviderType(ctx, claims.Provider, claims.EmailOptional) return withSignature(ctx, state), nil } diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index 8369b2644..fe7327231 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -141,29 +141,12 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa oidcCtx = oidc.InsecureIssuerURLContext(ctx, issuer) } - var oidcProvider *oidc.Provider - var err error - - if providerType == "snapchat" { - // TODO: Remove this once Snapchat confirm discovery URL - snapProviderConfig := oidc.ProviderConfig{ - IssuerURL: "https://accounts.snapchat.com/accounts/oauth2/token", - AuthURL: "https://accounts.snapchat.com/accounts/oauth2/auth", - TokenURL: "https://accounts.snapchat.com/accounts/oauth2/token", - UserInfoURL: "https://accounts.snapchat.com/accounts/oauth2/userinfo", - JWKSURL: "https://accounts.snapchat.com/oauth2/.well-known/jwks.json", - Algorithms: []string{"ES256"}, - } - - oidcProvider = snapProviderConfig.NewProvider(oidcCtx) - } else { - oidcProvider, err = oidc.NewProvider(oidcCtx, issuer) - if err != nil { - return nil, false, "", nil, cfg.AllowNoEmail, err - } + oidcProvider, err := oidc.NewProvider(oidcCtx, issuer) + if err != nil { + return nil, false, "", nil, cfg.EmailOptional, err } - return oidcProvider, cfg.SkipNonceCheck, providerType, acceptableClientIDs, cfg.AllowNoEmail, nil + return oidcProvider, cfg.SkipNonceCheck, providerType, acceptableClientIDs, cfg.EmailOptional, nil } // IdTokenGrant implements the id_token grant type flow diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index d3fd6962f..d3e971923 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -59,13 +59,13 @@ func (t *Time) UnmarshalText(text []byte) error { // OAuthProviderConfiguration holds all config related to external account providers. type OAuthProviderConfiguration struct { - ClientID []string `json:"client_id" split_words:"true"` - Secret string `json:"secret"` - RedirectURI string `json:"redirect_uri" split_words:"true"` - URL string `json:"url"` - ApiURL string `json:"api_url" split_words:"true"` - Enabled bool `json:"enabled"` - AllowNoEmail bool `json:"allow_no_email" split_words:"true"` + ClientID []string `json:"client_id" split_words:"true"` + Secret string `json:"secret"` + RedirectURI string `json:"redirect_uri" split_words:"true"` + URL string `json:"url"` + ApiURL string `json:"api_url" split_words:"true"` + Enabled bool `json:"enabled"` + EmailOptional bool `json:"email_optional" split_words:"true"` // SkipNonceCheck bypasses nonce verification during OIDC token validation. // Note: Nonce verification helps prevent replay attacks; only disable when necessary. SkipNonceCheck bool `json:"skip_nonce_check" split_words:"true"` From bf6c72e319b6a246babfe60d5befae36fe90ce14 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 7 Aug 2025 12:20:21 +0200 Subject: [PATCH 03/10] fix tests --- hack/test.env | 1 + internal/api/external_snapchat_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hack/test.env b/hack/test.env index 9b80f1dc9..97a01ba03 100644 --- a/hack/test.env +++ b/hack/test.env @@ -80,6 +80,7 @@ GOTRUE_EXTERNAL_SNAPCHAT_ENABLED=true GOTRUE_EXTERNAL_SNAPCHAT_CLIENT_ID=testclientid GOTRUE_EXTERNAL_SNAPCHAT_SECRET=testsecret GOTRUE_EXTERNAL_SNAPCHAT_REDIRECT_URI=https://identity.services.netlify.com/callback +GOTRUE_EXTERNAL_SNAPCHAT_EMAIL_OPTIONAL=true GOTRUE_EXTERNAL_SPOTIFY_ENABLED=true GOTRUE_EXTERNAL_SPOTIFY_CLIENT_ID=testclientid GOTRUE_EXTERNAL_SPOTIFY_SECRET=testsecret diff --git a/internal/api/external_snapchat_test.go b/internal/api/external_snapchat_test.go index ab30a14ef..879651574 100644 --- a/internal/api/external_snapchat_test.go +++ b/internal/api/external_snapchat_test.go @@ -72,7 +72,7 @@ func (ts *ExternalTestSuite) TestSignupExternalSnapchat_AuthorizationCode() { u := performAuthorization(ts, "snapchat", code, "") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "snapchattestid@snapchat.id", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") + assertAuthorizationSuccess(ts, u, tokenCount, userCount, "", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") } func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupErrorWhenNoUser() { @@ -91,7 +91,7 @@ func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupErrorWhenNoU func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupSuccessWithExistingUser() { ts.Config.DisableSignup = true - ts.createUser("snapchatTestId", "snapchattestid@snapchat.id", "Snapchat Test", "http://example.com/bitmoji", "") + ts.createUser("snapchatTestId", "", "Snapchat Test", "http://example.com/bitmoji", "") tokenCount, userCount := 0, 0 code := "authcode" @@ -100,13 +100,13 @@ func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupSuccessWithE u := performAuthorization(ts, "snapchat", code, "") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "snapchattestid@snapchat.id", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") + assertAuthorizationSuccess(ts, u, tokenCount, userCount, "", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") } func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatSuccessWhenMatchingToken() { // name and avatar should be populated from Snapchat API // Use the same email that the provider will generate - converted to lowercase - ts.createUser("snapchatTestId", "snapchattestid@snapchat.id", "", "", "invite_token") + ts.createUser("snapchatTestId", "", "", "", "invite_token") tokenCount, userCount := 0, 0 code := "authcode" @@ -115,7 +115,7 @@ func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatSuccessWhenMatchingT u := performAuthorization(ts, "snapchat", code, "invite_token") - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "snapchattestid@snapchat.id", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") + assertAuthorizationSuccess(ts, u, tokenCount, userCount, "", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") } func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatErrorWhenNoMatchingToken() { From e9b0a39197dd9c582e7b86ec8a2938fed9be4cf1 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 7 Aug 2025 12:24:42 +0200 Subject: [PATCH 04/10] remove stray code --- internal/api/provider/snapchat.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/api/provider/snapchat.go b/internal/api/provider/snapchat.go index a97cc06ef..6755969b5 100644 --- a/internal/api/provider/snapchat.go +++ b/internal/api/provider/snapchat.go @@ -2,8 +2,6 @@ package provider import ( "context" - "encoding/json" - "fmt" "net/url" "strings" @@ -121,9 +119,6 @@ func parseSnapchatIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData return nil, nil, err } - m, _ := json.Marshal(data.Metadata) - fmt.Printf("@@@@@@@@@@@@ %s\n", m) - if data.Metadata.Email != "" { data.Emails = append(data.Emails, Email{ Email: data.Metadata.Email, From 7f7b075f37b8cc65f3142f8b83817138e8e1a0a6 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 7 Aug 2025 13:22:52 +0200 Subject: [PATCH 05/10] fix tests --- internal/api/external_snapchat_test.go | 37 +------------------------- internal/api/external_test.go | 12 ++++++++- 2 files changed, 12 insertions(+), 37 deletions(-) diff --git a/internal/api/external_snapchat_test.go b/internal/api/external_snapchat_test.go index 879651574..cb3e05815 100644 --- a/internal/api/external_snapchat_test.go +++ b/internal/api/external_snapchat_test.go @@ -103,39 +103,4 @@ func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupSuccessWithE assertAuthorizationSuccess(ts, u, tokenCount, userCount, "", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") } -func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatSuccessWhenMatchingToken() { - // name and avatar should be populated from Snapchat API - // Use the same email that the provider will generate - converted to lowercase - ts.createUser("snapchatTestId", "", "", "", "invite_token") - - tokenCount, userCount := 0, 0 - code := "authcode" - server := SnapchatTestSignupSetup(ts, &tokenCount, &userCount, code, snapchatUser) - defer server.Close() - - u := performAuthorization(ts, "snapchat", code, "invite_token") - - assertAuthorizationSuccess(ts, u, tokenCount, userCount, "", "Snapchat Test", "snapchatTestId", "http://example.com/bitmoji") -} - -func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatErrorWhenNoMatchingToken() { - tokenCount, userCount := 0, 0 - code := "authcode" - server := SnapchatTestSignupSetup(ts, &tokenCount, &userCount, code, snapchatUser) - defer server.Close() - - w := performAuthorizationRequest(ts, "snapchat", "invite_token") - ts.Require().Equal(http.StatusNotFound, w.Code) -} - -func (ts *ExternalTestSuite) TestInviteTokenExternalSnapchatErrorWhenWrongToken() { - ts.createUser("snapchatTestId", "", "", "", "invite_token") - - tokenCount, userCount := 0, 0 - code := "authcode" - server := SnapchatTestSignupSetup(ts, &tokenCount, &userCount, code, snapchatUser) - defer server.Close() - - w := performAuthorizationRequest(ts, "snapchat", "wrong_token") - ts.Require().Equal(http.StatusNotFound, w.Code) -} +// Snapchat may not send email address so Invite Token flow can't apply. diff --git a/internal/api/external_test.go b/internal/api/external_test.go index bef89d736..ef49370e2 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -169,7 +169,17 @@ func assertAuthorizationSuccess(ts *ExternalTestSuite, u *url.URL, tokenCount in } // ensure user has been created with metadata - user, err := models.FindUserByEmailAndAudience(ts.API.db, email, ts.Config.JWT.Aud) + var user *models.User + if email != "" { + user, err = models.FindUserByEmailAndAudience(ts.API.db, email, ts.Config.JWT.Aud) + } else { + identity := &models.Identity{} + err = ts.API.db.Q().Where("provider_id = ?", providerId).First(identity) + ts.Require().NoError(err) + + user, err = models.FindUserByID(ts.API.db, identity.UserID) + } + ts.Require().NoError(err) ts.Equal(providerId, user.UserMetaData["provider_id"]) ts.Equal(name, user.UserMetaData["full_name"]) From 919a8537ee701ce740946f6eee6f0e6c81c2bddd Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Thu, 7 Aug 2025 15:21:08 +0200 Subject: [PATCH 06/10] fix more tests --- internal/api/external_snapchat_test.go | 2 +- internal/api/external_test.go | 40 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/api/external_snapchat_test.go b/internal/api/external_snapchat_test.go index cb3e05815..29cd052a9 100644 --- a/internal/api/external_snapchat_test.go +++ b/internal/api/external_snapchat_test.go @@ -91,7 +91,7 @@ func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupErrorWhenNoU func (ts *ExternalTestSuite) TestSignupExternalSnapchatDisableSignupSuccessWithExistingUser() { ts.Config.DisableSignup = true - ts.createUser("snapchatTestId", "", "Snapchat Test", "http://example.com/bitmoji", "") + ts.createUserWithIdentity("snapchat", "snapchatTestId", "", "Snapchat Test", "http://example.com/bitmoji", "") tokenCount, userCount := 0, 0 code := "authcode" diff --git a/internal/api/external_test.go b/internal/api/external_test.go index ef49370e2..d2adcac42 100644 --- a/internal/api/external_test.go +++ b/internal/api/external_test.go @@ -70,6 +70,46 @@ func (ts *ExternalTestSuite) createUser(providerId string, email string, name st return u, err } +func (ts *ExternalTestSuite) createUserWithIdentity(providerType, providerId string, email string, name string, avatar string, confirmationToken string) (*models.User, error) { + // Cleanup existing user, if they already exist + if u, _ := models.FindUserByEmailAndAudience(ts.API.db, email, ts.Config.JWT.Aud); u != nil { + require.NoError(ts.T(), ts.API.db.Destroy(u), "Error deleting user") + } + + userData := map[string]interface{}{"provider_id": providerId, "full_name": name} + if avatar != "" { + userData["avatar_url"] = avatar + } + u, err := models.NewUser("", email, "test", ts.Config.JWT.Aud, userData) + + if confirmationToken != "" { + u.ConfirmationToken = confirmationToken + } + ts.Require().NoError(err, "Error making new user") + ts.Require().NoError(ts.API.db.Create(u), "Error creating user") + + if confirmationToken != "" { + ts.Require().NoError(models.CreateOneTimeToken(ts.API.db, u.ID, email, u.ConfirmationToken, models.ConfirmationToken), "Error creating one-time confirmation/invite token") + } + + if email != "" { + i, err := models.NewIdentity(u, "email", map[string]interface{}{ + "sub": u.ID.String(), + "email": email, + }) + ts.Require().NoError(err) + ts.Require().NoError(ts.API.db.Create(i), "Error creating identity") + } + + i, err := models.NewIdentity(u, providerType, map[string]interface{}{ + "sub": providerId, + }) + ts.Require().NoError(err) + ts.Require().NoError(ts.API.db.Create(i), "Error creating identity") + + return u, err +} + func performAuthorizationRequest(ts *ExternalTestSuite, provider string, inviteToken string) *httptest.ResponseRecorder { authorizeURL := "http://localhost/authorize?provider=" + provider if inviteToken != "" { From f79d386b3e7eb6c3e8bd1643468521f68019276e Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Fri, 8 Aug 2025 09:44:35 +0200 Subject: [PATCH 07/10] only block account creation, not sign in without an email address in oidc --- internal/api/external.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/api/external.go b/internal/api/external.go index 6d36e5f7a..21858b750 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -382,10 +382,10 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeUserBanned, "User is banned") } - hasEmails := providerType != "web3" && !(allowNoEmail && len(userData.Emails) == 0) + hasEmails := providerType != "web3" && !(allowNoEmail && decision.CandidateEmail.Email == "") if hasEmails && !user.IsConfirmed() { - if len(userData.Emails) == 0 && decision.CandidateEmail.Email == "" { + if decision.CandidateEmail.Email == "" && decision.Decision == models.CreateAccount { return nil, apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailAddressNotProvided, fmt.Sprintf("An email address is required to create an account or sign in with %v, please set up an email address for your profile in %v and try again", providerType, providerType)) } From 4308bc2db0ab94563a8506d23e295678aa7fe98d Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Fri, 8 Aug 2025 11:45:39 +0200 Subject: [PATCH 08/10] simplify change --- internal/api/context.go | 16 ++++++++-------- internal/api/external.go | 16 ++++++---------- internal/api/token_oidc.go | 4 ++-- internal/api/token_oidc_test.go | 4 ++-- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/internal/api/context.go b/internal/api/context.go index 01a60149f..5f0f744c4 100644 --- a/internal/api/context.go +++ b/internal/api/context.go @@ -15,8 +15,8 @@ func (c contextKey) String() string { } const ( - externalProviderTypeKey = contextKey("external_provider_type") - externalProviderAllowNoEmailKey = contextKey("external_provider_allow_no_email") + externalProviderTypeKey = contextKey("external_provider_type") + externalProviderEmailOptionalKey = contextKey("external_provider_allow_no_email") tokenKey = contextKey("jwt") inviteTokenKey = contextKey("invite_token") @@ -154,26 +154,26 @@ func getInviteToken(ctx context.Context) string { } // withExternalProviderType adds the provided request ID to the context. -func withExternalProviderType(ctx context.Context, id string, allowNoEmail bool) context.Context { - return context.WithValue(context.WithValue(ctx, externalProviderTypeKey, id), externalProviderAllowNoEmailKey, allowNoEmail) +func withExternalProviderType(ctx context.Context, id string, emailOptional bool) context.Context { + return context.WithValue(context.WithValue(ctx, externalProviderTypeKey, id), externalProviderEmailOptionalKey, emailOptional) } // getExternalProviderType returns the provider type and whether user data without email address should be allowed. func getExternalProviderType(ctx context.Context) (string, bool) { idValue := ctx.Value(externalProviderTypeKey) - allowNoEmailValue := ctx.Value(externalProviderAllowNoEmailKey) + emailOptionalValue := ctx.Value(externalProviderEmailOptionalKey) id, okID := idValue.(string) if !okID { return "", false } - allowNoEmail, okAllowNoEmail := allowNoEmailValue.(bool) - if !okAllowNoEmail { + emailOptional, okEmailOptional := emailOptionalValue.(bool) + if !okEmailOptional { return "", false } - return id, allowNoEmail + return id, emailOptional } func withExternalReferrer(ctx context.Context, token string) context.Context { diff --git a/internal/api/external.go b/internal/api/external.go index 21858b750..ad56c34b2 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -170,7 +170,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re var grantParams models.GrantParams grantParams.FillGrantParams(r) - providerType, allowNoEmail := getExternalProviderType(ctx) + providerType, emailOptional := getExternalProviderType(ctx) data, err := a.handleOAuthCallback(r) if err != nil { return err @@ -178,7 +178,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re userData := data.userData - if len(userData.Emails) == 0 && !allowNoEmail { + if len(userData.Emails) == 0 && !emailOptional { return apierrors.NewInternalServerError("Error getting user email from external provider") } @@ -230,7 +230,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re return terr } } else { - if user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, allowNoEmail); terr != nil { + if user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, emailOptional); terr != nil { return terr } } @@ -289,7 +289,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re return nil } -func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.Request, userData *provider.UserProvidedData, providerType string, allowNoEmail bool) (*models.User, error) { +func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.Request, userData *provider.UserProvidedData, providerType string, emailOptional bool) (*models.User, error) { ctx := r.Context() aud := a.requestAud(ctx, r) config := a.config @@ -382,13 +382,9 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, apierrors.NewForbiddenError(apierrors.ErrorCodeUserBanned, "User is banned") } - hasEmails := providerType != "web3" && !(allowNoEmail && decision.CandidateEmail.Email == "") + hasEmails := providerType != "web3" && !(emailOptional && decision.CandidateEmail.Email == "") if hasEmails && !user.IsConfirmed() { - if decision.CandidateEmail.Email == "" && decision.Decision == models.CreateAccount { - return nil, apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailAddressNotProvided, fmt.Sprintf("An email address is required to create an account or sign in with %v, please set up an email address for your profile in %v and try again", providerType, providerType)) - } - // The user may have other unconfirmed email + password // combination, phone or oauth identities. These identities // need to be removed when a new oauth identity is being added @@ -420,7 +416,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. A confirmation email has been sent to your %v email", providerType, providerType))) } - return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. Verify the email with %v and try to sign in again", providerType, providerType))) + return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeProviderEmailNeedsVerification, fmt.Sprintf("Unverified email with %v. Verify the email with %v in order to sign in", providerType, providerType))) } } } else { diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index fe7327231..841168fbe 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -169,7 +169,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R return apierrors.NewOAuthError("invalid request", "provider or client_id and issuer required") } - oidcProvider, skipNonceCheck, providerType, acceptableClientIDs, allowNoEmail, err := params.getProvider(ctx, config, r) + oidcProvider, skipNonceCheck, providerType, acceptableClientIDs, emailOptional, err := params.getProvider(ctx, config, r) if err != nil { return err } @@ -267,7 +267,7 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R var user *models.User var terr error - user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, allowNoEmail) + user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType, emailOptional) if terr != nil { return terr } diff --git a/internal/api/token_oidc_test.go b/internal/api/token_oidc_test.go index bef57d1e3..23e872eb1 100644 --- a/internal/api/token_oidc_test.go +++ b/internal/api/token_oidc_test.go @@ -60,11 +60,11 @@ func (ts *TokenOIDCTestSuite) TestGetProvider() { ts.Config.External.AllowedIdTokenIssuers = []string{server.URL} req := httptest.NewRequest(http.MethodPost, "http://localhost", nil) - oidcProvider, skipNonceCheck, providerType, acceptableClientIds, allowNoEmail, err := params.getProvider(context.Background(), ts.Config, req) + oidcProvider, skipNonceCheck, providerType, acceptableClientIds, emailOptional, err := params.getProvider(context.Background(), ts.Config, req) require.NoError(ts.T(), err) require.NotNil(ts.T(), oidcProvider) require.False(ts.T(), skipNonceCheck) - require.False(ts.T(), allowNoEmail) + require.False(ts.T(), emailOptional) require.Equal(ts.T(), params.Provider, providerType) require.NotEmpty(ts.T(), acceptableClientIds) } From e0cd085d9a696f6b9e77090b02baa38819fe2738 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Mon, 18 Aug 2025 11:29:23 +0200 Subject: [PATCH 09/10] format code --- internal/api/apierrors/errorcode.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/api/apierrors/errorcode.go b/internal/api/apierrors/errorcode.go index 00797d027..710797548 100644 --- a/internal/api/apierrors/errorcode.go +++ b/internal/api/apierrors/errorcode.go @@ -90,11 +90,11 @@ const ( ErrorCodeMFAWebAuthnVerifyDisabled ErrorCode = "mfa_webauthn_verify_not_enabled" ErrorCodeMFAVerifiedFactorExists ErrorCode = "mfa_verified_factor_exists" //#nosec G101 -- Not a secret value. - ErrorCodeInvalidCredentials ErrorCode = "invalid_credentials" - ErrorCodeEmailAddressNotAuthorized ErrorCode = "email_address_not_authorized" - ErrorCodeEmailAddressInvalid ErrorCode = "email_address_invalid" - ErrorCodeWeb3ProviderDisabled ErrorCode = "web3_provider_disabled" - ErrorCodeWeb3UnsupportedChain ErrorCode = "web3_unsupported_chain" + ErrorCodeInvalidCredentials ErrorCode = "invalid_credentials" + ErrorCodeEmailAddressNotAuthorized ErrorCode = "email_address_not_authorized" + ErrorCodeEmailAddressInvalid ErrorCode = "email_address_invalid" + ErrorCodeWeb3ProviderDisabled ErrorCode = "web3_provider_disabled" + ErrorCodeWeb3UnsupportedChain ErrorCode = "web3_unsupported_chain" ErrorCodeOAuthDynamicClientRegistrationDisabled ErrorCode = "oauth_dynamic_client_registration_disabled" - ErrorCodeEmailAddressNotProvided ErrorCode = "email_address_not_provided" + ErrorCodeEmailAddressNotProvided ErrorCode = "email_address_not_provided" ) From 42cd7db1b7803294434da885e8fe1aafa401eb32 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Mon, 18 Aug 2025 11:29:52 +0200 Subject: [PATCH 10/10] remove code --- internal/api/provider/snapchat.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/api/provider/snapchat.go b/internal/api/provider/snapchat.go index 6755969b5..74af9b853 100644 --- a/internal/api/provider/snapchat.go +++ b/internal/api/provider/snapchat.go @@ -127,13 +127,6 @@ func parseSnapchatIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData }) data.Metadata.EmailVerified = true - } else { - data.Emails = append(data.Emails, Email{ - // TODO(hf): Remove this once Supabase allowed to receive email address from developer accounts - Email: data.Metadata.Subject + "@snapchat.id", - Verified: true, - Primary: true, - }) } return token, &data, nil