From c0c5fd5af87e09f59e9bfe6e3252caef586c87aa Mon Sep 17 00:00:00 2001
From: Marcin Kuzminski <marcin@rhodecode.com>
Date: Fri, 29 Sep 2017 21:40:06 +0200
Subject: [PATCH] user-groups: fix potential problem with group sync of
 external plugins.

- when using external plugin we used to check for a parameter that set the
sync mode. The problem is we only checked if the flag was there. So
toggling sync on and off set the value and then left the key still set
but with None. This confused the sync and thought the group should be
synced !
---
 rhodecode/model/user_group.py | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/rhodecode/model/user_group.py b/rhodecode/model/user_group.py
index 0a883147..20e0b02d 100644
--- a/rhodecode/model/user_group.py
+++ b/rhodecode/model/user_group.py
@@ -490,18 +490,22 @@ class UserGroupModel(BaseModel):
 
     def enforce_groups(self, user, groups, extern_type=None):
         user = self._get_user(user)
-        log.debug('Enforcing groups %s on user %s', groups, user)
         current_groups = user.group_member
-        # find the external created groups
-        externals = [x.users_group for x in current_groups
-                     if 'extern_type' in x.users_group.group_data]
 
+        # find the external created groups, i.e automatically created
+        log.debug('Enforcing user group set `%s` on user %s', groups, user)
         # calculate from what groups user should be removed
-        # externals that are not in groups
-        for gr in externals:
-            if gr.users_group_name not in groups:
-                log.debug('Removing user %s from user group %s', user, gr)
-                self.remove_user_from_group(gr, user)
+        # external_groups that are not in groups
+        for gr in [x.users_group for x in current_groups]:
+            managed = gr.group_data.get('extern_type')
+            if managed:
+                if gr.users_group_name not in groups:
+                    log.debug('Removing user %s from user group %s. '
+                              'Group sync managed by: %s', user, gr, managed)
+                    self.remove_user_from_group(gr, user)
+            else:
+                log.debug('Skipping removal from group %s since it is '
+                          'not set to be automatically synchronized' % gr)
 
         # now we calculate in which groups user should be == groups params
         owner = User.get_first_super_admin().username
@@ -513,8 +517,10 @@ class UserGroupModel(BaseModel):
                 existing_group = UserGroupModel().create(
                     gr, desc, owner, group_data={'extern_type': extern_type})
 
-            # we can only add users to special groups created via plugins
-            managed = 'extern_type' in existing_group.group_data
+            # we can only add users to groups which have set sync flag via
+            # extern_type attribute.
+            # This is either set and created via plugins, or manually
+            managed = existing_group.group_data.get('extern_type')
             if managed:
                 log.debug('Adding user %s to user group %s', user, gr)
                 UserGroupModel().add_user_to_group(existing_group, user)