fix: redirects on email change

openware · Sep 7, 2021 · d9cba5b · d9cba5b
1 parent 6f75827
commit d9cba5b
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 8 deletions.
diff --git a/api/mail.go b/api/mail.go
@@ -232,13 +232,21 @@ func (a *API) sendMagicLink(tx *storage.Connection, u *models.User, mailer maile
 func (a *API) sendEmailChange(tx *storage.Connection, u *models.User, mailer mailer.Mailer, email string, referrerURL string) error {
 	u.EmailChangeTokenCurrent, u.EmailChangeTokenNew = crypto.SecureToken(), crypto.SecureToken()
 	u.EmailChange = email
+	u.EmailChangeConfirmStatus = noneConfirmed
 	now := time.Now()
 	if err := mailer.EmailChangeMail(u, referrerURL); err != nil {
 		return err
 	}
 
 	u.EmailChangeSentAt = &now
-	return errors.Wrap(tx.UpdateOnly(u, "email_change_token_current", "email_change_token_new", "email_change", "email_change_sent_at"), "Database error updating user for email change")
+	return errors.Wrap(tx.UpdateOnly(
+		u,
+		"email_change_token_current",
+		"email_change_token_new",
+		"email_change",
+		"email_change_sent_at",
+		"email_change_confirm_status",
+	), "Database error updating user for email change")
 }
 
 func (a *API) validateEmail(ctx context.Context, email string) error {

diff --git a/api/verify.go b/api/verify.go
@@ -28,6 +28,11 @@ const (
 	smsVerification         = "sms"
 )
 
+const (
+	noneConfirmed int = iota
+	oneConfirmed
+)
+
 // VerifyParams are the parameters the Verify endpoint accepts
 type VerifyParams struct {
 	Type       string `json:"type"`
@@ -306,7 +311,7 @@ func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, p
 	nextDay := user.EmailChangeSentAt.Add(24 * time.Hour)
 	if user.EmailChangeSentAt != nil && time.Now().After(nextDay) {
 		err = a.db.Transaction(func(tx *storage.Connection) error {
-			user.EmailChangeConfirmStatus = 0
+			user.EmailChangeConfirmStatus = noneConfirmed
 			return tx.UpdateOnly(user, "email_change_confirm_status")
 		})
 		if err != nil {
@@ -315,9 +320,9 @@ func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, p
 		return nil, expiredTokenError("Email change token expired").WithInternalError(redirectWithQueryError)
 	}
 
-	if user.EmailChangeConfirmStatus < 1 {
+	if user.EmailChangeConfirmStatus == noneConfirmed {
 		err = a.db.Transaction(func(tx *storage.Connection) error {
-			user.EmailChangeConfirmStatus++
+			user.EmailChangeConfirmStatus = oneConfirmed
 			if params.Token == user.EmailChangeTokenCurrent {
 				user.EmailChangeTokenCurrent = ""
 			} else if params.Token == user.EmailChangeTokenNew {
@@ -331,9 +336,10 @@ func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, p
 		if err != nil {
 			return nil, err
 		}
-		return nil, acceptedTokenError("Email change request accepted")
+		return nil, acceptedTokenError("Email change request accepted").WithInternalError(redirectWithQueryError)
 	}
 
+	// one email is confirmed at this point
 	err = a.db.Transaction(func(tx *storage.Connection) error {
 		var terr error
 
@@ -345,7 +351,7 @@ func (a *API) emailChangeVerify(ctx context.Context, conn *storage.Connection, p
 			return terr
 		}
 
-		if terr = user.ConfirmEmailChange(tx); terr != nil {
+		if terr = user.ConfirmEmailChange(tx, noneConfirmed); terr != nil {
 			return internalServerError("Error confirm email").WithInternalError(terr)
 		}
 

diff --git a/models/user.go b/models/user.go
@@ -279,12 +279,12 @@ func (u *User) UpdateLastSignInAt(tx *storage.Connection) error {
 }
 
 // ConfirmEmailChange confirm the change of email for a user
-func (u *User) ConfirmEmailChange(tx *storage.Connection) error {
+func (u *User) ConfirmEmailChange(tx *storage.Connection, status int) error {
 	u.Email = storage.NullString(u.EmailChange)
 	u.EmailChange = ""
 	u.EmailChangeTokenCurrent = ""
 	u.EmailChangeTokenNew = ""
-	u.EmailChangeConfirmStatus = 0
+	u.EmailChangeConfirmStatus = status
 	return tx.UpdateOnly(
 		u,
 		"email",