Skip to content

Commit

Permalink
add remote site access checks (#1090)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikkonie
mikkonie committed Jan 21, 2025
1 parent 158b5c0 commit 24930d3
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 5 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ Added
- Old owner "remove role" option in ``RoleAssignmentOwnerTransferForm`` (#836)
- Project deletion (#1090)


Changed
-------

Expand Down
156 changes: 155 additions & 1 deletion projectroles/tests/test_permissions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,9 @@ def test_get_category_anon(self):
self.assert_response(self.url_cat, self.user_no_roles, 302)


class TestProjectDeleteView(ProjectPermissionTestBase):
class TestProjectDeleteView(
RemoteSiteMixin, RemoteProjectMixin, ProjectPermissionTestBase
):
"""Tests for ProjectDeleteView permissions"""

def setUp(self):
Expand Down Expand Up @@ -1014,6 +1016,158 @@ def test_get_category_anon(self):
self.project.set_public()
self.assert_response(self.url_cat, self.user_no_roles, 302)

def test_get_remote_not_revoked(self):
"""Test GET with non-revoked remote project as target site"""
site = self.make_site(
name=REMOTE_SITE_NAME,
url=REMOTE_SITE_URL,
mode=SODAR_CONSTANTS['SITE_MODE_TARGET'],
description='',
secret=REMOTE_SITE_SECRET,
)
self.make_remote_project(
project_uuid=self.project.sodar_uuid,
project=self.project,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'],
)
bad_users = [
self.superuser,
self.user_owner_cat,
self.user_delegate_cat,
self.user_contributor_cat,
self.user_guest_cat,
self.user_finder_cat,
self.user_owner,
self.user_delegate,
self.user_contributor,
self.user_guest,
self.user_no_roles,
self.anonymous,
]
self.assert_response(self.url, bad_users, 302)
self.project.set_public()
self.assert_response(self.url, self.user_no_roles, 302)

def test_get_remote_revoked(self):
"""Test GET with revoked remote project"""
site = self.make_site(
name=REMOTE_SITE_NAME,
url=REMOTE_SITE_URL,
mode=SODAR_CONSTANTS['SITE_MODE_TARGET'],
description='',
secret=REMOTE_SITE_SECRET,
)
self.make_remote_project(
project_uuid=self.project.sodar_uuid,
project=self.project,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'],
)
good_users = [
self.superuser,
self.user_owner_cat,
self.user_delegate_cat,
self.user_owner,
self.user_delegate,
]
bad_users = [
self.user_contributor_cat,
self.user_guest_cat,
self.user_finder_cat,
self.user_contributor,
self.user_guest,
self.user_no_roles,
self.anonymous,
]
self.assert_response(self.url, good_users, 200)
self.assert_response(self.url, bad_users, 302)
self.project.set_public()
self.assert_response(self.url, self.user_no_roles, 302)

@override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET)
def test_get_remote_not_revoked_target(self):
"""Test GET with non-revoked remote project as target site"""
site = self.make_site(
name=REMOTE_SITE_NAME,
url=REMOTE_SITE_URL,
mode=SODAR_CONSTANTS['SITE_MODE_SOURCE'],
description='',
secret=REMOTE_SITE_SECRET,
)
self.make_remote_project(
project_uuid=self.category.sodar_uuid,
project=self.category,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'],
)
self.make_remote_project(
project_uuid=self.project.sodar_uuid,
project=self.project,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'],
)
bad_users = [
self.superuser,
self.user_owner_cat,
self.user_delegate_cat,
self.user_contributor_cat,
self.user_guest_cat,
self.user_finder_cat,
self.user_owner,
self.user_delegate,
self.user_contributor,
self.user_guest,
self.user_no_roles,
self.anonymous,
]
self.assert_response(self.url, bad_users, 302)
self.project.set_public()
self.assert_response(self.url, self.user_no_roles, 302)

@override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET)
def test_get_remote_revoked_target(self):
"""Test GET with revoked remote project as target site"""
site = self.make_site(
name=REMOTE_SITE_NAME,
url=REMOTE_SITE_URL,
mode=SODAR_CONSTANTS['SITE_MODE_SOURCE'],
description='',
secret=REMOTE_SITE_SECRET,
)
self.make_remote_project(
project_uuid=self.category.sodar_uuid,
project=self.category,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_READ_ROLES'],
)
self.make_remote_project(
project_uuid=self.project.sodar_uuid,
project=self.project,
site=site,
level=SODAR_CONSTANTS['REMOTE_LEVEL_REVOKED'],
)
good_users = [
self.superuser,
self.user_owner_cat,
self.user_delegate_cat,
self.user_owner,
self.user_delegate,
]
bad_users = [
self.user_contributor_cat,
self.user_guest_cat,
self.user_finder_cat,
self.user_contributor,
self.user_guest,
self.user_no_roles,
self.anonymous,
]
self.assert_response(self.url, good_users, 200)
self.assert_response(self.url, bad_users, 302)
self.project.set_public()
self.assert_response(self.url, self.user_no_roles, 302)


class TestProjectRoleView(ProjectPermissionTestBase):
"""Tests for ProjectRoleView permissions"""
Expand Down
42 changes: 39 additions & 3 deletions projectroles/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@
PROJECT_DELETE_MSG = (
'{project_type} "{project_title}" deleted by user {user_name}.'
)
PROJECT_DELETE_CAT_ERR_MSG = (
'Deletion not allowed for {project_type} with children. Delete the '
'children before attempting deletion.'
)
PROJECT_DELETE_TARGET_ERR_MSG = (
'Deletion not allowed for remote {project_type} with non-revoked access '
'level. Revoke remote access on source site to enable deletion.'
)
PROJECT_DELETE_SOURCE_ERR_MSG = (
'Non-revoked remotes of {project_type} found. Revoke access to them to '
'enable deletion.'
)
TARGET_CREATE_DISABLED_MSG = (
'PROJECTROLES_TARGET_CREATE=False, creation not allowed.'
)
Expand Down Expand Up @@ -1774,17 +1786,41 @@ def dispatch(self, request, *args, **kwargs):
return self.handle_no_permission()
# Prevent access in certain conditions, even for superusers
project = self.get_object()
p_type = get_display_name(project.type)
# Disallow for categories with children
if (
project.type == PROJECT_TYPE_CATEGORY
and project.get_children().count() > 0
):
messages.error(
self.request,
f'Deletion not allowed for {get_display_name(project.type)} '
f'with children. Please first delete the children.',
PROJECT_DELETE_CAT_ERR_MSG.format(project_type=p_type),
)
return redirect('home')
# TODO: Add remote site checks
# Disallow for remote projects which haven't been revoked
if project.is_remote():
rp = RemoteProject.objects.filter(
project_uuid=project.sodar_uuid,
site__mode=SITE_MODE_SOURCE,
).first()
if rp.level != REMOTE_LEVEL_REVOKED:
messages.error(
self.request,
PROJECT_DELETE_TARGET_ERR_MSG.format(project_type=p_type),
)
return redirect('home')
# Disallow for source projects with non-revoked remote projects
# NOTE: Categories can be deleted
elif project.type == PROJECT_TYPE_PROJECT:
rps = RemoteProject.objects.filter(
project_uuid=project.sodar_uuid, site__mode=SITE_MODE_TARGET
).exclude(level__in=[REMOTE_LEVEL_NONE, REMOTE_LEVEL_REVOKED])
if rps.count() > 0:
messages.error(
self.request,
PROJECT_DELETE_SOURCE_ERR_MSG.format(project_type=p_type),
)
return redirect('home')
return super().dispatch(request, *args, **kwargs)

def post(self, *args, **kwargs):
Expand Down

0 comments on commit 24930d3

Please sign in to comment.