From ff81bfbd280c42fb4f225706a036af20469a5bd5 Mon Sep 17 00:00:00 2001 From: Felix Engelmann Date: Sat, 14 Mar 2026 21:12:45 +0100 Subject: [PATCH 1/2] Fix issues from initial language deployment --- README.md | 2 +- .../user/user-list/user-list.component.html | 6 +- client/src/assets/i18n/de.json | 4 +- client/src/assets/i18n/en.json | 14 +++- client/src/assets/i18n/it.json | 14 +++- server/src/messages/messages.py | 3 +- .../migrations/util_scripts/add_superadmin.py | 57 ++++++++----- .../b318163f4e81_add_language_columns.py | 17 +++- server/src/resources/account_resources.py | 2 +- server/src/resources/auth_resources.py | 7 ++ server/src/resources/user_resources.py | 11 +-- server/tests/conftest.py | 35 ++++++-- server/tests/test_account_resources.py | 6 +- .../tests/test_account_settings_resources.py | 7 +- server/tests/test_ascent_resources.py | 8 +- server/tests/test_auth_resources.py | 13 ++- server/tests/test_comment_resources.py | 16 ++-- server/tests/test_user_resources.py | 80 ++++++++++++++++++- 18 files changed, 226 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 3c994b7a..f2f37980 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Save time while creating your topo by using a simple click-editor for drawing li ## Installation -If you want to use LocalCrag for your own crag, either deploy [via docker](./docs/docker-compose-installation.md) or [helm on k8s](./helm/README.md) or join our cloud, it's up to you. If you join our cloud you will get automated updates, but you will have to pay a hosting fee (we will not make money charging this fee, it's 1:1 what our cloud provider charges us). +If you want to use LocalCrag for your own crag, either deploy [via docker](./docs/docker-compose-installation.md) or [helm on k8s](./helm/localcrag/README.md) or join our cloud, it's up to you. If you join our cloud you will get automated updates, but you will have to pay a hosting fee (we will not make money charging this fee, it's 1:1 what our cloud provider charges us). ### Configuration diff --git a/client/src/app/modules/user/user-list/user-list.component.html b/client/src/app/modules/user/user-list/user-list.component.html index abec6e6b..30b06991 100644 --- a/client/src/app/modules/user/user-list/user-list.component.html +++ b/client/src/app/modules/user/user-list/user-list.component.html @@ -64,7 +64,7 @@ @if (!user.avatar) { @@ -72,7 +72,7 @@ @if (user.avatar) { @@ -124,7 +124,7 @@ [popup]="true" appendTo="body" > - @if (currentUser.id !== user.id && !user.admin) { + @if (currentUser.id !== user.id && !(user.admin && !currentUser.superadmin)) { 0 THEN + UPDATE instance_settings SET language='de'; + UPDATE account_settings SET language='de'; + END IF; + END $$; + """ + ) op.drop_column("users", "language") diff --git a/server/src/resources/account_resources.py b/server/src/resources/account_resources.py index 131a1f3d..29e8c2c0 100644 --- a/server/src/resources/account_resources.py +++ b/server/src/resources/account_resources.py @@ -20,7 +20,7 @@ def delete(self): """ user = User.find_by_email(get_jwt_identity()) if user.superadmin: - raise BadRequest(ResponseMessage.CANNOT_DELETE_SUPERADMIN.value) + raise BadRequest(ResponseMessage.SUPERADMINS_CANNOT_DELETE_OWN_USER.value) db.session.delete(user) db.session.commit() diff --git a/server/src/resources/auth_resources.py b/server/src/resources/auth_resources.py index 4b4c90f9..c35537ba 100644 --- a/server/src/resources/auth_resources.py +++ b/server/src/resources/auth_resources.py @@ -148,6 +148,13 @@ def post(self): user.password = User.generate_hash(data["newPassword"]) user.reset_password_hash = None user.reset_password_hash_created = None + + # If the user is not yet activated, we may need to activate him + # (could be that he used forgot password before first regular login) + user.activated = True + if not user.activated_at: + user.activated_at = datetime.now(pytz.utc) + db.session.add(user) db.session.commit() access_token = create_access_token(identity=user.email, additional_claims=get_access_token_claims(user)) diff --git a/server/src/resources/user_resources.py b/server/src/resources/user_resources.py index 7054e540..800f34d4 100644 --- a/server/src/resources/user_resources.py +++ b/server/src/resources/user_resources.py @@ -128,16 +128,17 @@ def delete(self, user_id): :param user_id: ID of the User to delete. """ - user: User = User.find_by_id(user_id) + user_to_delete: User = User.find_by_id(user_id) + request_user = User.find_by_email(get_jwt_identity()) - if user.email == get_jwt_identity(): + if user_to_delete.id == request_user.id: # Own user can only be deleted via account settings raise BadRequest(ResponseMessage.CANNOT_DELETE_OWN_USER.value) - if user.superadmin: - raise BadRequest(ResponseMessage.CANNOT_DELETE_SUPERADMIN.value) + if user_to_delete.admin and not request_user.superadmin: + raise Unauthorized(ResponseMessage.ONLY_SUPERADMINS_CAN_DELETE_OTHER_ADMINS.value) - db.session.delete(user) + db.session.delete(user_to_delete) db.session.commit() return jsonify(None), 204 diff --git a/server/tests/conftest.py b/server/tests/conftest.py index 8cdae888..3e884e6d 100644 --- a/server/tests/conftest.py +++ b/server/tests/conftest.py @@ -133,11 +133,20 @@ def gym_mode(): db.session.add(instance_settings) +@pytest.fixture(scope="session") +def superadmin_token(): + return create_access_token( + identity="superadmin@localcrag.invalid.org", + additional_claims={"superadmin": True, "admin": True, "moderator": True, "member": True}, + expires_delta=timedelta(days=1), + ) + + @pytest.fixture(scope="session") def admin_token(): return create_access_token( identity="admin@localcrag.invalid.org", - additional_claims={"admin": True, "moderator": True, "member": True}, + additional_claims={"superadmin": False, "admin": True, "moderator": True, "member": True}, expires_delta=timedelta(days=1), ) @@ -146,7 +155,7 @@ def admin_token(): def admin_refresh_token(): return create_refresh_token( identity="admin@localcrag.invalid.org", - additional_claims={"admin": True, "moderator": True, "member": True}, + additional_claims={"superadmin": False, "admin": True, "moderator": True, "member": True}, expires_delta=timedelta(days=1), ) @@ -155,7 +164,7 @@ def admin_refresh_token(): def moderator_token(): return create_access_token( identity="moderator@localcrag.invalid.org", - additional_claims={"admin": False, "moderator": True, "member": True}, + additional_claims={"superadmin": False, "admin": False, "moderator": True, "member": True}, expires_delta=timedelta(days=1), ) @@ -164,7 +173,7 @@ def moderator_token(): def member_token(): return create_access_token( identity="member@localcrag.invalid.org", - additional_claims={"admin": False, "moderator": False, "member": True}, + additional_claims={"superadmin": False, "admin": False, "moderator": False, "member": True}, expires_delta=timedelta(days=1), ) @@ -173,7 +182,7 @@ def member_token(): def user_token(): return create_access_token( identity="user@localcrag.invalid.org", - additional_claims={"admin": False, "moderator": False, "member": False}, + additional_claims={"superadmin": False, "admin": False, "moderator": False, "member": False}, expires_delta=timedelta(days=1), ) @@ -247,13 +256,27 @@ def clean_uploads_after_all_tests(): def fill_db_with_sample_data(): add_scales() + user = User() + user.email = "superadmin@localcrag.invalid.org" + user.password = User.generate_hash("superadmin") + user.firstname = "superadmin" + user.lastname = "superadmin" + user.activated = True + user.superadmin = True + user.admin = True + user.moderator = True + user.member = True + db.session.add(user) + db.session.flush() + db.session.add(AccountSettings(user_id=user.id)) + user = User() user.email = "admin@localcrag.invalid.org" user.password = User.generate_hash("admin") user.firstname = "admin" user.lastname = "admin" user.activated = True - user.superadmin = True + user.superadmin = False user.admin = True user.moderator = True user.member = True diff --git a/server/tests/test_account_resources.py b/server/tests/test_account_resources.py index 24a9905d..6214817a 100644 --- a/server/tests/test_account_resources.py +++ b/server/tests/test_account_resources.py @@ -13,10 +13,10 @@ def test_delete_own_user_success(client, member_token): assert User.find_by_email("member@localcrag.invalid.org") is None -def test_delete_own_user_forbidden_for_superadmin(client, admin_token): - rv = client.delete("/api/users/account/delete-own-user", token=admin_token, json=None) +def test_delete_own_user_forbidden_for_superadmin(client, superadmin_token): + rv = client.delete("/api/users/account/delete-own-user", token=superadmin_token, json=None) assert rv.status_code == 400, rv.text - assert rv.json["message"] == ResponseMessage.CANNOT_DELETE_SUPERADMIN.value + assert rv.json["message"] == ResponseMessage.SUPERADMINS_CANNOT_DELETE_OWN_USER.value def test_delete_own_user_unauthorized(client): diff --git a/server/tests/test_account_settings_resources.py b/server/tests/test_account_settings_resources.py index 1487b4bf..42314437 100644 --- a/server/tests/test_account_settings_resources.py +++ b/server/tests/test_account_settings_resources.py @@ -45,8 +45,8 @@ def test_comment_reply_email_sent_when_enabled(client, admin_token, member_token }, ) assert rv.status_code == 201, rv.text - # Two new emails: one for root comment to admins, one for reply to member - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 2 + # Four new emails: two for root comment to admin and superadmin, two for reply to member and superadmin + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 4 def test_comment_reply_email_not_sent_when_disabled(client, admin_token, member_token, smtp_mock): @@ -78,7 +78,8 @@ def test_comment_reply_email_not_sent_when_disabled(client, admin_token, member_ }, ) assert rv.status_code == 201, rv.text - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 1 # Only admin notification + # Three mails: Two for root comment (admin and superadmin), one for replay (superadmin), member gets none + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 3 def test_update_account_settings_invalid_language(client, member_token): diff --git a/server/tests/test_ascent_resources.py b/server/tests/test_ascent_resources.py index ee34f101..b421ef28 100644 --- a/server/tests/test_ascent_resources.py +++ b/server/tests/test_ascent_resources.py @@ -278,9 +278,9 @@ def test_send_project_climbed_message(client, smtp_mock, moderator_token, user_t rv = client.post("/api/ascents/send-project-climbed-message", token=user_token, json=project_climbed_data) assert rv.status_code == 204 - assert smtp_mock.return_value.__enter__.return_value.login.call_count == 1 - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 1 - assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 1 + assert smtp_mock.return_value.__enter__.return_value.login.call_count == 2 + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 2 + assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 2 treppe = str(Line.get_id_by_slug("treppe")) @@ -292,7 +292,7 @@ def test_send_project_climbed_message(client, smtp_mock, moderator_token, user_t rv = client.post("/api/ascents/send-project-climbed-message", token=user_token, json=project_climbed_data) assert rv.status_code == 400 - # Try for a non existing line + # Try for a non-existing line project_climbed_data = { "line": "1c39fd1f-6341-4161-a83f-e5de0f861c49", "message": "I climbed the project! I think it's a 9A+ boulder. Cheers, Aidan Roberts", diff --git a/server/tests/test_auth_resources.py b/server/tests/test_auth_resources.py index 3d3b82f2..37a4a042 100644 --- a/server/tests/test_auth_resources.py +++ b/server/tests/test_auth_resources.py @@ -232,8 +232,8 @@ def test_change_password_password_old_pw_incorrect(client, member_token): assert res["message"] == ResponseMessage.OLD_PASSWORD_INCORRECT.value -def test_cannot_promote_admins(client, admin_token): - user = User.find_by_email("admin@localcrag.invalid.org") +def test_cannot_promote_superadmins(client, admin_token): + user = User.find_by_email("superadmin@localcrag.invalid.org") data = { "promotionTarget": "USER", } @@ -241,6 +241,15 @@ def test_cannot_promote_admins(client, admin_token): assert rv.status_code == 401 +def test_cannot_promote_own_user(client, admin_token): + user = User.find_by_email("admin@localcrag.invalid.org") + data = { + "promotionTarget": "USER", + } + rv = client.put(f"/api/users/{user.id}/promote", token=admin_token, json=data) + assert rv.status_code == 409 + + def test_permission_levels(client, user_token, member_token, moderator_token): admin = User.find_by_email("admin@localcrag.invalid.org") any_file_id = str(File.query.first().id) diff --git a/server/tests/test_comment_resources.py b/server/tests/test_comment_resources.py index 41cb36d7..1ab1f4a7 100644 --- a/server/tests/test_comment_resources.py +++ b/server/tests/test_comment_resources.py @@ -487,10 +487,10 @@ def test_admins_receive_email_on_new_comment(client, user_token, smtp_mock): json={"message": "Admin notify", "objectType": "Line", "objectId": str(line_id)}, ) assert rv.status_code == 201, rv.text - # Expect exactly one email (to the single admin) - assert smtp_mock.return_value.__enter__.return_value.login.call_count == 1 - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 1 - assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 1 + # Expect exactly two email (to the admin and superadmin) + assert smtp_mock.return_value.__enter__.return_value.login.call_count == 2 + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 2 + assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 2 def test_parent_receives_email_on_reply(client, member_token, smtp_mock): @@ -521,7 +521,7 @@ def test_parent_receives_email_on_reply(client, member_token, smtp_mock): ) assert rv.status_code == 201, rv.text - # Two emails total: first to admin on root creation, second to parent on reply - assert smtp_mock.return_value.__enter__.return_value.login.call_count == 2 - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 2 - assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 2 + # Four emails total: first to admin and superadmin on root creation, second to parent and superadmin on reply + assert smtp_mock.return_value.__enter__.return_value.login.call_count == 4 + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 4 + assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 4 diff --git a/server/tests/test_user_resources.py b/server/tests/test_user_resources.py index d54ffd91..2ea9d44e 100644 --- a/server/tests/test_user_resources.py +++ b/server/tests/test_user_resources.py @@ -1,8 +1,10 @@ from datetime import datetime, timedelta from uuid import uuid4 +import pytest import pytz +from error_handling.http_exceptions.not_found import NotFound from extensions import db from messages.messages import ResponseMessage from models.file import File @@ -86,9 +88,9 @@ def test_successful_register_user(client, member_token, smtp_mock): assert not res["activated"] assert res["accountLanguage"] == "en" assert res["avatar"] is None - assert smtp_mock.return_value.__enter__.return_value.login.call_count == 2 - assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 2 - assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 2 + assert smtp_mock.return_value.__enter__.return_value.login.call_count == 3 + assert smtp_mock.return_value.__enter__.return_value.sendmail.call_count == 3 + assert smtp_mock.return_value.__enter__.return_value.quit.call_count == 3 def test_unsuccessful_create_user_email_taken(client, member_token): @@ -120,8 +122,78 @@ def test_delete_own_user(client, admin_token): def test_delete_other_user(client, admin_token): - rv = client.delete(f'/api/users/{User.get_id_by_slug("member-member")}', token=admin_token, json=None) + user_id = User.get_id_by_slug("member-member") + rv = client.delete(f"/api/users/{user_id}", token=admin_token, json=None) assert rv.status_code == 204 + with pytest.raises(NotFound): + User.get_id_by_slug("other-user") + + +def test_delete_user_requires_jwt(client): + user_id = User.get_id_by_slug("member-member") + rv = client.delete(f"/api/users/{user_id}") + assert rv.status_code == 401 + + +def test_delete_user_forbidden_for_non_admin(client, member_token): + user_id = User.get_id_by_slug("user-user") + rv = client.delete(f"/api/users/{user_id}", token=member_token, json=None) + assert rv.status_code == 401 + + +def test_delete_user_forbidden_for_moderator(client, moderator_token): + user_id = User.get_id_by_slug("user-user") + rv = client.delete(f"/api/users/{user_id}", token=moderator_token, json=None) + assert rv.status_code == 401 + + +def test_admin_cannot_delete_other_admin(client, admin_token): + # Create a second admin user (not superadmin) to delete + admin_user = User( + email="other-admin@localcrag.invalid.org", + password=User.generate_hash("pw"), + firstname="Other", + lastname="Admin", + activated=True, + superadmin=False, + admin=True, + moderator=True, + member=True, + ) + db.session.add(admin_user) + db.session.commit() + + rv = client.delete(f"/api/users/{admin_user.id}", token=admin_token, json=None) + assert rv.status_code == 401 + assert rv.json["message"] == ResponseMessage.ONLY_SUPERADMINS_CAN_DELETE_OTHER_ADMINS.value + + +def test_superadmin_can_delete_other_admin(client, superadmin_token): + # admin_token fixture represents the superadmin user in seeded test data + admin_user = User( + email="other-admin-2@localcrag.invalid.org", + password=User.generate_hash("pw"), + firstname="Other2", + lastname="Admin2", + activated=True, + superadmin=False, + admin=True, + moderator=True, + member=True, + ) + db.session.add(admin_user) + db.session.commit() + + rv = client.delete(f"/api/users/{admin_user.id}", token=superadmin_token, json=None) + assert rv.status_code == 204 + with pytest.raises(NotFound): + User.find_by_id(admin_user.id) + + +def test_delete_nonexistent_user_returns_404(client, admin_token): + missing_id = str(uuid4()) + rv = client.delete(f"/api/users/{missing_id}", token=admin_token, json=None) + assert rv.status_code == 404 def test_update_user(client, admin_token): From 1bd2fdf7526f258e263b045ac6241dc552e95621 Mon Sep 17 00:00:00 2001 From: Felix Engelmann Date: Sat, 14 Mar 2026 21:16:59 +0100 Subject: [PATCH 2/2] Formatting --- .../src/app/modules/user/user-list/user-list.component.html | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client/src/app/modules/user/user-list/user-list.component.html b/client/src/app/modules/user/user-list/user-list.component.html index 30b06991..13299577 100644 --- a/client/src/app/modules/user/user-list/user-list.component.html +++ b/client/src/app/modules/user/user-list/user-list.component.html @@ -124,7 +124,10 @@ [popup]="true" appendTo="body" > - @if (currentUser.id !== user.id && !(user.admin && !currentUser.superadmin)) { + @if ( + currentUser.id !== user.id && + !(user.admin && !currentUser.superadmin) + ) {