Do not allow changing tokens, user role, or delete users

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}

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) {
+		if errors.Is(err, user.ErrProvisionedUserChange) {
-			return errHTTPConflictProvisionedUserPasswordChange
+			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).

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 := a.CanChangeUser(username); err != nil {
-	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)
 	})
 }
 
+// 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

user/types.go

@@ -244,16 +244,17 @@ const (
 
 // Error constants used by the package
 var (
-	ErrUnauthenticated               = errors.New("unauthenticated")
+	ErrUnauthenticated        = errors.New("unauthenticated")
-	ErrUnauthorized                  = errors.New("unauthorized")
+	ErrUnauthorized           = errors.New("unauthorized")
-	ErrInvalidArgument               = errors.New("invalid argument")
+	ErrInvalidArgument        = errors.New("invalid argument")
-	ErrUserNotFound                  = errors.New("user not found")
+	ErrUserNotFound           = errors.New("user not found")
-	ErrUserExists                    = errors.New("user already exists")
+	ErrUserExists             = errors.New("user already exists")
-	ErrPasswordHashInvalid           = errors.New("password hash but be a bcrypt hash, use 'ntfy user hash' to generate")
+	ErrPasswordHashInvalid    = errors.New("password hash but be a bcrypt hash, use 'ntfy user hash' to generate")
-	ErrTierNotFound                  = errors.New("tier not found")
+	ErrTierNotFound           = errors.New("tier not found")
-	ErrTokenNotFound                 = errors.New("token not found")
+	ErrTokenNotFound          = errors.New("token not found")
-	ErrPhoneNumberNotFound           = errors.New("phone number not found")
+	ErrPhoneNumberNotFound    = errors.New("phone number not found")
-	ErrTooManyReservations           = errors.New("new tier has lower reservation limit")
+	ErrTooManyReservations    = errors.New("new tier has lower reservation limit")
-	ErrPhoneNumberExists             = errors.New("phone number already exists")
+	ErrPhoneNumberExists      = errors.New("phone number already exists")
-	ErrProvisionedUserPasswordChange = errors.New("cannot change password of provisioned user")
+	ErrProvisionedUserChange  = errors.New("cannot change or delete provisioned user")
+	ErrProvisionedTokenChange = errors.New("cannot change or delete provisioned token")
 )