Refactor user schema to store provider data better

api/admin.go

@@ -236,7 +236,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
 	if user.AppMetaData == nil {
 		user.AppMetaData = make(map[string]interface{})
 	}
-	user.AppMetaData["provider"] = "email"
+	user.AppMetaData["provider"] = []string{"email"}
 
 	err = a.db.Transaction(func(tx *storage.Connection) error {
 		if terr := models.NewAuditLogEntry(tx, instanceID, adminUser, models.UserSignedUpAction, map[string]interface{}{

api/admin_test.go

@@ -54,7 +54,10 @@ func (ts *AdminTestSuite) makeSuperAdmin(email string) string {
 
 	u.Role = "supabase_admin"
 
-	token, err := generateAccessToken(u, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
+	identities, err := models.FindIdentitiesByUser(ts.API.db, u)
+	require.NoError(ts.T(), err, "Error retrieving identities")
+
+	token, err := generateAccessToken(u, identities, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
 	require.NoError(ts.T(), err, "Error generating access token")
 
 	p := jwt.Parser{ValidMethods: []string{jwt.SigningMethodHS256.Name}}
@@ -69,7 +72,11 @@ func (ts *AdminTestSuite) makeSuperAdmin(email string) string {
 func (ts *AdminTestSuite) makeSystemUser() string {
 	u := models.NewSystemUser(uuid.Nil, ts.Config.JWT.Aud)
 	u.Role = "service_role"
-	token, err := generateAccessToken(u, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
+
+	identities, err := models.FindIdentitiesByUser(ts.API.db, u)
+	require.NoError(ts.T(), err, "Error retrieving identities")
+
+	token, err := generateAccessToken(u, identities, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
 	require.NoError(ts.T(), err, "Error generating access token")
 
 	p := jwt.Parser{ValidMethods: []string{jwt.SigningMethodHS256.Name}}
@@ -278,7 +285,7 @@ func (ts *AdminTestSuite) TestAdminUserCreate() {
 	require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))
 	assert.Equal(ts.T(), "test1@example.com", data.GetEmail())
 	assert.Equal(ts.T(), "123456789", data.GetPhone())
-	assert.Equal(ts.T(), "email", data.AppMetaData["provider"])
+	assert.Equal(ts.T(), []interface{}{"email"}, data.AppMetaData["provider"])
 }
 
 // TestAdminUserGet tests API /admin/user route (GET)

api/audit_test.go

@@ -51,7 +51,9 @@ func (ts *AuditTestSuite) makeSuperAdmin(email string) string {
 
 	u.Role = "supabase_admin"
 
-	token, err := generateAccessToken(u, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
+	identities, err := models.FindIdentitiesByUser(ts.API.db, u)
+	require.NoError(ts.T(), err, "Error retrieving identities")
+	token, err := generateAccessToken(u, identities, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
 	require.NoError(ts.T(), err, "Error generating access token")
 
 	p := jwt.Parser{ValidMethods: []string{jwt.SigningMethodHS256.Name}}

api/external.go

@@ -15,7 +15,6 @@ import (
 	"github.com/netlify/gotrue/models"
 	"github.com/netlify/gotrue/storage"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/oauth2"
 )
 
 // ExternalProviderClaims are the JWT claims sent as the state in the external oauth provider signup flow
@@ -86,16 +85,6 @@ func (a *API) ExternalProviderRedirect(w http.ResponseWriter, r *http.Request) e
 		if err != nil {
 			return internalServerError("Error storing request token in session").WithInternalError(err)
 		}
-	case *provider.AppleProvider:
-		opts := make([]oauth2.AuthCodeOption, 0, 1)
-		opts = append(opts, oauth2.SetAuthURLParam("response_mode", "form_post"))
-		authURL = externalProvider.Config.AuthCodeURL(tokenString, opts...)
-		if authURL != "" {
-			if u, err := url.Parse(authURL); err == nil {
-				u.RawQuery = strings.ReplaceAll(u.RawQuery, "+", "%20")
-				authURL = u.String()
-			}
-		}
 	default:
 		authURL = p.AuthCodeURL(tokenString)
 	}
@@ -152,53 +141,86 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 			}
 		} else {
 			aud := a.requestAud(ctx, r)
-
-			// search user using all available emails
 			var emailData provider.Email
-			for _, e := range userData.Emails {
-				if e.Verified || config.Mailer.Autoconfirm {
-					user, terr = models.FindUserByEmailAndAudience(tx, instanceID, e.Email, aud)
-					if terr != nil && !models.IsNotFoundError(terr) {
-						return internalServerError("Error checking for duplicate users").WithInternalError(terr)
-					}
-
-					if user != nil {
-						emailData = e
-						break
-					}
+			var identityData map[string]interface{}
+			if userData.Metadata != nil {
+				identityData, terr = userData.Metadata.ToMap()
+				if terr != nil {
+					return terr
 				}
 			}
 
-			if user == nil {
-				if config.DisableSignup {
-					return forbiddenError("Signups not allowed for this instance")
-				}
-
-				// prefer primary email for new signups
-				emailData = userData.Emails[0]
-				for _, e := range userData.Emails {
-					if e.Primary {
-						emailData = e
-						break
+			var identity *models.Identity
+			// check if identity exists
+			if identity, terr = models.FindIdentityByIdAndProvider(tx, userData.Metadata.Subject, providerType); terr != nil {
+				if models.IsNotFoundError(terr) {
+					// search user using all available emails
+					for _, e := range userData.Emails {
+						if e.Verified || config.Mailer.Autoconfirm {
+							user, terr = models.FindUserByEmailAndAudience(tx, instanceID, e.Email, aud)
+							if terr != nil && !models.IsNotFoundError(terr) {
+								return internalServerError("Error checking for duplicate users").WithInternalError(terr)
+							}
+							if user != nil {
+								emailData = e
+								break
+							}
+						}
 					}
-				}
-
-				params := &SignupParams{
-					Provider: providerType,
-					Email:    emailData.Email,
-					Aud:      aud,
-					Data:     make(map[string]interface{}),
-				}
-				for k, v := range userData.Metadata {
-					if v != "" {
-						params.Data[k] = v
+					if user != nil {
+						if identity, terr = a.createNewIdentity(tx, user, providerType, identityData); terr != nil {
+							return terr
+						}
+						if terr = user.UpdateAppMetaDataProvider(tx); terr != nil {
+							return terr
+						}
+					} else {
+						if config.DisableSignup {
+							return forbiddenError("Signups not allowed for this instance")
+						}
+
+						// prefer primary email for new signups
+						emailData = userData.Emails[0]
+						for _, e := range userData.Emails {
+							if e.Primary {
+								emailData = e
+								break
+							}
+						}
+
+						params := &SignupParams{
+							Provider: providerType,
+							Email:    emailData.Email,
+							Aud:      aud,
+							Data:     identityData,
+						}
+
+						user, terr = a.signupNewUser(ctx, tx, params)
+						if terr != nil {
+							return terr
+						}
+
+						if identity, terr = a.createNewIdentity(tx, user, providerType, identityData); terr != nil {
+							return terr
+						}
 					}
+				} else {
+					return terr
 				}
+			}
 
-				user, terr = a.signupNewUser(ctx, tx, params)
+			if identity != nil && user == nil {
+				// get user associated with identity
+				user, terr = models.FindUserByID(tx, identity.UserID)
 				if terr != nil {
 					return terr
 				}
+				if terr = tx.UpdateOnly(identity, "identity_data", "last_sign_in_at"); terr != nil {
+					return terr
+				}
+				if terr = user.UpdateAppMetaDataProvider(tx); terr != nil {
+					return terr
+				}
 			}
 
 			if !user.IsConfirmed() {
@@ -281,19 +303,20 @@ func (a *API) processInvite(ctx context.Context, tx *storage.Connection, userDat
 		return nil, badRequestError("Invited email does not match emails from external provider").WithInternalMessage("invited=%s external=%s", user.Email, strings.Join(emails, ", "))
 	}
 
-	if err := user.UpdateAppMetaData(tx, map[string]interface{}{
-		"provider": providerType,
-	}); err != nil {
-		return nil, internalServerError("Database error updating user").WithInternalError(err)
-	}
-
-	updates := make(map[string]interface{})
-	for k, v := range userData.Metadata {
-		if v != "" {
-			updates[k] = v
+	var identityData map[string]interface{}
+	if userData.Metadata != nil {
+		identityData, err = userData.Metadata.ToMap()
+		if err != nil {
+			return nil, internalServerError("Error serialising user metadata").WithInternalError(err)
 		}
 	}
-	if err := user.UpdateUserMetaData(tx, updates); err != nil {
+	if _, err := a.createNewIdentity(tx, user, providerType, identityData); err != nil {
+		return nil, err
+	}
+	if err = user.UpdateAppMetaDataProvider(tx); err != nil {
+		return nil, err
+	}
+	if err := user.UpdateUserMetaData(tx, identityData); err != nil {
 		return nil, internalServerError("Database error updating user").WithInternalError(err)
 	}
 
@@ -417,3 +440,23 @@ func (a *API) getExternalRedirectURL(r *http.Request) string {
 	}
 	return config.SiteURL
 }
+
+func (a *API) createNewIdentity(conn *storage.Connection, user *models.User, providerType string, identityData map[string]interface{}) (*models.Identity, error) {
+	identity, err := models.NewIdentity(user, providerType, identityData)
+	if err != nil {
+		return nil, err
+	}
+
+	err = conn.Transaction(func(tx *storage.Connection) error {
+		if terr := tx.Create(identity); terr != nil {
+			return internalServerError("Error creating identity").WithInternalError(terr)
+		}
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return identity, nil
+}

api/external_azure_test.go

@@ -9,6 +9,11 @@ import (
 	jwt "github.com/golang-jwt/jwt"
 )
 
+const (
+	azureUser        string = `{"name":"Azure Test","email":"azure@example.com","sub":"azuretestid"}`
+	azureUserNoEmail string = `{"name":"Azure Test","sub":"azuretestid"}`
+)
+
 func (ts *ExternalTestSuite) TestSignupExternalAzure() {
 	req := httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=azure", nil)
 	w := httptest.NewRecorder()
@@ -61,23 +66,21 @@ func AzureTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int
 
 func (ts *ExternalTestSuite) TestSignupExternalAzure_AuthorizationCode() {
 	ts.Config.DisableSignup = false
-	ts.createUser("azure@example.com", "Azure Test", "", "")
+	ts.createUser("azuretestid", "azure@example.com", "Azure Test", "", "")
 	tokenCount, userCount := 0, 0
 	code := "authcode"
-	azureUser := `{"name":"Azure Test","email":"azure@example.com"}`
 	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser)
 	defer server.Close()
 
 	u := performAuthorization(ts, "azure", code, "")
 
-	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "")
+	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "")
 }
 
 func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoUser() {
 	ts.Config.DisableSignup = true
 	tokenCount, userCount := 0, 0
 	code := "authcode"
-	azureUser := `{"name":"Azure Test","email":"azure@example.com"}`
 	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser)
 	defer server.Close()
 
@@ -90,8 +93,7 @@ func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoEmai
 	ts.Config.DisableSignup = true
 	tokenCount, userCount := 0, 0
 	code := "authcode"
-	azureUser := `{"name":"Azure Test"}`
-	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser)
+	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUserNoEmail)
 	defer server.Close()
 
 	u := performAuthorization(ts, "azure", code, "")
@@ -103,32 +105,30 @@ func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupErrorWhenNoEmai
 func (ts *ExternalTestSuite) TestSignupExternalAzureDisableSignupSuccessWithPrimaryEmail() {
 	ts.Config.DisableSignup = true
 
-	ts.createUser("azure@example.com", "Azure Test", "", "")
+	ts.createUser("azuretestid", "azure@example.com", "Azure Test", "", "")
 
 	tokenCount, userCount := 0, 0
 	code := "authcode"
-	azureUser := `{"name":"Azure Test","email":"azure@example.com"}`
 	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser)
 	defer server.Close()
 
 	u := performAuthorization(ts, "azure", code, "")
 
-	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "")
+	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "")
 }
 
 func (ts *ExternalTestSuite) TestInviteTokenExternalAzureSuccessWhenMatchingToken() {
 	// name and avatar should be populated from Azure API
-	ts.createUser("azure@example.com", "", "", "invite_token")
+	ts.createUser("azuretestid", "azure@example.com", "", "", "invite_token")
 
 	tokenCount, userCount := 0, 0
 	code := "authcode"
-	azureUser := `{"name":"Azure Test","email":"azure@example.com"}}`
 	server := AzureTestSignupSetup(ts, &tokenCount, &userCount, code, azureUser)
 	defer server.Close()
 
 	u := performAuthorization(ts, "azure", code, "invite_token")
 
-	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "")
+	assertAuthorizationSuccess(ts, u, tokenCount, userCount, "azure@example.com", "Azure Test", "azuretestid", "")
 }
 
 func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenNoMatchingToken() {
@@ -143,7 +143,7 @@ func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenNoMatchingToke
 }
 
 func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenWrongToken() {
-	ts.createUser("azure@example.com", "", "", "invite_token")
+	ts.createUser("azuretestid", "azure@example.com", "", "", "invite_token")
 
 	tokenCount, userCount := 0, 0
 	code := "authcode"
@@ -156,7 +156,7 @@ func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenWrongToken() {
 }
 
 func (ts *ExternalTestSuite) TestInviteTokenExternalAzureErrorWhenEmailDoesntMatch() {
-	ts.createUser("azure@example.com", "", "", "invite_token")
+	ts.createUser("azuretestid", "azure@example.com", "", "", "invite_token")
 
 	tokenCount, userCount := 0, 0
 	code := "authcode"