Skip to content

Public implicit viewer #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
desafinadude wants to merge 3 commits into from
Closed

Public implicit viewer #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc1f739
turn on all skipped tests
cyber237 Dec 14, 2025
bb6f02c
refactor: streamline token retrieval for project contributors in semi…
desafinadude Dec 14, 2025
eef92dd
feat: enhance permission checking for public project submissions
desafinadude Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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']

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down
3 changes: 0 additions & 3 deletions test/test_search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 25 additions & 25 deletions test/test_submissions_rbac.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -846,25 +846,26 @@ 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(
client,
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(
Expand All @@ -889,31 +890,30 @@ 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(
client,
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",
)

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Loading