Skip to content

Commit

Permalink
Don't crash if no permissions are present
Browse files Browse the repository at this point in the history
We can't always guarantee the permissions field is present, as our API
does not currently enforce this. Don't crash in this case.
  • Loading branch information
nickstenning committed Sep 25, 2015
1 parent f61da6d commit d0cebd4
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
8 changes: 7 additions & 1 deletion h/api/groups/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@

def set_permissions(annotation):
"""Set the given annotation's permissions according to its group."""
# If this annotation doesn't have a permissions field, we don't know how to
# handle it and should bail.
permissions = annotation.get('permissions')
if permissions is None:
return

# For private annotations (visible only to the user who created them) the
# client sends just the user's ID in the read permissions.
is_private = (annotation['permissions']['read'] == [annotation['user']])
is_private = (permissions.get('read') == [annotation['user']])

if is_private:
# The groups feature doesn't change the permissions for private
Expand Down
16 changes: 16 additions & 0 deletions h/api/groups/test/auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ def _mock_group(hashid):
return mock.Mock(hashid=hashid)


def test_set_permissions_does_not_modify_annotations_with_no_permissions():
annotations = [{
'user': 'acct:jack@hypothes.is',
},
{
'user': 'acct:jack@hypothes.is',
'group': 'xyzabc',
}]

for ann in annotations:
before = copy.deepcopy(ann)
auth.set_permissions(ann)

assert ann == before


def test_set_permissions_does_not_modify_private_annotations():
original_annotation = {
'user': 'acct:jack@hypothes.is',
Expand Down

0 comments on commit d0cebd4

Please sign in to comment.