Implement handling of invalid request markers in create_user as well

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:
@@ -476,7 +476,7 @@
 
 
 def create_user_in_db(configuration, db_path, client_id, user, now, authorized,
-                      reset_token, reset_auth_type, pw_match,
+                      reset_token, reset_auth_type, pw_match, invalid,
                       accepted_peer_list, force, verbose, ask_renew,
                       default_renew, do_lock, from_edit_user, ask_change_pw,
                       auto_create_db, create_backup):
@@ -547,6 +547,17 @@
             raise Exception(
                 'A conflicting user with alias %s already exists' % alias)
 
+    # Reject all other obviously invalid requests
+    if invalid:
+        if do_lock:
+            unlock_user_db(flock)
+        _logger.warning("%r requested account with invalid values: %s"
+                        % (client_id, ', '.join(invalid)))
+        if verbose:
+            print("User requested account with invalid values")
+        err = "Cannot create/renew user account with invalid values!"
+        raise Exception(err)
+
     if client_id not in user_db:
         _logger.debug('add new user %r in user DB' % client_id)
         user['created'] = now
@@ -610,16 +621,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 +661,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 +678,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 +699,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 = []
 
@@ -767,7 +778,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 +818,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)
@@ -1095,6 +1106,11 @@
         pw_match = user['pw_match']
         # Always remove any pw_match fields before DB insert
         del user['pw_match']
+    invalid = False
+    if 'invalid' in user:
+        invalid = user['invalid']
+        # Always remove any invalid markers before DB insert
+        del user['invalid']
 
     _logger.info('trying to create or renew user %r' % client_id)
     if verbose:
@@ -1123,8 +1139,8 @@
 
     created = create_user_in_db(configuration, db_path, client_id, user, now,
                                 authorized, reset_token, reset_auth_type,
-                                pw_match, accepted_peer_list, force, verbose,
-                                ask_renew, default_renew, do_lock,
+                                pw_match, invalid, accepted_peer_list, force,
+                                verbose, ask_renew, default_renew, do_lock,
                                 from_edit_user, ask_change_pw, auto_create_db,
                                 create_backup)
     # Mark user updated for all logins
@@ -1182,7 +1198,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 +1486,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 +1514,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 +1533,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 +1557,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:

tests/test_mig_shared_useradm.py

@@ -431,6 +431,67 @@ def test_user_creation_with_id_collission_fails(self):
                 ask_renew=False,
             )
 
+    def test_user_creation_with_invalid_marker_fails(self):
+        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
+        # NOTE: inject invalid marker to prevent create
+        user_dict["invalid"] = ["invalid request detected"]
+
+        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('invalid values' in msg for msg in
+                            log_capture.output))
+
+    def test_user_creation_with_invalid_markers_fails_renewal(self):
+        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:
+            created = create_user(
+                user_dict, self.configuration, keyword_auto, default_renew=True
+            )
+        except Exception:
+            self.assertFalse(True, "should not be reached")
+
+        # Double check that valid user was created without 'invalid' field
+        self.assertTrue(created)
+        self.assertNotIn('invalid', created)
+
+        # NOTE: inject invalid marker to prevent renew
+        user_dict["invalid"] = ["invalid request detected"]
+        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('invalid values' in msg for msg in
+                            log_capture.output))
+
 
 class MigSharedUseradm__assure_current_htaccess(MigTestCase):
     """Coverage of useradm behaviours around htaccess."""