Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PXP-4102 Feat/arb admin #319

Merged
merged 9 commits into from
Mar 24, 2020
Merged
2 changes: 1 addition & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ flasgger==0.9.1
-e git+https://git@github.com/uc-cdis/cdisutils-test.git@1.0.0#egg=cdisutilstest
-e git+https://git@github.com/uc-cdis/indexd.git@2.3.0#egg=indexd
# indexd dependencies
authutils==3.1.1
authutils==4.0.0
gen3rbac==0.1.2
-e git+https://git@github.com/uc-cdis/doiclient.git@1.0.0#egg=doiclient
-e git+https://git@github.com/uc-cdis/dosclient.git@1.0.0#egg=dosclient
44 changes: 44 additions & 0 deletions sheepdog/auth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,50 @@ def authorize_and_call(program, project, *args, **kwargs):
return wrapper


def require_sheepdog_program_admin(func):
"""
Wrap a function to allow access to the handler if the user has access to
the resource /services/sheepdog/submission/program (Sheepdog program admin)
"""

@functools.wraps(func)
def authorize_and_call(*args, **kwargs):
jwt = get_jwt_from_header()
authz = flask.current_app.auth.auth_request(
jwt=jwt,
service="sheepdog",
methods="*",
resources=["/services/sheepdog/submission/program"],
)
if not authz:
raise AuthZError("Unauthorized: User must be Sheepdog program admin")
return func(*args, **kwargs)

return authorize_and_call


def require_sheepdog_project_admin(func):
"""
Wrap a function to allow access to the handler if the user has access to
the resource /services/sheepdog/submission/project (Sheepdog project admin)
"""

@functools.wraps(func)
def authorize_and_call(*args, **kwargs):
jwt = get_jwt_from_header()
authz = flask.current_app.auth.auth_request(
jwt=jwt,
service="sheepdog",
methods="*",
resources=["/services/sheepdog/submission/project"],
)
if not authz:
raise AuthZError("Unauthorized: User must be Sheepdog project admin")
return func(*args, **kwargs)

return authorize_and_call


def authorize(program, project, roles):
resource = "/programs/{}/projects/{}".format(program, project)
jwt = get_jwt_from_header()
Expand Down
2 changes: 1 addition & 1 deletion sheepdog/blueprint/routes/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def get_programs():
return flask.jsonify({"links": links})


@auth.require_sheepdog_program_admin
def root_create():
"""
Register a program.
Expand Down Expand Up @@ -111,7 +112,6 @@ def root_create():
"dbgap_accession_number": "phs000178"
}
"""
auth.current_user.require_admin()
message = None
node_id = None
doc = parse.parse_request_json()
Expand Down
4 changes: 2 additions & 2 deletions sheepdog/blueprint/routes/views/program/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def get_projects(program):


@utils.assert_program_exists
@auth.require_sheepdog_project_admin
def create_project(program):
"""
Register a project.
Expand Down Expand Up @@ -137,7 +138,6 @@ def create_project(program):
}
"""
res = None
auth.current_user.require_admin()
doc = utils.parse.parse_request_json()
if not isinstance(doc, dict):
raise UserError("Program endpoint only supports single documents")
Expand Down Expand Up @@ -217,6 +217,7 @@ def create_project(program):


@utils.assert_program_exists
@auth.require_sheepdog_program_admin
def delete_program(program):
"""
Delete a program given program name. If the program
Expand All @@ -237,7 +238,6 @@ def delete_program(program):
404: Program not found.
403: Unauthorized request.
"""
auth.current_user.require_admin()
with flask.current_app.db.session_scope() as session:
node = utils.lookup_program(flask.current_app.db, program)
if node.edges_in:
Expand Down
10 changes: 5 additions & 5 deletions sheepdog/blueprint/routes/views/program/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,10 @@ def create_files_viewer(dry_run=False, reassign=False):

@utils.assert_project_exists
@auth.authorize_for_project(*auth_roles)
# admin only
# TODO: check if we need these (pauline)
@auth.require_sheepdog_program_admin
@auth.require_sheepdog_project_admin
def file_operations(program, project, file_uuid):
"""
Handle molecular file operations. This will only be available once the
Expand Down Expand Up @@ -522,10 +526,6 @@ def file_operations(program, project, file_uuid):
: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.items() if v and k != "X-Auth-Token"
}
Expand Down Expand Up @@ -1102,6 +1102,7 @@ def update_entities_clinical_bcr(program, project):


@utils.assert_project_exists
@auth.require_sheepdog_project_admin
def delete_project(program, project):
"""
Delete project under a specific program
Expand All @@ -1122,7 +1123,6 @@ def delete_project(program, project):
404: Resource not found.
403: Unauthorized request.
"""
auth.current_user.require_admin()
with flask.current_app.db.session_scope() as session:
node = utils.lookup_project(flask.current_app.db, program, project)
if node.edges_in:
Expand Down
66 changes: 32 additions & 34 deletions tests/integration/datadict/submission/test_admin_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,63 +94,61 @@ def test_to_delete(
assert sur_node.sysan.get("to_delete") is to_delete


def do_reassign(client, headers):
"""Perform the http reassign action

