Skip to content

Commit

Permalink
Add place permissions check (#118)
Browse files Browse the repository at this point in the history
  • Loading branch information
goldpbear authored May 12, 2018
1 parent 192617e commit 47a204c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 30 deletions.
12 changes: 11 additions & 1 deletion src/sa_api_v2/models/data_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from .. import utils
from .core import CacheClearingModel
from .core import DataSet
from .core import Place
from .mixins import CloneableModelMixin

class DataPermissionManager (models.Manager):
Expand Down Expand Up @@ -155,7 +156,7 @@ def any_allow(permissions, do_action, submission_set, protected=False):
return True
return False

def check_data_permission(user, client, do_action, dataset, submission_set, protected=False):
def check_data_permission(user, client, place_id, do_action, dataset, submission_set, protected=False):
"""
Check whether the given user has permission on the submission_set in
the context of the given client (e.g., an API key or an origin). Specify
Expand Down Expand Up @@ -188,5 +189,14 @@ def check_data_permission(user, client, do_action, dataset, submission_set, prot
any_allow(group.permissions.all(), do_action, submission_set, protected)):
return True

# Finally, check place permissions
if place_id is not None and user is not None and user.is_authenticated:
target_place = Place.objects.get(id=place_id)
submitter = getattr(target_place, 'submitter', None)
place_submitter_id = getattr(submitter, 'id', None)
user_id = getattr(user, 'id', None)
if place_submitter_id == user_id:
return True

return False

8 changes: 5 additions & 3 deletions src/sa_api_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ def to_native(self, obj):
user = getattr(request, 'user', None)
client = getattr(request, 'client', None)
dataset = obj
if not check_data_permission(user, client, 'retrieve', dataset, set_name):
if not check_data_permission(user, client, None, 'retrieve', dataset, set_name):
continue

obj.submission_set_name = set_name
Expand Down Expand Up @@ -753,7 +753,8 @@ def get_submission_set_summaries(self, place):
user = getattr(request, 'user', None)
client = getattr(request, 'client', None)
dataset = getattr(request, 'get_dataset', lambda: None)()
if not check_data_permission(user, client, 'retrieve', dataset, set_name):

if not check_data_permission(user, client, None, 'retrieve', dataset, set_name):
continue

summaries[set_name] = self.summary_to_native(set_name, submissions)
Expand All @@ -779,7 +780,8 @@ def get_detailed_submission_sets(self, place):
user = getattr(request, 'user', None)
client = getattr(request, 'client', None)
dataset = getattr(request, 'get_dataset', lambda: None)()
if not check_data_permission(user, client, 'retrieve', dataset, set_name):

if not check_data_permission(user, client, None, 'retrieve', dataset, set_name):
continue

# We know that the submission datasets will be the same as the place
Expand Down
71 changes: 46 additions & 25 deletions src/sa_api_v2/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,33 +401,35 @@ def test_default_dataset_permissions_allow_reading(self):
user = User.objects.create(username='myuser')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)')
place_id = place.id

# Make sure a permission objects were created
self.assertEqual(dataset.permissions.count(), 1)

# Make sure anonymous is allowed to read, not write.
self.assertEqual(check_data_permission(None, None, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(None, None, 'update', dataset, 'comments'), False)
self.assertEqual(check_data_permission(None, None, 'create', dataset, 'comments'), False)
self.assertEqual(check_data_permission(None, None, 'destroy', dataset, 'comments'), False)
self.assertEqual(check_data_permission(None, None, place_id, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(None, None, place_id, 'update', dataset, 'comments'), False)
self.assertEqual(check_data_permission(None, None, place_id, 'create', dataset, 'comments'), False)
self.assertEqual(check_data_permission(None, None, place_id, 'destroy', dataset, 'comments'), False)

# Make sure authenticated is allowed to read.
self.assertEqual(check_data_permission(user, None, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(user, None, 'update', dataset, 'comments'), False)
self.assertEqual(check_data_permission(user, None, 'create', dataset, 'comments'), False)
self.assertEqual(check_data_permission(user, None, 'destroy', dataset, 'comments'), False)
self.assertEqual(check_data_permission(user, None, place_id, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(user, None, place_id, 'update', dataset, 'comments'), False)
self.assertEqual(check_data_permission(user, None, place_id, 'create', dataset, 'comments'), False)
self.assertEqual(check_data_permission(user, None, place_id, 'destroy', dataset, 'comments'), False)

# Make sure owner is allowed to read.
self.assertEqual(check_data_permission(owner, None, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, 'update', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, 'create', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, 'destroy', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, place_id, 'retrieve', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, place_id, 'update', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, place_id, 'create', dataset, 'comments'), True)
self.assertEqual(check_data_permission(owner, None, place_id, 'destroy', dataset, 'comments'), True)

def test_dataset_permissions_can_restrict_reading(self):
owner = User.objects.create(username='myowner')
user = User.objects.create(username='myuser')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)')
place_id = place.id

# Make sure a permission objects were created
self.assertEqual(dataset.permissions.count(), 1)
Expand All @@ -438,22 +440,23 @@ def test_dataset_permissions_can_restrict_reading(self):
perm.save()

# Make sure anonymous is not allowed to read.
has_permission = check_data_permission(None, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(None, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, False)

# Make sure authenticated is not allowed to read.
has_permission = check_data_permission(user, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(user, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, False)

# Make sure owner is allowed to read.
has_permission = check_data_permission(owner, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(owner, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, True)

def test_specific_dataset_permissions_can_allow_or_restrict_reading(self):
owner = User.objects.create(username='myowner')
user = User.objects.create(username='myuser')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)')
place_id = place.id

# Make sure a permission objects were created
self.assertEqual(dataset.permissions.count(), 1)
Expand All @@ -468,31 +471,32 @@ def test_specific_dataset_permissions_can_allow_or_restrict_reading(self):
dataset.permissions.add(places_perm)

# Make sure anonymous can read comments, but not places.
has_permission = check_data_permission(None, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(None, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, True)

has_permission = check_data_permission(None, None, 'retrieve', dataset, 'places')
has_permission = check_data_permission(None, None, place_id, 'retrieve', dataset, 'places')
self.assertEqual(has_permission, False)

# Make sure authenticated can read comments, but not places.
has_permission = check_data_permission(user, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(user, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, True)

has_permission = check_data_permission(user, None, 'retrieve', dataset, 'places')
has_permission = check_data_permission(user, None, place_id, 'retrieve', dataset, 'places')
self.assertEqual(has_permission, False)

# Make sure owner is allowed to read everything.
has_permission = check_data_permission(owner, None, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(owner, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, True)

has_permission = check_data_permission(owner, None, 'retrieve', dataset, 'places')
has_permission = check_data_permission(owner, None, place_id, 'retrieve', dataset, 'places')
self.assertEqual(has_permission, True)

def test_group_permissions_can_restrict_reading(self):
owner = User.objects.create(username='myowner')
user = User.objects.create(username='myuser')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)')
place_id = place.id

# Create a key for the dataset
key = ApiKey.objects.create(key='abc', dataset=dataset)
Expand All @@ -510,24 +514,41 @@ def test_group_permissions_can_restrict_reading(self):
permission.save()

# Make sure we're not allowed to read.
has_permission = check_data_permission(user, key, 'retrieve', dataset, 'comments')
has_permission = check_data_permission(user, key, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(has_permission, False)

def test_fails_when_requesting_an_unknown_permission(self):
user = client = dataset = submission_set = None
user = place_id = client = dataset = submission_set = None
with self.assertRaises(ValueError):
check_data_permission(user, client, 'obliterate', dataset, submission_set)
check_data_permission(user, client, place_id, 'obliterate', dataset, submission_set)

def test_accepts_submission_set_name(self):
owner = User.objects.create(username='myowner')
user = User.objects.create(username='myuser')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)')
place_id = place.id

with patch('sa_api_v2.models.data_permissions.any_allow') as any_allow:
check_data_permission(user, None, 'retrieve', dataset, 'comments')
check_data_permission(user, None, place_id, 'retrieve', dataset, 'comments')
self.assertEqual(any_allow.call_args[0][2], 'comments')

def test_place_permissions_allow_all_actions_by_submitter(self):
owner = User.objects.create(username='myowner')
submitter = User.objects.create(username='mysubmitter')
dataset = DataSet.objects.create(slug='data', owner_id=owner.id)
place = Place.objects.create(dataset_id=dataset.id, geometry='POINT(0 0)', submitter_id=submitter.id)
place_id = place.id

# Get rid of the dataset permissions
dataset.permissions.all().delete()

# Place permissions should allow all operations by the submitter
self.assertEqual(check_data_permission(submitter, None, place_id, 'retrieve', dataset, 'places'), True)
self.assertEqual(check_data_permission(submitter, None, place_id, 'update', dataset, 'places'), True)
self.assertEqual(check_data_permission(submitter, None, place_id, 'create', dataset, 'places'), True)
self.assertEqual(check_data_permission(submitter, None, place_id, 'destroy', dataset, 'places'), True)


# More permissions tests to write:
# - General client permission allows reading and restricts writing
Expand Down
5 changes: 4 additions & 1 deletion src/sa_api_v2/views/base_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,11 @@ def has_permission(self, request, view):
user = getattr(request, 'user', None)
client = getattr(request, 'client', None)
dataset = getattr(request, 'get_dataset', lambda: None)()
place_id = None
if 'id' in request.DATA:
place_id = request.DATA['id']

return models.check_data_permission(user, client, do_action, dataset, data_type, protected)
return models.check_data_permission(user, client, place_id, do_action, dataset, data_type, protected)


###############################################################################
Expand Down

0 comments on commit 47a204c

Please sign in to comment.