From 2528e8a5c3895ccd1e06a413c158d1f68735c9a1 Mon Sep 17 00:00:00 2001
From: Renzo Frigato <rfrigato@stanford.edu>
Date: Thu, 3 Sep 2015 11:55:36 -0700
Subject: [PATCH] create storage and permissions checkers

introduce dao layer to handle db interactions for lists
use permissions checkers to validate queries
---
 api/api.py            |  6 ++-
 api/dao/grouproles.py | 31 --------------
 api/dao/notes.py      | 44 --------------------
 api/dao/storage.py    | 92 +++++++++++++++++++++++++++++++++--------
 api/listhandler.py    | 96 ++++++++++++++++++++++---------------------
 api/permchecker.py    |  8 +++-
 6 files changed, 137 insertions(+), 140 deletions(-)
 delete mode 100644 api/dao/grouproles.py
 delete mode 100644 api/dao/notes.py

diff --git a/api/api.py b/api/api.py
index ca03a7fa..74449a29 100644
--- a/api/api.py
+++ b/api/api.py
@@ -16,6 +16,7 @@ from . import acquisitions
 from . import collections
 from . import listhandler
 from . import permchecker
+from dao import storage
 
 
 routes = [
@@ -46,7 +47,8 @@ routes = [
         webapp2.Route(r'/<:[^/]+>',                                 users.Group, name='group'),
         webapp2.Route(r'/<gid:[^/]+>/projects',                     projects.Projects, name='g_projects'),
         webapp2.Route(r'/<gid:[^/]+>/sessions',                     sessions.Sessions, name='g_sessions', methods=['GET']),
-        webapp2.Route(r'/<cid:[^/]+>/<list:roles>/<_id:[^/]+>',     listhandler.ListHandler, name='g_roles', defaults={'collection': 'groups', 'permchecker': permchecker.default_sublist}),
+        webapp2.Route(r'/<cid:[^/]+>/roles/<_id:[^/]+>',     listhandler.ListHandler, name='g_roles',
+            defaults={'permchecker': permchecker.group_roles_sublist, 'storage': storage.ObjectListStorage('groups', 'roles')}),
     ]),
     webapp2.Route(r'/api/projects',                                 projects.Projects, methods=['GET'], name='projects'),
     webapp2_extras.routes.PathPrefixRoute(r'/api/projects', [
@@ -84,6 +86,8 @@ routes = [
         webapp2.Route(r'/<:[0-9a-f]{24}>',                          acquisitions.Acquisition, name='acquisition'),
         webapp2.Route(r'/<:[0-9a-f]{24}>/file',                     acquisitions.Acquisition, handler_method='file', methods=['POST']),
         webapp2.Route(r'/<:[0-9a-f]{24}>/file/<:[^/]+>',            acquisitions.Acquisition, handler_method='file'),
+        webapp2.Route(r'/<cid:[^/]+>/roles/<_id:[^/]+>',     listhandler.ListHandler, name='aq_tags',
+            defaults={'permchecker': permchecker.default_sublist, 'storage': storage.StringListStorage('acquisitions', 'tags')}),
     ]),
     webapp2.Route(r'/api/jobs',                                     jobs.Jobs),
     webapp2_extras.routes.PathPrefixRoute(r'/api/jobs', [
diff --git a/api/dao/grouproles.py b/api/dao/grouproles.py
deleted file mode 100644
index 5d17b95b..00000000
--- a/api/dao/grouproles.py
+++ /dev/null
@@ -1,31 +0,0 @@
-import storage
-import copy
-
-class GroupRolesStorage(object):
-
-    def init(self, dbc):
-        self.dbc = dbc
-
-    def get_container(self, _id):
-        return super(GroupRolesStorage, storage.Storage).get_container(self, _id)
-
-    def store_change(self, action, _id, elem_match=None, payload=None):
-        if action == 'DELETE':
-            return self._delete_role(_id, elem_match)
-        if action == 'PUT':
-            return self._update_role(_id, elem_match, payload)
-        if action == 'POST':
-            return self._create_role(_id, payload)
-        raise ValueError('action should be one of POST, PUT, DELETE')
-
-    def _create_role(self, _id, payload):
-        return super(GroupRolesStorage, self)._delete_el(_id, 'roles', payload)
-
-    def _delete_role(self, _id, elem_match):
-        return super(GroupRolesStorage, self)._delete_el(_id, 'roles', elem_match)
-
-    def _update_role(self, _id, elem_match, payload):
-        return super(GroupRolesStorage, self)._update_el(_id, 'roles', elem_match, payload)
-
-
-storage.Storage.register(GroupRolesStorage)
\ No newline at end of file
diff --git a/api/dao/notes.py b/api/dao/notes.py
deleted file mode 100644
index 9465ab29..00000000
--- a/api/dao/notes.py
+++ /dev/null
@@ -1,44 +0,0 @@
-import storage
-
-class NotesStorage(object):
-
-    def init(self, dbc):
-        self.dbc = dbc
-
-    def get_container(self, _id):
-        return super(NotesStorage, storage.Storage).get_container(self, _id)
-
-    def store_change(self, action, **kwargs):
-        if action == 'DELETE':
-            return self._delete_note(**kwargs)
-        if action == 'PUT':
-            return self._update_note(**kwargs)
-        if action == 'POST':
-            return self._create_note(**kwargs)
-        raise ValueError('action should be one of POST, PUT, DELETE')
-
-    def _create_note(self, _id=None, **kwargs):
-        _id = _id or bson.objectid.ObjectId()
-        query = {'_id': _id, 'notes': {'$not': {'$elemMatch': kwargs} } }
-        update = {'$push': {'notes': payload} }
-        return dbc.update_one(query, update)
-
-    def _delete_note(self, _id, **kwargs):
-        query = {'_id': _id}
-        update = {'$pull': {'notes': payload} }
-        return dbc.update_one(query, update)
-
-
-    def _update_note(self, _id, **kwargs):
-        for k,v in kwargs.items():
-            mod_payload['notes.$.' + k] = v
-
-        query = {'_id': _id, 'notes': {'$elemMatch': kwargs} }
-
-        update = {
-            '$set': mod_payload
-        }
-
-        return dbc.update_one(query, update)
-
-storage.Storage.register(NotesStorage)
\ No newline at end of file
diff --git a/api/dao/storage.py b/api/dao/storage.py
index e9e8191b..f717a96b 100644
--- a/api/dao/storage.py
+++ b/api/dao/storage.py
@@ -1,42 +1,102 @@
+# @author:  Renzo Frigato
+
 from abc import ABCMeta, abstractmethod
+import bson.objectid
+import copy
+import logging
+log = logging.getLogger('scitran.api')
 
-class AbstractStorage:
+class ListStorage:
     __metaclass__ = ABCMeta
 
-    @abstractmethod
-    def get_container(self, _id):
-        return self.dbc.find_one(_id)
+    def __init__(self, coll_name, list_name):
+        self.coll_name = coll_name
+        self.list_name = list_name
+        self.dbc = None
+
+
+    def load_collection(self, db):
+        self.dbc = db.get_collection(self.coll_name)
+        return self.dbc
+
+    def get_container(self, _id, elem_match = None):
+        if self.dbc is None:
+            raise RuntimeError('collection not initialized before calling get_container')
+        query = {'_id': _id}
+        if elem_match is not None:
+            query[self.list_name] = {'$elemMatch': elem_match}
+        log.error('query {}'.format(query))
+        return self.dbc.find_one(query)
 
     @abstractmethod
-    def store_change(self, action, list_, elem_match=None, payload=None):
+    def apply_change(self, action, _id, elem_match=None, payload=None):
         pass
 
-    def _create_el(self, _id, list_, payload):
-        if !isinstance(payload, str):
+    def _create_el(self, _id, payload):
+        log.debug('payload {}'.format(payload))
+        if isinstance(payload, dict) and payload.get('_id') is None:
             elem_match = copy.deepcopy(payload)
             payload['_id'] = bson.objectid.ObjectId()
         else:
              elem_match = payload
-        query = {'_id': _id, list_: {'$not': {'$elemMatch': elem_match} } }
-        update = {'$push': {list_: payload} }
-        return dbc.update_one(query, update)
+        query = {'_id': _id, self.list_name: {'$not': {'$elemMatch': elem_match} } }
+        update = {'$push': {self.list_name: payload} }
+        log.debug('query {}'.format(query))
+        log.debug('update {}'.format(update))
+        return self.dbc.update_one(query, update)
 
-    def _update_el(self, _id, list_, elem_match, payload):
+    def _update_el(self, _id, elem_match, payload):
+        log.debug('elem_match {}'.format(payload))
+        log.debug('payload {}'.format(elem_match))
         if isinstance(payload, str):
             mod_elem = {
-                list_ + '.$': v
+                self.list_name + '.$': v
             }
         else:
             mod_elem = {}
             for k,v in payload.items():
-                mod_elem[list_ + '.$.' + k] = v
-        query = {'_id': _id, 'notes': {'$elemMatch': elem_match} }
+                mod_elem[self.list_name + '.$.' + k] = v
+        query = {'_id': _id, self.list_name: {'$elemMatch': elem_match} }
         update = {
             '$set': mod_elem
         }
+        log.debug('query {}'.format(query))
+        log.debug('update {}'.format(update))
         return self.dbc.update_one(query, update)
 
-    def _delete_el(self, _id, list_, elem_match):
+    def _delete_el(self, _id, elem_match):
+        log.debug('elem_match {}'.format(payload))
         query = {'_id': _id}
-        update = {'$pull': {list_: elem_match} }
+        update = {'$pull': {self.list_name: elem_match} }
+        log.debug('query {}'.format(query))
+        log.debug('update {}'.format(update))
         return self.dbc.update_one(query, update)
+
+
+class ObjectListStorage(ListStorage):
+
+    def apply_change(self, action, _id, elem_match=None, payload=None):
+        if action == 'DELETE':
+            return super(ObjectListStorage, self)._delete_el(_id, elem_match)
+        if action == 'PUT':
+            return super(ObjectListStorage, self)._update_el(_id, elem_match, payload)
+        if action == 'POST':
+            return super(ObjectListStorage, self)._create_el(_id, payload)
+        raise ValueError('action should be one of POST, PUT, DELETE')
+
+
+
+class StringListStorage(ListStorage):
+
+    def __init__(self, coll_name, list_name, key_name):
+        super(StringListStorage, self).__init__(coll_name, list_name)
+        self.key_name = key_name
+
+    def apply_change(self, action, _id, elem_match=None, payload=None):
+        if action == 'DELETE':
+            return super(StringListStorage, self)._delete_el(_id, elem_match[self.key_name])
+        if action == 'PUT':
+            return super(StringListStorage, self)._update_el(_id, elem_match[self.key_name], payload[self.key_name])
+        if action == 'POST':
+            return super(StringListStorage, self)._create_el(_id, payload[self.key_name])
+        raise ValueError('action should be one of POST, PUT, DELETE')
diff --git a/api/listhandler.py b/api/listhandler.py
index 581629ec..a35f9039 100644
--- a/api/listhandler.py
+++ b/api/listhandler.py
@@ -3,85 +3,89 @@
 import logging
 import base
 import json
-log = logging.getLogger('scitran.api.listhandler')
+log = logging.getLogger('scitran.api')
 
 class ListHandler(base.RequestHandler):
 
     def __init__(self, request=None, response=None):
         super(ListHandler, self).__init__(request, response)
-        self.permissions = None
 
     def get(self, *args, **kwargs):
-        #permchecker = kwargs.pop('permchecker')
-        collection = kwargs.pop('collection')
-        list_ = kwargs.pop('list')
+        permchecker = kwargs.pop('permchecker')
+        storage = kwargs.pop('storage')
+        list_name = storage.list_name
         _id = kwargs.pop('cid', None) or kwargs.pop('_id')
-        query = {'_id': _id}
-        projection = {}
-        query[list_] = projection[list_] = {'$elemMatch': kwargs}
-
-        dbc = self.app.db.get_collection(collection)
-        result = dbc.find_one(query)
-
-        if result is not None:
-            return result[list_][0]
+        storage.load_collection(self.app.db)
+        container = storage.get_container(_id, elem_match = kwargs)
+        if container is not None:
+            if not permchecker(container, 'GET', self.uid):
+                self.abort(403, 'user not authorized to get container')
+            return container[list_name][0]
         else:
-            self.abort(404, 'Element not found in list {} of collection {}'.format(list_, collection))
+            self.abort(404, 'Element not found in list {} of collection {} {}'.format(list_name, storage.coll_name, _id))
 
     def post(self, *args, **kwargs):
-        collection = kwargs.pop('collection')
-        list_ = kwargs.pop('list')
+        permchecker = kwargs.pop('permchecker')
+        storage = kwargs.pop('storage')
+        list_name = storage.list_name
         _id = kwargs.pop('cid', None) or kwargs.pop('_id')
 
+        storage.load_collection(self.app.db)
+        container = storage.get_container(_id)
+        if container is not None:
+            if not permchecker(container, 'POST', self.uid):
+                self.abort(403, 'user not authorized to create element in list')
+        else:
+            self.abort(404, 'Element not found in list {} of collection {} {}'.format(list_name, storage.coll_name, _id))
         payload = self.request.POST.mixed()
         payload.update(kwargs)
-        query = {'_id': _id, list_: {'$not': {'$elemMatch': kwargs} } }
-        update = {'$push': {list_: payload} }
-
-        dbc = self.app.db.get_collection(collection)
-        result = dbc.update_one(query, update)
+        result = storage.apply_change('POST', _id, payload=payload)
 
         if result.modified_count == 1:
             return {'modified':result.modified_count}
         else:
-            self.abort(404, 'Element not added in list {} of collection {}'.format(list_, collection))
+            self.abort(404, 'Element not added in list {} of collection {} {}'.format(list_name, storage.coll_name, _id))
 
     def put(self, *args, **kwargs):
-        collection = kwargs.pop('collection')
-        list_ = kwargs.pop('list')
+        method = 'PUT'
+        permchecker = kwargs.pop('permchecker')
+        storage = kwargs.pop('storage')
+        list_name = storage.list_name
         _id = kwargs.pop('cid', None) or kwargs.pop('_id')
-        payload = self.request.POST.mixed()
-        mod_payload = {}
-        for k,v in payload.items():
-            mod_payload[list_ + '.$.' + k] = v
-
-        query = {'_id': _id, list_: {'$elemMatch': kwargs} }
 
-        update = {
-            '$set': mod_payload
-        }
+        storage.load_collection(self.app.db)
+        container = storage.get_container(_id)
+        if container is not None:
+            if not permchecker(container, method, self.uid):
+                self.abort(403, 'user not authorized to update element in list')
+        else:
+            self.abort(404, 'Element not found in list {} of collection {}'.format(list_name, storage.coll_name))
 
-        dbc = self.app.db.get_collection(collection)
-        result = dbc.update_one(query, update)
+        result = storage.apply_change(method, _id, elem_match = kwargs, payload = self.request.POST.mixed())
 
         if result.modified_count == 1:
             return {'modified':result.modified_count}
         else:
-            self.abort(404, 'Element not added in list {} of collection {}'.format(list_, collection))
+            self.abort(404, 'Element not updated in list {} of collection {} {}'.format(list_name, storage.coll_name, _id))
 
     def delete(self, *args, **kwargs):
-        collection = kwargs.pop('collection')
-        list_ = kwargs.pop('list')
+        method = 'DELETE'
+        permchecker = kwargs.pop('permchecker')
+        storage = kwargs.pop('storage')
+        list_name = storage.list_name
         _id = kwargs.pop('cid', None) or kwargs.pop('_id')
-        payload = self.request.POST.mixed()
-        payload.update(kwargs)
-        query = {'_id': _id}
-        update = {'$pull': {list_: payload} }
 
-        dbc = self.app.db.get_collection(collection)
-        result = dbc.update_one(query, update)
+        storage.load_collection(self.app.db)
+        container = storage.get_container(_id)
+        if container is not None:
+            if not permchecker(container, method, self.uid):
+                self.abort(403, 'user not authorized to delete element from list')
+        else:
+            self.abort(404, 'Element not found in list {} of collection {}'.format(list_name, storage.coll_name))
+
+        result = storage.apply_change(method, _id, elem_match = kwargs)
 
         if result.modified_count == 1:
             return {'modified':result.modified_count}
         else:
-            self.abort(404, 'Element not added in list {} of collection {}'.format(list_, collection))
+            self.abort(404, 'Element not removed from list {} in collection {} {}'.format(list_name, storage.coll_name, _id))
diff --git a/api/permchecker.py b/api/permchecker.py
index 8106eccc..ae126674 100644
--- a/api/permchecker.py
+++ b/api/permchecker.py
@@ -1,10 +1,12 @@
+# @author:  Renzo Frigato
+
 from users import INTEGER_ROLES
 
 def _get_access(container, uid):
     permissions_list = container['roles'] or container['permissions']
     for perm in permissions_list:
-        if perm._id == uid:
-            return INTEGER_ROLES[perm.access]
+        if perm['_id'] == uid:
+            return INTEGER_ROLES[perm['access']]
     else:
         return -1
 
@@ -21,4 +23,6 @@ def group_roles_sublist(container, method, uid):
     access = _get_access(container, uid)
     return access >= INTEGER_ROLES['admin']
 
+def always_true(container, method, uid):
+    return True
 
-- 
GitLab