Merge pull request ome#5780 from will-moore/fix_admin_privilege_loss

Fix loss of privileges when Full Admin edits themselves

components/tools/OmeroWeb/omeroweb/webadmin/views.py

@@ -588,18 +588,19 @@ def manage_experimenter(request, action, eid=None, conn=None, **kwargs):
 
                 # Update 'AdminPrivilege' config roles for user
                 privileges = conn.get_privileges_from_form(form)
-                if privileges is None:
-                    privileges = []
-                # Only process privileges that we have permission to set
                 to_add = []
                 to_remove = []
-                for p in conn.getCurrentAdminPrivileges():
-                    if p in privileges:
-                        to_add.append(p)
-                    else:
-                        to_remove.append(p)
-
-                conn.updateAdminPrivileges(experimenter.id, to_add, to_remove)
+                # privileges may be None if disabled in form
+                if privileges is not None:
+                    # Only update privileges that we have permission to set
+                    # (prevents privilege escalation)
+                    for p in conn.getCurrentAdminPrivileges():
+                        if p in privileges:
+                            to_add.append(p)
+                        else:
+                            to_remove.append(p)
+                    conn.updateAdminPrivileges(experimenter.id,
+                                               to_add, to_remove)
 
                 conn.updateExperimenter(
                     experimenter, omename, firstName, lastName, email, admin,

components/tools/OmeroWeb/omeroweb/webclient/webclient_gateway.py

@@ -1097,7 +1097,7 @@ def createExperimenter(self, omeName, firstName, lastName, email, isAdmin,
         @type lastName String
         @param email A new email.
         @type email String
-        @param isAdmin An Admin permission.
+        @param isAdmin If True, new user is an Admin or Restricted Admin.
         @type isAdmin Boolean
         @param isActive Active user (user can log in).
         @type isActive Boolean
@@ -1107,6 +1107,8 @@ def createExperimenter(self, omeName, firstName, lastName, email, isAdmin,
         @type otherGroupIds L{ExperimenterGroupI}
         @param password Must pass validation in the security sub-system.
         @type password String
+        @param privileges   List of Admin Privileges. Ignored if isAdmin False.
+        @type privileges    List of Strings
         @param middleName A middle name.
         @type middleName String
         @param institution An institution.
@@ -1143,7 +1145,10 @@ def createExperimenter(self, omeName, firstName, lastName, email, isAdmin,
 
         defaultGroup = ExperimenterGroupI(defaultGroupId, False)
 
-        if privileges is not None:
+        if isAdmin:
+            if privileges is None:
+                privileges = []
+
             if defaultGroupId not in otherGroupIds:
                 listOfGroups.append(ExperimenterGroupI(defaultGroupId, False))
 
@@ -1273,17 +1278,22 @@ def get_privileges_from_form(self, experimenter_form):
         """
         Get 'AdminPrivilege' roles from Experimenter Form
 
-        Returns None if Role is User
+        Returns None if Role section of form is disabled.
+        Returns empty list if role is regular 'user', not admin.
+        If role is 'administrator' returns ALL privileges.
+
+        @param experimenter_form    Submitted instance of ExperimenterForm
         """
         privileges = []
         role = experimenter_form.cleaned_data['role']
-        if role not in ('restricted_administrator', 'administrator'):
+        # If Role element is disabled, we don't update privileges
+        if role == '':
             return None
         # If user is Admin, we give them ALL privileges!
         if role == 'administrator':
             for p in self.getEnumerationEntries('AdminPrivilege'):
                 privileges.append(p.getValue())
-        else:
+        elif role == 'restricted_administrator':
             # Otherwise, restrict to 'checked' privileges on form
             form_privileges = ['Chgrp',
                                'Chown',