diff --git a/docs/releases.md b/docs/releases.md index b84f774f..722dc50d 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -2007,6 +2007,12 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release ## Not released yet +### ntfy server v2.27.0 (UNRELEASED) + +**Features:** + +* Allow logging in with your verified primary email address (in addition to your username), so a password reset no longer leaves you unable to sign in when you only remember the email you signed up with + ### ntfy Android v1.25.2 (UNRELEASED) This release makes the "connection lost" alert configurable and turns it off by default. Folks did not like it and many reached out diff --git a/server/server.go b/server/server.go index 47418001..f3c47270 100644 --- a/server/server.go +++ b/server/server.go @@ -110,6 +110,7 @@ var ( apiUsersPath = "/v1/users" apiUsersAccessPath = "/v1/users/access" apiAccountPath = "/v1/account" + apiAccountLoginPath = "/v1/account/login" apiAccountTokenPath = "/v1/account/token" apiAccountPasswordPath = "/v1/account/password" apiAccountSettingsPath = "/v1/account/settings" @@ -579,6 +580,8 @@ func (s *Server) handleInternal(w http.ResponseWriter, r *http.Request, v *visit return s.ensureUser(s.withAccountSync(s.handleAccountDelete))(w, r, v) } else if r.Method == http.MethodPost && r.URL.Path == apiAccountPasswordPath { return s.ensureUser(s.handleAccountPasswordChange)(w, r, v) + } else if r.Method == http.MethodPost && r.URL.Path == apiAccountLoginPath { + return s.ensureUser(s.withAccountSync(s.handleAccountLogin))(w, r, v) } else if r.Method == http.MethodPost && r.URL.Path == apiAccountTokenPath { return s.ensureUser(s.withAccountSync(s.handleAccountTokenCreate))(w, r, v) } else if r.Method == http.MethodPatch && r.URL.Path == apiAccountTokenPath { diff --git a/server/server_account.go b/server/server_account.go index 17094d1d..6b972163 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -268,6 +268,24 @@ func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Requ return s.writeJSON(w, newSuccessResponse()) } +// handleAccountLogin authenticates a username-or-email + password (via the ensureUser wrapper's +// Basic Auth), mints a session token, and returns it together with the canonical username. Unlike +// the token endpoint (which exists to mint arbitrary API tokens), this endpoint's job is to log a +// user in, so it also reports who they are (the identifier they typed may be a primary email). +func (s *Server) handleAccountLogin(w http.ResponseWriter, r *http.Request, v *visitor) error { + u := v.User() + logvr(v, r).Tag(tagAccount).Info("Logging in user %s", u.Name) + token, err := s.userManager.CreateToken(u.ID, "", time.Now().Add(tokenExpiryDuration), v.IP(), false) + if err != nil { + return err + } + response := &apiAccountLoginResponse{ + Token: token.Value, + Username: u.Name, + } + return s.writeJSON(w, response) +} + func (s *Server) handleAccountTokenCreate(w http.ResponseWriter, r *http.Request, v *visitor) error { req, err := readJSONWithLimit[apiAccountTokenIssueRequest](r.Body, jsonBodyBytesLimit, true) // Allow empty body! if err != nil { diff --git a/server/server_account_email_test.go b/server/server_account_email_test.go index 793c89be..24aacaf5 100644 --- a/server/server_account_email_test.go +++ b/server/server_account_email_test.go @@ -224,6 +224,22 @@ func canLogin(t *testing.T, s *Server, username, password string) bool { return rr.Code == 200 } +func TestAccount_LoginByPrimaryEmail(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + s, mailer, auth := newEmailTestServer(t, databaseURL) + defer s.closeDatabases() + verifyEmailFor(t, s, mailer, auth, "ben@example.com") + + // Basic Auth works with either the username or the verified primary email + require.True(t, canLogin(t, s, "ben", "ben")) + require.True(t, canLogin(t, s, "ben@example.com", "ben")) + + // ...but not with the wrong password or an unknown email + require.False(t, canLogin(t, s, "ben@example.com", "wrong")) + require.False(t, canLogin(t, s, "nobody@example.com", "ben")) + }) +} + func TestAccount_PasswordReset_ByUsername(t *testing.T) { forEachBackend(t, func(t *testing.T, databaseURL string) { s, mailer, auth := newEmailTestServer(t, databaseURL) diff --git a/server/server_account_test.go b/server/server_account_test.go index cee5c57b..5f704053 100644 --- a/server/server_account_test.go +++ b/server/server_account_test.go @@ -55,6 +55,58 @@ func TestAccount_Signup_Success(t *testing.T) { }) } +func TestAccount_Login_Success(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + conf := newTestConfigWithAuthFile(t, databaseURL) + s := newTestServer(t, conf) + defer s.closeDatabases() + + require.Nil(t, s.userManager.AddUser("phil", "mypass", user.RoleUser, false)) + u, err := s.userManager.User("phil") + require.Nil(t, err) + require.Nil(t, s.userManager.AddEmail(u.ID, "phil@example.com")) + require.Nil(t, s.userManager.SetPrimaryEmail(u.ID, "phil@example.com")) + + // Login by username returns a token and the canonical username + rr := request(t, s, "POST", "/v1/account/login", "", map[string]string{ + "Authorization": util.BasicAuth("phil", "mypass"), + }) + require.Equal(t, 200, rr.Code) + resp, _ := util.UnmarshalJSON[apiAccountLoginResponse](io.NopCloser(rr.Body)) + require.True(t, strings.HasPrefix(resp.Token, "tk_")) + require.Equal(t, "phil", resp.Username) + + // The returned token actually authenticates + rr = request(t, s, "GET", "/v1/account", "", map[string]string{ + "Authorization": util.BearerAuth(resp.Token), + }) + require.Equal(t, 200, rr.Code) + + // Login by primary email returns the canonical username, not the email that was typed + rr = request(t, s, "POST", "/v1/account/login", "", map[string]string{ + "Authorization": util.BasicAuth("phil@example.com", "mypass"), + }) + require.Equal(t, 200, rr.Code) + resp, _ = util.UnmarshalJSON[apiAccountLoginResponse](io.NopCloser(rr.Body)) + require.True(t, strings.HasPrefix(resp.Token, "tk_")) + require.Equal(t, "phil", resp.Username) + }) +} + +func TestAccount_Login_InvalidCredentials(t *testing.T) { + forEachBackend(t, func(t *testing.T, databaseURL string) { + conf := newTestConfigWithAuthFile(t, databaseURL) + s := newTestServer(t, conf) + defer s.closeDatabases() + require.Nil(t, s.userManager.AddUser("phil", "mypass", user.RoleUser, false)) + + rr := request(t, s, "POST", "/v1/account/login", "", map[string]string{ + "Authorization": util.BasicAuth("phil", "wrongpass"), + }) + require.Equal(t, 401, rr.Code) + }) +} + func TestAccount_Signup_UserExists(t *testing.T) { forEachBackend(t, func(t *testing.T, databaseURL string) { conf := newTestConfigWithAuthFile(t, databaseURL) diff --git a/server/types.go b/server/types.go index 5c5e65ec..2b9f76f7 100644 --- a/server/types.go +++ b/server/types.go @@ -217,6 +217,14 @@ type apiAccountTokenResponse struct { Provisioned bool `json:"provisioned,omitempty"` // True if this token was provisioned by the server config } +// apiAccountLoginResponse is the body of POST /v1/account/login: it authenticates a +// username-or-email + password, mints a session token, and returns the token together with the +// canonical username (which may differ from the identifier the user typed, e.g. a primary email). +type apiAccountLoginResponse struct { + Token string `json:"token"` + Username string `json:"username"` +} + type apiAccountPhoneNumberVerifyRequest struct { Number string `json:"number"` Channel string `json:"channel"` diff --git a/user/manager.go b/user/manager.go index 023243e7..da9190a6 100644 --- a/user/manager.go +++ b/user/manager.go @@ -153,24 +153,26 @@ func (a *Manager) asyncExpiredMagicLinkReapLoop(interval time.Duration) { } } -// Authenticate checks username and password and returns a User if correct, and the user has not been -// marked as deleted. The method returns in constant-ish time, regardless of whether the user exists or -// the password is correct or incorrect. -func (a *Manager) Authenticate(username, password string) (*User, error) { - if username == Everyone { +// Authenticate checks a login identifier (a username or a verified primary email) and password, and +// returns a User if correct and not marked as deleted. The identifier is resolved in a single query +// via userByNameOrEmail, so a user can log in with either their username or their primary +// email. The method returns in constant-ish time (one query, one bcrypt compare), regardless of +// whether the identifier exists or the password is correct or incorrect. +func (a *Manager) Authenticate(identifier, password string) (*User, error) { + if identifier == Everyone { return nil, ErrUnauthenticated } - user, err := a.User(username) + user, err := a.userByNameOrEmail(identifier) if err != nil { - log.Tag(tag).Field("user_name", username).Err(err).Trace("Authentication of user failed (1)") + log.Tag(tag).Field("user_name", identifier).Err(err).Trace("Authentication of user failed (1)") bcrypt.CompareHashAndPassword([]byte(userAuthIntentionalSlowDownHash), []byte("intentional slow-down to avoid timing attacks")) return nil, ErrUnauthenticated } else if user.Deleted { - log.Tag(tag).Field("user_name", username).Trace("Authentication of user failed (2): user marked deleted") + log.Tag(tag).Field("user_name", identifier).Trace("Authentication of user failed (2): user marked deleted") bcrypt.CompareHashAndPassword([]byte(userAuthIntentionalSlowDownHash), []byte("intentional slow-down to avoid timing attacks")) return nil, ErrUnauthenticated } else if err := bcrypt.CompareHashAndPassword([]byte(user.Hash), []byte(password)); err != nil { - log.Tag(tag).Field("user_name", username).Err(err).Trace("Authentication of user failed (3)") + log.Tag(tag).Field("user_name", identifier).Err(err).Trace("Authentication of user failed (3)") return nil, ErrUnauthenticated } return user, nil @@ -532,6 +534,21 @@ func (a *Manager) UserByEmailOrUsername(identifier string) (*User, error) { return a.User(identifier) } +// userByNameOrEmail resolves a login identifier to a single user in one query, matching it +// against the username first and a verified primary email address second. This is the INVERSE +// precedence of UserByEmailOrUsername (used by password reset): at login a freely-chosen username +// must win over a look-alike primary email, so a user whose username happens to equal another +// account's email is not locked out of their own account. Because Authenticate still gates the match +// on a password check, returning the username owner here never grants access to the email owner's +// account. Returns ErrUserNotFound if neither matches. +func (a *Manager) userByNameOrEmail(identifier string) (*User, error) { + rows, err := a.db.Query(a.queries.selectUserByNameOrPrimaryEmail, identifier, identifier, identifier) + if err != nil { + return nil, err + } + return a.readUser(rows) +} + // userByToken returns the user with the given token if it exists and is not expired, or ErrUserNotFound otherwise func (a *Manager) userByToken(token string) (*User, error) { rows, err := a.db.Query(a.queries.selectUserByToken, token, time.Now().Unix()) diff --git a/user/manager_postgres.go b/user/manager_postgres.go index ad6d63b8..0fa2604c 100644 --- a/user/manager_postgres.go +++ b/user/manager_postgres.go @@ -33,6 +33,15 @@ const ( LEFT JOIN tier t on t.id = u.tier_id WHERE user_name = $1 ` + postgresSelectUserByNameOrPrimaryEmailQuery = ` + SELECT u.id, u.user_name, u.pass, u.role, u.prefs, u.sync_topic, u.provisioned, u.stats_messages, u.stats_emails, u.stats_calls, u.stripe_customer_id, u.stripe_subscription_id, u.stripe_subscription_status, u.stripe_subscription_interval, u.stripe_subscription_paid_until, u.stripe_subscription_cancel_at, u.deleted, t.id, t.code, t.name, t.messages_limit, t.messages_expiry_duration, t.emails_limit, t.calls_limit, t.reservations_limit, t.attachment_file_size_limit, t.attachment_total_size_limit, t.attachment_expiry_duration, t.attachment_bandwidth_limit, t.stripe_monthly_price_id, t.stripe_yearly_price_id + FROM "user" u + LEFT JOIN tier t on t.id = u.tier_id + WHERE u.user_name = $1 + OR u.id = (SELECT user_id FROM user_email WHERE email = $2 AND is_primary) + ORDER BY CASE WHEN u.user_name = $3 THEN 0 ELSE 1 END + LIMIT 1 + ` postgresSelectUserByTokenQuery = ` SELECT u.id, u.user_name, u.pass, u.role, u.prefs, u.sync_topic, u.provisioned, u.stats_messages, u.stats_emails, u.stats_calls, u.stripe_customer_id, u.stripe_subscription_id, u.stripe_subscription_status, u.stripe_subscription_interval, u.stripe_subscription_paid_until, u.stripe_subscription_cancel_at, u.deleted, t.id, t.code, t.name, t.messages_limit, t.messages_expiry_duration, t.emails_limit, t.calls_limit, t.reservations_limit, t.attachment_file_size_limit, t.attachment_total_size_limit, t.attachment_expiry_duration, t.attachment_bandwidth_limit, t.stripe_monthly_price_id, t.stripe_yearly_price_id FROM "user" u @@ -260,80 +269,81 @@ func postgresSelectAccessCacheUsersQuery(n int) string { // NewPostgresManager creates a new Manager backed by a PostgreSQL database using an existing connection pool. var postgresQueries = queries{ - selectUserByID: postgresSelectUserByIDQuery, - selectUserByName: postgresSelectUserByNameQuery, - selectUserByToken: postgresSelectUserByTokenQuery, - selectUserByStripeCustomerID: postgresSelectUserByStripeCustomerIDQuery, - selectUsernames: postgresSelectUsernamesQuery, - selectUsers: postgresSelectUsersQuery, - selectUserCount: postgresSelectUserCountQuery, - selectUserIDFromUsername: postgresSelectUserIDFromUsernameQuery, - insertUser: postgresInsertUserQuery, - updateUserPass: postgresUpdateUserPassQuery, - updateUserRole: postgresUpdateUserRoleQuery, - updateUserProvisioned: postgresUpdateUserProvisionedQuery, - updateUserPrefs: postgresUpdateUserPrefsQuery, - updateUserStats: postgresUpdateUserStatsQuery, - updateUserStatsResetAll: postgresUpdateUserStatsResetAllQuery, - updateUserTier: postgresUpdateUserTierQuery, - updateUserDeleted: postgresUpdateUserDeletedQuery, - deleteUser: postgresDeleteUserQuery, - deleteUserTier: postgresDeleteUserTierQuery, - deleteUsersMarked: postgresDeleteUsersMarkedQuery, - deleteUsersProvisioned: postgresDeleteUsersProvisionedQuery, - selectTopicPerms: postgresSelectTopicPermsQuery, - selectAccessCacheAll: postgresSelectAccessCacheAllQuery, - selectAccessCacheUsers: postgresSelectAccessCacheUsersQuery, - selectUserAllAccess: postgresSelectUserAllAccessQuery, - selectUserAccess: postgresSelectUserAccessQuery, - selectUserReservations: postgresSelectUserReservationsQuery, - selectUserReservationsCount: postgresSelectUserReservationsCountQuery, - selectUserReservationsOwner: postgresSelectUserReservationsOwnerQuery, - selectUserHasReservation: postgresSelectUserHasReservationQuery, - selectOtherAccessCount: postgresSelectOtherAccessCountQuery, - upsertUserAccess: postgresUpsertUserAccessQuery, - deleteUserAccess: postgresDeleteUserAccessQuery, - deleteUserAccessProvisioned: postgresDeleteUserAccessProvisionedQuery, - deleteTopicAccess: postgresDeleteTopicAccessQuery, - deleteAllAccess: postgresDeleteAllAccessQuery, - selectToken: postgresSelectTokenQuery, - selectTokens: postgresSelectTokensQuery, - selectTokenCount: postgresSelectTokenCountQuery, - selectAllProvisionedTokens: postgresSelectAllProvisionedTokensQuery, - upsertToken: postgresUpsertTokenQuery, - updateToken: postgresUpdateTokenQuery, - updateTokenLastAccess: postgresUpdateTokenLastAccessQuery, - deleteToken: postgresDeleteTokenQuery, - deleteProvisionedToken: postgresDeleteProvisionedTokenQuery, - deleteAllProvisionedTokens: postgresDeleteAllProvisionedTokensQuery, - deleteAllToken: postgresDeleteAllTokenQuery, - deleteExpiredTokens: postgresDeleteExpiredTokensQuery, - deleteExcessTokens: postgresDeleteExcessTokensQuery, - insertTier: postgresInsertTierQuery, - selectTiers: postgresSelectTiersQuery, - selectTierByCode: postgresSelectTierByCodeQuery, - selectTierByPriceID: postgresSelectTierByPriceIDQuery, - updateTier: postgresUpdateTierQuery, - deleteTier: postgresDeleteTierQuery, - selectPhoneNumbers: postgresSelectPhoneNumbersQuery, - insertPhoneNumber: postgresInsertPhoneNumberQuery, - deletePhoneNumber: postgresDeletePhoneNumberQuery, - selectEmails: postgresSelectEmailsQuery, - insertEmail: postgresInsertEmailQuery, - insertEmailIgnore: postgresInsertEmailIgnoreQuery, - deleteEmail: postgresDeleteEmailQuery, - selectPrimaryEmail: postgresSelectPrimaryEmailQuery, - selectUserIDByPrimary: postgresSelectUserIDByPrimaryQuery, - updateEmailSetPrimary: postgresUpdateEmailSetPrimaryQuery, - updateEmailClearPrimary: postgresUpdateEmailClearPrimaryQuery, - insertMagicLink: postgresInsertMagicLinkQuery, - selectMagicLinkByHash: postgresSelectMagicLinkByHashQuery, - deleteMagicLinkByHash: postgresDeleteMagicLinkByHashQuery, - deleteMagicLinkEmailVerify: postgresDeleteVerifyScopeQuery, - deleteMagicLinkResetPassword: postgresDeleteResetScopeQuery, - selectPendingEmails: postgresSelectPendingEmailsQuery, - deleteExpiredMagicLinks: postgresDeleteExpiredMagicLinksQuery, - updateBilling: postgresUpdateBillingQuery, + selectUserByID: postgresSelectUserByIDQuery, + selectUserByName: postgresSelectUserByNameQuery, + selectUserByNameOrPrimaryEmail: postgresSelectUserByNameOrPrimaryEmailQuery, + selectUserByToken: postgresSelectUserByTokenQuery, + selectUserByStripeCustomerID: postgresSelectUserByStripeCustomerIDQuery, + selectUsernames: postgresSelectUsernamesQuery, + selectUsers: postgresSelectUsersQuery, + selectUserCount: postgresSelectUserCountQuery, + selectUserIDFromUsername: postgresSelectUserIDFromUsernameQuery, + insertUser: postgresInsertUserQuery, + updateUserPass: postgresUpdateUserPassQuery, + updateUserRole: postgresUpdateUserRoleQuery, + updateUserProvisioned: postgresUpdateUserProvisionedQuery, + updateUserPrefs: postgresUpdateUserPrefsQuery, + updateUserStats: postgresUpdateUserStatsQuery, + updateUserStatsResetAll: postgresUpdateUserStatsResetAllQuery, + updateUserTier: postgresUpdateUserTierQuery, + updateUserDeleted: postgresUpdateUserDeletedQuery, + deleteUser: postgresDeleteUserQuery, + deleteUserTier: postgresDeleteUserTierQuery, + deleteUsersMarked: postgresDeleteUsersMarkedQuery, + deleteUsersProvisioned: postgresDeleteUsersProvisionedQuery, + selectTopicPerms: postgresSelectTopicPermsQuery, + selectAccessCacheAll: postgresSelectAccessCacheAllQuery, + selectAccessCacheUsers: postgresSelectAccessCacheUsersQuery, + selectUserAllAccess: postgresSelectUserAllAccessQuery, + selectUserAccess: postgresSelectUserAccessQuery, + selectUserReservations: postgresSelectUserReservationsQuery, + selectUserReservationsCount: postgresSelectUserReservationsCountQuery, + selectUserReservationsOwner: postgresSelectUserReservationsOwnerQuery, + selectUserHasReservation: postgresSelectUserHasReservationQuery, + selectOtherAccessCount: postgresSelectOtherAccessCountQuery, + upsertUserAccess: postgresUpsertUserAccessQuery, + deleteUserAccess: postgresDeleteUserAccessQuery, + deleteUserAccessProvisioned: postgresDeleteUserAccessProvisionedQuery, + deleteTopicAccess: postgresDeleteTopicAccessQuery, + deleteAllAccess: postgresDeleteAllAccessQuery, + selectToken: postgresSelectTokenQuery, + selectTokens: postgresSelectTokensQuery, + selectTokenCount: postgresSelectTokenCountQuery, + selectAllProvisionedTokens: postgresSelectAllProvisionedTokensQuery, + upsertToken: postgresUpsertTokenQuery, + updateToken: postgresUpdateTokenQuery, + updateTokenLastAccess: postgresUpdateTokenLastAccessQuery, + deleteToken: postgresDeleteTokenQuery, + deleteProvisionedToken: postgresDeleteProvisionedTokenQuery, + deleteAllProvisionedTokens: postgresDeleteAllProvisionedTokensQuery, + deleteAllToken: postgresDeleteAllTokenQuery, + deleteExpiredTokens: postgresDeleteExpiredTokensQuery, + deleteExcessTokens: postgresDeleteExcessTokensQuery, + insertTier: postgresInsertTierQuery, + selectTiers: postgresSelectTiersQuery, + selectTierByCode: postgresSelectTierByCodeQuery, + selectTierByPriceID: postgresSelectTierByPriceIDQuery, + updateTier: postgresUpdateTierQuery, + deleteTier: postgresDeleteTierQuery, + selectPhoneNumbers: postgresSelectPhoneNumbersQuery, + insertPhoneNumber: postgresInsertPhoneNumberQuery, + deletePhoneNumber: postgresDeletePhoneNumberQuery, + selectEmails: postgresSelectEmailsQuery, + insertEmail: postgresInsertEmailQuery, + insertEmailIgnore: postgresInsertEmailIgnoreQuery, + deleteEmail: postgresDeleteEmailQuery, + selectPrimaryEmail: postgresSelectPrimaryEmailQuery, + selectUserIDByPrimary: postgresSelectUserIDByPrimaryQuery, + updateEmailSetPrimary: postgresUpdateEmailSetPrimaryQuery, + updateEmailClearPrimary: postgresUpdateEmailClearPrimaryQuery, + insertMagicLink: postgresInsertMagicLinkQuery, + selectMagicLinkByHash: postgresSelectMagicLinkByHashQuery, + deleteMagicLinkByHash: postgresDeleteMagicLinkByHashQuery, + deleteMagicLinkEmailVerify: postgresDeleteVerifyScopeQuery, + deleteMagicLinkResetPassword: postgresDeleteResetScopeQuery, + selectPendingEmails: postgresSelectPendingEmailsQuery, + deleteExpiredMagicLinks: postgresDeleteExpiredMagicLinksQuery, + updateBilling: postgresUpdateBillingQuery, } // NewPostgresManager creates a new Manager backed by a PostgreSQL database diff --git a/user/manager_sqlite.go b/user/manager_sqlite.go index 1d6f2fa1..f016a4e2 100644 --- a/user/manager_sqlite.go +++ b/user/manager_sqlite.go @@ -37,6 +37,15 @@ const ( LEFT JOIN tier t on t.id = u.tier_id WHERE user = ? ` + sqliteSelectUserByNameOrPrimaryEmailQuery = ` + SELECT u.id, u.user, u.pass, u.role, u.prefs, u.sync_topic, u.provisioned, u.stats_messages, u.stats_emails, u.stats_calls, u.stripe_customer_id, u.stripe_subscription_id, u.stripe_subscription_status, u.stripe_subscription_interval, u.stripe_subscription_paid_until, u.stripe_subscription_cancel_at, deleted, t.id, t.code, t.name, t.messages_limit, t.messages_expiry_duration, t.emails_limit, t.calls_limit, t.reservations_limit, t.attachment_file_size_limit, t.attachment_total_size_limit, t.attachment_expiry_duration, t.attachment_bandwidth_limit, t.stripe_monthly_price_id, t.stripe_yearly_price_id + FROM user u + LEFT JOIN tier t on t.id = u.tier_id + WHERE u.user = ? + OR u.id = (SELECT user_id FROM user_email WHERE email = ? AND is_primary = 1) + ORDER BY CASE WHEN u.user = ? THEN 0 ELSE 1 END + LIMIT 1 + ` sqliteSelectUserByTokenQuery = ` SELECT u.id, u.user, u.pass, u.role, u.prefs, u.sync_topic, u.provisioned, u.stats_messages, u.stats_emails, u.stats_calls, u.stripe_customer_id, u.stripe_subscription_id, u.stripe_subscription_status, u.stripe_subscription_interval, u.stripe_subscription_paid_until, u.stripe_subscription_cancel_at, deleted, t.id, t.code, t.name, t.messages_limit, t.messages_expiry_duration, t.emails_limit, t.calls_limit, t.reservations_limit, t.attachment_file_size_limit, t.attachment_total_size_limit, t.attachment_expiry_duration, t.attachment_bandwidth_limit, t.stripe_monthly_price_id, t.stripe_yearly_price_id FROM user u @@ -256,80 +265,81 @@ func sqliteSelectAccessCacheUsersQuery(n int) string { } var sqliteQueries = queries{ - selectUserByID: sqliteSelectUserByIDQuery, - selectUserByName: sqliteSelectUserByNameQuery, - selectUserByToken: sqliteSelectUserByTokenQuery, - selectUserByStripeCustomerID: sqliteSelectUserByStripeCustomerIDQuery, - selectUsernames: sqliteSelectUsernamesQuery, - selectUsers: sqliteSelectUsersQuery, - selectUserCount: sqliteSelectUserCountQuery, - selectUserIDFromUsername: sqliteSelectUserIDFromUsernameQuery, - insertUser: sqliteInsertUserQuery, - updateUserPass: sqliteUpdateUserPassQuery, - updateUserRole: sqliteUpdateUserRoleQuery, - updateUserProvisioned: sqliteUpdateUserProvisionedQuery, - updateUserPrefs: sqliteUpdateUserPrefsQuery, - updateUserStats: sqliteUpdateUserStatsQuery, - updateUserStatsResetAll: sqliteUpdateUserStatsResetAllQuery, - updateUserTier: sqliteUpdateUserTierQuery, - updateUserDeleted: sqliteUpdateUserDeletedQuery, - deleteUser: sqliteDeleteUserQuery, - deleteUserTier: sqliteDeleteUserTierQuery, - deleteUsersMarked: sqliteDeleteUsersMarkedQuery, - deleteUsersProvisioned: sqliteDeleteUsersProvisionedQuery, - selectTopicPerms: sqliteSelectTopicPermsQuery, - selectAccessCacheAll: sqliteSelectAccessCacheAllQuery, - selectAccessCacheUsers: sqliteSelectAccessCacheUsersQuery, - selectUserAllAccess: sqliteSelectUserAllAccessQuery, - selectUserAccess: sqliteSelectUserAccessQuery, - selectUserReservations: sqliteSelectUserReservationsQuery, - selectUserReservationsCount: sqliteSelectUserReservationsCountQuery, - selectUserReservationsOwner: sqliteSelectUserReservationsOwnerQuery, - selectUserHasReservation: sqliteSelectUserHasReservationQuery, - selectOtherAccessCount: sqliteSelectOtherAccessCountQuery, - upsertUserAccess: sqliteUpsertUserAccessQuery, - deleteUserAccess: sqliteDeleteUserAccessQuery, - deleteUserAccessProvisioned: sqliteDeleteUserAccessProvisionedQuery, - deleteTopicAccess: sqliteDeleteTopicAccessQuery, - deleteAllAccess: sqliteDeleteAllAccessQuery, - selectToken: sqliteSelectTokenQuery, - selectTokens: sqliteSelectTokensQuery, - selectTokenCount: sqliteSelectTokenCountQuery, - selectAllProvisionedTokens: sqliteSelectAllProvisionedTokensQuery, - upsertToken: sqliteUpsertTokenQuery, - updateToken: sqliteUpdateTokenQuery, - updateTokenLastAccess: sqliteUpdateTokenLastAccessQuery, - deleteToken: sqliteDeleteTokenQuery, - deleteProvisionedToken: sqliteDeleteProvisionedTokenQuery, - deleteAllProvisionedTokens: sqliteDeleteAllProvisionedTokensQuery, - deleteAllToken: sqliteDeleteAllTokenQuery, - deleteExpiredTokens: sqliteDeleteExpiredTokensQuery, - deleteExcessTokens: sqliteDeleteExcessTokensQuery, - insertTier: sqliteInsertTierQuery, - selectTiers: sqliteSelectTiersQuery, - selectTierByCode: sqliteSelectTierByCodeQuery, - selectTierByPriceID: sqliteSelectTierByPriceIDQuery, - updateTier: sqliteUpdateTierQuery, - deleteTier: sqliteDeleteTierQuery, - selectPhoneNumbers: sqliteSelectPhoneNumbersQuery, - insertPhoneNumber: sqliteInsertPhoneNumberQuery, - deletePhoneNumber: sqliteDeletePhoneNumberQuery, - selectEmails: sqliteSelectEmailsQuery, - insertEmail: sqliteInsertEmailQuery, - insertEmailIgnore: sqliteInsertEmailIgnoreQuery, - deleteEmail: sqliteDeleteEmailQuery, - selectPrimaryEmail: sqliteSelectPrimaryEmailQuery, - selectUserIDByPrimary: sqliteSelectUserIDByPrimaryQuery, - updateEmailSetPrimary: sqliteUpdateEmailSetPrimaryQuery, - updateEmailClearPrimary: sqliteUpdateEmailClearPrimaryQuery, - insertMagicLink: sqliteInsertMagicLinkQuery, - selectMagicLinkByHash: sqliteSelectMagicLinkByHashQuery, - deleteMagicLinkByHash: sqliteDeleteMagicLinkByHashQuery, - deleteMagicLinkEmailVerify: sqliteDeleteVerifyScopeQuery, - deleteMagicLinkResetPassword: sqliteDeleteResetScopeQuery, - selectPendingEmails: sqliteSelectPendingEmailsQuery, - deleteExpiredMagicLinks: sqliteDeleteExpiredMagicLinksQuery, - updateBilling: sqliteUpdateBillingQuery, + selectUserByID: sqliteSelectUserByIDQuery, + selectUserByName: sqliteSelectUserByNameQuery, + selectUserByNameOrPrimaryEmail: sqliteSelectUserByNameOrPrimaryEmailQuery, + selectUserByToken: sqliteSelectUserByTokenQuery, + selectUserByStripeCustomerID: sqliteSelectUserByStripeCustomerIDQuery, + selectUsernames: sqliteSelectUsernamesQuery, + selectUsers: sqliteSelectUsersQuery, + selectUserCount: sqliteSelectUserCountQuery, + selectUserIDFromUsername: sqliteSelectUserIDFromUsernameQuery, + insertUser: sqliteInsertUserQuery, + updateUserPass: sqliteUpdateUserPassQuery, + updateUserRole: sqliteUpdateUserRoleQuery, + updateUserProvisioned: sqliteUpdateUserProvisionedQuery, + updateUserPrefs: sqliteUpdateUserPrefsQuery, + updateUserStats: sqliteUpdateUserStatsQuery, + updateUserStatsResetAll: sqliteUpdateUserStatsResetAllQuery, + updateUserTier: sqliteUpdateUserTierQuery, + updateUserDeleted: sqliteUpdateUserDeletedQuery, + deleteUser: sqliteDeleteUserQuery, + deleteUserTier: sqliteDeleteUserTierQuery, + deleteUsersMarked: sqliteDeleteUsersMarkedQuery, + deleteUsersProvisioned: sqliteDeleteUsersProvisionedQuery, + selectTopicPerms: sqliteSelectTopicPermsQuery, + selectAccessCacheAll: sqliteSelectAccessCacheAllQuery, + selectAccessCacheUsers: sqliteSelectAccessCacheUsersQuery, + selectUserAllAccess: sqliteSelectUserAllAccessQuery, + selectUserAccess: sqliteSelectUserAccessQuery, + selectUserReservations: sqliteSelectUserReservationsQuery, + selectUserReservationsCount: sqliteSelectUserReservationsCountQuery, + selectUserReservationsOwner: sqliteSelectUserReservationsOwnerQuery, + selectUserHasReservation: sqliteSelectUserHasReservationQuery, + selectOtherAccessCount: sqliteSelectOtherAccessCountQuery, + upsertUserAccess: sqliteUpsertUserAccessQuery, + deleteUserAccess: sqliteDeleteUserAccessQuery, + deleteUserAccessProvisioned: sqliteDeleteUserAccessProvisionedQuery, + deleteTopicAccess: sqliteDeleteTopicAccessQuery, + deleteAllAccess: sqliteDeleteAllAccessQuery, + selectToken: sqliteSelectTokenQuery, + selectTokens: sqliteSelectTokensQuery, + selectTokenCount: sqliteSelectTokenCountQuery, + selectAllProvisionedTokens: sqliteSelectAllProvisionedTokensQuery, + upsertToken: sqliteUpsertTokenQuery, + updateToken: sqliteUpdateTokenQuery, + updateTokenLastAccess: sqliteUpdateTokenLastAccessQuery, + deleteToken: sqliteDeleteTokenQuery, + deleteProvisionedToken: sqliteDeleteProvisionedTokenQuery, + deleteAllProvisionedTokens: sqliteDeleteAllProvisionedTokensQuery, + deleteAllToken: sqliteDeleteAllTokenQuery, + deleteExpiredTokens: sqliteDeleteExpiredTokensQuery, + deleteExcessTokens: sqliteDeleteExcessTokensQuery, + insertTier: sqliteInsertTierQuery, + selectTiers: sqliteSelectTiersQuery, + selectTierByCode: sqliteSelectTierByCodeQuery, + selectTierByPriceID: sqliteSelectTierByPriceIDQuery, + updateTier: sqliteUpdateTierQuery, + deleteTier: sqliteDeleteTierQuery, + selectPhoneNumbers: sqliteSelectPhoneNumbersQuery, + insertPhoneNumber: sqliteInsertPhoneNumberQuery, + deletePhoneNumber: sqliteDeletePhoneNumberQuery, + selectEmails: sqliteSelectEmailsQuery, + insertEmail: sqliteInsertEmailQuery, + insertEmailIgnore: sqliteInsertEmailIgnoreQuery, + deleteEmail: sqliteDeleteEmailQuery, + selectPrimaryEmail: sqliteSelectPrimaryEmailQuery, + selectUserIDByPrimary: sqliteSelectUserIDByPrimaryQuery, + updateEmailSetPrimary: sqliteUpdateEmailSetPrimaryQuery, + updateEmailClearPrimary: sqliteUpdateEmailClearPrimaryQuery, + insertMagicLink: sqliteInsertMagicLinkQuery, + selectMagicLinkByHash: sqliteSelectMagicLinkByHashQuery, + deleteMagicLinkByHash: sqliteDeleteMagicLinkByHashQuery, + deleteMagicLinkEmailVerify: sqliteDeleteVerifyScopeQuery, + deleteMagicLinkResetPassword: sqliteDeleteResetScopeQuery, + selectPendingEmails: sqliteSelectPendingEmailsQuery, + deleteExpiredMagicLinks: sqliteDeleteExpiredMagicLinksQuery, + updateBilling: sqliteUpdateBillingQuery, } // NewSQLiteManager creates a new Manager backed by a SQLite database diff --git a/user/manager_test.go b/user/manager_test.go index a7b205b0..d2fa3b9e 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -2947,6 +2947,108 @@ func TestUser_MagicLink_PrimaryGlobalUniqueness(t *testing.T) { }) } +func TestManager_Authenticate_ByPrimaryEmail(t *testing.T) { + forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { + a := newTestManager(t, newManager, PermissionDenyAll) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) + phil, err := a.User("phil") + require.Nil(t, err) + + // phil verifies phil@example.com -> becomes his primary (recovery) email + _, err = a.VerifyEmail(addVerifyLink(t, a, phil.ID, "phil@example.com", 24*time.Hour)) + require.Nil(t, err) + + // Login by username still works + u, err := a.Authenticate("phil", "phil") + require.Nil(t, err) + require.Equal(t, "phil", u.Name) + + // Login by primary email works and resolves to the same account + u, err = a.Authenticate("phil@example.com", "phil") + require.Nil(t, err) + require.Equal(t, "phil", u.Name) + + // Login by primary email with the wrong password fails + u, err = a.Authenticate("phil@example.com", "wrong") + require.Nil(t, u) + require.Equal(t, ErrUnauthenticated, err) + + // An unknown email fails + u, err = a.Authenticate("nobody@example.com", "phil") + require.Nil(t, u) + require.Equal(t, ErrUnauthenticated, err) + }) +} + +func TestManager_Authenticate_BySecondaryEmailDenied(t *testing.T) { + forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { + a := newTestManager(t, newManager, PermissionDenyAll) + require.Nil(t, a.AddUser("phil", "phil", RoleUser, false)) + require.Nil(t, a.AddUser("ben", "ben", RoleUser, false)) + phil, err := a.User("phil") + require.Nil(t, err) + ben, err := a.User("ben") + require.Nil(t, err) + + // phil verifies shared@ first -> his primary; ben verifies it too -> only secondary for ben + _, err = a.VerifyEmail(addVerifyLink(t, a, phil.ID, "shared@example.com", 24*time.Hour)) + require.Nil(t, err) + _, err = a.VerifyEmail(addVerifyLink(t, a, ben.ID, "shared@example.com", 24*time.Hour)) + require.Nil(t, err) + + // Login by the shared address resolves to the primary owner (phil), never the secondary (ben) + u, err := a.Authenticate("shared@example.com", "phil") + require.Nil(t, err) + require.Equal(t, "phil", u.Name) + + // ben's password must not authenticate via the shared address (it is not his primary) + u, err = a.Authenticate("shared@example.com", "ben") + require.Nil(t, u) + require.Equal(t, ErrUnauthenticated, err) + }) +} + +func TestManager_Authenticate_UsernameLookalikeEmailPrecedence(t *testing.T) { + forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { + a := newTestManager(t, newManager, PermissionDenyAll) + + // The collision: a squatter whose USERNAME is literally "phil@example.com" (usernames may + // contain '@' and '.'), and a different account (ben) that owns "phil@example.com" as its + // verified primary email. Both are reachable; nothing links usernames to email addresses. + require.Nil(t, a.AddUser("phil@example.com", "squatterpass", RoleUser, false)) + require.Nil(t, a.AddUser("ben", "benpass", RoleUser, false)) + ben, err := a.User("ben") + require.Nil(t, err) + _, err = a.VerifyEmail(addVerifyLink(t, a, ben.ID, "phil@example.com", 24*time.Hour)) + require.Nil(t, err) + + // Login resolves the ambiguous identifier username-FIRST (the ORDER BY CASE in the query): + // the squatter owns the login, and returns deterministically even though both rows match. + squatter, err := a.Authenticate("phil@example.com", "squatterpass") + require.Nil(t, err) + require.Equal(t, "phil@example.com", squatter.Name) + + // Consequently the email owner's password does NOT authenticate via the colliding identifier + // at login, but the owner is not locked out: their real username still works. + u, err := a.Authenticate("phil@example.com", "benpass") + require.Nil(t, u) + require.Equal(t, ErrUnauthenticated, err) + u, err = a.Authenticate("ben", "benpass") + require.Nil(t, err) + require.Equal(t, "ben", u.Name) + + // The inverse: password reset (UserByEmailOrUsername) resolves the SAME identifier email-FIRST, + // so the reset link goes to the verified email owner (ben), never the look-alike username. The + // two flows deliberately use opposite precedence. + loginUser, err := a.userByNameOrEmail("phil@example.com") + require.Nil(t, err) + require.Equal(t, "phil@example.com", loginUser.Name) // username owner (squatter) + resetUser, err := a.UserByEmailOrUsername("phil@example.com") + require.Nil(t, err) + require.Equal(t, "ben", resetUser.Name) // email owner + }) +} + func TestUser_MagicLink_SetPrimary_NotVerified(t *testing.T) { forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { a := newTestManager(t, newManager, PermissionDenyAll) diff --git a/user/types.go b/user/types.go index e416113a..30be6941 100644 --- a/user/types.go +++ b/user/types.go @@ -47,10 +47,10 @@ func (u *User) IsUser() bool { // Auther is an interface for authentication and authorization type Auther interface { - // Authenticate checks username and password and returns a user if correct. The method - // returns in constant-ish time, regardless of whether the user exists or the password is - // correct or incorrect. - Authenticate(username, password string) (*User, error) + // Authenticate checks a login identifier (username or verified primary email) and password + // and returns a user if correct. The method returns in constant-ish time, regardless of + // whether the identifier exists or the password is correct or incorrect. + Authenticate(identifier, password string) (*User, error) // Authorize returns nil if the given user has access to the given topic using the desired // permission. The user param may be nil to signal an anonymous user. @@ -338,27 +338,28 @@ var ( // queries holds the database-specific SQL queries type queries struct { // User queries - selectUserByID string - selectUserByName string - selectUserByToken string - selectUserByStripeCustomerID string - selectUsernames string - selectUsers string - selectUserCount string - selectUserIDFromUsername string - insertUser string - updateUserPass string - updateUserRole string - updateUserProvisioned string - updateUserPrefs string - updateUserStats string - updateUserStatsResetAll string - updateUserTier string - updateUserDeleted string - deleteUser string - deleteUserTier string - deleteUsersMarked string - deleteUsersProvisioned string + selectUserByID string + selectUserByName string + selectUserByNameOrPrimaryEmail string + selectUserByToken string + selectUserByStripeCustomerID string + selectUsernames string + selectUsers string + selectUserCount string + selectUserIDFromUsername string + insertUser string + updateUserPass string + updateUserRole string + updateUserProvisioned string + updateUserPrefs string + updateUserStats string + updateUserStatsResetAll string + updateUserTier string + updateUserDeleted string + deleteUser string + deleteUserTier string + deleteUsersMarked string + deleteUsersProvisioned string // Access queries selectTopicPerms string // Direct-DB authorizeTopicAccess query; used when the in-memory cache is disabled diff --git a/web/public/static/langs/en.json b/web/public/static/langs/en.json index b55568a0..46fbb32f 100644 --- a/web/public/static/langs/en.json +++ b/web/public/static/langs/en.json @@ -26,6 +26,7 @@ "signup_error_username_taken": "Username {{username}} is already taken", "signup_error_creation_limit_reached": "Account creation limit reached", "login_title": "Sign in to your ntfy account", + "login_form_username_label": "Username or email", "login_form_button_submit": "Sign in", "login_link_signup": "Sign up", "login_link_forgot_password": "Forgot password", diff --git a/web/src/app/AccountApi.js b/web/src/app/AccountApi.js index 0cc65c2c..6b731ca3 100644 --- a/web/src/app/AccountApi.js +++ b/web/src/app/AccountApi.js @@ -6,6 +6,7 @@ import { accountEmailVerifyUrl, accountEmailPrimaryUrl, accountEmailResendUrl, + accountLoginUrl, accountPasswordResetRequestUrl, accountPasswordResetUrl, accountPasswordUrl, @@ -47,8 +48,8 @@ class AccountApi { } async login(user) { - const url = accountTokenUrl(config.base_url); - console.log(`[AccountApi] Checking auth for ${url}`); + const url = accountLoginUrl(config.base_url); + console.log(`[AccountApi] Logging in at ${url}`); const response = await fetchOrThrow(url, { method: "POST", headers: withBasicAuth({}, user.username, user.password), @@ -57,7 +58,10 @@ class AccountApi { if (!json.token) { throw new Error(`Unexpected server response: Cannot find token`); } - return json.token; + // The identifier the user typed may be a primary email; login returns the canonical username + // so callers can store it and show the real username rather than whatever was typed. Fall back + // to the typed identifier if an older server omits the username, so the session still stores one. + return { token: json.token, username: json.username || user.username }; } async logout() { diff --git a/web/src/app/AccountApi.test.js b/web/src/app/AccountApi.test.js index 79348c2c..af3977cc 100644 --- a/web/src/app/AccountApi.test.js +++ b/web/src/app/AccountApi.test.js @@ -42,15 +42,19 @@ afterEach(() => { }); describe("AccountApi.login", () => { - it("POSTs basic auth to the token URL and returns the token", async () => { - fetchMock.mockResolvedValue(ok({ token: "tk_returned" })); - const token = await accountApi.login({ username: "phil", password: "secret" }); + it("POSTs basic auth to the login URL and returns the token and canonical username", async () => { + // The typed identifier is an email; the login endpoint returns the canonical username in one + // request, so login() surfaces it (callers store it) rather than echoing what was typed. + fetchMock.mockResolvedValue(ok({ token: "tk_returned", username: "phil" })); + const result = await accountApi.login({ username: "phil@example.com", password: "secret" }); + + expect(result).toEqual({ token: "tk_returned", username: "phil" }); + expect(fetchMock).toHaveBeenCalledTimes(1); - expect(token).toBe("tk_returned"); const [url, options] = fetchMock.mock.calls[0]; - expect(url).toBe("https://ntfy.sh/v1/account/token"); + expect(url).toBe("https://ntfy.sh/v1/account/login"); expect(options.method).toBe("POST"); - expect(options.headers.Authorization).toBe(`Basic ${btoa("phil:secret")}`); + expect(options.headers.Authorization).toBe(`Basic ${btoa("phil@example.com:secret")}`); }); it("throws when the server response has no token", async () => { diff --git a/web/src/app/utils.js b/web/src/app/utils.js index 1ea39541..c35f890e 100644 --- a/web/src/app/utils.js +++ b/web/src/app/utils.js @@ -23,6 +23,7 @@ export const topicUrlAuth = (baseUrl, topic) => `${topicUrl(baseUrl, topic)}/aut export const topicShortUrl = (baseUrl, topic) => shortUrl(topicUrl(baseUrl, topic)); export const webPushUrl = (baseUrl) => `${baseUrl}/v1/webpush`; export const accountUrl = (baseUrl) => `${baseUrl}/v1/account`; +export const accountLoginUrl = (baseUrl) => `${baseUrl}/v1/account/login`; export const accountPasswordUrl = (baseUrl) => `${baseUrl}/v1/account/password`; export const accountTokenUrl = (baseUrl) => `${baseUrl}/v1/account/token`; export const accountSettingsUrl = (baseUrl) => `${baseUrl}/v1/account/settings`; diff --git a/web/src/components/Login.jsx b/web/src/components/Login.jsx index 18eec626..8fc4f626 100644 --- a/web/src/components/Login.jsx +++ b/web/src/components/Login.jsx @@ -24,14 +24,14 @@ const Login = () => { event.preventDefault(); const user = { username, password }; try { - const token = await accountApi.login(user); - console.log(`[Login] User auth for user ${user.username} successful, token is ${token}`); - await session.store(user.username, token); + const { token, username: canonicalUsername } = await accountApi.login(user); + console.log(`[Login] User auth for user ${user.username} successful, logged in as ${canonicalUsername}`); + await session.store(canonicalUsername, token); fadeReload(routes.app); } catch (e) { console.log(`[Login] User auth for user ${user.username} failed`, e); if (e instanceof UnauthorizedError) { - setError(t("Login failed: Invalid username or password")); + setError(t("Login failed: Invalid username/email or password")); } else { setError(e.message); } @@ -53,7 +53,7 @@ const Login = () => { required fullWidth id="username" - label={t("signup_form_username")} + label={t("login_form_username_label")} name="username" value={username} onChange={(ev) => setUsername(ev.target.value.trim())} diff --git a/web/src/components/Signup.jsx b/web/src/components/Signup.jsx index 81379d5b..4421bdb6 100644 --- a/web/src/components/Signup.jsx +++ b/web/src/components/Signup.jsx @@ -28,9 +28,9 @@ const Signup = () => { const user = { username, password }; try { await accountApi.create(user.username, user.password, email); - const token = await accountApi.login(user); - console.log(`[Signup] User signup for user ${user.username} successful, token is ${token}`); - await session.store(user.username, token); + const { token, username: canonicalUsername } = await accountApi.login(user); + console.log(`[Signup] User signup for user ${user.username} successful, logged in as ${canonicalUsername}`); + await session.store(canonicalUsername, token); fadeReload(routes.app); } catch (e) { console.log(`[Signup] Signup for user ${user.username} failed`, e);