Implement main_id conflict checks in create_user

mig/shared/useradm.py

@@ -147,7 +147,7 @@
         return True
     try:
         os.remove(link_path)
     except:
         raise Exception('could not remove symlink: %s' % link_path)
     return True
 
@@ -369,7 +369,7 @@
         regex_patterns = ['distinguished_name']
     else:
         regex_patterns = []
     (_, hits) = search_users(search_filter, configuration, db_path, force, verbose,
                              regex_match=regex_patterns)
     peer_notes = []
     if not hits:
@@ -524,28 +524,43 @@
     # NOTE: careful to skip GDP project users here
     if configuration.user_openid_providers and \
             configuration.user_openid_alias:
+        user_main_ids = dict([(key, val.get('main_id', None))
+                              for (key, val) in user_db.items()])
         if not configuration.site_enable_gdp:
             user_aliases = dict([(key, val[configuration.user_openid_alias])
                                  for (key, val) in user_db.items()])
             alias = user[configuration.user_openid_alias]
+            main_id = user.get('main_id', 'UNSET')
         elif not is_gdp_user(configuration, client_id):
             user_aliases = dict([(key, val[configuration.user_openid_alias])
                                  for (key, val) in user_db.items() if not
                                  is_gdp_user(configuration, key)])
             alias = user[configuration.user_openid_alias]
+            main_id = user.get('main_id', 'UNSET')
         else:
             user_aliases = {}
             alias = None
+            main_id = 'UNSET'
 
+        alias_conflict, main_id_conflict = False, False
         if alias in user_aliases.values() and \
                 user_aliases.get(client_id, None) != alias:
+            alias_conflict = True
+        if main_id in user_main_ids.values() and \
+                user_main_ids.get(client_id, None) != main_id:
+            main_id_conflict = True
+
+        if alias_conflict or main_id_conflict:
             if do_lock:
                 unlock_user_db(flock)
+            _logger.warning('refuse create %r with alias %s / id %s conflict'
+                            % (client_id, alias, main_id))
             if verbose:
-                print('Attempting create user with conflicting alias %s'
-                      % alias)
+                print('Attempting create user with alias %s / id %s conflict'
+                      % (alias, main_id))
             raise Exception(
-                'A conflicting user with alias %s already exists' % alias)
+                'A conflicting user with alias %s / id %s already exists' %
+                (alias, main_id))
 
     if client_id not in user_db:
         _logger.debug('add new user %r in user DB' % client_id)
@@ -569,6 +584,8 @@
         else:
             if do_lock:
                 unlock_user_db(flock)
+            _logger.warning('refusing to renew %s account for %r' %
+                            (account_status, client_id))
             if verbose:
                 print('Refusing to renew %s account for %r' % (account_status,
                                                                client_id))
@@ -610,16 +627,16 @@
                                                      reset_auth_type,
                                                      req_timestamp)
                     if valid_reset:
                         _logger.info("%r requested and authorized password reset"
                                      % client_id)
                         if verbose:
                             print("User requested and authorized password reset")
                         authorized = True
                     else:
                         _logger.warning("%r requested password reset with bad token"
                                         % client_id)
                         if verbose:
                             print("User requested password reset with bad token")
 
                 if authorized:
                     _logger.info("%r authorized password update" % client_id)
@@ -650,7 +667,7 @@
                         if verbose:
                             print("""Renewal request supplied a different
 password and you didn't accept change anyway - nothing more to do""")
                         err = """Cannot renew account using a new password!
 Please tell user to use the original password, request password reset or go
 through renewal using Xgi-bin with proper authentication to authorize the
 change."""
@@ -667,7 +684,7 @@
                         not isinstance(val, basestring) and \
                         isinstance(val, list):
                     val_list = updated_user.get(key, [])
                     val_list += [i for i in val if not i in val_list]
                     updated_user[key] = val_list
                 elif val:
                     updated_user[key] = val
