Permalink
Browse files

Refine the authorization system by allowing limited public read acces…

…s. Closes #30.

Copying and pasting from the comments for authz.py, we find...

    There are 2 basic authorization scenarios:

    1) Public request: no user provided

    2) Registered request: user known and (later) authenticated

    In scenario 1, we allow the action if BOTH of the following criteria are
    satisfied, namely a) the annotation has a null or empty permissions field
    for that action, AND b) the action is in the PUBLIC_ACTIONS list.

    In scenario 2, we allow the action if EITHER of the following criteria are
    satisfied, namely a) the annotation has a null or empty permissions field
    for that action, OR b) the user is listed in the permissions field for
    that action.

In addition, this commit adds proper authorization checking to the search
APIs. As a guest user, you will only ever get annotations with null or empty
'read' permissions fields. As an authenticated user, you will get annotations
which have either null/empty 'read' fields, or 'read' fields in which you are
included.
  • Loading branch information...
1 parent e40411f commit ff9223ce45e7cf851717980f4d8bbe5c0d14409a @nickstenning nickstenning committed Feb 8, 2012
Showing with 147 additions and 74 deletions.
  1. +26 −6 annotator/authz.py
  2. +19 −7 annotator/model/annotation.py
  3. +71 −55 annotator/store.py
  4. +26 −1 tests/model/test_annotation.py
  5. +3 −3 tests/test_authz.py
  6. +2 −2 tests/test_store.py
