Merge pull request #5284 from prateekj117/handle-deleted-condition

Handle case of deleted journalists

docs/admin.rst

@@ -132,6 +132,10 @@ password manager. Then, you will select whether you would like them to also be
 an admin (this allows them to add or delete other journalist accounts), and
 whether they will be using FreeOTP or a YubiKey for two-factor authentication.
 
+.. note::
+  We don't allow the username **deleted** as we use it to mark the
+  journalists which are deleted from the system.
+
 FreeOTP
 ^^^^^^^
 

securedrop/journalist_app/forms.py

@@ -37,10 +37,16 @@ def name_length_validation(form, field):
             .format(max_chars=Journalist.MAX_NAME_LEN)))
 
 
+def check_invalid_usernames(form, field):
+    if field.data in Journalist.INVALID_USERNAMES:
+        raise ValidationError(gettext(
+            "This username is invalid because it is reserved for internal use by the software."))
+
+
 class NewUserForm(FlaskForm):
     username = StringField('username', validators=[
         InputRequired(message=gettext('This field is required.')),
-        minimum_length_validation
+        minimum_length_validation, check_invalid_usernames
     ])
     first_name = StringField('first_name', validators=[name_length_validation, Optional()])
     last_name = StringField('last_name', validators=[name_length_validation, Optional()])

securedrop/models.py

@@ -414,6 +414,7 @@ class Journalist(db.Model):
     MIN_USERNAME_LEN = 3
     MIN_NAME_LEN = 0
     MAX_NAME_LEN = 100
+    INVALID_USERNAMES = ['deleted']
 
     def __init__(self,
                  username: str,
@@ -641,12 +642,18 @@ def login(cls,
               username: str,
               password: str,
               token: str) -> 'Journalist':
+
         try:
             user = Journalist.query.filter_by(username=username).one()
         except NoResultFound:
             raise InvalidUsernameException(
                 "invalid username '{}'".format(username))
 
+        if user.username in Journalist.INVALID_USERNAMES and \
+                user.uuid in Journalist.INVALID_USERNAMES:
+            raise InvalidUsernameException(
+                "Invalid username")
+
         if LOGIN_HARDENING:
             cls.throttle_login(user)
 

securedrop/tests/functional/journalist_navigation_steps.py

@@ -322,6 +322,28 @@ def _add_user(self, username, first_name="", last_name="", is_admin=False, hotp=
 
         self.wait_for(lambda: self.driver.find_element_by_id("check-token"))
 
+    def _admin_adds_a_user_with_invalid_username(self):
+        self.safe_click_by_id("add-user")
+
+        self.wait_for(lambda: self.driver.find_element_by_id("username"))
+
+        if not hasattr(self, "accept_languages"):
+            # The add user page has a form with an "ADD USER" button
+            btns = self.driver.find_elements_by_tag_name("button")
+            assert "ADD USER" in [el.text for el in btns]
+
+        invalid_username = 'deleted'
+
+        self.safe_send_keys_by_css_selector('input[name="username"]', invalid_username)
+
+        self.safe_click_by_css_selector("button[type=submit]")
+
+        self.wait_for(lambda: self.driver.find_element_by_css_selector(".form-validation-error"))
+
+        error_msg = self.driver.find_element_by_css_selector(".form-validation-error")
+        assert "This username is invalid because it is reserved for internal use " \
+               "by the software." in error_msg.text
+
     def _admin_adds_a_user(self, is_admin=False, new_username=""):
         self.safe_click_by_id("add-user")
 

securedrop/tests/functional/test_admin_interface.py

@@ -56,6 +56,12 @@ def test_ossec_alert_button(self):
         self._admin_visits_system_config_page()
         self._admin_can_send_test_alert()
 
+    def test_admin_adds_user_with_invalid_username(self):
+        self._admin_logs_in()
+        self._admin_visits_admin_interface()
+        # Add an user with invalid username
+        self._admin_adds_a_user_with_invalid_username()
+
     def test_admin_adds_admin_user(self):
         self._admin_logs_in()
         self._admin_visits_admin_interface()

securedrop/tests/test_journalist.py

@@ -26,7 +26,7 @@
 
 from db import db
 from models import (InvalidPasswordLength, InstanceConfig, Journalist, Reply, Source,
-                    Submission)
+                    InvalidUsernameException, Submission)
 from .utils.instrument import InstrumentedApp
 
 # Smugly seed the RNG for deterministic testing
@@ -1076,6 +1076,66 @@ def test_admin_add_user(journalist_app, test_admin):
                                                uid=new_user.id))
 
 
+def test_admin_add_user_with_invalid_username(journalist_app, test_admin):
+    username = 'deleted'
+
+    with journalist_app.test_client() as app:
+        _login_user(app, test_admin['username'], test_admin['password'], test_admin['otp_secret'])
+
+        resp = app.post(url_for('admin.add_user'),
+                        data=dict(username=username,
+                                  first_name='',
+                                  last_name='',
+                                  password=VALID_PASSWORD,
+                                  is_admin=None))
+
+    assert "This username is invalid because it is reserved for internal use by the software." \
+           in resp.data.decode('utf-8')
+
+
+def test_deleted_user_cannot_login(journalist_app):
+    username = 'deleted'
+    uuid = 'deleted'
+
+    # Create a user with username and uuid as deleted
+    with journalist_app.app_context():
+        user, password = utils.db_helper.init_journalist(is_admin=False)
+        otp_secret = user.otp_secret
+        user.username = username
+        user.uuid = uuid
+        db.session.add(user)
+        db.session.commit()
+
+    # Verify that deleted user is not able to login
+    with journalist_app.test_client() as app:
+        resp = app.post(url_for('main.login'),
+                        data=dict(username=username,
+                                  password=password,
+                                  token=otp_secret))
+    assert resp.status_code == 200
+    text = resp.data.decode('utf-8')
+    assert "Login failed" in text
+
+
+def test_deleted_user_cannot_login_exception(journalist_app):
+    username = 'deleted'
+    uuid = 'deleted'
+
+    # Create a user with username and uuid as deleted
+    with journalist_app.app_context():
+        user, password = utils.db_helper.init_journalist(is_admin=False)
+        otp_secret = user.otp_secret
+        user.username = username
+        user.uuid = uuid
+        db.session.add(user)
+        db.session.commit()
+
+    with pytest.raises(InvalidUsernameException):
+        Journalist.login(username,
+                         password,
+                         TOTP(otp_secret).now())
+
+
 def test_admin_add_user_without_username(journalist_app, test_admin):
     with journalist_app.test_client() as app:
         _login_user(app, test_admin['username'], test_admin['password'],