Repository: supabase/gotrue
Pull request #889 (2023-01-11 09:10:05)

fix: return signup confirmation if signup is incomplete for magiclink / otp
## What kind of change does this PR introduce?
* On subsequent requests for magiclink or sms OTP, if the user hasn't completed the signup process yet (i.e they haven't confirmed their email or phone), then the endpoint should resend the signup confirmation email or sms.
* The previous behaviour would have sent the magiclink email template even though user hasn't accepted the signup confirmation yet. This led to an undesirable onboarding experience when resending magiclinks because subsequent requests would deliver the wrong email template for a user who hasn't completed the signup process. 
* Partially address #312. Specifically, this comment:
> Send a magic link. This will work, though you'll be using a different email template. Instead of sending a confirmation email, you'll be sending a log-in email. Functionally this is the same, but users could be confused because the email's contents are not exactly what they were looking for. You could copy the password confirmation email into the magic link template, but then you can't use the magic link.



Patch:
diff --git a/api/admin.go b/api/admin.go
index bdca289a..bd46a5d6 100644
--- a/api/admin.go
+++ b/api/admin.go
@@ -2,14 +2,17 @@ package api
 
 import (
 	"context"
+	"crypto/sha256"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
 
 	"github.com/go-chi/chi"
 	"github.com/gofrs/uuid"
+	"github.com/netlify/gotrue/crypto"
 	"github.com/netlify/gotrue/models"
 	"github.com/netlify/gotrue/observability"
 	"github.com/netlify/gotrue/storage"
@@ -29,6 +32,10 @@ type AdminUserParams struct {
 	BanDuration  string                 `json:"ban_duration"`
 }
 
+type adminUserDeleteParams struct {
+	ShouldSoftDelete bool `json:"should_soft_delete"`
+}
+
 type adminUserUpdateFactorParams struct {
 	FriendlyName string `json:"friendly_name"`
 	FactorType   string `json:"factor_type"`
@@ -374,14 +381,31 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
 	return sendJSON(w, http.StatusOK, user)
 }
 
-// adminUserDelete delete a user
+// adminUserDelete deletes a user
 func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error {
 	ctx := r.Context()
-	db := a.db.WithContext(ctx)
 	user := getUser(ctx)
 	adminUser := getAdminUser(ctx)
 
-	err := db.Transaction(func(tx *storage.Connection) error {
+	if user.IsSSOUser {
+		return badRequestError("user should be removed via identity provider instead")
+	}
+
+	var err error
+	params := &adminUserDeleteParams{}
+	body, err := getBodyBytes(r)
+	if err != nil {
+		return badRequestError("Could not read body").WithInternalError(err)
+	}
+	if len(body) > 0 {
+		if err := json.Unmarshal(body, params); err != nil {
+			return badRequestError("Could not read params: %v", err)
+		}
+	} else {
+		params.ShouldSoftDelete = false
+	}
+
+	err = a.db.Transaction(func(tx *storage.Connection) error {
 		if terr := models.NewAuditLogEntry(r, tx, adminUser, models.UserDeletedAction, "", map[string]interface{}{
 			"user_id":    user.ID,
 			"user_email": user.Email,
@@ -390,9 +414,76 @@ func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error {
 			return internalServerError("Error recording audit log entry").WithInternalError(terr)
 		}
 
-		if terr := tx.Destroy(user); terr != nil {
-			return internalServerError("Database error deleting user").WithInternalError(terr)
+		if params.ShouldSoftDelete {
+			if user.DeletedAt != nil {
+				// user has been soft deleted already
+				return nil
+			}
+			user.Email = storage.NullString(obfuscateFieldForSoftDelete(user.GetEmail()))
+			user.Phone = storage.NullString(obfuscateFieldForSoftDelete(user.GetPhone()))
+			user.EncryptedPassword = ""
+			user.EmailChange = obfuscateFieldForSoftDelete(user.EmailChange)
+			user.PhoneChange = obfuscateFieldForSoftDelete(user.PhoneChange)
+			now := time.Now()
+			user.DeletedAt = &now
+			if terr := tx.UpdateOnly(user, "email", "phone", "encrypted_password", "email_change", "phone_change", "deleted_at"); terr != nil {
+				return internalServerError("Error soft deleting user").WithInternalError(terr)
+			}
+
+			// set raw_user_meta_data to {}
+			userMetaDataUpdates := map[string]interface{}{}
+			for k := range user.UserMetaData {
+				userMetaDataUpdates[k] = nil
+			}
+			if terr := user.UpdateUserMetaData(tx, userMetaDataUpdates); terr != nil {
+				return internalServerError("Error soft deleting user meta data").WithInternalError(terr)
+			}
+
+			// set raw_app_meta_data to {}
+			appMetaDataUpdates := map[string]interface{}{}
+			for k := range user.AppMetaData {
+				appMetaDataUpdates[k] = nil
+			}
+			if terr := user.UpdateAppMetaData(tx, appMetaDataUpdates); terr != nil {
+				return internalServerError("Error soft deleting app meta data").WithInternalError(terr)
+			}
+
+			identities, terr := models.FindIdentitiesByUserID(tx, user.ID)
+			if terr != nil {
+				return internalServerError("Error retrieving identities").WithInternalError(terr)
+			}
+			// set identity_data to {}
+			for _, identity := range identities {
+				identity.ID = obfuscateFieldForSoftDelete(identity.ID)
+				if terr := tx.UpdateOnly(identity, "id"); terr != nil {
+					return internalServerError("Error soft deleting identity id").WithInternalError(terr)
+				}
+				identityDataUpdates := map[string]interface{}{}
+				for k := range identity.IdentityData {
+					identityDataUpdates[k] = nil
+				}
+				if terr := identity.UpdateIdentityData(tx, identityDataUpdates); terr != nil {
+					return internalServerError("Error soft deleting identity data").WithInternalError(terr)
+				}
+			}
+			// hard delete all associated factors
+			if terr := models.DeleteFactorsByUserId(tx, user.ID); terr != nil {
+				return internalServerError("Error deleting user's factors").WithInternalError(terr)
+			}
+			// hard delete all associated sessions
+			if terr := models.Logout(tx, user.ID); terr != nil {
+				return internalServerError("Error deleting user's sessions").WithInternalError(terr)
+			}
+			// for backward compatibility: hard delete all associated refresh tokens
+			if terr = models.LogoutAllRefreshTokens(tx, user.ID); terr != nil {
+				return internalServerError("Error deleting user's refresh tokens").WithInternalError(terr)
+			}
+		} else {
+			if terr := tx.Destroy(user); terr != nil {
+				return internalServerError("Database error deleting user").WithInternalError(terr)
+			}
 		}
+
 		return nil
 	})
 	if err != nil {
@@ -402,6 +493,11 @@ func (a *API) adminUserDelete(w http.ResponseWriter, r *http.Request) error {
 	return sendJSON(w, http.StatusOK, map[string]interface{}{})
 }
 
+func obfuscateFieldForSoftDelete(field string) string {
+	softDeleteId, _ := crypto.GenerateNanoId(5)
+	return fmt.Sprintf("%s-%x", softDeleteId, sha256.Sum256([]byte(field)))
+}
+
 func (a *API) adminUserDeleteFactor(w http.ResponseWriter, r *http.Request) error {
 	ctx := r.Context()
 	user := getUser(ctx)
diff --git a/api/external.go b/api/external.go
index ea75697e..db6f4429 100644
--- a/api/external.go
+++ b/api/external.go
@@ -192,7 +192,7 @@ func (a *API) internalExternalProviderCallback(w http.ResponseWriter, r *http.Re
 			return internalServerError("Failed to set JWT cookie. %s", err)
 		}
 	} else {
-		rurl = a.prepErrorRedirectURL(unauthorizedError("Unverified email with %v", providerType), r, rurl)
+		rurl = a.prepErrorRedirectURL(unauthorizedError("Unverified email with %v", providerType), w, r, rurl)
 	}
 
 	http.Redirect(w, r, rurl, http.StatusFound)
diff --git a/api/magic_link.go b/api/magic_link.go
index 41975faa..fdd35bfc 100644
--- a/api/magic_link.go
+++ b/api/magic_link.go
@@ -48,53 +48,62 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
 		return err
 	}
 
+	var isNewUser bool
 	aud := a.requestAud(ctx, r)
 	user, err := models.FindUserByEmailAndAudience(db, params.Email, aud)
 	if err != nil {
 		if models.IsNotFoundError(err) {
-			// User doesn't exist, sign them up with temporary password
-			password, err := password.Generate(64, 10, 0, false, true)
-			if err != nil {
-				internalServerError("error creating user").WithInternalError(err)
-			}
+			isNewUser = true
+		} else {
+			return internalServerError("Database error finding user").WithInternalError(err)
+		}
+	}
+	if user != nil {
+		isNewUser = !user.IsConfirmed()
+	}
+	if isNewUser {
+		// User either doesn't exist or hasn't completed the signup process.
+		// Sign them up with temporary password.
+		password, err := password.Generate(64, 10, 0, false, true)
+		if err != nil {
+			internalServerError("error creating user").WithInternalError(err)
+		}
 
-			signUpParams := &SignupParams{
-				Email:    params.Email,
-				Password: password,
-				Data:     params.Data,
+		signUpParams := &SignupParams{
+			Email:    params.Email,
+			Password: password,
+			Data:     params.Data,
+		}
+		newBodyContent, err := json.Marshal(signUpParams)
+		if err != nil {
+			return badRequestError("Could not parse metadata: %v", err)
+		}
+		r.Body = io.NopCloser(strings.NewReader(string(newBodyContent)))
+		r.ContentLength = int64(len(string(newBodyContent)))
+
+		fakeResponse := &responseStub{}
+		if config.Mailer.Autoconfirm {
+			// signups are autoconfirmed, send magic link after signup
+			if err := a.Signup(fakeResponse, r); err != nil {
+				return err
+			}
+			newBodyContent := &SignupParams{
+				Email: params.Email,
+				Data:  params.Data,
 			}
-			newBodyContent, err := json.Marshal(signUpParams)
+			metadata, err := json.Marshal(newBodyContent)
 			if err != nil {
 				return badRequestError("Could not parse metadata: %v", err)
 			}
-			r.Body = io.NopCloser(strings.NewReader(string(newBodyContent)))
-			r.ContentLength = int64(len(string(newBodyContent)))
-
-			fakeResponse := &responseStub{}
-			if config.Mailer.Autoconfirm {
-				// signups are autoconfirmed, send magic link after signup
-				if err := a.Signup(fakeResponse, r); err != nil {
-					return err
-				}
-				newBodyContent := &SignupParams{
-					Email: params.Email,
-					Data:  params.Data,
-				}
-				metadata, err := json.Marshal(newBodyContent)
-				if err != nil {
-					return badRequestError("Could not parse metadata: %v", err)
-				}
-				r.Body = io.NopCloser(bytes.NewReader(metadata))
-				return a.MagicLink(w, r)
-			}
-			// otherwise confirmation email already contains 'magic link'
-			if err := a.Signup(fakeResponse, r); err != nil {
-				return err
-			}
-
-			return sendJSON(w, http.StatusOK, make(map[string]string))
+			r.Body = io.NopCloser(bytes.NewReader(metadata))
+			return a.MagicLink(w, r)
 		}
-		return internalServerError("Database error finding user").WithInternalError(err)
+		// otherwise confirmation email already contains 'magic link'
+		if err := a.Signup(fakeResponse, r); err != nil {
+			return err
+		}
+
+		return sendJSON(w, http.StatusOK, make(map[string]string))
 	}
 
 	err = db.Transaction(func(tx *storage.Connection) error {
diff --git a/api/otp.go b/api/otp.go
index f565310d..8e4e1606 100644
--- a/api/otp.go
+++ b/api/otp.go
@@ -92,53 +92,61 @@ func (a *API) SmsOtp(w http.ResponseWriter, r *http.Request) error {
 		return err
 	}
 
+	var isNewUser bool
 	aud := a.requestAud(ctx, r)
+	user, err := models.FindUserByPhoneAndAudience(db, params.Phone, aud)
+	if err != nil {
+		if models.IsNotFoundError(err) {
+			isNewUser = true
+		} else {
+			return internalServerError("Database error finding user").WithInternalError(err)
+		}
+	}
+	if user != nil {
+		isNewUser = !user.IsPhoneConfirmed()
+	}
+	if isNewUser {
+		// User either doesn't exist or hasn't completed the signup process.
+		// Sign them up with temporary password.
+		password, err := password.Generate(64, 10, 0, false, true)
+		if err != nil {
+			internalServerError("error creating user").WithInternalError(err)
+		}
 
-	user, uerr := models.FindUserByPhoneAndAudience(db, params.Phone, aud)
-	if uerr != nil {
-		// if user does not exists, sign up the user
-		if models.IsNotFoundError(uerr) {
-			password, err := password.Generate(64, 10, 0, false, true)
-			if err != nil {
-				internalServerError("error creating user").WithInternalError(err)
+		signUpParams := &SignupParams{
+			Phone:    params.Phone,
+			Password: password,
+			Data:     params.Data,
+		}
+		newBodyContent, err := json.Marshal(signUpParams)
+		if err != nil {
+			return badRequestError("Could not parse metadata: %v", err)
+		}
+		r.Body = io.NopCloser(bytes.NewReader(newBodyContent))
+
+		fakeResponse := &responseStub{}
+
+		if config.Sms.Autoconfirm {
+			// signups are autoconfirmed, send otp after signup
+			if err := a.Signup(fakeResponse, r); err != nil {
+				return err
 			}
 
 			signUpParams := &SignupParams{
-				Phone:    params.Phone,
-				Password: password,
-				Data:     params.Data,
+				Phone: params.Phone,
 			}
 			newBodyContent, err := json.Marshal(signUpParams)
 			if err != nil {
 				return badRequestError("Could not parse metadata: %v", err)
 			}
 			r.Body = io.NopCloser(bytes.NewReader(newBodyContent))
+			return a.SmsOtp(w, r)
+		}
 
-			fakeResponse := &responseStub{}
-
-			if config.Sms.Autoconfirm {
-				// signups are autoconfirmed, send otp after signup
-				if err := a.Signup(fakeResponse, r); err != nil {
-					return err
-				}
-
-				signUpParams := &SignupParams{
-					Phone: params.Phone,
-				}
-				newBodyContent, err := json.Marshal(signUpParams)
-				if err != nil {
-					return badRequestError("Could not parse metadata: %v", err)
-				}
-				r.Body = io.NopCloser(bytes.NewReader(newBodyContent))
-				return a.SmsOtp(w, r)
-			}
-
-			if err := a.Signup(fakeResponse, r); err != nil {
-				return err
-			}
-			return sendJSON(w, http.StatusOK, make(map[string]string))
+		if err := a.Signup(fakeResponse, r); err != nil {
+			return err
 		}
-		return internalServerError("Database error finding user").WithInternalError(uerr)
+		return sendJSON(w, http.StatusOK, make(map[string]string))
 	}
 
 	err = db.Transaction(func(tx *storage.Connection) error {
diff --git a/api/verify.go b/api/verify.go
index 0d424e1d..98e19b25 100644
--- a/api/verify.go
+++ b/api/verify.go
@@ -31,6 +31,8 @@ const (
 	emailChangeVerification = "email_change"
 	smsVerification         = "sms"
 	phoneChangeVerification = "phone_change"
+	// includes signupVerification and magicLinkVerification
+	emailOTPVerification = "email"
 )
 
 const (
@@ -38,12 +40,6 @@ const (
 	singleConfirmation
 )
 
-const (
-	// v1 uses crypto.SecureToken()
-	v1OtpLength      = 22
-	sum224HashLength = 28
-)
-
 // Only applicable when SECURE_EMAIL_CHANGE_ENABLED
 const singleConfirmationAccepted = "Confirmation link accepted. Please proceed to confirm link sent to the other email"
 
@@ -89,10 +85,8 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request) error {
 		if params.Token == "" {
 			return badRequestError("Verify requires a token")
 		}
-		if len(params.Token) > v1OtpLength {
-			// token follows the v2 format and includes "-"
-			params.Token = strings.ReplaceAll(params.Token, "-", "")
-		}
+		params.Token = strings.ReplaceAll(params.Token, "-", "")
+
 		if params.Type == "" {
 			return badRequestError("Verify requires a verification type")
 		}
@@ -138,7 +132,7 @@ func (a *API) verifyGet(w http.ResponseWriter, r *http.Request) error {
 	if err != nil {
 		var herr *HTTPError
 		if errors.As(err, &herr) {
-			rurl := a.prepErrorRedirectURL(herr, r, params.RedirectTo)
+			rurl := a.prepErrorRedirectURL(herr, w, r, params.RedirectTo)
 			http.Redirect(w, r, rurl, http.StatusSeeOther)
 			return nil
 		}
@@ -174,10 +168,7 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request) error {
 	if params.Token == "" {
 		return badRequestError("Verify requires a token")
 	}
-	if len(params.Token) > v1OtpLength {
-		// token follows the v2 format and includes "-"
-		params.Token = strings.ReplaceAll(params.Token, "-", "")
-	}
+	params.Token = strings.ReplaceAll(params.Token, "-", "")
 
 	if params.Type == "" {
 		return badRequestError("Verify requires a verification type")
@@ -340,12 +331,12 @@ func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Conn
 	return user, nil
 }
 
-func (a *API) prepErrorRedirectURL(err *HTTPError, r *http.Request, rurl string) string {
+func (a *API) prepErrorRedirectURL(err *HTTPError, w http.ResponseWriter, r *http.Request, rurl string) string {
 	q := url.Values{}
-
 	log := observability.GetLogEntry(r)
-	log.Error(err.Message)
-
+	errorID := getRequestID(r.Context())
+	err.ErrorID = errorID
+	log.WithError(err.Cause()).Info(err.Error())
 	if str, ok := oauthErrorMap[err.Code]; ok {
 		q.Set("error", str)
 	}
@@ -501,38 +492,27 @@ func (a *API) verifyUserAndToken(ctx context.Context, conn *storage.Connection,
 
 	var isValid bool
 	switch params.Type {
-	case signupVerification, inviteVerification:
-		// TODO(km): remove when old token format is deprecated
-		// the new token format is represented by a MD5 hash which is 32 characters (128 bits) long
-		// anything shorter than 32 characters can safely be assumed to be using the old token format
-		if len(user.ConfirmationToken) < sum224HashLength {
-			tokenHash = params.Token
+	case emailOTPVerification:
+		// if the type is emailOTPVerification, we'll check both the confirmation_token and recovery_token columns
+		if isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp) {
+			isValid = true
+			params.Type = signupVerification
+		} else if isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp) {
+			isValid = true
+			params.Type = magicLinkVerification
+		} else {
+			isValid = false
 		}
+	case signupVerification, inviteVerification:
 		isValid = isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Mailer.OtpExp)
 	case recoveryVerification, magicLinkVerification:
-		// TODO(km): remove when old token format is deprecated
-		if len(user.RecoveryToken) < sum224HashLength {
-			tokenHash = params.Token
-		}
 		isValid = isOtpValid(tokenHash, user.RecoveryToken, user.RecoverySentAt, config.Mailer.OtpExp)
 	case emailChangeVerification:
-		// TODO(km): remove when old token format is deprecated
-		if len(user.EmailChangeTokenCurrent) < sum224HashLength && len(user.EmailChangeTokenNew) < sum224HashLength {
-			tokenHash = params.Token
-		}
 		isValid = isOtpValid(tokenHash, user.EmailChangeTokenCurrent, user.EmailChangeSentAt, config.Mailer.OtpExp) ||
 			isOtpValid(tokenHash, user.EmailChangeTokenNew, user.EmailChangeSentAt, config.Mailer.OtpExp)
 	case phoneChangeVerification:
-		// TODO(km): remove when old token format is deprecated
-		if len(user.PhoneChangeToken) < sum224HashLength {
-			tokenHash = params.Token
-		}
 		isValid = isOtpValid(tokenHash, user.PhoneChangeToken, user.PhoneChangeSentAt, config.Sms.OtpExp)
 	case smsVerification:
-		// TODO(km): remove when old token format is deprecated
-		if len(user.ConfirmationToken) < sum224HashLength {
-			tokenHash = params.Token
-		}
 		isValid = isOtpValid(tokenHash, user.ConfirmationToken, user.ConfirmationSentAt, config.Sms.OtpExp)
 	}
 
diff --git a/crypto/crypto.go b/crypto/crypto.go
index 70980c16..2b4fb96c 100644
--- a/crypto/crypto.go
+++ b/crypto/crypto.go
@@ -37,3 +37,22 @@ func GenerateOtp(digits int) (string, error) {
 	otp := fmt.Sprintf(expr, val.String())
 	return otp, nil
 }
+
+// generateOtpFromCharset generates a random n-length otp from a charset
+func generateOtpFromCharset(length int, charset string) (string, error) {
+	b := make([]byte, length)
+	for i := range b {
+		val, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset))))
+		if err != nil {
+			return "", errors.WithMessage(err, "Error generating otp from charset")
+		}
+		b[i] = charset[val.Int64()]
+	}
+	return string(b), nil
+}
+
+// GenerateNanoId generates a random n-length alphanumeric otp
+func GenerateNanoId(length int) (string, error) {
+	const charset = "0123456789abcdefghijklmnopqrstuvwxyz"
+	return generateOtpFromCharset(length, charset)
+}
diff --git a/hack/postgresd.sh b/hack/postgresd.sh
index ace18257..c4b6a58e 100755
--- a/hack/postgresd.sh
+++ b/hack/postgresd.sh
@@ -11,4 +11,4 @@ docker run --name gotrue_postgresql \
 	-e POSTGRES_DB=postgres \
 	--volume postgres_data:/var/lib/postgresql/data \
 	--volume "$(pwd)"/hack/init_postgres.sql:/docker-entrypoint-initdb.d/init.sql \
-	-d postgres:14
+	-d postgres:15
diff --git a/migrations/20230116124310_alter_phone_type.up.sql b/migrations/20230116124310_alter_phone_type.up.sql
new file mode 100644
index 00000000..49d4cdc9
--- /dev/null
+++ b/migrations/20230116124310_alter_phone_type.up.sql
@@ -0,0 +1,5 @@
+-- alter phone field column type to accomodate for soft deletion 
+
+alter table auth.users
+alter column phone type text,
+alter column phone_change type text;
diff --git a/migrations/20230116124412_add_deleted_at.up.sql b/migrations/20230116124412_add_deleted_at.up.sql
new file mode 100644
index 00000000..999abaa8
--- /dev/null
+++ b/migrations/20230116124412_add_deleted_at.up.sql
@@ -0,0 +1,4 @@
+-- adds deleted_at column to auth.users 
+
+alter table {{ index .Options "Namespace" }}.users 
+add column if not exists deleted_at timestamptz null;
diff --git a/models/factor.go b/models/factor.go
index 64861528..2abe912e 100644
--- a/models/factor.go
+++ b/models/factor.go
@@ -176,5 +176,11 @@ func (f *Factor) DowngradeSessionsToAAL1(tx *storage.Connection) error {
 		}
 	}
 	return updateFactorAssociatedSessions(tx, f.UserID, f.ID, AAL1.String())
+}
 
+func DeleteFactorsByUserId(tx *storage.Connection, userId uuid.UUID) error {
+	if err := tx.RawQuery("DELETE FROM "+(&pop.Model{Value: Factor{}}).TableName()+" WHERE user_id = ?", userId).Exec(); err != nil {
+		return err
+	}
+	return nil
 }
diff --git a/models/identity.go b/models/identity.go
index cd2d714b..a31152f1 100644
--- a/models/identity.go
+++ b/models/identity.go
@@ -28,14 +28,14 @@ func (Identity) TableName() string {
 
 // NewIdentity returns an identity associated to the user's id.
 func NewIdentity(user *User, provider string, identityData map[string]interface{}) (*Identity, error) {
-	id, ok := identityData["sub"]
+	providerId, ok := identityData["sub"]
 	if !ok {
 		return nil, errors.New("error missing provider id")
 	}
 	now := time.Now()
 
 	identity := &Identity{
-		ID:           id.(string),
+		ID:           providerId.(string),
 		UserID:       user.ID,
 		IdentityData: identityData,
 		Provider:     provider,
@@ -60,7 +60,7 @@ func (i *Identity) IsForSSOProvider() bool {
 	return strings.HasPrefix(i.Provider, "sso:")
 }
 
-// FindIdentityById searches for an identity with the matching provider_id and provider given.
+// FindIdentityById searches for an identity with the matching id and provider given.
 func FindIdentityByIdAndProvider(tx *storage.Connection, providerId, provider string) (*Identity, error) {
 	identity := &Identity{}
 	if err := tx.Q().Where("id = ? AND provider = ?", providerId, provider).First(identity); err != nil {
@@ -99,3 +99,21 @@ func FindProvidersByUser(tx *storage.Connection, user *User) ([]string, error) {
 	}
 	return providers, nil
 }
+
+// UpdateIdentityData sets all identity_data from a map of updates,
+// ensuring that it doesn't override attributes that are not
+// in the provided map.
+func (i *Identity) UpdateIdentityData(tx *storage.Connection, updates map[string]interface{}) error {
+	if i.IdentityData == nil {
+		i.IdentityData = updates
+	} else {
+		for key, value := range updates {
+			if value != nil {
+				i.IdentityData[key] = value
+			} else {
+				delete(i.IdentityData, key)
+			}
+		}
+	}
+	return tx.UpdateOnly(i, "identity_data")
+}
diff --git a/models/user.go b/models/user.go
index c13c5db8..394dedb2 100644
--- a/models/user.go
+++ b/models/user.go
@@ -62,6 +62,7 @@ type User struct {
 	CreatedAt   time.Time  `json:"created_at" db:"created_at"`
 	UpdatedAt   time.Time  `json:"updated_at" db:"updated_at"`
 	BannedUntil *time.Time `json:"banned_until,omitempty" db:"banned_until"`
+	DeletedAt   *time.Time `json:"deleted_at,omitempty" db:"deleted_at"`
 
 	DONTUSEINSTANCEID uuid.UUID `json:"-" db:"instance_id"`
 }


Test patch:
diff --git a/api/admin_test.go b/api/admin_test.go
index daa3a934..6c4c90b2 100644
--- a/api/admin_test.go
+++ b/api/admin_test.go
@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -433,20 +434,88 @@ func (ts *AdminTestSuite) TestAdminUserUpdateBannedUntilFailed() {
 	})
 }
 
-// TestAdminUserDelete tests API /admin/user route (DELETE)
+// TestAdminUserDelete tests API /admin/users route (DELETE)
 func (ts *AdminTestSuite) TestAdminUserDelete() {
-	u, err := models.NewUser("123456789", "test-delete@example.com", "test", ts.Config.JWT.Aud, nil)
-	require.NoError(ts.T(), err, "Error making new user")
-	require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user")
+	type expected struct {
+		code int
+		err  error
+	}
+	signupParams := &SignupParams{
+		Email:    "test-delete@example.com",
+		Password: "test",
+		Data:     map[string]interface{}{"name": "test"},
+		Provider: "email",
+		Aud:      ts.Config.JWT.Aud,
+	}
+	cases := []struct {
+		desc         string
+		body         map[string]interface{}
+		isSoftDelete string
+		isSSOUser    bool
+		expected     expected
+	}{
+		{
+			desc:         "Test admin delete user (default)",
+			isSoftDelete: "",
+			isSSOUser:    false,
+			expected:     expected{code: http.StatusOK, err: models.UserNotFoundError{}},
+			body:         nil,
+		},
+		{
+			desc:         "Test admin delete user (hard deletion)",
+			isSoftDelete: "?is_soft_delete=false",
+			isSSOUser:    false,
+			expected:     expected{code: http.StatusOK, err: models.UserNotFoundError{}},
+			body: map[string]interface{}{
+				"should_soft_delete": false,
+			},
+		},
+		{
+			desc:         "Test admin delete user (soft deletion)",
+			isSoftDelete: "?is_soft_delete=true",
+			isSSOUser:    false,
+			expected:     expected{code: http.StatusOK, err: models.UserNotFoundError{}},
+			body: map[string]interface{}{
+				"should_soft_delete": true,
+			},
+		},
+		{
+			desc:         "Test admin delete user (soft deletion & sso user)",
+			isSoftDelete: "?is_soft_delete=true",
+			isSSOUser:    true,
+			expected:     expected{code: http.StatusBadRequest, err: nil},
+			body: map[string]interface{}{
+				"should_soft_delete": true,
+			},
+		},
+	}
 
-	// Setup request
-	w := httptest.NewRecorder()
-	req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/admin/users/%s", u.ID), nil)
+	for _, c := range cases {
+		ts.Run(c.desc, func() {
+			var buffer bytes.Buffer
+			require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.body))
+			u, err := ts.API.signupNewUser(context.Background(), ts.API.db, signupParams, c.isSSOUser)
+			require.NoError(ts.T(), err, "Error creating user")
 
-	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token))
+			// Setup request
+			w := httptest.NewRecorder()
+			req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/admin/users/%s", u.ID), &buffer)
+
+			req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", ts.token))
+
+			ts.API.handler.ServeHTTP(w, req)
+			require.Equal(ts.T(), c.expected.code, w.Code)
+
+			if c.isSSOUser {
+				u, err = models.FindUserByID(ts.API.db, u.ID)
+				require.NotNil(ts.T(), u)
+			} else {
+				_, err = models.FindUserByEmailAndAudience(ts.API.db, signupParams.Email, ts.Config.JWT.Aud)
+			}
+			require.Equal(ts.T(), c.expected.err, err)
+		})
+	}
 
-	ts.API.handler.ServeHTTP(w, req)
-	require.Equal(ts.T(), http.StatusOK, w.Code)
 }
 
 func (ts *AdminTestSuite) TestAdminUserCreateWithDisabledLogin() {
diff --git a/api/otp_test.go b/api/otp_test.go
index 0ac8b6d9..beea22c3 100644
--- a/api/otp_test.go
+++ b/api/otp_test.go
@@ -131,3 +131,52 @@ func (ts *OtpTestSuite) TestNoSignupsForOtp() {
 		"msg":  "Signups not allowed for otp",
 	})
 }
+
+func (ts *OtpTestSuite) TestSubsequentOtp() {
+	ts.Config.SMTP.MaxFrequency = 0
+	userEmail := "foo@example.com"
+	var buffer bytes.Buffer
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"email": userEmail,
+	}))
+
+	req := httptest.NewRequest(http.MethodPost, "/otp", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w := httptest.NewRecorder()
+
+	ts.API.handler.ServeHTTP(w, req)
+
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	newUser, err := models.FindUserByEmailAndAudience(ts.API.db, userEmail, ts.Config.JWT.Aud)
+	require.NoError(ts.T(), err)
+	require.NotEmpty(ts.T(), newUser.ConfirmationToken)
+	require.NotEmpty(ts.T(), newUser.ConfirmationSentAt)
+	require.Empty(ts.T(), newUser.RecoveryToken)
+	require.Empty(ts.T(), newUser.RecoverySentAt)
+	require.Empty(ts.T(), newUser.EmailConfirmedAt)
+
+	// since the signup process hasn't been completed,
+	// subsequent requests for another magiclink should not create a recovery token
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"email": userEmail,
+	}))
+
+	req = httptest.NewRequest(http.MethodPost, "/otp", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w = httptest.NewRecorder()
+
+	ts.API.handler.ServeHTTP(w, req)
+
+	require.Equal(ts.T(), http.StatusOK, w.Code)
+
+	user, err := models.FindUserByEmailAndAudience(ts.API.db, userEmail, ts.Config.JWT.Aud)
+	require.NoError(ts.T(), err)
+	require.NotEmpty(ts.T(), user.ConfirmationToken)
+	require.NotEmpty(ts.T(), user.ConfirmationSentAt)
+	require.Empty(ts.T(), user.RecoveryToken)
+	require.Empty(ts.T(), user.RecoverySentAt)
+	require.Empty(ts.T(), user.EmailConfirmedAt)
+}
diff --git a/api/verify_test.go b/api/verify_test.go
index 3a3e77e1..ad16ed6d 100644
--- a/api/verify_test.go
+++ b/api/verify_test.go
@@ -658,6 +658,17 @@ func (ts *VerifyTestSuite) TestVerifyValidOtp() {
 			},
 			expected: expectedResponse,
 		},
+		{
+			desc:     "Valid Email OTP",
+			sentTime: time.Now(),
+			body: map[string]interface{}{
+				"type":      emailOTPVerification,
+				"tokenHash": fmt.Sprintf("%x", sha256.Sum224([]byte(u.GetEmail()+"123456"))),
+				"token":     "123456",
+				"email":     u.GetEmail(),
+			},
+			expected: expectedResponse,
+		},
 		{
 			desc:     "Valid Email Change OTP",
 			sentTime: time.Now(),