From 7ba7334f1033cdf0eecf3da02cc7774e2e3631d3 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Tue, 16 May 2023 13:53:53 +0200 Subject: [PATCH 1/5] feat: use Google ID token in OAuth flow --- internal/api/external.go | 2 +- internal/api/provider/google.go | 166 ++++++++++++++++++++++++++------ 2 files changed, 138 insertions(+), 30 deletions(-) diff --git a/internal/api/external.go b/internal/api/external.go index 4abf91c78..da9f7c295 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -526,7 +526,7 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide case "gitlab": return provider.NewGitlabProvider(config.External.Gitlab, scopes) case "google": - return provider.NewGoogleProvider(config.External.Google, scopes) + return provider.NewGoogleProvider(ctx, config.External.Google, scopes) case "kakao": return provider.NewKakaoProvider(config.External.Kakao, scopes) case "keycloak": diff --git a/internal/api/provider/google.go b/internal/api/provider/google.go index 843d2dfe0..317dd1525 100644 --- a/internal/api/provider/google.go +++ b/internal/api/provider/google.go @@ -3,38 +3,116 @@ package provider import ( "context" "errors" + "fmt" "strings" + "github.com/coreos/go-oidc/v3/oidc" + "github.com/sirupsen/logrus" "github.com/supabase/gotrue/internal/conf" "golang.org/x/oauth2" ) -const ( - defaultGoogleAuthBase = "accounts.google.com" - defaultGoogleAPIBase = "www.googleapis.com" -) - -type googleProvider struct { - *oauth2.Config - APIPath string -} - type googleUser struct { ID string `json:"id"` + Subject string `json:"sub"` + Issuer string `json:"iss"` Name string `json:"name"` AvatarURL string `json:"picture"` Email string `json:"email"` - EmailVerified bool `json:"verified_email"` + VerifiedEmail bool `json:"verified_email"` + EmailVerified bool `json:"email_verified"` + HostedDomain string `json:"hd"` +} + +func (u googleUser) IsEmailVerified() bool { + return u.VerifiedEmail || u.EmailVerified +} + +type GoogleOIDCProvider struct { + *oidc.Provider +} + +// ParseIDToken parses a Google issued OIDC ID token. You should verify the aud +// claim on your own! +func (p *GoogleOIDCProvider) ParseIDToken(ctx context.Context, idToken string) (*oidc.IDToken, *UserProvidedData, error) { + verifier := p.Verifier(&oidc.Config{ + // aud claim check to be performed by other flows + SkipClientIDCheck: true, + }) + + token, err := verifier.Verify(ctx, idToken) + if err != nil { + return nil, nil, err + } + + var claims googleUser + if err := token.Claims(&claims); err != nil { + return nil, nil, err + } + + var data UserProvidedData + + if claims.Email != "" { + data.Emails = append(data.Emails, Email{ + Email: claims.Email, + Verified: claims.IsEmailVerified(), + Primary: true, + }) + } + + if len(data.Emails) <= 0 { + return nil, nil, errors.New("provider: Google ID token must contain an email address") + } + + data.Metadata = &Claims{ + Issuer: claims.Issuer, + Subject: claims.Subject, + Name: claims.Name, + Picture: claims.AvatarURL, + Email: claims.Email, + EmailVerified: claims.IsEmailVerified(), + + // To be deprecated + AvatarURL: claims.AvatarURL, + FullName: claims.Name, + ProviderId: claims.Subject, + } + + if claims.HostedDomain != "" { + data.Metadata.CustomClaims = map[string]any{ + "hd": claims.HostedDomain, + } + } + + return token, &data, nil } -// NewGoogleProvider creates a Google account provider. -func NewGoogleProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { +// NewGoogleOIDCProvider creates a new ODIC provider with +// https://accounts.google.com as the issuer. +func NewGoogleOIDCProvider(ctx context.Context) (*GoogleOIDCProvider, error) { + provider, err := oidc.NewProvider(ctx, "https://accounts.google.com") + if err != nil { + return nil, err + } + + return &GoogleOIDCProvider{provider}, nil +} + +type googleProvider struct { + *oauth2.Config + + oidc *GoogleOIDCProvider +} + +// NewGoogleProvider creates a Google OAuth2 identity provider. +func NewGoogleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { if err := ext.Validate(); err != nil { return nil, err } - authHost := chooseHost(ext.URL, defaultGoogleAuthBase) - apiPath := chooseHost(ext.URL, defaultGoogleAPIBase) + "/userinfo/v2/me" + if ext.URL != "" { + logrus.Warn("Google OAuth provider has URL config set which is ignored (check GOTRUE_EXTERNAL_GOOGLE_URL)") + } oauthScopes := []string{ "email", @@ -45,18 +123,20 @@ func NewGoogleProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAut oauthScopes = append(oauthScopes, strings.Split(scopes, ",")...) } + oidcProvider, err := NewGoogleOIDCProvider(ctx) + if err != nil { + return nil, err + } + return &googleProvider{ Config: &oauth2.Config{ ClientID: ext.ClientID, ClientSecret: ext.Secret, - Endpoint: oauth2.Endpoint{ - AuthURL: authHost + "/o/oauth2/auth", - TokenURL: authHost + "/o/oauth2/token", - }, - Scopes: oauthScopes, - RedirectURL: ext.RedirectURI, + Endpoint: oidcProvider.Endpoint(), + Scopes: oauthScopes, + RedirectURL: ext.RedirectURI, }, - APIPath: apiPath, + oidc: oidcProvider, }, nil } @@ -64,33 +144,61 @@ func (g googleProvider) GetOAuthToken(code string) (*oauth2.Token, error) { return g.Exchange(context.Background(), code) } +const oauthGoogleUserInfoEndpoint = "https://www.googleapis.com/userinfo/v2/me" + func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { + if idToken := tok.Extra("id_token"); idToken != nil { + token, data, err := g.oidc.ParseIDToken(ctx, idToken.(string)) + if err != nil { + return nil, err + } + + matchesAudience := false + for _, aud := range token.Audience { + if g.Config.ClientID == aud { + matchesAudience = true + break + } + } + + if !matchesAudience { + return nil, fmt.Errorf("provider: Google ID token issued for audience(s) %q but expected %q", strings.Join(token.Audience, ", "), g.Config.ClientID) + } + + return data, err + } + + // This whole section offers legacy support in case the Google OAuth2 + // flow does not return an ID Token for the user, which appears to + // always be the case. + logrus.Info("Using Google OAuth2 user info endpoint, an ID token was not returned by Google") + var u googleUser - if err := makeRequest(ctx, tok, g.Config, g.APIPath, &u); err != nil { + if err := makeRequest(ctx, tok, g.Config, oauthGoogleUserInfoEndpoint, &u); err != nil { return nil, err } - data := &UserProvidedData{} + var data UserProvidedData if u.Email != "" { data.Emails = append(data.Emails, Email{ Email: u.Email, - Verified: u.EmailVerified, + Verified: u.IsEmailVerified(), Primary: true, }) } if len(data.Emails) <= 0 { - return nil, errors.New("unable to find email with Google provider") + return nil, errors.New("provider: Google OAuth2 user info endpoint did not return an email address") } data.Metadata = &Claims{ - Issuer: g.APIPath, + Issuer: oauthGoogleUserInfoEndpoint, Subject: u.ID, Name: u.Name, Picture: u.AvatarURL, Email: u.Email, - EmailVerified: u.EmailVerified, + EmailVerified: u.IsEmailVerified(), // To be deprecated AvatarURL: u.AvatarURL, @@ -98,5 +206,5 @@ func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us ProviderId: u.ID, } - return data, nil + return &data, nil } From 5d7677bffc03da3e23a6dcf236192dc7c3bfc7b9 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Tue, 16 May 2023 13:56:05 +0200 Subject: [PATCH 2/5] refactor: move id token grant flow to separate file --- internal/api/token.go | 264 ----------------------------------- internal/api/token_oidc.go | 276 +++++++++++++++++++++++++++++++++++++ 2 files changed, 276 insertions(+), 264 deletions(-) create mode 100644 internal/api/token_oidc.go diff --git a/internal/api/token.go b/internal/api/token.go index 5412854f3..6d73dd48f 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -2,23 +2,19 @@ package api import ( "context" - "crypto/sha256" "encoding/json" "errors" - "fmt" "net/http" "net/url" "strconv" "time" - "github.com/coreos/go-oidc/v3/oidc" "github.com/gofrs/uuid" "github.com/golang-jwt/jwt" "github.com/supabase/gotrue/internal/conf" "github.com/supabase/gotrue/internal/metering" "github.com/supabase/gotrue/internal/models" - "github.com/supabase/gotrue/internal/observability" "github.com/supabase/gotrue/internal/storage" ) @@ -69,15 +65,6 @@ type RefreshTokenGrantParams struct { RefreshToken string `json:"refresh_token"` } -// IdTokenGrantParams are the parameters the IdTokenGrant method accepts -type IdTokenGrantParams struct { - IdToken string `json:"id_token"` - Nonce string `json:"nonce"` - Provider string `json:"provider"` - ClientID string `json:"client_id"` - Issuer string `json:"issuer"` -} - // PKCEGrantParams are the parameters the PKCEGrant method accepts type PKCEGrantParams struct { AuthCode string `json:"auth_code"` @@ -87,64 +74,6 @@ type PKCEGrantParams struct { const useCookieHeader = "x-use-cookie" const InvalidLoginMessage = "Invalid login credentials" -func (p *IdTokenGrantParams) getVerifier(ctx context.Context, config *conf.GlobalConfiguration) (*oidc.IDTokenVerifier, error) { - var provider *oidc.Provider - var err error - var oAuthProvider conf.OAuthProviderConfiguration - var oAuthProviderClientId string - switch p.Provider { - case "apple": - oAuthProvider = config.External.Apple - oAuthProviderClientId = config.External.IosBundleId - if oAuthProviderClientId == "" { - oAuthProviderClientId = oAuthProvider.ClientID - } - provider, err = oidc.NewProvider(ctx, "https://appleid.apple.com") - case "azure": - oAuthProvider = config.External.Azure - oAuthProviderClientId = oAuthProvider.ClientID - url := oAuthProvider.URL - if url == "" { - url = "https://login.microsoftonline.com/common" - } - provider, err = oidc.NewProvider(ctx, url+"/v2.0") - case "facebook": - oAuthProvider = config.External.Facebook - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, "https://www.facebook.com") - case "google": - oAuthProvider = config.External.Google - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, "https://accounts.google.com") - case "keycloak": - oAuthProvider = config.External.Keycloak - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, oAuthProvider.URL) - default: - return nil, fmt.Errorf("Provider %s doesn't support the id_token grant flow", p.Provider) - } - - if err != nil { - return nil, err - } - - if !oAuthProvider.Enabled { - return nil, badRequestError("Provider is not enabled") - } - - return provider.Verifier(&oidc.Config{ClientID: oAuthProviderClientId}), nil -} - -func (p *IdTokenGrantParams) getVerifierFromClientIDandIssuer(ctx context.Context) (*oidc.IDTokenVerifier, error) { - var provider *oidc.Provider - var err error - provider, err = oidc.NewProvider(ctx, p.Issuer) - if err != nil { - return nil, fmt.Errorf("issuer %s doesn't support the id_token grant flow", p.Issuer) - } - return provider.Verifier(&oidc.Config{ClientID: p.ClientID}), nil -} - func getEmailVerified(v interface{}) bool { var emailVerified bool var err error @@ -382,199 +311,6 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h return sendJSON(w, http.StatusOK, newTokenResponse) } -// IdTokenGrant implements the id_token grant type flow -func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.Request) error { - db := a.db.WithContext(ctx) - config := a.config - log := observability.GetLogEntry(r) - - params := &IdTokenGrantParams{} - - body, err := getBodyBytes(r) - if err != nil { - return badRequestError("Could not read body").WithInternalError(err) - } - - if err := json.Unmarshal(body, params); err != nil { - return badRequestError("Could not read id token grant params: %v", err) - } - - if params.IdToken == "" { - return oauthError("invalid request", "id_token required") - } - - if params.Provider == "" && (params.ClientID == "" || params.Issuer == "") { - return oauthError("invalid request", "provider or client_id and issuer required") - } - - var verifier *oidc.IDTokenVerifier - if params.Provider != "" { - verifier, err = params.getVerifier(ctx, a.config) - } else if params.ClientID != "" && params.Issuer != "" { - log.WithField("issuer", params.Issuer).WithField("client_id", params.ClientID).Warn("Use of POST /token with issuer and client_id is deprecated for security reasons. Please switch to using the API with provider only!") - - for _, issuer := range a.config.External.AllowedIdTokenIssuers { - if params.Issuer == issuer { - verifier, err = params.getVerifierFromClientIDandIssuer(ctx) - break - } - } - if err != nil { - return err - } - if verifier == nil { - return badRequestError("Issuer not allowed") - } - } else { - return badRequestError("%v", err) - } - if err != nil { - return err - } - - idToken, err := verifier.Verify(ctx, params.IdToken) - if err != nil { - return badRequestError("%v", err) - } - - claims := make(map[string]interface{}) - if err := idToken.Claims(&claims); err != nil { - return err - } - - hashedNonce, ok := claims["nonce"] - if (!ok && params.Nonce != "") || (ok && params.Nonce == "") { - return oauthError("invalid request", "Passed nonce and nonce in id_token should either both exist or not.") - } - - if ok && params.Nonce != "" { - // verify nonce to mitigate replay attacks - hash := fmt.Sprintf("%x", sha256.Sum256([]byte(params.Nonce))) - if hash != hashedNonce.(string) { - return oauthError("invalid nonce", "").WithInternalMessage("Possible abuse attempt: %v", r) - } - } - - sub, ok := claims["sub"].(string) - if !ok { - return oauthError("invalid request", "missing sub claim in id_token") - } - - email, ok := claims["email"].(string) - if !ok { - email = "" - } - - var user *models.User - var grantParams models.GrantParams - var token *AccessTokenResponse - err = db.Transaction(func(tx *storage.Connection) error { - var terr error - var identity *models.Identity - - if identity, terr = models.FindIdentityByIdAndProvider(tx, sub, params.Provider); terr != nil { - // create new identity & user if identity is not found - if models.IsNotFoundError(terr) { - if config.DisableSignup { - return forbiddenError("Signups not allowed for this instance") - } - aud := a.requestAud(ctx, r) - signupParams := &SignupParams{ - Provider: params.Provider, - Email: email, - Aud: aud, - Data: claims, - } - - user, terr = a.signupNewUser(ctx, tx, signupParams, false /* <- isSSOUser */) - if terr != nil { - return terr - } - if _, terr = a.createNewIdentity(tx, user, params.Provider, claims); terr != nil { - return terr - } - } else { - return terr - } - } else { - user, terr = models.FindUserByID(tx, identity.UserID) - if terr != nil { - return terr - } - if email != "" { - identity.IdentityData["email"] = email - } - if user.IsBanned() { - return oauthError("invalid_grant", "invalid id token grant") - } - if terr = tx.UpdateOnly(identity, "identity_data", "last_sign_in_at"); terr != nil { - return terr - } - if terr = user.UpdateAppMetaDataProviders(tx); terr != nil { - return terr - } - } - - if !user.IsConfirmed() { - isEmailVerified := false - emailVerified, ok := claims["email_verified"] - if ok { - isEmailVerified = getEmailVerified(emailVerified) - } - if (!ok || !isEmailVerified) && !config.Mailer.Autoconfirm { - - mailer := a.Mailer(ctx) - referrer := a.getReferrer(r) - externalURL := getExternalHost(ctx) - if terr = sendConfirmation(tx, user, mailer, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow); terr != nil { - return internalServerError("Error sending confirmation mail").WithInternalError(terr) - } - return unauthorizedError("Error unverified email") - } - - if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ - "provider": params.Provider, - }); terr != nil { - return terr - } - - if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { - return terr - } - - if terr = user.Confirm(tx); terr != nil { - return internalServerError("Error updating user").WithInternalError(terr) - } - } else { - if terr := models.NewAuditLogEntry(r, tx, user, models.LoginAction, "", map[string]interface{}{ - "provider": params.Provider, - }); terr != nil { - return terr - } - if terr = triggerEventHooks(ctx, tx, LoginEvent, user, config); terr != nil { - return terr - } - } - token, terr = a.issueRefreshToken(ctx, tx, user, models.OAuth, grantParams) - - if terr != nil { - return oauthError("server_error", terr.Error()) - } - return nil - }) - - if err != nil { - return err - } - - if err := a.setCookieTokens(config, token, false, w); err != nil { - return internalServerError("Failed to set JWT cookie. %s", err) - } - - metering.RecordLogin("id_token", user.ID) - return sendJSON(w, http.StatusOK, token) -} - func (a *API) PKCE(ctx context.Context, w http.ResponseWriter, r *http.Request) error { db := a.db.WithContext(ctx) var grantParams models.GrantParams diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go new file mode 100644 index 000000000..a709468e4 --- /dev/null +++ b/internal/api/token_oidc.go @@ -0,0 +1,276 @@ +package api + +import ( + "context" + "crypto/sha256" + "encoding/json" + "fmt" + "net/http" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/supabase/gotrue/internal/conf" + "github.com/supabase/gotrue/internal/metering" + "github.com/supabase/gotrue/internal/models" + "github.com/supabase/gotrue/internal/observability" + "github.com/supabase/gotrue/internal/storage" +) + +// IdTokenGrantParams are the parameters the IdTokenGrant method accepts +type IdTokenGrantParams struct { + IdToken string `json:"id_token"` + Nonce string `json:"nonce"` + Provider string `json:"provider"` + ClientID string `json:"client_id"` + Issuer string `json:"issuer"` +} + +func (p *IdTokenGrantParams) getVerifier(ctx context.Context, config *conf.GlobalConfiguration) (*oidc.IDTokenVerifier, error) { + var provider *oidc.Provider + var err error + var oAuthProvider conf.OAuthProviderConfiguration + var oAuthProviderClientId string + switch p.Provider { + case "apple": + oAuthProvider = config.External.Apple + oAuthProviderClientId = config.External.IosBundleId + if oAuthProviderClientId == "" { + oAuthProviderClientId = oAuthProvider.ClientID + } + provider, err = oidc.NewProvider(ctx, "https://appleid.apple.com") + case "azure": + oAuthProvider = config.External.Azure + oAuthProviderClientId = oAuthProvider.ClientID + url := oAuthProvider.URL + if url == "" { + url = "https://login.microsoftonline.com/common" + } + provider, err = oidc.NewProvider(ctx, url+"/v2.0") + case "facebook": + oAuthProvider = config.External.Facebook + oAuthProviderClientId = oAuthProvider.ClientID + provider, err = oidc.NewProvider(ctx, "https://www.facebook.com") + case "google": + oAuthProvider = config.External.Google + oAuthProviderClientId = oAuthProvider.ClientID + provider, err = oidc.NewProvider(ctx, "https://accounts.google.com") + case "keycloak": + oAuthProvider = config.External.Keycloak + oAuthProviderClientId = oAuthProvider.ClientID + provider, err = oidc.NewProvider(ctx, oAuthProvider.URL) + default: + return nil, fmt.Errorf("Provider %s doesn't support the id_token grant flow", p.Provider) + } + + if err != nil { + return nil, err + } + + if !oAuthProvider.Enabled { + return nil, badRequestError("Provider is not enabled") + } + + return provider.Verifier(&oidc.Config{ClientID: oAuthProviderClientId}), nil +} + +func (p *IdTokenGrantParams) getVerifierFromClientIDandIssuer(ctx context.Context) (*oidc.IDTokenVerifier, error) { + var provider *oidc.Provider + var err error + provider, err = oidc.NewProvider(ctx, p.Issuer) + if err != nil { + return nil, fmt.Errorf("issuer %s doesn't support the id_token grant flow", p.Issuer) + } + return provider.Verifier(&oidc.Config{ClientID: p.ClientID}), nil +} + +// IdTokenGrant implements the id_token grant type flow +func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.Request) error { + db := a.db.WithContext(ctx) + config := a.config + log := observability.GetLogEntry(r) + + params := &IdTokenGrantParams{} + + body, err := getBodyBytes(r) + if err != nil { + return badRequestError("Could not read body").WithInternalError(err) + } + + if err := json.Unmarshal(body, params); err != nil { + return badRequestError("Could not read id token grant params: %v", err) + } + + if params.IdToken == "" { + return oauthError("invalid request", "id_token required") + } + + if params.Provider == "" && (params.ClientID == "" || params.Issuer == "") { + return oauthError("invalid request", "provider or client_id and issuer required") + } + + var verifier *oidc.IDTokenVerifier + if params.Provider != "" { + verifier, err = params.getVerifier(ctx, a.config) + } else if params.ClientID != "" && params.Issuer != "" { + log.WithField("issuer", params.Issuer).WithField("client_id", params.ClientID).Warn("Use of POST /token with issuer and client_id is deprecated for security reasons. Please switch to using the API with provider only!") + + for _, issuer := range a.config.External.AllowedIdTokenIssuers { + if params.Issuer == issuer { + verifier, err = params.getVerifierFromClientIDandIssuer(ctx) + break + } + } + if err != nil { + return err + } + if verifier == nil { + return badRequestError("Issuer not allowed") + } + } else { + return badRequestError("%v", err) + } + if err != nil { + return err + } + + idToken, err := verifier.Verify(ctx, params.IdToken) + if err != nil { + return badRequestError("%v", err) + } + + claims := make(map[string]interface{}) + if err := idToken.Claims(&claims); err != nil { + return err + } + + hashedNonce, ok := claims["nonce"] + if (!ok && params.Nonce != "") || (ok && params.Nonce == "") { + return oauthError("invalid request", "Passed nonce and nonce in id_token should either both exist or not.") + } + + if ok && params.Nonce != "" { + // verify nonce to mitigate replay attacks + hash := fmt.Sprintf("%x", sha256.Sum256([]byte(params.Nonce))) + if hash != hashedNonce.(string) { + return oauthError("invalid nonce", "").WithInternalMessage("Possible abuse attempt: %v", r) + } + } + + sub, ok := claims["sub"].(string) + if !ok { + return oauthError("invalid request", "missing sub claim in id_token") + } + + email, ok := claims["email"].(string) + if !ok { + email = "" + } + + var user *models.User + var grantParams models.GrantParams + var token *AccessTokenResponse + err = db.Transaction(func(tx *storage.Connection) error { + var terr error + var identity *models.Identity + + if identity, terr = models.FindIdentityByIdAndProvider(tx, sub, params.Provider); terr != nil { + // create new identity & user if identity is not found + if models.IsNotFoundError(terr) { + if config.DisableSignup { + return forbiddenError("Signups not allowed for this instance") + } + aud := a.requestAud(ctx, r) + signupParams := &SignupParams{ + Provider: params.Provider, + Email: email, + Aud: aud, + Data: claims, + } + + user, terr = a.signupNewUser(ctx, tx, signupParams, false /* <- isSSOUser */) + if terr != nil { + return terr + } + if _, terr = a.createNewIdentity(tx, user, params.Provider, claims); terr != nil { + return terr + } + } else { + return terr + } + } else { + user, terr = models.FindUserByID(tx, identity.UserID) + if terr != nil { + return terr + } + if email != "" { + identity.IdentityData["email"] = email + } + if user.IsBanned() { + return oauthError("invalid_grant", "invalid id token grant") + } + if terr = tx.UpdateOnly(identity, "identity_data", "last_sign_in_at"); terr != nil { + return terr + } + if terr = user.UpdateAppMetaDataProviders(tx); terr != nil { + return terr + } + } + + if !user.IsConfirmed() { + isEmailVerified := false + emailVerified, ok := claims["email_verified"] + if ok { + isEmailVerified = getEmailVerified(emailVerified) + } + if (!ok || !isEmailVerified) && !config.Mailer.Autoconfirm { + + mailer := a.Mailer(ctx) + referrer := a.getReferrer(r) + externalURL := getExternalHost(ctx) + if terr = sendConfirmation(tx, user, mailer, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow); terr != nil { + return internalServerError("Error sending confirmation mail").WithInternalError(terr) + } + return unauthorizedError("Error unverified email") + } + + if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ + "provider": params.Provider, + }); terr != nil { + return terr + } + + if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { + return terr + } + + if terr = user.Confirm(tx); terr != nil { + return internalServerError("Error updating user").WithInternalError(terr) + } + } else { + if terr := models.NewAuditLogEntry(r, tx, user, models.LoginAction, "", map[string]interface{}{ + "provider": params.Provider, + }); terr != nil { + return terr + } + if terr = triggerEventHooks(ctx, tx, LoginEvent, user, config); terr != nil { + return terr + } + } + token, terr = a.issueRefreshToken(ctx, tx, user, models.OAuth, grantParams) + + if terr != nil { + return oauthError("server_error", terr.Error()) + } + return nil + }) + + if err != nil { + return err + } + + if err := a.setCookieTokens(config, token, false, w); err != nil { + return internalServerError("Failed to set JWT cookie. %s", err) + } + + metering.RecordLogin("id_token", user.ID) + return sendJSON(w, http.StatusOK, token) +} From 55b8439fc8bed1fea3202d30fccbf7b47a1e5681 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Wed, 17 May 2023 12:58:43 +0200 Subject: [PATCH 3/5] feat: align OAuth and OIDC flows --- internal/api/external.go | 2 +- internal/api/provider/apple.go | 132 +++--------- internal/api/provider/azure.go | 2 + internal/api/provider/facebook.go | 2 + internal/api/provider/google.go | 78 +------ internal/api/provider/oidc.go | 168 +++++++++++++++ internal/api/provider/oidc_test.go | 49 +++++ internal/api/token.go | 17 -- internal/api/token_oidc.go | 316 ++++++++++++----------------- openapi.yaml | 3 + 10 files changed, 389 insertions(+), 380 deletions(-) create mode 100644 internal/api/provider/oidc.go create mode 100644 internal/api/provider/oidc_test.go diff --git a/internal/api/external.go b/internal/api/external.go index da9f7c295..a0d36639c 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -514,7 +514,7 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide switch name { case "apple": - return provider.NewAppleProvider(config.External.Apple) + return provider.NewAppleProvider(ctx, config.External.Apple) case "azure": return provider.NewAzureProvider(config.External.Azure, scopes) case "bitbucket": diff --git a/internal/api/provider/apple.go b/internal/api/provider/apple.go index f5242b27d..a2cb8825d 100644 --- a/internal/api/provider/apple.go +++ b/internal/api/provider/apple.go @@ -2,37 +2,23 @@ package provider import ( "context" - "crypto/rsa" - "crypto/sha256" - "encoding/base64" "encoding/json" - "fmt" - "net/http" "net/url" "strings" - "github.com/golang-jwt/jwt" - "github.com/lestrrat-go/jwx/jwk" + "github.com/coreos/go-oidc/v3/oidc" + "github.com/sirupsen/logrus" "github.com/supabase/gotrue/internal/conf" "golang.org/x/oauth2" ) -const ( - defaultAppleAPIBase = "appleid.apple.com" - authEndpoint = "/auth/authorize" - tokenEndpoint = "/auth/token" //#nosec G101 -- Not a secret value. - - scopeEmail = "email" - scopeName = "name" - - appleAudOrIss = "https://appleid.apple.com" - idTokenVerificationKeyEndpoint = "/auth/keys" //#nosec G101 -- Not a secret value. -) +const IssuerApple = "https://appleid.apple.com" // AppleProvider stores the custom config for apple provider type AppleProvider struct { *oauth2.Config - UserInfoURL string + + oidc *oidc.Provider } type appleName struct { @@ -45,38 +31,33 @@ type appleUser struct { Email string `json:"email"` } -type idTokenClaims struct { - jwt.StandardClaims - AccessTokenHash string `json:"at_hash"` - AuthTime int `json:"auth_time"` - Email string `json:"email"` - IsPrivateEmail bool `json:"is_private_email,string"` - Sub string `json:"sub"` -} - // NewAppleProvider creates a Apple account provider. -func NewAppleProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { +func NewAppleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { if err := ext.Validate(); err != nil { return nil, err } - authHost := chooseHost(ext.URL, defaultAppleAPIBase) + if ext.URL != "" { + logrus.Warn("Apple OAuth provider has URL config set which is ignored (check GOTRUE_EXTERNAL_APPLE_URL)") + } + + oidcProvider, err := oidc.NewProvider(ctx, IssuerApple) + if err != nil { + return nil, err + } return &AppleProvider{ Config: &oauth2.Config{ ClientID: ext.ClientID, ClientSecret: ext.Secret, - Endpoint: oauth2.Endpoint{ - AuthURL: authHost + authEndpoint, - TokenURL: authHost + tokenEndpoint, - }, + Endpoint: oidcProvider.Endpoint(), Scopes: []string{ - scopeEmail, - scopeName, + "email", + "name", }, RedirectURL: ext.RedirectURI, }, - UserInfoURL: authHost + idTokenVerificationKeyEndpoint, + oidc: oidcProvider, }, nil } @@ -104,73 +85,22 @@ func (p AppleProvider) AuthCodeURL(state string, args ...oauth2.AuthCodeOption) // GetUserData returns the user data fetched from the apple provider func (p AppleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { - var user *UserProvidedData - if tok.AccessToken == "" { + idToken := tok.Extra("id_token") + if tok.AccessToken == "" || idToken == nil { + // Apple returns user data only the first time return &UserProvidedData{}, nil } - if idToken := tok.Extra("id_token"); idToken != nil { - idToken, err := jwt.ParseWithClaims(idToken.(string), &idTokenClaims{}, func(t *jwt.Token) (interface{}, error) { - kid := t.Header["kid"].(string) - claims := t.Claims.(*idTokenClaims) - vErr := new(jwt.ValidationError) - if !claims.VerifyAudience(p.ClientID, true) { - vErr.Inner = fmt.Errorf("incorrect audience") - vErr.Errors |= jwt.ValidationErrorAudience - } - if !claims.VerifyIssuer(appleAudOrIss, true) { - vErr.Inner = fmt.Errorf("incorrect issuer") - vErr.Errors |= jwt.ValidationErrorIssuer - } - if vErr.Errors > 0 { - return nil, vErr - } - - // per OpenID Connect Core 1.0 ยง3.2.2.9, Access Token Validation - hash := sha256.Sum256([]byte(tok.AccessToken)) - halfHash := hash[0:(len(hash) / 2)] - encodedHalfHash := base64.RawURLEncoding.EncodeToString(halfHash) - if encodedHalfHash != claims.AccessTokenHash { - vErr.Inner = fmt.Errorf(`invalid identity token`) - vErr.Errors |= jwt.ValidationErrorClaimsInvalid - return nil, vErr - } - - // get the public key for verifying the identity token signature - set, err := jwk.Fetch(ctx, p.UserInfoURL, jwk.WithHTTPClient(http.DefaultClient)) - if err != nil { - return nil, err - } - selectedKey, ok := set.LookupKeyID(kid) - if !ok { - return nil, fmt.Errorf("unable to lookup Apple ID key with kid = %q", kid) - } - var pubKey rsa.PublicKey - if err := selectedKey.Raw(&pubKey); err != nil { - return nil, fmt.Errorf("expected RSA public key from %q with kid %q", p.UserInfoURL, kid) - } - return &pubKey, nil - }) - if err != nil { - return &UserProvidedData{}, err - } - user = &UserProvidedData{ - Emails: []Email{{ - Email: idToken.Claims.(*idTokenClaims).Email, - Verified: true, - Primary: true, - }}, - Metadata: &Claims{ - Issuer: p.UserInfoURL, - Subject: idToken.Claims.(*idTokenClaims).Sub, - Email: idToken.Claims.(*idTokenClaims).Email, - EmailVerified: true, - - // To be deprecated - ProviderId: idToken.Claims.(*idTokenClaims).Sub, - }, - } + + _, data, err := ParseIDToken(ctx, p.oidc, &oidc.Config{ + ClientID: p.ClientID, + }, idToken.(string), ParseIDTokenOptions{ + AccessToken: tok.AccessToken, + }) + if err != nil { + return nil, err } - return user, nil + + return data, nil } // ParseUser parses the apple user's info diff --git a/internal/api/provider/azure.go b/internal/api/provider/azure.go index 3daff33b8..cbfc82066 100644 --- a/internal/api/provider/azure.go +++ b/internal/api/provider/azure.go @@ -9,6 +9,8 @@ import ( "golang.org/x/oauth2" ) +const IssuerAzure = "https://login.microsoftonline.com/common/v2.0" + const ( defaultAzureAuthBase = "login.microsoftonline.com/common" defaultAzureAPIBase = "graph.microsoft.com" diff --git a/internal/api/provider/facebook.go b/internal/api/provider/facebook.go index 6c21a93d8..a511ad0d4 100644 --- a/internal/api/provider/facebook.go +++ b/internal/api/provider/facebook.go @@ -12,6 +12,8 @@ import ( "golang.org/x/oauth2" ) +const IssuerFacebook = "https://www.facebook.com" + const ( defaultFacebookAuthBase = "www.facebook.com" defaultFacebookTokenBase = "graph.facebook.com" //#nosec G101 -- Not a secret value. diff --git a/internal/api/provider/google.go b/internal/api/provider/google.go index 317dd1525..48a84ab2d 100644 --- a/internal/api/provider/google.go +++ b/internal/api/provider/google.go @@ -28,80 +28,12 @@ func (u googleUser) IsEmailVerified() bool { return u.VerifiedEmail || u.EmailVerified } -type GoogleOIDCProvider struct { - *oidc.Provider -} - -// ParseIDToken parses a Google issued OIDC ID token. You should verify the aud -// claim on your own! -func (p *GoogleOIDCProvider) ParseIDToken(ctx context.Context, idToken string) (*oidc.IDToken, *UserProvidedData, error) { - verifier := p.Verifier(&oidc.Config{ - // aud claim check to be performed by other flows - SkipClientIDCheck: true, - }) - - token, err := verifier.Verify(ctx, idToken) - if err != nil { - return nil, nil, err - } - - var claims googleUser - if err := token.Claims(&claims); err != nil { - return nil, nil, err - } - - var data UserProvidedData - - if claims.Email != "" { - data.Emails = append(data.Emails, Email{ - Email: claims.Email, - Verified: claims.IsEmailVerified(), - Primary: true, - }) - } - - if len(data.Emails) <= 0 { - return nil, nil, errors.New("provider: Google ID token must contain an email address") - } - - data.Metadata = &Claims{ - Issuer: claims.Issuer, - Subject: claims.Subject, - Name: claims.Name, - Picture: claims.AvatarURL, - Email: claims.Email, - EmailVerified: claims.IsEmailVerified(), - - // To be deprecated - AvatarURL: claims.AvatarURL, - FullName: claims.Name, - ProviderId: claims.Subject, - } - - if claims.HostedDomain != "" { - data.Metadata.CustomClaims = map[string]any{ - "hd": claims.HostedDomain, - } - } - - return token, &data, nil -} - -// NewGoogleOIDCProvider creates a new ODIC provider with -// https://accounts.google.com as the issuer. -func NewGoogleOIDCProvider(ctx context.Context) (*GoogleOIDCProvider, error) { - provider, err := oidc.NewProvider(ctx, "https://accounts.google.com") - if err != nil { - return nil, err - } - - return &GoogleOIDCProvider{provider}, nil -} +const IssuerGoogle = "https://accounts.google.com" type googleProvider struct { *oauth2.Config - oidc *GoogleOIDCProvider + oidc *oidc.Provider } // NewGoogleProvider creates a Google OAuth2 identity provider. @@ -123,7 +55,7 @@ func NewGoogleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration, oauthScopes = append(oauthScopes, strings.Split(scopes, ",")...) } - oidcProvider, err := NewGoogleOIDCProvider(ctx) + oidcProvider, err := oidc.NewProvider(ctx, IssuerGoogle) if err != nil { return nil, err } @@ -148,7 +80,9 @@ const oauthGoogleUserInfoEndpoint = "https://www.googleapis.com/userinfo/v2/me" func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { if idToken := tok.Extra("id_token"); idToken != nil { - token, data, err := g.oidc.ParseIDToken(ctx, idToken.(string)) + token, data, err := ParseIDToken(ctx, g.oidc, nil, idToken.(string), ParseIDTokenOptions{ + AccessToken: tok.AccessToken, + }) if err != nil { return nil, err } diff --git a/internal/api/provider/oidc.go b/internal/api/provider/oidc.go new file mode 100644 index 000000000..750f04cc7 --- /dev/null +++ b/internal/api/provider/oidc.go @@ -0,0 +1,168 @@ +package provider + +import ( + "context" + "errors" + "fmt" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/golang-jwt/jwt" +) + +type ParseIDTokenOptions struct { + SkipAccessTokenCheck bool + AccessToken string +} + +func ParseIDToken(ctx context.Context, provider *oidc.Provider, config *oidc.Config, idToken string, options ParseIDTokenOptions) (*oidc.IDToken, *UserProvidedData, error) { + if config == nil { + config = &oidc.Config{ + // aud claim check to be performed by other flows + SkipClientIDCheck: true, + } + } + + verifier := provider.VerifierContext(ctx, config) + + token, err := verifier.Verify(ctx, idToken) + if err != nil { + return nil, nil, err + } + + var data *UserProvidedData + + switch token.Issuer { + case IssuerGoogle: + token, data, err = parseGoogleIDToken(token) + + case IssuerApple: + token, data, err = parseAppleIDToken(token) + + default: + token, data, err = parseGenericIDToken(token) + } + + if err != nil { + return nil, nil, err + } + + if !options.SkipAccessTokenCheck && token.AccessTokenHash != "" { + if err := token.VerifyAccessToken(options.AccessToken); err != nil { + return nil, nil, err + } + } + + return token, data, nil +} + +func parseGoogleIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { + var claims googleUser + if err := token.Claims(&claims); err != nil { + return nil, nil, err + } + + var data UserProvidedData + + if claims.Email != "" { + data.Emails = append(data.Emails, Email{ + Email: claims.Email, + Verified: claims.IsEmailVerified(), + Primary: true, + }) + } + + if len(data.Emails) <= 0 { + return nil, nil, errors.New("provider: Google ID token must contain an email address") + } + + data.Metadata = &Claims{ + Issuer: claims.Issuer, + Subject: claims.Subject, + Name: claims.Name, + Picture: claims.AvatarURL, + Email: claims.Email, + EmailVerified: claims.IsEmailVerified(), + + // To be deprecated + AvatarURL: claims.AvatarURL, + FullName: claims.Name, + ProviderId: claims.Subject, + } + + if claims.HostedDomain != "" { + data.Metadata.CustomClaims = map[string]any{ + "hd": claims.HostedDomain, + } + } + + return token, &data, nil +} + +type AppleIDTokenClaims struct { + jwt.StandardClaims + + Email string `json:"email"` + + AuthTime *float64 `json:"auth_time"` + IsPrivateEmail *bool `json:"is_private_email,string"` +} + +func parseAppleIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { + var claims AppleIDTokenClaims + if err := token.Claims(&claims); err != nil { + return nil, nil, err + } + + var data UserProvidedData + + data.Emails = append(data.Emails, Email{ + Email: claims.Email, + Verified: true, + Primary: true, + }) + + data.Metadata = &Claims{ + Issuer: token.Issuer, + Subject: token.Subject, + Email: claims.Email, + EmailVerified: true, + ProviderId: token.Subject, + CustomClaims: make(map[string]any), + } + + if claims.IsPrivateEmail != nil { + data.Metadata.CustomClaims["is_private_email"] = *claims.IsPrivateEmail + } + + if claims.AuthTime != nil { + data.Metadata.CustomClaims["auth_time"] = *claims.AuthTime + } + + if len(data.Metadata.CustomClaims) < 1 { + data.Metadata.CustomClaims = nil + } + + return token, &data, nil +} + +func parseGenericIDToken(token *oidc.IDToken) (*oidc.IDToken, *UserProvidedData, error) { + var data UserProvidedData + + if err := token.Claims(&data.Metadata); err != nil { + return nil, nil, err + } + + if data.Metadata.Email != "" { + data.Emails = append(data.Emails, Email{ + Email: data.Metadata.Email, + Verified: data.Metadata.EmailVerified, + Primary: true, + }) + } + + 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/oidc_test.go b/internal/api/provider/oidc_test.go new file mode 100644 index 000000000..d43e57887 --- /dev/null +++ b/internal/api/provider/oidc_test.go @@ -0,0 +1,49 @@ +package provider + +import ( + "context" + "testing" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/stretchr/testify/require" +) + +type realIDToken struct { + AccessToken string + IDToken string + Time time.Time + Email string +} + +var realIDTokens map[string]realIDToken = map[string]realIDToken{ + IssuerGoogle: realIDToken{ + AccessToken: "ya29.a0AWY7CkkHwdKOkkLSsAqEe8aGuw-_1RP-PTTUKO3WeX0cdkSY86h4W-xkajQgd6rXFjVHl44R69kDdFt0QZIQgdubGbwVNk5URkxegz9TkC1Tw055edvob7Y2dLo3VAzccs4CTTwT1qSnr1u1BIjheSEUbQguaCgYKASsSARESFQG1tDrpoU1f2gq-2cSRA0xjJf1sxA0163", + IDToken: "eyJhbGciOiJSUzI1NiIsImtpZCI6IjgyMjgzOGMxYzhiZjllZGNmMWY1MDUwNjYyZTU0YmNiMWFkYjViNWYiLCJ0eXAiOiJKV1QifQ.eyJpc3MiOiJodHRwczovL2FjY291bnRzLmdvb2dsZS5jb20iLCJhenAiOiI5MTQ2NjY0MjA3NS03OWNwaWs4aWNxYzU4NjY5bjdtaXY5NjZsYmFwOTNhMi5hcHBzLmdvb2dsZXVzZXJjb250ZW50LmNvbSIsImF1ZCI6IjkxNDY2NjQyMDc1LTc5Y3BpazhpY3FjNTg2NjluN21pdjk2NmxiYXA5M2EyLmFwcHMuZ29vZ2xldXNlcmNvbnRlbnQuY29tIiwic3ViIjoiMTAzNzgzMTkwMTI2NDM5NzUxMjY5IiwiaGQiOiJzdXBhYmFzZS5pbyIsImVtYWlsIjoic3RvamFuQHN1cGFiYXNlLmlvIiwiZW1haWxfdmVyaWZpZWQiOnRydWUsImF0X2hhc2giOiJGYU1La0Q5YlhJd2ZyY1JJWnVXZUV3IiwibmFtZSI6IlN0b2phbiBEaW1pdHJvdnNraSIsInBpY3R1cmUiOiJodHRwczovL2xoMy5nb29nbGV1c2VyY29udGVudC5jb20vYS9BR05teXhhUWNrdUxnOXZxNGZyOF9KMkllc0daRl93TVNvQks3WEN2cXNRYz1zOTYtYyIsImdpdmVuX25hbWUiOiJTdG9qYW4iLCJmYW1pbHlfbmFtZSI6IkRpbWl0cm92c2tpIiwibG9jYWxlIjoiZW4tR0IiLCJpYXQiOjE2ODQ1Nzg1MzEsImV4cCI6MTY4NDU4MjEzMX0.Rwa8ebG0rUNowXsDLshqCEAEjkfxzurrhsEVm4DuJ9ncxBMyijw1-pwjLyqREaDnZbr8GUn8Nlft2gzw7ImgR5750sxOFwDIKEOBFfYIGq3-1tJMvVLG3G9zIPkm7mOrPvAAc5nM8JB15hB4ep7Bt_YcTSPXebewFJo5oBC9XQ_WsnIsvvpwdIdiSIhYmSfuWK-IjfsUIsysuM93mUcDhu_jzMJfeCqda4CQbNRE_WzcHS4B12bLmfT1Ho4ZSl0M4dKkMH_lUbIhi6kgu8xsW8lPMYrsvzvtOJWwK4tF3gL1lD_5JOs8eTnemn956yiPfL3dfMj6Kp6w9yMndgVbOQ", + Time: time.Unix(1684578532, 0), // 1 sec after iat + }, + //IssuerApple: realIDToken{}, +} + +func TestParseIDToken(t *testing.T) { + // note that this test can fail if/when the issuers rotate their + // signing keys (which happens rarely if ever) + // then you should obtain new ID tokens and update this test + for issuer, token := range realIDTokens { + oidcProvider, err := oidc.NewProvider(context.Background(), issuer) + require.NoError(t, err) + + _, user, err := ParseIDToken(context.Background(), oidcProvider, &oidc.Config{ + SkipClientIDCheck: true, + Now: func() time.Time { + return token.Time + }, + }, token.IDToken, ParseIDTokenOptions{ + AccessToken: token.AccessToken, + }) + require.NoError(t, err) + + require.NotEmpty(t, user.Emails[0].Email) + require.Equal(t, user.Emails[0].Verified, true) + } +} diff --git a/internal/api/token.go b/internal/api/token.go index 6d73dd48f..1e8e2accf 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -74,23 +74,6 @@ type PKCEGrantParams struct { const useCookieHeader = "x-use-cookie" const InvalidLoginMessage = "Invalid login credentials" -func getEmailVerified(v interface{}) bool { - var emailVerified bool - var err error - switch v := v.(type) { - case string: - emailVerified, err = strconv.ParseBool(v) - case bool: - emailVerified = v - default: - emailVerified = false - } - if err != nil { - return false - } - return emailVerified -} - // Token is the endpoint for OAuth access token requests func (a *API) Token(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index a709468e4..c078c7177 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -4,12 +4,13 @@ import ( "context" "crypto/sha256" "encoding/json" + "errors" "fmt" "net/http" "github.com/coreos/go-oidc/v3/oidc" + "github.com/supabase/gotrue/internal/api/provider" "github.com/supabase/gotrue/internal/conf" - "github.com/supabase/gotrue/internal/metering" "github.com/supabase/gotrue/internal/models" "github.com/supabase/gotrue/internal/observability" "github.com/supabase/gotrue/internal/storage" @@ -17,76 +18,94 @@ import ( // IdTokenGrantParams are the parameters the IdTokenGrant method accepts type IdTokenGrantParams struct { - IdToken string `json:"id_token"` - Nonce string `json:"nonce"` - Provider string `json:"provider"` - ClientID string `json:"client_id"` - Issuer string `json:"issuer"` + IdToken string `json:"id_token"` + AccessToken string `json:"access_token"` + Nonce string `json:"nonce"` + Provider string `json:"provider"` + ClientID string `json:"client_id"` + Issuer string `json:"issuer"` } -func (p *IdTokenGrantParams) getVerifier(ctx context.Context, config *conf.GlobalConfiguration) (*oidc.IDTokenVerifier, error) { - var provider *oidc.Provider - var err error - var oAuthProvider conf.OAuthProviderConfiguration - var oAuthProviderClientId string - switch p.Provider { - case "apple": - oAuthProvider = config.External.Apple - oAuthProviderClientId = config.External.IosBundleId - if oAuthProviderClientId == "" { - oAuthProviderClientId = oAuthProvider.ClientID - } - provider, err = oidc.NewProvider(ctx, "https://appleid.apple.com") - case "azure": - oAuthProvider = config.External.Azure - oAuthProviderClientId = oAuthProvider.ClientID - url := oAuthProvider.URL - if url == "" { - url = "https://login.microsoftonline.com/common" +func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.GlobalConfiguration, r *http.Request) (*oidc.Provider, string, []string, error) { + log := observability.GetLogEntry(r) + + enabled := true + var issuer string + var providerType string + var acceptableClientIDs []string + + switch true { + case p.Provider == "apple" || p.Issuer == provider.IssuerApple: + enabled = config.External.Apple.Enabled + providerType = "apple" + issuer = provider.IssuerApple + acceptableClientIDs = []string{config.External.Apple.ClientID} + + if config.External.IosBundleId != "" { + acceptableClientIDs = append(acceptableClientIDs, config.External.IosBundleId) } - provider, err = oidc.NewProvider(ctx, url+"/v2.0") - case "facebook": - oAuthProvider = config.External.Facebook - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, "https://www.facebook.com") - case "google": - oAuthProvider = config.External.Google - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, "https://accounts.google.com") - case "keycloak": - oAuthProvider = config.External.Keycloak - oAuthProviderClientId = oAuthProvider.ClientID - provider, err = oidc.NewProvider(ctx, oAuthProvider.URL) + + case p.Provider == "google" || p.Issuer == provider.IssuerGoogle: + enabled = config.External.Google.Enabled + providerType = "google" + issuer = provider.IssuerGoogle + acceptableClientIDs = []string{config.External.Google.ClientID} + + case p.Provider == "azure" || p.Issuer == provider.IssuerAzure: + enabled = config.External.Azure.Enabled + providerType = "azure" + issuer = provider.IssuerAzure + acceptableClientIDs = []string{config.External.Azure.ClientID} + + case p.Provider == "facebook" || p.Issuer == provider.IssuerFacebook: + enabled = config.External.Facebook.Enabled + providerType = "facebook" + issuer = provider.IssuerFacebook + acceptableClientIDs = []string{config.External.Facebook.ClientID} + + case p.Provider == "keycloak" || (config.External.Keycloak.Enabled && config.External.Keycloak.URL != "" && p.Issuer == config.External.Keycloak.URL): + enabled = config.External.Keycloak.Enabled + providerType = "keycloak" + issuer = config.External.Keycloak.URL + acceptableClientIDs = []string{config.External.Keycloak.ClientID} + default: - return nil, fmt.Errorf("Provider %s doesn't support the id_token grant flow", p.Provider) - } + 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!") + + allowed := false + for _, allowedIssuer := range config.External.AllowedIdTokenIssuers { + if p.Issuer == allowedIssuer { + allowed = true + providerType = allowedIssuer + acceptableClientIDs = []string{p.ClientID} + issuer = allowedIssuer + break + } + } - if err != nil { - return nil, err + if !allowed { + return nil, "", nil, badRequestError(fmt.Sprintf("Custom OIDC provider %q not allowed", p.Issuer)) + } } - if !oAuthProvider.Enabled { - return nil, badRequestError("Provider is not enabled") + if !enabled { + return nil, "", nil, badRequestError(fmt.Sprintf("Provider (issuer %q) is not enabled", issuer)) } - return provider.Verifier(&oidc.Config{ClientID: oAuthProviderClientId}), nil -} - -func (p *IdTokenGrantParams) getVerifierFromClientIDandIssuer(ctx context.Context) (*oidc.IDTokenVerifier, error) { - var provider *oidc.Provider - var err error - provider, err = oidc.NewProvider(ctx, p.Issuer) + oidcProvider, err := oidc.NewProvider(ctx, issuer) if err != nil { - return nil, fmt.Errorf("issuer %s doesn't support the id_token grant flow", p.Issuer) + return nil, "", nil, err } - return provider.Verifier(&oidc.Config{ClientID: p.ClientID}), nil + + return oidcProvider, providerType, acceptableClientIDs, nil } // IdTokenGrant implements the id_token grant type flow func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.Request) error { + log := observability.GetLogEntry(r) + db := a.db.WithContext(ctx) config := a.config - log := observability.GetLogEntry(r) params := &IdTokenGrantParams{} @@ -107,170 +126,89 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R return oauthError("invalid request", "provider or client_id and issuer required") } - var verifier *oidc.IDTokenVerifier - if params.Provider != "" { - verifier, err = params.getVerifier(ctx, a.config) - } else if params.ClientID != "" && params.Issuer != "" { - log.WithField("issuer", params.Issuer).WithField("client_id", params.ClientID).Warn("Use of POST /token with issuer and client_id is deprecated for security reasons. Please switch to using the API with provider only!") - - for _, issuer := range a.config.External.AllowedIdTokenIssuers { - if params.Issuer == issuer { - verifier, err = params.getVerifierFromClientIDandIssuer(ctx) - break - } - } - if err != nil { - return err - } - if verifier == nil { - return badRequestError("Issuer not allowed") - } - } else { - return badRequestError("%v", err) - } + oidcProvider, providerType, acceptableClientIDs, err := params.getProvider(ctx, config, r) if err != nil { return err } - idToken, err := verifier.Verify(ctx, params.IdToken) + idToken, userData, err := provider.ParseIDToken(ctx, oidcProvider, nil, params.IdToken, provider.ParseIDTokenOptions{ + SkipAccessTokenCheck: params.AccessToken == "", + AccessToken: params.AccessToken, + }) if err != nil { - return badRequestError("%v", err) + return oauthError("invalid request", "Bad ID token").WithInternalError(err) } - claims := make(map[string]interface{}) - if err := idToken.Claims(&claims); err != nil { - return err + if idToken.Subject == "" { + return oauthError("invalid request", "Missing sub claim in id_token") } - hashedNonce, ok := claims["nonce"] - if (!ok && params.Nonce != "") || (ok && params.Nonce == "") { - return oauthError("invalid request", "Passed nonce and nonce in id_token should either both exist or not.") - } + correctAudience := false + for _, clientID := range acceptableClientIDs { + for _, aud := range idToken.Audience { + if aud == clientID { + correctAudience = true + break + } + } - if ok && params.Nonce != "" { - // verify nonce to mitigate replay attacks - hash := fmt.Sprintf("%x", sha256.Sum256([]byte(params.Nonce))) - if hash != hashedNonce.(string) { - return oauthError("invalid nonce", "").WithInternalMessage("Possible abuse attempt: %v", r) + if correctAudience { + break } } - sub, ok := claims["sub"].(string) - if !ok { - return oauthError("invalid request", "missing sub claim in id_token") + if !correctAudience { + return oauthError("invalid request", "Unacceptable audience in id_token") } - email, ok := claims["email"].(string) - if !ok { - email = "" + tokenHasNonce := idToken.Nonce != "" + paramsHasNonce := params.Nonce != "" + + if tokenHasNonce != paramsHasNonce { + return oauthError("invalid request", "Passed nonce and nonce in id_token should either both exist or not.") + } else if tokenHasNonce && paramsHasNonce { + // verify nonce to mitigate replay attacks + hash := fmt.Sprintf("%x", sha256.Sum256([]byte(params.Nonce))) + if hash != idToken.Nonce { + return oauthError("invalid nonce", "Nonces mismatch") + } } - var user *models.User - var grantParams models.GrantParams - var token *AccessTokenResponse - err = db.Transaction(func(tx *storage.Connection) error { - var terr error - var identity *models.Identity - - if identity, terr = models.FindIdentityByIdAndProvider(tx, sub, params.Provider); terr != nil { - // create new identity & user if identity is not found - if models.IsNotFoundError(terr) { - if config.DisableSignup { - return forbiddenError("Signups not allowed for this instance") - } - aud := a.requestAud(ctx, r) - signupParams := &SignupParams{ - Provider: params.Provider, - Email: email, - Aud: aud, - Data: claims, - } - - user, terr = a.signupNewUser(ctx, tx, signupParams, false /* <- isSSOUser */) - if terr != nil { - return terr - } - if _, terr = a.createNewIdentity(tx, user, params.Provider, claims); terr != nil { - return terr - } - } else { - return terr - } - } else { - user, terr = models.FindUserByID(tx, identity.UserID) - if terr != nil { - return terr - } - if email != "" { - identity.IdentityData["email"] = email - } - if user.IsBanned() { - return oauthError("invalid_grant", "invalid id token grant") - } - if terr = tx.UpdateOnly(identity, "identity_data", "last_sign_in_at"); terr != nil { - return terr - } - if terr = user.UpdateAppMetaDataProviders(tx); terr != nil { - return terr - } + if params.AccessToken == "" { + if idToken.AccessTokenHash != "" { + log.Warn("ID token has a at_hash claim, but no access_token parameter was provided. In future versions, access_token will be mandatory as it's security best practice.") } + } else { + if idToken.AccessTokenHash == "" { + log.Info("ID token does not have a at_hash claim, access_token parameter is unused.") + } + } - if !user.IsConfirmed() { - isEmailVerified := false - emailVerified, ok := claims["email_verified"] - if ok { - isEmailVerified = getEmailVerified(emailVerified) - } - if (!ok || !isEmailVerified) && !config.Mailer.Autoconfirm { - - mailer := a.Mailer(ctx) - referrer := a.getReferrer(r) - externalURL := getExternalHost(ctx) - if terr = sendConfirmation(tx, user, mailer, config.SMTP.MaxFrequency, referrer, externalURL, config.Mailer.OtpLength, models.ImplicitFlow); terr != nil { - return internalServerError("Error sending confirmation mail").WithInternalError(terr) - } - return unauthorizedError("Error unverified email") - } + var token *AccessTokenResponse + var grantParams models.GrantParams - if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{ - "provider": params.Provider, - }); terr != nil { - return terr - } + if err := db.Transaction(func(tx *storage.Connection) error { + var user *models.User + var terr error - if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil { - return terr + user, terr = a.createAccountFromExternalIdentity(tx, r, userData, providerType) + if terr != nil { + if errors.Is(terr, errReturnNil) { + return nil } - if terr = user.Confirm(tx); terr != nil { - return internalServerError("Error updating user").WithInternalError(terr) - } - } else { - if terr := models.NewAuditLogEntry(r, tx, user, models.LoginAction, "", map[string]interface{}{ - "provider": params.Provider, - }); terr != nil { - return terr - } - if terr = triggerEventHooks(ctx, tx, LoginEvent, user, config); terr != nil { - return terr - } + return terr } - token, terr = a.issueRefreshToken(ctx, tx, user, models.OAuth, grantParams) + token, terr = a.issueRefreshToken(ctx, tx, user, models.OAuth, grantParams) if terr != nil { - return oauthError("server_error", terr.Error()) + return terr } - return nil - }) - if err != nil { - return err - } - - if err := a.setCookieTokens(config, token, false, w); err != nil { - return internalServerError("Failed to set JWT cookie. %s", err) + return nil + }); err != nil { + return oauthError("server_error", "Internal Server Error").WithInternalError(err) } - metering.RecordLogin("id_token", user.ID) return sendJSON(w, http.StatusOK, token) } diff --git a/openapi.yaml b/openapi.yaml index d0360d05c..809b40fa9 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -109,6 +109,9 @@ paths: format: phone id_token: type: string + access_token: + type: string + description: Provide only when `grant_type` is `id_token` and the provided ID token requires the presence of an access token to be accepted (usually by having an `at_hash` claim). nonce: type: string provider: From 962d55865066328507520393b61b8d710be37707 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Tue, 23 May 2023 17:24:43 +0200 Subject: [PATCH 4/5] feat: update Google OAuth mock tests --- internal/api/external_google_test.go | 18 ++++++++++++++++-- internal/api/provider/google.go | 24 ++++++++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/internal/api/external_google_test.go b/internal/api/external_google_test.go index 8992d0a65..81d9579ea 100644 --- a/internal/api/external_google_test.go +++ b/internal/api/external_google_test.go @@ -1,12 +1,15 @@ package api import ( + "encoding/json" "fmt" "net/http" "net/http/httptest" "net/url" jwt "github.com/golang-jwt/jwt" + "github.com/stretchr/testify/require" + "github.com/supabase/gotrue/internal/api/provider" ) const ( @@ -16,6 +19,8 @@ const ( ) func (ts *ExternalTestSuite) TestSignupExternalGoogle() { + provider.ResetGoogleProvider() + req := httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=google", nil) w := httptest.NewRecorder() ts.API.handler.ServeHTTP(w, req) @@ -40,8 +45,17 @@ func (ts *ExternalTestSuite) TestSignupExternalGoogle() { } func GoogleTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int, code string, user string) *httptest.Server { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + provider.ResetGoogleProvider() + + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { + case "/.well-known/openid-configuration": + w.Header().Add("Content-Type", "application/json") + require.NoError(ts.T(), json.NewEncoder(w).Encode(map[string]any{ + "issuer": server.URL, + "token_endpoint": server.URL + "/o/oauth2/token", + })) case "/o/oauth2/token": *tokenCount++ ts.Equal(code, r.FormValue("code")) @@ -60,7 +74,7 @@ func GoogleTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *in } })) - ts.Config.External.Google.URL = server.URL + provider.OverrideGoogleProvider(server.URL, server.URL+"/userinfo/v2/me") return server } diff --git a/internal/api/provider/google.go b/internal/api/provider/google.go index 48a84ab2d..b9e7e9569 100644 --- a/internal/api/provider/google.go +++ b/internal/api/provider/google.go @@ -30,6 +30,8 @@ func (u googleUser) IsEmailVerified() bool { const IssuerGoogle = "https://accounts.google.com" +var internalIssuerGoogle = IssuerGoogle + type googleProvider struct { *oauth2.Config @@ -55,7 +57,7 @@ func NewGoogleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration, oauthScopes = append(oauthScopes, strings.Split(scopes, ",")...) } - oidcProvider, err := oidc.NewProvider(ctx, IssuerGoogle) + oidcProvider, err := oidc.NewProvider(ctx, internalIssuerGoogle) if err != nil { return nil, err } @@ -76,7 +78,9 @@ func (g googleProvider) GetOAuthToken(code string) (*oauth2.Token, error) { return g.Exchange(context.Background(), code) } -const oauthGoogleUserInfoEndpoint = "https://www.googleapis.com/userinfo/v2/me" +const UserInfoEndpointGoogle = "https://www.googleapis.com/userinfo/v2/me" + +var internalUserInfoEndpointGoogle = UserInfoEndpointGoogle func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { if idToken := tok.Extra("id_token"); idToken != nil { @@ -108,7 +112,7 @@ func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us logrus.Info("Using Google OAuth2 user info endpoint, an ID token was not returned by Google") var u googleUser - if err := makeRequest(ctx, tok, g.Config, oauthGoogleUserInfoEndpoint, &u); err != nil { + if err := makeRequest(ctx, tok, g.Config, internalUserInfoEndpointGoogle, &u); err != nil { return nil, err } @@ -127,7 +131,7 @@ func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us } data.Metadata = &Claims{ - Issuer: oauthGoogleUserInfoEndpoint, + Issuer: internalUserInfoEndpointGoogle, Subject: u.ID, Name: u.Name, Picture: u.AvatarURL, @@ -142,3 +146,15 @@ func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Us return &data, nil } + +// ResetGoogleProvider should only be used in tests! +func ResetGoogleProvider() { + internalIssuerGoogle = IssuerGoogle + internalUserInfoEndpointGoogle = UserInfoEndpointGoogle +} + +// OverrideGoogleProvider should only be used in tests! +func OverrideGoogleProvider(issuer, userInfo string) { + internalIssuerGoogle = issuer + internalUserInfoEndpointGoogle = userInfo +} From 38482db21f3d049d3f2054206dbf8109300d3031 Mon Sep 17 00:00:00 2001 From: Stojan Dimitrovski Date: Fri, 26 May 2023 09:13:27 +0200 Subject: [PATCH 5/5] feat: support multiple client IDs --- internal/api/external_apple_test.go | 2 +- internal/api/external_azure_test.go | 2 +- internal/api/external_bitbucket_test.go | 2 +- internal/api/external_discord_test.go | 2 +- internal/api/external_facebook_test.go | 2 +- internal/api/external_github_test.go | 2 +- internal/api/external_gitlab_test.go | 2 +- internal/api/external_google_test.go | 2 +- internal/api/external_kakao_test.go | 2 +- internal/api/external_keycloak_test.go | 2 +- internal/api/external_linkedin_test.go | 2 +- internal/api/external_notion_test.go | 2 +- internal/api/external_twitch_test.go | 2 +- internal/api/external_workos_test.go | 6 +++--- internal/api/external_zoom_test.go | 2 +- internal/api/provider/apple.go | 4 ++-- internal/api/provider/azure.go | 4 ++-- internal/api/provider/bitbucket.go | 4 ++-- internal/api/provider/discord.go | 4 ++-- internal/api/provider/facebook.go | 4 ++-- internal/api/provider/github.go | 4 ++-- internal/api/provider/gitlab.go | 4 ++-- internal/api/provider/google.go | 23 ++++++-------------- internal/api/provider/kakao.go | 4 ++-- internal/api/provider/keycloak.go | 4 ++-- internal/api/provider/linkedin.go | 4 ++-- internal/api/provider/notion.go | 4 ++-- internal/api/provider/slack.go | 4 ++-- internal/api/provider/spotify.go | 4 ++-- internal/api/provider/twitch.go | 4 ++-- internal/api/provider/twitter.go | 4 ++-- internal/api/provider/workos.go | 4 ++-- internal/api/provider/zoom.go | 4 ++-- internal/api/token_oidc.go | 28 ++++++++++++++----------- internal/conf/configuration.go | 20 +++++++++--------- 35 files changed, 83 insertions(+), 90 deletions(-) diff --git a/internal/api/external_apple_test.go b/internal/api/external_apple_test.go index 8b2e99a98..0413e2bda 100644 --- a/internal/api/external_apple_test.go +++ b/internal/api/external_apple_test.go @@ -17,7 +17,7 @@ func (ts *ExternalTestSuite) TestSignupExternalApple() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Apple.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Apple.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Apple.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("email name", q.Get("scope")) diff --git a/internal/api/external_azure_test.go b/internal/api/external_azure_test.go index 4e8c874bf..db8edc59e 100644 --- a/internal/api/external_azure_test.go +++ b/internal/api/external_azure_test.go @@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalAzure() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Azure.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Azure.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Azure.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("openid", q.Get("scope")) diff --git a/internal/api/external_bitbucket_test.go b/internal/api/external_bitbucket_test.go index ab48a1e1b..fad66456f 100644 --- a/internal/api/external_bitbucket_test.go +++ b/internal/api/external_bitbucket_test.go @@ -22,7 +22,7 @@ func (ts *ExternalTestSuite) TestSignupExternalBitbucket() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Bitbucket.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Bitbucket.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Bitbucket.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("account email", q.Get("scope")) diff --git a/internal/api/external_discord_test.go b/internal/api/external_discord_test.go index 916f8855e..eeae84e13 100644 --- a/internal/api/external_discord_test.go +++ b/internal/api/external_discord_test.go @@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalDiscord() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Discord.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Discord.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Discord.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("email identify", q.Get("scope")) diff --git a/internal/api/external_facebook_test.go b/internal/api/external_facebook_test.go index 253715438..c484218f8 100644 --- a/internal/api/external_facebook_test.go +++ b/internal/api/external_facebook_test.go @@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalFacebook() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Facebook.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Facebook.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Facebook.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("email", q.Get("scope")) diff --git a/internal/api/external_github_test.go b/internal/api/external_github_test.go index fb7c5125a..fecedc8eb 100644 --- a/internal/api/external_github_test.go +++ b/internal/api/external_github_test.go @@ -25,7 +25,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGithub() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Github.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Github.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Github.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("user:email", q.Get("scope")) diff --git a/internal/api/external_gitlab_test.go b/internal/api/external_gitlab_test.go index 8a8b0fbf0..1a32655cf 100644 --- a/internal/api/external_gitlab_test.go +++ b/internal/api/external_gitlab_test.go @@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGitlab() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Gitlab.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Gitlab.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Gitlab.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("read_user", q.Get("scope")) diff --git a/internal/api/external_google_test.go b/internal/api/external_google_test.go index 81d9579ea..378d69dcb 100644 --- a/internal/api/external_google_test.go +++ b/internal/api/external_google_test.go @@ -29,7 +29,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGoogle() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Google.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Google.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Google.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("email profile", q.Get("scope")) diff --git a/internal/api/external_kakao_test.go b/internal/api/external_kakao_test.go index 7cfb51b38..cb86a97f2 100644 --- a/internal/api/external_kakao_test.go +++ b/internal/api/external_kakao_test.go @@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalKakao() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Kakao.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Kakao.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Kakao.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) claims := ExternalProviderClaims{} diff --git a/internal/api/external_keycloak_test.go b/internal/api/external_keycloak_test.go index 81072049f..5cffc7df3 100644 --- a/internal/api/external_keycloak_test.go +++ b/internal/api/external_keycloak_test.go @@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalKeycloak() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Keycloak.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Keycloak.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Keycloak.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("profile email", q.Get("scope")) diff --git a/internal/api/external_linkedin_test.go b/internal/api/external_linkedin_test.go index cce8b9583..27449dd3a 100644 --- a/internal/api/external_linkedin_test.go +++ b/internal/api/external_linkedin_test.go @@ -25,7 +25,7 @@ func (ts *ExternalTestSuite) TestSignupExternalLinkedin() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Linkedin.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Linkedin.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Linkedin.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("r_emailaddress r_liteprofile", q.Get("scope")) diff --git a/internal/api/external_notion_test.go b/internal/api/external_notion_test.go index be82e09ed..cb25ad279 100644 --- a/internal/api/external_notion_test.go +++ b/internal/api/external_notion_test.go @@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalNotion() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Notion.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Notion.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Notion.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) claims := ExternalProviderClaims{} diff --git a/internal/api/external_twitch_test.go b/internal/api/external_twitch_test.go index a4e473cae..8c0b16ecf 100644 --- a/internal/api/external_twitch_test.go +++ b/internal/api/external_twitch_test.go @@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalTwitch() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Twitch.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Twitch.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Twitch.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("user:read:email", q.Get("scope")) diff --git a/internal/api/external_workos_test.go b/internal/api/external_workos_test.go index b2e1e599e..4ade89883 100644 --- a/internal/api/external_workos_test.go +++ b/internal/api/external_workos_test.go @@ -25,7 +25,7 @@ func (ts *ExternalTestSuite) TestSignupExternalWorkOSWithConnection() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.WorkOS.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.WorkOS.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.WorkOS.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("", q.Get("scope")) ts.Equal(connection, q.Get("connection")) @@ -51,7 +51,7 @@ func (ts *ExternalTestSuite) TestSignupExternalWorkOSWithOrganization() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.WorkOS.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.WorkOS.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.WorkOS.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("", q.Get("scope")) ts.Equal(organization, q.Get("organization")) @@ -77,7 +77,7 @@ func (ts *ExternalTestSuite) TestSignupExternalWorkOSWithProvider() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.WorkOS.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.WorkOS.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.WorkOS.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) ts.Equal("", q.Get("scope")) ts.Equal(provider, q.Get("provider")) diff --git a/internal/api/external_zoom_test.go b/internal/api/external_zoom_test.go index 63b7e8091..cac2c647f 100644 --- a/internal/api/external_zoom_test.go +++ b/internal/api/external_zoom_test.go @@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalZoom() { ts.Require().NoError(err, "redirect url parse failed") q := u.Query() ts.Equal(ts.Config.External.Zoom.RedirectURI, q.Get("redirect_uri")) - ts.Equal(ts.Config.External.Zoom.ClientID, q.Get("client_id")) + ts.Equal(ts.Config.External.Zoom.ClientID, []string{q.Get("client_id")}) ts.Equal("code", q.Get("response_type")) claims := ExternalProviderClaims{} diff --git a/internal/api/provider/apple.go b/internal/api/provider/apple.go index a2cb8825d..082795ad5 100644 --- a/internal/api/provider/apple.go +++ b/internal/api/provider/apple.go @@ -33,7 +33,7 @@ type appleUser struct { // NewAppleProvider creates a Apple account provider. func NewAppleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -48,7 +48,7 @@ func NewAppleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration) return &AppleProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oidcProvider.Endpoint(), Scopes: []string{ diff --git a/internal/api/provider/azure.go b/internal/api/provider/azure.go index cbfc82066..5aa1c4f5c 100644 --- a/internal/api/provider/azure.go +++ b/internal/api/provider/azure.go @@ -29,7 +29,7 @@ type azureUser struct { // NewAzureProvider creates a Azure account provider. func NewAzureProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -44,7 +44,7 @@ func NewAzureProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuth return &azureProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authHost + "/oauth2/v2.0/authorize", diff --git a/internal/api/provider/bitbucket.go b/internal/api/provider/bitbucket.go index 6bde65735..64a3828bc 100644 --- a/internal/api/provider/bitbucket.go +++ b/internal/api/provider/bitbucket.go @@ -38,7 +38,7 @@ type bitbucketEmails struct { // NewBitbucketProvider creates a Bitbucket account provider. func NewBitbucketProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -47,7 +47,7 @@ func NewBitbucketProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, e return &bitbucketProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authHost + "/site/oauth2/authorize", diff --git a/internal/api/provider/discord.go b/internal/api/provider/discord.go index 2af7b0436..4bb3468c6 100644 --- a/internal/api/provider/discord.go +++ b/internal/api/provider/discord.go @@ -31,7 +31,7 @@ type discordUser struct { // NewDiscordProvider creates a Discord account provider. func NewDiscordProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -48,7 +48,7 @@ func NewDiscordProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAu return &discordProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: apiPath + "/oauth2/authorize", diff --git a/internal/api/provider/facebook.go b/internal/api/provider/facebook.go index a511ad0d4..c23bee833 100644 --- a/internal/api/provider/facebook.go +++ b/internal/api/provider/facebook.go @@ -40,7 +40,7 @@ type facebookUser struct { // NewFacebookProvider creates a Facebook account provider. func NewFacebookProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -58,7 +58,7 @@ func NewFacebookProvider(ext conf.OAuthProviderConfiguration, scopes string) (OA return &facebookProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, RedirectURL: ext.RedirectURI, Endpoint: oauth2.Endpoint{ diff --git a/internal/api/provider/github.go b/internal/api/provider/github.go index ae0c59013..f26894aac 100644 --- a/internal/api/provider/github.go +++ b/internal/api/provider/github.go @@ -38,7 +38,7 @@ type githubUserEmail struct { // NewGithubProvider creates a Github account provider. func NewGithubProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -58,7 +58,7 @@ func NewGithubProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAut return &githubProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authHost + "/login/oauth/authorize", diff --git a/internal/api/provider/gitlab.go b/internal/api/provider/gitlab.go index 2504a1050..8f7a79a12 100644 --- a/internal/api/provider/gitlab.go +++ b/internal/api/provider/gitlab.go @@ -34,7 +34,7 @@ type gitlabUserEmail struct { // NewGitlabProvider creates a Gitlab account provider. func NewGitlabProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -49,7 +49,7 @@ func NewGitlabProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAut host := chooseHost(ext.URL, defaultGitLabAuthBase) return &gitlabProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: host + "/oauth/authorize", diff --git a/internal/api/provider/google.go b/internal/api/provider/google.go index b9e7e9569..e436e4f49 100644 --- a/internal/api/provider/google.go +++ b/internal/api/provider/google.go @@ -3,7 +3,6 @@ package provider import ( "context" "errors" - "fmt" "strings" "github.com/coreos/go-oidc/v3/oidc" @@ -40,7 +39,7 @@ type googleProvider struct { // NewGoogleProvider creates a Google OAuth2 identity provider. func NewGoogleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -64,7 +63,7 @@ func NewGoogleProvider(ctx context.Context, ext conf.OAuthProviderConfiguration, return &googleProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oidcProvider.Endpoint(), Scopes: oauthScopes, @@ -84,26 +83,16 @@ var internalUserInfoEndpointGoogle = UserInfoEndpointGoogle func (g googleProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*UserProvidedData, error) { if idToken := tok.Extra("id_token"); idToken != nil { - token, data, err := ParseIDToken(ctx, g.oidc, nil, idToken.(string), ParseIDTokenOptions{ + _, data, err := ParseIDToken(ctx, g.oidc, &oidc.Config{ + ClientID: g.Config.ClientID, + }, idToken.(string), ParseIDTokenOptions{ AccessToken: tok.AccessToken, }) if err != nil { return nil, err } - matchesAudience := false - for _, aud := range token.Audience { - if g.Config.ClientID == aud { - matchesAudience = true - break - } - } - - if !matchesAudience { - return nil, fmt.Errorf("provider: Google ID token issued for audience(s) %q but expected %q", strings.Join(token.Audience, ", "), g.Config.ClientID) - } - - return data, err + return data, nil } // This whole section offers legacy support in case the Google OAuth2 diff --git a/internal/api/provider/kakao.go b/internal/api/provider/kakao.go index 917facffe..3307270c3 100644 --- a/internal/api/provider/kakao.go +++ b/internal/api/provider/kakao.go @@ -71,7 +71,7 @@ func (p kakaoProvider) GetUserData(ctx context.Context, tok *oauth2.Token) (*Use } func NewKakaoProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -90,7 +90,7 @@ func NewKakaoProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuth return &kakaoProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthStyle: oauth2.AuthStyleInParams, diff --git a/internal/api/provider/keycloak.go b/internal/api/provider/keycloak.go index 9297c5480..1a170abba 100644 --- a/internal/api/provider/keycloak.go +++ b/internal/api/provider/keycloak.go @@ -24,7 +24,7 @@ type keycloakUser struct { // NewKeycloakProvider creates a Keycloak account provider. func NewKeycloakProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -48,7 +48,7 @@ func NewKeycloakProvider(ext conf.OAuthProviderConfiguration, scopes string) (OA return &keycloakProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: ext.URL + "/protocol/openid-connect/auth", diff --git a/internal/api/provider/linkedin.go b/internal/api/provider/linkedin.go index ac9aa7953..9841cc596 100644 --- a/internal/api/provider/linkedin.go +++ b/internal/api/provider/linkedin.go @@ -68,7 +68,7 @@ type linkedinElements struct { // NewLinkedinProvider creates a Linkedin account provider. func NewLinkedinProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -85,7 +85,7 @@ func NewLinkedinProvider(ext conf.OAuthProviderConfiguration, scopes string) (OA return &linkedinProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: apiPath + "/oauth/v2/authorization", diff --git a/internal/api/provider/notion.go b/internal/api/provider/notion.go index fa5ae9ff1..d44970e05 100644 --- a/internal/api/provider/notion.go +++ b/internal/api/provider/notion.go @@ -40,7 +40,7 @@ type notionUser struct { // NewNotionProvider creates a Notion account provider. func NewNotionProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -48,7 +48,7 @@ func NewNotionProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, erro return ¬ionProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authHost + "/v1/oauth/authorize", diff --git a/internal/api/provider/slack.go b/internal/api/provider/slack.go index 9d09c41cd..967adaf28 100644 --- a/internal/api/provider/slack.go +++ b/internal/api/provider/slack.go @@ -26,7 +26,7 @@ type slackUser struct { // NewSlackProvider creates a Slack account provider. func NewSlackProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -45,7 +45,7 @@ func NewSlackProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuth return &slackProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authPath + "/authorize", diff --git a/internal/api/provider/spotify.go b/internal/api/provider/spotify.go index c40105d92..f278c3ff3 100644 --- a/internal/api/provider/spotify.go +++ b/internal/api/provider/spotify.go @@ -34,7 +34,7 @@ type spotifyUserImage struct { // NewSpotifyProvider creates a Spotify account provider. func NewSpotifyProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -51,7 +51,7 @@ func NewSpotifyProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAu return &spotifyProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authPath + "/authorize", diff --git a/internal/api/provider/twitch.go b/internal/api/provider/twitch.go index 458afbd0b..0a7e7e396 100644 --- a/internal/api/provider/twitch.go +++ b/internal/api/provider/twitch.go @@ -45,7 +45,7 @@ type twitchUsers struct { // NewTwitchProvider creates a Twitch account provider. func NewTwitchProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -62,7 +62,7 @@ func NewTwitchProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAut return &twitchProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authHost + "/oauth2/authorize", diff --git a/internal/api/provider/twitter.go b/internal/api/provider/twitter.go index 5628196e8..96e2f09e7 100644 --- a/internal/api/provider/twitter.go +++ b/internal/api/provider/twitter.go @@ -46,12 +46,12 @@ type twitterUser struct { // NewTwitterProvider creates a Twitter account provider. func NewTwitterProvider(ext conf.OAuthProviderConfiguration, scopes string) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } authHost := chooseHost(ext.URL, defaultTwitterAPIBase) p := &TwitterProvider{ - ClientKey: ext.ClientID, + ClientKey: ext.ClientID[0], Secret: ext.Secret, CallbackURL: ext.RedirectURI, UserInfoURL: authHost + endpointProfile, diff --git a/internal/api/provider/workos.go b/internal/api/provider/workos.go index 8b58541b9..81c93581d 100644 --- a/internal/api/provider/workos.go +++ b/internal/api/provider/workos.go @@ -35,14 +35,14 @@ type workosUser struct { // NewWorkOSProvider creates a WorkOS account provider. func NewWorkOSProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } apiPath := chooseHost(ext.URL, defaultWorkOSAPIBase) return &workosProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: apiPath + "/sso/authorize", diff --git a/internal/api/provider/zoom.go b/internal/api/provider/zoom.go index 6397dfc43..819eb4a7b 100644 --- a/internal/api/provider/zoom.go +++ b/internal/api/provider/zoom.go @@ -31,7 +31,7 @@ type zoomUser struct { // NewZoomProvider creates a Zoom account provider. func NewZoomProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) { - if err := ext.Validate(); err != nil { + if err := ext.ValidateOAuth(); err != nil { return nil, err } @@ -40,7 +40,7 @@ func NewZoomProvider(ext conf.OAuthProviderConfiguration) (OAuthProvider, error) return &zoomProvider{ Config: &oauth2.Config{ - ClientID: ext.ClientID, + ClientID: ext.ClientID[0], ClientSecret: ext.Secret, Endpoint: oauth2.Endpoint{ AuthURL: authPath + "/authorize", diff --git a/internal/api/token_oidc.go b/internal/api/token_oidc.go index c078c7177..9a8c57ccb 100644 --- a/internal/api/token_oidc.go +++ b/internal/api/token_oidc.go @@ -29,45 +29,45 @@ type IdTokenGrantParams struct { func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.GlobalConfiguration, r *http.Request) (*oidc.Provider, string, []string, error) { log := observability.GetLogEntry(r) - enabled := true + var cfg *conf.OAuthProviderConfiguration var issuer string var providerType string var acceptableClientIDs []string switch true { case p.Provider == "apple" || p.Issuer == provider.IssuerApple: - enabled = config.External.Apple.Enabled + cfg = &config.External.Apple providerType = "apple" issuer = provider.IssuerApple - acceptableClientIDs = []string{config.External.Apple.ClientID} + acceptableClientIDs = append(acceptableClientIDs, config.External.Apple.ClientID...) if config.External.IosBundleId != "" { acceptableClientIDs = append(acceptableClientIDs, config.External.IosBundleId) } case p.Provider == "google" || p.Issuer == provider.IssuerGoogle: - enabled = config.External.Google.Enabled + cfg = &config.External.Google providerType = "google" issuer = provider.IssuerGoogle - acceptableClientIDs = []string{config.External.Google.ClientID} + acceptableClientIDs = append(acceptableClientIDs, config.External.Google.ClientID...) case p.Provider == "azure" || p.Issuer == provider.IssuerAzure: - enabled = config.External.Azure.Enabled + cfg = &config.External.Azure providerType = "azure" issuer = provider.IssuerAzure - acceptableClientIDs = []string{config.External.Azure.ClientID} + acceptableClientIDs = append(acceptableClientIDs, config.External.Azure.ClientID...) case p.Provider == "facebook" || p.Issuer == provider.IssuerFacebook: - enabled = config.External.Facebook.Enabled + cfg = &config.External.Facebook providerType = "facebook" issuer = provider.IssuerFacebook - acceptableClientIDs = []string{config.External.Facebook.ClientID} + acceptableClientIDs = append(acceptableClientIDs, config.External.Facebook.ClientID...) case p.Provider == "keycloak" || (config.External.Keycloak.Enabled && config.External.Keycloak.URL != "" && p.Issuer == config.External.Keycloak.URL): - enabled = config.External.Keycloak.Enabled + cfg = &config.External.Keycloak providerType = "keycloak" issuer = config.External.Keycloak.URL - acceptableClientIDs = []string{config.External.Keycloak.ClientID} + acceptableClientIDs = append(acceptableClientIDs, config.External.Keycloak.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!") @@ -88,7 +88,7 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa } } - if !enabled { + if cfg != nil && !cfg.Enabled { return nil, "", nil, badRequestError(fmt.Sprintf("Provider (issuer %q) is not enabled", issuer)) } @@ -145,6 +145,10 @@ func (a *API) IdTokenGrant(ctx context.Context, w http.ResponseWriter, r *http.R correctAudience := false for _, clientID := range acceptableClientIDs { + if clientID == "" { + continue + } + for _, aud := range idToken.Audience { if aud == clientID { correctAudience = true diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index c6de717a9..7098df12e 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -19,12 +19,12 @@ const defaultFlowStateExpiryDuration time.Duration = 300 * time.Second // 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"` } type EmailProviderConfiguration struct { @@ -445,15 +445,15 @@ func (c *GlobalConfiguration) Validate() error { return nil } -func (o *OAuthProviderConfiguration) Validate() error { +func (o *OAuthProviderConfiguration) ValidateOAuth() error { if !o.Enabled { return errors.New("provider is not enabled") } - if o.ClientID == "" { - return errors.New("missing Oauth client ID") + if len(o.ClientID) == 0 { + return errors.New("missing OAuth client ID") } if o.Secret == "" { - return errors.New("missing Oauth secret") + return errors.New("missing OAuth secret") } if o.RedirectURI == "" { return errors.New("missing redirect URI")