Skip to content

Commit

Permalink
lowercase all usernames
Browse files Browse the repository at this point in the history
  • Loading branch information
mishaschwartz committed Nov 11, 2023
1 parent c6909c1 commit 9e88d1d
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 2 deletions.
8 changes: 7 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ Changes
`Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
------------------------------------------------------------------------------------

* Nothing new for the moment.
Bug Fixes
~~~~~~~~~

* Ensure that ``user_name`` values for all `User` are lowercase.
Ziggurat foundations assumes that a `User` will not have a ``user_name`` that differs from another only in terms of
case. The simplest way to enforce this is to ensure that all ``user_name`` values are lowercase
Previously, this was not enforced so we could create two `User` which could not be differentiated properly.

.. _changes_3.37.1:

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
"""
Case Insensitive User_Name Constraint
Revision ID: 91af68bcdc67
Revises: 5e5acc33adce
Create Date: 2023-11-10 15:58:23.068465
"""

import sqlalchemy as sa
from alembic import op
from sqlalchemy.orm.session import sessionmaker

# Revision identifiers, used by Alembic.
# pylint: disable=C0103,invalid-name # revision control variables not uppercase
revision = "91af68bcdc67"
down_revision = "5e5acc33adce"
branch_labels = None
depends_on = None

Session = sessionmaker()

users = sa.table(
"users",
sa.column("id", sa.Integer),
sa.column("user_name", sa.String),
)


def upgrade():
op.create_index(
"ix_users_user_name_unique_case_insensitive",
"users",
[sa.text("lower(user_name)")],
unique=True,
)
session = Session(bind=op.get_bind())
for user in session.execute(sa.select(users)):
session.execute(users.update().where(users.c.id == user.id).values(user_name=user.user_name.lower()))
session.commit()


def downgrade():
op.drop_index("ix_users_user_name_unique_case_insensitive", "users")
# Previous case information is lost and cannot be restored
5 changes: 4 additions & 1 deletion magpie/api/management/user/user_utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import re
from inspect import cleandoc
from secrets import compare_digest # noqa 'python2-secrets'
from typing import TYPE_CHECKING
Expand Down Expand Up @@ -62,6 +63,8 @@
UserServicesType
)

USERNAME_REGEX = re.compile(r"^[a-z0-9]+(?:[_\-\.][a-z0-9]+)*$")


def create_user(user_name, # type: Str
password, # type: Optional[Str]
Expand Down Expand Up @@ -869,7 +872,7 @@ def check_user_info(user_name=None, email=None, password=None, group_name=None,
ax.verify_param(user_name, not_none=True, not_empty=True, param_name="user_name",
http_error=HTTPBadRequest,
msg_on_fail=s.Users_CheckInfo_UserNameValue_BadRequestResponseSchema.description)
ax.verify_param(user_name, matches=True, param_name="user_name", param_compare=ax.PARAM_REGEX,
ax.verify_param(user_name, matches=True, param_name="user_name", param_compare=USERNAME_REGEX,
http_error=HTTPBadRequest,
msg_on_fail=s.Users_CheckInfo_UserNameValue_BadRequestResponseSchema.description)
extra_regex = get_constant("MAGPIE_USER_NAME_EXTRA_REGEX", raise_missing=False, raise_not_set=False)
Expand Down
100 changes: 100 additions & 0 deletions tests/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -4549,6 +4549,40 @@ def test_PostUsers_Conflict_Name(self):
headers=self.json_headers, cookies=self.cookies, expect_errors=True)
utils.check_response_basic_info(resp, 409, expected_method="POST")

@runner.MAGPIE_TEST_USERS
def test_PostUsers_Invalid_Name_Uppercase(self):
"""
Do not allow creation of users that have uppercase characters.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow uppercase characters or spaces in user_names", "3.37.2", skip=True)
data = {
"user_name": self.test_user_name.upper(),
"password": self.test_user_name,
"email": "{}@mail.com".format(self.test_user_name),
}
resp = utils.test_request(self, "POST", "/users", data=data,
headers=self.json_headers, cookies=self.cookies, expect_errors=True)
utils.check_response_basic_info(resp, 400, expected_method="POST")

@runner.MAGPIE_TEST_USERS
def test_PostUsers_Invalid_Name_Whitespace(self):
"""
Do not allow creation of users that have whitespace.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow uppercase characters or spaces in user_names", "3.37.2", skip=True)
data = {
"user_name": "a {}".format(self.test_user_name),
"password": self.test_user_name,
"email": "{}@mail.com".format(self.test_user_name),
}
resp = utils.test_request(self, "POST", "/users", data=data,
headers=self.json_headers, cookies=self.cookies, expect_errors=True)
utils.check_response_basic_info(resp, 400, expected_method="POST")

@runner.MAGPIE_TEST_USERS
def test_PostUsers_Conflict_Email(self):
utils.warn_version(self, "user creation with duplicate email conflict", "3.11.0", skip=True)
Expand Down Expand Up @@ -4708,6 +4742,36 @@ def test_UpdateUser_username_ReservedKeyword_LoggedUser(self):
headers=self.json_headers, cookies=self.cookies)
utils.check_response_basic_info(resp, 403, expected_method=self.update_method)

@runner.MAGPIE_TEST_USERS
def test_UpdateUser_username_Invalid_Uppercase(self):
"""
Do not allow user_name that contains uppercase characters.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow uppercase characters or spaces in user_names", "3.37.2", skip=True)
utils.TestSetup.create_TestUser(self, override_group_name=get_constant("MAGPIE_ANONYMOUS_GROUP"))
data = {"user_name": self.test_user_name.upper()}
path = "/users/{usr}".format(usr=self.test_user_name)
resp = utils.test_request(self, self.update_method, path, data=data, expect_errors=True,
headers=self.json_headers, cookies=self.cookies)
utils.check_response_basic_info(resp, 400, expected_method=self.update_method)

@runner.MAGPIE_TEST_USERS
def test_UpdateUser_username_Invalid_Whitespace(self):
"""
Do not allow user_name that contains whitespace.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow uppercase characters or spaces in user_names", "3.37.2", skip=True)
utils.TestSetup.create_TestUser(self, override_group_name=get_constant("MAGPIE_ANONYMOUS_GROUP"))
data = {"user_name": "a {}".format(self.test_user_name)}
path = "/users/{usr}".format(usr=self.test_user_name)
resp = utils.test_request(self, self.update_method, path, data=data, expect_errors=True,
headers=self.json_headers, cookies=self.cookies)
utils.check_response_basic_info(resp, 400, expected_method=self.update_method)

@runner.MAGPIE_TEST_USERS
def test_UpdateUser_email(self):
utils.TestSetup.create_TestUser(self, override_group_name=get_constant("MAGPIE_ANONYMOUS_GROUP"))
Expand Down Expand Up @@ -7608,6 +7672,42 @@ def test_AddUser_FormSubmit_WithExtraUsernameRegex_InvalidBadUsername(self):
msg = s.Users_CheckInfo_UserNameValueExtraRegex_BadRequestResponseSchema.description
utils.check_val_not_in(msg, html.unescape(body))

@runner.MAGPIE_TEST_USERS
def test_AddUser_FormSubmit_InvalidUsername_Uppercase(self):
"""
Check that the form submit to create a user fails if the user name contains uppercase characters.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow case-insensitive matching usernames", "3.37.2", skip=True)
data = {"user_name": self.test_user_name.upper(), "group_name": get_constant("MAGPIE_USERS_GROUP"),
"email": "{}@mail.com".format(self.test_user_name), "password": self.test_user_name,
"confirm": self.test_user_name}
path = "/ui/users/add"
form = "add_user_form"
resp = utils.TestSetup.check_FormSubmit(self, form_match=form, form_submit="create", form_data=data,
path=path)
body = utils.check_ui_response_basic_info(resp)
utils.check_val_is_in("Invalid 'user_name' value specified.", html.unescape(body))

@runner.MAGPIE_TEST_USERS
def test_AddUser_FormSubmit_InvalidUsername_Whitespace(self):
"""
Check that the form submit to create a user fails if the user name contains whitespace characters.
.. versionadded:: 3.37.2
"""
utils.warn_version(self, "do not allow case-insensitive matching usernames", "3.37.2", skip=True)
data = {"user_name": "a {}".format(self.test_user_name), "group_name": get_constant("MAGPIE_USERS_GROUP"),
"email": "{}@mail.com".format(self.test_user_name), "password": self.test_user_name,
"confirm": self.test_user_name}
path = "/ui/users/add"
form = "add_user_form"
resp = utils.TestSetup.check_FormSubmit(self, form_match=form, form_submit="create", form_data=data,
path=path)
body = utils.check_ui_response_basic_info(resp)
utils.check_val_is_in("Invalid 'user_name' value specified.", html.unescape(body))

@runner.MAGPIE_TEST_STATUS
def test_AddGroup_PageStatus(self):
path = "/ui/groups/add"
Expand Down

0 comments on commit 9e88d1d

Please sign in to comment.