From 6ceedb0cd4c9b9028e0f2fb4a8f6336da4b08f33 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 27 Aug 2019 09:49:48 -0500 Subject: [PATCH 01/16] feat(arborist): use auth client for auth check --- sheepdog/api.py | 6 ++- sheepdog/auth/__init__.py | 101 ++++---------------------------------- 2 files changed, 14 insertions(+), 93 deletions(-) diff --git a/sheepdog/api.py b/sheepdog/api.py index bcfae4ac..a41a2bd7 100644 --- a/sheepdog/api.py +++ b/sheepdog/api.py @@ -12,9 +12,8 @@ from cdispyutils.uwsgi import setup_user_harakiri from dictionaryutils import DataDictionary, dictionary from datamodelutils import models, validators, postgres_admin - - from indexclient.client import IndexClient +from gen3auth import ArboristClient import sheepdog from sheepdog.errors import ( @@ -149,6 +148,9 @@ def app_init(app): except KeyError: app.logger.error("Secret key not set in config! Authentication will not work") + # TODO: load config for base URL + app.auth = ArboristClient() + app = Flask(__name__) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index 9a332a33..cea5dbfe 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -9,94 +9,8 @@ import functools -from cdislogging import get_logger import flask -import authutils -from authutils import ROLES, dbgap -from authutils.user import AuthError, current_user, set_global_user -from authutils.token.validate import current_token -from cdiserrors import AuthZError - -from sheepdog import models - -LOGGER = get_logger("sheepdog_auth") - - -def _log_import_error(module_name): - """ - Log which module cannot be imported. - - Just in case this currently short list grows, make it a function. - """ - LOGGER.info("Unable to import %s, assuming it is not there", module_name) - - -# planx only modules (for now) - -# Separate try blocks in case one gets brought into gdc authutils. -# This is done with try blocks because when sheepdog.api imports -# sheepdog.auth you can't use flask.current_app. It hasn't been -# instantiated yet (application out of context error) - -try: - from authutils.token.validate import validate_request -except ImportError: - _log_import_error("validate_request") - - -def _role_error_msg(user_name, roles, project): - role_names = [role if role != "_member_" else "read (_member_)" for role in roles] - return "User {} doesn't have {} access in {}".format( - user_name, " or ".join(role_names), project - ) - - -def get_program_project_roles(program, project): - """ - A lot of places (submission entities etc.) confuse the terminology and have - a ``project_id`` attribute which is actually ``{program}-{project}``, so - in those places call this function like - - get_program_project_roles(*project_id.split('-', 1)) - - Args: - program (str): program name (NOT id) - project (str): project name (NOT id) - - Return: - Set[str]: roles - """ - if not hasattr(flask.g, "sheepdog_roles"): - flask.g.sheepdog_roles = dict() - - if not (program, project) in flask.g.sheepdog_roles: - user_roles = set() - with flask.current_app.db.session_scope(): - if program: - program_node = ( - flask.current_app.db.nodes(models.Program) - .props(name=program) - .scalar() - ) - if program_node: - program_id = program_node.dbgap_accession_number - roles = current_user.projects.get(program_id, set()) - user_roles.update(set(roles)) - if project: - project_node = ( - flask.current_app.db.nodes(models.Project) - .props(code=project) - .scalar() - ) - if project_node: - project_id = project_node.dbgap_accession_number - roles = current_user.projects.get(project_id, set()) - user_roles.update(set(roles)) - flask.g.sheepdog_roles[(program, project)] = user_roles - - return flask.g.sheepdog_roles[(program, project)] - def authorize_for_project(*required_roles): """ @@ -107,11 +21,16 @@ def authorize_for_project(*required_roles): def wrapper(func): @functools.wraps(func) def authorize_and_call(program, project, *args, **kwargs): - user_roles = get_program_project_roles(program, project) - if not user_roles & set(required_roles): - raise AuthZError( - _role_error_msg(current_user.username, required_roles, project) - ) + resource = "/programs/{}/projects/{}".format(program, project) + flask.current_app.auth.auth_request({ + "requests": [ + { + "resource": resource, + "action": {"service": "sheepdog", "method": role} + } + for role in required_roles + ] + }) return func(program, project, *args, **kwargs) return authorize_and_call From da18d94c9ebd2bfb50a2c64870fbb2c07ea72593 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 30 Aug 2019 09:09:36 -0500 Subject: [PATCH 02/16] feat(arborist): add real auth client --- requirements.txt | 4 ++++ sheepdog/api.py | 5 +++-- sheepdog/auth/__init__.py | 20 +++++++++++--------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/requirements.txt b/requirements.txt index 8d06f94e..fc20a8e4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -34,3 +34,7 @@ gen3dictionary==2.0.0 gen3datamodel==2.0.2 -e git+https://git@github.com/uc-cdis/indexclient.git@1.6.0#egg=indexclient -e git+https://git@github.com/uc-cdis/storage-client.git@0.1.7#egg=storageclient + +# FIXME: TEMPORARY +# REPLACE WITH gen3authz PACKAGE +git+https://github.com/uc-cdis/gen3authz.git@master#egg=gen3authz&subdirectory=python diff --git a/sheepdog/api.py b/sheepdog/api.py index a41a2bd7..f93cb67c 100644 --- a/sheepdog/api.py +++ b/sheepdog/api.py @@ -13,7 +13,8 @@ from dictionaryutils import DataDictionary, dictionary from datamodelutils import models, validators, postgres_admin from indexclient.client import IndexClient -from gen3auth import ArboristClient +from gen3authz.client.arborist.client import ArboristClient + import sheepdog from sheepdog.errors import ( @@ -148,7 +149,7 @@ def app_init(app): except KeyError: app.logger.error("Secret key not set in config! Authentication will not work") - # TODO: load config for base URL + # FIXME: load config for base URL app.auth = ArboristClient() diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index cea5dbfe..77125cc9 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -11,6 +11,8 @@ import flask +from sheepdog.errors import AuthError, AuthZError + def authorize_for_project(*required_roles): """ @@ -22,15 +24,15 @@ def wrapper(func): @functools.wraps(func) def authorize_and_call(program, project, *args, **kwargs): resource = "/programs/{}/projects/{}".format(program, project) - flask.current_app.auth.auth_request({ - "requests": [ - { - "resource": resource, - "action": {"service": "sheepdog", "method": role} - } - for role in required_roles - ] - }) + try: + jwt = flask.request.headers["Authorization"].split("Bearer ")[1] + except Exception: # this is the MVP, okay? + raise AuthError("didn't receive JWT correctly") + authz = flask.current_app.auth.auth_request( + jwt, "sheepdog", required_roles, [resource] + ) + if not authz: + raise AuthZError("user is unauthorized") return func(program, project, *args, **kwargs) return authorize_and_call From c11b467873affe54e9f3964cde7be263cc0a7448 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 30 Aug 2019 09:25:02 -0500 Subject: [PATCH 03/16] feat(arborist): fix imports --- sheepdog/auth/__init__.py | 14 ++++++ sheepdog/transactions/deletion/entity.py | 11 +++-- .../transactions/submission/transaction.py | 12 +++--- sheepdog/transactions/upload/entity.py | 43 ++++++------------- sheepdog/transactions/upload/transaction.py | 2 +- tests/conftest.py | 2 +- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index 77125cc9..d0b91fad 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -9,6 +9,7 @@ import functools +from authutils.token.validate import current_token import flask from sheepdog.errors import AuthError, AuthZError @@ -38,3 +39,16 @@ def authorize_and_call(program, project, *args, **kwargs): return authorize_and_call return wrapper + + +def authorize(program, project, roles): + resource = "/programs/{}/projects/{}".format(program, project) + try: + jwt = flask.request.headers["Authorization"].split("Bearer ")[1] + except Exception: # this is the MVP, okay? + raise AuthError("didn't receive JWT correctly") + authz = flask.current_app.auth.auth_request( + jwt, "sheepdog", roles, [resource] + ) + if not authz: + raise AuthZError("user is unauthorized") diff --git a/sheepdog/transactions/deletion/entity.py b/sheepdog/transactions/deletion/entity.py index eb95ba17..a3306919 100644 --- a/sheepdog/transactions/deletion/entity.py +++ b/sheepdog/transactions/deletion/entity.py @@ -1,4 +1,5 @@ -from sheepdog.auth import get_program_project_roles +from sheepdog.auth import authorize +from sheepdog.errors import AuthZError from sheepdog.globals import submitted_state, ALLOWED_DELETION_STATES from sheepdog.transactions.entity_base import EntityBase, EntityErrors from sheepdog.transactions.transaction_base import MissingNode @@ -22,9 +23,11 @@ def __init__(self, transaction, node): self.neighbors = (edge.src for edge in node.edges_in) # Check user permissions for deleting nodes - roles = get_program_project_roles(*self.transaction.project_id.split("-", 1)) - if "delete" not in roles: - self.record_error( + try: + program, project = self.transaction.project_id.split("-") + authorize(program, project, ["delete"]) + except AuthZError: + return self.record_error( "You do not have delete permission for project {}".format( self.transaction.project_id ) diff --git a/sheepdog/transactions/submission/transaction.py b/sheepdog/transactions/submission/transaction.py index fcbd93fc..cdb840d4 100644 --- a/sheepdog/transactions/submission/transaction.py +++ b/sheepdog/transactions/submission/transaction.py @@ -4,7 +4,8 @@ from flask import current_app as capp from sheepdog import utils -from sheepdog.auth import get_program_project_roles +from sheepdog.auth import authorize +from sheepdog.errors import AuthZError from sheepdog.globals import ( ENTITY_STATE_CATEGORIES, FLAG_IS_ASYNC, @@ -33,15 +34,16 @@ def __init__(self, smtp_conf=None, **kwargs): if utils.should_send_email(self.app_config): self.smtp_conf = smtp_conf - roles = get_program_project_roles(*self.project_id.split("-", 1)) - if ROLE_SUBMIT not in roles: - self.record_error( + try: + program, project = self.transaction.project_id.split("-") + authorize(program, project, [ROLE_SUBMIT]) + except AuthZError: + return self.record_error( "You do not have submit permission for project {}".format( self.project_id ), type=EntityErrors.INVALID_PERMISSIONS, ) - return self.project_node = utils.lookup_project( self.db_driver, self.program, self.project diff --git a/sheepdog/transactions/upload/entity.py b/sheepdog/transactions/upload/entity.py index fea52722..1b9d16ea 100644 --- a/sheepdog/transactions/upload/entity.py +++ b/sheepdog/transactions/upload/entity.py @@ -11,8 +11,8 @@ from sheepdog import dictionary from sheepdog import models -from sheepdog.auth import get_program_project_roles -from sheepdog.errors import InternalError +from sheepdog.auth import authorize +from sheepdog.errors import AuthZError, InternalError from sheepdog.globals import ( REGEX_UUID, UNVERIFIED_PROGRAM_NAMES, @@ -231,11 +231,13 @@ def get_node_create(self, skip_node_lookup=False): psqlgraph.Node """ # Check user permissions for updating nodes - roles = self.get_user_roles() - if "create" not in roles: + try: + program, project = self.transaction.project_id.split("-") + authorize(program, project, ["create"]) + except AuthZError: return self.record_error( - "You do not have create permission for project {} only {}".format( - self.transaction.project_id, roles + "You do not have create permission for project {}".format( + self.transaction.project_id ), type=EntityErrors.INVALID_PERMISSIONS, ) @@ -328,14 +330,16 @@ def get_node_merge(self): return None # Check user permissions for updating nodes - if "update" not in self.get_user_roles(): - self.record_error( + try: + program, project = self.transaction.project_id.split("-") + authorize(program, project, ["update"]) + except AuthZError: + return self.record_error( "You do not have update permission for project {}".format( self.transaction.project_id ), type=EntityErrors.INVALID_PERMISSIONS, ) - return None self.old_props = {k: v for k, v in node.props.iteritems()} @@ -616,27 +620,6 @@ def get_system_property_defaults(self): doc[key] = prop["default"] return doc - def get_user_roles(self): - return get_program_project_roles(*self.transaction.project_id.split("-", 1)) - - def is_case_creation_allowed(self, case_id): - """ - Check if case creation is allowed: - - #. Does the case exist in dbGaP? - #. Is the case in a predefined list of cases to allow? - #. Is the owning project in a predefined list of projects? - """ - program, project = self.transaction.project_id.split("-", 1) - if program in UNVERIFIED_PROGRAM_NAMES: - return True - elif project in UNVERIFIED_PROJECT_CODES: - return True - else: - return self.transaction.dbgap_x_referencer.case_exists( - program, project, self.doc.get("submitter_id") - ) - def set_association_proxies(self): """ Set all links on the actual node instance. diff --git a/sheepdog/transactions/upload/transaction.py b/sheepdog/transactions/upload/transaction.py index f9f25f73..911a1a06 100644 --- a/sheepdog/transactions/upload/transaction.py +++ b/sheepdog/transactions/upload/transaction.py @@ -6,12 +6,12 @@ from collections import Counter # Validating Entity Existence in dbGaP +from authutils import dbgap from datamodelutils import validators from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.attributes import flag_modified from gdcdictionary import gdcdictionary -from sheepdog.auth import dbgap from sheepdog import models from sheepdog import utils from sheepdog.errors import UserError, HandledIntegrityError diff --git a/tests/conftest.py b/tests/conftest.py index 4c853fe3..58c56fd8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,7 +2,7 @@ import pytest -from sheepdog.auth import ROLES +from sheepdog.globals import ROLES from sheepdog.test_settings import JWT_KEYPAIR_FILES from tests import utils From cdb69126e93fa70594d31c710e55991d31ae5bbf Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Fri, 30 Aug 2019 16:58:18 -0500 Subject: [PATCH 04/16] feat(arborist): fix header handling --- sheepdog/auth/__init__.py | 9 ++++++++- tests/conftest.py | 1 + tests/integration/datadict/conftest.py | 5 +++++ tests/integration/datadictwithobjid/conftest.py | 5 +++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index d0b91fad..eeb0e8e3 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -9,8 +9,10 @@ import functools +from authutils.user import current_user from authutils.token.validate import current_token import flask +import re from sheepdog.errors import AuthError, AuthZError @@ -26,7 +28,12 @@ def wrapper(func): def authorize_and_call(program, project, *args, **kwargs): resource = "/programs/{}/projects/{}".format(program, project) try: - jwt = flask.request.headers["Authorization"].split("Bearer ")[1] + auth_header = flask.request.headers["Authorization"] + if auth_header: + items = auth_header.split(" ") + if len(items) == 2 and items[0].lower() == "bearer": + jwt = items[1] + assert jwt except Exception: # this is the MVP, okay? raise AuthError("didn't receive JWT correctly") authz = flask.current_app.auth.auth_request( diff --git a/tests/conftest.py b/tests/conftest.py index 58c56fd8..3216dfbf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -100,6 +100,7 @@ def client(app): """ Overriding the `client` fixture from pytest_flask to fix this bug: https://github.com/pytest-dev/pytest-flask/issues/42 + Fixed in Flask 1.1.0 """ with app.test_client() as client: yield client diff --git a/tests/integration/datadict/conftest.py b/tests/integration/datadict/conftest.py index ab97a85d..e011418e 100644 --- a/tests/integration/datadict/conftest.py +++ b/tests/integration/datadict/conftest.py @@ -14,6 +14,7 @@ from cdispyutils.hmac4 import get_auth from dictionaryutils import DataDictionary, dictionary from datamodelutils import models, validators +from gen3authz.client.arborist.client import ArboristClient import sheepdog from sheepdog.test_settings import ( @@ -132,6 +133,10 @@ def teardown(): _app.jwt_public_keys = {_app.config['USER_API']: { 'key-test': utils.read_file('./integration/resources/keys/test_public_key.pem') }} + + # FIXME: load config for base URL + _app.auth = ArboristClient() + return _app diff --git a/tests/integration/datadictwithobjid/conftest.py b/tests/integration/datadictwithobjid/conftest.py index 8557b8b1..706a310c 100644 --- a/tests/integration/datadictwithobjid/conftest.py +++ b/tests/integration/datadictwithobjid/conftest.py @@ -15,6 +15,7 @@ from cdispyutils.hmac4 import get_auth from dictionaryutils import DataDictionary, dictionary from datamodelutils import models, validators +from gen3authz.client.arborist.client import ArboristClient import sheepdog @@ -130,6 +131,10 @@ def teardown(): ) } } + + # FIXME: load config for base URL + _app.auth = ArboristClient() + return _app From 3993ab0b15d9dacc44df2dbae6e9a4dbfc9be906 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 3 Sep 2019 15:26:02 -0500 Subject: [PATCH 05/16] feat(arborist): fix tests --- sheepdog/auth/__init__.py | 42 ++++++++------ .../blueprint/routes/views/program/project.py | 5 -- tests/conftest.py | 55 ++++++++++++++++--- .../datadict/submission/test_endpoints.py | 10 ++-- .../submission/test_endpoints.py | 18 +++--- tests/utils.py | 2 +- 6 files changed, 89 insertions(+), 43 deletions(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index eeb0e8e3..d11d80af 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -14,7 +14,22 @@ import flask import re -from sheepdog.errors import AuthError, AuthZError +from sheepdog.errors import AuthNError, AuthZError + + +def get_jwt_from_header(): + try: + jwt = None + auth_header = flask.request.headers["Authorization"] + if auth_header: + items = auth_header.split(" ") + if len(items) == 2 and items[0].lower() == "bearer": + jwt = items[1] + assert jwt, "Unable to parse header" + except Exception as e: # this is the MVP, okay? TODO better exception handling + print(e) + raise AuthNError("Didn't receive JWT correctly") + return jwt def authorize_for_project(*required_roles): @@ -27,17 +42,12 @@ def wrapper(func): @functools.wraps(func) def authorize_and_call(program, project, *args, **kwargs): resource = "/programs/{}/projects/{}".format(program, project) - try: - auth_header = flask.request.headers["Authorization"] - if auth_header: - items = auth_header.split(" ") - if len(items) == 2 and items[0].lower() == "bearer": - jwt = items[1] - assert jwt - except Exception: # this is the MVP, okay? - raise AuthError("didn't receive JWT correctly") + jwt = get_jwt_from_header() authz = flask.current_app.auth.auth_request( - jwt, "sheepdog", required_roles, [resource] + jwt=jwt, + service="sheepdog", + methods=required_roles, + resources=[resource] ) if not authz: raise AuthZError("user is unauthorized") @@ -50,12 +60,12 @@ def authorize_and_call(program, project, *args, **kwargs): def authorize(program, project, roles): resource = "/programs/{}/projects/{}".format(program, project) - try: - jwt = flask.request.headers["Authorization"].split("Bearer ")[1] - except Exception: # this is the MVP, okay? - raise AuthError("didn't receive JWT correctly") + jwt = get_jwt_from_header() authz = flask.current_app.auth.auth_request( - jwt, "sheepdog", roles, [resource] + jwt=jwt, + service="sheepdog", + methods=roles, + resources=[resource] ) if not authz: raise AuthZError("user is unauthorized") diff --git a/sheepdog/blueprint/routes/views/program/project.py b/sheepdog/blueprint/routes/views/program/project.py index c9b4ae92..199fdb73 100644 --- a/sheepdog/blueprint/routes/views/program/project.py +++ b/sheepdog/blueprint/routes/views/program/project.py @@ -551,11 +551,6 @@ def file_operations(program, project, file_uuid): raise UserError("Unsupported file operation", code=405) project_id = program + "-" + project - role = PERMISSIONS[action] - roles = auth.get_program_project_roles(*project_id.split("-", 1)) - if role not in roles: - raise AuthError("You don't have {} role to do '{}'".format(role, action)) - resp = utils.proxy_request( project_id, file_uuid, diff --git a/tests/conftest.py b/tests/conftest.py index 3216dfbf..8ae8d294 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,17 @@ import flask import pytest +import requests +# Python 2 and 3 compatible +try: + from unittest.mock import MagicMock + from unittest.mock import patch +except ImportError: + from mock import MagicMock + from mock import patch + +from sheepdog.errors import AuthZError from sheepdog.globals import ROLES from sheepdog.test_settings import JWT_KEYPAIR_FILES @@ -10,7 +20,6 @@ SUBMITTER_USERNAME = "submitter" ADMIN_USERNAME = "admin" -MEMBER_USERNAME = "member" @pytest.fixture(scope="session") @@ -88,13 +97,6 @@ def admin(create_user_header): return create_user_header(ADMIN_USERNAME, project_access, is_admin=True) -@pytest.fixture() -def member(create_user_header): - project_ids = ["phs000218", "phs000235", "phs000178"] - project_access = {project: ["_member"] for project in project_ids} - return create_user_header(MEMBER_USERNAME, project_access) - - @pytest.yield_fixture def client(app): """ @@ -111,3 +113,40 @@ def client(app): top.pop() else: break + + +@pytest.fixture(scope="function") +def mock_arborist_requests(request): + """ + This fixture returns a function which you call to mock the call to + arborist client's auth_request method. + By default, it returns a 200 response. If parameter "authorized" is set + to False, it raises a 401 error. + """ + + def do_patch(authorized=True): + def make_mock_response(): + def response(*args, **kwargs): + if not authorized: + raise AuthZError('Mocked Arborist says no') + mocked_response = MagicMock(requests.Response) + mocked_response.status_code = 200 + return mocked_response + return response + + mocked_get = MagicMock(side_effect=make_mock_response()) + patch_get = patch("gen3authz.client.arborist.client.ArboristClient.auth_request", mocked_get) + patch_get.start() + request.addfinalizer(patch_get.stop) + + return do_patch + + +@pytest.fixture(autouse=True) +def arborist_authorized(mock_arborist_requests): + """ + By default, mocked arborist calls return Authorized. + To mock an unauthorized response, use fixture + "mock_arborist_requests(authorized=False)" in the test itself + """ + mock_arborist_requests() diff --git a/tests/integration/datadict/submission/test_endpoints.py b/tests/integration/datadict/submission/test_endpoints.py index 0d8e7f3c..924eba71 100644 --- a/tests/integration/datadict/submission/test_endpoints.py +++ b/tests/integration/datadict/submission/test_endpoints.py @@ -69,7 +69,9 @@ def put_cgci(client, auth=None): def put_cgci_blgsp(client, auth=None): - put_cgci(client, auth=auth) + r = put_cgci(client, auth=auth) + assert r.status_code == 200, r.data + path = '/v0/submission/CGCI/' headers = auth data = json.dumps({ @@ -186,9 +188,9 @@ def test_unauthenticated_post(client, pg_driver, cgci_blgsp, submitter): assert resp.status_code == 401 -def test_unauthorized_post(client, pg_driver, cgci_blgsp, member): - # token only has _member_ role in CGCI - headers = member +def test_unauthorized_post(client, pg_driver, cgci_blgsp, submitter, mock_arborist_requests): + headers = submitter + mock_arborist_requests(authorized=False) resp = client.post( BLGSP_PATH, headers=headers, data=json.dumps({ "type": "experiment", diff --git a/tests/integration/datadictwithobjid/submission/test_endpoints.py b/tests/integration/datadictwithobjid/submission/test_endpoints.py index 3a3f78e5..8ea8c640 100644 --- a/tests/integration/datadictwithobjid/submission/test_endpoints.py +++ b/tests/integration/datadictwithobjid/submission/test_endpoints.py @@ -207,9 +207,9 @@ def test_unauthenticated_post(client, pg_driver, cgci_blgsp, submitter): assert resp.status_code == 401 -def test_unauthorized_post(client, pg_driver, cgci_blgsp, member): - # token only has _member_ role in CGCI - headers = member +def test_unauthorized_post(client, pg_driver, cgci_blgsp, submitter, mock_arborist_requests): + headers = submitter + mock_arborist_requests(authorized=False) resp = client.post( BLGSP_PATH, headers=headers, data=json.dumps({ "type": "experiment", @@ -679,12 +679,12 @@ def test_delete_non_empty_project(client, pg_driver, cgci_blgsp, submitter, admi assert resp.status_code == 400 -def test_delete_project_without_admin_token(client, pg_driver, cgci_blgsp, member): +def test_delete_project_without_admin_token(client, pg_driver, cgci_blgsp, submitter): """ Test that returns error when attemping to delete non-empty project """ path = '/v0/submission/CGCI/BLGSP' - resp = client.delete(path, headers=member) + resp = client.delete(path, headers=submitter) assert resp.status_code == 403 @@ -721,14 +721,14 @@ def test_delete_empty_non_program(client, pg_driver, cgci_blgsp, admin): assert resp.status_code == 400 -def test_delete_program_without_admin_token(client, pg_driver, admin, member): +def test_delete_program_without_admin_token(client, pg_driver, admin, submitter): """ Test that returns error since the client does not have privillege to delele the program """ path = '/v0/submission/CGCI' put_cgci(client, admin) - resp = client.delete(path, headers=member) + resp = client.delete(path, headers=submitter) assert resp.status_code == 403 @@ -748,7 +748,7 @@ def test_delete_program(client, pg_driver, admin): assert not program -def test_update_program_without_admin_token(client, pg_driver, admin, member): +def test_update_program_without_admin_token(client, pg_driver, admin, submitter): """ Test that returns authentication error since client does not have privilege to update the program @@ -758,7 +758,7 @@ def test_update_program_without_admin_token(client, pg_driver, admin, member): 'name': 'CGCI', 'type': 'program', 'dbgap_accession_number': 'phs000235_2' }) - resp = client.put('/v0/submission', headers=member, data=data) + resp = client.put('/v0/submission', headers=submitter, data=data) assert resp.status_code == 403 diff --git a/tests/utils.py b/tests/utils.py index b3000c51..0ec61c2c 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -119,6 +119,6 @@ def generate_signed_access_token( # Browser may clip cookies larger than 4096 bytes if len(token) > 4096: - raise JWTSizeError("JWT exceeded 4096 bytes") + raise Exception("JWT exceeded 4096 bytes") return JWTResult(token=token, kid=kid, claims=claims) From 7c6fa9b800928c2f3ee8fc8ba1edaf707c9a5e78 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 3 Sep 2019 15:42:21 -0500 Subject: [PATCH 06/16] feat(arborist): remove project access from mocked fence user --- tests/conftest.py | 12 +++--------- tests/utils.py | 1 - 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8ae8d294..17d11192 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,7 +12,6 @@ from mock import patch from sheepdog.errors import AuthZError -from sheepdog.globals import ROLES from sheepdog.test_settings import JWT_KEYPAIR_FILES from tests import utils @@ -56,7 +55,7 @@ def encoded_jwt_function(private_key, user): @pytest.fixture(scope="session") def create_user_header(encoded_jwt): - def create_user_header_function(username, project_access, **kwargs): + def create_user_header_function(username, **kwargs): private_key = utils.read_file( "./integration/resources/keys/test_private_key.pem" ) @@ -66,7 +65,6 @@ def create_user_header_function(username, project_access, **kwargs): "id": 1, "username": "submitter", "is_admin": False, - "project_access": project_access, "policies": [], "google_proxy_group_id": None, } @@ -80,9 +78,7 @@ def create_user_header_function(username, project_access, **kwargs): @pytest.fixture() def submitter(create_user_header): - project_ids = ["phs000218", "phs000235", "phs000178"] - project_access = {project: ROLES.values() for project in project_ids} - return create_user_header(SUBMITTER_USERNAME, project_access) + return create_user_header(SUBMITTER_USERNAME) @pytest.fixture() @@ -92,9 +88,7 @@ def submitter_name(): @pytest.fixture() def admin(create_user_header): - project_ids = ["phs000218", "phs000235", "phs000178"] - project_access = {project: ROLES.values() for project in project_ids} - return create_user_header(ADMIN_USERNAME, project_access, is_admin=True) + return create_user_header(ADMIN_USERNAME, is_admin=True) @pytest.yield_fixture diff --git a/tests/utils.py b/tests/utils.py index 0ec61c2c..6e054bdb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -98,7 +98,6 @@ def generate_signed_access_token( "user": { "name": user.username, "is_admin": user.is_admin, - "projects": dict(user.project_access), "google": {"proxy_group": user.google_proxy_group_id}, } }, From c534388e1c4c5d3e6942701a48d4f935bef8567f Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 3 Sep 2019 15:53:24 -0500 Subject: [PATCH 07/16] feat(arborist): jwt parsing error handling --- sheepdog/auth/__init__.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index d11d80af..56b81e79 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -18,16 +18,13 @@ def get_jwt_from_header(): - try: - jwt = None - auth_header = flask.request.headers["Authorization"] - if auth_header: - items = auth_header.split(" ") - if len(items) == 2 and items[0].lower() == "bearer": - jwt = items[1] - assert jwt, "Unable to parse header" - except Exception as e: # this is the MVP, okay? TODO better exception handling - print(e) + jwt = None + auth_header = flask.request.headers.get("Authorization") + if auth_header: + items = auth_header.split(" ") + if len(items) == 2 and items[0].lower() == "bearer": + jwt = items[1] + if not jwt: raise AuthNError("Didn't receive JWT correctly") return jwt From e9a5a8652b38d81276217da3909c229da61194da Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 3 Sep 2019 16:13:41 -0500 Subject: [PATCH 08/16] feat(arborist): base URL config --- sheepdog/api.py | 7 +++++-- sheepdog/dev_settings.py | 2 ++ sheepdog/test_settings.py | 3 +++ tests/integration/datadict/conftest.py | 1 - tests/integration/datadictwithobjid/conftest.py | 1 - 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sheepdog/api.py b/sheepdog/api.py index f93cb67c..65c3a00b 100644 --- a/sheepdog/api.py +++ b/sheepdog/api.py @@ -149,8 +149,11 @@ def app_init(app): except KeyError: app.logger.error("Secret key not set in config! Authentication will not work") - # FIXME: load config for base URL - app.auth = ArboristClient() + if app.config.get("ARBORIST"): + app.auth = ArboristClient(arborist_base_url=app.config["ARBORIST"]) + else: + app.logger.info("Using default Arborist base URL") + app.auth = ArboristClient() app = Flask(__name__) diff --git a/sheepdog/dev_settings.py b/sheepdog/dev_settings.py index bea3eef7..6611bc6e 100644 --- a/sheepdog/dev_settings.py +++ b/sheepdog/dev_settings.py @@ -21,6 +21,8 @@ "user_domain_name": env.get("KEYSTONE_DOMAIN"), } +ARBORIST = "http://arborist-service/" + # Storage CLEVERSAFE_HOST = env.get("CLEVERSAFE_HOST", "cleversafe.service.consul") diff --git a/sheepdog/test_settings.py b/sheepdog/test_settings.py index bd718c99..b5b76146 100644 --- a/sheepdog/test_settings.py +++ b/sheepdog/test_settings.py @@ -15,6 +15,9 @@ "auth_url": "https://fake_auth_url", "user_domain_name": "some_domain", } + +ARBORIST = "http://arborist-service/" + SUBMISSION = {"bucket": "test_submission", "host": "host"} diff --git a/tests/integration/datadict/conftest.py b/tests/integration/datadict/conftest.py index e011418e..9fc38dac 100644 --- a/tests/integration/datadict/conftest.py +++ b/tests/integration/datadict/conftest.py @@ -134,7 +134,6 @@ def teardown(): 'key-test': utils.read_file('./integration/resources/keys/test_public_key.pem') }} - # FIXME: load config for base URL _app.auth = ArboristClient() return _app diff --git a/tests/integration/datadictwithobjid/conftest.py b/tests/integration/datadictwithobjid/conftest.py index 706a310c..60559758 100644 --- a/tests/integration/datadictwithobjid/conftest.py +++ b/tests/integration/datadictwithobjid/conftest.py @@ -132,7 +132,6 @@ def teardown(): } } - # FIXME: load config for base URL _app.auth = ArboristClient() return _app From bf58eddc5ca48a875c6061fb5b9d6706583e4de9 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Wed, 4 Sep 2019 16:20:51 -0500 Subject: [PATCH 09/16] gen3authz fix pin --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index fc20a8e4..6d3a5c38 100644 --- a/requirements.txt +++ b/requirements.txt @@ -37,4 +37,4 @@ gen3datamodel==2.0.2 # FIXME: TEMPORARY # REPLACE WITH gen3authz PACKAGE -git+https://github.com/uc-cdis/gen3authz.git@master#egg=gen3authz&subdirectory=python +git+https://github.com/uc-cdis/gen3authz.git@fix/json#egg=gen3authz&subdirectory=python From 23147deaedde0a3af0de107a057cc484b3d3f396 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Thu, 5 Sep 2019 14:19:42 -0500 Subject: [PATCH 10/16] fix(project_id): handle dash in project name --- sheepdog/transactions/deletion/entity.py | 2 +- sheepdog/transactions/submission/transaction.py | 2 +- sheepdog/transactions/upload/entity.py | 4 ++-- sheepdog/utils/__init__.py | 3 +-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sheepdog/transactions/deletion/entity.py b/sheepdog/transactions/deletion/entity.py index a3306919..9cc147a4 100644 --- a/sheepdog/transactions/deletion/entity.py +++ b/sheepdog/transactions/deletion/entity.py @@ -24,7 +24,7 @@ def __init__(self, transaction, node): # Check user permissions for deleting nodes try: - program, project = self.transaction.project_id.split("-") + program, project = self.transaction.project_id.split("-", 1) authorize(program, project, ["delete"]) except AuthZError: return self.record_error( diff --git a/sheepdog/transactions/submission/transaction.py b/sheepdog/transactions/submission/transaction.py index cdb840d4..2be6893c 100644 --- a/sheepdog/transactions/submission/transaction.py +++ b/sheepdog/transactions/submission/transaction.py @@ -35,7 +35,7 @@ def __init__(self, smtp_conf=None, **kwargs): self.smtp_conf = smtp_conf try: - program, project = self.transaction.project_id.split("-") + program, project = self.transaction.project_id.split("-", 1) authorize(program, project, [ROLE_SUBMIT]) except AuthZError: return self.record_error( diff --git a/sheepdog/transactions/upload/entity.py b/sheepdog/transactions/upload/entity.py index 1b9d16ea..9da03d60 100644 --- a/sheepdog/transactions/upload/entity.py +++ b/sheepdog/transactions/upload/entity.py @@ -232,7 +232,7 @@ def get_node_create(self, skip_node_lookup=False): """ # Check user permissions for updating nodes try: - program, project = self.transaction.project_id.split("-") + program, project = self.transaction.project_id.split("-", 1) authorize(program, project, ["create"]) except AuthZError: return self.record_error( @@ -331,7 +331,7 @@ def get_node_merge(self): # Check user permissions for updating nodes try: - program, project = self.transaction.project_id.split("-") + program, project = self.transaction.project_id.split("-", 1) authorize(program, project, ["update"]) except AuthZError: return self.record_error( diff --git a/sheepdog/utils/__init__.py b/sheepdog/utils/__init__.py index 33417a7f..ea01862f 100644 --- a/sheepdog/utils/__init__.py +++ b/sheepdog/utils/__init__.py @@ -177,8 +177,7 @@ def create_entity_list(nodes): props["id"] = node.node_id props["type"] = node.label if hasattr(node, "project_id"): - program = node.project_id.split("-")[0] - project = "-".join(node.project_id.split("-")[1:]) + program, project = node.project_id.split("-", 1) else: program, project = None, None for link_name in node._pg_links: # pylint: disable=W0212 From f8842fb321aeb6433914bff70971492f852a4206 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Thu, 5 Sep 2019 16:08:07 -0500 Subject: [PATCH 11/16] feat(arborist): deprecate sheepdog admin role --- requirements.txt | 2 +- sheepdog/blueprint/routes/views/program/project.py | 13 ++++++------- sheepdog/globals.py | 1 - 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6d3a5c38..fc20a8e4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -37,4 +37,4 @@ gen3datamodel==2.0.2 # FIXME: TEMPORARY # REPLACE WITH gen3authz PACKAGE -git+https://github.com/uc-cdis/gen3authz.git@fix/json#egg=gen3authz&subdirectory=python +git+https://github.com/uc-cdis/gen3authz.git@master#egg=gen3authz&subdirectory=python diff --git a/sheepdog/blueprint/routes/views/program/project.py b/sheepdog/blueprint/routes/views/program/project.py index 199fdb73..8d7a7307 100644 --- a/sheepdog/blueprint/routes/views/program/project.py +++ b/sheepdog/blueprint/routes/views/program/project.py @@ -285,7 +285,7 @@ def create_delete_entities_viewer(dry_run=False): """ @utils.assert_project_exists - @auth.authorize_for_project(ROLES["DELETE"], ROLES["ADMIN"]) + @auth.authorize_for_project(ROLES["DELETE"]) def delete_entities(program, project, ids, to_delete=None): """ Delete existing GDC entities. @@ -330,9 +330,6 @@ def delete_entities(program, project, ids, to_delete=None): fields = flask.request.args.get("fields") if to_delete is not None: - # to_delete is admin only - auth.current_user.require_admin() - # get value of that flag from string if to_delete.lower() == "false": to_delete = False @@ -453,7 +450,6 @@ def create_files_viewer(dry_run=False, reassign=False): ROLES["DELETE"], ROLES["DOWNLOAD"], ROLES["READ"], - ROLES["ADMIN"], ] @utils.assert_project_exists @@ -514,6 +510,11 @@ def file_operations(program, project, file_uuid): :reqheader X-Auth-Token: |reqheader_X-Auth-Token| :resheader Content-Type: |resheader_Content-Type| """ + + # admin only + # TODO: check if we need these (pauline) + auth.current_user.require_admin() + headers = { k: v for k, v in flask.request.headers.iteritems() @@ -535,8 +536,6 @@ def file_operations(program, project, file_uuid): action = "upload" elif flask.request.method == "PUT": if reassign: - # admin only - auth.current_user.require_admin() action = "reassign" elif flask.request.args.get("partNumber"): action = "upload_part" diff --git a/sheepdog/globals.py b/sheepdog/globals.py index ac5e1c03..661296b5 100644 --- a/sheepdog/globals.py +++ b/sheepdog/globals.py @@ -22,7 +22,6 @@ SUPPORTED_FORMATS = ["csv", "tsv", "json"] ROLES = { - "ADMIN": "admin", "CREATE": "create", "DELETE": "delete", "DOWNLOAD": "download", From e87e16b49045fcc0ea59fbdcca8a674ea2d8f851 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Thu, 5 Sep 2019 17:17:12 -0500 Subject: [PATCH 12/16] feat(arborist): fix delete unit test --- .../submission/test_admin_endpoints.py | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/tests/integration/datadict/submission/test_admin_endpoints.py b/tests/integration/datadict/submission/test_admin_endpoints.py index 8948f856..1559708d 100644 --- a/tests/integration/datadict/submission/test_admin_endpoints.py +++ b/tests/integration/datadict/submission/test_admin_endpoints.py @@ -49,7 +49,7 @@ def post_blgsp_files(client, headers): @pytest.mark.parametrize( "headers,status_code,to_delete", - [("submitter", 403, None), ("admin", 200, True), ("admin", 200, False)], + [("admin", 200, True), ("admin", 200, False)], ) def test_to_delete( headers, @@ -62,7 +62,7 @@ def test_to_delete( submitter, require_index_exists_off, ): - """Try to set the sysan of a node with admin credentials + """Try to set the sysan of a node with delete access Url: DELETE: /admin///entities//to_delete/ @@ -85,6 +85,40 @@ def test_to_delete( assert sur_node.sysan.get("to_delete") is to_delete +def test_to_delete_unauthorized( + request, + client, + pg_driver, + cgci_blgsp, + submitter, + require_index_exists_off, + mock_arborist_requests, +): + """Try to set the sysan of a node without delete access + + Url: + DELETE: /admin///entities//to_delete/ + """ + + headers = submitter + + # submit files as submitter + entities = post_blgsp_files(client, submitter) + did = entities["submitted_unaligned_reads"] + + to_delete = True + base_delete_path = create_blgsp_url("/entities/{}/to_delete/".format(did)) + to_delete_path = base_delete_path + str(to_delete).lower() + + mock_arborist_requests(authorized=False) + resp = client.delete(to_delete_path, headers=headers) + assert resp.status_code == 403, resp.data + with pg_driver.session_scope(): + sur_node = pg_driver.nodes(md.SubmittedUnalignedReads).first() + assert sur_node + assert not sur_node.sysan.get("to_delete") + + def do_reassign(client, headers): """Perform the http reassign action From d7ece5413d62db08d6097563ada355d0318226b5 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 10 Sep 2019 10:27:52 -0500 Subject: [PATCH 13/16] feat(arborist): auto program/project resource creation --- sheepdog/auth/__init__.py | 23 +++++++++++++++++++ sheepdog/blueprint/routes/views/__init__.py | 4 ++++ .../routes/views/program/__init__.py | 8 ++++++- tests/conftest.py | 19 +++++++++++---- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index 56b81e79..662d7235 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -11,12 +11,16 @@ from authutils.user import current_user from authutils.token.validate import current_token +from cdislogging import get_logger import flask import re from sheepdog.errors import AuthNError, AuthZError +logger = get_logger(__name__) + + def get_jwt_from_header(): jwt = None auth_header = flask.request.headers.get("Authorization") @@ -66,3 +70,22 @@ def authorize(program, project, roles): ) if not authz: raise AuthZError("user is unauthorized") + + +def create_resource(program, project=None): + resource = "/programs/{}".format(program) + if project: + resource += "/projects/{}".format(project) + logger.info("Creating arborist resource {}".format(resource)) + + json_data = { + "name": resource, + "description": "Created by sheepdog" # TODO use authz provider field + } + resp = flask.current_app.auth.create_resource( + parent_path="", + resource_json=json_data, + create_parents=True + ) + if resp.get("error"): + logger.error("Unable to create resource: code {} - {}".format(resp.error.code, resp.error.message)) diff --git a/sheepdog/blueprint/routes/views/__init__.py b/sheepdog/blueprint/routes/views/__init__.py index c85339cd..5ce548aa 100644 --- a/sheepdog/blueprint/routes/views/__init__.py +++ b/sheepdog/blueprint/routes/views/__init__.py @@ -124,6 +124,7 @@ def root_create(): if not program: raise UserError("No program specified in key 'name'") + # create the resource in sheepdog DB with current_app.db.session_scope(can_inherit=False) as session: node = current_app.db.nodes(models.Program).props(name=program).scalar() if node: @@ -139,6 +140,9 @@ def root_create(): ) message = "Program registered." + # create the resource in arborist + auth.create_resource(phsid) + return flask.jsonify({"id": node_id, "name": program, "message": message}) diff --git a/sheepdog/blueprint/routes/views/program/__init__.py b/sheepdog/blueprint/routes/views/program/__init__.py index 769fff1d..4bb4e51c 100644 --- a/sheepdog/blueprint/routes/views/program/__init__.py +++ b/sheepdog/blueprint/routes/views/program/__init__.py @@ -136,6 +136,7 @@ def create_project(program): "state": "active" } """ + res = None auth.current_user.require_admin() doc = utils.parse.parse_request_json() if not isinstance(doc, dict): @@ -208,7 +209,12 @@ def create_project(program): entity.node = node entity.entity_id = entity.node.node_id trans.entities = [entity] - return flask.jsonify(trans.json) + res = flask.jsonify(trans.json) + + # create the resource in arborist + auth.create_resource(program_node.dbgap_accession_number, phsid) + + return res @utils.assert_program_exists diff --git a/tests/conftest.py b/tests/conftest.py index 17d11192..3713b921 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -125,13 +125,24 @@ def response(*args, **kwargs): raise AuthZError('Mocked Arborist says no') mocked_response = MagicMock(requests.Response) mocked_response.status_code = 200 + + def mocked_get(*args, **kwargs): + return None + mocked_response.get = mocked_get + return mocked_response return response - mocked_get = MagicMock(side_effect=make_mock_response()) - patch_get = patch("gen3authz.client.arborist.client.ArboristClient.auth_request", mocked_get) - patch_get.start() - request.addfinalizer(patch_get.stop) + mocked_auth_request = MagicMock(side_effect=make_mock_response()) + + patch_auth_request = patch("gen3authz.client.arborist.client.ArboristClient.auth_request", mocked_auth_request) + patch_create_resource = patch("gen3authz.client.arborist.client.ArboristClient.create_resource", mocked_auth_request) + + patch_auth_request.start() + patch_create_resource.start() + + request.addfinalizer(patch_auth_request.stop) + request.addfinalizer(patch_create_resource.stop) return do_patch From ef27fa74d219f8dd94b78a8a5b44a6d0c7d4975d Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 10 Sep 2019 10:53:35 -0500 Subject: [PATCH 14/16] feat(arborist): refactor mocked arborist response + use gen3authz package --- requirements.txt | 5 +---- tests/conftest.py | 22 ++++++++++------------ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/requirements.txt b/requirements.txt index fc20a8e4..bc36847f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,7 @@ Flask==0.12.4 flask-cors==3.0.3 Flask-SQLAlchemy-Session==1.1 fuzzywuzzy==0.6.1 +gen3authz==0.2.1 graphene==2.0.1 rx==1.6.1 jsonschema==2.5.1 @@ -34,7 +35,3 @@ gen3dictionary==2.0.0 gen3datamodel==2.0.2 -e git+https://git@github.com/uc-cdis/indexclient.git@1.6.0#egg=indexclient -e git+https://git@github.com/uc-cdis/storage-client.git@0.1.7#egg=storageclient - -# FIXME: TEMPORARY -# REPLACE WITH gen3authz PACKAGE -git+https://github.com/uc-cdis/gen3authz.git@master#egg=gen3authz&subdirectory=python diff --git a/tests/conftest.py b/tests/conftest.py index 3713b921..8ea2bc33 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -119,21 +119,19 @@ def mock_arborist_requests(request): """ def do_patch(authorized=True): - def make_mock_response(): - def response(*args, **kwargs): - if not authorized: - raise AuthZError('Mocked Arborist says no') - mocked_response = MagicMock(requests.Response) - mocked_response.status_code = 200 + def make_mock_response(*args, **kwargs): + if not authorized: + raise AuthZError('Mocked Arborist says no') + mocked_response = MagicMock(requests.Response) + mocked_response.status_code = 200 - def mocked_get(*args, **kwargs): - return None - mocked_response.get = mocked_get + def mocked_get(*args, **kwargs): + return None + mocked_response.get = mocked_get - return mocked_response - return response + return mocked_response - mocked_auth_request = MagicMock(side_effect=make_mock_response()) + mocked_auth_request = MagicMock(side_effect=make_mock_response) patch_auth_request = patch("gen3authz.client.arborist.client.ArboristClient.auth_request", mocked_auth_request) patch_create_resource = patch("gen3authz.client.arborist.client.ArboristClient.create_resource", mocked_auth_request) From 53b3a94d4e33c1d9daa23694b292678975b046d1 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Tue, 10 Sep 2019 15:17:37 -0500 Subject: [PATCH 15/16] feat(arborist): refactor test_delete --- sheepdog/auth/__init__.py | 1 - .../submission/test_admin_endpoints.py | 42 +++---------------- 2 files changed, 6 insertions(+), 37 deletions(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index 662d7235..48ad8d5d 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -13,7 +13,6 @@ from authutils.token.validate import current_token from cdislogging import get_logger import flask -import re from sheepdog.errors import AuthNError, AuthZError diff --git a/tests/integration/datadict/submission/test_admin_endpoints.py b/tests/integration/datadict/submission/test_admin_endpoints.py index 1559708d..090d448e 100644 --- a/tests/integration/datadict/submission/test_admin_endpoints.py +++ b/tests/integration/datadict/submission/test_admin_endpoints.py @@ -49,7 +49,7 @@ def post_blgsp_files(client, headers): @pytest.mark.parametrize( "headers,status_code,to_delete", - [("admin", 200, True), ("admin", 200, False)], + [("submitter", 403, None), ("admin", 200, True), ("admin", 200, False)], ) def test_to_delete( headers, @@ -61,8 +61,9 @@ def test_to_delete( cgci_blgsp, submitter, require_index_exists_off, + mock_arborist_requests, ): - """Try to set the sysan of a node with delete access + """Try to set the sysan of a node with and without delete access Url: DELETE: /admin///entities//to_delete/ @@ -77,6 +78,9 @@ def test_to_delete( base_delete_path = create_blgsp_url("/entities/{}/to_delete/".format(did)) to_delete_path = base_delete_path + str(to_delete).lower() + if status_code != 200: + mock_arborist_requests(authorized=False) + resp = client.delete(to_delete_path, headers=headers) assert resp.status_code == status_code, resp.data with pg_driver.session_scope(): @@ -85,40 +89,6 @@ def test_to_delete( assert sur_node.sysan.get("to_delete") is to_delete -def test_to_delete_unauthorized( - request, - client, - pg_driver, - cgci_blgsp, - submitter, - require_index_exists_off, - mock_arborist_requests, -): - """Try to set the sysan of a node without delete access - - Url: - DELETE: /admin///entities//to_delete/ - """ - - headers = submitter - - # submit files as submitter - entities = post_blgsp_files(client, submitter) - did = entities["submitted_unaligned_reads"] - - to_delete = True - base_delete_path = create_blgsp_url("/entities/{}/to_delete/".format(did)) - to_delete_path = base_delete_path + str(to_delete).lower() - - mock_arborist_requests(authorized=False) - resp = client.delete(to_delete_path, headers=headers) - assert resp.status_code == 403, resp.data - with pg_driver.session_scope(): - sur_node = pg_driver.nodes(md.SubmittedUnalignedReads).first() - assert sur_node - assert not sur_node.sysan.get("to_delete") - - def do_reassign(client, headers): """Perform the http reassign action From 268ac8a208e6a28f40201051da4ff3cd9d72caa9 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre Date: Wed, 11 Sep 2019 13:42:36 -0500 Subject: [PATCH 16/16] feat(arborist): fix- when program/project already exists, response is empty --- sheepdog/auth/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sheepdog/auth/__init__.py b/sheepdog/auth/__init__.py index 48ad8d5d..a2dfb782 100644 --- a/sheepdog/auth/__init__.py +++ b/sheepdog/auth/__init__.py @@ -86,5 +86,5 @@ def create_resource(program, project=None): resource_json=json_data, create_parents=True ) - if resp.get("error"): + if resp and resp.get("error"): logger.error("Unable to create resource: code {} - {}".format(resp.error.code, resp.error.message))