diff --git a/docs/_extra/api-reference/hypothesis-v1.yaml b/docs/_extra/api-reference/hypothesis-v1.yaml index a13f57568e7..797e7549270 100644 --- a/docs/_extra/api-reference/hypothesis-v1.yaml +++ b/docs/_extra/api-reference/hypothesis-v1.yaml @@ -194,7 +194,7 @@ components: The userID should be of the format `acct:@` schema: type: string - pattern: 'acct:^[A-Za-z0-9._]{3,30}@.*$' + pattern: 'acct:^[A-Za-z0.9_][A-Za-z0-9._]{1,28}[A-Za-z0-9_]@.*$' # ------------------------- # Reusable responses diff --git a/docs/_extra/api-reference/hypothesis-v2.yaml b/docs/_extra/api-reference/hypothesis-v2.yaml index 1e327c42a52..d1963a77382 100644 --- a/docs/_extra/api-reference/hypothesis-v2.yaml +++ b/docs/_extra/api-reference/hypothesis-v2.yaml @@ -192,7 +192,7 @@ components: The userID should be of the format `acct:@` schema: type: string - pattern: 'acct:^[A-Za-z0-9._]{3,30}@.*$' + pattern: 'acct:^[A-Za-z0.9_][A-Za-z0-9._]{1,28}[A-Za-z0-9_]@.*$' # ------------------------- # Reusable responses diff --git a/h/accounts/schemas.py b/h/accounts/schemas.py index a0003bfb9a6..81e9303bdc0 100644 --- a/h/accounts/schemas.py +++ b/h/accounts/schemas.py @@ -160,7 +160,9 @@ class RegisterSchema(CSRFSchema): validators.Length(min=USERNAME_MIN_LENGTH, max=USERNAME_MAX_LENGTH), colander.Regex( USERNAME_PATTERN, - msg=_("Must have only letters, numbers, periods, and underscores."), + msg=_( + "Must have only letters, numbers, periods and underscores. May not start or end with period." + ), ), unique_username, unblacklisted_username, diff --git a/h/models/user.py b/h/models/user.py index d14c7d29164..e5f8137d68b 100644 --- a/h/models/user.py +++ b/h/models/user.py @@ -19,7 +19,11 @@ USERNAME_MIN_LENGTH = 3 USERNAME_MAX_LENGTH = 30 -USERNAME_PATTERN = "(?i)^[A-Z0-9._]+$" + +# nb. This pattern is used in Python code, JSON schemas and HTML forms, so it +# needs to use portable syntax. +USERNAME_PATTERN = "^[A-Za-z0-9_][A-Za-z0-9._]+[A-Za-z0-9_]$" + EMAIL_MAX_LENGTH = 100 DISPLAY_NAME_MAX_LENGTH = 30 USER_PUBID_LENGTH = 12 diff --git a/h/schemas/api/user.py b/h/schemas/api/user.py index 116450486a0..78ac70f6053 100644 --- a/h/schemas/api/user.py +++ b/h/schemas/api/user.py @@ -3,6 +3,7 @@ EMAIL_MAX_LENGTH, USERNAME_MAX_LENGTH, USERNAME_MIN_LENGTH, + USERNAME_PATTERN, ) from h.schemas.base import JSONSchema @@ -18,7 +19,7 @@ class CreateUserAPISchema(JSONSchema): "type": "string", "minLength": USERNAME_MIN_LENGTH, "maxLength": USERNAME_MAX_LENGTH, - "pattern": "^[A-Za-z0-9._]+$", + "pattern": USERNAME_PATTERN, }, "email": { "type": "string", diff --git a/h/templates/admin/users.html.jinja2 b/h/templates/admin/users.html.jinja2 index 168f8c58e93..3476f7d65ab 100644 --- a/h/templates/admin/users.html.jinja2 +++ b/h/templates/admin/users.html.jinja2 @@ -73,7 +73,7 @@
- +
diff --git a/h/views/admin/users.py b/h/views/admin/users.py index e289d1fcf6b..50713cd9561 100644 --- a/h/views/admin/users.py +++ b/h/views/admin/users.py @@ -5,6 +5,7 @@ from h import models from h.accounts.events import ActivationEvent from h.i18n import TranslationString as _ +from h.models.user import USERNAME_PATTERN from h.security import Permission from h.services.user_rename import UserRenameError, UserRenameService @@ -69,6 +70,7 @@ def users_index(request): "authority": authority, "user": user, "user_meta": user_meta, + "username_pattern": USERNAME_PATTERN, "format_date": format_date, } diff --git a/tests/unit/h/accounts/schemas_test.py b/tests/unit/h/accounts/schemas_test.py index 80781a523a6..3d089b1bc93 100644 --- a/tests/unit/h/accounts/schemas_test.py +++ b/tests/unit/h/accounts/schemas_test.py @@ -86,8 +86,8 @@ def test_it_is_invalid_when_username_too_short(self, pyramid_request): schema = schemas.RegisterSchema().bind(request=pyramid_request) with pytest.raises(colander.Invalid) as exc: - schema.deserialize({"username": "a"}) - assert exc.value.asdict()["username"] == ("Must be 3 characters or more.") + schema.deserialize({"username": "ab"}) + assert "Must be 3 characters or more." in exc.value.asdict()["username"] def test_it_is_invalid_when_username_too_long(self, pyramid_request): schema = schemas.RegisterSchema().bind(request=pyramid_request) @@ -102,7 +102,7 @@ def test_it_is_invalid_with_invalid_characters_in_username(self, pyramid_request with pytest.raises(colander.Invalid) as exc: schema.deserialize({"username": "Fred Flintstone"}) assert exc.value.asdict()["username"] == ( - "Must have only letters, numbers, periods, and underscores." + "Must have only letters, numbers, periods and underscores. May not start or end with period." ) def test_it_is_invalid_with_false_privacy_accepted(self, pyramid_request): diff --git a/tests/unit/h/schemas/api/user_test.py b/tests/unit/h/schemas/api/user_test.py index 9d44267c2b0..0aff8acbb78 100644 --- a/tests/unit/h/schemas/api/user_test.py +++ b/tests/unit/h/schemas/api/user_test.py @@ -47,8 +47,16 @@ def test_it_raises_when_username_too_long(self, schema, payload): with pytest.raises(ValidationError): schema.validate(payload) - def test_it_raises_when_username_format_invalid(self, schema, payload): - payload["username"] = "dagr!un" + @pytest.mark.parametrize( + "username", + [ + "invalid!chars", + ".starts_with_period", + "ends_with_period.", + ], + ) + def test_it_raises_when_username_format_invalid(self, schema, payload, username): + payload["username"] = username with pytest.raises(ValidationError): schema.validate(payload) diff --git a/tests/unit/h/views/admin/users_test.py b/tests/unit/h/views/admin/users_test.py index 93a70e651fb..d5c5ea33e92 100644 --- a/tests/unit/h/views/admin/users_test.py +++ b/tests/unit/h/views/admin/users_test.py @@ -2,6 +2,7 @@ from unittest import mock import pytest +from h_matchers import Any from pyramid import httpexceptions from h.models import Annotation @@ -35,6 +36,7 @@ def test_users_index(pyramid_request): "username": None, "authority": None, "user": None, + "username_pattern": Any.string(), "user_meta": {}, "format_date": format_date, } @@ -89,6 +91,7 @@ def test_users_index_no_user_found(models, pyramid_request): "authority": "foo.org", "user": None, "user_meta": {}, + "username_pattern": Any.string(), "format_date": format_date, } @@ -107,6 +110,7 @@ def test_users_index_user_marked_as_deleted(models, pyramid_request, factories): "authority": "foo.org", "user": None, "user_meta": {}, + "username_pattern": Any.string(), "format_date": format_date, } @@ -128,6 +132,7 @@ def test_users_index_user_found( "authority": "foo.org", "user": user, "user_meta": {"annotations_count": 8}, + "username_pattern": Any.string(), "format_date": format_date, }