@@ -688,7 +705,7 @@
     short_id = user.get('short_id', '')
     # For cert users short_id is the full DN so we should ignore then
     if short_id and short_id != client_id and short_id.find(' ') == -1 and \
             not short_id in openid_names:
         openid_names.append(short_id)
     add_names = []
 
@@ -722,11 +739,13 @@
                 # NOTE: bail out here on edit/renewal as IDs are not unique
                 if do_lock:
                     unlock_user_db(flock)
-                raise ValueError("unique ID for %s (%r) has a collission!" %
+                _logger.warning("refuse renew %s (%r) with collision" %
+                                (client_id, user['unique_id']))
+                raise ValueError("unique ID for %s (%r) has a collision!" %
                                  (client_id, user['unique_id']))
             else:
-                _logger.warning("retry unique ID for %s (%r) - collission" %
-                                (client_id, user['unique_id']))
+                _logger.info("regenerate unique ID for %s (%r) - in use" %
+                             (client_id, user['unique_id']))
             user['unique_id'] = generate_random_ascii(unique_id_length,
                                                       unique_id_charset)
         if not found_unique:
@@ -767,7 +786,7 @@
     return user
 
 
 def create_user_in_fs(configuration, client_id, user, now, renew, force, verbose):
     """Handle all the parts of user creation or renewal relating to making
     directories and writing default files.
     """
@@ -807,7 +826,7 @@
     required_dir_links = _get_required_user_dir_links(configuration, real_dir,
                                                       link_dir)
     required_dirs = [path[0] for path in required_dir_links] + [ssh_dir,
                                                                 davs_dir, ftps_dir]
 
     # Make sure we set permissions tight enough for e.g. ssh auth keys to work
     os.umask(0o22)
@@ -1182,7 +1201,7 @@
         for (share_id, share_dict) in sharelinks.items():
             # Update owner and use generic update helper to replace symlink
             share_dict['owner'] = client_id
             (mod_status, err) = update_share_link(share_dict, client_id,
                                                   configuration, sharelinks)
             if verbose:
                 if mod_status:
@@ -1470,14 +1489,14 @@
         res_map = get_resource_map(configuration)
         for (res_id, res) in res_map.items():
             if client_id in res[OWNERS]:
                 (add_status, err) = resource_add_owners(configuration, res_id,
                                                         [new_id])
                 if not add_status:
                     if verbose:
                         print('Could not add new %s owner of %s: %s'
                               % (new_id, res_id, err))
                     continue
                 (del_status, err) = resource_remove_owners(configuration, res_id,
                                                            [client_id])
                 if not del_status:
                     if verbose:
@@ -1498,14 +1517,14 @@
     for (vgrid_name, vgrid) in vgrid_map[VGRIDS].items():
         if client_id in vgrid[OWNERS]:
 
             (add_status, err) = vgrid_add_owners(configuration, vgrid_name,
                                                  [new_id])
             if not add_status:
                 if verbose:
                     print('Could not add new %s owner of %s: %s'
                           % (new_id, vgrid_name, err))
                 continue
             (del_status, err) = vgrid_remove_owners(configuration, vgrid_name,
                                                     [client_id])
             if not del_status:
                 if verbose:
@@ -1517,14 +1536,14 @@
                                                           client_id,
                                                           new_id))
         elif client_id in vgrid[MEMBERS]:
             (add_status, err) = vgrid_add_members(configuration, vgrid_name,
                                                   [new_id])
             if not add_status:
                 if verbose:
                     print('Could not add new %s member of %s: %s'
                           % (new_id, vgrid_name, err))
                 continue
             (del_status, err) = vgrid_remove_members(configuration, vgrid_name,
                                                      [client_id])
             if not del_status:
                 if verbose:
@@ -1541,7 +1560,7 @@
     (re_status, re_list) = list_runtime_environments(configuration)
     if re_status:
         for re_name in re_list:
             (re_status, err) = update_runtimeenv_owner(re_name, client_id,
                                                        new_id, configuration)
             if verbose:
                 if re_status:
