From cc1f7396301e5a66ff8639f4dc59958662229e82 Mon Sep 17 00:00:00 2001 From: NINTAI DICK Date: Sun, 14 Dec 2025 19:51:00 +0100 Subject: [PATCH 1/3] turn on all skipped tests --- test/test_search.py | 3 --- test/test_submissions_rbac.py | 30 +++++++++++++++--------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/test/test_search.py b/test/test_search.py index c88004c..d4fafaf 100644 --- a/test/test_search.py +++ b/test/test_search.py @@ -697,9 +697,6 @@ def test_search_access_control_private_project_all_roles( client, role_fixture, role_name, request, private_project_with_submission ): """Test that all project roles (admin, contributor, viewer) can search private project data""" - # skip if role name not org-admin: fix later - if role_name != "org-admin": - pytest.skip("Skipping non org-admin roles for now") # Get the token from the fixture token = request.getfixturevalue(role_fixture) diff --git a/test/test_submissions_rbac.py b/test/test_submissions_rbac.py index 19f7420..7aad649 100644 --- a/test/test_submissions_rbac.py +++ b/test/test_submissions_rbac.py @@ -356,9 +356,9 @@ def test_contributor_can_delete_own_draft( assert cursor.fetchone() is None -@pytest.mark.skip( - reason="Application doesn't check submission ownership for delete operations - any contributor can delete any submission in their project" -) +# @pytest.mark.skip( +# reason="Application doesn't check submission ownership for delete operations - any contributor can delete any submission in their project" +# ) @pytest.mark.rbac @pytest.mark.submission def test_contributor_cannot_delete_other_draft( @@ -668,9 +668,9 @@ def test_contributor_can_unpublish_own_submission( cursor.execute("DELETE FROM submissions WHERE id = %s", (draft["id"],)) -@pytest.mark.skip( - reason="Application doesn't check submission ownership for unpublish operations - any contributor can unpublish any submission in their project" -) +# @pytest.mark.skip( +# reason="Application doesn't check submission ownership for unpublish operations - any contributor can unpublish any submission in their project" +# ) @pytest.mark.rbac @pytest.mark.submission def test_contributor_cannot_unpublish_other_submission( @@ -846,7 +846,7 @@ def test_viewer_cannot_unpublish_submission( # ============================================================================ -@pytest.mark.skip(reason="Semi-private projects not supported by database schema") +# @pytest.mark.skip(reason="Semi-private projects not supported by database schema") @pytest.mark.rbac @pytest.mark.submission def test_semi_private_drafts_same_as_private_project( @@ -889,7 +889,7 @@ def test_semi_private_drafts_same_as_private_project( cursor.execute("DELETE FROM submissions WHERE id = %s", (draft["id"],)) -@pytest.mark.skip(reason="Semi-private projects not supported by database schema") +# @pytest.mark.skip(reason="Semi-private projects not supported by database schema") @pytest.mark.rbac @pytest.mark.submission def test_semi_private_internal_access_same_as_private( @@ -944,7 +944,7 @@ def test_semi_private_internal_access_same_as_private( cursor.execute("DELETE FROM submissions WHERE id = %s", (draft["id"],)) -@pytest.mark.skip(reason="Semi-private projects not supported by database schema") +# @pytest.mark.skip(reason="Semi-private projects not supported by database schema") @pytest.mark.rbac @pytest.mark.submission def test_semi_private_external_cannot_list_submissions( @@ -1009,9 +1009,9 @@ def test_public_project_drafts_remain_private( cursor.execute("DELETE FROM submissions WHERE id = %s", (draft["id"],)) -@pytest.mark.skip( - reason="Application doesn't grant implicit viewer access for public projects - requires explicit project membership" -) +# @pytest.mark.skip( +# reason="Application doesn't grant implicit viewer access for public projects - requires explicit project membership" +# ) @pytest.mark.rbac @pytest.mark.submission def test_public_project_external_user_has_implicit_viewer_role( @@ -1122,9 +1122,9 @@ def test_public_project_external_user_cannot_manage_submissions( cursor.execute("DELETE FROM submissions WHERE id = %s", (draft["id"],)) -@pytest.mark.skip( - reason="Application doesn't grant implicit viewer access for public projects - requires explicit project membership" -) +# @pytest.mark.skip( +# reason="Application doesn't grant implicit viewer access for public projects - requires explicit project membership" +# ) @pytest.mark.rbac @pytest.mark.submission def test_public_project_external_user_can_view_published_data( From bb6f02c06a0a28fb1f87b220ac61ff85f8effcc7 Mon Sep 17 00:00:00 2001 From: desafinadude Date: Sun, 14 Dec 2025 21:58:23 +0200 Subject: [PATCH 2/3] refactor: streamline token retrieval for project contributors in semi-private draft tests --- test/test_submissions_rbac.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_submissions_rbac.py b/test/test_submissions_rbac.py index 7aad649..9b4fe76 100644 --- a/test/test_submissions_rbac.py +++ b/test/test_submissions_rbac.py @@ -854,17 +854,18 @@ def test_semi_private_drafts_same_as_private_project( system_admin_token, semi_private_project, project_contributor, - project_contributor_token, - external_user_token, + external_user, ): """Drafts on semi-private projects behave like private projects""" - add_project_member( + # Get fresh token after adding user to project + project_contributor_token = add_project_member_and_get_token( client, system_admin_token, semi_private_project["id"], - project_contributor["user_id"], + project_contributor, "project-contributor", ) + external_user_token = get_fresh_token(external_user["email"]) # Create draft draft = create_draft_submission( @@ -897,23 +898,22 @@ def test_semi_private_internal_access_same_as_private( system_admin_token, semi_private_project, project_contributor, - project_contributor_token, project_viewer, - project_viewer_token, ): """Internal access rules are same as private projects""" - add_project_member( + # Get fresh tokens after adding users to project + project_contributor_token = add_project_member_and_get_token( client, system_admin_token, semi_private_project["id"], - project_contributor["user_id"], + project_contributor, "project-contributor", ) - add_project_member( + project_viewer_token = add_project_member_and_get_token( client, system_admin_token, semi_private_project["id"], - project_viewer["user_id"], + project_viewer, "project-viewer", ) From eef92dd025634755f5ea6bf792e8cb48b15086e7 Mon Sep 17 00:00:00 2001 From: desafinadude Date: Mon, 15 Dec 2025 08:18:33 +0200 Subject: [PATCH 3/3] feat: enhance permission checking for public project submissions --- auth.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/auth.py b/auth.py index 55f6a4b..a0b82ac 100644 --- a/auth.py +++ b/auth.py @@ -1158,7 +1158,7 @@ def user_has_permission(user_info, permission_name, resource_type=None, resource Unified permission checking function that handles all access control logic. Organization checks are performed for org-level roles but not for attribute-based permissions. """ - from database import get_db_cursor # Import here to avoid circular imports + from database import get_db_cursor user_roles = user_info.get('roles', []) user_id = user_info.get('user_id') @@ -1185,8 +1185,25 @@ def user_has_permission(user_info, permission_name, resource_type=None, resource access_details['access_granted_by'] = 'system_admin_role' access_details['reason'] = 'User has system-admin role' return True, access_details + + # 2. For view_project_submissions on public projects, grant access to all authenticated users + if permission_name == 'view_project_submissions' and resource_type == 'project' and resource_id: + access_details['checks_performed'].append('public_project_check') + try: + with get_db_cursor() as cursor: + cursor.execute(""" + SELECT privacy FROM projects + WHERE id = %s AND deleted_at IS NULL + """, (resource_id,)) + project = cursor.fetchone() + if project and project['privacy'] == 'public': + access_details['access_granted_by'] = 'public_project_implicit_viewer' + access_details['reason'] = 'User has implicit viewer access to public project' + return True, access_details + except Exception as e: + access_details['reason'] = f'Error checking project privacy: {str(e)}' - # 2. Check standard org roles (WITH organization check) + # 3. Check standard org roles (WITH organization check) access_details['checks_performed'].append('org_role_check') org_roles = ['agari-org-owner', 'agari-org-admin', 'agari-org-contributor', 'agari-org-viewer'] @@ -1247,7 +1264,7 @@ def user_has_permission(user_info, permission_name, resource_type=None, resource access_details['reason'] = f'User has org role "{required_role}" (no resource specified)' return True, access_details - # 3. Check attribute-based roles (NO organization check - project-specific permissions) + # 4. Check attribute-based roles (NO organization check - project-specific permissions) if resource_id and user_id: access_details['checks_performed'].append('attribute_role_check') for required_role in required_roles: @@ -1296,7 +1313,7 @@ def user_has_permission(user_info, permission_name, resource_type=None, resource else: access_details['attribute_checks'][-1]['result'] = 'not_found' - # 4. If no access granted + # 5. If no access granted access_details['reason'] = 'User does not have required permissions' return False, access_details