diff --git a/cmd/admin_cmd.go b/cmd/admin_cmd.go index 7997bb5c52..1f0d25285b 100644 --- a/cmd/admin_cmd.go +++ b/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 { diff --git a/internal/api/external.go b/internal/api/external.go index ef6032d9a7..2e6e1e2836 100644 --- a/internal/api/external.go +++ b/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 diff --git a/internal/api/invite.go b/internal/api/invite.go index 2e07e71355..2808ac54bc 100644 --- a/internal/api/invite.go +++ b/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 { diff --git a/internal/api/magic_link.go b/internal/api/magic_link.go index eeabafd39e..342fc9002a 100644 --- a/internal/api/magic_link.go +++ b/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. diff --git a/internal/api/mail.go b/internal/api/mail.go index 30f358ad29..6708099a87 100644 --- a/internal/api/mail.go +++ b/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 { diff --git a/internal/api/reauthenticate.go b/internal/api/reauthenticate.go index cf86d102fb..afe31817ec 100644 --- a/internal/api/reauthenticate.go +++ b/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 != "" { diff --git a/internal/api/resend.go b/internal/api/resend.go index b9e16df512..701dbf5d67 100644 --- a/internal/api/resend.go +++ b/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{}) } diff --git a/internal/api/signup.go b/internal/api/signup.go index b396178db2..9420e474da 100644 --- a/internal/api/signup.go +++ b/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 diff --git a/internal/api/token.go b/internal/api/token.go index 3362faa937..2d7eab7cab 100644 --- a/internal/api/token.go +++ b/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") diff --git a/internal/api/token_test.go b/internal/api/token_test.go index 53a492b11d..083c54facf 100644 --- a/internal/api/token_test.go +++ b/internal/api/token_test.go @@ -569,7 +569,7 @@ func (ts *TokenTestSuite) TestMagicLinkPKCESignIn() { u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", 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) diff --git a/internal/api/verify.go b/internal/api/verify.go index 97a13b93bc..b8ffb9c4d8 100644 --- a/internal/api/verify.go +++ b/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 } diff --git a/internal/api/verify_test.go b/internal/api/verify_test.go index 73ef6f768e..558a63da31 100644 --- a/internal/api/verify_test.go +++ b/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, "test@example.com", 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, "test@example.com", 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, "test@example.com", 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, "test@example.com", 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, "test@example.com", 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) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 35024ea52e..8d209e1c0a 100644 --- a/internal/conf/configuration.go +++ b/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"` diff --git a/internal/models/user.go b/internal/models/user.go index 3c871e5430..fc9ee561da 100644 --- a/internal/models/user.go +++ b/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 } diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 47d16178de..8b1d63907f 100644 --- a/internal/models/user_test.go +++ b/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))