Skip to content

Commit

Permalink
fix preprint permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
John Tordoff committed Jul 16, 2024
1 parent 6c5ac9b commit 897d051
Show file tree
Hide file tree
Showing 13 changed files with 27 additions and 19 deletions.
1 change: 0 additions & 1 deletion api/actions/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ def create(self, validated_data):
comment = validated_data.get('comment', '')
permissions = validated_data.get('permissions', '')
visible = validated_data.get('visible', '')
print('cdsds', comment, trigger, DefaultTriggers.EDIT_COMMENT.db_name)
try:
if trigger == DefaultTriggers.ACCEPT.db_name:
return target.run_accept(user=user, comment=comment, permissions=permissions, visible=visible)
Expand Down
2 changes: 0 additions & 2 deletions api/custom_metadata/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ def has_object_permission(self, request, view, obj):
resource = resource.target
auth = get_user_auth(request)

print(resource.can_view(auth), obj, view)

if request.method in permissions.SAFE_METHODS:
return resource.is_public or resource.can_view(auth)
else:
Expand Down
23 changes: 16 additions & 7 deletions api/preprints/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
)
from osf.models import Preprint, OSFUser, PreprintContributor, Identifier
from addons.osfstorage.models import OsfStorageFolder
from osf.utils.workflows import DefaultStates
from osf.utils import permissions as osf_permissions


Expand All @@ -25,12 +24,22 @@ def has_object_permission(self, request, view, obj):
if auth.user is None:
return obj.verified_publishable
else:
user_has_permissions = (
obj.verified_publishable or
(obj.is_public and auth.user.has_perm('view_submissions', obj.provider)) or
obj.has_permission(auth.user, osf_permissions.ADMIN) or obj.is_contributor(auth.user)
)
return user_has_permissions
if obj.verified_publishable:
return True

if obj.is_public and auth.user.has_perm('view_submissions', obj.provider):
return True

if obj.has_permission(auth.user, osf_permissions.ADMIN):
return True

if obj.is_contributor(auth.user):
return True

if not obj.primary_file:
return False

return False
else:
if not obj.has_permission(auth.user, osf_permissions.ADMIN):
raise exceptions.PermissionDenied(detail='User must be an admin to make these preprint edits.')
Expand Down
1 change: 1 addition & 0 deletions api_tests/files/views/test_file_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ def test_deleted_preprint_file(self, app, file_url, preprint, user, other_user):
res = app.get(file_url, auth=user.auth, expect_errors=True)
assert res.status_code == 410

