Skip to content

Commit 9fc1c84

Browse files
authored
[ENG-10256] 2.1.9 BE: Fix permission issue where users without permission to an object can access the metadata (#11588)
* Fix permission issue where users without permission to an object can access the metadata (add decorator is_contributor_or_public_resource) * refactor code * resolve CR | refactor code * resolve CR | add unittests * add *args, **kwargs for decorated view
1 parent 22d2abd commit 9fc1c84

File tree

3 files changed

+80
-3
lines changed

3 files changed

+80
-3
lines changed

framework/auth/decorators.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,29 @@ def wrapped(*args, **kwargs):
7676

7777
return wrapped
7878

79+
80+
def is_contributor_or_public_resource(resource_kw='resource'):
81+
"""
82+
Require that user be contributor or resource be public.
83+
"""
84+
def decorator(func):
85+
@wraps(func)
86+
def wrapped(*args, **kwargs):
87+
from osf.models import BaseFileNode, Guid
88+
referent = kwargs.get(resource_kw)
89+
if isinstance(referent, Guid):
90+
referent = referent.referent
91+
target_resource = referent.target if isinstance(referent, BaseFileNode) else referent
92+
if target_resource.is_public:
93+
return func(*args, **kwargs)
94+
auth = Auth.from_kwargs(request.args.to_dict(), {})
95+
if auth.logged_in and target_resource.is_contributor(auth.user):
96+
return func(*args, **kwargs)
97+
raise HTTPError(http_status.HTTP_403_FORBIDDEN)
98+
return wrapped
99+
return decorator
100+
101+
79102
# TODO Can remove after Waterbutler is sending requests to V2 endpoints.
80103
# This decorator has been adapted for use in an APIv2 parser - HMACSignedParser
81104
def must_be_signed(func):

tests/test_auth.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,59 @@ def test_must_have_permission_not_logged_in(self, mock_from_kwargs, mock_to_node
722722
thriller(node=project)
723723
assert ctx.value.code == http_status.HTTP_401_UNAUTHORIZED
724724

725+
def decorated_view(self, *args, **kwargs):
726+
return 'Success'
727+
728+
## 1. Public Access
729+
@mock.patch('framework.auth.decorators.Auth.from_kwargs')
730+
def test_private_resource_not_allow_anonymous(self, mock_auth):
731+
from framework.auth.decorators import is_contributor_or_public_resource
732+
decorator = is_contributor_or_public_resource('resource')
733+
project = ProjectFactory(is_public=False)
734+
mock_auth.return_value = Auth(user=None)
735+
decorated_func = decorator(self.decorated_view)
736+
with decoratorapp.test_request_context():
737+
with pytest.raises(HTTPError) as excinfo:
738+
decorated_func(resource=project)
739+
assert excinfo.value.code == http_status.HTTP_403_FORBIDDEN
740+
741+
@mock.patch('framework.auth.decorators.Auth.from_kwargs')
742+
def test_public_resource_allow_anonymous(self, mock_auth):
743+
from framework.auth.decorators import is_contributor_or_public_resource
744+
decorator = is_contributor_or_public_resource('resource')
745+
project = ProjectFactory(is_public=True)
746+
mock_auth.return_value = Auth(user=None)
747+
decorated_func = decorator(self.decorated_view)
748+
with decoratorapp.test_request_context():
749+
result = decorated_func(resource=project)
750+
assert result == 'Success'
751+
752+
@mock.patch('framework.auth.decorators.Auth.from_kwargs')
753+
def test_private_resource_allows_contributor(self, mock_auth):
754+
from framework.auth.decorators import is_contributor_or_public_resource
755+
user = UserFactory()
756+
project = ProjectFactory(is_public=False)
757+
project.add_contributor(user, save=True)
758+
mock_auth.return_value = Auth(user=user)
759+
decorator = is_contributor_or_public_resource('resource')
760+
decorated_func = decorator(self.decorated_view)
761+
with decoratorapp.test_request_context():
762+
result = decorated_func(resource=project)
763+
assert result == 'Success'
764+
765+
@mock.patch('framework.auth.decorators.Auth.from_kwargs')
766+
def test_private_resource_not_allow_non_contributor_auth_user(self, mock_auth):
767+
from framework.auth.decorators import is_contributor_or_public_resource
768+
user = UserFactory()
769+
project = ProjectFactory(is_public=False)
770+
mock_auth.return_value = Auth(user=user)
771+
decorator = is_contributor_or_public_resource('resource')
772+
decorated_func = decorator(self.decorated_view)
773+
with decoratorapp.test_request_context():
774+
with pytest.raises(HTTPError) as excinfo:
775+
decorated_func(resource=project)
776+
assert excinfo.value.code == http_status.HTTP_403_FORBIDDEN
777+
725778

726779
def needs_addon_view(**kwargs):
727780
return 'openaddon'

website/views.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from flask import request, send_from_directory, Response, stream_with_context
1212

1313
from framework.auth import Auth
14-
from framework.auth.decorators import must_be_logged_in
14+
from framework.auth.decorators import must_be_logged_in, is_contributor_or_public_resource
1515
from framework.auth.forms import SignInForm, ForgotPasswordForm
1616
from framework.exceptions import HTTPError
1717
from framework.flask import redirect # VOL-aware redirect
@@ -297,7 +297,7 @@ def resolve_guid(guid, suffix=None):
297297
if clean_suffix == 'metadata':
298298
format_arg = request.args.get('format')
299299
if format_arg:
300-
return guid_metadata_download(guid, resource, format_arg)
300+
return guid_metadata_download(guid, resource=resource, metadata_format=format_arg)
301301
else:
302302
return use_ember_app()
303303

@@ -401,6 +401,7 @@ def get_storage_region_list(user, node=False):
401401
return available_regions
402402

403403

404+
@is_contributor_or_public_resource('resource')
404405
def guid_metadata_download(guid, resource, metadata_format):
405406
try:
406407
result = pls_gather_metadata_file(resource, metadata_format)
@@ -422,4 +423,4 @@ def guid_metadata_download(guid, resource, metadata_format):
422423
def metadata_download(guid):
423424
format_arg = request.args.get('format', 'datacite-json')
424425
resource = Guid.load(guid)
425-
return guid_metadata_download(guid, resource, format_arg)
426+
return guid_metadata_download(guid, resource=resource, metadata_format=format_arg)

0 commit comments

Comments
 (0)