diff --git a/server/errors.go b/server/errors.go index 1ab12f7c..098f785d 100644 --- a/server/errors.go +++ b/server/errors.go @@ -132,7 +132,8 @@ 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} + 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} 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 4fe392b1..de5a41d5 100644 --- a/server/server_account.go +++ b/server/server_account.go @@ -174,6 +174,12 @@ func (s *Server) handleAccountDelete(w http.ResponseWriter, r *http.Request, v * if _, err := s.userManager.Authenticate(u.Name, req.Password); err != nil { return errHTTPBadRequestIncorrectPasswordConfirmation } + if err := s.userManager.CanChangeUser(u.Name); err != nil { + if errors.Is(err, user.ErrProvisionedUserChange) { + return errHTTPConflictProvisionedUserChange + } + return err + } if s.webPush != nil && u.ID != "" { if err := s.webPush.RemoveSubscriptionsByUserID(u.ID); err != nil { logvr(v, r).Err(err).Warn("Error removing web push subscriptions for %s", u.Name) @@ -208,8 +214,8 @@ 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 + if errors.Is(err, user.ErrProvisionedUserChange) { + return errHTTPConflictProvisionedUserChange } return err } @@ -277,6 +283,9 @@ func (s *Server) handleAccountTokenUpdate(w http.ResponseWriter, r *http.Request Debug("Updating token for user %s as deleted", u.Name) token, err := s.userManager.ChangeToken(u.ID, req.Token, req.Label, expires) if err != nil { + if errors.Is(err, user.ErrProvisionedTokenChange) { + return errHTTPConflictProvisionedTokenChange + } return err } response := &apiAccountTokenResponse{ @@ -299,6 +308,9 @@ func (s *Server) handleAccountTokenDelete(w http.ResponseWriter, r *http.Request } } if err := s.userManager.RemoveToken(u.ID, token); err != nil { + if errors.Is(err, user.ErrProvisionedTokenChange) { + return errHTTPConflictProvisionedTokenChange + } return err } logvr(v, r). diff --git a/user/manager.go b/user/manager.go index 76abff9d..8cea653c 100644 --- a/user/manager.go +++ b/user/manager.go @@ -773,6 +773,9 @@ func (a *Manager) ChangeToken(userID, token string, label *string, expires *time if token == "" { return nil, errNoTokenProvided } + if err := a.CanChangeToken(userID, token); err != nil { + return nil, err + } tx, err := a.db.Begin() if err != nil { return nil, err @@ -796,6 +799,9 @@ func (a *Manager) ChangeToken(userID, token string, label *string, expires *time // RemoveToken deletes the token defined in User.Token func (a *Manager) RemoveToken(userID, token string) error { + if err := a.CanChangeToken(userID, token); err != nil { + return err + } return execTx(a.db, func(tx *sql.Tx) error { return a.removeTokenTx(tx, userID, token) }) @@ -811,6 +817,17 @@ func (a *Manager) removeTokenTx(tx *sql.Tx, userID, token string) error { return nil } +// CanChangeToken checks if the token can be changed. If the token is provisioned, it cannot be changed. +func (a *Manager) CanChangeToken(userID, token string) error { + t, err := a.Token(userID, token) + if err != nil { + return err + } else if t.Provisioned { + return ErrProvisionedTokenChange + } + return nil +} + // RemoveExpiredTokens deletes all expired tokens from the database func (a *Manager) RemoveExpiredTokens() error { if _, err := a.db.Exec(deleteExpiredTokensQuery, time.Now().Unix()); err != nil { @@ -1072,6 +1089,9 @@ func (a *Manager) addUserTx(tx *sql.Tx, username, password string, role Role, ha // RemoveUser deletes the user with the given username. The function returns nil on success, even // if the user did not exist in the first place. func (a *Manager) RemoveUser(username string) error { + if err := a.CanChangeUser(username); err != nil { + return err + } return execTx(a.db, func(tx *sql.Tx) error { return a.removeUserTx(tx, username) }) @@ -1389,19 +1409,26 @@ 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 { + if err := a.CanChangeUser(username); 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) }) } +// CanChangeUser checks if the user with the given username can be changed. +// This is used to prevent changes to provisioned users, which are defined in the config file. +func (a *Manager) CanChangeUser(username string) error { + user, err := a.User(username) + if err != nil { + return err + } else if user.Provisioned { + return ErrProvisionedUserChange + } + return nil +} + func (a *Manager) changePasswordTx(tx *sql.Tx, username, password string, hashed bool) error { var hash string var err error @@ -1425,6 +1452,9 @@ func (a *Manager) changePasswordTx(tx *sql.Tx, username, password string, hashed // ChangeRole changes a user's role. When a role is changed from RoleUser to RoleAdmin, // all existing access control entries (Grant) are removed, since they are no longer needed. func (a *Manager) ChangeRole(username string, role Role) error { + if err := a.CanChangeUser(username); err != nil { + return err + } return execTx(a.db, func(tx *sql.Tx) error { return a.changeRoleTx(tx, username, role) }) @@ -1445,14 +1475,8 @@ func (a *Manager) changeRoleTx(tx *sql.Tx, username string, role Role) error { return nil } -// ChangeProvisioned changes the provisioned status of a user. This is used to mark users as +// changeProvisionedTx changes the provisioned status of a user. This is used to mark users as // provisioned. A provisioned user is a user defined in the config file. -func (a *Manager) ChangeProvisioned(username string, provisioned bool) error { - return execTx(a.db, func(tx *sql.Tx) error { - return a.changeProvisionedTx(tx, username, provisioned) - }) -} - func (a *Manager) changeProvisionedTx(tx *sql.Tx, username string, provisioned bool) error { if _, err := tx.Exec(updateUserProvisionedQuery, provisioned, username); err != nil { return err @@ -1678,7 +1702,7 @@ func (a *Manager) Tiers() ([]*Tier, error) { tiers := make([]*Tier, 0) for { tier, err := a.readTier(rows) - if err == ErrTierNotFound { + if errors.Is(err, ErrTierNotFound) { break } else if err != nil { return nil, err diff --git a/user/types.go b/user/types.go index 18019bf2..e501e732 100644 --- a/user/types.go +++ b/user/types.go @@ -244,16 +244,17 @@ 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") - ErrProvisionedUserPasswordChange = errors.New("cannot change password of provisioned user") + 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") + ErrProvisionedUserChange = errors.New("cannot change or delete provisioned user") + ErrProvisionedTokenChange = errors.New("cannot change or delete provisioned token") )