Merge pull request #5380 from freedomofpress/more_checks_invalid_user…

…name

Fixes #5378, adds more checks for invalid username

securedrop/models.py

@@ -484,6 +484,10 @@ def check_username_acceptable(cls, username: str) -> None:
             raise InvalidUsernameException(
                         'Username "{}" must be at least {} characters long.'
                         .format(username, cls.MIN_USERNAME_LEN))
+        if username in cls.INVALID_USERNAMES:
+            raise InvalidUsernameException(
+                    "This username is invalid because it is reserved "
+                    "for internal use by the software.")
 
     @classmethod
     def check_name_acceptable(cls, name):

securedrop/tests/functional/journalist_navigation_steps.py

@@ -529,6 +529,49 @@ def edit_user_page_loaded():
         hotp_reset_uid = hotp_reset_button.find_element_by_name("uid")
         assert hotp_reset_uid.is_displayed() is False
 
+    def _admin_editing_invalid_username(self):
+        # Log the new user out
+        self._logout()
+
+        self.wait_for(lambda: self.driver.find_element_by_css_selector(".login-form"))
+
+        self._login_user(self.admin, self.admin_pw, self.admin_user["totp"])
+
+        # Go to the admin interface
+        self.safe_click_by_id("link-admin-index")
+
+        self.wait_for(lambda: self.driver.find_element_by_css_selector("button#add-user"))
+
+        # Click the "edit user" link for the new user
+        # self._edit_user(self.new_user['username'])
+        new_user_edit_links = [
+            el
+            for el in self.driver.find_elements_by_tag_name("a")
+            if (el.get_attribute("data-username") == self.new_user["username"])
+        ]
+        assert len(new_user_edit_links) == 1
+        new_user_edit_links[0].click()
+
+        def can_edit_user():
+            h = self.driver.find_elements_by_tag_name("h1")[0]
+            assert 'Edit user "{}"'.format(self.new_user["username"]) == h.text
+
+        self.wait_for(can_edit_user)
+
+        new_username = "deleted"
+
+        self.safe_send_keys_by_css_selector('input[name="username"]', Keys.CONTROL + "a")
+        self.safe_send_keys_by_css_selector('input[name="username"]', Keys.DELETE)
+        self.safe_send_keys_by_css_selector('input[name="username"]', new_username)
+        self.safe_click_by_css_selector("button[type=submit]")
+
+        def user_edited():
+            if not hasattr(self, "accept_languages"):
+                flash_msg = self.driver.find_element_by_css_selector(".flash")
+                assert "Invalid username" in flash_msg.text
+
+        self.wait_for(user_edited)
+
     def _admin_can_edit_new_user(self):
         # Log the new user out
         self._logout()

securedrop/tests/functional/test_admin_interface.py

@@ -15,6 +15,13 @@ def test_admin_interface(self):
         self._new_user_can_log_in()
         self._admin_can_edit_new_user()
 
+    def test_admin_edit_invalid_username(self):
+        self._admin_logs_in()
+        self._admin_visits_admin_interface()
+        self._admin_adds_a_user()
+        self._new_user_can_log_in()
+        self._admin_editing_invalid_username()
+
     def test_admin_edits_hotp_secret(self):
         # Toggle security slider to force prefs change
         self.set_tbb_securitylevel(ft.TBB_SECURITY_HIGH)

securedrop/tests/test_journalist.py

@@ -820,6 +820,29 @@ def test_admin_edits_user_invalid_username(
                 'error')
 
 
+def test_admin_edits_user_invalid_username_deleted(
+        journalist_app, test_admin, test_journo):
+    """Test expected error message when admin attempts to change a user's
+    username to deleted"""
+    new_username = "deleted"
+    with journalist_app.test_client() as app:
+        _login_user(app, test_admin['username'], test_admin['password'],
+                    test_admin['otp_secret'])
+
+        with InstrumentedApp(journalist_app) as ins:
+            app.post(
+                url_for('admin.edit_user', user_id=test_admin['id']),
+                data=dict(username=new_username,
+                          first_name='',
+                          last_name='',
+                          is_admin=None))
+
+            ins.assert_message_flashed(
+                    'Invalid username: This username is invalid because it '
+                    'is reserved for internal use by the software.',
+                    'error')
+
+
 def test_admin_resets_user_hotp_format_non_hexa(
         journalist_app, test_admin, test_journo):