Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #5378, adds more checks for invalid username #5380

Merged
merged 1 commit into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions securedrop/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
43 changes: 43 additions & 0 deletions securedrop/tests/functional/journalist_navigation_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 7 additions & 0 deletions securedrop/tests/functional/test_admin_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down