diff --git a/docs/releases.md b/docs/releases.md index d0551d6b..30490217 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1958,7 +1958,7 @@ email. All of this rides on the existing SMTP configuration -- no new config fla **Features:** * Add password reset via emailed magic link, with a "Forgot password?" link on the login page and a `ntfy user reset-pass` CLI command for admins -* Rework email verification to use durable, single-use, expiring magic links instead of in-memory 6-digit codes, and add a "primary" (recovery) email with verified/unverified state in the account UI +* Rework email verification to use durable, single-use, expiring magic links instead of in-memory 6-digit codes, and add a "primary" email (used for account recovery and as the `X-Email: yes` target) with verified/unverified state in the account UI * Auto-send a verification link to the billing email after a Stripe checkout, so paying users can set up password recovery **Bug fixes + maintenance:** diff --git a/server/server_account.go b/server/server_account.go index ac79b05a..e28df08d 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -735,8 +735,6 @@ func (s *Server) handleAccountEmailSetPrimary(w http.ResponseWriter, r *http.Req return err } else if !emailAddressRegex.MatchString(req.Email) { return errHTTPBadRequestEmailAddressInvalid - } else if u.Provisioned { - return errHTTPConflictProvisionedUserChange // Provisioned users can't reset, so a recovery email is meaningless } logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Setting primary email") err = s.userManager.SetPrimaryEmail(u.ID, req.Email) diff --git a/server/server_account_email_test.go b/server/server_account_email_test.go index 88ebf1cb..793c89be 100644 --- a/server/server_account_email_test.go +++ b/server/server_account_email_test.go @@ -364,7 +364,7 @@ func TestAccount_Signup_WithoutEmail_NoSend(t *testing.T) { }) } -func TestAccount_Email_ProvisionedNoPrimary(t *testing.T) { +func TestAccount_Email_ProvisionedPrimary(t *testing.T) { forEachBackend(t, func(t *testing.T, databaseURL string) { hash, err := user.HashPassword("provpass", user.DefaultUserPasswordBcryptCost) require.Nil(t, err) @@ -379,16 +379,19 @@ func TestAccount_Email_ProvisionedNoPrimary(t *testing.T) { defer s.closeDatabases() auth := map[string]string{"Authorization": util.BasicAuth("prov", "provpass")} - // A provisioned user can verify an email, but it must NOT become their primary + // A provisioned user's first verified email becomes their primary (used by X-Email: yes; + // password reset stays blocked separately for provisioned users) verifyEmailFor(t, s, mailer, auth, "prov@example.com") account := getAccount(t, s, auth) require.Equal(t, []string{"prov@example.com"}, verifiedAddrs(account)) - require.Equal(t, "", primaryAddr(account)) + require.Equal(t, "prov@example.com", primaryAddr(account)) - // Explicitly setting it primary is rejected - rr := request(t, s, "POST", "/v1/account/email/primary", `{"email":"prov@example.com"}`, auth) - require.Equal(t, 409, rr.Code) - require.Equal(t, 40905, toHTTPError(t, rr.Body.String()).Code) + // Verify a second address and explicitly set it primary -> allowed, star moves + verifyEmailFor(t, s, mailer, auth, "prov2@example.com") + rr := request(t, s, "POST", "/v1/account/email/primary", `{"email":"prov2@example.com"}`, auth) + require.Equal(t, 200, rr.Code) + account = getAccount(t, s, auth) + require.Equal(t, "prov2@example.com", primaryAddr(account)) }) } diff --git a/server/server_test.go b/server/server_test.go index eaa7f350..5ebde045 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1728,6 +1728,33 @@ func TestServer_PublishEmailVerify_BoolValueAnonymousRejected(t *testing.T) { }) } +func TestServer_PublishEmailVerify_BoolValueProvisionedUsesPrimary(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + hash, err := user.HashPassword("provpass", user.DefaultUserPasswordBcryptCost) + require.Nil(t, err) + conf := newTestConfigWithAuthFile(t, databaseURL) + conf.AuthUsers = []*user.User{{Name: "prov", Hash: hash, Role: user.RoleUser}} + s := newTestServer(t, conf) + mailer := &testMailer{} + s.mailer = mailer + defer s.closeDatabases() + + prov, err := s.userManager.User("prov") + require.Nil(t, err) + require.Nil(t, s.userManager.AddEmail(prov.ID, "aaa@example.com")) + require.Nil(t, s.userManager.AddEmail(prov.ID, "zzz@example.com")) + require.Nil(t, s.userManager.SetPrimaryEmail(prov.ID, "zzz@example.com")) + + // A provisioned user's "yes" resolves to their chosen primary, not the alphabetically-first + response := request(t, s, "PUT", "/mytopic", "hi", map[string]string{ + "Email": "yes", + "Authorization": util.BasicAuth("prov", "provpass"), + }) + require.Equal(t, 200, response.Code) + require.Equal(t, "zzz@example.com", mailer.LastTo()) + }) +} + func TestServer_PublishEmailVerify_Anonymous(t *testing.T) { forEachBackend(t, func(t *testing.T, databaseURL string) { conf := newTestConfigWithAuthFile(t, databaseURL) diff --git a/user/manager.go b/user/manager.go index 6378ab60..c8b1d39e 100644 --- a/user/manager.go +++ b/user/manager.go @@ -1669,10 +1669,6 @@ func (a *Manager) VerifyEmail(rawToken string) (*MagicLink, error) { if m.Kind != MagicLinkKindEmailVerify || time.Now().Unix() > m.Expires { return nil, ErrMagicLinkNotFound } - u, err := a.UserByID(m.UserID) - if err != nil { - return nil, err - } err = db.ExecTx(a.db, func(tx *sql.Tx) error { // Single use: delete the link, then add the (idempotent) verified address if _, err := tx.Exec(a.queries.deleteMagicLinkByHash, tokenHash); err != nil { @@ -1681,10 +1677,6 @@ func (a *Manager) VerifyEmail(rawToken string) (*MagicLink, error) { if _, err := tx.Exec(a.queries.insertEmailIgnore, m.UserID, m.Email); err != nil { return err } - // Must stay before the promotion block; covered by TestUser_MagicLink_VerifyEmail_ProvisionedNoPrimary - if u.Provisioned { - return nil // Provisioned users don't get a primary (recovery) email - } // Promote to primary only if the user has none yet and the address is globally free. // We check with SELECTs rather than catching a unique violation, because Postgres aborts // the whole transaction on any constraint error (which would undo the verified-email add). diff --git a/user/manager_test.go b/user/manager_test.go index c6dd2467..e7073773 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -3212,7 +3212,7 @@ func TestUser_MagicLink_ResetPassword_WrongKindRejected(t *testing.T) { }) } -func TestUser_MagicLink_VerifyEmail_ProvisionedNoPrimary(t *testing.T) { +func TestUser_MagicLink_VerifyEmail_ProvisionedGetsPrimary(t *testing.T) { forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { a := newTestManagerFromConfig(t, newManager, &Config{ DefaultAccess: PermissionDenyAll, @@ -3224,7 +3224,8 @@ func TestUser_MagicLink_VerifyEmail_ProvisionedNoPrimary(t *testing.T) { prov, err := a.User("prov") require.Nil(t, err) - // A provisioned user can verify an email (for notifications), but it must NOT become primary + // A provisioned user's first verified email becomes their primary, just like a regular user + // (the primary is also the X-Email: yes target; password reset stays blocked separately). _, err = a.VerifyEmail(addVerifyLink(t, a, prov.ID, "prov@example.com", time.Hour)) require.Nil(t, err) @@ -3233,7 +3234,7 @@ func TestUser_MagicLink_VerifyEmail_ProvisionedNoPrimary(t *testing.T) { require.Equal(t, []string{"prov@example.com"}, emails) primary, err := a.PrimaryEmail(prov.ID) require.Nil(t, err) - require.Equal(t, "", primary) + require.Equal(t, "prov@example.com", primary) }) } diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json index d24e9936..a6e942a0 100644 --- a/web/public/static/langs/en.json +++ b/web/public/static/langs/en.json @@ -244,7 +244,7 @@ "account_basics_emails_description": "For email notifications and password reset", "account_basics_emails_no_emails_yet": "No emails yet", "account_basics_emails_copied_to_clipboard": "Email address copied to clipboard", - "account_basics_emails_chip_actions_primary": "Primary address, can be used for account recovery and notifications. Click for actions.", + "account_basics_emails_chip_actions_primary": "Primary address, used as your default email address. Click for actions.", "account_basics_emails_chip_actions_verified": "Can be used for notifications. Click for actions.", "account_basics_emails_chip_actions_unverified": "Unverified address, check your inbox to verify. Click for actions.", "account_basics_emails_unverified": "unverified", @@ -255,7 +255,6 @@ "account_basics_emails_primary_elsewhere": "This email address is used as the primary address on another account", "account_basics_emails_no_recovery_warning": "Add at least one email address to ensure you can recover your account if you lose your password.", "account_basics_emails_no_primary_warning": "Add a primary email address to ensure you can recover your account if you lose your password.", - "account_basics_emails_provisioned_info": "Provisioned users cannot add a primary email address, but you can still add an email address for notifications.", "account_basics_emails_dialog_title": "Add email address", "account_basics_emails_dialog_description": "Enter an email address to add it to your account. A verification link will be sent to confirm it is yours.", "account_basics_emails_dialog_email_label": "Email address", diff --git a/web/src/components/Account.jsx b/web/src/components/Account.jsx index f225cc81..24631dc9 100644 --- a/web/src/components/Account.jsx +++ b/web/src/components/Account.jsx @@ -521,7 +521,7 @@ const Emails = () => { {t("common_copy_to_clipboard")} - {menuEmail && !menuEmail.pending && !menuEmail.primary && !account?.provisioned && ( + {menuEmail && !menuEmail.pending && !menuEmail.primary && ( runMenuAction(handleSetPrimary)}> @@ -555,7 +555,6 @@ const Emails = () => { const AddEmailDialog = (props) => { const theme = useTheme(); const { t } = useTranslation(); - const { account } = useContext(AccountContext); const [error, setError] = useState(""); const [email, setEmail] = useState(""); const [sending, setSending] = useState(false); @@ -592,11 +591,6 @@ const AddEmailDialog = (props) => { ) : ( <> {t("account_basics_emails_dialog_description")} - {config.enable_reset_password && account?.provisioned && ( - - {t("account_basics_emails_provisioned_info")} - - )}