feat: set email_confirmed_at to null when autoconfirm is on

cmd/admin_cmd.go

@@ -93,11 +93,6 @@ func adminCreateUser(config *conf.GlobalConfiguration, args []string) {
 			}
 		}
 
-		if config.Mailer.Autoconfirm || autoconfirm {
-			if terr = user.Confirm(tx); terr != nil {
-				return terr
-			}
-		}
 		return nil
 	})
 	if err != nil {

internal/api/external.go

@@ -365,7 +365,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
 		return nil, forbiddenError(ErrorCodeUserBanned, "User is banned")
 	}
 
-	if !user.IsConfirmed() {
+	if !user.IsConfirmed(config.Mailer.Autoconfirm) {
 		// The user may have other unconfirmed email + password
 		// combination, phone or oauth identities. These identities
 		// need to be removed when a new oauth identity is being added

internal/api/invite.go

@@ -19,6 +19,8 @@ type InviteParams struct {
 func (a *API) Invite(w http.ResponseWriter, r *http.Request) error {
 	ctx := r.Context()
 	db := a.db.WithContext(ctx)
+	config := a.config
+
 	adminUser := getAdminUser(ctx)
 	params := &InviteParams{}
 	if err := retrieveRequestParams(r, params); err != nil {
@@ -39,7 +41,7 @@ func (a *API) Invite(w http.ResponseWriter, r *http.Request) error {
 
 	err = db.Transaction(func(tx *storage.Connection) error {
 		if user != nil {
-			if user.IsConfirmed() {
+			if user.IsConfirmed(config.Mailer.Autoconfirm) {
 				return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg)
 			}
 		} else {

internal/api/magic_link.go

@@ -75,7 +75,7 @@ func (a *API) MagicLink(w http.ResponseWriter, r *http.Request) error {
 		}
 	}
 	if user != nil {
-		isNewUser = !user.IsConfirmed()
+		isNewUser = !user.IsConfirmed(config.Mailer.Autoconfirm)
 	}
 	if isNewUser {
 		// User either doesn't exist or hasn't completed the signup process.

internal/api/mail.go

@@ -134,7 +134,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error {
 			}
 		case mail.InviteVerification:
 			if user != nil {
-				if user.IsConfirmed() {
+				if user.IsConfirmed( /* autoconfirm: */ false) {
 					return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg)
 				}
 			} else {
@@ -187,7 +187,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error {
 			}
 		case mail.SignupVerification:
 			if user != nil {
-				if user.IsConfirmed() {
+				if user.IsConfirmed( /* autoconfirm: */ false) {
 					return unprocessableEntityError(ErrorCodeEmailExists, DuplicateEmailMsg)
 				}
 				if err := user.UpdateUserMetaData(tx, params.Data); err != nil {

internal/api/reauthenticate.go

@@ -27,7 +27,7 @@ func (a *API) Reauthenticate(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	if email != "" {
-		if !user.IsConfirmed() {
+		if !user.IsConfirmed(config.Mailer.Autoconfirm) {
 			return unprocessableEntityError(ErrorCodeEmailNotConfirmed, "Please verify your email first.")
 		}
 	} else if phone != "" {

internal/api/resend.go

@@ -93,7 +93,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error {
 
 	switch params.Type {
 	case mail.SignupVerification:
-		if user.IsConfirmed() {
+		if user.IsConfirmed( /* autoconfirm: */ false) {
 			// if the user's email is confirmed already, we don't need to send a confirmation email again
 			return sendJSON(w, http.StatusOK, map[string]string{})
 		}

internal/api/signup.go

@@ -187,7 +187,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 	err = db.Transaction(func(tx *storage.Connection) error {
 		var terr error
 		if user != nil {
-			if (params.Provider == "email" && user.IsConfirmed()) || (params.Provider == "phone" && user.IsPhoneConfirmed()) {
+			if (params.Provider == "email" && user.IsConfirmed(config.Mailer.Autoconfirm)) || (params.Provider == "phone" && user.IsPhoneConfirmed()) {
 				return UserExistsError
 			}
 			// do not update the user because we can't be sure of their claimed identity
@@ -221,16 +221,15 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 		}
 		user.Identities = []models.Identity{*identity}
 
-		if params.Provider == "email" && !user.IsConfirmed() {
+		if params.Provider == "email" && !user.IsConfirmed(config.Mailer.Autoconfirm) {
 			if config.Mailer.Autoconfirm {
 				if terr = models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{
 					"provider": params.Provider,
 				}); terr != nil {
 					return terr
 				}
-				if terr = user.Confirm(tx); terr != nil {
-					return internalServerError("Database error updating user").WithInternalError(terr)
-				}
+
+				// note the lack of user.Confirm() call here, as "autoconfirm" no longer sets the email_confirmed_at column, but leaves it to be null
 			} else {
 				if terr = models.NewAuditLogEntry(r, tx, user, models.UserConfirmationRequestedAction, "", map[string]interface{}{
 					"provider": params.Provider,
@@ -313,7 +312,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
 	}
 
 	// handles case where Mailer.Autoconfirm is true or Phone.Autoconfirm is true
-	if user.IsConfirmed() || user.IsPhoneConfirmed() {
+	if user.IsConfirmed(config.Mailer.Autoconfirm) || user.IsPhoneConfirmed() {
 		var token *AccessTokenResponse
 		err = db.Transaction(func(tx *storage.Connection) error {
 			var terr error

internal/api/token.go

@@ -202,7 +202,7 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri
 		return oauthError("invalid_grant", InvalidLoginMessage)
 	}
 
-	if params.Email != "" && !user.IsConfirmed() {
+	if params.Email != "" && !user.IsConfirmed(config.Mailer.Autoconfirm) {
 		return oauthError("invalid_grant", "Email not confirmed")
 	} else if params.Phone != "" && !user.IsPhoneConfirmed() {
 		return oauthError("invalid_grant", "Phone not confirmed")

internal/api/token_test.go

@@ -569,7 +569,7 @@ func (ts *TokenTestSuite) TestMagicLinkPKCESignIn() {
 
 	u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
-	assert.True(ts.T(), u.IsConfirmed())
+	assert.True(ts.T(), u.IsConfirmed(false))
 
 	f, err := url.ParseQuery(rURL.RawQuery)
 	require.NoError(ts.T(), err)

internal/api/verify.go

@@ -346,12 +346,14 @@ func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.C
 }
 
 func (a *API) recoverVerify(r *http.Request, conn *storage.Connection, user *models.User) (*models.User, error) {
+	config := a.config
+
 	err := conn.Transaction(func(tx *storage.Connection) error {
 		var terr error
 		if terr = user.Recover(tx); terr != nil {
 			return terr
 		}
-		if !user.IsConfirmed() {
+		if !user.IsConfirmed(config.Mailer.Autoconfirm) {
 			if terr = models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil {
 				return terr
 			}

internal/api/verify_test.go

@@ -106,7 +106,7 @@ func (ts *VerifyTestSuite) TestVerifyPasswordRecovery() {
 			require.NoError(ts.T(), err)
 
 			assert.WithinDuration(ts.T(), time.Now(), *u.RecoverySentAt, 1*time.Second)
-			assert.False(ts.T(), u.IsConfirmed())
+			assert.False(ts.T(), u.IsConfirmed(false))
 
 			recoveryToken := u.RecoveryToken
 
@@ -119,7 +119,7 @@ func (ts *VerifyTestSuite) TestVerifyPasswordRecovery() {
 
 			u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 			require.NoError(ts.T(), err)
-			assert.True(ts.T(), u.IsConfirmed())
+			assert.True(ts.T(), u.IsConfirmed(false))
 
 			if c.isPKCE {
 				rURL, _ := w.Result().Location()
@@ -212,7 +212,7 @@ func (ts *VerifyTestSuite) TestVerifySecureEmailChange() {
 			require.NoError(ts.T(), err)
 
 			assert.WithinDuration(ts.T(), time.Now(), *u.EmailChangeSentAt, 1*time.Second)
-			assert.False(ts.T(), u.IsConfirmed())
+			assert.False(ts.T(), u.IsConfirmed(false))
 
 			// Verify new email
 			reqURL := fmt.Sprintf("http://localhost/verify?type=%s&token=%s", mail.EmailChangeVerification, newTokenHash)
@@ -478,7 +478,7 @@ func (ts *VerifyTestSuite) TestVerifyPermitedCustomUri() {
 	require.NoError(ts.T(), err)
 
 	assert.WithinDuration(ts.T(), time.Now(), *u.RecoverySentAt, 1*time.Second)
-	assert.False(ts.T(), u.IsConfirmed())
+	assert.False(ts.T(), u.IsConfirmed(false))
 
 	redirectURL, _ := url.Parse(ts.Config.URIAllowList[0])
 
@@ -493,7 +493,7 @@ func (ts *VerifyTestSuite) TestVerifyPermitedCustomUri() {
 
 	u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
-	assert.True(ts.T(), u.IsConfirmed())
+	assert.True(ts.T(), u.IsConfirmed(false))
 }
 
 func (ts *VerifyTestSuite) TestVerifyNotPermitedCustomUri() {
@@ -524,7 +524,7 @@ func (ts *VerifyTestSuite) TestVerifyNotPermitedCustomUri() {
 	require.NoError(ts.T(), err)
 
 	assert.WithinDuration(ts.T(), time.Now(), *u.RecoverySentAt, 1*time.Second)
-	assert.False(ts.T(), u.IsConfirmed())
+	assert.False(ts.T(), u.IsConfirmed(false))
 
 	fakeredirectURL, _ := url.Parse("http://custom-url.com")
 	siteURL, _ := url.Parse(ts.Config.SiteURL)
@@ -540,7 +540,7 @@ func (ts *VerifyTestSuite) TestVerifyNotPermitedCustomUri() {
 
 	u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
-	assert.True(ts.T(), u.IsConfirmed())
+	assert.True(ts.T(), u.IsConfirmed(false))
 }
 
 func (ts *VerifyTestSuite) TestVerifySignupWithRedirectURLContainedPath() {
@@ -671,7 +671,7 @@ func (ts *VerifyTestSuite) TestVerifySignupWithRedirectURLContainedPath() {
 
 			u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 			require.NoError(ts.T(), err)
-			assert.True(ts.T(), u.IsConfirmed())
+			assert.True(ts.T(), u.IsConfirmed(false))
 		})
 	}
 }
@@ -734,7 +734,7 @@ func (ts *VerifyTestSuite) TestVerifyPKCEOTP() {
 
 			u, err = models.FindUserByEmailAndAudience(ts.API.db, "[email protected]", ts.Config.JWT.Aud)
 			require.NoError(ts.T(), err)
-			assert.True(ts.T(), u.IsConfirmed())
+			assert.True(ts.T(), u.IsConfirmed(false))
 
 			f, err := url.ParseQuery(rURL.RawQuery)
 			require.NoError(ts.T(), err)

internal/conf/configuration.go

@@ -327,6 +327,7 @@ func (c *SMTPConfiguration) Validate() error {
 }
 
 type MailerConfiguration struct {
+	// Autoconfirm actually means allow unverified email sign-ups.
 	Autoconfirm                 bool `json:"autoconfirm"`
 	AllowUnverifiedEmailSignIns bool `json:"allow_unverified_email_sign_ins" split_words:"true" default:"false"`
 

internal/models/user.go

@@ -179,8 +179,8 @@ func (u *User) BeforeSave(tx *pop.Connection) error {
 
 // IsConfirmed checks if a user has already been
 // registered and confirmed.
-func (u *User) IsConfirmed() bool {
-	return u.EmailConfirmedAt != nil
+func (u *User) IsConfirmed(autoconfirm bool) bool {
+	return autoconfirm || u.EmailConfirmedAt != nil
 }
 
 // HasBeenInvited checks if user has been invited
@@ -488,7 +488,7 @@ func (u *User) ConfirmEmailChange(tx *storage.Connection, status int) error {
 		return err
 	}
 
-	if !u.IsConfirmed() {
+	if !u.IsConfirmed( /* autoconfirm: */ false) {
 		if err := u.Confirm(tx); err != nil {
 			return err
 		}

internal/models/user_test.go

@@ -234,7 +234,7 @@ func (ts *UserTestSuite) TestRemoveUnconfirmedIdentities() {
 	// reload the user
 	require.NoError(ts.T(), ts.db.Load(user))
 
-	require.False(ts.T(), user.IsConfirmed(), "user's email must not be confirmed")
+	require.False(ts.T(), user.IsConfirmed(false), "user's email must not be confirmed")
 
 	require.NoError(ts.T(), user.RemoveUnconfirmedIdentities(ts.db, idTwitter))