From e8a234da113dff1c0e1022e46b3a1b1e51f725a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= Date: Mon, 10 May 2021 13:47:51 +0200 Subject: [PATCH] api: allow mixing anonymous restriction and basic authentication (#53883) --- tests/api/test_formdata.py | 59 ++++++++++++++++++++++++++------------ wcs/api.py | 24 +++++++++------- wcs/api_access.py | 5 +++- wcs/forms/common.py | 5 ++-- wcs/qommon/http_request.py | 5 ++++ 5 files changed, 64 insertions(+), 34 deletions(-) diff --git a/tests/api/test_formdata.py b/tests/api/test_formdata.py index bef2e18ca..905c8f35c 100644 --- a/tests/api/test_formdata.py +++ b/tests/api/test_formdata.py @@ -58,6 +58,8 @@ coucou = 1234 ''' ) + pub.user_class.wipe() + return pub @@ -67,7 +69,6 @@ def teardown_module(module): @pytest.fixture def local_user(): - get_publisher().user_class.wipe() user = get_publisher().user_class() user.name = 'Jean Darmette' user.email = 'jean.darmette@triffouilis.fr' @@ -78,7 +79,6 @@ def local_user(): @pytest.fixture def admin_user(): - get_publisher().user_class.wipe() user = get_publisher().user_class() user.name = 'John Doe Admin' user.email = 'john.doe@example.com' @@ -788,7 +788,8 @@ def test_api_anonymized_formdata(pub, local_user, admin_user): assert 'name' in resp.json['evolution'][1]['who'] -def test_api_access_restrict_to_anonymised_data(pub, local_user): +@pytest.mark.parametrize('http_basic_auth', [False, True]) +def test_api_access_restrict_to_anonymised_data(pub, local_user, http_basic_auth): pub.role_class.wipe() role = pub.role_class(name='test') role.store() @@ -823,36 +824,56 @@ def test_api_access_restrict_to_anonymised_data(pub, local_user): access.access_key = '12345' access.store() - resp = get_app(pub).get( - sign_uri( - '/api/forms/test/list?full=on', - user=local_user, - orig=access.access_identifier, - key=access.access_key, - ) - ) + app = get_app(pub) + + if http_basic_auth: + # there's not "defaults to admin" permissions in case of basic authentication. + access.roles = [role] + access.store() + + def get_url(url, **kwargs): + app.set_authorization(('Basic', ('test', '12345'))) + return app.get(url, **kwargs) + + else: + + def get_url(url, **kwargs): + return app.get( + sign_uri(url, user=local_user, orig=access.access_identifier, key=access.access_key), **kwargs + ) + + resp = get_url('/api/forms/test/list?full=on') assert len(resp.json) == 10 assert resp.json[0]['fields']['foobar'] == 'FOO BAR1' assert resp.json[0]['fields']['foobar2'] == 'FOO BAR 2' assert resp.json[0].get('user') + # get a single formdata + resp = get_url('/api/forms/test/%s/' % formdata.id) + assert 'user' in resp.json + # restrict API access to anonymised data access.restrict_to_anonymised_data = True access.store() - resp = get_app(pub).get( - sign_uri( - '/api/forms/test/list?full=on', - user=local_user, - orig=access.access_identifier, - key=access.access_key, - ) - ) + resp = get_url('/api/forms/test/list?full=on') assert len(resp.json) == 10 assert 'foobar' not in resp.json[0]['fields'] assert resp.json[0]['fields']['foobar2'] == 'FOO BAR 2' assert not resp.json[0].get('user') + # get a single formdata + resp = get_url('/api/forms/test/%s/' % formdata.id) + assert 'user' not in resp.json + + if http_basic_auth: + # for basic HTTP authentication, check there's no access if roles are not given. + access.roles = [] + access.store() + + get_url('/api/forms/test/list?full=on', status=403) + get_url('/api/forms/test/%s/' % formdata.id, status=403) + def test_api_geojson_formdata(pub, local_user): pub.role_class.wipe() diff --git a/wcs/api.py b/wcs/api.py index b63f41ff4..c5710673f 100644 --- a/wcs/api.py +++ b/wcs/api.py @@ -189,17 +189,19 @@ class ApiFormPageMixin: raise TraversalError() def check_access(self, api_name=None): - if get_request().has_anonymised_data_api_restriction(): - if not is_url_signed() or (get_request().user and get_request().user.is_admin): - raise AccessForbiddenError('user not authenticated') - else: - if get_request().user and get_request().user.is_admin: - return # grant access to admins, to ease debug - api_user = get_user_from_api_query_string(api_name=api_name) - if not api_user: - raise AccessForbiddenError('user not authenticated') - if not self.formdef.is_of_concern_for_user(api_user): - raise AccessForbiddenError('unsufficient roles') + if get_request().user and get_request().user.is_admin: + return # grant access to admins, to ease debug + + if get_request().has_anonymised_data_api_restriction() and is_url_signed(): + # when requesting anonymous data, a signature is enough + return + + api_user = get_user_from_api_query_string(api_name=api_name) + + if not api_user: + raise AccessForbiddenError('user not authenticated') + if not self.formdef.is_of_concern_for_user(api_user): + raise AccessForbiddenError('unsufficient roles') def _q_lookup(self, component): if component == 'ics': diff --git a/wcs/api_access.py b/wcs/api_access.py index b3d22662d..c74cd597f 100644 --- a/wcs/api_access.py +++ b/wcs/api_access.py @@ -89,6 +89,9 @@ class ApiAccess(XmlStorableObject): is_api_user = True anonymous = False + def __init__(self, api_access): + self.api_access = api_access + def can_go_in_admin(self): return False @@ -98,7 +101,7 @@ class ApiAccess(XmlStorableObject): def get_roles(self): return self.roles - user = RestrictedApiUser() + user = RestrictedApiUser(self) user.roles = [x.id for x in self.get_roles()] return user diff --git a/wcs/forms/common.py b/wcs/forms/common.py index b9925590b..77ebeffb8 100644 --- a/wcs/forms/common.py +++ b/wcs/forms/common.py @@ -158,13 +158,12 @@ class FormStatusPage(Directory, FormTemplateMixin): session = get_session() mine = False if api_call: - if get_request().has_anonymised_data_api_restriction(): + user = get_user_from_api_query_string() or get_request().user + if get_request().has_anonymised_data_api_restriction() and (not user or not user.is_api_user): if is_url_signed() or (get_request().user and get_request().user.is_admin): return None else: raise errors.AccessUnauthorizedError() - else: - user = get_user_from_api_query_string() or get_request().user else: user = get_request().user if user and not user.anonymous: diff --git a/wcs/qommon/http_request.py b/wcs/qommon/http_request.py index e22d129cb..ad99764a2 100644 --- a/wcs/qommon/http_request.py +++ b/wcs/qommon/http_request.py @@ -223,11 +223,16 @@ class HTTPRequest(quixote.http_request.HTTPRequest): if 'anonymise' in self.form: return True + orig = self.form.get('orig') if orig: api_access = ApiAccess.get_by_identifier(orig) if api_access: return api_access.restrict_to_anonymised_data + + if self.user and self.user.is_api_user: + return self.user.api_access.restrict_to_anonymised_data + return False @property