hypothesis / hypothesis/h

unexpected/undocumented behavior from `PATCH /annotations/{id}` for group and permissions

Open
#4,705 1 comment 1 reaction 0 assignees View on GitHub
API bug
Dominant language
Python
Stars
3.2k
Forks
459
Avg merge
27d 1h
Merged PRs (30d)
1

Description

### Steps to reproduce
1. Issue a PATCH request changing the group and read permissions for an annotation following the [api docs](https://h.readthedocs.io/en/latest/api-reference/#operation/updateAnnotation). For example if I want to move an annotation from a private group to `__world__`:
```
{'group': '__world__',
'permissions': {'admin': ['acct:scibot@hypothes.is'],
'delete': ['acct:scibot@hypothes.is'],
'read': ['group:__world__'],
'update': ['acct:scibot@hypothes.is']}}
```

### Expected behaviour
The annotation group and permissions should appear as above and be visible in the client to those with access to the specified group and read permissions.
Alternately, if the fields cannot be change the API should return a 400 error and no changes should be made to the annotation.

### Actual behaviour
A 200 is returned. The annotation is not moved and is in fact made private.
```
{'group': '{original_private_group}',
'permissions': {'admin': ['acct:scibot@hypothes.is'],
'delete': ['acct:scibot@hypothes.is'],
'read': ['acct:scibot@hypothes.is'],
'update': ['acct:scibot@hypothes.is']}}
```

### Browser/system information
N/A. [The code in question](https://github.com/SciCrunch/scibot/blob/master/release.py#L464-L491).

### Additional details
@robertknight pointed me to this line as the cause https://github.com/hypothesis/h/blob/34438d3d48ddb26c8bae370ba1d7c5b9c69ffea0/h/schemas/annotation.py#L198-L200

The very best solution to this would be to be able to include `__world__` in the read permissions without having to also change the group as well. That way 'releasing to public' (or any other group) is merely a matter of adding that group to the read permissions. Not entirely sure how the current implementation would handle having replies to a single annotation in multiple groups.

Our use case is to be able to POST a large number of annotations into a private staging group where we (@bandrow @memartone) can check them for errors and then simply 'flip a switch' to make all reviewed annotations public by moving them to `__world__` without having to worry about things like https://github.com/hypothesis/h/issues/4704 or some other issue with a change to the POSTing code.

I think one original concern related to implementing this functionality was that moving annotations from one group to another could orphan replies, but this routinely happens in our use due to deletes and the current behavior of making the annotation private also orphans replies (maybe a bug in itself?). I don't know the internals of the system so I don't know how moving an annotation to a group that the reply doesn't have access to interacts with permissions and security as compared to a delete where there is no possibility of leaking access. (There seems to be a longer conversation here about cross-group replies, but it is probably out of scope for this issue.)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.