diff --git a/api/api.py b/api/api.py index 4e7e88400..69776098e 100644 --- a/api/api.py +++ b/api/api.py @@ -9,13 +9,14 @@ from .handlers.devicehandler import DeviceHandler from .handlers.grouphandler import GroupHandler from .handlers.listhandler import FileListHandler, NotesListHandler, PermissionsListHandler, TagsListHandler +from .handlers.projectsettings import ProjectSettings, RulesHandler, RuleHandler from .handlers.refererhandler import AnalysesHandler from .handlers.reporthandler import ReportHandler from .handlers.resolvehandler import ResolveHandler from .handlers.roothandler import RootHandler from .handlers.schemahandler import SchemaHandler from .handlers.userhandler import UserHandler -from .jobs.handlers import BatchHandler, JobsHandler, JobHandler, GearsHandler, GearHandler, RulesHandler, RuleHandler +from .jobs.handlers import BatchHandler, JobsHandler, JobHandler, GearsHandler, GearHandler from .upload import Upload from .web.base import RequestHandler from . import config @@ -201,8 +202,10 @@ def prefix(path, routes): prefix('/projects', [ route('/groups', ContainerHandler, h='get_groups_with_project', m=['GET']), route('/recalc', ContainerHandler, h='calculate_project_compliance', m=['POST']), - route('//template', ContainerHandler, h='set_project_template', m=['POST']), - route('//template', ContainerHandler, h='delete_project_template', m=['DELETE']), + route('//phi', ProjectSettings, h='get_phi', m=['GET']), + route('//phi', ProjectSettings, h='update_phi', m=['POST', 'PUT']), + route('//template', ProjectSettings, h='set_project_template', m=['POST']), + route('//template', ProjectSettings, h='delete_project_template', m=['DELETE']), route('//recalc', ContainerHandler, h='calculate_project_compliance', m=['POST']), route('//rules', RulesHandler, m=['GET', 'POST']), route('//rules/', RuleHandler, m=['GET', 'PUT', 'DELETE']), diff --git a/api/auth/listauth.py b/api/auth/listauth.py index 9253bb5ed..caa0eb9fa 100644 --- a/api/auth/listauth.py +++ b/api/auth/listauth.py @@ -6,7 +6,7 @@ import sys from .. import config -from . import _get_access, INTEGER_PERMISSIONS +from . import _get_access, check_phi, INTEGER_PERMISSIONS log = config.log @@ -19,7 +19,8 @@ def default_sublist(handler, container): """ access = _get_access(handler.uid, container) def g(exec_op): - def f(method, _id, query_params=None, payload=None, exclude_params=None): + def f(method, _id, query_params=None, payload=None, exclude_params=None, phi=False): + log.debug(method) if method == 'GET' and container.get('public', False): min_access = -1 elif method == 'GET': @@ -28,7 +29,10 @@ def f(method, _id, query_params=None, payload=None, exclude_params=None): min_access = INTEGER_PERMISSIONS['rw'] else: min_access = sys.maxint - + if method == 'GET': + log.debug(container) + if (not check_phi(handler.uid, container)) and phi: + handler.abort(403, "User not authorized to view PHI fields on the container.") if access >= min_access: return exec_op(method, _id, query_params, payload, exclude_params) else: @@ -58,7 +62,11 @@ def group_tags_sublist(handler, container): """ access = _get_access(handler.uid, container) def g(exec_op): - def f(method, _id, query_params = None, payload = None, exclude_params=None): + def f(method, _id, query_params = None, payload = None, exclude_params=None, phi=False): + if method == 'GET': + log.debug(container) + if (not check_phi(handler.uid, container)) and phi: + handler.abort(403, "User not authorized to view PHI fields on the container.") if method == 'GET' and access >= INTEGER_PERMISSIONS['ro']: return exec_op(method, _id, query_params, payload, exclude_params) elif access >= INTEGER_PERMISSIONS['rw']: @@ -90,17 +98,24 @@ def notes_sublist(handler, container): """ access = _get_access(handler.uid, container) def g(exec_op): - def f(method, _id, query_params = None, payload = None, exclude_params=None): + def f(method, _id, query_params = None, payload = None, exclude_params=None, phi=False): if access >= INTEGER_PERMISSIONS['admin']: pass elif method == 'POST' and access >= INTEGER_PERMISSIONS['rw'] and payload['user'] == handler.uid: pass elif method == 'GET' and (access >= INTEGER_PERMISSIONS['ro'] or container.get('public')): - pass + if not check_phi(handler.uid, container) and phi: + handler.abort(403, "User not authorized to view PHI fields on the container.") elif method in ['GET', 'DELETE', 'PUT'] and container['notes'][0]['user'] == handler.uid: pass else: handler.abort(403, 'user not authorized to perform a {} operation on the list'.format(method)) + + # Check phi access if GET and notes is considered phi by the project or site + if method == 'GET': + log.debug(container) + if (not check_phi(handler.uid, container)) and phi: + handler.abort(403, "User not authorized to view PHI fields on the container.") return exec_op(method, _id, query_params, payload, exclude_params) return f return g diff --git a/api/config.py b/api/config.py index 287ca064e..b25ac5174 100644 --- a/api/config.py +++ b/api/config.py @@ -158,10 +158,12 @@ def apply_env_variables(config): 'collection.json', 'collection-update.json', 'container.json', + 'custom-phi-update.json', 'device.json', 'file.json', 'file-update.json', 'group-new.json', + "group-permission.json", 'group-update.json', 'info_update.json', 'note.json', diff --git a/api/dao/containerstorage.py b/api/dao/containerstorage.py index 7566fb32a..7a009c932 100644 --- a/api/dao/containerstorage.py +++ b/api/dao/containerstorage.py @@ -96,6 +96,16 @@ def recalc_sessions_compliance(self, project_id=None): changed_sessions.append(s['_id']) return changed_sessions + def get_phi_fields(self, cid, projection=None): + log.debug(cid) + phi = config.db.project_phi.find_one({"project_id": str(cid)}, projection=projection) + if phi == None: + return {"fields":[]} + else: + return phi + + def add_phi_fields(self, cid, update): + return config.db.project_phi.update({"project_id": cid}, {"$set": update}, upsert=True) class SessionStorage(ContainerStorage): diff --git a/api/dao/liststorage.py b/api/dao/liststorage.py index 50df34cc3..a0ee78e47 100644 --- a/api/dao/liststorage.py +++ b/api/dao/liststorage.py @@ -46,7 +46,7 @@ def get_container(self, _id, query_params=None): projection = {self.list_name + '.$': 1, 'permissions': 1, 'public': 1} return self.dbc.find_one(query, projection) - def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_params=None): + def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_params=None, phi=False): # pylint: disable=unused-argument """ Generic method to exec an operation. The request is dispatched to the corresponding private methods. @@ -220,7 +220,7 @@ def get_container(self, _id, query_params=None): projection = {self.list_name : 1, 'permissions': 1, 'public': 1} return self.dbc.find_one(query, projection) - def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_params=None): + def exec_op(self, action, _id=None, query_params=None, payload=None, exclude_params=None, phi=False): # pylint: disable=unused-argument """ This method "flattens" the query parameter and the payload to handle string lists """ diff --git a/api/handlers/collectionshandler.py b/api/handlers/collectionshandler.py index 7bafdba59..6652581fc 100644 --- a/api/handlers/collectionshandler.py +++ b/api/handlers/collectionshandler.py @@ -9,6 +9,7 @@ from ..web.request import AccessType from .containerhandler import ContainerHandler +from .projectsettings import get_phi_fields log = config.log @@ -32,6 +33,7 @@ def __init__(self, request=None, response=None): self.storage = self.container_handler_configurations['collections']['storage'] def get(self, **kwargs): + log.debug(kwargs) return super(CollectionsHandler, self).get(cont_name='collections', **kwargs) def post(self): @@ -166,8 +168,7 @@ def get_sessions(self, cid): permchecker = containerauth.list_public_request else: permchecker = containerauth.list_permission_checker(self) - - projection = self.PHI_FIELDS.copy() + projection = get_phi_fields("sessions", "site") if self.is_true('phi'): projection = None phi = True @@ -175,7 +176,6 @@ def get_sessions(self, cid): phi = False sessions = permchecker(containerstorage.SessionStorage().exec_op)('GET', query=query, public=self.public_request, projection=projection, phi=phi) - log.debug(sessions) self._filter_all_permissions(sessions, self.uid) if self.is_true('measurements'): self._add_session_measurements(sessions) @@ -210,8 +210,7 @@ def get_acquisitions(self, cid): permchecker = containerauth.list_public_request else: permchecker = containerauth.list_permission_checker(self) - - projection = self.PHI_FIELDS.copy() + projection = get_phi_fields("acquisitions", "site") if self.is_true('phi'): projection = None phi = True diff --git a/api/handlers/containerhandler.py b/api/handlers/containerhandler.py index 4825dcb09..24867e2a5 100644 --- a/api/handlers/containerhandler.py +++ b/api/handlers/containerhandler.py @@ -15,6 +15,7 @@ from ..web import base from ..web.errors import APIStorageException from ..web.request import log_access, AccessType +from .projectsettings import phi_payload, get_phi_fields, check_phi_enabled log = config.log @@ -43,10 +44,6 @@ class ContainerHandler(base.RequestHandler): 'acquisitions': True } - # Hard-coded PHI fields, will be changed to user set PHI fields - PHI_FIELDS = {'info': 0, 'subject.firstname':0, 'subject.lastname': 0, 'subject.sex': 0, - 'subject.age': 0, 'subject.race': 0, 'subject.ethnicity': 0, 'subject.info': 0, 'tags': 0, 'files.info':0} - # This configurations are used by the ContainerHandler class to load the storage, # the permissions checker and the json schema validators used to handle a request. # @@ -92,14 +89,16 @@ def __init__(self, request=None, response=None): @log_access(AccessType.view_container) def get(self, cont_name, **kwargs): _id = kwargs.get('cid') - projection = self.PHI_FIELDS.copy() - log.debug(self.PHI_FIELDS) - log.debug(projection) self.config = self.container_handler_configurations[cont_name] self.storage = self.config['storage'] + projection = get_phi_fields(cont_name, _id) container = self._get_container(_id) log.debug(container) - if check_phi(self.uid, container) or self.superuser_request: + self.phi = False + if not check_phi_enabled(cont_name, _id): + self.phi = False + projection = None + elif check_phi(self.uid, container) or self.superuser_request: log.debug("PHI") log.debug(self.superuser_request) self.phi = True @@ -111,6 +110,8 @@ def get(self, cont_name, **kwargs): result = permchecker(self.storage.exec_op)('GET', _id, projection=projection, phi=self.phi) except APIStorageException as e: self.abort(400, e.message) + if not check_phi_enabled(cont_name, _id): + self.phi = True if result is None: self.abort(404, 'Element not found in container {} {}'.format(self.storage.cont_name, _id)) if not self.superuser_request and not self.is_true('join_avatars'): @@ -123,7 +124,7 @@ def get(self, cont_name, **kwargs): fileinfo['path'] = util.path_from_hash(fileinfo['hash']) inflate_job_info = cont_name == 'sessions' - result['analyses'] = AnalysisStorage().get_analyses(cont_name, _id, inflate_job_info, self.PHI_FIELDS.copy()) + result['analyses'] = AnalysisStorage().get_analyses(cont_name, _id, inflate_job_info, projection) return self.handle_origin(result) def handle_origin(self, result): @@ -328,9 +329,9 @@ def get_all(self, cont_name, par_cont_name=None, par_id=None): else: phi = False if projection == None: - projection = self.PHI_FIELDS.copy() + projection = get_phi_fields(cont_name, "site") else: - projection.update(self.PHI_FIELDS) + projection.update(get_phi_fields(cont_name, "site")) # select which permission filter will be applied to the list of results. if self.superuser_request: permchecker = always_ok @@ -408,8 +409,7 @@ def get_all_for_user(self, cont_name, uid): if self.is_true('phi'): phi = True else: - projection = self.PHI_FIELDS.copy() - + projection = get_phi_fields(cont_name, "site") # select which permission filter will be applied to the list of results. if self.superuser_request or self.user_is_admin: permchecker = always_ok @@ -433,6 +433,7 @@ def get_all_for_user(self, cont_name, uid): self._filter_all_permissions(results, uid) return results + @phi_payload(method="POST") def post(self, cont_name): self.config = self.container_handler_configurations[cont_name] self.storage = self.config['storage'] @@ -472,6 +473,7 @@ def post(self, cont_name): else: self.abort(404, 'Element not added in container {}'.format(self.storage.cont_name)) + @phi_payload(method="PUT") @validators.verify_payload_exists def put(self, cont_name, **kwargs): _id = kwargs.pop('cid') @@ -585,35 +587,6 @@ def get_groups_with_project(self): group_ids = list(set((p['group'] for p in self.get_all('projects')))) return list(config.db.groups.find({'_id': {'$in': group_ids}}, ['label'])) - def set_project_template(self, **kwargs): - project_id = kwargs.pop('cid') - self.config = self.container_handler_configurations['projects'] - self.storage = self.config['storage'] - container = self._get_container(project_id) - - template = self.request.json_body - validators.validate_data(template, 'project-template.json', 'input', 'POST') - payload = {'template': template} - payload['modified'] = datetime.datetime.utcnow() - - permchecker = self._get_permchecker(container) - result = permchecker(self.storage.exec_op)('PUT', _id=project_id, payload=payload) - return {'modified': result.modified_count} - - def delete_project_template(self, **kwargs): - project_id = kwargs.pop('cid') - self.config = self.container_handler_configurations['projects'] - self.storage = self.config['storage'] - container = self._get_container(project_id) - - payload = {'modified': datetime.datetime.utcnow()} - unset_payload = {'template': ''} - - permchecker = self._get_permchecker(container) - result = permchecker(self.storage.exec_op)('PUT', _id=project_id, payload=payload, unset_payload=unset_payload) - return {'modified': result.modified_count} - - def calculate_project_compliance(self, **kwargs): project_id = kwargs.pop('cid', None) log.debug("project_id is {}".format(project_id)) @@ -659,6 +632,7 @@ def _get_parent_container(self, payload): def _get_container(self, _id, projection=None, get_children=False): try: + log.debug(self.storage) container = self.storage.get_container(_id, projection=projection, get_children=get_children) except APIStorageException as e: self.abort(400, e.message) diff --git a/api/handlers/listhandler.py b/api/handlers/listhandler.py index 984d2034f..cac79ce04 100644 --- a/api/handlers/listhandler.py +++ b/api/handlers/listhandler.py @@ -17,8 +17,11 @@ from ..dao import liststorage from ..dao import containerutil from ..web.errors import APIStorageException +from ..dao.containerstorage import ProjectStorage +from ..handlers.projectsettings import get_project_id, check_phi_enabled from ..web.request import log_access, AccessType +log = config.log def initialize_list_configurations(): """ @@ -67,7 +70,7 @@ def initialize_list_configurations(): 'use_object_id': False, 'get_full_container': True, 'storage_schema_file': 'permission.json', - 'input_schema_file': 'permission.json' + 'input_schema_file': 'group-permission.json' }, 'tags': { 'storage': liststorage.StringListStorage, @@ -121,8 +124,15 @@ def __init__(self, request=None, response=None): def get(self, cont_name, list_name, **kwargs): _id = kwargs.pop('cid') permchecker, storage, _, _, keycheck = self._initialize_request(cont_name, list_name, _id, query_params=kwargs) + project_storage = ProjectStorage() try: - result = keycheck(permchecker(storage.exec_op))('GET', _id, query_params=kwargs) + # Check to see if list_name in phi lists of site level or project + if list_name in ["tags", "notes"]: + phi = list_name in (project_storage.get_phi_fields("site")["fields"] + project_storage.get_phi_fields(get_project_id(cont_name,_id))["fields"]) and check_phi_enabled(cont_name, _id) + log.debug("Phi: {}".format(phi)) + result = keycheck(permchecker(storage.exec_op))('GET', _id, query_params=kwargs, phi=phi) + else: + result = keycheck(permchecker(storage.exec_op))('GET', _id, query_params=kwargs) except APIStorageException as e: self.abort(400, e.message) @@ -188,7 +198,7 @@ def _initialize_request(self, cont_name, list_name, _id, query_params=None): query_params = None container = storage.get_container(_id, query_params) if container is not None: - if self.superuser_request or self.user_is_admin: + if self.superuser_request: permchecker = always_ok elif self.public_request: permchecker = listauth.public_request(self, container) @@ -213,6 +223,8 @@ def post(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(PermissionsListHandler, self).post(cont_name, list_name, **kwargs) payload = self.request.json_body + if cont_name == "groups" and not payload.get("phi-access"): + payload["phi-access"] = True if cont_name == 'groups' and self.request.params.get('propagate') =='true': self._propagate_permissions(cont_name, _id, query={'permissions._id' : payload['_id']}, update={'$set': {'permissions.$.access': payload['access'], 'permissions.$.phi-access': payload['phi-access']}}) @@ -319,7 +331,6 @@ class TagsListHandler(ListHandler): TagsListHandler overrides put, delete methods of ListHandler to propagate changes to group tags If a tag is renamed or deleted at the group level, project, session and acquisition tags will also be renamed/deleted """ - def put(self, cont_name, list_name, **kwargs): _id = kwargs.get('cid') result = super(TagsListHandler, self).put(cont_name, list_name, **kwargs) diff --git a/api/handlers/projectsettings.py b/api/handlers/projectsettings.py new file mode 100644 index 000000000..ff9597ab6 --- /dev/null +++ b/api/handlers/projectsettings.py @@ -0,0 +1,298 @@ +import bson +import datetime + +from .. import config +from .. import util +from .. import validators +from ..auth import containerauth, always_ok, has_access, check_phi +from ..dao import containerstorage +from ..dao.basecontainerstorage import PARENT_MAP +from ..dao.containerutil import singularize +from ..jobs.gears import get_gear_by_name +from ..jobs.rules import validate_regexes +from ..validators import validate_data, verify_payload_exists +from ..web import base +from ..web.errors import APIPermissionException, APINotFoundException, APIValidationException +from ..web.request import log_access, AccessType + + + +log = config.log + +######## I don't think this is the right spot but util methods for phi access +CONT_STORAGE = { + "groups": containerstorage.GroupStorage(), + "projects": containerstorage.ProjectStorage(), + "sessions": containerstorage.SessionStorage(), + "acquisitions": containerstorage.AcquisitionStorage(), + "analyses": containerstorage.AnalysisStorage(), + "collections": containerstorage.CollectionStorage() +} + +# PHI fields are limited to these fields +PHI_CANDIDATES = ['info', 'files.info', 'files.tags', 'notes', 'subject.firstname', 'subject.lastname', 'subject.sex', + 'subject.age', 'subject.race', 'subject.ethnicity', 'subject.info', 'tags'] + +def get_project_id(cont_name, _id): + if _id == "site" or cont_name == 'groups': + return "site" + if cont_name == "projects": + return _id + elif cont_name == "sessions": + session = get_container(_id, cont_name, projection={'project':1}) + return session.get('project') + elif cont_name == "acquisitions": + return containerstorage.SessionStorage().get_container(get_container(_id, cont_name, projection={'session':1}).get('session'), projection={'project':1}).get('project') + elif cont_name == "collections": + return "site" + else: + raise APINotFoundException("{} are not a container".format(cont_name)) + +def check_phi_enabled(cont_name, _id): + project_id = get_project_id(cont_name, _id) + if project_id != "site": + return get_container(project_id, "projects").get("phi") + else: + return True +def get_phi_fields(cont_name, _id): + project_storage = containerstorage.ProjectStorage() + project_id = get_project_id(cont_name, _id) + if _id == "site" or project_id == "site": + site_phi = project_storage.get_phi_fields("site") + phi_fields = list(set(site_phi.get("fields",[]))) + else: + project_phi = project_storage.get_phi_fields(project_id) + site_phi = project_storage.get_phi_fields("site") + phi_fields = list(set(site_phi.get("fields",[]) + project_phi.get("fields",[]))) + projection = {x:0 for x in phi_fields} + if len(projection) == 0: + projection = None + return projection + +def get_container(_id, cont_name, projection=None, get_children=False): + container = CONT_STORAGE[cont_name].get_container(_id, projection=projection, get_children=get_children) + if container is not None: + return container + else: + raise APINotFoundException(404, 'Element {} not found in container {}'.format(_id, cont_name)) + + +def phi_payload(method=None): + def phi_payload_decorator(handler_method): + def phi_payload_check(self, *args, **kwargs): + if not self.superuser_request and method in ["POST", "PUT"]: + cid = kwargs.get('cid') + cont_name = kwargs.get('cont_name') + if cont_name == "analyses" and self.is_true("job"): + payload = util.mongo_dict(self.request.json_body.get("analysis")) + else: + payload = util.mongo_dict(self.request.json_body) + self.storage = CONT_STORAGE[cont_name] + # Check the using the parent of the container to be created + if method == "POST": + cont_name = PARENT_MAP[cont_name] + cid = payload[singularize(cont_name)] + if not cid or not cont_name: + raise APIValidationException("Request body malformed") + phi_fields = get_phi_fields(cont_name, cid) if cont_name != "groups" else get_phi_fields("projects", "site") + + # If the request is not a superuser/has phi and one of fields in the payload is considered phi by the project the container is in, Raise a permission exception + if not check_phi(self.uid, get_container(cid, cont_name)) and phi_fields and any([True for x in payload if x.startswith(tuple(phi_fields.keys()))]): + log.debug("PHI fields: " + str(phi_fields)) + raise APIPermissionException("User not allowed to write to phi fields") + return handler_method(self, *args, **kwargs) + return phi_payload_check + return phi_payload_decorator + +class ProjectSettings(base.RequestHandler): + + def __init__(self, request=None, response=None): + super(ProjectSettings, self).__init__(request, response) + self.storage = containerstorage.ProjectStorage() + + def set_project_template(self, **kwargs): + project_id = kwargs.pop('cid') + container = self.storage.get_container(project_id) + + template = self.request.json_body + validators.validate_data(template, 'project-template.json', 'input', 'POST') + payload = {'template': template} + payload['modified'] = datetime.datetime.utcnow() + + permchecker = self._get_permchecker(container) + result = permchecker(self.storage.exec_op)('PUT', _id=project_id, payload=payload) + return {'modified': result.modified_count} + + def delete_project_template(self, **kwargs): + project_id = kwargs.pop('cid') + container = self.storage.get_container(project_id) + + payload = {'modified': datetime.datetime.utcnow()} + unset_payload = {'template': ''} + + permchecker = self._get_permchecker(container) + result = permchecker(self.storage.exec_op)('PUT', _id=project_id, payload=payload, unset_payload=unset_payload) + return {'modified': result.modified_count} + + def get_phi(self, cid): + projection = None + + if cid == 'site': + if self.public_request: + raise APIPermissionException('Viewing site-level PHI fields requires login.') + projection = {'project_id': 0} + else: + project = self.storage.get_container(cid, projection={'permissions': 1, 'phi': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'ro'): + raise APIPermissionException('User does not have access to project {} PHI fields'.format(cid)) + if not project.get('phi'): + self.abort(400, "Project {} does not have custom phi list enabled".format(project.get('_id'))) + + return self.storage.get_phi_fields(cid, projection=projection) + + @log_access(AccessType.modify_phi_list) + def update_phi(self, cid): + payload = self.request.json_body + if cid == 'site': + if not self.user_is_admin: + raise APIPermissionException('Modifying site-level PHI fields can only be done by a site admin.') + else: + project = self.storage.get_container(cid, projection={'permissions': 1, 'phi':1}) + if not self.user_is_admin and not has_access(self.uid, project, 'admin'): + raise APIPermissionException('User does not have access to project {} PHI fields'.format(cid)) + if not project.get('phi'): + self.abort(400, "Project {} does not have custom phi list enabled".format(project.get('_id'))) + if not all([x.startswith(tuple(PHI_CANDIDATES)) for x in payload["fields"]]): + self.abort(400, "All fields to be set must be valid phi candidates") + result = self.storage.add_phi_fields(cid, payload) + if result['nModified'] == 1 or result.get('upserted'): + return {"modified": result['nModified'], "upserted": result.get('upserted')} + else: + self.abort(404, "Unable to update phi fields for project_id:{}".format(cid)) + + def _get_permchecker(self, container=None, parent_container=None): + if self.superuser_request: + return always_ok + elif self.public_request: + return containerauth.public_request(self, container) + else: + permchecker = containerauth.default_container + return permchecker(self, container, parent_container) + + +class RulesHandler(base.RequestHandler): + + def get(self, cid): + """List rules""" + + projection = None + + if cid == 'site': + if self.public_request: + raise APIPermissionException('Viewing site-level rules requires login.') + projection = {'project_id': 0} + else: + project = containerstorage.ProjectStorage().get_container(cid, projection={'permissions': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'ro'): + raise APIPermissionException('User does not have access to project {} rules'.format(cid)) + + return config.db.project_rules.find({'project_id' : cid}, projection=projection) + + + @verify_payload_exists + def post(self, cid): + """Add a rule""" + + if cid == 'site': + if not self.user_is_admin: + raise APIPermissionException('Adding site-level rules can only be done by a site admin.') + else: + project = containerstorage.ProjectStorage().get_container(cid, projection={'permissions': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'admin'): + raise APIPermissionException('Adding rules to a project can only be done by a project admin.') + + doc = self.request.json + + validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True) + validate_regexes(doc) + try: + get_gear_by_name(doc['alg']) + except APINotFoundException: + self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(doc['alg'])) + + doc['project_id'] = cid + + result = config.db.project_rules.insert_one(doc) + return { '_id': result.inserted_id } + +class RuleHandler(base.RequestHandler): + + def get(self, cid, rid): + """Get rule""" + + projection = None + if cid == 'site': + if self.public_request: + raise APIPermissionException('Viewing site-level rules requires login.') + projection = {'project_id': 0} + else: + project = containerstorage.ProjectStorage().get_container(cid, projection={'permissions': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'ro'): + raise APIPermissionException('User does not have access to project {} rules'.format(cid)) + + result = config.db.project_rules.find_one({'project_id' : cid, '_id': bson.ObjectId(rid)}, projection=projection) + + if not result: + raise APINotFoundException('Rule not found.') + + return result + + + @verify_payload_exists + def put(self, cid, rid): + """Change a rule""" + + if cid == 'site': + if not self.user_is_admin: + raise APIPermissionException('Modifying site-level rules can only be done by a site admin.') + else: + project = containerstorage.ProjectStorage().get_container(cid, projection={'permissions': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'admin'): + raise APIPermissionException('Modifying project rules can only be done by a project admin.') + + doc = config.db.project_rules.find_one({'project_id' : cid, '_id': bson.ObjectId(rid)}) + + if not doc: + raise APINotFoundException('Rule not found.') + + updates = self.request.json + validate_data(updates, 'rule-update.json', 'input', 'POST', optional=True) + validate_regexes(updates) + if updates.get('alg'): + try: + get_gear_by_name(updates['alg']) + except APINotFoundException: + self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(updates['alg'])) + + doc.update(updates) + config.db.project_rules.replace_one({'_id': bson.ObjectId(rid)}, doc) + + return + + + def delete(self, cid, rid): + """Remove a rule""" + + if cid == 'site': + if not self.user_is_admin: + raise APIPermissionException('Modifying site-level rules can only be done by a site admin.') + else: + project = containerstorage.ProjectStorage().get_container(cid, projection={'permissions': 1}) + if not self.user_is_admin and not has_access(self.uid, project, 'admin'): + raise APIPermissionException('Modifying project rules can only be done by a project admin.') + + + result = config.db.project_rules.delete_one({'project_id' : cid, '_id': bson.ObjectId(rid)}) + if result.deleted_count != 1: + raise APINotFoundException('Rule not found.') + return diff --git a/api/handlers/refererhandler.py b/api/handlers/refererhandler.py index c747ffd96..4a72ce048 100644 --- a/api/handlers/refererhandler.py +++ b/api/handlers/refererhandler.py @@ -19,11 +19,12 @@ from ..auth import containerauth, always_ok, check_phi from ..dao import containerstorage, noop from ..dao.basecontainerstorage import ContainerStorage -from ..dao.containerutil import singularize +from ..dao.containerutil import pluralize, singularize from ..web import base from ..web.errors import APIStorageException from ..web.request import log_access, AccessType from .listhandler import FileListHandler +from .projectsettings import get_phi_fields, check_phi_enabled log = config.log @@ -63,13 +64,12 @@ class AnalysesHandler(RefererHandler): payload_schema_file = 'analysis.json' update_payload_schema_file = 'analysis-update.json' - # Hard-coded PHI fields, will be changed to user set PHI fields - PHI_FIELDS = {'info': 0, 'tags': 0, 'files.info':0} def __init__(self, request=None, response=None): super(AnalysesHandler, self).__init__(request, response) self.phi = False + # @phi_payload(method="Analysis POST") def post(self, cont_name, cid): """ Default behavior: @@ -110,6 +110,7 @@ def post(self, cont_name, cid): else: self.abort(500, 'Analysis not added for container {} {}'.format(cont_name, cid)) + # @phi_payload(method="PUT") @validators.verify_payload_exists def put(self, cont_name, **kwargs): cid = kwargs.pop('cid') @@ -136,8 +137,12 @@ def get(self, **kwargs): _id = kwargs.get('_id') analysis = self.storage.get_container(_id) parent = self.storage.get_parent(analysis['parent']['type'], analysis['parent']['id']) - projection = self.PHI_FIELDS.copy() - if check_phi(self.uid, parent)or self.superuser_request: + projection = get_phi_fields(pluralize(analysis['parent']['type']), analysis['parent']['id']) + self.phi = False + if not check_phi_enabled(pluralize(analysis['parent']['type']), analysis['parent']['id']): + self.phi = False + projection = None + elif check_phi(self.uid, parent) or self.superuser_request: self.phi = True projection = None permchecker = self.get_permchecker(parent) @@ -147,6 +152,8 @@ def get(self, **kwargs): if self.is_true('inflate_job'): self.storage.inflate_job_info(analysis) + if not check_phi_enabled(pluralize(analysis['parent']['type']), analysis['parent']['id']): + self.phi = True self.log_user_access(AccessType.view_container, cont_name=analysis['parent']['type'], cont_id=analysis['parent']['id']) return analysis diff --git a/api/handlers/reporthandler.py b/api/handlers/reporthandler.py index d15f3b0b1..b449b6b58 100644 --- a/api/handlers/reporthandler.py +++ b/api/handlers/reporthandler.py @@ -42,6 +42,7 @@ "context.analysis.label", "context.collection.id", "context.collection.label", + "context.new_phi_list", "context.ticket_id", "request_method", "request_path" @@ -879,7 +880,7 @@ def _build_project_report(self, base_query): # For the project and each session and acquisition, create a list of analysis ids parent_ids = session_ids + acquisition_ids + [p['_id']] analysis_ids = [an['_id'] for an in config.db.analyses.find({'parent.id': {'$in': parent_ids}})] - + report_obj['session_count'] = len(session_ids) # for each type of container below it will have a slightly modified match query diff --git a/api/jobs/handlers.py b/api/jobs/handlers.py index 46802aa0a..671ed7f55 100644 --- a/api/jobs/handlers.py +++ b/api/jobs/handlers.py @@ -10,23 +10,21 @@ from .. import upload from .. import util from ..auth import require_login, has_access -from ..dao.containerstorage import ProjectStorage, SessionStorage, AcquisitionStorage +from ..dao.containerstorage import AcquisitionStorage, SessionStorage from ..dao.containerutil import ContainerReference from ..web import base from ..web.encoder import pseudo_consistent_json_encode -from ..web.errors import APIPermissionException, APINotFoundException, InputValidationException +from ..web.errors import APIPermissionException, InputValidationException from .. import config from . import batch -from ..validators import validate_data, verify_payload_exists - from ..auth.apikeys import JobApiKey -from .gears import validate_gear_config, get_gears, get_gear, get_invocation_schema, remove_gear, upsert_gear, suggest_container, get_gear_by_name, check_for_gear_insertion +from .gears import check_for_gear_insertion + +from .gears import validate_gear_config, get_gears, get_gear, get_invocation_schema, remove_gear, upsert_gear, suggest_container from .jobs import Job, Logs from .batch import check_state, update from .queue import Queue -from .rules import validate_regexes - class GearsHandler(base.RequestHandler): @@ -143,123 +141,6 @@ def delete(self, _id): return remove_gear(_id) -class RulesHandler(base.RequestHandler): - - def get(self, cid): - """List rules""" - - projection = None - - if cid == 'site': - if self.public_request: - raise APIPermissionException('Viewing site-level rules requires login.') - projection = {'project_id': 0} - else: - project = ProjectStorage().get_container(cid, projection={'permissions': 1}) - if not self.user_is_admin and not has_access(self.uid, project, 'ro'): - raise APIPermissionException('User does not have access to project {} rules'.format(cid)) - - return config.db.project_rules.find({'project_id' : cid}, projection=projection) - - - @verify_payload_exists - def post(self, cid): - """Add a rule""" - - if cid == 'site': - if not self.user_is_admin: - raise APIPermissionException('Adding site-level rules can only be done by a site admin.') - else: - project = ProjectStorage().get_container(cid, projection={'permissions': 1}) - if not self.user_is_admin and not has_access(self.uid, project, 'admin'): - raise APIPermissionException('Adding rules to a project can only be done by a project admin.') - - doc = self.request.json - - validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True) - validate_regexes(doc) - try: - get_gear_by_name(doc['alg']) - except APINotFoundException: - self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(doc['alg'])) - - doc['project_id'] = cid - - result = config.db.project_rules.insert_one(doc) - return { '_id': result.inserted_id } - -class RuleHandler(base.RequestHandler): - - def get(self, cid, rid): - """Get rule""" - - projection = None - if cid == 'site': - if self.public_request: - raise APIPermissionException('Viewing site-level rules requires login.') - projection = {'project_id': 0} - else: - project = ProjectStorage().get_container(cid, projection={'permissions': 1}) - if not self.user_is_admin and not has_access(self.uid, project, 'ro'): - raise APIPermissionException('User does not have access to project {} rules'.format(cid)) - - result = config.db.project_rules.find_one({'project_id' : cid, '_id': bson.ObjectId(rid)}, projection=projection) - - if not result: - raise APINotFoundException('Rule not found.') - - return result - - - @verify_payload_exists - def put(self, cid, rid): - """Change a rule""" - - if cid == 'site': - if not self.user_is_admin: - raise APIPermissionException('Modifying site-level rules can only be done by a site admin.') - else: - project = ProjectStorage().get_container(cid, projection={'permissions': 1}) - if not self.user_is_admin and not has_access(self.uid, project, 'admin'): - raise APIPermissionException('Modifying project rules can only be done by a project admin.') - - doc = config.db.project_rules.find_one({'project_id' : cid, '_id': bson.ObjectId(rid)}) - - if not doc: - raise APINotFoundException('Rule not found.') - - updates = self.request.json - validate_data(updates, 'rule-update.json', 'input', 'POST', optional=True) - validate_regexes(updates) - if updates.get('alg'): - try: - get_gear_by_name(updates['alg']) - except APINotFoundException: - self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(updates['alg'])) - - doc.update(updates) - config.db.project_rules.replace_one({'_id': bson.ObjectId(rid)}, doc) - - return - - - def delete(self, cid, rid): - """Remove a rule""" - - if cid == 'site': - if not self.user_is_admin: - raise APIPermissionException('Modifying site-level rules can only be done by a site admin.') - else: - project = ProjectStorage().get_container(cid, projection={'permissions': 1}) - if not self.user_is_admin and not has_access(self.uid, project, 'admin'): - raise APIPermissionException('Modifying project rules can only be done by a project admin.') - - - result = config.db.project_rules.delete_one({'project_id' : cid, '_id': bson.ObjectId(rid)}) - if result.deleted_count != 1: - raise APINotFoundException('Rule not found.') - return - class JobsHandler(base.RequestHandler): """Provide /jobs API routes.""" def get(self): # pragma: no cover (no route) diff --git a/api/web/base.py b/api/web/base.py index b2a5c0b1f..c4ab3a556 100644 --- a/api/web/base.py +++ b/api/web/base.py @@ -395,7 +395,7 @@ def log_user_access(self, access_type, cont_name=None, cont_id=None, multifile=F 'timestamp': datetime.datetime.utcnow() } - if access_type not in [AccessType.user_login, AccessType.user_logout]: + if access_type not in [AccessType.user_login, AccessType.user_logout, AccessType.modify_phi_list]: if cont_name is None or cont_id is None: raise Exception('Container information not available.') @@ -413,6 +413,8 @@ def log_user_access(self, access_type, cont_name=None, cont_id=None, multifile=F if k == 'subject': context[k]['label'] = v.get('code') log_map['context'] = context + elif access_type == AccessType.modify_phi_list: + log_map['context'] = {'project': {"id" : cont_id}, 'new_phi_list': self.request.json_body["fields"]} if access_type is AccessType.download_file and self.get_param('ticket') and not multifile: # If this is a ticket download, log only once per ticket diff --git a/api/web/request.py b/api/web/request.py index 16b1a3270..9bda48f7a 100644 --- a/api/web/request.py +++ b/api/web/request.py @@ -14,7 +14,8 @@ 'delete_file': 'delete_file', 'delete_analysis': 'delete_analysis', 'user_login': 'user_login', - 'user_logout': 'user_logout' + 'user_logout': 'user_logout', + 'modify_phi_list': 'modify_phi_list', }) AccessTypeList = [type_name for type_name, member in AccessType.__members__.items()] diff --git a/bin/database.py b/bin/database.py index 13e980a54..0dd92a817 100755 --- a/bin/database.py +++ b/bin/database.py @@ -22,7 +22,7 @@ from api.types import Origin from api.jobs import batch -CURRENT_DATABASE_VERSION = 40 # An int that is bumped when a new schema change is made +CURRENT_DATABASE_VERSION = 41 # An int that is bumped when a new schema change is made def get_db_version(): @@ -1219,23 +1219,6 @@ def upgrade_to_37(): cursor = config.db[coll].find({'permissions.site': {'$exists': True}}) process_cursor(cursor, upgrade_to_32_closure, context = coll) -def upgrade_to_38_closure(coll_item, coll): - permissions = coll_item.get('permissions', []) - for permission_ in permissions: - permission_['phi-access'] = True - result = config.db[coll].update_one({'_id': coll_item['_id']}, {'$set': {'permissions' : permissions}}) - if result.modified_count == 0: - return "Failed to add phi access to permissions in {} container {}".format(coll_item.get("_id"), coll) - return True - -def upgrade_to_38(): - """ - permissions now have a mandatory phi-access field - """ - for coll in ['acquisitions', 'groups', 'projects', 'sessions', 'analyses']: - cursor = config.db[coll].find({'permissions.site': {'$exists': True}}) - process_cursor(cursor, upgrade_to_38_closure, context = coll) - def upgrade_to_38_closure(user): # if user has existing API key in correct db location, remove API key stored on user and move on @@ -1317,6 +1300,22 @@ def upgrade_to_40(): cursor = config.db.acquisitions.find({'timestamp':{'$type':'string'}}) process_cursor(cursor, upgrade_to_40_closure) +def upgrade_to_41_closure(coll_item, coll): + permissions = coll_item.get('permissions', []) + for permission_ in permissions: + permission_['phi-access'] = True + result = config.db[coll].update_one({'_id': coll_item['_id']}, {'$set': {'permissions' : permissions}}) + if result.modified_count == 0: + return "Failed to add phi access to permissions in {} container {}".format(coll_item.get("_id"), coll) + return True + +def upgrade_to_41(): + """ + permissions now have a mandatory phi-access field + """ + for coll in ['acquisitions', 'groups', 'projects', 'sessions', 'analyses']: + cursor = config.db[coll].find({'permissions.site': {'$exists': True}}) + process_cursor(cursor, upgrade_to_38_closure, context = coll) ### ### BEGIN RESERVED UPGRADE SECTION ### diff --git a/raml/examples/input/custom-phi-update.json b/raml/examples/input/custom-phi-update.json new file mode 100644 index 000000000..b482e6aa5 --- /dev/null +++ b/raml/examples/input/custom-phi-update.json @@ -0,0 +1 @@ +{"fields" : ["subject.firstname", "info"]} diff --git a/raml/examples/input/group-permission.json b/raml/examples/input/group-permission.json new file mode 100644 index 000000000..ae383f550 --- /dev/null +++ b/raml/examples/input/group-permission.json @@ -0,0 +1,4 @@ +{ + "access": "admin", + "_id": "coltonlw@flywheel.io" +} diff --git a/raml/examples/input/project.json b/raml/examples/input/project.json index 017b5c8a1..3aa7f9c5e 100644 --- a/raml/examples/input/project.json +++ b/raml/examples/input/project.json @@ -1 +1 @@ -{"group":"scitran","label":"test2","description":"test project"} +{"group":"scitran","label":"test2","description":"test project", "phi":true} diff --git a/raml/examples/output/custom-phi.json b/raml/examples/output/custom-phi.json new file mode 100644 index 000000000..b482e6aa5 --- /dev/null +++ b/raml/examples/output/custom-phi.json @@ -0,0 +1 @@ +{"fields" : ["subject.firstname", "info"]} diff --git a/raml/schemas/definitions/custom-phi.json b/raml/schemas/definitions/custom-phi.json new file mode 100644 index 000000000..9c784a15a --- /dev/null +++ b/raml/schemas/definitions/custom-phi.json @@ -0,0 +1,14 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "definitions":{ + "fields": { + "type": "array", + "item": { + "type": "string" + } + } + }, + "required": [ + "fields" + ] +} \ No newline at end of file diff --git a/raml/schemas/definitions/project.json b/raml/schemas/definitions/project.json index 0c3c21a1a..f18d1f910 100644 --- a/raml/schemas/definitions/project.json +++ b/raml/schemas/definitions/project.json @@ -2,6 +2,7 @@ "$schema": "http://json-schema.org/draft-04/schema#", "definitions":{ "description": {"type": "string"}, + "phi": {"type": "boolean"}, "project-input":{ "type": "object", "properties": { @@ -10,7 +11,8 @@ "info": {"$ref": "../definitions/container.json#/definitions/info"}, "description": {"$ref": "../definitions/project.json#/definitions/description"}, "group":{"$ref":"../definitions/group.json#/definitions/_id"}, - "archived": {"$ref":"../definitions/container.json#/definitions/archived"} + "archived": {"$ref":"../definitions/container.json#/definitions/archived"}, + "phi": {"$ref": "../definitions/project.json#/definitions/phi"} }, "additionalProperties": false }, @@ -30,6 +32,7 @@ "type":"array", "items":{"$ref":"../definitions/permission.json#/definitions/permission-output-default-required"} }, + "phi": {"$ref": "../definitions/project.json#/definitions/phi"}, "files":{ "type":"array", "items":{ diff --git a/raml/schemas/input/custom-phi-update.json b/raml/schemas/input/custom-phi-update.json new file mode 100644 index 000000000..76ee66d77 --- /dev/null +++ b/raml/schemas/input/custom-phi-update.json @@ -0,0 +1,6 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Custom-PHI", + "type": "object", + "allOf": [{"$ref": "../definitions/custom-phi.json#/definitions"}] +} \ No newline at end of file diff --git a/raml/schemas/input/group-permission.json b/raml/schemas/input/group-permission.json new file mode 100644 index 000000000..85172151f --- /dev/null +++ b/raml/schemas/input/group-permission.json @@ -0,0 +1,7 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "allOf":[{"$ref":"../definitions/permission.json#/definitions/permission"}], + "key_fields": ["_id"], + "required": ["_id", "access"] +} diff --git a/raml/schemas/input/project.json b/raml/schemas/input/project.json index ab2ea95ba..a34607bea 100644 --- a/raml/schemas/input/project.json +++ b/raml/schemas/input/project.json @@ -3,5 +3,5 @@ "title": "Project", "type": "object", "allOf": [{"$ref": "../definitions/project.json#/definitions/project-input"}], - "required": ["label", "group"] + "required": ["label", "group", "phi"] } diff --git a/raml/schemas/mongo/project.json b/raml/schemas/mongo/project.json index 3454ff078..da5bf2490 100644 --- a/raml/schemas/mongo/project.json +++ b/raml/schemas/mongo/project.json @@ -16,6 +16,7 @@ "tags": {}, "info": {}, "description": {}, + "phi": {}, "group": {"type": "string"} }, diff --git a/raml/schemas/output/custom-phi.json b/raml/schemas/output/custom-phi.json new file mode 100644 index 000000000..76ee66d77 --- /dev/null +++ b/raml/schemas/output/custom-phi.json @@ -0,0 +1,6 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "title": "Custom-PHI", + "type": "object", + "allOf": [{"$ref": "../definitions/custom-phi.json#/definitions"}] +} \ No newline at end of file diff --git a/tests/integration_tests/abao/load_fixture.py b/tests/integration_tests/abao/load_fixture.py index 32c036bdc..1b6fe7179 100644 --- a/tests/integration_tests/abao/load_fixture.py +++ b/tests/integration_tests/abao/load_fixture.py @@ -347,7 +347,8 @@ def main(): r = as_root.post('/projects', json={ 'group': 'test-group', 'label': 'Project with template', - 'public': False + 'public': False, + 'phi': True }) assert r.ok st_project = r.json() diff --git a/tests/integration_tests/python/conftest.py b/tests/integration_tests/python/conftest.py index e84f98ea5..a47816091 100644 --- a/tests/integration_tests/python/conftest.py +++ b/tests/integration_tests/python/conftest.py @@ -103,7 +103,7 @@ def default_payload(): return attrdict.AttrDict({ 'user': {'firstname': 'test', 'lastname': 'user'}, 'group': {}, - 'project': {'public': True}, + 'project': {'public': True, 'phi':True}, 'session': {'public': True}, 'acquisition': {'public': True}, 'collection': {}, @@ -256,7 +256,7 @@ def create(self, resource, **kwargs): # add missing label fields using randstr # such fields are: [project.label, session.label, acquisition.label, group.label] - if resource in ['project', 'group', 'groups', 'session', 'acquisition'] and 'label' not in payload: + if resource in ['project', 'group', 'session', 'acquisition'] and 'label' not in payload: payload['label'] = self.randstr() # add missing parent container when creating child container diff --git a/tests/integration_tests/python/test_collection.py b/tests/integration_tests/python/test_collection.py index 15c90636d..cb4e2967e 100644 --- a/tests/integration_tests/python/test_collection.py +++ b/tests/integration_tests/python/test_collection.py @@ -118,6 +118,10 @@ def test_collections_phi(data_builder, as_admin, as_user, log_db, as_root,file_f session = data_builder.create_session() acquisition = data_builder.create_acquisition() + # Set sitewide phi fields + r = as_admin.post("/projects/site/phi", json={"fields":["files.info"]}) + assert r.ok + # create collection r = as_admin.post('/collections', json={ 'label': 'SciTran/Testing', @@ -208,3 +212,6 @@ def test_collections_phi(data_builder, as_admin, as_user, log_db, as_root,file_f r = as_admin.delete('/collections/'+ collection) assert r.ok + # Unset sitewide phi fields + r = as_admin.put("/projects/site/phi", json={"fields":[]}) + assert r.ok diff --git a/tests/integration_tests/python/test_containers.py b/tests/integration_tests/python/test_containers.py index 429f0e464..f15586d89 100644 --- a/tests/integration_tests/python/test_containers.py +++ b/tests/integration_tests/python/test_containers.py @@ -262,11 +262,35 @@ def test_get_all_for_user(as_admin, as_public): r = as_admin.get('/users/' + user_id + '/sessions') assert r.ok -def test_phi_access(as_user, as_admin, as_root, data_builder, log_db): +def test_phi_access_get(as_user, as_admin, as_root, data_builder, log_db): group = data_builder.create_group() - project = data_builder.create_project() + project = data_builder.create_project(phi=True) session = data_builder.create_session() - r = as_admin.put('/sessions/' + session, json={"subject":{"firstname":"FirstName", "code":"Subject_Code"}}, params={'replace_metadata':True}) + + project_2 = data_builder.create_project(phi=False) + age = 4 + session_no_phi = data_builder.create_session(project=project_2, info={"age": age}, subject={"firstname":"FirstName", "code":"Subject_Code", "lastname": "LastName"}) + r = as_root.get('/sessions/'+ session_no_phi) + assert r.ok + assert r.json()["info"]["age"] + + # Set site wide phi fields + r = as_admin.post('/projects/site/phi', json={'fields': [ 'info', 'subject.firstname', 'subject.lastname', 'subject.sex', + 'subject.age', 'subject.race', 'subject.ethnicity', 'subject.info', + 'tags', 'files.info']}) + r = as_admin.get('/projects/site/phi') + assert r.ok + assert r.json()["fields"] == [ 'info', 'subject.firstname', 'subject.lastname', 'subject.sex', + 'subject.age', 'subject.race', 'subject.ethnicity', 'subject.info', + 'tags', 'files.info'] + + r = as_admin.put('/projects/site/phi', json={'fields': ['info', 'subject.firstname', 'subject.lastname', 'subject.sex', + 'subject.age', 'subject.race', 'subject.ethnicity', 'subject.info']}) + assert r.ok + r = as_admin.post('/projects/' + project + '/phi', json={"fields": ['tags', 'files.info']}) + assert r.ok + + r = as_admin.put('/sessions/' + session, json={"subject":{"firstname":"FirstName", "code":"Subject_Code", "lastname": "LastName"}}, params={'replace_metadata':True}) assert r.ok # Test phi access for list returns with phi access level @@ -295,19 +319,21 @@ def test_phi_access(as_user, as_admin, as_root, data_builder, log_db): assert r.json().get('subject').get('code') == 'Subject_Code' assert pre_log == log_db.access_log.count({}) - 2 - # Set no-phi flag to true + # Set phi-access to false r = as_admin.put('/projects/' + project + '/permissions/admin@user.com', json={'phi-access': False}) assert r.ok r = as_admin.post('/projects/' + project + '/permissions', json={'access': 'ro', 'phi-access': False, '_id': 'user@user.com'}) assert r.ok + r = as_admin.post('/projects/' + project_2 + '/permissions', json={'access': 'ro', 'phi-access': False, '_id': 'user@user.com'}) + assert r.ok # Test phi access for list returns without phi access level pre_log = log_db.access_log.count({}) r = as_admin.get('/sessions') assert r.ok for session_ in r.json(): - assert session_.get('subject').get('firstname') == None - assert session_.get('subject').get('code') == 'Subject_Code' + assert session_.get('subject').get('firstname') == None # Phi at site level + assert session_.get('subject').get('code') == 'Subject_Code' # Not considered phi at any level assert pre_log == log_db.access_log.count({}) r = as_admin.get('/sessions', params={'phi':True}) assert r.status_code == 403 @@ -328,16 +354,139 @@ def test_phi_access(as_user, as_admin, as_root, data_builder, log_db): pre_log = log_db.access_log.count({}) r = as_user.get('/sessions/' + session) assert r.ok - assert r.json().get('subject').get('firstname') == None - assert r.json().get('subject').get('code') == 'Subject_Code' + assert r.json().get('subject').get('firstname') == None # Phi at site level + assert r.json().get('subject').get('code') == 'Subject_Code' # Not considered phi at any level assert pre_log == log_db.access_log.count({}) r = as_user.get('/sessions/' + session, params={'phi':True}) assert r.status_code == 200 + # Change the site level and project level phi fields + r = as_admin.put('/projects/site/phi', json={'fields': ['info', 'tags', 'notes', 'subject.lastname']}) + assert r.ok + r = as_admin.put('/projects/' + project + '/phi', json={'fields': ['info', 'subject.firstname', 'subject.lastname', 'subject.sex', + 'subject.age', 'subject.race', 'subject.ethnicity', 'subject.info']}) + assert r.ok + + # Check that get all on sessions is only affected by the site level custom phi list + r = as_admin.get('/sessions') + assert r.ok + + for session_ in r.json(): + assert session_.get('subject').get('firstname') == 'FirstName' # Only considered phi for the project + assert session_.get('subject').get('code') == 'Subject_Code' # Not considered phi + assert session_.get('subject').get('lastname') == None # Considered phi at site level + + + # Test that project with phi disabled, user without phi can access site level phi fields + pre_log = log_db.access_log.count({}) + r = as_user.get('/sessions/'+ session_no_phi) + assert r.ok + assert r.json()["info"]["age"] == age + assert pre_log == log_db.access_log.count({}) - 1 + r = as_admin.delete('/sessions/' + session) assert r.ok + r = as_admin.put('/projects/site/phi', json={"fields":[]}) + assert r.ok + +# Test both POST and PUT containers and on the project phi lists +def test_phi_access_post_put(data_builder, as_user, as_admin, as_root, log_db): + group = data_builder.create_group() + project = data_builder.create_project() + project_phi_disabled = data_builder.create_project(phi=False) + + r = as_admin.post('/projects/' + project + '/permissions', json={'access': 'admin', 'phi-access': False, '_id': 'user@user.com'}) + assert r.ok + + # Test that user cannot post custom list to project_phi_disabled + r = as_admin.post('/projects/' + project_phi_disabled + '/phi', json={"fields": []}) + assert r.status_code == 400 + + pre_log = log_db.access_log.count({}) + # Try setting a field that is not a PHI candidate as phi + r = as_admin.post('/projects/site/phi', json={"fields": ["label"]}) + assert r.status_code == 400 + assert pre_log == log_db.access_log.count({}) + + pre_log = log_db.access_log.count({}) + # Set a valid field as phi at site level + r = as_admin.post('/projects/site/phi', json={"fields": ["info.age", "info"]}) + assert r.ok + assert pre_log == log_db.access_log.count({}) - 1 + + # Post a session writing to phi field with phi-access + r = as_admin.post('/sessions', json={"timestamp": "2017-11-08T21:20:00.000Z", "label": "s1", "subject": {"code": "ex1"}, "project": project, "info": {"phi":"possible"}}) + assert r.ok + session_2 = r.json()["_id"] + # Post a session writing to phi field without phi-access + r = as_user.post('/sessions', json={"timestamp": "2017-11-08T21:20:00.000Z", "label": "s1", "subject": {"code": "ex1"}, "project": project, "info": {"phi":"possible"}}) + assert r.status_code == 403 + + # Post a session writing to info field when only a different info field is phi + r = as_admin.post('/projects/site/phi', json={"fields": ["info.age"]}) + assert r.ok + r = as_user.post('/sessions', json={"timestamp": "2017-11-08T21:20:00.000Z", "label": "s1", "subject": {"code": "ex1"}, "project": project, "info": {"phi":"possible"}}) + assert r.ok + session_3 = r.json()['_id'] + + r = as_admin.post('/projects/site/phi', json={"fields": ["tags", "notes"]}) + assert r.ok + + # Test posting and viewing tags + r = as_admin.post('/groups/' + group + '/tags', json={'value': "TagName"}) + assert r.ok + + # Test that user can post tags even if it is a phi field + r = as_user.post('/projects/' + project + '/tags', json={'value': "TagName"}) + assert r.ok + + # Test that user w/o phi-access cannot see tags + r = as_user.get('/projects/' + project + '/tags/TagName') + assert r.status_code == 403 + + # Test that user w/ phi-access can see tags + r = as_admin.put('/projects/' + project + '/permissions/user@user.com', json={'phi-access': True}) + assert r.ok + r = as_user.get('/projects/' + project + '/tags/TagName') + assert r.ok + r = as_admin.put('/projects/' + project + '/permissions/user@user.com', json={'phi-access': False}) + assert r.ok + + # Test that user w/o phi-access can delete tags + r = as_user.delete('/projects/' + project + '/tags/TagName') + assert r.ok + + # Test that user w/o phi can add notes + r = as_user.post('/projects/' + project + '/notes', json={'text': "Note"}) + assert r.ok + + # Test that user cannot see any notes (ideally user would be able to see only notes that they created) + r = as_admin.get("/projects/" + project) + assert r.ok + note = r.json()['notes'][0]["_id"] + r = as_user.get('/projects/' + project + '/notes/' + note) + assert r.status_code == 403 + + # Test that user with phi-access can see the notes + r = as_admin.put('/projects/' + project + '/permissions/user@user.com', json={'phi-access': True}) + assert r.ok + r = as_user.get('/projects/' + project + '/notes/' + note) + assert r.ok + r = as_admin.put('/projects/' + project + '/permissions/user@user.com', json={'phi-access': False}) + assert r.ok + + # Test that user w/o phi-access can delete notes + r = as_user.delete('/projects/' + project + '/notes/' + note) + assert r.ok + + r = as_admin.post('/projects/site/phi', json={"fields": []}) + assert r.ok + r = as_admin.delete('/sessions/' + session_2) + assert r.ok + r = as_admin.delete('/sessions/' + session_3) + assert r.ok def test_get_container(data_builder, default_payload, file_form, as_drone, as_admin, as_public, api_db): project = data_builder.create_project() @@ -467,7 +616,8 @@ def test_post_container(data_builder, as_admin, as_user): # create project w/ param inherit=true r = as_admin.post('/projects', params={'inherit': 'true'}, json={ 'group': group, - 'label': 'test-inheritance-project' + 'label': 'test-inheritance-project', + 'phi' : True }) assert r.ok project = r.json()['_id'] @@ -487,7 +637,8 @@ def test_post_container(data_builder, as_admin, as_user): # try to add project without admin on group r = as_user.post('/projects', json={ 'group': group, - 'label': 'test project post' + 'label': 'test project post', + 'phi' : True }) assert r.status_code == 403 diff --git a/tests/integration_tests/python/test_propagation.py b/tests/integration_tests/python/test_propagation.py index 994f6b595..c635413e5 100644 --- a/tests/integration_tests/python/test_propagation.py +++ b/tests/integration_tests/python/test_propagation.py @@ -213,7 +213,7 @@ def get_user_in_perms(perms, uid): assert r.ok # Add project without default group perms - r = as_admin.post('/projects', params={'inherit': 'false'}, json={'label': 'project2', 'group': group}) + r = as_admin.post('/projects', params={'inherit': 'false'}, json={'label': 'project2', 'group': group, 'phi' : True}) assert r.ok project2 = r.json()['_id'] diff --git a/tests/integration_tests/python/test_reports.py b/tests/integration_tests/python/test_reports.py index c8d70b7b2..6c90b2b97 100644 --- a/tests/integration_tests/python/test_reports.py +++ b/tests/integration_tests/python/test_reports.py @@ -288,7 +288,7 @@ def test_usage_report(data_builder, file_form, as_user, as_admin): # Test if empty project breaks Usage report group = data_builder.create_group() - r = as_admin.post('/projects', params={'inherit': 'false'}, json={'label': 'project2', 'group': group}) + r = as_admin.post('/projects', params={'inherit': 'false'}, json={'label': 'project2', 'group': group, 'phi' : True}) assert r.ok project = r.json()['_id'] diff --git a/tests/integration_tests/python/test_rules.py b/tests/integration_tests/python/test_rules.py index 78a6a8f1b..0856cd507 100644 --- a/tests/integration_tests/python/test_rules.py +++ b/tests/integration_tests/python/test_rules.py @@ -176,7 +176,8 @@ def test_site_rules_copied_to_new_projects(randstr, data_builder, file_form, as_ group = data_builder.create_group() r = as_admin.post('/projects', json={ 'group': group, - 'label': 'project_1' + 'label': 'project_1', + 'phi' : True }) assert r.ok project_id = r.json()['_id']