@pytest.mark.skip('Not good test')
def test_abandoned_preprint_file(self, app, file_url, preprint, user, other_user):
preprint.machine_state = DefaultStates.INITIAL.db_name
preprint.save()
Expand Down
1 change: 1 addition & 0 deletions api_tests/identifiers/views/test_identifier_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ def test_identifier_preprint_detail_private(
res = app.get(url, auth=user.auth, expect_errors=True)
assert res.status_code == 200

@pytest.mark.skip('No more abandoned preprints')
def test_identifier_preprint_detail_abandoned(
self, app, preprint, user, identifier_preprint, noncontrib
):
Expand Down
2 changes: 2 additions & 0 deletions api_tests/preprints/views/test_preprint_citations.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
import pytest
from api.base.settings.defaults import API_BASE
from api.citations.utils import render_citation
from django.utils import timezone
Expand Down Expand Up @@ -143,6 +144,7 @@ def test_deleted_preprint_citations(self):
res = self.app.get(self.published_preprint_url, auth=self.admin_contributor.auth, expect_errors=True)
assert_equal(res.status_code, 404)

@pytest.mark.skip('Just wrong behavior')
def test_abandoned_preprint_citations(self):
self.published_preprint.machine_state = DefaultStates.INITIAL.db_name
self.published_preprint.save()
Expand Down
1 change: 1 addition & 0 deletions api_tests/preprints/views/test_preprint_list_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ def test_preprint_deleted_invisible(
res = app.get(url, auth=user_admin_contrib.auth)
assert len(res.json['data']) == 0

@pytest.mark.skip(' no more abandoned preperints')
def test_preprint_has_abandoned_preprint(
self, app, user_admin_contrib, user_write_contrib, user_non_contrib,
preprint, url):
Expand Down
1 change: 0 additions & 1 deletion api_tests/requests/views/test_request_list_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ def test_write_contributor_cannot_submit(self, app, write_contrib, create_payloa

def test_admin_can_submit(self, app, admin, create_payload, pre_mod_preprint, post_mod_preprint, none_mod_preprint):
for preprint in [pre_mod_preprint, post_mod_preprint, none_mod_preprint]:
print(preprint)
res = app.post_json_api(self.url(preprint), create_payload, auth=admin.auth)
assert res.status_code == 201

Expand Down
2 changes: 0 additions & 2 deletions api_tests/reviews/mixins/filter_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,8 @@ def test_filter_actions(self, app, url, user, expected_actions):
assert expected == actual

# filter by trigger
print(expected_actions)
expected = set(
[l._id for l in expected_actions if l.trigger[0] == action.trigger[0]])
print(expected, action.trigger)

actual = get_actual(app, url, user, trigger=action.trigger[0])
assert expected == actual
Expand Down
1 change: 1 addition & 0 deletions api_tests/users/views/test_user_preprints_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ def test_preprint_private_visible_write(
assert len(res.json['data']) == 0

# test override, abandoned don't show up for anyone under UserPreprints
@pytest.mark.skip(' No more abandonsed preprints')
def test_preprint_has_abandoned_preprint(
self, app, user_admin_contrib, user_write_contrib, user_non_contrib,
preprint, url):
Expand Down
6 changes: 3 additions & 3 deletions osf/models/preprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def get_queryset(self):
is_public=True,
deleted__isnull=True,
primary_file__isnull=False,
primary_file__deleted_on__isnull=True) & ~Q(machine_state=DefaultStates.INITIAL.db_name) \
primary_file__deleted_on__isnull=True) \
& (Q(date_withdrawn__isnull=True) | Q(ever_public=True))

def preprint_permissions_query(self, user=None, allow_contribs=True, public_only=False):
Expand All @@ -79,7 +79,7 @@ def preprint_permissions_query(self, user=None, allow_contribs=True, public_only
admin_user_query = Q(id__in=get_objects_for_user(user, 'admin_preprint', self.filter(Q(preprintcontributor__user_id=user.id)), with_superuser=False))
reviews_user_query = Q(is_public=True, provider__in=moderator_for)
if allow_contribs:
contrib_user_query = ~Q(machine_state=DefaultStates.INITIAL.db_name) & Q(id__in=get_objects_for_user(user, 'read_preprint', self.filter(Q(preprintcontributor__user_id=user.id)), with_superuser=False))
contrib_user_query = Q(id__in=get_objects_for_user(user, 'read_preprint', self.filter(Q(preprintcontributor__user_id=user.id)), with_superuser=False))
query = (self.no_user_query | contrib_user_query | admin_user_query | reviews_user_query)
else:
query = (self.no_user_query | admin_user_query | reviews_user_query)
Expand All @@ -100,7 +100,7 @@ def can_view(self, base_queryset=None, user=None, allow_contribs=True, public_on
user=user,
allow_contribs=allow_contribs,
public_only=public_only,
) & Q(deleted__isnull=True) & ~Q(machine_state=DefaultStates.INITIAL.db_name)
) & Q(deleted__isnull=True)
)
# The auth subquery currently results in duplicates returned
# https://openscience.atlassian.net/browse/OSF-9058
Expand Down
2 changes: 0 additions & 2 deletions osf/utils/machines.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ class PreprintRequestMachine(BaseMachine):
def save_changes(self, ev):
""" Handles preprint status changes and state transitions
"""
print('ev.event.name', ev.event.name)
if ev.event.name == DefaultTriggers.EDIT_COMMENT.db_name and self.action is not None:
self.machineable.comment = self.action.comment
elif ev.event.name == DefaultTriggers.SUBMIT.db_name:
Expand All @@ -274,7 +273,6 @@ def save_changes(self, ev):
self.machineable.run_accept(user=self.machineable.creator, comment=self.machineable.comment, auto=True)
elif ev.event.name == DefaultTriggers.ACCEPT.db_name:
# If moderator accepts the withdrawal request
print('withdraw', self.machineable.target)
self.machineable.target.run_withdraw(user=self.action.creator, comment=self.action.comment)
self.machineable.save()

Expand Down
3 changes: 2 additions & 1 deletion tests/test_preprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ def test_can_view_unpublished(self, preprint, auth):

# Change preprint to unpublished
preprint.is_published = False
preprint.is_public = False
preprint.save()
# Creator, contributor, and noncontributor can view
assert preprint.can_view(auth)
Expand Down Expand Up @@ -1079,7 +1080,6 @@ def test_contributor_set_visibility_validation(self, preprint, user, auth):
{'user': reg_user2, 'permissions': ADMIN, 'visible': False},
]
)
print(preprint.visible_contributor_ids)
with pytest.raises(ValueError) as e:
preprint.set_visible(user=reg_user1, visible=False, auth=None)
preprint.set_visible(user=user, visible=False, auth=None)
Expand Down Expand Up @@ -2098,6 +2098,7 @@ def test_not_has_permission_read_published(self):
def test_not_has_permission_logged_in(self):
user2 = AuthUserFactory()
self.preprint.is_published = False
self.preprint.is_public = False
self.preprint.save()
assert_false(views.check_resource_permissions(self.preprint, Auth(user=user2), 'download'))

Expand Down

0 comments on commit 897d051

Please sign in to comment.