@@ -1563,7 +1582,7 @@
         for (share_id, share_dict) in sharelinks.items():
             # Update owner and use generic update helper to replace symlink
             share_dict['owner'] = new_id
             (mod_status, err) = update_share_link(share_dict, new_id,
                                                   configuration, sharelinks)
             if verbose:
                 if mod_status:

tests/test_mig_shared_useradm.py

@@ -401,7 +401,11 @@ def test_user_creation_fails_in_renew_when_locked(self):
         except Exception:
             self.assertFalse(True, "should not be reached")
 
-    def test_user_creation_with_id_collission_fails(self):
+    def test_user_creation_with_main_id_conflict_fails(self):
+        # NOTE: we need fake openid setup here for conflict detection code path
+        self.configuration.user_openid_providers = ["dummyoidprovider.org"]
+        self.configuration.user_openid_alias = "email"
+
         user_dict = {}
         user_dict["full_name"] = "Test User"
         user_dict["organization"] = "Test Org"
@@ -411,6 +415,7 @@ def test_user_creation_with_id_collission_fails(self):
         user_dict["comment"] = "This is the create comment"
         user_dict["password"] = "password"
         user_dict["distinguished_name"] = TEST_USER_DN
+        user_dict["main_id"] = "testmainid1234"
 
         try:
             create_user(
@@ -419,17 +424,61 @@ def test_user_creation_with_id_collission_fails(self):
         except Exception:
             self.assertFalse(True, "should not be reached")
 
-        # NOTE: reset distinguished_name and introduce an ID conflict to test
+        # NOTE: reset distinguished_name and most ID fields but keep main_id
+        #       to test that main ID conflict still prevents the creation.
         del user_dict["distinguished_name"]
-        user_dict["organization"] = "Another Org"
-        with self.assertRaises(Exception):
+        del user_dict["unique_id"]
+        user_dict["full_name"] = "Changed Name"
+        user_dict["organization"] = "Changed Org"
+        user_dict["email"] = OTHER_USER_EMAIL
+        with self.assertLogs(level='WARNING') as log_capture:
+            with self.assertRaises(Exception):
+                create_user(
+                    user_dict,
+                    self.configuration,
+                    keyword_auto,
+                    default_renew=True,
+                    ask_renew=False,
+                )
+        self.assertTrue(any('conflict' in msg for msg in
+                            log_capture.output))
+
+    def test_user_creation_with_openid_alias_conflict_fails(self):
+        # NOTE: we need fake openid setup here for conflict detection code path
+        self.configuration.user_openid_providers = ["dummyoidprovider.org"]
+        self.configuration.user_openid_alias = "email"
+
+        user_dict = {}
+        user_dict["full_name"] = "Test User"
+        user_dict["organization"] = "Test Org"
+        user_dict["state"] = "NA"
+        user_dict["country"] = "DK"
+        user_dict["email"] = "user@example.com"
+        user_dict["comment"] = "This is the create comment"
+        user_dict["password"] = "password"
+        user_dict["distinguished_name"] = TEST_USER_DN
+
+        try:
             create_user(
-                user_dict,
-                self.configuration,
-                keyword_auto,
-                default_renew=True,
-                ask_renew=False,
+                user_dict, self.configuration, keyword_auto, default_renew=True
             )
+        except Exception:
+            self.assertFalse(True, "should not be reached")
+
+        # NOTE: reset distinguished_name and introduce an ID conflict to test
+        del user_dict["distinguished_name"]
+        user_dict["organization"] = "Another Org"
+        with self.assertLogs(level='WARNING') as log_capture:
+            with self.assertRaises(Exception):
+                create_user(
+                    user_dict,
+                    self.configuration,
+                    keyword_auto,
+                    default_renew=True,
+                    ask_renew=False,
+                )
+        self.assertTrue(any('conflict' in msg for msg in
+                            log_capture.output))
 
 
 class MigSharedUseradm__assure_current_htaccess(MigTestCase):