diff --git a/cmd/user_test.go b/cmd/user_test.go index 7a1d5378..ed6f5de4 100644 --- a/cmd/user_test.go +++ b/cmd/user_test.go @@ -60,6 +60,9 @@ func TestCLI_User_Add_Password_Mismatch(t *testing.T) { func TestCLI_User_ChangePass(t *testing.T) { s, conf, port := newTestServerWithAuth(t) + conf.AuthUsers = []*user.User{ + {Name: "philuser", Hash: "$2a$10$U4WSIYY6evyGmZaraavM2e2JeVG6EMGUKN1uUwufUeeRd4Jpg6cGC", Role: user.RoleUser}, // philuser:philpass + } defer test.StopServer(t, s, port) // Add user @@ -73,6 +76,11 @@ func TestCLI_User_ChangePass(t *testing.T) { stdin.WriteString("newpass\nnewpass") require.Nil(t, runUserCommand(app, conf, "change-pass", "phil")) require.Contains(t, stdout.String(), "changed password for user phil") + + // Cannot change provisioned user's pass + app, stdin, _, _ = newTestApp() + stdin.WriteString("newpass\nnewpass") + require.Error(t, runUserCommand(app, conf, "change-pass", "philuser")) } func TestCLI_User_ChangeRole(t *testing.T) { diff --git a/server/errors.go b/server/errors.go index c6745779..1ab12f7c 100644 --- a/server/errors.go +++ b/server/errors.go @@ -132,6 +132,7 @@ var ( errHTTPConflictTopicReserved = &errHTTP{40902, http.StatusConflict, "conflict: access control entry for topic or topic pattern already exists", "", nil} errHTTPConflictSubscriptionExists = &errHTTP{40903, http.StatusConflict, "conflict: topic subscription already exists", "", nil} errHTTPConflictPhoneNumberExists = &errHTTP{40904, http.StatusConflict, "conflict: phone number already exists", "", nil} + errHTTPConflictProvisionedUserPasswordChange = &errHTTP{40905, http.StatusConflict, "conflict: cannot change password of provisioned user", "", 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_account.go b/server/server_account.go index f6dcab54..4fe392b1 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -208,6 +208,9 @@ func (s *Server) handleAccountPasswordChange(w http.ResponseWriter, r *http.Requ } logvr(v, r).Tag(tagAccount).Debug("Changing password for user %s", u.Name) if err := s.userManager.ChangePassword(u.Name, req.NewPassword, false); err != nil { + if errors.Is(err, user.ErrProvisionedUserPasswordChange) { + return errHTTPConflictProvisionedUserPasswordChange + } return err } return s.writeJSON(w, newSuccessResponse()) diff --git a/server/server_account_test.go b/server/server_account_test.go index ba6d0850..409ccfcf 100644 --- a/server/server_account_test.go +++ b/server/server_account_test.go @@ -251,7 +251,11 @@ func TestAccount_Subscription_AddUpdateDelete(t *testing.T) { } func TestAccount_ChangePassword(t *testing.T) { - s := newTestServer(t, newTestConfigWithAuthFile(t)) + conf := newTestConfigWithAuthFile(t) + conf.AuthUsers = []*user.User{ + {Name: "philuser", Hash: "$2a$10$U4WSIYY6evyGmZaraavM2e2JeVG6EMGUKN1uUwufUeeRd4Jpg6cGC", Role: user.RoleUser}, // philuser:philpass + } + s := newTestServer(t, conf) defer s.closeDatabases() require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser, false)) @@ -281,6 +285,12 @@ func TestAccount_ChangePassword(t *testing.T) { "Authorization": util.BasicAuth("phil", "new password"), }) require.Equal(t, 200, rr.Code) + + // Cannot change password of provisioned user + rr = request(t, s, "POST", "/v1/account/password", `{"password": "philpass", "new_password": "new password"}`, map[string]string{ + "Authorization": util.BasicAuth("philuser", "philpass"), + }) + require.Equal(t, 409, rr.Code) } func TestAccount_ChangePassword_NoAccount(t *testing.T) { diff --git a/user/manager.go b/user/manager.go index e24eb542..76abff9d 100644 --- a/user/manager.go +++ b/user/manager.go @@ -1389,6 +1389,14 @@ func (a *Manager) ReservationOwner(topic string) (string, error) { // ChangePassword changes a user's password func (a *Manager) ChangePassword(username, password string, hashed bool) error { + user, err := a.User(username) + if err != nil { + return err + } + if user.Provisioned { + return ErrProvisionedUserPasswordChange + } + return execTx(a.db, func(tx *sql.Tx) error { return a.changePasswordTx(tx, username, password, hashed) }) diff --git a/user/manager_test.go b/user/manager_test.go index d51b9b96..c2ae8b87 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -1209,6 +1209,9 @@ func TestManager_WithProvisionedUsers(t *testing.T) { require.Equal(t, "tk_u48wqendnkx9er21pqqcadlytbutx", tokens[1].Value) require.Equal(t, "Another token", tokens[1].Label) + // Try changing provisioned user's password + require.Error(t, a.ChangePassword("philuser", "new-pass", false)) + // Re-open the DB again (third app start) require.Nil(t, a.db.Close()) conf.Users = []*User{} diff --git a/user/types.go b/user/types.go index 726d40e0..18019bf2 100644 --- a/user/types.go +++ b/user/types.go @@ -244,15 +244,16 @@ const ( // Error constants used by the package var ( - ErrUnauthenticated = errors.New("unauthenticated") - ErrUnauthorized = errors.New("unauthorized") - ErrInvalidArgument = errors.New("invalid argument") - ErrUserNotFound = errors.New("user not found") - ErrUserExists = errors.New("user already exists") - ErrPasswordHashInvalid = errors.New("password hash but be a bcrypt hash, use 'ntfy user hash' to generate") - ErrTierNotFound = errors.New("tier not found") - ErrTokenNotFound = errors.New("token not found") - ErrPhoneNumberNotFound = errors.New("phone number not found") - ErrTooManyReservations = errors.New("new tier has lower reservation limit") - ErrPhoneNumberExists = errors.New("phone number already exists") + ErrUnauthenticated = errors.New("unauthenticated") + ErrUnauthorized = errors.New("unauthorized") + ErrInvalidArgument = errors.New("invalid argument") + ErrUserNotFound = errors.New("user not found") + ErrUserExists = errors.New("user already exists") + ErrPasswordHashInvalid = errors.New("password hash but be a bcrypt hash, use 'ntfy user hash' to generate") + ErrTierNotFound = errors.New("tier not found") + ErrTokenNotFound = errors.New("token not found") + ErrPhoneNumberNotFound = errors.New("phone number not found") + ErrTooManyReservations = errors.New("new tier has lower reservation limit") + ErrPhoneNumberExists = errors.New("phone number already exists") + ErrProvisionedUserPasswordChange = errors.New("cannot change password of provisioned user") ) diff --git a/web/src/app/errors.js b/web/src/app/errors.js index 28f49af1..0f39d705 100644 --- a/web/src/app/errors.js +++ b/web/src/app/errors.js @@ -31,6 +31,14 @@ export class TopicReservedError extends Error { } } +export class ProvisionedUserPasswordError extends Error { + static CODE = 40905; // errHTTPConflictTopicReserved + + constructor() { + super("Cannot change the password of a provisioned user"); + } +} + export class AccountCreateLimitReachedError extends Error { static CODE = 42906; // errHTTPTooManyRequestsLimitAccountCreation