From 1a46828fd745a5f533cb1a307a14dc95acf18ffe Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 28 Nov 2025 12:50:11 +0100 Subject: [PATCH 01/14] return 404 to anonymous user requesting private project info --- server/mergin/sync/permissions.py | 3 +++ server/mergin/tests/test_public_api_v2.py | 8 +++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 4305a15f..b249e93c 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -219,6 +219,9 @@ def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled if not scheduled: project = project.filter(Project.removed_at.is_(None)) project = project.first_or_404() + # we don't want to tell anonymous user if a private project exists + if current_user.is_anonymous and not project.public: + abort(404) workspace = project.workspace if not workspace: abort(404) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index dda0bc53..9f636dc1 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -10,6 +10,8 @@ create_workspace, create_project, upload_file_to_project, + login, + file_info, ) from ..auth.models import User @@ -47,7 +49,6 @@ _get_changes_with_diff_0_size, _get_changes_without_added, ) -from .utils import add_user, file_info def test_schedule_delete_project(client): @@ -173,8 +174,13 @@ def test_get_project(client): admin = User.query.filter_by(username=DEFAULT_USER[0]).first() test_workspace = create_workspace() project = create_project("new_project", test_workspace, admin) + user = add_user("tests", "tests") logout(client) + # anonymous user cannot access the resource + response = client.get(f"v2/projects/{project.id}") + assert response.status_code == 404 # lack of permissions + login(client, user.username, "tests") response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 # access public project From e8b1ba78d37364f278cbbb130d36c16c8145948d Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 28 Nov 2025 13:12:29 +0100 Subject: [PATCH 02/14] failing test --- server/mergin/tests/test_public_api_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 9f636dc1..7ab7c190 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -174,12 +174,12 @@ def test_get_project(client): admin = User.query.filter_by(username=DEFAULT_USER[0]).first() test_workspace = create_workspace() project = create_project("new_project", test_workspace, admin) - user = add_user("tests", "tests") logout(client) # anonymous user cannot access the resource response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 # lack of permissions + user = add_user("tests", "tests") login(client, user.username, "tests") response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 From 1152229cf316a2d48212f6c50d19217b88de9a0e Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 28 Nov 2025 13:44:13 +0100 Subject: [PATCH 03/14] failing test 2 --- server/mergin/tests/test_public_api_v2.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 7ab7c190..41100f41 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -1,6 +1,7 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import time from . import DEFAULT_USER from .utils import ( @@ -175,12 +176,13 @@ def test_get_project(client): test_workspace = create_workspace() project = create_project("new_project", test_workspace, admin) logout(client) - # anonymous user cannot access the resource + # anonymous user cannot access the private resource response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 # lack of permissions user = add_user("tests", "tests") login(client, user.username, "tests") + time.sleep(1) response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 # access public project From b350cd506530cb74922ecadb124358d0a6413a7d Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 28 Nov 2025 15:29:21 +0100 Subject: [PATCH 04/14] DEBUG failing test 3 --- server/mergin/sync/permissions.py | 4 ++++ server/mergin/tests/test_public_api_v2.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index b249e93c..65399e4e 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -222,6 +222,10 @@ def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled # we don't want to tell anonymous user if a private project exists if current_user.is_anonymous and not project.public: abort(404) + if current_user.is_anonymous: + print("Permissions info: User is anonymous") + else: + print(f"Permissions info: Logged in as {current_user.username}") workspace = project.workspace if not workspace: abort(404) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 41100f41..6e0bfef4 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -1,7 +1,6 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import time from . import DEFAULT_USER from .utils import ( @@ -181,8 +180,9 @@ def test_get_project(client): assert response.status_code == 404 # lack of permissions user = add_user("tests", "tests") + print(f"Test info: User '{user.username}' created.") login(client, user.username, "tests") - time.sleep(1) + print(f"Test info: User '{user.username}' logged in.") response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 # access public project From 05c88d8c834876e213e073b99200474251db7576 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Fri, 28 Nov 2025 15:47:30 +0100 Subject: [PATCH 05/14] DEBUG try to print to stderr - 4 --- server/mergin/sync/permissions.py | 7 +++++-- server/mergin/tests/test_public_api_v2.py | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 65399e4e..1f5fee05 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -2,6 +2,7 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import sys import os from functools import wraps from typing import Optional @@ -223,9 +224,11 @@ def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled if current_user.is_anonymous and not project.public: abort(404) if current_user.is_anonymous: - print("Permissions info: User is anonymous") + print("Permissions info: User is anonymous", file=sys.stderr) else: - print(f"Permissions info: Logged in as {current_user.username}") + print( + f"Permissions info: Logged in as {current_user.username}", file=sys.stderr + ) workspace = project.workspace if not workspace: abort(404) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 6e0bfef4..5487fac8 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -21,6 +21,7 @@ from sqlalchemy.exc import IntegrityError import pytest from datetime import datetime, timedelta, timezone +import sys from mergin.app import db from mergin.config import Configuration @@ -180,9 +181,9 @@ def test_get_project(client): assert response.status_code == 404 # lack of permissions user = add_user("tests", "tests") - print(f"Test info: User '{user.username}' created.") + print(f"Test info: User '{user.username}' created.", file=sys.stderr) login(client, user.username, "tests") - print(f"Test info: User '{user.username}' logged in.") + print(f"Test info: User '{user.username}' logged in.", file=sys.stderr) response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 # access public project From be212c2ccaae300fb98fccaea47ab6a15d6a9819 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 11:31:10 +0100 Subject: [PATCH 06/14] DEBUG 5: reset Global read --- server/mergin/tests/test_public_api_v2.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 5487fac8..59312744 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -168,6 +168,7 @@ def test_project_members(client): # access provided by workspace role cannot be removed directly response = client.delete(url + f"/{user.id}") assert response.status_code == 404 + Configuration.GLOBAL_READ = 0 def test_get_project(client): @@ -180,6 +181,10 @@ def test_get_project(client): response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 # lack of permissions + print( + f"Global permissions: Global read is {Configuration.GLOBAL_READ}, Global write is {Configuration.GLOBAL_WRITE}, Global admin is {Configuration.GLOBAL_ADMIN}", + file=sys.stderr, + ) user = add_user("tests", "tests") print(f"Test info: User '{user.username}' created.", file=sys.stderr) login(client, user.username, "tests") From 19798f5dbba5980e9fdaecedf8683b6140a5dc1b Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 11:49:03 +0100 Subject: [PATCH 07/14] validate version param schema --- server/mergin/sync/public_api_v2.yaml | 11 ++++++----- server/mergin/tests/test_public_api_v2.py | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/server/mergin/sync/public_api_v2.yaml b/server/mergin/sync/public_api_v2.yaml index bf3db007..c1c74f68 100644 --- a/server/mergin/sync/public_api_v2.yaml +++ b/server/mergin/sync/public_api_v2.yaml @@ -87,8 +87,7 @@ paths: description: Include list of files at specific version required: false schema: - type: string - example: v3 + $ref: "#/components/schemas/VersionName" responses: "200": description: Success @@ -305,9 +304,7 @@ paths: default: false example: true version: - type: string - pattern: '^$|^v\d+$' - example: v2 + $ref: "#/components/schemas/VersionName" changes: type: object required: @@ -849,3 +846,7 @@ components: - editor - writer - owner + VersionName: + type: string + pattern: '^$|^v\d+$' + example: v2 diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 59312744..d30fab2f 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -247,6 +247,9 @@ def test_get_project(client): ) assert len(response.json["files"]) == 3 assert {f["path"] for f in response.json["files"]} == set(files) + # invalid version format parameter + response = client.get(f"v2/projects/{project.id}?files_at_version=3") + assert response.status_code == 400 push_data = [ From 99e4414b454fdf880841e71bb29f940570518403 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 11:49:44 +0100 Subject: [PATCH 08/14] rm prints --- server/mergin/sync/permissions.py | 6 ------ server/mergin/tests/test_public_api_v2.py | 6 ------ 2 files changed, 12 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 1f5fee05..28dbadee 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -223,12 +223,6 @@ def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled # we don't want to tell anonymous user if a private project exists if current_user.is_anonymous and not project.public: abort(404) - if current_user.is_anonymous: - print("Permissions info: User is anonymous", file=sys.stderr) - else: - print( - f"Permissions info: Logged in as {current_user.username}", file=sys.stderr - ) workspace = project.workspace if not workspace: abort(404) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index d30fab2f..8b57e781 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -181,14 +181,8 @@ def test_get_project(client): response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 # lack of permissions - print( - f"Global permissions: Global read is {Configuration.GLOBAL_READ}, Global write is {Configuration.GLOBAL_WRITE}, Global admin is {Configuration.GLOBAL_ADMIN}", - file=sys.stderr, - ) user = add_user("tests", "tests") - print(f"Test info: User '{user.username}' created.", file=sys.stderr) login(client, user.username, "tests") - print(f"Test info: User '{user.username}' logged in.", file=sys.stderr) response = client.get(f"v2/projects/{project.id}") assert response.status_code == 403 # access public project From e1fab33e62bac4629ec81b5032f0a4408255337d Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 12:19:30 +0100 Subject: [PATCH 09/14] add flag for v1 compatibility --- server/mergin/sync/permissions.py | 26 +++++++++++++++---- .../mergin/sync/public_api_v2_controller.py | 18 +++++++------ 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 28dbadee..b18f5835 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -210,7 +210,21 @@ def require_project(ws, project_name, permission) -> Project: return project -def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled=False): +def require_project_by_uuid( + uuid: str, permission: ProjectPermissions, scheduled=False, expose=True +) -> Project: + """ + Retrieves a project by UUID after validating existence, workspace status, and permissions. + + Args: + uuid (str): The unique identifier of the project. + permission (ProjectPermissions): The permission level required to access the project. + scheduled (bool, optional): If ``True``, bypasses the check for projects marked for deletion. + expose (bool, optional): Controls security disclosure behavior on permission failure. + - If `True`: Returns 403 Forbidden (reveals project exists but access is denied). + - If `False`: Returns 404 Not Found (hides project existence for security). + Defaults to `True` for v1 endpoints compatibility. + """ if not is_valid_uuid(uuid): abort(404) @@ -220,16 +234,18 @@ def require_project_by_uuid(uuid: str, permission: ProjectPermissions, scheduled if not scheduled: project = project.filter(Project.removed_at.is_(None)) project = project.first_or_404() - # we don't want to tell anonymous user if a private project exists - if current_user.is_anonymous and not project.public: - abort(404) + workspace = project.workspace if not workspace: abort(404) if not is_active_workspace(workspace): abort(404, "Workspace doesn't exist") if not permission.check(project, current_user): - abort(403, "You do not have permissions for this project") + # we don't want to tell anonymous user if a private project exists + if expose: + abort(403, "You do not have permissions for this project") + else: + abort(404) return project diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 27e0355a..48469353 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -57,7 +57,7 @@ def schedule_delete_project(id): Celery job `mergin.sync.tasks.remove_projects_backups` does the rest. """ - project = require_project_by_uuid(id, ProjectPermissions.Delete) + project = require_project_by_uuid(id, ProjectPermissions.Delete, expose=False) project.removed_at = datetime.utcnow() project.removed_by = current_user.id db.session.commit() @@ -68,7 +68,9 @@ def schedule_delete_project(id): @auth_required def delete_project_now(id): """Delete the project immediately""" - project = require_project_by_uuid(id, ProjectPermissions.Delete, scheduled=True) + project = require_project_by_uuid( + id, ProjectPermissions.Delete, scheduled=True, expose=False + ) project.delete() return NoContent, 204 @@ -77,7 +79,7 @@ def delete_project_now(id): @auth_required def update_project(id): """Rename project""" - project = require_project_by_uuid(id, ProjectPermissions.Update) + project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) new_name = request.json["name"].strip() validation_error = project_name_validation(new_name) if validation_error: @@ -100,7 +102,7 @@ def update_project(id): @auth_required def get_project_collaborators(id): """Get project collaborators, with both direct role and inherited role from workspace""" - project = require_project_by_uuid(id, ProjectPermissions.Update) + project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) project_members = [] for user, workspace_role in project.workspace.members(): project_role = project.get_role(user.id) @@ -123,7 +125,7 @@ def get_project_collaborators(id): @auth_required def add_project_collaborator(id): """Add project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update) + project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) user = User.get_by_login(request.json["user"]) if not user: abort(404) @@ -140,7 +142,7 @@ def add_project_collaborator(id): @auth_required def update_project_collaborator(id, user_id): """Update project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update) + project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) user = User.query.filter_by(id=user_id, active=True).first_or_404() if not project.get_role(user_id): abort(404) @@ -154,7 +156,7 @@ def update_project_collaborator(id, user_id): @auth_required def remove_project_collaborator(id, user_id): """Remove project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update) + project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) if not project.get_role(user_id): abort(404) @@ -165,7 +167,7 @@ def remove_project_collaborator(id, user_id): def get_project(id, files_at_version=None): """Get project info. Include list of files at specific version if requested.""" - project = require_project_by_uuid(id, ProjectPermissions.Read) + project = require_project_by_uuid(id, ProjectPermissions.Read, expose=False) data = ProjectSchemaV2().dump(project) if files_at_version: pv = ProjectVersion.query.filter_by( From d80900bea3e05ca36676c4f281ec531e46c56469 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 12:47:14 +0100 Subject: [PATCH 10/14] actions should expose --- server/mergin/sync/public_api_v2_controller.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 48469353..dadd4a34 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -57,7 +57,7 @@ def schedule_delete_project(id): Celery job `mergin.sync.tasks.remove_projects_backups` does the rest. """ - project = require_project_by_uuid(id, ProjectPermissions.Delete, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Delete) project.removed_at = datetime.utcnow() project.removed_by = current_user.id db.session.commit() @@ -68,9 +68,7 @@ def schedule_delete_project(id): @auth_required def delete_project_now(id): """Delete the project immediately""" - project = require_project_by_uuid( - id, ProjectPermissions.Delete, scheduled=True, expose=False - ) + project = require_project_by_uuid(id, ProjectPermissions.Delete, scheduled=True) project.delete() return NoContent, 204 @@ -79,7 +77,7 @@ def delete_project_now(id): @auth_required def update_project(id): """Rename project""" - project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Update) new_name = request.json["name"].strip() validation_error = project_name_validation(new_name) if validation_error: @@ -102,7 +100,7 @@ def update_project(id): @auth_required def get_project_collaborators(id): """Get project collaborators, with both direct role and inherited role from workspace""" - project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Update) project_members = [] for user, workspace_role in project.workspace.members(): project_role = project.get_role(user.id) @@ -125,7 +123,7 @@ def get_project_collaborators(id): @auth_required def add_project_collaborator(id): """Add project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Update) user = User.get_by_login(request.json["user"]) if not user: abort(404) @@ -142,7 +140,7 @@ def add_project_collaborator(id): @auth_required def update_project_collaborator(id, user_id): """Update project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Update) user = User.query.filter_by(id=user_id, active=True).first_or_404() if not project.get_role(user_id): abort(404) @@ -156,7 +154,7 @@ def update_project_collaborator(id, user_id): @auth_required def remove_project_collaborator(id, user_id): """Remove project collaborator""" - project = require_project_by_uuid(id, ProjectPermissions.Update, expose=False) + project = require_project_by_uuid(id, ProjectPermissions.Update) if not project.get_role(user_id): abort(404) From e8451429b41e3a39687f546041eed6c390b0729f Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 13:08:33 +0100 Subject: [PATCH 11/14] GET project never exposes --- server/mergin/sync/permissions.py | 2 +- server/mergin/tests/test_public_api_v2.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index b18f5835..0d61e407 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -223,7 +223,7 @@ def require_project_by_uuid( expose (bool, optional): Controls security disclosure behavior on permission failure. - If `True`: Returns 403 Forbidden (reveals project exists but access is denied). - If `False`: Returns 404 Not Found (hides project existence for security). - Defaults to `True` for v1 endpoints compatibility. + Standard is that reading results in 404, while writing results in 403 """ if not is_valid_uuid(uuid): abort(404) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 8b57e781..63336be5 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -180,11 +180,11 @@ def test_get_project(client): # anonymous user cannot access the private resource response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 - # lack of permissions + # lack of permissions also results in 404 for GET project user = add_user("tests", "tests") login(client, user.username, "tests") response = client.get(f"v2/projects/{project.id}") - assert response.status_code == 403 + assert response.status_code == 404 # access public project project.public = True db.session.commit() From b1e895c896e54acb73dea79bfe96216f80a5d65e Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 14:19:26 +0100 Subject: [PATCH 12/14] cleanup --- server/mergin/sync/permissions.py | 3 +-- server/mergin/sync/public_api_v2_controller.py | 1 - server/mergin/tests/test_public_api_v2.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 0d61e407..6401663b 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -2,7 +2,6 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import sys import os from functools import wraps from typing import Optional @@ -241,8 +240,8 @@ def require_project_by_uuid( if not is_active_workspace(workspace): abort(404, "Workspace doesn't exist") if not permission.check(project, current_user): - # we don't want to tell anonymous user if a private project exists if expose: + # we don't want to tell anonymous user if a private project exists abort(403, "You do not have permissions for this project") else: abort(404) diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index dadd4a34..67b75820 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -42,7 +42,6 @@ from .public_api_controller import catch_sync_failure from .schemas import ( ProjectMemberSchema, - ProjectVersionSchema, UploadChunkSchema, ProjectSchema, ) diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 63336be5..98850b82 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -21,7 +21,6 @@ from sqlalchemy.exc import IntegrityError import pytest from datetime import datetime, timedelta, timezone -import sys from mergin.app import db from mergin.config import Configuration From 0007fa2fea9147413861f236e300489acf6849bb Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Tue, 2 Dec 2025 16:09:45 +0100 Subject: [PATCH 13/14] return 403 for authenticated without permission --- server/mergin/sync/permissions.py | 10 +++++----- server/mergin/tests/test_public_api_v2.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/mergin/sync/permissions.py b/server/mergin/sync/permissions.py index 6401663b..7dd042d5 100644 --- a/server/mergin/sync/permissions.py +++ b/server/mergin/sync/permissions.py @@ -233,6 +233,9 @@ def require_project_by_uuid( if not scheduled: project = project.filter(Project.removed_at.is_(None)) project = project.first_or_404() + if not expose and current_user.is_anonymous and not project.public: + # we don't want to tell anonymous user if a private project exists + abort(404) workspace = project.workspace if not workspace: @@ -240,11 +243,8 @@ def require_project_by_uuid( if not is_active_workspace(workspace): abort(404, "Workspace doesn't exist") if not permission.check(project, current_user): - if expose: - # we don't want to tell anonymous user if a private project exists - abort(403, "You do not have permissions for this project") - else: - abort(404) + abort(403, "You do not have permissions for this project") + return project diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 98850b82..b890f0b9 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -179,11 +179,11 @@ def test_get_project(client): # anonymous user cannot access the private resource response = client.get(f"v2/projects/{project.id}") assert response.status_code == 404 - # lack of permissions also results in 404 for GET project + # lack of permissions user = add_user("tests", "tests") login(client, user.username, "tests") response = client.get(f"v2/projects/{project.id}") - assert response.status_code == 404 + assert response.status_code == 403 # access public project project.public = True db.session.commit() From 31518fb1589041ab6748850b4b55a12a47354794 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Tue, 25 Nov 2025 16:01:34 +0100 Subject: [PATCH 14/14] Fix failing tests with random 504 Do not update global config variable for gevent mode. Make sure we do not use gevent env for tests apart of dedicated tests. In those tests mock configuration rather than modifing global variable. --- server/.test.env | 1 + server/mergin/tests/test_middleware.py | 99 +++++++++++++++----------- 2 files changed, 59 insertions(+), 41 deletions(-) diff --git a/server/.test.env b/server/.test.env index bdaa7bfa..63294a3f 100644 --- a/server/.test.env +++ b/server/.test.env @@ -24,3 +24,4 @@ SECURITY_BEARER_SALT='bearer' SECURITY_EMAIL_SALT='email' SECURITY_PASSWORD_SALT='password' DIAGNOSTIC_LOGS_DIR=/tmp/diagnostic_logs +GEVENT_WORKER=0 \ No newline at end of file diff --git a/server/mergin/tests/test_middleware.py b/server/mergin/tests/test_middleware.py index 82b9cf26..2f5cbe4f 100644 --- a/server/mergin/tests/test_middleware.py +++ b/server/mergin/tests/test_middleware.py @@ -6,6 +6,7 @@ import psycogreen.gevent import pytest import sqlalchemy +from unittest.mock import patch from ..app import create_simple_app, GeventTimeoutMiddleware, db from ..config import Configuration @@ -14,58 +15,74 @@ @pytest.mark.parametrize("use_middleware", [True, False]) def test_use_middleware(use_middleware): """Test using middleware""" - Configuration.GEVENT_WORKER = use_middleware - Configuration.GEVENT_REQUEST_TIMEOUT = 1 - application = create_simple_app() + with patch.object( + Configuration, + "GEVENT_WORKER", + use_middleware, + ), patch.object( + Configuration, + "GEVENT_REQUEST_TIMEOUT", + 1, + ): + application = create_simple_app() - def ping(): - gevent.sleep(Configuration.GEVENT_REQUEST_TIMEOUT + 1) - return "pong" + def ping(): + gevent.sleep(Configuration.GEVENT_REQUEST_TIMEOUT + 1) + return "pong" - application.add_url_rule("/test", "ping", ping) - app_context = application.app_context() - app_context.push() + application.add_url_rule("/test", "ping", ping) + app_context = application.app_context() + app_context.push() - assert isinstance(application.wsgi_app, GeventTimeoutMiddleware) == use_middleware - # in case of gevent, dummy endpoint it set to time out - assert application.test_client().get("/test").status_code == ( - 504 if use_middleware else 200 - ) + assert ( + isinstance(application.wsgi_app, GeventTimeoutMiddleware) == use_middleware + ) + # in case of gevent, dummy endpoint it set to time out + assert application.test_client().get("/test").status_code == ( + 504 if use_middleware else 200 + ) def test_catch_timeout(): """Test proper handling of gevent timeout with db.session.rollback""" psycogreen.gevent.patch_psycopg() - Configuration.GEVENT_WORKER = True - Configuration.GEVENT_REQUEST_TIMEOUT = 1 - application = create_simple_app() + with patch.object( + Configuration, + "GEVENT_WORKER", + True, + ), patch.object( + Configuration, + "GEVENT_REQUEST_TIMEOUT", + 1, + ): + application = create_simple_app() - def unhandled(): - try: - db.session.execute("SELECT pg_sleep(1.1);") - finally: - db.session.execute("SELECT 1;") - return "" + def unhandled(): + try: + db.session.execute("SELECT pg_sleep(1.1);") + finally: + db.session.execute("SELECT 1;") + return "" - def timeout(): - try: - db.session.execute("SELECT pg_sleep(1.1);") - except gevent.timeout.Timeout: - db.session.rollback() - raise - finally: - db.session.execute("SELECT 1;") - return "" + def timeout(): + try: + db.session.execute("SELECT pg_sleep(1.1);") + except gevent.timeout.Timeout: + db.session.rollback() + raise + finally: + db.session.execute("SELECT 1;") + return "" - application.add_url_rule("/unhandled", "unhandled", unhandled) - application.add_url_rule("/timeout", "timeout", timeout) - app_context = application.app_context() - app_context.push() + application.add_url_rule("/unhandled", "unhandled", unhandled) + application.add_url_rule("/timeout", "timeout", timeout) + app_context = application.app_context() + app_context.push() - assert application.test_client().get("/timeout").status_code == 504 + assert application.test_client().get("/timeout").status_code == 504 - # in case of missing rollback sqlalchemy would raise error - with pytest.raises(sqlalchemy.exc.PendingRollbackError): - application.test_client().get("/unhandled") + # in case of missing rollback sqlalchemy would raise error + with pytest.raises(sqlalchemy.exc.PendingRollbackError): + application.test_client().get("/unhandled") - db.session.rollback() + db.session.rollback()