lowercase all usernames

Ouranosinc · Nov 11, 2023 · 7ef871d · 7ef871d
1 parent c6909c1
commit 7ef871d
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 2 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -9,7 +9,21 @@ 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 and do not contain whitespace.
+
+  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.
+
+  This change includes a database migration that will convert all ``user_name`` that contain uppercase characters
+  to lowercase. This may cause a database conflict if there are two ``user_name`` values that differ only in terms of
+  case. For example "Test" and "test". If this occurs, please manually update those ``user_name`` values to no longer
+  conflict and try the migration again.
+
+  This also prevents new users from being created that contain whitespace.
 
 .. _changes_3.37.1:
 

diff --git a/magpie/alembic/versions/2023-11-10_91af68bcdc67_case_insensitive_user_name_constraint.py b/magpie/alembic/versions/2023-11-10_91af68bcdc67_case_insensitive_user_name_constraint.py
@@ -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
diff --git a/magpie/api/management/user/user_utils.py b/magpie/api/management/user/user_utils.py
@@ -1,3 +1,4 @@
+import re
 from inspect import cleandoc
 from secrets import compare_digest  # noqa 'python2-secrets'
 from typing import TYPE_CHECKING
@@ -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]
@@ -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)

diff --git a/tests/interfaces.py b/tests/interfaces.py
@@ -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)
@@ -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"))
@@ -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"