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

Add supplemental regex setting to validate username #592

Merged
merged 9 commits into from
Oct 25, 2023
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
7 changes: 6 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ Changes
`Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
------------------------------------------------------------------------------------

* Nothing new for the moment.
Features / Changes
~~~~~~~~~~~~~~~~~~

* Create an additional settings/environment variable ``MAGPIE_USER_NAME_EXTRA_REGEX`` that acts as an additional
check for whether a ``user_name`` is valid. This creates a further restriction on this value which is useful when there
are additional limits on the ``user_name`` that should be enforced by `Magpie`.

.. _changes_3.36.0:

Expand Down
4 changes: 4 additions & 0 deletions config/magpie.ini
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ magpie.user_registration_notify_enabled = false
magpie.user_registration_notify_email_recipient =
magpie.user_registration_notify_email_template =

# --- user validation settings ---

#magpie.user_name_extra_regex =

# --- user assignment to groups with t&c ---
magpie.group_terms_submission_email_template =
magpie.group_terms_approved_email_template =
Expand Down
17 changes: 17 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,22 @@ remain available as described at the start of the :ref:`Configuration` section.
This value **MUST NOT** be greater than the token length used to identify a :term:`User` to preserve internal
functionalities.

.. envvar:: MAGPIE_USER_NAME_EXTRA_REGEX

(Default: ``None``)

.. versionadded:: 3.37

A (python3 syntax) regular expression used to validate a ``user_name`` when creating or updating a :term:`User`.

For example, if ``MAGPIE_USER_NAME_EXTRA_REGEX='^\w+$'``, then a :term:`User` can have ``userA`` as a ``user_name``
but not ``user.A`` or ``user-A``.

Note that `Magpie` enforces other restrictions that must also be met for a ``user_name`` to be considered valid.
This creates an additional restriction, it does not replace an existing restriction on the ``user_name``.

If this variable is empty or unset, then no additional ``user_name`` validations will be performed.

.. envvar:: MAGPIE_PASSWORD_MIN_LENGTH

[:class:`int`]
Expand Down Expand Up @@ -1428,6 +1444,7 @@ approval procedures.
obtained from the :term:`Pending User`. Parameter ``approval_required`` is provided to generate alternative
`Mako Template`_ contents in case different messages should be sent for each situation.


.. envvar:: MAGPIE_USER_REGISTRATION_DECLINED_EMAIL_TEMPLATE

(Default: |email_ur_declined_mako|_)
Expand Down
8 changes: 8 additions & 0 deletions magpie/api/management/user/user_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,14 @@ def check_user_info(user_name=None, email=None, password=None, group_name=None,
ax.verify_param(user_name, matches=True, param_name="user_name", param_compare=ax.PARAM_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)
if extra_regex:
ax.verify_param(
user_name, matches=True, param_name="user_name", param_compare=extra_regex,
http_error=HTTPBadRequest,
msg_on_fail=s.Users_CheckInfo_UserNameValueExtraRegex_BadRequestResponseSchema.description,
param_content={"setting": "magpie.user_name_extra_regex"}
)
name_range = range(1, 1 + get_constant("MAGPIE_USER_NAME_MAX_LENGTH"))
ax.verify_param(len(user_name), is_in=True, param_name="user_name", param_compare=name_range,
http_error=HTTPBadRequest,
Expand Down
5 changes: 5 additions & 0 deletions magpie/api/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,11 @@ class Users_CheckInfo_UserNameValue_BadRequestResponseSchema(BaseResponseSchemaA
body = ErrorResponseBodySchema(code=HTTPBadRequest.code, description=description)


class Users_CheckInfo_UserNameValueExtraRegex_BadRequestResponseSchema(BaseResponseSchemaAPI):
description = "Invalid 'user_name' value specified. Does not match the extra user name regex."
body = ErrorResponseBodySchema(code=HTTPBadRequest.code, description=description)


class Users_CheckInfo_UserNameSize_BadRequestResponseSchema(BaseResponseSchemaAPI):
description = "Invalid 'user_name' length specified (>{length} characters)." \
.format(length=get_constant("MAGPIE_USER_NAME_MAX_LENGTH"))
Expand Down
72 changes: 72 additions & 0 deletions tests/interfaces.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import html
import itertools
import os
import secrets
Expand Down Expand Up @@ -7515,6 +7516,77 @@ def test_AddUser_FormSubmit(self):
utils.check_val_is_in("id=\"view_users_list\"", body)
utils.check_val_is_in(self.test_user_name, body)

@runner.MAGPIE_TEST_USERS
fmigneault marked this conversation as resolved.
Show resolved Hide resolved
def test_AddUser_FormSubmit_WithExtraUsernameRegex_Invalid(self):
"""
Check that the extra_user_name_regex setting is used to validate a new user name when the user name is
invalid according to that regex but is valid according to the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
fmigneault marked this conversation as resolved.
Show resolved Hide resolved
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^$"}):
data = {"user_name": 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)
msg = s.Users_CheckInfo_UserNameValueExtraRegex_BadRequestResponseSchema.description
utils.check_val_is_in(msg, html.unescape(body))

@runner.MAGPIE_TEST_USERS
def test_AddUser_FormSubmit_WithExtraUsernameRegex_ValidGoodUsername(self):
"""
Check that the user_name_extra_regex setting is used to validate a new user name when the user name is
valid according to that regex and the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^.*$"}):
data = {"user_name": 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)

# redirected response should be directly the list of users with the new one added
utils.check_val_is_in("<h1>Users</h1>", body)
utils.check_val_is_in("id=\"view_users_list\"", body)
utils.check_val_is_in(self.test_user_name, body)

@runner.MAGPIE_TEST_USERS
def test_AddUser_FormSubmit_WithExtraUsernameRegex_InvalidBadUsername(self):
"""
Check that the extra_user_name_regex setting is used to validate a new user name when the user name is
valid according to that regex but not the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^.*$"}):
email = "{}@mail.com".format(self.test_user_name) # the @ symbol is invalid according to the ax.PARAM_REGEX
data = {"user_name": email, "group_name": get_constant("MAGPIE_USERS_GROUP"), "email": email,
"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)

# the error message should indicate that the user name is invalid according to the original regex
# (ax.PARAM_REGEX) not the extra regex
msg = s.Users_CheckInfo_UserNameValue_BadRequestResponseSchema.description
utils.check_val_is_in(msg, html.unescape(body))
msg = s.Users_CheckInfo_UserNameValueExtraRegex_BadRequestResponseSchema.description
utils.check_val_not_in(msg, html.unescape(body))

@runner.MAGPIE_TEST_STATUS
def test_AddGroup_PageStatus(self):
path = "/ui/groups/add"
Expand Down
76 changes: 76 additions & 0 deletions tests/test_magpie_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,82 @@ def test_PostUserGroupWithTerms_Fail(self):
utils.check_val_type(body["user_names"], list)
utils.check_val_not_in(self.test_user_name, body["user_names"])

@runner.MAGPIE_TEST_USERS
def test_PostUsers_NoExtraRegex_ValidRegex(self):
"""
Check that the user_name_extra_regex setting is not used to validate a new user name when user_name_extra_regex
is falsy.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": None}):
data = {
"user_name": self.test_user_name,
"password": self.test_user_name,
"email": "email@example.com",
}
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, 201, expected_method="POST")

@runner.MAGPIE_TEST_USERS
def test_PostUsers_WithExtraRegex_InvalidExtraRegex(self):
"""
Check that the user_name_extra_regex setting is used to validate a new user name when the user name is
invalid according to that regex but is valid according to the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^$"}):
data = {
"user_name": self.test_user_name,
"password": self.test_user_name,
"email": "email@example.com",
}
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_WithExtraRegex_InvalidRegex(self):
"""
Check that the user_name_extra_regex setting is used to validate a new user name when the user name is
valid according to that regex but is invalid according to the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^.*$"}):
data = {
"user_name": "email@example.com",
"password": self.test_user_name,
"email": "email@example.com",
}
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_WithExtraRegex_ValidBoth(self):
"""
Check that the user_name_extra_regex setting is used to validate a new user name when the user name is
valid according to that regex and the ax.PARAM_REGEX.

.. versionadded:: 3.37
"""
utils.warn_version(self, "extra username regex added", "3.37", skip=True)
with utils.mocked_get_settings(settings={"magpie.user_name_extra_regex": "^.*$"}):
data = {
"user_name": self.test_user_name,
"password": self.test_user_name,
"email": "email@example.com",
}
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, 201, expected_method="POST")


@runner.MAGPIE_TEST_API
@runner.MAGPIE_TEST_LOCAL
Expand Down
Loading