From 9a7836046da2514390203767438f1b3c05b8451c Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Fri, 7 Feb 2025 17:32:36 +0100 Subject: [PATCH 01/21] Enhance project_name_validation - make sure that validation functions will have some data in field.data. - If we are using this data with form validations, there is no need to check emtyness of incoming data. --- server/mergin/auth/forms.py | 16 ++++++++++------ server/mergin/sync/forms.py | 3 ++- server/mergin/sync/public_api_controller.py | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/server/mergin/auth/forms.py b/server/mergin/auth/forms.py index f4baa69e..74d2ad44 100644 --- a/server/mergin/auth/forms.py +++ b/server/mergin/auth/forms.py @@ -27,12 +27,16 @@ def username_validation(form, field): is_reserved_word, ) - errors = [ - has_valid_characters(field.data), - has_valid_first_character(field.data), - is_reserved_word(field.data), - is_valid_filename(field.data), - ] + errors = ( + [ + has_valid_characters(field.data), + has_valid_first_character(field.data), + is_reserved_word(field.data), + is_valid_filename(field.data), + ] + if field.data + else [] + ) for error in errors: if error: raise ValidationError(error) diff --git a/server/mergin/sync/forms.py b/server/mergin/sync/forms.py index f5aa484e..a3afb11a 100644 --- a/server/mergin/sync/forms.py +++ b/server/mergin/sync/forms.py @@ -16,7 +16,8 @@ def project_name_validation(name: str) -> str | None: """Check whether project name is valid""" - if not name.strip(): + name = name.strip() if name is not None else name + if not name: return "Project name cannot be empty" errors = [ has_valid_characters(name), diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 0dd3d3a9..e6783f39 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -1209,7 +1209,7 @@ def clone_project(namespace, project_name): # noqa: E501 if not ws.user_has_permissions(current_user, "admin"): abort(403, "You do not have permissions for this workspace") validation = project_name_validation(dest_project) - if validation: + if validation and dest_project != cloned_project.name: abort(400, validation) _project = Project.query.filter_by(name=dest_project, workspace_id=ws.id).first() From 8bac3a7d45e5e3d2d41814587dfdce4d83cca248 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Fri, 7 Feb 2025 19:28:50 +0100 Subject: [PATCH 02/21] Remove username from /app/admin/user - fe: enhance placeholders and error messages in reg forms - remove username from admin create user --- server/mergin/auth/api.yaml | 4 --- server/mergin/auth/controller.py | 3 +- server/mergin/tests/test_auth.py | 34 +++++-------------- .../admin/components/CreateUserForm.vue | 20 +---------- .../admin-lib/src/modules/admin/types.ts | 1 - .../user/components/ChangePasswordForm.vue | 12 ++++--- .../modules/user/views/ChangePasswordView.vue | 6 +++- 7 files changed, 24 insertions(+), 56 deletions(-) diff --git a/server/mergin/auth/api.yaml b/server/mergin/auth/api.yaml index df924109..07831e36 100644 --- a/server/mergin/auth/api.yaml +++ b/server/mergin/auth/api.yaml @@ -201,14 +201,10 @@ paths: schema: type: object required: - - username - email - password - confirm properties: - username: - type: string - example: john.doe email: type: string format: email diff --git a/server/mergin/auth/controller.py b/server/mergin/auth/controller.py index 86fa5610..980bc14c 100644 --- a/server/mergin/auth/controller.py +++ b/server/mergin/auth/controller.py @@ -383,7 +383,8 @@ def register_user(): # pylint: disable=W0613,W0612 from ..celery import send_email_async form = UserRegistrationForm() - if form.validate(): + form.username.data = User.generate_username(form.email.data) + if form.validate_on_submit(): user = User.create(form.username.data, form.email.data, form.password.data) user_created.send(user, source="admin") token = generate_confirmation_token( diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 18b50ffb..fcd98d8b 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -88,65 +88,47 @@ def test_logout(client): # user registration tests test_user_reg_data = [ - ("test", "test@test.com", "#pwd1234", 201), # success + ("test@test.com", "#pwd1234", 201), # success ( - "TesTUser", "test@test.com", "#pwd1234", 201, ), # tests with upper case, but user does not exist ( - "TesTUser2", "test2@test.com", "#pwd1234", 201, ), # tests with upper case, but user does not exist - ("bob", "test@test.com", "#pwd1234", 400), # invalid (short) username - ("test", "test.com", "#pwd1234", 400), # invalid email - ("mergin", "test@test.com", "#pwd1234", 400), # existing user + ("test.com", "#pwd1234", 400), # invalid email + ("admin@example.com", "#pwd1234", 400), # existing user ( - "MerGin", - "tests@test.com", - "#pwd1234", - 400, - ), # tests with upper case but mergin already exists - ( - " mergin ", - "tests@test.com", - "#pwd1234", - 400, - ), # tests with blank spaces, but mergin user already exists - ( - "XmerginX", " tests@test.com ", "#pwd1234", 201, ), # tests with blank spaces, whitespace to be removed ( - "mergin2", " mergin@mergin.com ", "#pwd1234", 400, ), # tests with blank spaces, but email already exists ( - "mergin3", " merGIN@mergin.com ", "#pwd1234", 400, ), # tests with upper case, but email already exists - ("XmerginX", " mergin@mergin.com ", "#pwd123", 400), # invalid password + (" mergin@mergin.com ", "#pwd123", 400), # invalid password ] -@pytest.mark.parametrize("username,email,pwd,expected", test_user_reg_data) -def test_user_register(client, username, email, pwd, expected): +@pytest.mark.parametrize("email,pwd,expected", test_user_reg_data) +def test_user_register(client, email, pwd, expected): login_as_admin(client) url = url_for("/.mergin_auth_controller_register_user") - data = {"username": username, "email": email, "password": pwd, "confirm": pwd} + data = {"email": email, "password": pwd, "confirm": pwd} resp = client.post(url, data=json.dumps(data), headers=json_headers) assert resp.status_code == expected if expected == 201: - user = User.query.filter_by(username=username).first() + user = User.query.filter_by(email=email.strip()).first() assert user assert user.active assert not user.verified_email # awaits user confirmation diff --git a/web-app/packages/admin-lib/src/modules/admin/components/CreateUserForm.vue b/web-app/packages/admin-lib/src/modules/admin/components/CreateUserForm.vue index a65aa841..8de69926 100644 --- a/web-app/packages/admin-lib/src/modules/admin/components/CreateUserForm.vue +++ b/web-app/packages/admin-lib/src/modules/admin/components/CreateUserForm.vue @@ -6,22 +6,6 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial