update remote sync (#1367)

projectroles/remote_projects.py

@@ -52,6 +52,7 @@
 REMOTE_LEVEL_VIEW_AVAIL = SODAR_CONSTANTS['REMOTE_LEVEL_VIEW_AVAIL']
 REMOTE_LEVEL_READ_INFO = SODAR_CONSTANTS['REMOTE_LEVEL_READ_INFO']
 REMOTE_LEVEL_READ_ROLES = SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES']
+SYSTEM_USER_GROUP = SODAR_CONSTANTS['SYSTEM_USER_GROUP']
 
 # Local constants
 APP_NAME = 'projectroles'
@@ -80,6 +81,9 @@ def __init__(self):
         #: Updated parent projects in current sync operation
         self.updated_parents = []
 
+        #: User name/UUID lookup
+        self.user_lookup = {}
+
     # Internal Source Site Functions -------------------------------------------
 
     @classmethod
@@ -370,6 +374,18 @@ def get_remote_data(self, site):
 
     # Internal Target Site Functions -------------------------------------------
 
+    @staticmethod
+    def _is_local_user(user_data):
+        """
+        Return True if user data denotes a local user.
+
+        :param user_data: Dict
+        :return: Boolean
+        """
+        return (
+            SYSTEM_USER_GROUP in user_data['groups'] or not user_data['groups']
+        )
+
     @staticmethod
     def _update_obj(obj, obj_data, fields):
         """
@@ -422,7 +438,7 @@ def _sync_user(self, uuid, user_data):
         """
         # Don't sync local users if disallowed
         allow_local = getattr(settings, 'PROJECTROLES_ALLOW_LOCAL_USERS', False)
-        if '@' not in user_data['username'] and not allow_local:
+        if self._is_local_user(user_data) and not allow_local:
             logger.info(NO_LOCAL_USERS_MSG)
             return
         # Add UUID to user_data to ensure it gets synced
@@ -477,7 +493,7 @@ def _sync_user(self, uuid, user_data):
 
         # Create new user
         except User.DoesNotExist:
-            if '@' not in user_data['username']:
+            if self._is_local_user(user_data):
                 logger.warning(
                     'Local user "{}" not found: local users must '
                     'be manually created before they can be synced'.format(
@@ -724,8 +740,11 @@ def _update_roles(self, project, project_data):
                 continue
 
             # Ensure the user is valid
+            local_user = self._is_local_user(
+                self.remote_data['users'][self.user_lookup[r['user']]]
+            )
             if (
-                '@' not in r['user']
+                local_user
                 and not allow_local
                 and r['role'] != PROJECT_ROLE_OWNER
             ):
@@ -735,10 +754,9 @@ def _update_roles(self, project, project_data):
                 )
                 self._handle_user_error(error_msg, project, r_uuid)
                 continue
-
             # If local user, ensure they exist
             elif (
-                '@' not in r['user']
+                local_user
                 and allow_local
                 and r['role'] != PROJECT_ROLE_OWNER
                 and not User.objects.filter(username=r['user']).first()
@@ -749,16 +767,15 @@ def _update_roles(self, project, project_data):
                 )
                 self._handle_user_error(error_msg, project, r_uuid)
                 continue
-
             # Use the default owner, if owner role is for local user and local
             # users are not allowed
             if (
-                r['role'] == PROJECT_ROLE_OWNER
+                local_user
+                and r['role'] == PROJECT_ROLE_OWNER
                 and (
                     not allow_local
                     or not User.objects.filter(username=r['user']).first()
                 )
-                and '@' not in r['user']
             ):
                 role_user = self.default_owner
                 # Notify of assigning role to default owner
@@ -1118,10 +1135,13 @@ def _sync_app_setting(self, uuid, set_data):
                 )
                 return
         if user_name:
+            local_user = self._is_local_user(
+                self.remote_data['users'][user_uuid]
+            )
             allow_local = getattr(
                 settings, 'PROJECTROLES_ALLOW_LOCAL_USERS', False
             )
-            if '@' not in user_name and not allow_local:
+            if local_user and not allow_local:
                 logger.info(skip_msg + NO_LOCAL_USERS_MSG)
                 return
             user = User.objects.filter(sodar_uuid=user_uuid).first()
@@ -1270,6 +1290,8 @@ def sync_remote_data(self, site, remote_data, request=None):
         # NOTE: Add all users here, only update local users within _sync_user()
         for u_uuid, u_data in self.remote_data['users'].items():
             self._sync_user(u_uuid, u_data)
+            # HACK: Populate user lookup
+            self.user_lookup[u_data['username']] = u_uuid
         logger.info('User sync OK')
 
         # Categories and Projects

projectroles/tests/test_remote_project_api.py

@@ -53,6 +53,7 @@
 REMOTE_LEVEL_READ_INFO = SODAR_CONSTANTS['REMOTE_LEVEL_READ_INFO']
 REMOTE_LEVEL_READ_ROLES = SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES']
 REMOTE_LEVEL_REVOKED = SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED']
+SYSTEM_USER_GROUP = SODAR_CONSTANTS['SYSTEM_USER_GROUP']
 
 # Local constants
 SOURCE_SITE_NAME = 'Test source site'
@@ -1240,6 +1241,7 @@ def test_create_local_owner(self):
 
         remote_data = self.default_data
         remote_data['users'][SOURCE_USER_UUID]['username'] = 'source_admin'
+        remote_data['users'][SOURCE_USER_UUID]['groups'] = [SYSTEM_USER_GROUP]
         remote_data['projects'][SOURCE_CATEGORY_UUID]['roles'][
             SOURCE_CATEGORY_ROLE_UUID
         ]['user'] = 'source_admin'
@@ -1345,7 +1347,7 @@ def test_create_local_user(self):
             'last_name': SOURCE_USER_LAST_NAME,
             'email': SOURCE_USER_EMAIL,
             'additional_emails': [],
-            'groups': ['system'],
+            'groups': [SYSTEM_USER_GROUP],
         }
         remote_data['projects'][SOURCE_PROJECT_UUID]['roles'][role_uuid] = {
             'user': local_user_username,
@@ -1377,7 +1379,7 @@ def test_create_local_user_allow(self):
             'last_name': SOURCE_USER_LAST_NAME,
             'email': SOURCE_USER_EMAIL,
             'additional_emails': [],
-            'groups': ['system'],
+            'groups': [SYSTEM_USER_GROUP],
         }
         remote_data['projects'][SOURCE_PROJECT_UUID]['roles'][role_uuid] = {
             'user': local_user_username,
@@ -1407,7 +1409,7 @@ def test_create_local_user_allow_unavailable(self):
             'last_name': SOURCE_USER_LAST_NAME,
             'email': SOURCE_USER_EMAIL,
             'additional_emails': [],
-            'groups': ['system'],
+            'groups': [SYSTEM_USER_GROUP],
         }
         remote_data['projects'][SOURCE_PROJECT_UUID]['roles'][role_uuid] = {
             'user': local_user_username,
@@ -1438,7 +1440,7 @@ def test_create_local_owner_allow(self):
             'last_name': SOURCE_USER_LAST_NAME,
             'email': SOURCE_USER_EMAIL,
             'additional_emails': [],
-            'groups': ['system'],
+            'groups': [SYSTEM_USER_GROUP],
         }
         remote_data['projects'][SOURCE_PROJECT_UUID]['roles'] = {
             role_uuid: {
@@ -1473,7 +1475,7 @@ def test_create_local_owner_allow_unavailable(self):
             'last_name': SOURCE_USER_LAST_NAME,
             'email': SOURCE_USER_EMAIL,
             'additional_emails': [],
-            'groups': ['system'],
+            'groups': [SYSTEM_USER_GROUP],
         }
         remote_data['projects'][SOURCE_PROJECT_UUID]['roles'] = {
             role_uuid: {
@@ -1982,7 +1984,7 @@ def test_update_user_uuid_local_disallow(self):
             'last_name': local_name.split(' ')[1],
             'email': '[email protected]',
             'additional_emails': [],
-            'groups': [SOURCE_USER_GROUP],
+            'groups': [SYSTEM_USER_GROUP],
         }
         self.assertEqual(User.objects.all().count(), 3)
         self.remote_api.sync_remote_data(self.source_site, remote_data)