fix: remove unconfirmed identities on repeat signup

internal/api/identity_test.go

@@ -2,6 +2,8 @@ package api
 
 import (
  "context"
+ "net/http"
+ "net/http/httptest"
  "testing"
 
  "github.com/stretchr/testify/require"
@@ -56,7 +58,9 @@ func (ts *IdentityTestSuite) TestLinkIdentityToUser() {
  Subject: "test_subject",
  },
  }
- u, err = ts.API.linkIdentityToUser(ctx, ts.API.db, testValidUserData, "test")
+ // request is just used as a placeholder in the function
+ r := httptest.NewRequest(http.MethodGet, "/identities", nil)
+ u, err = ts.API.linkIdentityToUser(r, ctx, ts.API.db, testValidUserData, "test")
  require.NoError(ts.T(), err)
 
  // load associated identities for the user
@@ -71,7 +75,7 @@ func (ts *IdentityTestSuite) TestLinkIdentityToUser() {
  Subject: u.ID.String(),
  },
  }
- u, err = ts.API.linkIdentityToUser(ctx, ts.API.db, testExistingUserData, "email")
+ u, err = ts.API.linkIdentityToUser(r, ctx, ts.API.db, testExistingUserData, "email")
  require.ErrorIs(ts.T(), err, badRequestError("Identity is already linked"))
  require.Nil(ts.T(), u)
 }

internal/api/signup.go

@@ -199,20 +199,33 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
  if (params.Provider == "email" && user.IsConfirmed()) || (params.Provider == "phone" && user.IsPhoneConfirmed()) {
  return UserExistsError
  }
-
  // do not update the user because we can't be sure of their claimed identity
  } else {
  user, terr = a.signupNewUser(ctx, tx, signupUser)
  if terr != nil {
  return terr
  }
- identity, terr := a.createNewIdentity(tx, user, params.Provider, structs.Map(provider.Claims{
+ }
+ if _, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "email"); terr != nil {
+ if !models.IsNotFoundError(terr) {
+ return terr
+ }
+ identityData := structs.Map(provider.Claims{
  Subject: user.ID.String(),
  Email: user.GetEmail(),
- }))
+ })
+ for k, v := range params.Data {
+ if _, ok := identityData[k]; !ok {
+ identityData[k] = v
+ }
+ }
+ identity, terr := a.createNewIdentity(tx, user, params.Provider, identityData)
  if terr != nil {
  return terr
  }
+ if terr := user.RemoveUnconfirmedIdentities(tx, identity); terr != nil {
+ return terr
+ }
  user.Identities = []models.Identity{*identity}
  }
 

internal/models/user.go

@@ -709,10 +709,12 @@ func (u *User) UpdateBannedUntil(tx *storage.Connection) error {
 
 // RemoveUnconfirmedIdentities removes potentially malicious unconfirmed identities from a user (if any)
 func (u *User) RemoveUnconfirmedIdentities(tx *storage.Connection, identity *Identity) error {
- // user is unconfirmed so the password should be reset
- u.EncryptedPassword = ""
- if terr := tx.UpdateOnly(u, "encrypted_password"); terr != nil {
- return terr
+ if identity.Provider != "email" && identity.Provider != "phone" {
+ // user is unconfirmed so the password should be reset
+ u.EncryptedPassword = ""
+ if terr := tx.UpdateOnly(u, "encrypted_password"); terr != nil {
+ return terr
+ }
  }
 
  // user is unconfirmed so existing user_metadata should be overwritten