View
@@ -1,9 +1,29 @@
+PUBLIC_ACTIONS = ['read']
+
+# There are 2 basic authorization scenarios:
+#
+# 1) Public request: no user provided
+#
+# 2) Registered request: user known and (later) authenticated
+#
+# In scenario 1, we allow the action if BOTH of the following criteria are
+# satisfied, namely a) the annotation has a null or empty permissions field
+# for that action, AND b) the action is in the PUBLIC_ACTIONS list.
+#
+# In scenario 2, we allow the action if EITHER of the following criteria are
+# satisfied, namely a) the annotation has a null or empty permissions field
+# for that action, OR b) the user is listed in the permissions field for that
+# action.
+
def authorize(annotation, action, user=None):
permissions = annotation.get('permissions', {})
- authorized_list = permissions.get(action, [])
- # no permissions or empty list indicates anyone can do that action
- if not authorized_list:
- return True
- else:
- return user in authorized_list
+ action_field = permissions.get(action, [])
+
+ if not user: # Scenario 1, as described above
+ return (not action_field) and (action in PUBLIC_ACTIONS)
+ else: # Scenario 2, as described above
+ if not action_field:
+ return True
+ else:
+ return user in action_field
@@ -48,23 +48,35 @@ def fetch(cls, id):
return Annotation(doc['_source'], id=id)
@classmethod
- def _build_query(cls, limit=20, **kwargs):
- q = {'match_all': {}}
+ def _build_query(cls, offset=0, limit=20, **kwargs):
+ f = {'and': []}
+ q = {'filtered': {'query': {'match_all': {}}, 'filter': f}}
- if kwargs:
- f = {'and': [{'term': {k: v}} for k, v in kwargs.iteritems()]}
- q = {'filtered': {'query': q, 'filter': f}}
+ uid = kwargs.pop('_user_id', None)
+
+ for k, v in kwargs.iteritems():
+ q['filtered']['filter']['and'].append({'term': {k: v}})
+
+ # Append permissions filter
+ uidq = {'missing': {'field': 'permissions.read'}}
+
+ if uid:
+ user_readable = {'term': {'permissions.read': uid}}
+ uidq = {'or': [uidq, user_readable]}
+
+ q['filtered']['filter']['and'].append(uidq)
return {
# Sort most recent first
'sort': [{'updated': {'order': 'desc'}}],
+ 'from': offset,
'size': limit,
'query': q
}
@classmethod
- def search(cls, limit=20, **kwargs):
- q = cls._build_query(limit, **kwargs)
+ def search(cls, **kwargs):
+ q = cls._build_query(**kwargs)
res = conn.search(q, index, TYPE)
docs = res['hits']['hits']
return [cls(d['_source'], id=d['_id']) for d in docs]
View
@@ -2,32 +2,16 @@
from flask import abort, redirect, request, g
from annotator.model import Annotation
-from annotator.authz import authorize
from annotator.util import jsonify
-from annotator.user import get_current_user
-from annotator import auth
+from annotator import auth, authz
__all__ = ["store"]
store = Blueprint('store', __name__)
-def get_current_userid():
+def current_user_id():
return auth.get_request_userid(request)
-@store.before_request
-def before_request():
- # Don't require authentication headers with OPTIONS request
- if request.method == 'OPTIONS':
- return
-
- # Don't require authentication headers for auth token request
- # (expecting a session cookie instead)
- if request.endpoint in ['store.root', 'store.auth_token']:
- return
-
- if not auth.verify_request(request):
- return jsonify("Cannot authorise request. Perhaps you didn't send the x-annotator headers?", status=401)
-
@store.after_request
def after_request(response):
ac = 'Access-Control-'
@@ -51,62 +35,72 @@ def root():
# INDEX
@store.route('/annotations')
def index():
- annotations = [anno for anno in Annotation.search() if authorize(anno, 'read', get_current_userid())]
+ uid = current_user_id()
+
+ if uid:
+ if not auth.verify_request(request):
+ return _failed_auth_response()
+ annotations = Annotation.search(_user_id=uid)
+ else:
+ annotations = Annotation.search()
+
return jsonify(annotations)
# CREATE
@store.route('/annotations', methods=['POST'])
def create_annotation():
- consumer_key = request.headers.get('x-annotator-consumer-key')
- user_id = request.headers.get('x-annotator-user-id')
+ # Only registered users can create annotations
+ if not auth.verify_request(request):
+ return _failed_auth_response()
if request.json:
annotation = Annotation(_filter_input(request.json))
- if consumer_key:
- annotation['consumer'] = consumer_key
- if user_id:
- annotation['user'] = user_id
+ annotation['consumer'] = request.headers[auth.HEADER_PREFIX + 'consumer-key']
+ annotation['user'] = request.headers[auth.HEADER_PREFIX + 'user-id']
annotation.save()
+
return jsonify(annotation)
else:
- return jsonify('No parameters given. Annotation not created.', status=400)
+ return jsonify('No JSON payload sent. Annotation not created.', status=400)
# READ
@store.route('/annotations/<id>')
def read_annotation(id):
annotation = Annotation.fetch(id)
-
if not annotation:
- return jsonify('Annotation not found.', status=404)
- elif authorize(annotation, 'read', get_current_userid()):
- return jsonify(annotation)
- else:
- return jsonify('Could not authorise request. Read not allowed', status=401)
+ return jsonify('Annotation not found!', status=404)
+
+ failure = _check_action(annotation, 'read', current_user_id())
+ if failure:
+ return failure
+
+ return jsonify(annotation)
# UPDATE
@store.route('/annotations/<id>', methods=['POST', 'PUT'])
def update_annotation(id):
annotation = Annotation.fetch(id)
-
if not annotation:
- return jsonify('Annotation not found. No update performed.', status=404)
+ return jsonify('Annotation not found! No update performed.', status=404)
+
+ failure = _check_action(annotation, 'update', current_user_id())
+ if failure:
+ return failure
- elif request.json and authorize(annotation, 'update', get_current_userid()):
+ if request.json:
updated = _filter_input(request.json)
- updated['id'] = id # use id from URL, regardless of what arrives in payload json
+ updated['id'] = id # use id from URL, regardless of what arrives in JSON payload
- if 'permissions' in updated and updated.get('permissions') != annotation.get('permissions', {}):
- if not authorize(annotation, 'admin', get_current_userid()):
- return jsonify('Could not authorise request (permissions change). No update performed', status=401)
+ if 'permissions' in updated and updated['permissions'] != annotation.get('permissions', {}):
+ if not authz.authorize(annotation, 'admin', current_user_id()):
+ return _failed_authz_response('permissions update')
annotation.update(updated)
annotation.save()
- return jsonify(annotation)
- else:
- return jsonify('Could not authorise request. No update performed', status=401)
+ return jsonify(annotation)
# DELETE
@store.route('/annotations/<id>', methods=['DELETE'])
@@ -116,32 +110,37 @@ def delete_annotation(id):
if not annotation:
return jsonify('Annotation not found. No delete performed.', status=404)
- elif authorize(annotation, 'delete', get_current_userid()):
- annotation.delete()
- return None, 204
+ failure = _check_action(annotation, 'delete', current_user_id())
+ if failure:
+ return failure
- else:
- return jsonify('Could not authorise request. No update performed', status=401)
+ annotation.delete()
+ return None, 204
# SEARCH
@store.route('/search')
def search_annotations():
kwargs = dict(request.args.items())
- results = [anno for anno in Annotation.search(**kwargs) if authorize(anno, 'read', get_current_userid())]
+ uid = current_user_id()
+
+ if uid:
+ if not auth.verify_request(request):
+ return _failed_auth_response()
+
+ kwargs['_user_id'] = uid
+
+ results = Annotation.search(**kwargs)
total = Annotation.count(**kwargs)
- qrows = {
+ return jsonify({
'total': total,
'rows': results,
- }
- return jsonify(qrows)
+ })
# AUTH TOKEN
@store.route('/token')
def auth_token():
- user = get_current_user()
-
- if user:
- return jsonify(auth.generate_token('annotateit', user.username))
+ if g.user:
+ return jsonify(auth.generate_token('annotateit', g.user.username))
else:
root = current_app.config['ROOT_URL']
return jsonify('Please go to {0} to log in!'.format(root), status=401)
@@ -152,3 +151,20 @@ def _filter_input(obj):
del obj[field]
return obj
+
+def _check_action(annotation, action, uid):
+ if not authz.authorize(annotation, action, uid):
+ return _failed_authz_response()
+
+ if uid and not auth.verify_request(request):
+ return _failed_auth_response()
+
+def _failed_authz_response(msg=''):
+ return jsonify("Cannot authorize request{0}. Perhaps you're not logged in as "
+ "a user with appropriate permissions on this annotation?".format(' (' + msg + ')'),
+ status=401)
+
+def _failed_auth_response():
+ return jsonify("Cannot authenticate request. Perhaps you didn't send the "
+ "X-Annotator-* headers?",
+ status=401)
@@ -56,7 +56,7 @@ def test_search(self):
anno2.save()
anno3.save()
- annotator.es.refresh()
+ annotator.es.refresh(timesleep=0.01)
res = Annotation.search()
h.assert_equal(len(res), 3)
@@ -90,3 +90,28 @@ def test_search(self):
res = Annotation.count(user=user1, uri=uri2)
h.assert_equal(res, 1)
+ def test_search_permissions(self):
+ anno = Annotation(text='Foobar', permissions={'read': []})
+ anno.save()
+
+ annotator.es.refresh(timesleep=0.01)
+
+ res = Annotation.search()
+ h.assert_equal(len(res), 1)
+
+ res = Annotation.search(_user_id='bob')
+ h.assert_equal(len(res), 1)
+
+ anno = Annotation(text='Foobar', permissions={'read': ['bob']})
+ anno.save()
+
+ annotator.es.refresh(timesleep=0.01)
+
+ res = Annotation.search()
+ h.assert_equal(len(res), 1)
+
+ res = Annotation.search(_user_id='alice')
+ h.assert_equal(len(res), 1)
+
+ res = Annotation.search(_user_id='bob')
+ h.assert_equal(len(res), 2)
View
@@ -15,7 +15,7 @@ def test_authorize_read_user(self):
def test_authorize_update_nouser(self):
ann = Annotation()
- assert authorize(ann, 'update')
+ assert not authorize(ann, 'update')
assert authorize(ann, 'update', 'bob')
def test_authorize_update_user(self):
@@ -25,7 +25,7 @@ def test_authorize_update_user(self):
def test_authorize_delete_nouser(self):
ann = Annotation()
- assert authorize(ann, 'delete')
+ assert not authorize(ann, 'delete')
assert authorize(ann, 'delete', 'bob')
def test_authorize_delete_user(self):
@@ -35,7 +35,7 @@ def test_authorize_delete_user(self):
def test_authorize_admin_nouser(self):
ann = Annotation()
- assert authorize(ann, 'admin')
+ assert not authorize(ann, 'admin')
assert authorize(ann, 'admin', 'bob')
def test_authorize_admin_user(self):
View
@@ -217,7 +217,7 @@ def test_search(self):
annoid = anno.id
anno2id = anno2.id
- annotator.es.refresh()
+ annotator.es.refresh(timesleep=0.01)
url = '/api/search'
res = self.app.get(url, headers=self.headers)
@@ -334,7 +334,7 @@ def test_update_change_permissions_not_allowed(self):
content_type='application/json',
headers=self.charlie_headers)
assert response.status_code == 401, "response should be 401 NOT AUTHORIZED"
- assert '(permissions change)' in response.data
+ assert 'permissions update' in response.data
response = self.app.put('/api/annotations/123',
data=payload,

0 comments on commit ff9223c

Please sign in to comment.