refactor the auth for user role editing

flexmeasures/auth/policy.py

@@ -210,7 +210,7 @@ def can_modify_role(
     :param user: The current attempting to modify a role.
     :param roles_to_modify: A list of roles to modify - can be a Role or a role ID.
     :param modified_user: The user whose roles are being modified.
-    :return: True if the user can modify the roles, False otherwise.
+    :return: True if the user can modify each of the roles, False otherwise.
 
     The roles are:
     - admin: can only be changed in CLI / directly in the DB, so not here
@@ -219,23 +219,36 @@ def can_modify_role(
     - consultant: can be added and removed by admins and account-admins (in same account)
 
     """
-
+    roles = []
     for role in roles_to_modify:
         if isinstance(role, int):
             from flexmeasures.data.models.user import Role
 
-            role = current_app.db.session.get(Role, role)
-
-    if role is not None:
-        if role.name != ADMIN_ROLE and user.has_role(ADMIN_ROLE):
-            return True  # admin can do all changes, aside from admin status
-        if role.name == ACCOUNT_ADMIN_ROLE and user.has_role(CONSULTANT_ROLE):
-            if modified_user.account.consultancy_account is not None:
-                if user.account.id == modified_user.account.consultancy_account.id:
-                    return True
-        if role.name == CONSULTANT_ROLE and user.has_role(ACCOUNT_ADMIN_ROLE):
-            if user.account.id and modified_user.account.id:
-                if user.account.id == modified_user.account.id:
-                    return True
-
-    return False
+            roles.append(current_app.db.session.get(Role, role))
+        else:
+            roles.append(role)
+
+    if user.has_role(ADMIN_ROLE) and ADMIN_ROLE not in [role.name for role in roles]:
+        return True  # admins can do all changes, aside from admin status
+
+    for role in [r for r in roles if r is not None]:
+        if role.name == ADMIN_ROLE:
+            return False  # nobody can do this here, only in CLI or directly in the DB
+        if role.name == ADMIN_READER_ROLE:
+            if not user.has_role(ADMIN_ROLE):
+                return False  # only admins can change admin-reader status
+        if role.name == ACCOUNT_ADMIN_ROLE:  # consultants can do this
+            if (
+                modified_user.account.consultancy_account is None
+                or not user.has_role(CONSULTANT_ROLE)
+                or not user.account.id == modified_user.account.consultancy_account.id
+            ):
+                return False
+        if role.name == CONSULTANT_ROLE:  # account-admins can do this
+            if (
+                not user.has_role(ACCOUNT_ADMIN_ROLE)
+                or not user.account.id == modified_user.account.id
+            ):
+                return False
+
+    return True

flexmeasures/data/models/user.py

@@ -289,6 +289,7 @@ def __acl__(self):
         """
         Within the same account, everyone can read. Consultants as well.
         Only the user themselves, consultants or account-admins can edit their user record.
+        Check policy.can_modify_role() for special treatment of roles.
         Creation and deletion are left to site admins in CLI.
         """
         return {

flexmeasures/ui/views/users/views.py

@@ -6,7 +6,7 @@
 from werkzeug.exceptions import Forbidden, Unauthorized
 from sqlalchemy import select
 
-from flexmeasures.auth.policy import check_access
+from flexmeasures.auth.policy import check_access, ADMIN_ROLE
 from flexmeasures.data import db
 from flexmeasures.data.models.audit_log import AuditLog
 from flexmeasures.data.models.user import User, Role, Account
@@ -43,7 +43,8 @@ def render_user(user: User | None, msg: str | None = None):
 
     roles = {}
     for role in db.session.scalars(select(Role)).all():
-        roles[role.name] = role.id
+        if role.name != ADMIN_ROLE:
+            roles[role.name] = role.id
 
     user_roles = []
     if user is not None: