From bbcda1a033eeb5aed95aa640fe84d88c0ff8a6a8 Mon Sep 17 00:00:00 2001
From: h44z <christoph.h@sprinternet.at>
Date: Tue, 24 Feb 2026 22:32:37 +0100
Subject: [PATCH] Merge commit from fork

* fix: improve user permission checks

* fix: improve user permission checks
---
 internal/app/api/v0/backend/user_service.go | 11 +++++
 internal/app/users/user_manager.go          | 49 ++++++++++++++++++++-
 internal/domain/user.go                     | 27 ++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/internal/app/api/v0/backend/user_service.go b/internal/app/api/v0/backend/user_service.go
index 6f2307f..e8f936a 100644
--- a/internal/app/api/v0/backend/user_service.go
+++ b/internal/app/api/v0/backend/user_service.go
@@ -54,6 +54,17 @@ func (u UserService) GetAllUsers(ctx context.Context) ([]domain.User, error) {
 }
 
 func (u UserService) UpdateUser(ctx context.Context, user *domain.User) (*domain.User, error) {
+	sessionUser := domain.GetUserInfo(ctx)
+	currentUser, err := u.users.GetUser(ctx, user.Identifier)
+	if err != nil {
+		return nil, err
+	}
+
+	// if this endpoint is used by non-admins, make sure that the user can only modify a specific subset of attributes
+	if !sessionUser.IsAdmin {
+		user.CopyAdminAttributes(currentUser, u.cfg.Advanced.ApiAdminOnly)
+	}
+
 	return u.users.UpdateUser(ctx, user)
 }
 
diff --git a/internal/app/users/user_manager.go b/internal/app/users/user_manager.go
index 33aef0f..a3aae07 100644
--- a/internal/app/users/user_manager.go
+++ b/internal/app/users/user_manager.go
@@ -272,8 +272,9 @@ func (m Manager) DeactivateApi(ctx context.Context, id domain.UserIdentifier) (*
 func (m Manager) validateModifications(ctx context.Context, old, new *domain.User) error {
 	currentUser := domain.GetUserInfo(ctx)
 
-	if currentUser.Id != new.Identifier && !currentUser.IsAdmin {
-		return fmt.Errorf("insufficient permissions")
+	adminErrors := m.validateAdminModifications(ctx, old, new)
+	if adminErrors != nil {
+		return adminErrors
 	}
 
 	if err := old.EditAllowed(new); err != nil && currentUser.Id != domain.SystemAdminContextUserInfo().Id {
@@ -303,6 +304,46 @@ func (m Manager) validateModifications(ctx context.Context, old, new *domain.Use
 	return nil
 }
 
+func (m Manager) validateAdminModifications(ctx context.Context, old, new *domain.User) error {
+	currentUser := domain.GetUserInfo(ctx)
+
+	if currentUser.IsAdmin {
+		if currentUser.Id == old.Identifier && !new.IsAdmin {
+			return fmt.Errorf("cannot remove own admin rights: %w", domain.ErrInvalidData)
+		}
+
+		return nil // admins can do (almost) everything
+	}
+
+	// non-admins can only modify very their own profile data
+
+	if currentUser.Id != new.Identifier {
+		return fmt.Errorf("insufficient permissions: %w", domain.ErrInvalidData)
+	}
+
+	if new.IsAdmin {
+		return fmt.Errorf("cannot grant admin rights: %w", domain.ErrInvalidData)
+	}
+
+	if new.Notes != old.Notes {
+		return fmt.Errorf("cannot update notes: %w", domain.ErrInvalidData)
+	}
+
+	if old.Locked != new.Locked || old.LockedReason != new.LockedReason {
+		return fmt.Errorf("cannot change lock state: %w", domain.ErrInvalidData)
+	}
+
+	if old.Disabled != new.Disabled || old.DisabledReason != new.DisabledReason {
+		return fmt.Errorf("cannot change disabled state: %w", domain.ErrInvalidData)
+	}
+
+	if old.PersistLocalChanges != new.PersistLocalChanges {
+		return fmt.Errorf("cannot change disabled state: %w", domain.ErrInvalidData)
+	}
+
+	return nil
+}
+
 func (m Manager) validateCreation(ctx context.Context, new *domain.User) error {
 	currentUser := domain.GetUserInfo(ctx)
 
@@ -374,6 +415,10 @@ func (m Manager) validateDeletion(ctx context.Context, del *domain.User) error {
 func (m Manager) validateApiChange(ctx context.Context, user *domain.User) error {
 	currentUser := domain.GetUserInfo(ctx)
 
+	if !currentUser.IsAdmin && m.cfg.Advanced.ApiAdminOnly {
+		return fmt.Errorf("insufficient permissions to change API access: %w", domain.ErrNoPermission)
+	}
+
 	if currentUser.Id != user.Identifier {
 		return fmt.Errorf("cannot change API access of user: %w", domain.ErrNoPermission)
 	}
diff --git a/internal/domain/user.go b/internal/domain/user.go
index fc84e2b..09b6f4a 100644
--- a/internal/domain/user.go
+++ b/internal/domain/user.go
@@ -212,6 +212,33 @@ func (u *User) CopyCalculatedAttributes(src *User, withAuthentications bool) {
 	}
 }
 
+// CopyAdminAttributes copies all attributes from the given user except password, passkey and
+// api-token if apiAdminOnly is false.
+func (u *User) CopyAdminAttributes(src *User, apiAdminOnly bool) {
+	u.BaseModel = src.BaseModel
+	u.Identifier = src.Identifier
+	u.Email = src.Email
+	u.Source = src.Source
+	u.ProviderName = src.ProviderName
+	u.IsAdmin = src.IsAdmin
+	u.Authentications = src.Authentications
+	u.PersistLocalChanges = src.PersistLocalChanges
+	u.Firstname = src.Firstname
+	u.Lastname = src.Lastname
+	u.Phone = src.Phone
+	u.Department = src.Department
+	u.Notes = src.Notes
+	u.Disabled = src.Disabled
+	u.DisabledReason = src.DisabledReason
+	u.Locked = src.Locked
+	u.LockedReason = src.LockedReason
+	u.LinkedPeerCount = src.LinkedPeerCount
+	if apiAdminOnly {
+		u.ApiToken = src.ApiToken
+		u.ApiTokenCreated = src.ApiTokenCreated
+	}
+}
+
 // MergeAuthSources merges the given authentication sources with the existing ones.
 // Already existing sources are not overwritten, nor will be added any duplicates.
 func (u *User) MergeAuthSources(extSources ...UserAuthentication) {