diff --git a/mail/sender.go b/mail/sender.go index 5511cb04..b840adf1 100644 --- a/mail/sender.go +++ b/mail/sender.go @@ -6,17 +6,14 @@ import ( "net" "net/smtp" "strings" - "sync" "time" "heckel.io/ntfy/v2/log" - "heckel.io/ntfy/v2/util" ) const ( - verifyCodeExpiry = 10 * time.Minute - verifyCodeLength = 6 - verifyCodeSubject = "ntfy email verification" + emailVerificationSubject = "Verify your email for ntfy" + passwordResetSubject = "Reset your ntfy password" ) // Config holds the SMTP configuration for the mail sender @@ -27,33 +24,22 @@ type Config struct { From string // Sender email address } -// Sender sends emails and manages email verification codes +// Sender sends emails via SMTP, including the magic-link emails for email verification and +// password reset. Pending verification/reset state lives in the database (see user.Manager), +// not in this struct. type Sender struct { - config *Config - codes map[string]verifyCode // Verification codes, keyed by email - mu sync.Mutex - closeChan chan struct{} -} - -type verifyCode struct { - code string - expires time.Time + config *Config } // NewSender creates a new mail Sender with the given SMTP config func NewSender(config *Config) *Sender { - s := &Sender{ - config: config, - codes: make(map[string]verifyCode), - closeChan: make(chan struct{}), - } - go s.expireLoop() - return s + return &Sender{config: config} } -// Close stops the background expiry loop +// Close is a no-op, kept so callers don't need to special-case the sender. The sender holds +// no background goroutines (magic-link expiry is swept by the user.Manager reaper). func (s *Sender) Close() { - close(s.closeChan) + // Nothing to do } // Addr returns the SMTP server address @@ -104,51 +90,24 @@ Content-Type: text/plain; charset="utf-8" return s.SendRaw(to, []byte(message)) } -// SendVerification generates a random code, stores it in-memory, and sends a verification email -func (s *Sender) SendVerification(to string) error { - code := util.RandomString(verifyCodeLength) - s.mu.Lock() - s.codes[to] = verifyCode{ - code: code, - expires: time.Now().Add(verifyCodeExpiry), - } - s.mu.Unlock() - body := fmt.Sprintf("Your ntfy email verification code is: %s\n\nThis code expires in 10 minutes.", code) - return s.Send(to, verifyCodeSubject, body) +// SendEmailVerification sends an email containing a magic link to verify ownership of the +// recipient address. The link carries a one-time token validated against the database. +func (s *Sender) SendEmailVerification(to, link string) error { + body := fmt.Sprintf(`Click the link below to verify this email address for your ntfy account: + +%s + +This link expires in 24 hours. If you did not request this, you can safely ignore this email.`, link) + return s.Send(to, emailVerificationSubject, body) } -// CheckVerification checks if the code matches and hasn't expired. Removes the entry on success. -func (s *Sender) CheckVerification(email, code string) bool { - s.mu.Lock() - defer s.mu.Unlock() - vc, ok := s.codes[email] - if !ok || time.Now().After(vc.expires) || vc.code != code { - return false - } - delete(s.codes, email) - return true -} +// SendPasswordReset sends an email containing a magic link to set a new password. The link +// carries a one-time token validated against the database. +func (s *Sender) SendPasswordReset(to, link string) error { + body := fmt.Sprintf(`Click the link below to set a new password for your ntfy account: -func (s *Sender) expireLoop() { - ticker := time.NewTicker(time.Minute) - defer ticker.Stop() - for { - select { - case <-ticker.C: - s.expireVerificationCodes() - case <-s.closeChan: - return - } - } -} +%s -func (s *Sender) expireVerificationCodes() { - s.mu.Lock() - defer s.mu.Unlock() - now := time.Now() - for email, vc := range s.codes { - if now.After(vc.expires) { - delete(s.codes, email) - } - } +This link expires in 1 hour. If you did not request this, you can safely ignore this email -- your password will not change.`, link) + return s.Send(to, passwordResetSubject, body) } diff --git a/server/errors.go b/server/errors.go index 3197d6ca..5ac6ce38 100644 --- a/server/errors.go +++ b/server/errors.go @@ -143,7 +143,7 @@ var ( errHTTPBadRequestTemplateFileInvalid = &errHTTP{40048, http.StatusBadRequest, "invalid request: template file invalid", "https://ntfy.sh/docs/publish/#message-templating", nil} errHTTPBadRequestSequenceIDInvalid = &errHTTP{40049, http.StatusBadRequest, "invalid request: sequence ID invalid", "https://ntfy.sh/docs/publish/#updating-deleting-notifications", nil} errHTTPBadRequestEmailAddressInvalid = &errHTTP{40050, http.StatusBadRequest, "invalid request: invalid e-mail address", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil} - errHTTPBadRequestEmailVerificationCodeInvalid = &errHTTP{40051, http.StatusBadRequest, "invalid request: email verification code invalid or expired", "", nil} + errHTTPBadRequestEmailVerificationCodeInvalid = &errHTTP{40051, http.StatusBadRequest, "invalid request: email verification link invalid or expired", "", nil} errHTTPBadRequestEmailAddressNotVerified = &errHTTP{40052, http.StatusBadRequest, "invalid request: email address not verified", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil} errHTTPBadRequestAnonymousEmailNotAllowed = &errHTTP{40053, http.StatusBadRequest, "invalid request: anonymous email sending is not allowed", "https://ntfy.sh/docs/publish/#e-mail-notifications", nil} errHTTPNotFound = &errHTTP{40401, http.StatusNotFound, "page not found", "", nil} @@ -156,6 +156,7 @@ var ( errHTTPConflictProvisionedUserChange = &errHTTP{40905, http.StatusConflict, "conflict: cannot change or delete provisioned user", "", nil} errHTTPConflictProvisionedTokenChange = &errHTTP{40906, http.StatusConflict, "conflict: cannot change or delete provisioned token", "", nil} errHTTPConflictEmailExists = &errHTTP{40907, http.StatusConflict, "conflict: email address already exists", "", nil} + errHTTPConflictEmailPrimaryElsewhere = &errHTTP{40908, http.StatusConflict, "conflict: email address is the recovery email on another account", "", nil} errHTTPGonePhoneVerificationExpired = &errHTTP{41001, http.StatusGone, "phone number verification expired or does not exist", "", nil} errHTTPEntityTooLargeAttachment = &errHTTP{41301, http.StatusRequestEntityTooLarge, "attachment too large, or bandwidth limit reached", "https://ntfy.sh/docs/publish/#limitations", nil} errHTTPEntityTooLargeMatrixRequest = &errHTTP{41302, http.StatusRequestEntityTooLarge, "Matrix request is larger than the max allowed length", "", nil} diff --git a/server/server.go b/server/server.go index c380bd26..95d2ecf7 100644 --- a/server/server.go +++ b/server/server.go @@ -58,7 +58,7 @@ type Server struct { smtpServer *smtp.Server smtpServerBackend *smtpBackend smtpSender mailer - mailSender *mail.Sender + mailSender emailVerifier topics map[string]*topic visitors map[string]*visitor // ip: or user: firebaseClient *firebaseClient @@ -80,9 +80,10 @@ type handleFunc func(http.ResponseWriter, *http.Request, *visitor) error var ( // If changed, don't forget to update Android App and auth_sqlite.go - topicRegex = regexp.MustCompile(`^[-_A-Za-z0-9]{1,64}$`) // No /! - topicPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}$`) // Regex must match JS & Android app! - externalTopicPathRegex = regexp.MustCompile(`^/[^/]+\.[^/]+/[-_A-Za-z0-9]{1,64}$`) // Extended topic path, for web-app, e.g. /example.com/mytopic + topicRegex = regexp.MustCompile(`^[-_A-Za-z0-9]{1,64}$`) // No /! + topicPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}$`) // Regex must match JS & Android app! + externalTopicPathRegex = regexp.MustCompile(`^/[^/]+\.[^/]+/[-_A-Za-z0-9]{1,64}$`) // Extended topic path, for web-app, e.g. /example.com/mytopic + webAppEmailVerifyRegex = regexp.MustCompile(`^/account/email/verify/[-_A-Za-z0-9]+$`) // Magic-link landing (served by the web app) jsonPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}(,[-_A-Za-z0-9]{1,64})*/json$`) ssePathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}(,[-_A-Za-z0-9]{1,64})*/sse$`) rawPathRegex = regexp.MustCompile(`^/[-_A-Za-z0-9]{1,64}(,[-_A-Za-z0-9]{1,64})*/raw$`) @@ -116,6 +117,9 @@ var ( apiAccountPhoneVerifyPath = "/v1/account/phone/verify" apiAccountEmailPath = "/v1/account/email" apiAccountEmailVerifyPath = "/v1/account/email/verify" + apiAccountEmailPrimaryPath = "/v1/account/email/primary" + apiAccountEmailResendPath = "/v1/account/email/resend" + webAppEmailVerifyPathPrefix = "/account/email/verify/" // Browser landing route; raw token appended apiAccountBillingPortalPath = "/v1/account/billing/portal" apiAccountBillingWebhookPath = "/v1/account/billing/webhook" apiAccountBillingSubscriptionPath = "/v1/account/billing/subscription" @@ -177,15 +181,16 @@ const ( // subscriber (if configured). func New(conf *Config) (*Server, error) { var mailer mailer - var mailSender *mail.Sender + var emailSender emailVerifier // Stays untyped-nil when SMTP is unconfigured, so ensureEmailsEnabled gates correctly if conf.SMTPSenderAddr != "" { - mailSender = mail.NewSender(&mail.Config{ + mailSender := mail.NewSender(&mail.Config{ SMTPAddr: conf.SMTPSenderAddr, SMTPUser: conf.SMTPSenderUser, SMTPPass: conf.SMTPSenderPass, From: conf.SMTPSenderFrom, }) mailer = &smtpSender{config: conf, sender: mailSender} + emailSender = mailSender } var stripe stripeAPI if payments.Available && conf.StripeSecretKey != "" { @@ -291,7 +296,7 @@ func New(conf *Config) (*Server, error) { attachment: attachmentStore, firebaseClient: firebaseClient, smtpSender: mailer, - mailSender: mailSender, + mailSender: emailSender, topics: topics, userManager: userManager, messages: messages, @@ -611,12 +616,16 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit return s.ensureUser(s.ensureCallsEnabled(s.withAccountSync(s.handleAccountPhoneNumberAdd)))(w, r, v) } else if r.Method == http.MethodDelete && r.URL.Path == apiAccountPhonePath { return s.ensureUser(s.ensureCallsEnabled(s.withAccountSync(s.handleAccountPhoneNumberDelete)))(w, r, v) - } else if r.Method == http.MethodPut && r.URL.Path == apiAccountEmailVerifyPath { - return s.ensureUser(s.ensureEmailsEnabled(s.withAccountSync(s.handleAccountEmailVerify)))(w, r, v) } else if r.Method == http.MethodPut && r.URL.Path == apiAccountEmailPath { return s.ensureUser(s.ensureEmailsEnabled(s.withAccountSync(s.handleAccountEmailAdd)))(w, r, v) + } else if r.Method == http.MethodPost && r.URL.Path == apiAccountEmailVerifyPath { + return s.ensureEmailsEnabled(s.limitRequests(s.handleAccountEmailVerify))(w, r, v) // No ensureUser: clicked from a mail client, possibly logged out } else if r.Method == http.MethodDelete && r.URL.Path == apiAccountEmailPath { return s.ensureUser(s.ensureEmailsEnabled(s.withAccountSync(s.handleAccountEmailDelete)))(w, r, v) + } else if r.Method == http.MethodPost && r.URL.Path == apiAccountEmailPrimaryPath { + return s.ensureUser(s.withAccountSync(s.handleAccountEmailSetPrimary))(w, r, v) + } else if r.Method == http.MethodPost && r.URL.Path == apiAccountEmailResendPath { + return s.ensureUser(s.ensureEmailsEnabled(s.handleAccountEmailResend))(w, r, v) } else if r.Method == http.MethodPost && apiWebPushPath == r.URL.Path { return s.ensureWebPushEnabled(s.limitRequests(s.handleWebPushUpdate))(w, r, v) } else if r.Method == http.MethodDelete && apiWebPushPath == r.URL.Path { @@ -659,12 +668,25 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit return s.limitRequests(s.authorizeTopicRead(s.handleSubscribeWS))(w, r, v) } else if r.Method == http.MethodGet && authPathRegex.MatchString(r.URL.Path) { return s.limitRequests(s.authorizeTopicRead(s.handleTopicAuth))(w, r, v) + } else if r.Method == http.MethodGet && webAppEmailVerifyRegex.MatchString(r.URL.Path) { + return s.ensureWebEnabled(s.handleWebAppIndex)(w, r, v) // Magic-link landing page (client-side route) } else if r.Method == http.MethodGet && (topicPathRegex.MatchString(r.URL.Path) || externalTopicPathRegex.MatchString(r.URL.Path)) { return s.ensureWebEnabled(s.handleTopic)(w, r, v) } return errHTTPNotFound } +// handleWebAppIndex serves the embedded web app's index for client-side (SPA) routes the +// browser router resolves, such as the magic-link landing pages. Because these URLs carry a +// one-time token in the path, the response is marked no-referrer (so the token can't leak to +// third parties via the Referer header) and noindex (so it never gets indexed). +func (s *Server) handleWebAppIndex(w http.ResponseWriter, r *http.Request, v *visitor) error { + w.Header().Set("Referrer-Policy", "no-referrer") + w.Header().Set("X-Robots-Tag", "noindex") + r.URL.Path = webAppIndex + return s.handleStatic(w, r, v) +} + func (s *Server) handleRoot(w http.ResponseWriter, r *http.Request, v *visitor) error { r.URL.Path = webAppIndex return s.handleStatic(w, r, v) @@ -715,21 +737,22 @@ func (s *Server) handleWebConfig(w http.ResponseWriter, _ *http.Request, _ *visi func (s *Server) configResponse() *apiConfigResponse { return &apiConfigResponse{ - BaseURL: "", // Will translate to window.location.origin - AppRoot: s.config.WebRoot, - EnableLogin: s.config.EnableLogin, - RequireLogin: s.config.RequireLogin, - EnableSignup: s.config.EnableSignup, - EnablePayments: s.config.StripeSecretKey != "", - EnableCalls: s.config.TwilioAccount != "", - EnableEmails: s.config.SMTPSenderFrom != "", - EnableEmailVerify: s.config.SMTPSenderVerify, - EnableReservations: s.config.EnableReservations, - EnableWebPush: s.config.WebPushPublicKey != "", - BillingContact: s.config.BillingContact, - WebPushPublicKey: s.config.WebPushPublicKey, - DisallowedTopics: s.config.DisallowedTopics, - ConfigHash: s.config.Hash(), + BaseURL: "", // Will translate to window.location.origin + AppRoot: s.config.WebRoot, + EnableLogin: s.config.EnableLogin, + RequireLogin: s.config.RequireLogin, + EnableSignup: s.config.EnableSignup, + EnablePayments: s.config.StripeSecretKey != "", + EnableCalls: s.config.TwilioAccount != "", + EnableEmails: s.config.SMTPSenderFrom != "", + EnableEmailVerify: s.config.SMTPSenderVerify, + EnableResetPassword: s.config.SMTPSenderFrom != "" && s.config.BaseURL != "", // Reset links need SMTP + an absolute base-url + EnableReservations: s.config.EnableReservations, + EnableWebPush: s.config.WebPushPublicKey != "", + BillingContact: s.config.BillingContact, + WebPushPublicKey: s.config.WebPushPublicKey, + DisallowedTopics: s.config.DisallowedTopics, + ConfigHash: s.config.Hash(), } } diff --git a/server/server_account.go b/server/server_account.go index 7c5c03c3..fd3a5eba 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -15,8 +15,9 @@ import ( ) const ( - syncTopicAccountSyncEvent = "sync" - tokenExpiryDuration = 72 * time.Hour // Extend tokens by this much + syncTopicAccountSyncEvent = "sync" + tokenExpiryDuration = 72 * time.Hour // Extend tokens by this much + emailVerificationTokenExpiry = 24 * time.Hour // Magic-link lifetime for email verification ) func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v *visitor) error { @@ -168,6 +169,18 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, r *http.Request, v *vis 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 + } } } else { response.Username = user.Everyone @@ -615,74 +628,149 @@ func (s *Server) handleAccountPhoneNumberDelete(w http.ResponseWriter, r *http.R return s.writeJSON(w, newSuccessResponse()) } -func (s *Server) handleAccountEmailVerify(w http.ResponseWriter, r *http.Request, v *visitor) error { +// handleAccountEmailAdd starts email verification (PUT /v1/account/email): it generates a +// magic-link token, stores a pending verification, and emails the link. The address is NOT +// added to the verified list until the user clicks the link (handleAccountEmailVerify). +func (s *Server) handleAccountEmailAdd(w http.ResponseWriter, r *http.Request, v *visitor) error { u := v.User() - req, err := readJSONWithLimit[apiAccountEmailVerifyRequest](r.Body, jsonBodyBytesLimit, false) + req, err := readJSONWithLimit[apiAccountEmailRequest](r.Body, jsonBodyBytesLimit, false) if err != nil { return err } else if !emailAddressRegex.MatchString(req.Email) { return errHTTPBadRequestEmailAddressInvalid } - // Check user is allowed to add emails - if u == nil { - return errHTTPUnauthorized - } else if u.IsUser() && u.Tier != nil && u.Tier.EmailLimit == 0 { + // Check user is allowed to add emails (the tier email limit gates the feature) + if u.IsUser() && u.Tier != nil && u.Tier.EmailLimit == 0 { return errHTTPUnauthorized } else if u.IsUser() && u.Tier == nil && s.config.VisitorEmailLimitBurst == 0 { return errHTTPUnauthorized } - // Check if email already exists + // Reject if already verified on this account (pending re-requests are fine -- they replace) emails, err := s.userManager.Emails(u.ID) if err != nil { return err } else if util.Contains(emails, req.Email) { return errHTTPConflictEmailExists } - // Check email rate limit (counts against the user's email quota) + // Rate limit (counts against the user's email quota) if !v.EmailAllowed() { return errHTTPTooManyRequestsLimitEmails } - // Send verification email - logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Sending email verification") - if err := s.mailSender.SendVerification(req.Email); err != nil { + logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Starting email verification") + if err := s.enqueueEmailVerification(u.ID, req.Email); err != nil { return err } return s.writeJSON(w, newSuccessResponse()) } -func (s *Server) handleAccountEmailAdd(w http.ResponseWriter, r *http.Request, v *visitor) error { +// handleAccountEmailVerify performs verification from the (unauthenticated) landing page +// (POST /v1/account/email/verify): it validates the raw token, adds the address to the user's +// verified emails, and -- if the user has no primary yet -- promotes it. No auth is required; +// the token binds the action to a user, so the click works from a logged-out mail client. +func (s *Server) handleAccountEmailVerify(w http.ResponseWriter, r *http.Request, v *visitor) error { + req, err := readJSONWithLimit[apiAccountEmailVerifyRequest](r.Body, jsonBodyBytesLimit, false) + if err != nil { + return err + } else if req.Token == "" { + return errHTTPBadRequestEmailVerificationCodeInvalid + } + m, err := s.userManager.VerifyEmail(req.Token) + if errors.Is(err, user.ErrMagicLinkNotFound) { + return errHTTPBadRequestEmailVerificationCodeInvalid + } else if err != nil { + return err + } + logvr(v, r).Tag(tagAccount).Field("email", m.Email).Info("Email verified") + // Refresh the verified user's other sessions. The request is unauthenticated (v.User() is + // usually nil), so resolve the user from the token row and publish to their sync topic. + s.publishSyncEventForUserIDAsync(v, m.UserID) + return s.writeJSON(w, newSuccessResponse()) +} + +// handleAccountEmailDelete removes an email address, whether verified or still pending +// (DELETE /v1/account/email). Removing the primary leaves the account with no primary. +func (s *Server) handleAccountEmailDelete(w http.ResponseWriter, r *http.Request, v *visitor) error { u := v.User() - req, err := readJSONWithLimit[apiAccountEmailAddRequest](r.Body, jsonBodyBytesLimit, false) + req, err := readJSONWithLimit[apiAccountEmailRequest](r.Body, jsonBodyBytesLimit, false) if err != nil { return err } else if !emailAddressRegex.MatchString(req.Email) { return errHTTPBadRequestEmailAddressInvalid - } else if !s.mailSender.CheckVerification(req.Email, req.Code) { - return errHTTPBadRequestEmailVerificationCodeInvalid } - logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Adding email as verified") - if err := s.userManager.AddEmail(u.ID, req.Email); err != nil { + logvr(v, r).Tag(tagAccount).Field("email", req.Email).Debug("Deleting email (verified or pending)") + if err := s.userManager.RemoveEmail(u.ID, req.Email); err != nil { + return err + } + // Also drop any pending verification for the address (no-op if there is none) + if err := s.userManager.DeleteEmailVerification(u.ID, req.Email); err != nil { return err } return s.writeJSON(w, newSuccessResponse()) } -func (s *Server) handleAccountEmailDelete(w http.ResponseWriter, r *http.Request, v *visitor) error { +// handleAccountEmailSetPrimary marks an already-verified email as the user's primary (recovery) +// email (POST /v1/account/email/primary). +func (s *Server) handleAccountEmailSetPrimary(w http.ResponseWriter, r *http.Request, v *visitor) error { u := v.User() - req, err := readJSONWithLimit[apiAccountEmailVerifyRequest](r.Body, jsonBodyBytesLimit, false) + req, err := readJSONWithLimit[apiAccountEmailRequest](r.Body, jsonBodyBytesLimit, false) if err != nil { return err - } - if !emailAddressRegex.MatchString(req.Email) { + } else if !emailAddressRegex.MatchString(req.Email) { return errHTTPBadRequestEmailAddressInvalid } - logvr(v, r).Tag(tagAccount).Field("email", req.Email).Debug("Deleting verified email") - if err := s.userManager.RemoveEmail(u.ID, req.Email); err != nil { + logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Setting primary email") + err = s.userManager.SetPrimaryEmail(u.ID, req.Email) + if errors.Is(err, user.ErrEmailPrimaryElsewhere) { + return errHTTPConflictEmailPrimaryElsewhere + } else if errors.Is(err, user.ErrEmailNotFound) { + return errHTTPBadRequestEmailAddressNotVerified + } else if err != nil { return err } return s.writeJSON(w, newSuccessResponse()) } +// handleAccountEmailResend re-sends a pending email verification (POST /v1/account/email/resend). +func (s *Server) handleAccountEmailResend(w http.ResponseWriter, r *http.Request, v *visitor) error { + u := v.User() + req, err := readJSONWithLimit[apiAccountEmailRequest](r.Body, jsonBodyBytesLimit, false) + if err != nil { + return err + } else if !emailAddressRegex.MatchString(req.Email) { + return errHTTPBadRequestEmailAddressInvalid + } + // Only resend for an address that is actually pending on this account + pending, err := s.userManager.PendingEmails(u.ID) + if err != nil { + return err + } else if !util.Contains(pending, req.Email) { + return errHTTPBadRequestEmailAddressInvalid + } + if !v.EmailAllowed() { + return errHTTPTooManyRequestsLimitEmails + } + logvr(v, r).Tag(tagAccount).Field("email", req.Email).Info("Resending email verification") + if err := s.enqueueEmailVerification(u.ID, req.Email); err != nil { + return err + } + return s.writeJSON(w, newSuccessResponse()) +} + +// enqueueEmailVerification generates a magic-link token for the given address, stores the +// pending verification (replacing any existing one), and emails the link. Shared by the add, +// resend, signup, and Stripe paths. Requires base-url to build an absolute link. +func (s *Server) enqueueEmailVerification(userID, email string) error { + if s.config.BaseURL == "" { + return errHTTPInternalErrorMissingBaseURL + } + token, err := s.userManager.CreateMagicLink(user.MagicLinkKindEmailVerify, userID, email, emailVerificationTokenExpiry) + if err != nil { + return err + } + link := s.config.BaseURL + webAppEmailVerifyPathPrefix + token + return s.mailSender.SendEmailVerification(email, link) +} + // convertEmailAddress checks the email address against the user's verified email list. // If smtp-sender-verify is false (default), the email is passed through as-is for // backwards compatibility. If true, the user must be authenticated and the email must be @@ -721,9 +809,30 @@ func (s *Server) publishSyncEventAsync(v *visitor) { }() } -// publishSyncEvent publishes a sync message to the user's sync topic +// publishSyncEvent publishes a sync message to the authenticated user's sync topic func (s *Server) publishSyncEvent(v *visitor) error { - u := v.User() + return s.publishSyncEventForUser(v, v.User()) +} + +// publishSyncEventForUserIDAsync publishes a sync event to the sync topic of the user with the +// given ID, resolving the user first. Used by the unauthenticated email-verify handler, where +// the request visitor has no associated user but the token identifies the account to refresh. +func (s *Server) publishSyncEventForUserIDAsync(v *visitor, userID string) { + go func() { + u, err := s.userManager.UserByID(userID) + if err != nil { + logv(v).Err(err).Trace("Error loading user for sync event") + return + } + if err := s.publishSyncEventForUser(v, u); err != nil { + logv(v).Err(err).Trace("Error publishing to user's sync topic") + } + }() +} + +// publishSyncEventForUser publishes a sync message to the given user's sync topic, using v as +// the publishing visitor (for rate-limit accounting). No-op if the user has no sync topic. +func (s *Server) publishSyncEventForUser(v *visitor, u *user.User) error { if u == nil || u.SyncTopic == "" { return nil } diff --git a/server/server_account_email_test.go b/server/server_account_email_test.go new file mode 100644 index 00000000..b594684d --- /dev/null +++ b/server/server_account_email_test.go @@ -0,0 +1,190 @@ +package server + +import ( + "fmt" + "io" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "heckel.io/ntfy/v2/user" + "heckel.io/ntfy/v2/util" +) + +// captureMailer is a fake emailVerifier that records the magic links it is asked to send, so +// tests can "click" them without a real SMTP server. +type captureMailer struct { + verifyLinks map[string]string // email -> verification link + resetLinks map[string]string // email -> reset link +} + +func newCaptureMailer() *captureMailer { + return &captureMailer{verifyLinks: map[string]string{}, resetLinks: map[string]string{}} +} + +func (c *captureMailer) SendEmailVerification(to, link string) error { + c.verifyLinks[to] = link + return nil +} + +func (c *captureMailer) SendPasswordReset(to, link string) error { + c.resetLinks[to] = link + return nil +} + +func (c *captureMailer) Close() {} + +// newEmailTestServer creates a server with email sending "enabled" (SMTP + base-url configured) +// and a capturing mailer injected, plus a tier-less user "ben" logged in via basic auth. +func newEmailTestServer(t *testing.T, databaseURL string) (*Server, *captureMailer, map[string]string) { + conf := newTestConfigWithAuthFile(t, databaseURL) + conf.SMTPSenderAddr = "localhost:25" + conf.SMTPSenderFrom = "noreply@example.com" + conf.BaseURL = "https://ntfy.example.com" + s := newTestServer(t, conf) + mailer := newCaptureMailer() + s.mailSender = mailer + require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) + auth := map[string]string{"Authorization": util.BasicAuth("ben", "ben")} + return s, mailer, auth +} + +func getAccount(t *testing.T, s *Server, auth map[string]string) *apiAccountResponse { + rr := request(t, s, "GET", "/v1/account", "", auth) + require.Equal(t, 200, rr.Code) + account, err := util.UnmarshalJSON[apiAccountResponse](io.NopCloser(rr.Body)) + require.Nil(t, err) + return account +} + +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) +} + +func TestAccount_Email_AddVerifySetsPrimary(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, mailer, auth := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + + // Start verification + rr := request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, auth) + require.Equal(t, 200, rr.Code) + + // 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) + + // "Click" the captured link (unauthenticated POST) + token := tokenFromLink(t, mailer.verifyLinks["ben@example.com"], "https://ntfy.example.com/account/email/verify/") + rr = request(t, s, "POST", "/v1/account/email/verify", fmt.Sprintf(`{"token":"%s"}`, token), nil) + require.Equal(t, 200, rr.Code) + + // 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) + }) +} + +func TestAccount_Email_VerifyInvalidToken(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, _, _ := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + + rr := request(t, s, "POST", "/v1/account/email/verify", `{"token":"doesnotexist"}`, nil) + require.Equal(t, 400, rr.Code) + require.Equal(t, 40051, toHTTPError(t, rr.Body.String()).Code) + + // Empty token also rejected + rr = request(t, s, "POST", "/v1/account/email/verify", `{"token":""}`, nil) + require.Equal(t, 400, rr.Code) + }) +} + +func TestAccount_Email_DeletePending(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, _, auth := newEmailTestServer(t, databaseURL) + 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) + + // 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) + }) +} + +func TestAccount_Email_Resend(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, mailer, auth := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + + require.Equal(t, 200, request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, auth).Code) + firstLink := mailer.verifyLinks["ben@example.com"] + require.NotEmpty(t, firstLink) + + // Resend issues a fresh link (the old one is replaced) + require.Equal(t, 200, request(t, s, "POST", "/v1/account/email/resend", `{"email":"ben@example.com"}`, auth).Code) + require.NotEqual(t, firstLink, mailer.verifyLinks["ben@example.com"]) + + // The old token no longer verifies; the new one does + oldToken := tokenFromLink(t, firstLink, "https://ntfy.example.com/account/email/verify/") + require.Equal(t, 400, request(t, s, "POST", "/v1/account/email/verify", fmt.Sprintf(`{"token":"%s"}`, oldToken), nil).Code) + newToken := tokenFromLink(t, mailer.verifyLinks["ben@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"}`, newToken), nil).Code) + + // Resending for a non-pending address is rejected + require.Equal(t, 400, request(t, s, "POST", "/v1/account/email/resend", `{"email":"never@example.com"}`, auth).Code) + }) +} + +func TestAccount_Email_SetPrimaryCollision(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, mailer, auth := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + + // ben verifies shared@ -> becomes his primary + 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) + + // 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)) + aliceAuth := map[string]string{"Authorization": util.BasicAuth("alice", "alice")} + require.Equal(t, 200, request(t, s, "PUT", "/v1/account/email", `{"email":"shared@example.com"}`, aliceAuth).Code) + 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) + + // 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) + require.Equal(t, 409, rr.Code) + require.Equal(t, 40908, toHTTPError(t, rr.Body.String()).Code) + }) +} + +func TestAccount_Email_AddDuplicateVerified(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, mailer, auth := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + + require.Equal(t, 200, request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, auth).Code) + token := tokenFromLink(t, mailer.verifyLinks["ben@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"}`, token), nil).Code) + + // Adding the same already-verified address is a conflict + rr := request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, auth) + require.Equal(t, 409, rr.Code) + require.Equal(t, 40907, toHTTPError(t, rr.Body.String()).Code) + }) +} diff --git a/server/server_test.go b/server/server_test.go index bb4ddfba..1d19815b 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -1706,11 +1706,11 @@ func TestServer_AccountEmailVerify_UserWithoutTier(t *testing.T) { // Create a user without a tier require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) - // Verify email request should NOT return 401 - response := request(t, s, "PUT", "/v1/account/email/verify", `{"email":"ben@example.com"}`, map[string]string{ + // Starting email verification should NOT return 401 + response := request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, map[string]string{ "Authorization": util.BasicAuth("ben", "ben"), }) - // The request will fail (SMTP not available), but it must NOT be a 401 + // The request may fail (SMTP not available), but it must NOT be a 401 require.NotEqual(t, 401, response.Code) }) } @@ -1731,7 +1731,7 @@ func TestServer_AccountEmailVerify_UserWithoutTier_EmailLimitZero(t *testing.T) require.Nil(t, s.userManager.AddUser("ben", "ben", user.RoleUser, false)) // Should be rejected with 401 since email sending is disabled - response := request(t, s, "PUT", "/v1/account/email/verify", `{"email":"ben@example.com"}`, map[string]string{ + response := request(t, s, "PUT", "/v1/account/email", `{"email":"ben@example.com"}`, map[string]string{ "Authorization": util.BasicAuth("ben", "ben"), }) require.Equal(t, 401, response.Code) diff --git a/server/smtp_sender.go b/server/smtp_sender.go index 885f806b..1e7460e8 100644 --- a/server/smtp_sender.go +++ b/server/smtp_sender.go @@ -20,6 +20,14 @@ type mailer interface { Counts() (total int64, success int64, failure int64) } +// emailVerifier sends the magic-link emails for email verification and password reset. +// *mail.Sender implements it; tests inject a fake to capture the generated links. +type emailVerifier interface { + SendEmailVerification(to, link string) error + SendPasswordReset(to, link string) error + Close() +} + type smtpSender struct { config *Config sender *mail.Sender diff --git a/server/types.go b/server/types.go index 1f69d3de..b51e9f00 100644 --- a/server/types.go +++ b/server/types.go @@ -226,13 +226,16 @@ type apiAccountPhoneNumberAddRequest struct { Code string `json:"code"` // Only set when adding a phone number } -type apiAccountEmailVerifyRequest struct { +// apiAccountEmailRequest carries an email address for the add/delete/set-primary/resend +// endpoints (all of which identify an email by address in the JSON body). +type apiAccountEmailRequest struct { Email string `json:"email"` } -type apiAccountEmailAddRequest struct { - Email string `json:"email"` - Code string `json:"code"` +// apiAccountEmailVerifyRequest carries the raw magic-link token submitted (unauthenticated) +// from the verification landing page. +type apiAccountEmailVerifyRequest struct { + Token string `json:"token"` } type apiAccountTier struct { @@ -292,6 +295,8 @@ type apiAccountResponse struct { 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 Tier *apiAccountTier `json:"tier,omitempty"` Limits *apiAccountLimits `json:"limits,omitempty"` Stats *apiAccountStats `json:"stats,omitempty"` @@ -304,21 +309,22 @@ type apiAccountReservationRequest struct { } type apiConfigResponse struct { - BaseURL string `json:"base_url"` - AppRoot string `json:"app_root"` - EnableLogin bool `json:"enable_login"` - RequireLogin bool `json:"require_login"` - EnableSignup bool `json:"enable_signup"` - EnablePayments bool `json:"enable_payments"` - EnableCalls bool `json:"enable_calls"` - EnableEmails bool `json:"enable_emails"` - EnableEmailVerify bool `json:"enable_email_verify"` - EnableReservations bool `json:"enable_reservations"` - EnableWebPush bool `json:"enable_web_push"` - BillingContact string `json:"billing_contact"` - WebPushPublicKey string `json:"web_push_public_key"` - DisallowedTopics []string `json:"disallowed_topics"` - ConfigHash string `json:"config_hash"` + BaseURL string `json:"base_url"` + AppRoot string `json:"app_root"` + EnableLogin bool `json:"enable_login"` + RequireLogin bool `json:"require_login"` + EnableSignup bool `json:"enable_signup"` + EnablePayments bool `json:"enable_payments"` + EnableCalls bool `json:"enable_calls"` + EnableEmails bool `json:"enable_emails"` + EnableEmailVerify bool `json:"enable_email_verify"` + EnableResetPassword bool `json:"enable_reset_password"` + EnableReservations bool `json:"enable_reservations"` + EnableWebPush bool `json:"enable_web_push"` + BillingContact string `json:"billing_contact"` + WebPushPublicKey string `json:"web_push_public_key"` + DisallowedTopics []string `json:"disallowed_topics"` + ConfigHash string `json:"config_hash"` } type apiAccountBillingPrices struct { diff --git a/user/magic_link_test.go b/user/magic_link_test.go index 4be1fe55..fd96bb1a 100644 --- a/user/magic_link_test.go +++ b/user/magic_link_test.go @@ -7,18 +7,11 @@ import ( "github.com/stretchr/testify/require" ) -// addVerifyLink generates a raw token, stores an email-verification magic link for it, and -// returns the raw token so the test can "click" it via VerifyEmail. -func addVerifyLink(t *testing.T, a *Manager, userID, email string, expires int64) string { - raw := generateLinkToken() - require.Nil(t, a.AddMagicLink(&MagicLink{ - TokenHash: hashToken(raw), - Kind: MagicLinkKindEmailVerify, - UserID: userID, - Email: email, - Expires: expires, - Created: time.Now().Unix(), - })) +// addVerifyLink stores an email-verification magic link and returns the raw token so the test +// can "click" it via VerifyEmail. +func addVerifyLink(t *testing.T, a *Manager, userID, email string, ttl time.Duration) string { + raw, err := a.CreateMagicLink(MagicLinkKindEmailVerify, userID, email, ttl) + require.Nil(t, err) return raw } @@ -29,7 +22,7 @@ func TestUser_MagicLink_VerifyEmail_SetsPrimary(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw := addVerifyLink(t, a, phil.ID, "phil@example.com", time.Now().Add(24*time.Hour).Unix()) + raw := addVerifyLink(t, a, phil.ID, "phil@example.com", 24*time.Hour) // Before verifying: pending, not yet verified, no primary pending, err := a.PendingEmails(phil.ID) @@ -43,7 +36,7 @@ func TestUser_MagicLink_VerifyEmail_SetsPrimary(t *testing.T) { require.Equal(t, "", primary) // Verify: the first verified email auto-becomes primary - m, err := a.VerifyEmail(hashToken(raw)) + m, err := a.VerifyEmail(raw) require.Nil(t, err) require.Equal(t, "phil@example.com", m.Email) @@ -71,12 +64,12 @@ func TestUser_MagicLink_VerifyEmail_SecondStaysSecondary(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw1 := addVerifyLink(t, a, phil.ID, "first@example.com", time.Now().Add(24*time.Hour).Unix()) - _, err = a.VerifyEmail(hashToken(raw1)) + raw1 := addVerifyLink(t, a, phil.ID, "first@example.com", 24*time.Hour) + _, err = a.VerifyEmail(raw1) require.Nil(t, err) - raw2 := addVerifyLink(t, a, phil.ID, "second@example.com", time.Now().Add(24*time.Hour).Unix()) - _, err = a.VerifyEmail(hashToken(raw2)) + raw2 := addVerifyLink(t, a, phil.ID, "second@example.com", 24*time.Hour) + _, err = a.VerifyEmail(raw2) require.Nil(t, err) // Both verified, but primary is still the first @@ -100,16 +93,14 @@ func TestUser_MagicLink_PrimaryGlobalUniqueness(t *testing.T) { require.Nil(t, err) // phil verifies shared@ first -> becomes his primary - rawPhil := addVerifyLink(t, a, phil.ID, "shared@example.com", time.Now().Add(24*time.Hour).Unix()) - _, err = a.VerifyEmail(hashToken(rawPhil)) + _, err = a.VerifyEmail(addVerifyLink(t, a, phil.ID, "shared@example.com", 24*time.Hour)) require.Nil(t, err) primary, err := a.PrimaryEmail(phil.ID) require.Nil(t, err) require.Equal(t, "shared@example.com", primary) // ben verifies the same address -> allowed as secondary, but NOT his primary - rawBen := addVerifyLink(t, a, ben.ID, "shared@example.com", time.Now().Add(24*time.Hour).Unix()) - _, err = a.VerifyEmail(hashToken(rawBen)) + _, err = a.VerifyEmail(addVerifyLink(t, a, ben.ID, "shared@example.com", 24*time.Hour)) require.Nil(t, err) emails, err := a.Emails(ben.ID) require.Nil(t, err) @@ -120,7 +111,7 @@ func TestUser_MagicLink_PrimaryGlobalUniqueness(t *testing.T) { // Explicitly promoting ben's copy to primary collides with phil's require.ErrorIs(t, a.SetPrimaryEmail(ben.ID, "shared@example.com"), ErrEmailPrimaryElsewhere) - // ...and phil keeps his primary (the failed promotion rolled back ben's clear, which was a no-op anyway) + // ...and phil keeps his primary (the failed promotion rolled back ben's clear) primary, err = a.PrimaryEmail(phil.ID) require.Nil(t, err) require.Equal(t, "shared@example.com", primary) @@ -144,8 +135,8 @@ func TestUser_MagicLink_Expired(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw := addVerifyLink(t, a, phil.ID, "phil@example.com", time.Now().Add(-time.Minute).Unix()) - _, err = a.VerifyEmail(hashToken(raw)) + raw := addVerifyLink(t, a, phil.ID, "phil@example.com", -time.Minute) + _, err = a.VerifyEmail(raw) require.ErrorIs(t, err, ErrMagicLinkNotFound) // Nothing got verified @@ -162,11 +153,11 @@ func TestUser_MagicLink_SingleUse(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw := addVerifyLink(t, a, phil.ID, "phil@example.com", time.Now().Add(24*time.Hour).Unix()) - _, err = a.VerifyEmail(hashToken(raw)) + raw := addVerifyLink(t, a, phil.ID, "phil@example.com", 24*time.Hour) + _, err = a.VerifyEmail(raw) require.Nil(t, err) // Second click: token already consumed - _, err = a.VerifyEmail(hashToken(raw)) + _, err = a.VerifyEmail(raw) require.ErrorIs(t, err, ErrMagicLinkNotFound) }) } @@ -178,17 +169,17 @@ func TestUser_MagicLink_ReplaceOnReRequest(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw1 := addVerifyLink(t, a, phil.ID, "phil@example.com", time.Now().Add(24*time.Hour).Unix()) - raw2 := addVerifyLink(t, a, phil.ID, "phil@example.com", time.Now().Add(24*time.Hour).Unix()) + raw1 := addVerifyLink(t, a, phil.ID, "phil@example.com", 24*time.Hour) + raw2 := addVerifyLink(t, a, phil.ID, "phil@example.com", 24*time.Hour) // Only one pending row remains; the old token no longer works pending, err := a.PendingEmails(phil.ID) require.Nil(t, err) require.Equal(t, []string{"phil@example.com"}, pending) - _, err = a.MagicLinkByHash(hashToken(raw1)) + _, err = a.MagicLinkByToken(raw1) require.ErrorIs(t, err, ErrMagicLinkNotFound) - m, err := a.MagicLinkByHash(hashToken(raw2)) + m, err := a.MagicLinkByToken(raw2) require.Nil(t, err) require.Equal(t, "phil@example.com", m.Email) }) @@ -201,16 +192,10 @@ func TestUser_MagicLink_PasswordReset_RoundTrip(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - raw := generateLinkToken() - require.Nil(t, a.AddMagicLink(&MagicLink{ - TokenHash: hashToken(raw), - Kind: MagicLinkKindPasswordReset, - UserID: phil.ID, - Expires: time.Now().Add(time.Hour).Unix(), - Created: time.Now().Unix(), - })) + raw, err := a.CreateMagicLink(MagicLinkKindPasswordReset, phil.ID, "", time.Hour) + require.Nil(t, err) - m, err := a.MagicLinkByHash(hashToken(raw)) + m, err := a.MagicLinkByToken(raw) require.Nil(t, err) require.Equal(t, MagicLinkKindPasswordReset, m.Kind) require.Equal(t, phil.ID, m.UserID) @@ -222,20 +207,14 @@ func TestUser_MagicLink_PasswordReset_RoundTrip(t *testing.T) { require.Equal(t, 0, len(pending)) // New request replaces the old token - raw2 := generateLinkToken() - require.Nil(t, a.AddMagicLink(&MagicLink{ - TokenHash: hashToken(raw2), - Kind: MagicLinkKindPasswordReset, - UserID: phil.ID, - Expires: time.Now().Add(time.Hour).Unix(), - Created: time.Now().Unix(), - })) - _, err = a.MagicLinkByHash(hashToken(raw)) + raw2, err := a.CreateMagicLink(MagicLinkKindPasswordReset, phil.ID, "", time.Hour) + require.Nil(t, err) + _, err = a.MagicLinkByToken(raw) require.ErrorIs(t, err, ErrMagicLinkNotFound) // Single use: deleting consumes it - require.Nil(t, a.DeleteMagicLink(hashToken(raw2))) - _, err = a.MagicLinkByHash(hashToken(raw2)) + require.Nil(t, a.DeleteMagicLinkByToken(raw2)) + _, err = a.MagicLinkByToken(raw2) require.ErrorIs(t, err, ErrMagicLinkNotFound) }) } @@ -247,14 +226,14 @@ func TestUser_MagicLink_Reaper(t *testing.T) { phil, err := a.User("phil") require.Nil(t, err) - expired := addVerifyLink(t, a, phil.ID, "expired@example.com", time.Now().Add(-time.Hour).Unix()) - valid := addVerifyLink(t, a, phil.ID, "valid@example.com", time.Now().Add(time.Hour).Unix()) + expired := addVerifyLink(t, a, phil.ID, "expired@example.com", -time.Hour) + valid := addVerifyLink(t, a, phil.ID, "valid@example.com", time.Hour) require.Nil(t, a.deleteExpiredMagicLinks()) - _, err = a.MagicLinkByHash(hashToken(expired)) + _, err = a.MagicLinkByToken(expired) require.ErrorIs(t, err, ErrMagicLinkNotFound) - m, err := a.MagicLinkByHash(hashToken(valid)) + m, err := a.MagicLinkByToken(valid) require.Nil(t, err) require.Equal(t, "valid@example.com", m.Email) }) diff --git a/user/manager.go b/user/manager.go index e6df6910..4c62f940 100644 --- a/user/manager.go +++ b/user/manager.go @@ -1556,6 +1556,37 @@ func (a *Manager) SetPrimaryEmail(userID, email string) error { }) } +// CreateMagicLink generates a fresh magic-link token of the given kind, stores it (hashed, +// replacing any existing link in the same scope), and returns the RAW token for use in the +// emailed link. Only the hash is persisted; the raw token is never stored. email is the +// address being verified for email_verify, and "" for password_reset. +func (a *Manager) CreateMagicLink(kind MagicLinkKind, userID, email string, ttl time.Duration) (string, error) { + raw := generateLinkToken() + now := time.Now() + m := &MagicLink{ + TokenHash: hashToken(raw), + Kind: kind, + UserID: userID, + Email: email, + Expires: now.Add(ttl).Unix(), + Created: now.Unix(), + } + if err := a.AddMagicLink(m); err != nil { + return "", err + } + return raw, nil +} + +// MagicLinkByToken looks up a magic link by its raw token (hashing it first). See MagicLinkByHash. +func (a *Manager) MagicLinkByToken(rawToken string) (*MagicLink, error) { + return a.MagicLinkByHash(hashToken(rawToken)) +} + +// DeleteMagicLinkByToken deletes a magic link identified by its raw token (single-use consume). +func (a *Manager) DeleteMagicLinkByToken(rawToken string) error { + return a.DeleteMagicLink(hashToken(rawToken)) +} + // AddMagicLink stores a pending magic link, replacing any existing link in the same scope: // for email_verify that is the (user_id, email) pair (one pending verification per address); // for password_reset that is the user_id (one active reset per account). The replace-delete and @@ -1606,12 +1637,21 @@ func (a *Manager) DeleteMagicLink(tokenHash string) error { return err } -// VerifyEmail consumes an email-verification magic link: after validating the token (kind + -// expiry), it deletes the link, adds the address to the user's verified emails, and -- if the -// user has no primary email yet and the address is not already primary on another account -- -// promotes the new address to primary. All mutations run in one transaction. A primary -// collision simply leaves the address verified but non-primary. Returns the consumed link. -func (a *Manager) VerifyEmail(tokenHash string) (*MagicLink, error) { +// DeleteEmailVerification removes any pending email verification for (userID, email). Used when +// an unverified (pending) address is cancelled/deleted from the account. +func (a *Manager) DeleteEmailVerification(userID, email string) error { + _, err := a.db.Exec(a.queries.deleteVerifyScope, userID, email) + return err +} + +// VerifyEmail consumes an email-verification magic link, identified by its raw token: after +// validating the token (kind + expiry), it deletes the link, adds the address to the user's +// verified emails, and -- if the user has no primary email yet and the address is not already +// primary on another account -- promotes the new address to primary. All mutations run in one +// transaction. A primary collision simply leaves the address verified but non-primary. Returns +// the consumed link. +func (a *Manager) VerifyEmail(rawToken string) (*MagicLink, error) { + tokenHash := hashToken(rawToken) m, err := a.MagicLinkByHash(tokenHash) if err != nil { return nil, err diff --git a/user/manager_test.go b/user/manager_test.go index 3845752e..7e05f5db 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -1847,16 +1847,9 @@ func TestMigrationFrom7(t *testing.T) { require.Equal(t, "", primary) // The new magic-link machinery works post-migration - raw := generateLinkToken() - require.Nil(t, a.AddMagicLink(&MagicLink{ - TokenHash: hashToken(raw), - Kind: MagicLinkKindEmailVerify, - UserID: "u_phil", - Email: "new@example.com", - Expires: time.Now().Add(24 * time.Hour).Unix(), - Created: time.Now().Unix(), - })) - m, err := a.VerifyEmail(hashToken(raw)) + raw, err := a.CreateMagicLink(MagicLinkKindEmailVerify, "u_phil", "new@example.com", 24*time.Hour) + require.Nil(t, err) + m, err := a.VerifyEmail(raw) require.Nil(t, err) require.Equal(t, "new@example.com", m.Email) diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json index 2e06cc64..2dff3100 100644 --- a/web/public/static/langs/en.json +++ b/web/public/static/langs/en.json @@ -3,8 +3,15 @@ "common_save": "Save", "common_add": "Add", "common_back": "Back", + "common_close": "Close", "common_copy_to_clipboard": "Copy to clipboard", "common_refresh": "Refresh", + "email_verify_progress_title": "Verifying your email...", + "email_verify_success_title": "Email verified", + "email_verify_success_description": "Your email address has been verified and added to your account.", + "email_verify_error_title": "Verification failed", + "email_verify_error_description": "This verification link is invalid or has expired. You can request a new one from your account settings.", + "email_verify_button_account": "Go to account", "version_update_available_title": "New version available", "version_update_available_description": "The ntfy server has been updated. Please refresh the page.", "signup_title": "Create a ntfy account", @@ -216,18 +223,24 @@ "account_basics_phone_numbers_dialog_channel_sms": "SMS", "account_basics_phone_numbers_dialog_channel_call": "Call", "account_basics_emails_title": "Email addresses", - "account_basics_emails_description": "For email notifications", - "account_basics_emails_no_emails_yet": "No verified emails yet", + "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_primary_badge": "Primary", + "account_basics_emails_unverified": "unverified", + "account_basics_emails_set_primary": "Set as recovery email", + "account_basics_emails_delete": "Remove", + "account_basics_emails_cancel": "Cancel", + "account_basics_emails_resend": "Resend verification email", + "account_basics_emails_resent": "Verification email sent, check your inbox", + "account_basics_emails_primary_elsewhere": "This email is the recovery email on another account", + "account_basics_emails_no_recovery_warning": "No recovery email set. You will not be able to reset your password. Add and verify an email below, then set it as your recovery email.", "account_basics_emails_dialog_title": "Add email address", - "account_basics_emails_dialog_description": "To receive email notifications, you need to add and verify at least one email address. A verification code will be sent to your email.", + "account_basics_emails_dialog_description": "Enter an email address to add it to your account. We will send a verification link to confirm it is yours.", "account_basics_emails_dialog_email_label": "Email address", "account_basics_emails_dialog_email_placeholder": "e.g. user@example.com", - "account_basics_emails_dialog_verify_button": "Add email", - "account_basics_emails_dialog_code_label": "Verification code", - "account_basics_emails_dialog_code_placeholder": "e.g. 123456", - "account_basics_emails_dialog_code_invalid": "Verification code is invalid or expired", - "account_basics_emails_dialog_check_verification_button": "Confirm", + "account_basics_emails_dialog_verify_button": "Send verification link", + "account_basics_emails_dialog_check_inbox": "Check your inbox and click the verification link to confirm this email address. It will appear as unverified until you do.", "account_basics_cannot_edit_or_delete_provisioned_user": "A provisioned user cannot be edited or deleted", "account_usage_title": "Usage", "account_usage_of_limit": "of {{limit}}", diff --git a/web/src/app/AccountApi.js b/web/src/app/AccountApi.js index 4fadb8c5..e9d21d65 100644 --- a/web/src/app/AccountApi.js +++ b/web/src/app/AccountApi.js @@ -4,6 +4,8 @@ import { accountBillingSubscriptionUrl, accountEmailUrl, accountEmailVerifyUrl, + accountEmailPrimaryUrl, + accountEmailResendUrl, accountPasswordUrl, accountPhoneUrl, accountPhoneVerifyUrl, @@ -342,9 +344,11 @@ class AccountApi { }); } - async verifyEmail(email) { - const url = accountEmailVerifyUrl(config.base_url); - console.log(`[AccountApi] Sending email verification ${url}`); + // startEmailVerification begins adding an email: the server stores a pending verification and + // emails a magic link. The address is not verified until the link is clicked. + async startEmailVerification(email) { + const url = accountEmailUrl(config.base_url); + console.log(`[AccountApi] Starting email verification ${url}`); await fetchOrThrow(url, { method: "PUT", headers: withBearerAuth({}, session.token()), @@ -354,15 +358,41 @@ class AccountApi { }); } - async addEmail(email, code) { - const url = accountEmailUrl(config.base_url); - console.log(`[AccountApi] Adding email with verification code ${url}`); + // verifyEmailToken performs verification from the magic-link landing page. It is unauthenticated: + // the token identifies the account, so this works even when clicked from a logged-out browser. + async verifyEmailToken(token) { + const url = accountEmailVerifyUrl(config.base_url); + console.log(`[AccountApi] Verifying email token ${url}`); await fetchOrThrow(url, { - method: "PUT", + method: "POST", + body: JSON.stringify({ + token, + }), + }); + } + + // resendEmailVerification re-sends the magic link for a pending (unverified) address. + async resendEmailVerification(email) { + const url = accountEmailResendUrl(config.base_url); + console.log(`[AccountApi] Resending email verification ${url}`); + await fetchOrThrow(url, { + method: "POST", + headers: withBearerAuth({}, session.token()), + body: JSON.stringify({ + email, + }), + }); + } + + // setPrimaryEmail marks an already-verified address as the primary (recovery) email. + async setPrimaryEmail(email) { + const url = accountEmailPrimaryUrl(config.base_url); + console.log(`[AccountApi] Setting primary email ${url}`); + await fetchOrThrow(url, { + method: "POST", headers: withBearerAuth({}, session.token()), body: JSON.stringify({ email, - code, }), }); } diff --git a/web/src/app/errors.js b/web/src/app/errors.js index 4214ad84..5b749d14 100644 --- a/web/src/app/errors.js +++ b/web/src/app/errors.js @@ -51,7 +51,15 @@ export class EmailVerificationCodeInvalidError extends Error { static CODE = 40051; // errHTTPBadRequestEmailVerificationCodeInvalid constructor() { - super("Email verification code invalid or expired"); + super("Email verification link invalid or expired"); + } +} + +export class EmailPrimaryElsewhereError extends Error { + static CODE = 40908; // errHTTPConflictEmailPrimaryElsewhere + + constructor() { + super("Email address is the recovery email on another account"); } } @@ -73,6 +81,8 @@ export const throwAppError = async (response) => { throw new IncorrectPasswordError(); } else if (error.code === EmailVerificationCodeInvalidError.CODE) { throw new EmailVerificationCodeInvalidError(); + } else if (error.code === EmailPrimaryElsewhereError.CODE) { + throw new EmailPrimaryElsewhereError(); } else if (error?.error) { throw new Error(`Error ${error.code}: ${error.error}`); } diff --git a/web/src/app/utils.js b/web/src/app/utils.js index db38801b..9bad68bf 100644 --- a/web/src/app/utils.js +++ b/web/src/app/utils.js @@ -35,6 +35,8 @@ export const accountPhoneUrl = (baseUrl) => `${baseUrl}/v1/account/phone`; export const accountPhoneVerifyUrl = (baseUrl) => `${baseUrl}/v1/account/phone/verify`; export const accountEmailUrl = (baseUrl) => `${baseUrl}/v1/account/email`; export const accountEmailVerifyUrl = (baseUrl) => `${baseUrl}/v1/account/email/verify`; +export const accountEmailPrimaryUrl = (baseUrl) => `${baseUrl}/v1/account/email/primary`; +export const accountEmailResendUrl = (baseUrl) => `${baseUrl}/v1/account/email/resend`; export const validUrl = (url) => url.match(/^https?:\/\/.+/); diff --git a/web/src/components/Account.jsx b/web/src/components/Account.jsx index 42402d41..a45dce09 100644 --- a/web/src/components/Account.jsx +++ b/web/src/components/Account.jsx @@ -2,6 +2,7 @@ import * as React from "react"; import { useContext, useState } from "react"; import { Alert, + Box, CardActions, CardContent, Chip, @@ -38,6 +39,9 @@ import { import EditIcon from "@mui/icons-material/Edit"; import { Trans, useTranslation } from "react-i18next"; import DeleteOutlineIcon from "@mui/icons-material/DeleteOutline"; +import StarIcon from "@mui/icons-material/Star"; +import StarBorderIcon from "@mui/icons-material/StarBorder"; +import RefreshIcon from "@mui/icons-material/Refresh"; import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined"; import CelebrationIcon from "@mui/icons-material/Celebration"; import CloseIcon from "@mui/icons-material/Close"; @@ -52,7 +56,7 @@ import UpgradeDialog from "./UpgradeDialog"; import { AccountContext } from "./App"; import DialogFooter from "./DialogFooter"; import { Paragraph } from "./styles"; -import { EmailVerificationCodeInvalidError, IncorrectPasswordError, UnauthorizedError } from "../app/errors"; +import { EmailPrimaryElsewhereError, IncorrectPasswordError, UnauthorizedError } from "../app/errors"; import { ProChip } from "./SubscriptionPopup"; import session from "../app/Session"; @@ -359,7 +363,7 @@ const Emails = () => { const { account } = useContext(AccountContext); const [dialogKey, setDialogKey] = useState(0); const [dialogOpen, setDialogOpen] = useState(false); - const [snackOpen, setSnackOpen] = useState(false); + const [snack, setSnack] = useState(""); // Non-empty shows a transient snackbar message const labelId = "prefVerifiedEmails"; const handleDialogOpen = () => { @@ -373,20 +377,34 @@ const Emails = () => { const handleCopy = (email) => { copyToClipboard(email); - setSnackOpen(true); + setSnack(t("account_basics_emails_copied_to_clipboard")); }; - const handleDelete = async (email) => { + // runEmailAction wraps an account API call with the shared error handling (redirect on + // unauthorized, surface a message otherwise). The account list refreshes via the sync event. + const runEmailAction = async (fn, errorMessage) => { try { - await accountApi.deleteEmail(email); + await fn(); } catch (e) { - console.log(`[Account] Error deleting email`, e); + console.log(`[Account] Email action failed`, e); if (e instanceof UnauthorizedError) { await session.resetAndRedirect(routes.login); + } else if (e instanceof EmailPrimaryElsewhereError) { + setSnack(t("account_basics_emails_primary_elsewhere")); + } else { + setSnack(errorMessage ?? e.message); } } }; + const handleDelete = (email) => runEmailAction(() => accountApi.deleteEmail(email)); + const handleSetPrimary = (email) => runEmailAction(() => accountApi.setPrimaryEmail(email)); + const handleResend = (email) => + runEmailAction(async () => { + await accountApi.resendEmailVerification(email); + setSnack(t("account_basics_emails_resent")); + }); + if (!config.enable_email_verify) { return null; } @@ -407,35 +425,73 @@ const Emails = () => { ); } + const verifiedEmails = account?.emails ?? []; + const pendingEmails = account?.pending_emails ?? []; + const primaryEmail = account?.primary_email ?? ""; + const showNoRecoveryWarning = config.enable_reset_password && primaryEmail === ""; + return (
- {account?.emails?.map((email) => ( - - {email} + {showNoRecoveryWarning && ( + + {t("account_basics_emails_no_recovery_warning")} + + )} + + {verifiedEmails.map((email) => { + const isPrimary = email === primaryEmail; + return ( + + {email} + {isPrimary && } + {!isPrimary && ( + + handleSetPrimary(email)}> + + + + )} + {isPrimary && } + + handleCopy(email)}> + + + + + handleDelete(email)}> + + + + + ); + })} + {pendingEmails.map((email) => ( + + + {email} ({t("account_basics_emails_unverified")}) + + + handleResend(email)}> + + - } - variant="outlined" - onClick={() => handleCopy(email)} - onDelete={() => handleDelete(email)} - /> - ))} - {!account?.emails && {t("account_basics_emails_no_emails_yet")}} - + + handleDelete(email)}> + + + + + ))} + {verifiedEmails.length === 0 && pendingEmails.length === 0 && {t("account_basics_emails_no_emails_yet")}} + +
- setSnackOpen(false)} - message={t("account_basics_emails_copied_to_clipboard")} - /> + setSnack("")} message={snack} /> ); @@ -446,18 +502,19 @@ const AddEmailDialog = (props) => { const { t } = useTranslation(); const [error, setError] = useState(""); const [email, setEmail] = useState(""); - const [code, setCode] = useState(""); const [sending, setSending] = useState(false); - const [verificationCodeSent, setVerificationCodeSent] = useState(false); + const [sent, setSent] = useState(false); const fullScreen = useMediaQuery(theme.breakpoints.down("sm")); - const verifyEmail = async () => { + // handleSubmit starts verification: the server emails a magic link. The pending address shows + // up in the account list as "(unverified)" once the account refreshes. + const handleSubmit = async () => { try { setSending(true); - await accountApi.verifyEmail(email); - setVerificationCodeSent(true); + await accountApi.startEmailVerification(email); + setSent(true); } catch (e) { - console.log(`[Account] Error sending email verification`, e); + console.log(`[Account] Error starting email verification`, e); if (e instanceof UnauthorizedError) { await session.resetAndRedirect(routes.login); } else { @@ -468,81 +525,41 @@ const AddEmailDialog = (props) => { } }; - const checkVerifyEmail = async () => { - try { - setSending(true); - await accountApi.addEmail(email, code); - props.onClose(); - } catch (e) { - console.log(`[Account] Error confirming email verification`, e); - if (e instanceof UnauthorizedError) { - await session.resetAndRedirect(routes.login); - } else if (e instanceof EmailVerificationCodeInvalidError) { - setError(t("account_basics_emails_dialog_code_invalid")); - } else { - setError(e.message); - } - } finally { - setSending(false); - } - }; - - const handleDialogSubmit = async () => { - if (!verificationCodeSent) { - await verifyEmail(); - } else { - await checkVerifyEmail(); - } - }; - - const handleCancel = () => { - if (verificationCodeSent) { - setVerificationCodeSent(false); - setCode(""); - } else { - props.onClose(); - } - }; - return ( - + {t("account_basics_emails_dialog_title")} - {t("account_basics_emails_dialog_description")} - {!verificationCodeSent && ( - setEmail(ev.target.value)} - fullWidth - variant="standard" - /> - )} - {verificationCodeSent && ( - setCode(ev.target.value)} - fullWidth - inputProps={{ inputMode: "numeric", pattern: "[0-9]*" }} - variant="standard" - /> + {sent ? ( + {t("account_basics_emails_dialog_check_inbox")} + ) : ( + <> + {t("account_basics_emails_dialog_description")} + setEmail(ev.target.value)} + fullWidth + variant="standard" + /> + )} - - + {sent ? ( + + ) : ( + <> + + + + )} ); diff --git a/web/src/components/App.jsx b/web/src/components/App.jsx index 575304d4..ed1fff99 100644 --- a/web/src/components/App.jsx +++ b/web/src/components/App.jsx @@ -20,6 +20,7 @@ import Messaging from "./Messaging"; import Login from "./Login"; import Signup from "./Signup"; import Account from "./Account"; +import EmailVerify from "./EmailVerify"; import initI18n from "../app/i18n"; // Translations! import prefs from "../app/Prefs"; import RTLCacheProvider from "./RTLCacheProvider"; @@ -63,6 +64,7 @@ const App = () => { } /> } /> + } /> }> } /> } /> diff --git a/web/src/components/EmailVerify.jsx b/web/src/components/EmailVerify.jsx new file mode 100644 index 00000000..ce70a8d5 --- /dev/null +++ b/web/src/components/EmailVerify.jsx @@ -0,0 +1,74 @@ +import * as React from "react"; +import { useEffect, useRef, useState } from "react"; +import { Typography, Button, Box, CircularProgress } from "@mui/material"; +import CheckCircleOutlineIcon from "@mui/icons-material/CheckCircleOutline"; +import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline"; +import { useParams, NavLink } from "react-router-dom"; +import { useTranslation } from "react-i18next"; +import accountApi from "../app/AccountApi"; +import AvatarBox from "./AvatarBox"; +import routes from "./routes"; + +// EmailVerify is the magic-link landing page for email verification. It performs the verification +// via a POST (the GET that loads this page has no side effects, so link prefetchers / scanners +// cannot consume the single-use token). The raw token is stripped from the URL on load to keep +// it out of browser history and Referer headers. +const EmailVerify = () => { + const { t } = useTranslation(); + const { token } = useParams(); + const [status, setStatus] = useState("verifying"); // "verifying" | "success" | "error" + const ran = useRef(false); + + useEffect(() => { + if (ran.current) { + return; // Guard against double-invoke (e.g. React StrictMode) consuming the token twice + } + ran.current = true; + // Strip the token from the URL immediately (keep it out of history / Referer) + window.history.replaceState(null, "", routes.account); + (async () => { + try { + await accountApi.verifyEmailToken(token); + setStatus("success"); + } catch (e) { + console.log(`[EmailVerify] Verification failed`, e); + setStatus("error"); + } + })(); + }, [token]); + + return ( + + {status === "verifying" && ( + <> + + {t("email_verify_progress_title")} + + )} + {status === "success" && ( + <> + + {t("email_verify_success_title")} + {t("email_verify_success_description")} + + + )} + {status === "error" && ( + <> + + {t("email_verify_error_title")} + {t("email_verify_error_description")} + + + + + )} + + ); +}; + +export default EmailVerify; diff --git a/web/src/components/routes.js b/web/src/components/routes.js index 17e0eac6..d9c371eb 100644 --- a/web/src/components/routes.js +++ b/web/src/components/routes.js @@ -7,6 +7,7 @@ const routes = { app: config.app_root, account: "/account", settings: "/settings", + emailVerify: "/account/email/verify/:token", subscription: "/:topic", subscriptionExternal: "/:baseUrl/:topic", forSubscription: (subscription) => {