Args:
client (pytest.Fixture): Allows you to mock http requests through flask
headers (dict): http headers with token
def test_reassign(
client, pg_driver, cgci_blgsp, index_client, submitter, require_index_exists_off
):
"""Try to reassign a node's remote URL

Returns:
requests.Response: http response from doing reassign request
string: did
string: s3 url that you changed it to
Url:
PUT: /admin/<program>/<project>/files/<file_uuid>/reassign
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this (updating URLs) should be supported in indexd, not sheepdog... and this code only supports S3 URLs, and it assumes there is only 1 stored URL 🤦‍♀ we should probably get rid of this and add an endpoint in indexd instead

cc @Avantol13 re:conversation about indexd not supporting URL updates

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wait until indexd has the endpoint or shall I go ahead and remove this~

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can wait to be safe, though I don't think it's actually used anywhere? but if we change the public API we need a new major version of sheepdog

data: {"s3_url": "s3://whatever/you/want"}
"""
# Does not test authz.

entities = post_blgsp_files(client, headers)
# Set up for http reassign action
entities = post_blgsp_files(client, submitter)
dids = entities["submitted_unaligned_reads"]
s3_url = "s3://whatever/you/want"

reassign_path = create_blgsp_url("/files/{}/reassign".format(dids[0]))
data = json.dumps({"s3_url": s3_url})
# http reassign action
resp = client.put(reassign_path, headers=submitter, data=data)

return client.put(reassign_path, headers=headers, data=data), dids[0], s3_url
assert resp.status_code == 200, resp.data
assert index_client.get(dids[0]), "Did not register with indexd?"
assert s3_url in index_client.get(dids[0]).urls, "Did not successfully reassign"


def test_reassign_with_admin(
def test_reassign_unauthorized(
client,
pg_driver,
cgci_blgsp,
submitter,
index_client,
admin,
require_index_exists_off,
mock_arborist_requests,
):
"""Try to reassign a node's remote URL

Url:
PUT: /admin/<program>/<project>/files/<file_uuid>/reassign
data: {"s3_url": "s3://whatever/you/want"}
"""
# Just checks that this is guarded with an Arborist auth request.
# (Does not check that the auth request is for the Sheepdog admin policy.)

resp, did, s3_url = do_reassign(client, admin)
assert resp.status_code == 200, resp.data
assert index_client.get(did), "Did not register with indexd?"
assert s3_url in index_client.get(did).urls, "Did not successfully reassign"


def test_reassign_without_admin(
client, pg_driver, cgci_blgsp, submitter, index_client, require_index_exists_off
):
"""Try to reassign a node's remote URL

Url:
PUT: /admin/<program>/<project>/files/<file_uuid>/reassign
data: {"s3_url": "s3://whatever/you/want"}
"""
# Set up for http reassign action
entities = post_blgsp_files(client, submitter)
dids = entities["submitted_unaligned_reads"]
s3_url = "s3://whatever/you/want"
reassign_path = create_blgsp_url("/files/{}/reassign".format(dids[0]))
data = json.dumps({"s3_url": s3_url})
# Mock arborist auth requests so they return false
mock_arborist_requests(authorized=False)
# http reassign action
resp = client.put(reassign_path, headers=submitter, data=data)

resp, did, s3_url = do_reassign(client, submitter)
assert resp.status_code == 403, resp.data
assert index_client.get(did), "Index should have been created."
assert len(index_client.get(did).urls) == 0, "No files have been uploaded"
assert s3_url not in index_client.get(did).urls, "Should not have reassigned"
assert index_client.get(dids[0]), "Index should have been created."
assert len(index_client.get(dids[0]).urls) == 0, "No files have been uploaded"
assert s3_url not in index_client.get(dids[0]).urls, "Should not have reassigned"
27 changes: 20 additions & 7 deletions tests/integration/datadict/submission/test_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,21 @@ def add_and_get_new_experimental_metadata_count(pg_driver):
return experimental_metadata_count


def test_program_creation_endpoint(client, pg_driver, admin):
resp = put_cgci(client, auth=admin)
def test_program_creation_endpoint(client, pg_driver, submitter):
# Does not test authz.
resp = put_cgci(client, auth=submitter)
assert resp.status_code == 200, resp.data
print(resp.data)
resp = client.get("/v0/submission/")
assert resp.json["links"] == ["/v0/submission/CGCI"], resp.json


def test_program_creation_without_admin_token(client, pg_driver, submitter):
def test_program_creation_unauthorized(
client, pg_driver, submitter, mock_arborist_requests
):
# Just checks that this is guarded with an Arborist auth request.
# (Does not check that the auth request is for the Sheepdog admin policy.)
mock_arborist_requests(authorized=False)
path = "/v0/submission/"
headers = submitter
data = json.dumps({"name": "CGCI", "type": "program"})
Expand All @@ -147,8 +153,9 @@ def test_program_creation_endpoint_for_program_not_supported(
assert resp.status_code == 404


def test_project_creation_endpoint(client, pg_driver, admin):
resp = put_cgci_blgsp(client, auth=admin)
def test_project_creation_endpoint(client, pg_driver, submitter):
# Does not test authz.
resp = put_cgci_blgsp(client, auth=submitter)
assert resp.status_code == 200
resp = client.get("/v0/submission/CGCI/")
with pg_driver.session_scope():
Expand All @@ -158,9 +165,15 @@ def test_project_creation_endpoint(client, pg_driver, admin):
assert resp.json["links"] == ["/v0/submission/CGCI/BLGSP"], resp.json


def test_project_creation_without_admin_token(client, pg_driver, submitter, admin):
put_cgci(client, admin)
def test_project_creation_unauthorized(
client, pg_driver, submitter, mock_arborist_requests
):
# Just checks that this is guarded with an Arborist auth request.
# (Does not check that the auth request is for the Sheepdog admin policy.)
put_cgci(client, submitter)
path = "/v0/submission/CGCI/"

mock_arborist_requests(authorized=False)
resp = client.put(
path,
headers=submitter,
Expand Down
Loading