Lots of refinement

This commit is contained in:
binwiederhier
2026-06-12 17:36:40 -04:00
parent 33ae31055c
commit 4516adea36
16 changed files with 460 additions and 199 deletions
+18 -10
View File
@@ -167,20 +167,24 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, r *http.Request, v *vis
if err != nil {
return err
}
if len(emails) > 0 {
response.Emails = emails
}
primaryEmail, err := s.userManager.PrimaryEmail(u.ID)
if err != nil {
return err
}
response.PrimaryEmail = primaryEmail
pendingEmails, err := s.userManager.PendingEmails(u.ID)
if err != nil {
return err
}
if len(pendingEmails) > 0 {
response.PendingEmails = pendingEmails
// Combine verified (with primary flag) and pending (unverified) into one list
emailInfos := make([]*apiAccountEmailInfo, 0, len(emails)+len(pendingEmails))
for _, email := range emails {
emailInfos = append(emailInfos, &apiAccountEmailInfo{Address: email, Primary: email == primaryEmail})
}
for _, email := range pendingEmails {
emailInfos = append(emailInfos, &apiAccountEmailInfo{Address: email, Pending: true})
}
if len(emailInfos) > 0 {
response.Emails = emailInfos
}
}
} else {
@@ -718,6 +722,8 @@ 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)
@@ -810,13 +816,15 @@ func (s *Server) handleAccountPasswordResetRequest(w http.ResponseWriter, r *htt
// The identifier is tried first as a username, then as a primary email address. It returns
// ok=false if no account with a primary email matches (reset requires a verified primary email).
func (s *Server) resolveResetTarget(identifier string) (userID string, email string, ok bool) {
if u, err := s.userManager.User(identifier); err == nil && u != nil {
if u, err := s.userManager.User(identifier); err == nil && u != nil && !u.Provisioned {
if primary, perr := s.userManager.PrimaryEmail(u.ID); perr == nil && primary != "" {
return u.ID, primary, true
}
}
if uid, err := s.userManager.UserIDByPrimaryEmail(identifier); err == nil {
return uid, identifier, true
if u, uerr := s.userManager.UserByID(uid); uerr == nil && !u.Provisioned {
return uid, identifier, true
}
}
return "", "", false
}
@@ -834,8 +842,8 @@ func (s *Server) handleAccountPasswordReset(w http.ResponseWriter, r *http.Reque
return errHTTPBadRequest
}
err = s.userManager.ResetPassword(req.Token, req.Password)
if errors.Is(err, user.ErrMagicLinkNotFound) {
return errHTTPBadRequestResetLinkInvalid
if errors.Is(err, user.ErrMagicLinkNotFound) || errors.Is(err, user.ErrProvisionedUserChange) {
return errHTTPBadRequestResetLinkInvalid // Generic 400 (provisioned users can't be reset; don't leak that)
} else if err != nil {
return err
}
+100 -12
View File
@@ -57,6 +57,37 @@ func getAccount(t *testing.T, s *Server, auth map[string]string) *apiAccountResp
return account
}
// verifiedAddrs / pendingAddrs / primaryAddr extract the addresses from the structured email
// list returned by GET /v1/account, so assertions stay readable.
func verifiedAddrs(account *apiAccountResponse) []string {
addrs := make([]string, 0)
for _, e := range account.Emails {
if !e.Pending {
addrs = append(addrs, e.Address)
}
}
return addrs
}
func pendingAddrs(account *apiAccountResponse) []string {
addrs := make([]string, 0)
for _, e := range account.Emails {
if e.Pending {
addrs = append(addrs, e.Address)
}
}
return addrs
}
func primaryAddr(account *apiAccountResponse) string {
for _, e := range account.Emails {
if e.Primary {
return e.Address
}
}
return ""
}
func tokenFromLink(t *testing.T, link, prefix string) string {
require.True(t, strings.HasPrefix(link, prefix), "link %q missing prefix %q", link, prefix)
return strings.TrimPrefix(link, prefix)
@@ -73,9 +104,9 @@ func TestAccount_Email_AddVerifySetsPrimary(t *testing.T) {
// Pending, not yet verified, no primary
account := getAccount(t, s, auth)
require.Equal(t, []string{"ben@example.com"}, account.PendingEmails)
require.Empty(t, account.Emails)
require.Equal(t, "", account.PrimaryEmail)
require.Equal(t, []string{"ben@example.com"}, pendingAddrs(account))
require.Empty(t, verifiedAddrs(account))
require.Equal(t, "", primaryAddr(account))
// "Click" the captured link (unauthenticated POST)
token := tokenFromLink(t, mailer.verifyLinks["ben@example.com"], "https://ntfy.example.com/account/email/verify/")
@@ -84,9 +115,9 @@ func TestAccount_Email_AddVerifySetsPrimary(t *testing.T) {
// Now verified + primary, no longer pending
account = getAccount(t, s, auth)
require.Equal(t, []string{"ben@example.com"}, account.Emails)
require.Equal(t, "ben@example.com", account.PrimaryEmail)
require.Empty(t, account.PendingEmails)
require.Equal(t, []string{"ben@example.com"}, verifiedAddrs(account))
require.Equal(t, "ben@example.com", primaryAddr(account))
require.Empty(t, pendingAddrs(account))
})
}
@@ -111,13 +142,13 @@ func TestAccount_Email_DeletePending(t *testing.T) {
defer s.closeDatabases()
require.Equal(t, 200, request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, auth).Code)
require.Equal(t, []string{"ben@example.com"}, getAccount(t, s, auth).PendingEmails)
require.Equal(t, []string{"ben@example.com"}, pendingAddrs(getAccount(t, s, auth)))
// Deleting the pending address clears it (no verification ever happened)
require.Equal(t, 200, request(t, s, "DELETE", "/v1/account/email", `{"email":"ben@example.com"}`, auth).Code)
account := getAccount(t, s, auth)
require.Empty(t, account.PendingEmails)
require.Empty(t, account.Emails)
require.Empty(t, pendingAddrs(account))
require.Empty(t, verifiedAddrs(account))
})
}
@@ -154,7 +185,7 @@ func TestAccount_Email_SetPrimaryCollision(t *testing.T) {
require.Equal(t, 200, request(t, s, "PUT", "/v1/account/email", `{"email":"shared@example.com"}`, auth).Code)
benToken := tokenFromLink(t, mailer.verifyLinks["shared@example.com"], "https://ntfy.example.com/account/email/verify/")
require.Equal(t, 200, request(t, s, "POST", "/v1/account/email/verify", fmt.Sprintf(`{"token":"%s"}`, benToken), nil).Code)
require.Equal(t, "shared@example.com", getAccount(t, s, auth).PrimaryEmail)
require.Equal(t, "shared@example.com", primaryAddr(getAccount(t, s, auth)))
// alice verifies the same address -> allowed as secondary, but it is not her primary
require.Nil(t, s.userManager.AddUser("alice", "alice", user.RoleUser, false))
@@ -163,8 +194,8 @@ func TestAccount_Email_SetPrimaryCollision(t *testing.T) {
aliceToken := tokenFromLink(t, mailer.verifyLinks["shared@example.com"], "https://ntfy.example.com/account/email/verify/")
require.Equal(t, 200, request(t, s, "POST", "/v1/account/email/verify", fmt.Sprintf(`{"token":"%s"}`, aliceToken), nil).Code)
aliceAccount := getAccount(t, s, aliceAuth)
require.Equal(t, []string{"shared@example.com"}, aliceAccount.Emails)
require.Equal(t, "", aliceAccount.PrimaryEmail)
require.Equal(t, []string{"shared@example.com"}, verifiedAddrs(aliceAccount))
require.Equal(t, "", primaryAddr(aliceAccount))
// alice trying to promote it to primary collides with ben's
rr := request(t, s, "POST", "/v1/account/email/primary", `{"email":"shared@example.com"}`, aliceAuth)
@@ -245,6 +276,63 @@ func TestAccount_PasswordReset_NoPrimaryEmailNoSend(t *testing.T) {
})
}
func TestAccount_Email_ProvisionedNoPrimary(t *testing.T) {
forEachBackend(t, func(t *testing.T, databaseURL string) {
hash, err := user.HashPassword("provpass")
require.Nil(t, err)
conf := newTestConfigWithAuthFile(t, databaseURL)
conf.SMTPSenderAddr = "localhost:25"
conf.SMTPSenderFrom = "noreply@example.com"
conf.BaseURL = "https://ntfy.example.com"
conf.AuthUsers = []*user.User{{Name: "prov", Hash: hash, Role: user.RoleUser}}
s := newTestServer(t, conf)
mailer := newCaptureMailer()
s.mailSender = mailer
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
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))
// 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)
})
}
func TestAccount_PasswordReset_ProvisionedUserNoSend(t *testing.T) {
forEachBackend(t, func(t *testing.T, databaseURL string) {
// Provision a user via config (AuthUsers), with email sending enabled
conf := newTestConfigWithAuthFile(t, databaseURL)
conf.SMTPSenderAddr = "localhost:25"
conf.SMTPSenderFrom = "noreply@example.com"
conf.BaseURL = "https://ntfy.example.com"
conf.AuthUsers = []*user.User{
{Name: "prov", Hash: "$2a$10$YLiO8U21sX1uhZamTLJXHuxgVC0Z/GKISibrKCLohPgtG7yIxSk4C", Role: user.RoleUser},
}
s := newTestServer(t, conf)
mailer := newCaptureMailer()
s.mailSender = mailer
defer s.closeDatabases()
// Give the provisioned user a verified primary email anyway
prov, err := s.userManager.User("prov")
require.Nil(t, err)
require.True(t, prov.Provisioned)
require.Nil(t, s.userManager.AddEmail(prov.ID, "prov@example.com"))
require.Nil(t, s.userManager.SetPrimaryEmail(prov.ID, "prov@example.com"))
// Reset request by username and by email -> uniform 200, but no email sent (can't reset)
require.Equal(t, 200, request(t, s, "POST", "/v1/account/password/reset/request", `{"identifier":"prov"}`, nil).Code)
require.Equal(t, 200, request(t, s, "POST", "/v1/account/password/reset/request", `{"identifier":"prov@example.com"}`, nil).Code)
require.Empty(t, mailer.resetLinks)
})
}
func TestAccount_PasswordReset_InvalidToken(t *testing.T) {
forEachBackend(t, func(t *testing.T, databaseURL string) {
s, _, _ := newEmailTestServer(t, databaseURL)
+2 -1
View File
@@ -238,7 +238,8 @@ func (s *Server) handleAccountBillingSubscriptionCreateSuccess(w http.ResponseWr
return err
}
// Offer email recovery: auto-send a verification link to the billing email (best-effort).
if sess.CustomerDetails != nil {
// Provisioned users can't reset their password, so recovery setup doesn't apply to them.
if sess.CustomerDetails != nil && !u.Provisioned {
s.maybeEnqueueBillingEmailVerification(r, v, u.ID, sess.CustomerDetails.Email)
}
http.Redirect(w, r, s.config.BaseURL+accountPath, http.StatusSeeOther)
+10 -3
View File
@@ -287,6 +287,15 @@ type apiAccountReservation struct {
Everyone string `json:"everyone"`
}
// apiAccountEmailInfo describes one email address on the account, as returned by GET /v1/account.
// Verified addresses have pending=false; exactly one verified address may be primary (the
// recovery email). Pending addresses are awaiting a magic-link click and are never primary.
type apiAccountEmailInfo struct {
Address string `json:"address"`
Primary bool `json:"primary,omitempty"`
Pending bool `json:"pending,omitempty"`
}
type apiAccountBilling struct {
Customer bool `json:"customer"`
Subscription bool `json:"subscription"`
@@ -307,9 +316,7 @@ type apiAccountResponse struct {
Reservations []*apiAccountReservation `json:"reservations,omitempty"`
Tokens []*apiAccountTokenResponse `json:"tokens,omitempty"`
PhoneNumbers []string `json:"phone_numbers,omitempty"`
Emails []string `json:"emails,omitempty"`
PrimaryEmail string `json:"primary_email,omitempty"` // The verified recovery email, if set
PendingEmails []string `json:"pending_emails,omitempty"` // Unverified addresses awaiting a magic-link click
Emails []*apiAccountEmailInfo `json:"emails,omitempty"`
Tier *apiAccountTier `json:"tier,omitempty"`
Limits *apiAccountLimits `json:"limits,omitempty"`
Stats *apiAccountStats `json:"stats,omitempty"`