From c65eac62a61a932e565e132827f228a06204ca3b Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Wed, 2 Oct 2024 16:59:39 +1000 Subject: [PATCH 1/7] Add router functions to check whether users have authorization in experiment, experiment set and score set. Add some related tests. --- src/mavedb/routers/experiment_sets.py | 37 ++++++++++++++++++++++ src/mavedb/routers/experiments.py | 35 +++++++++++++++++++++ src/mavedb/routers/score_sets.py | 34 ++++++++++++++++++++ tests/routers/test_experiment_set.py | 45 +++++++++++++++++++++++++++ tests/routers/test_experiments.py | 35 +++++++++++++++++++++ tests/routers/test_score_set.py | 38 ++++++++++++++++++++++ 6 files changed, 224 insertions(+) create mode 100644 tests/routers/test_experiment_set.py diff --git a/src/mavedb/routers/experiment_sets.py b/src/mavedb/routers/experiment_sets.py index a5060ec9..4a797c9a 100644 --- a/src/mavedb/routers/experiment_sets.py +++ b/src/mavedb/routers/experiment_sets.py @@ -4,10 +4,13 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session +from sqlalchemy import or_ from mavedb import deps +from mavedb.lib.authentication import get_current_user, UserData from mavedb.lib.logging import LoggedRoute from mavedb.lib.logging.context import logging_context, save_to_logging_context +from mavedb.models.contributor import Contributor from mavedb.models.experiment_set import ExperimentSet from mavedb.view_models import experiment_set @@ -21,6 +24,40 @@ logger = logging.getLogger(__name__) +@router.get( + "/check-authorizations/{urn}", + status_code=200, + response_model=bool +) +async def check_experiment_set_authorization( + *, + urn: str, + db: Session = Depends(deps.get_db), + user_data: UserData = Depends(get_current_user), +) -> bool: + """ + Check whether users have authorizations in this experiment set. + """ + query = db.query(ExperimentSet).filter(ExperimentSet.urn == urn) + + if user_data is not None: + query = query.filter( + or_( + ExperimentSet.created_by_id == user_data.user.id, + ExperimentSet.contributors.any(Contributor.orcid_id == user_data.user.username), + ) + ) + else: + return False + + save_to_logging_context({"Experiment set requested resource": urn}) + item = query.first() + if item: + return True + else: + return False + + @router.get( "/{urn}", status_code=200, diff --git a/src/mavedb/routers/experiments.py b/src/mavedb/routers/experiments.py index 08237c7e..fbd625e4 100644 --- a/src/mavedb/routers/experiments.py +++ b/src/mavedb/routers/experiments.py @@ -45,6 +45,41 @@ ) +@router.get( + "/experiments/check-authorizations/{urn}", + status_code=200, + response_model=bool +) +async def check_experiment_authorization( + *, + urn: str, + db: Session = Depends(deps.get_db), + user_data: UserData = Depends(get_current_user), +) -> bool: + """ + Check whether users have authorizations in this experiment. + """ + query = db.query(Experiment).filter(Experiment.urn == urn) + + if user_data is not None: + query = query.filter( + or_( + Experiment.created_by_id == user_data.user.id, + Experiment.contributors.any(Contributor.orcid_id == user_data.user.username), + ) + ) + else: + return False + + save_to_logging_context({"Experiment requested resource": urn}) + item = query.first() + + if item: + return True + else: + return False + + # TODO: Rewrite this function. @router.get( "/experiments/", diff --git a/src/mavedb/routers/score_sets.py b/src/mavedb/routers/score_sets.py index 44fd2a30..dcf74f45 100644 --- a/src/mavedb/routers/score_sets.py +++ b/src/mavedb/routers/score_sets.py @@ -113,6 +113,40 @@ async def fetch_score_set_by_urn( ) +@router.get( + "/score-sets/check-authorizations/{urn}", + status_code=200, + response_model=bool +) +async def check_score_set_authorization( + *, + urn: str, + db: Session = Depends(deps.get_db), + user_data: UserData = Depends(get_current_user), +) -> bool: + """ + Check whether users have authorizations in this score set. + """ + query = db.query(ScoreSet).filter(ScoreSet.urn == urn) + + if user_data is not None: + query = query.filter( + or_( + ScoreSet.created_by_id == user_data.user.id, + ScoreSet.contributors.any(Contributor.orcid_id == user_data.user.username), + ) + ) + else: + return False + + save_to_logging_context({"Score set requested resource": urn}) + item = query.first() + if item: + return True + else: + return False + + @router.post("/score-sets/search", status_code=200, response_model=list[score_set.ShortScoreSet]) def search_score_sets(search: ScoreSetsSearch, db: Session = Depends(deps.get_db)) -> Any: # = Body(..., embed=True), """ diff --git a/tests/routers/test_experiment_set.py b/tests/routers/test_experiment_set.py new file mode 100644 index 00000000..734e42ec --- /dev/null +++ b/tests/routers/test_experiment_set.py @@ -0,0 +1,45 @@ +from tests.helpers.constants import TEST_USER +from tests.helpers.util import ( + add_contributor, + change_ownership, + create_experiment, +) +from mavedb.models.experiment import Experiment as ExperimentDbModel +from mavedb.models.experiment_set import ExperimentSet as ExperimentSetDbModel + + +def test_get_true_authorization_from_own_experiment_set_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) + add_contributor( + session, + experiment["experimentSetUrn"], + ExperimentSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_experiment_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) + + response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") + + assert response.status_code == 200 + assert response.json() == False \ No newline at end of file diff --git a/tests/routers/test_experiments.py b/tests/routers/test_experiments.py index b04d1625..c8e362af 100644 --- a/tests/routers/test_experiments.py +++ b/tests/routers/test_experiments.py @@ -559,6 +559,41 @@ def test_admin_can_update_other_users_public_experiment_set( assert response_data["title"] == "Second Experiment" +def test_get_true_authorization_from_own_experiment_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + add_contributor( + session, + experiment["urn"], + ExperimentDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_experiment_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + + response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") + + assert response.status_code == 200 + assert response.json() == False + + def test_edit_preserves_optional_metadata(client, setup_router_db): pass diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index e0db83c2..4ec8fd1d 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -496,6 +496,44 @@ def test_admin_can_add_scores_and_counts_to_other_user_score_set(session, client assert score_set == response_data +def test_get_true_authorization_from_own_score_set_check(client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_score_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + add_contributor( + session, + score_set["urn"], + ScoreSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_score_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + + response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") + + assert response.status_code == 200 + assert response.json() == False + + def test_publish_score_set(session, data_provider, client, setup_router_db, data_files): experiment = create_experiment(client) score_set = create_seq_score_set_with_variants( From 280edd4149b6f0f40f9a557376f1581f4ef625e9 Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Fri, 4 Oct 2024 16:33:45 +1000 Subject: [PATCH 2/7] Modify original check authorization function. Based on this new function, add more tests and move original tests to one file. --- src/mavedb/routers/authorization.py | 71 ++++++ src/mavedb/routers/experiment_sets.py | 36 --- src/mavedb/routers/experiments.py | 36 --- src/mavedb/routers/score_sets.py | 34 --- src/mavedb/server_main.py | 2 + tests/routers/test_authorization.py | 331 ++++++++++++++++++++++++++ tests/routers/test_experiment_set.py | 45 ---- tests/routers/test_experiments.py | 35 --- tests/routers/test_score_set.py | 38 --- 9 files changed, 404 insertions(+), 224 deletions(-) create mode 100644 src/mavedb/routers/authorization.py create mode 100644 tests/routers/test_authorization.py delete mode 100644 tests/routers/test_experiment_set.py diff --git a/src/mavedb/routers/authorization.py b/src/mavedb/routers/authorization.py new file mode 100644 index 00000000..583101e4 --- /dev/null +++ b/src/mavedb/routers/authorization.py @@ -0,0 +1,71 @@ +import logging +from enum import Enum + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.orm import Session + +from mavedb import deps +from mavedb.lib.authentication import get_current_user, UserData +from mavedb.lib.permissions import has_permission, Action +from mavedb.lib.logging import LoggedRoute +from mavedb.lib.logging.context import logging_context, save_to_logging_context +from mavedb.models.experiment import Experiment +from mavedb.models.experiment_set import ExperimentSet +from mavedb.models.score_set import ScoreSet + +router = APIRouter( + prefix="/api/v1", + tags=["authorizations"], + responses={404: {"description": "Not found"}}, + route_class=LoggedRoute, +) + +logger = logging.getLogger(__name__) + + +class ModelName(str, Enum): + experiment = "experiment" + experiment_set = "experiment-set" + score_set = "score-set" + + +@router.get( + "/user-is-authorized/{model_name}/{urn}/{action}", + status_code=200, + response_model=bool +) +async def check_authorization( + *, + model_name: str, + urn: str, + action: str, + db: Session = Depends(deps.get_db), + user_data: UserData = Depends(get_current_user), +) -> bool: + """ + Check whether users have authorizations in adding/editing/deleting/publishing experiment or score set. + """ + save_to_logging_context({"requested_resource": urn}) + if model_name == ModelName.experiment_set: + item = db.query(ExperimentSet).filter(ExperimentSet.urn == urn).one_or_none() + elif model_name == ModelName.experiment: + item = db.query(Experiment).filter(Experiment.urn == urn).one_or_none() + elif model_name == ModelName.score_set: + item = db.query(ScoreSet).filter(ScoreSet.urn == urn).one_or_none() + else: + item = None + + if item: + if user_data: + try: + action_enum = Action[action.upper()] + permission = has_permission(user_data, item, action_enum).permitted + return permission + except KeyError: + raise HTTPException(status_code=400, detail=f"Invalid action: {action}") + else: + logger.debug(msg="Miss user data", extra=logging_context()) + raise HTTPException(status_code=404, detail=f"User not found") + else: + logger.debug(msg="The requested resources does not exist.", extra=logging_context()) + raise HTTPException(status_code=404, detail=f"{model_name} with URN '{urn}' not found") diff --git a/src/mavedb/routers/experiment_sets.py b/src/mavedb/routers/experiment_sets.py index eb235619..eeaaf34c 100644 --- a/src/mavedb/routers/experiment_sets.py +++ b/src/mavedb/routers/experiment_sets.py @@ -4,14 +4,12 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session -from sqlalchemy import or_ from mavedb import deps from mavedb.lib.authentication import get_current_user, UserData from mavedb.lib.permissions import has_permission, Action from mavedb.lib.logging import LoggedRoute from mavedb.lib.logging.context import logging_context, save_to_logging_context -from mavedb.models.contributor import Contributor from mavedb.models.experiment_set import ExperimentSet from mavedb.view_models import experiment_set @@ -25,40 +23,6 @@ logger = logging.getLogger(__name__) -@router.get( - "/check-authorizations/{urn}", - status_code=200, - response_model=bool -) -async def check_experiment_set_authorization( - *, - urn: str, - db: Session = Depends(deps.get_db), - user_data: UserData = Depends(get_current_user), -) -> bool: - """ - Check whether users have authorizations in this experiment set. - """ - query = db.query(ExperimentSet).filter(ExperimentSet.urn == urn) - - if user_data is not None: - query = query.filter( - or_( - ExperimentSet.created_by_id == user_data.user.id, - ExperimentSet.contributors.any(Contributor.orcid_id == user_data.user.username), - ) - ) - else: - return False - - save_to_logging_context({"Experiment set requested resource": urn}) - item = query.first() - if item: - return True - else: - return False - - @router.get( "/{urn}", status_code=200, diff --git a/src/mavedb/routers/experiments.py b/src/mavedb/routers/experiments.py index e8a846b3..6ca47430 100644 --- a/src/mavedb/routers/experiments.py +++ b/src/mavedb/routers/experiments.py @@ -5,7 +5,6 @@ from fastapi import APIRouter, Depends, HTTPException from fastapi.encoders import jsonable_encoder import pydantic -from sqlalchemy import or_, and_ from sqlalchemy.orm import Session from mavedb import deps @@ -45,41 +44,6 @@ ) -@router.get( - "/experiments/check-authorizations/{urn}", - status_code=200, - response_model=bool -) -async def check_experiment_authorization( - *, - urn: str, - db: Session = Depends(deps.get_db), - user_data: UserData = Depends(get_current_user), -) -> bool: - """ - Check whether users have authorizations in this experiment. - """ - query = db.query(Experiment).filter(Experiment.urn == urn) - - if user_data is not None: - query = query.filter( - or_( - Experiment.created_by_id == user_data.user.id, - Experiment.contributors.any(Contributor.orcid_id == user_data.user.username), - ) - ) - else: - return False - - save_to_logging_context({"Experiment requested resource": urn}) - item = query.first() - - if item: - return True - else: - return False - - # TODO: Rewrite this function. @router.get( "/experiments/", diff --git a/src/mavedb/routers/score_sets.py b/src/mavedb/routers/score_sets.py index dcf74f45..44fd2a30 100644 --- a/src/mavedb/routers/score_sets.py +++ b/src/mavedb/routers/score_sets.py @@ -113,40 +113,6 @@ async def fetch_score_set_by_urn( ) -@router.get( - "/score-sets/check-authorizations/{urn}", - status_code=200, - response_model=bool -) -async def check_score_set_authorization( - *, - urn: str, - db: Session = Depends(deps.get_db), - user_data: UserData = Depends(get_current_user), -) -> bool: - """ - Check whether users have authorizations in this score set. - """ - query = db.query(ScoreSet).filter(ScoreSet.urn == urn) - - if user_data is not None: - query = query.filter( - or_( - ScoreSet.created_by_id == user_data.user.id, - ScoreSet.contributors.any(Contributor.orcid_id == user_data.user.username), - ) - ) - else: - return False - - save_to_logging_context({"Score set requested resource": urn}) - item = query.first() - if item: - return True - else: - return False - - @router.post("/score-sets/search", status_code=200, response_model=list[score_set.ShortScoreSet]) def search_score_sets(search: ScoreSetsSearch, db: Session = Depends(deps.get_db)) -> Any: # = Body(..., embed=True), """ diff --git a/src/mavedb/server_main.py b/src/mavedb/server_main.py index 8514fcae..597528f5 100644 --- a/src/mavedb/server_main.py +++ b/src/mavedb/server_main.py @@ -26,6 +26,7 @@ from mavedb.lib.logging.canonical import log_request from mavedb.routers import ( access_keys, + authorization, api_information, controlled_keywords, doi_identifiers, @@ -73,6 +74,7 @@ ) app.include_router(access_keys.router) app.include_router(api_information.router) +app.include_router(authorization.router) app.include_router(controlled_keywords.router) app.include_router(doi_identifiers.router) app.include_router(experiment_sets.router) diff --git a/tests/routers/test_authorization.py b/tests/routers/test_authorization.py new file mode 100644 index 00000000..52fd0893 --- /dev/null +++ b/tests/routers/test_authorization.py @@ -0,0 +1,331 @@ +from tests.helpers.constants import TEST_USER +from tests.helpers.util import ( + add_contributor, + change_ownership, + create_experiment, + create_seq_score_set, +) +from mavedb.models.experiment import Experiment as ExperimentDbModel +from mavedb.models.experiment_set import ExperimentSet as ExperimentSetDbModel +from mavedb.models.score_set import ScoreSet as ScoreSetDbModel + + +# Test check_authorization function +# Experiment set tests +def test_get_true_authorization_from_own_experiment_set_add_experiment_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_set_add_experiment_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) + add_contributor( + session, + experiment["experimentSetUrn"], + ExperimentSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_experiment_set_add_experiment_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) + + response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + + assert response.status_code == 200 + assert response.json() == False + + +def test_cannot_get_authorization_with_wrong_action_in_experiment_set(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/edit") + + assert response.status_code == 400 + response_data = response.json() + assert response_data["detail"] == "Invalid action: edit" + + +def test_cannot_get_authorization_with_non_existing_experiment_set(client, setup_router_db): + response = client.get(f"/api/v1/user-is-authorized/experiment-set/invalidUrn/update") + + assert response.status_code == 404 + response_data = response.json() + assert response_data["detail"] == "experiment-set with URN 'invalidUrn' not found" + + +# Experiment tests +def test_get_true_authorization_from_own_experiment_update_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_true_authorization_from_own_experiment_delete_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_true_authorization_from_own_experiment_add_score_set_check(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_update_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + add_contributor( + session, + experiment["urn"], + ExperimentDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_delete_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + add_contributor( + session, + experiment["urn"], + ExperimentDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_experiment_add_score_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + add_contributor( + session, + experiment["urn"], + ExperimentDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_experiment_add_score_set_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + + assert response.status_code == 200 + assert response.json() == False + + +def test_get_false_authorization_from_other_users_experiment_update_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + + assert response.status_code == 200 + assert response.json() == False + + +def test_get_false_authorization_from_other_users_experiment_delete_check(session, client, setup_router_db): + experiment = create_experiment(client) + change_ownership(session, experiment["urn"], ExperimentDbModel) + + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == False + + +def test_cannot_get_authorization_with_wrong_action_in_experiment(client, setup_router_db): + experiment = create_experiment(client) + response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/invalidAction") + + assert response.status_code == 400 + response_data = response.json() + assert response_data["detail"] == "Invalid action: invalidAction" + + +def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): + response = client.get(f"/api/v1/user-is-authorized/experiment/invalidUrn/update") + + assert response.status_code == 404 + response_data = response.json() + assert response_data["detail"] == "experiment with URN 'invalidUrn' not found" + + +# Score set tests +def test_get_true_authorization_from_own_score_set_update_check(client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_true_authorization_from_own_score_set_delete_check(client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_true_authorization_from_own_score_set_publish_check(client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_score_set_update_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + add_contributor( + session, + score_set["urn"], + ScoreSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_score_set_delete_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + add_contributor( + session, + score_set["urn"], + ScoreSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == True + + +def test_contributor_gets_true_authorization_from_others_score_set_publish_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + add_contributor( + session, + score_set["urn"], + ScoreSetDbModel, + TEST_USER["username"], + TEST_USER["first_name"], + TEST_USER["last_name"], + ) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + + assert response.status_code == 200 + assert response.json() == True + + +def test_get_false_authorization_from_other_users_score_set_delete_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + + assert response.status_code == 200 + assert response.json() == False + + +def test_get_false_authorization_from_other_users_score_set_update_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + + assert response.status_code == 200 + assert response.json() == False + + +def test_get_false_authorization_from_other_users_score_set_publish_check(session, client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + change_ownership(session, score_set["urn"], ScoreSetDbModel) + + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + + assert response.status_code == 200 + assert response.json() == False + + +def test_cannot_get_authorization_with_wrong_action_in_score_set(client, setup_router_db): + experiment = create_experiment(client) + score_set = create_seq_score_set(client, experiment["urn"]) + response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/invalidAction") + + assert response.status_code == 400 + response_data = response.json() + assert response_data["detail"] == "Invalid action: invalidAction" + + +def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): + response = client.get(f"/api/v1/user-is-authorized/score-set/invalidUrn/update") + + assert response.status_code == 404 + response_data = response.json() + assert response_data["detail"] == "score-set with URN 'invalidUrn' not found" + + +# Common invalid test +def test_cannot_get_authorization_with_non_existing_item(client, setup_router_db): + response = client.get(f"/api/v1/user-is-authorized/invalidModel/invalidUrn/update") + + assert response.status_code == 404 + response_data = response.json() + assert response_data["detail"] == "invalidModel with URN 'invalidUrn' not found" diff --git a/tests/routers/test_experiment_set.py b/tests/routers/test_experiment_set.py deleted file mode 100644 index 734e42ec..00000000 --- a/tests/routers/test_experiment_set.py +++ /dev/null @@ -1,45 +0,0 @@ -from tests.helpers.constants import TEST_USER -from tests.helpers.util import ( - add_contributor, - change_ownership, - create_experiment, -) -from mavedb.models.experiment import Experiment as ExperimentDbModel -from mavedb.models.experiment_set import ExperimentSet as ExperimentSetDbModel - - -def test_get_true_authorization_from_own_experiment_set_check(client, setup_router_db): - experiment = create_experiment(client) - response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_contributor_gets_true_authorization_from_others_experiment_set_check(session, client, setup_router_db): - experiment = create_experiment(client) - change_ownership(session, experiment["urn"], ExperimentDbModel) - change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) - add_contributor( - session, - experiment["experimentSetUrn"], - ExperimentSetDbModel, - TEST_USER["username"], - TEST_USER["first_name"], - TEST_USER["last_name"], - ) - response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_get_false_authorization_from_other_users_experiment_set_check(session, client, setup_router_db): - experiment = create_experiment(client) - change_ownership(session, experiment["urn"], ExperimentDbModel) - change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) - - response = client.get(f"/api/v1/experiment-sets/check-authorizations/{experiment['experimentSetUrn']}") - - assert response.status_code == 200 - assert response.json() == False \ No newline at end of file diff --git a/tests/routers/test_experiments.py b/tests/routers/test_experiments.py index 6241ed3a..51c29073 100644 --- a/tests/routers/test_experiments.py +++ b/tests/routers/test_experiments.py @@ -562,41 +562,6 @@ def test_admin_can_update_other_users_public_experiment_set( assert response_data["title"] == "Second Experiment" -def test_get_true_authorization_from_own_experiment_check(client, setup_router_db): - experiment = create_experiment(client) - response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_contributor_gets_true_authorization_from_others_experiment_check(session, client, setup_router_db): - experiment = create_experiment(client) - change_ownership(session, experiment["urn"], ExperimentDbModel) - add_contributor( - session, - experiment["urn"], - ExperimentDbModel, - TEST_USER["username"], - TEST_USER["first_name"], - TEST_USER["last_name"], - ) - response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_get_false_authorization_from_other_users_experiment_check(session, client, setup_router_db): - experiment = create_experiment(client) - change_ownership(session, experiment["urn"], ExperimentDbModel) - - response = client.get(f"/api/v1/experiments/check-authorizations/{experiment['urn']}") - - assert response.status_code == 200 - assert response.json() == False - - def test_edit_preserves_optional_metadata(client, setup_router_db): pass diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py index 4599b84e..b9cbc5b5 100644 --- a/tests/routers/test_score_set.py +++ b/tests/routers/test_score_set.py @@ -496,44 +496,6 @@ def test_admin_can_add_scores_and_counts_to_other_user_score_set(session, client assert score_set == response_data -def test_get_true_authorization_from_own_score_set_check(client, setup_router_db): - experiment = create_experiment(client) - score_set = create_seq_score_set(client, experiment["urn"]) - response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_contributor_gets_true_authorization_from_others_score_set_check(session, client, setup_router_db): - experiment = create_experiment(client) - score_set = create_seq_score_set(client, experiment["urn"]) - change_ownership(session, score_set["urn"], ScoreSetDbModel) - add_contributor( - session, - score_set["urn"], - ScoreSetDbModel, - TEST_USER["username"], - TEST_USER["first_name"], - TEST_USER["last_name"], - ) - response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") - - assert response.status_code == 200 - assert response.json() == True - - -def test_get_false_authorization_from_other_users_score_set_check(session, client, setup_router_db): - experiment = create_experiment(client) - score_set = create_seq_score_set(client, experiment["urn"]) - change_ownership(session, score_set["urn"], ScoreSetDbModel) - - response = client.get(f"/api/v1/score-sets/check-authorizations/{score_set['urn']}") - - assert response.status_code == 200 - assert response.json() == False - - def test_publish_score_set(session, data_provider, client, setup_router_db, data_files): experiment = create_experiment(client) score_set = create_seq_score_set_with_variants( From 83d2bf01f17fd7d0c79074f40a144355ca45efe4 Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Fri, 4 Oct 2024 17:02:38 +1000 Subject: [PATCH 3/7] Solve poetry error --- src/mavedb/routers/authorization.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mavedb/routers/authorization.py b/src/mavedb/routers/authorization.py index 583101e4..3c5559d0 100644 --- a/src/mavedb/routers/authorization.py +++ b/src/mavedb/routers/authorization.py @@ -3,6 +3,7 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session +from typing import Union, Optional from mavedb import deps from mavedb.lib.authentication import get_current_user, UserData @@ -46,14 +47,15 @@ async def check_authorization( Check whether users have authorizations in adding/editing/deleting/publishing experiment or score set. """ save_to_logging_context({"requested_resource": urn}) + + item: Optional[Union[ExperimentSet, Experiment, ScoreSet]] = None + if model_name == ModelName.experiment_set: item = db.query(ExperimentSet).filter(ExperimentSet.urn == urn).one_or_none() elif model_name == ModelName.experiment: item = db.query(Experiment).filter(Experiment.urn == urn).one_or_none() elif model_name == ModelName.score_set: item = db.query(ScoreSet).filter(ScoreSet.urn == urn).one_or_none() - else: - item = None if item: if user_data: From 23fd482ccf84e5066d53c629b7a4576602b96976 Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Tue, 8 Oct 2024 17:32:17 +1100 Subject: [PATCH 4/7] Modify codes and tests. --- src/mavedb/lib/permissions.py | 16 ++-- .../{authorization.py => permissions.py} | 20 ++--- src/mavedb/server_main.py | 4 +- ...t_authorization.py => test_permissions.py} | 74 ++++++++++--------- 4 files changed, 56 insertions(+), 58 deletions(-) rename src/mavedb/routers/{authorization.py => permissions.py} (75%) rename tests/routers/{test_authorization.py => test_permissions.py} (70%) diff --git a/src/mavedb/lib/permissions.py b/src/mavedb/lib/permissions.py index db3b05e1..7bbe3e56 100644 --- a/src/mavedb/lib/permissions.py +++ b/src/mavedb/lib/permissions.py @@ -15,14 +15,14 @@ class Action(Enum): - READ = 1 - UPDATE = 2 - DELETE = 3 - ADD_EXPERIMENT = 4 - ADD_SCORE_SET = 5 - SET_SCORES = 6 - ADD_ROLE = 7 - PUBLISH = 8 + READ = 'read' + UPDATE = 'update' + DELETE = 'delete' + ADD_EXPERIMENT = 'add_experiment' + ADD_SCORE_SET = 'add_score_set' + SET_SCORES = 'set_scores' + ADD_ROLE = 'add_role' + PUBLISH = 'publish' class PermissionResponse: diff --git a/src/mavedb/routers/authorization.py b/src/mavedb/routers/permissions.py similarity index 75% rename from src/mavedb/routers/authorization.py rename to src/mavedb/routers/permissions.py index 3c5559d0..8d9a17b2 100644 --- a/src/mavedb/routers/authorization.py +++ b/src/mavedb/routers/permissions.py @@ -15,8 +15,8 @@ from mavedb.models.score_set import ScoreSet router = APIRouter( - prefix="/api/v1", - tags=["authorizations"], + prefix="/api/v1/permissions", + tags=["permissions"], responses={404: {"description": "Not found"}}, route_class=LoggedRoute, ) @@ -31,7 +31,7 @@ class ModelName(str, Enum): @router.get( - "/user-is-authorized/{model_name}/{urn}/{action}", + "/user-is-permitted/{model_name}/{urn}/{action}", status_code=200, response_model=bool ) @@ -39,7 +39,7 @@ async def check_authorization( *, model_name: str, urn: str, - action: str, + action: Action, db: Session = Depends(deps.get_db), user_data: UserData = Depends(get_current_user), ) -> bool: @@ -58,16 +58,8 @@ async def check_authorization( item = db.query(ScoreSet).filter(ScoreSet.urn == urn).one_or_none() if item: - if user_data: - try: - action_enum = Action[action.upper()] - permission = has_permission(user_data, item, action_enum).permitted - return permission - except KeyError: - raise HTTPException(status_code=400, detail=f"Invalid action: {action}") - else: - logger.debug(msg="Miss user data", extra=logging_context()) - raise HTTPException(status_code=404, detail=f"User not found") + permission = has_permission(user_data, item, action).permitted + return permission else: logger.debug(msg="The requested resources does not exist.", extra=logging_context()) raise HTTPException(status_code=404, detail=f"{model_name} with URN '{urn}' not found") diff --git a/src/mavedb/server_main.py b/src/mavedb/server_main.py index 597528f5..b8429d1d 100644 --- a/src/mavedb/server_main.py +++ b/src/mavedb/server_main.py @@ -26,7 +26,6 @@ from mavedb.lib.logging.canonical import log_request from mavedb.routers import ( access_keys, - authorization, api_information, controlled_keywords, doi_identifiers, @@ -37,6 +36,7 @@ log, mapped_variant, orcid, + permissions, publication_identifiers, target_gene_identifiers, taxonomies, @@ -74,7 +74,6 @@ ) app.include_router(access_keys.router) app.include_router(api_information.router) -app.include_router(authorization.router) app.include_router(controlled_keywords.router) app.include_router(doi_identifiers.router) app.include_router(experiment_sets.router) @@ -84,6 +83,7 @@ # app.include_router(log.router) app.include_router(mapped_variant.router) app.include_router(orcid.router) +app.include_router(permissions.router) app.include_router(publication_identifiers.router) app.include_router(raw_read_identifiers.router) app.include_router(score_sets.router) diff --git a/tests/routers/test_authorization.py b/tests/routers/test_permissions.py similarity index 70% rename from tests/routers/test_authorization.py rename to tests/routers/test_permissions.py index 52fd0893..669c3194 100644 --- a/tests/routers/test_authorization.py +++ b/tests/routers/test_permissions.py @@ -14,7 +14,7 @@ # Experiment set tests def test_get_true_authorization_from_own_experiment_set_add_experiment_check(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/add_experiment") assert response.status_code == 200 assert response.json() == True @@ -32,7 +32,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_set_add_expe TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/add_experiment") assert response.status_code == 200 assert response.json() == True @@ -43,7 +43,7 @@ def test_get_false_authorization_from_other_users_experiment_set_add_experiment_ change_ownership(session, experiment["urn"], ExperimentDbModel) change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) - response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/add_experiment") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/add_experiment") assert response.status_code == 200 assert response.json() == False @@ -51,15 +51,17 @@ def test_get_false_authorization_from_other_users_experiment_set_add_experiment_ def test_cannot_get_authorization_with_wrong_action_in_experiment_set(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment-set/{experiment['experimentSetUrn']}/edit") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/edit") - assert response.status_code == 400 + assert response.status_code == 422 response_data = response.json() - assert response_data["detail"] == "Invalid action: edit" + assert response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', " \ + "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores'," \ + " 'add_role', 'publish'" def test_cannot_get_authorization_with_non_existing_experiment_set(client, setup_router_db): - response = client.get(f"/api/v1/user-is-authorized/experiment-set/invalidUrn/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/invalidUrn/update") assert response.status_code == 404 response_data = response.json() @@ -69,7 +71,7 @@ def test_cannot_get_authorization_with_non_existing_experiment_set(client, setup # Experiment tests def test_get_true_authorization_from_own_experiment_update_check(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/update") assert response.status_code == 200 assert response.json() == True @@ -77,7 +79,7 @@ def test_get_true_authorization_from_own_experiment_update_check(client, setup_r def test_get_true_authorization_from_own_experiment_delete_check(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/delete") assert response.status_code == 200 assert response.json() == True @@ -85,7 +87,7 @@ def test_get_true_authorization_from_own_experiment_delete_check(client, setup_r def test_get_true_authorization_from_own_experiment_add_score_set_check(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") assert response.status_code == 200 assert response.json() == True @@ -102,7 +104,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_update_check TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/update") assert response.status_code == 200 assert response.json() == True @@ -119,7 +121,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_delete_check TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/delete") assert response.status_code == 200 assert response.json() == True @@ -136,7 +138,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_add_score_se TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") assert response.status_code == 200 assert response.json() == True @@ -146,7 +148,7 @@ def test_get_false_authorization_from_other_users_experiment_add_score_set_check experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/add_score_set") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") assert response.status_code == 200 assert response.json() == False @@ -156,7 +158,7 @@ def test_get_false_authorization_from_other_users_experiment_update_check(sessio experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/update") assert response.status_code == 200 assert response.json() == False @@ -166,7 +168,7 @@ def test_get_false_authorization_from_other_users_experiment_delete_check(sessio experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/delete") assert response.status_code == 200 assert response.json() == False @@ -174,15 +176,17 @@ def test_get_false_authorization_from_other_users_experiment_delete_check(sessio def test_cannot_get_authorization_with_wrong_action_in_experiment(client, setup_router_db): experiment = create_experiment(client) - response = client.get(f"/api/v1/user-is-authorized/experiment/{experiment['urn']}/invalidAction") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/invalidAction") - assert response.status_code == 400 + assert response.status_code == 422 response_data = response.json() - assert response_data["detail"] == "Invalid action: invalidAction" + assert response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', " \ + "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores'," \ + " 'add_role', 'publish'" def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): - response = client.get(f"/api/v1/user-is-authorized/experiment/invalidUrn/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/invalidUrn/update") assert response.status_code == 404 response_data = response.json() @@ -193,7 +197,7 @@ def test_cannot_get_authorization_with_non_existing_experiment(client, setup_rou def test_get_true_authorization_from_own_score_set_update_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/update") assert response.status_code == 200 assert response.json() == True @@ -202,7 +206,7 @@ def test_get_true_authorization_from_own_score_set_update_check(client, setup_ro def test_get_true_authorization_from_own_score_set_delete_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/delete") assert response.status_code == 200 assert response.json() == True @@ -211,7 +215,7 @@ def test_get_true_authorization_from_own_score_set_delete_check(client, setup_ro def test_get_true_authorization_from_own_score_set_publish_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/publish") assert response.status_code == 200 assert response.json() == True @@ -229,7 +233,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_update_check( TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/update") assert response.status_code == 200 assert response.json() == True @@ -247,7 +251,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_delete_check( TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/delete") assert response.status_code == 200 assert response.json() == True @@ -265,7 +269,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_publish_check TEST_USER["first_name"], TEST_USER["last_name"], ) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/publish") assert response.status_code == 200 assert response.json() == True @@ -276,7 +280,7 @@ def test_get_false_authorization_from_other_users_score_set_delete_check(session score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/delete") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/delete") assert response.status_code == 200 assert response.json() == False @@ -287,7 +291,7 @@ def test_get_false_authorization_from_other_users_score_set_update_check(session score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/update") assert response.status_code == 200 assert response.json() == False @@ -298,7 +302,7 @@ def test_get_false_authorization_from_other_users_score_set_publish_check(sessio score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/publish") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/publish") assert response.status_code == 200 assert response.json() == False @@ -307,15 +311,17 @@ def test_get_false_authorization_from_other_users_score_set_publish_check(sessio def test_cannot_get_authorization_with_wrong_action_in_score_set(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) - response = client.get(f"/api/v1/user-is-authorized/score-set/{score_set['urn']}/invalidAction") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/invalidAction") - assert response.status_code == 400 + assert response.status_code == 422 response_data = response.json() - assert response_data["detail"] == "Invalid action: invalidAction" + assert response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', " \ + "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores'," \ + " 'add_role', 'publish'" def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): - response = client.get(f"/api/v1/user-is-authorized/score-set/invalidUrn/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/invalidUrn/update") assert response.status_code == 404 response_data = response.json() @@ -324,7 +330,7 @@ def test_cannot_get_authorization_with_non_existing_experiment(client, setup_rou # Common invalid test def test_cannot_get_authorization_with_non_existing_item(client, setup_router_db): - response = client.get(f"/api/v1/user-is-authorized/invalidModel/invalidUrn/update") + response = client.get(f"/api/v1/permissions/user-is-permitted/invalidModel/invalidUrn/update") assert response.status_code == 404 response_data = response.json() From 44e12ae8232b7715f706de155d9a8a3f5b04e33d Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Wed, 9 Oct 2024 11:47:52 +1100 Subject: [PATCH 5/7] Find a bug in has_permission function. A test can't pass due to it. --- tests/routers/test_permissions.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/routers/test_permissions.py b/tests/routers/test_permissions.py index 669c3194..2ab55934 100644 --- a/tests/routers/test_permissions.py +++ b/tests/routers/test_permissions.py @@ -144,14 +144,15 @@ def test_contributor_gets_true_authorization_from_others_experiment_add_score_se assert response.json() == True -def test_get_false_authorization_from_other_users_experiment_add_score_set_check(session, client, setup_router_db): +# TODO: This one has problem. Need to fix has_permission function from lib/permissions.py first +def test_get_true_authorization_from_other_users_experiment_add_score_set_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") assert response.status_code == 200 - assert response.json() == False + assert response.json() == True def test_get_false_authorization_from_other_users_experiment_update_check(session, client, setup_router_db): From 6f7892d05521c292338d0bbea1a07f823f8dc5cc Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Fri, 11 Oct 2024 16:47:31 +1100 Subject: [PATCH 6/7] Change function name. Change related tests names and add few more tests based on new has_permission function. --- src/mavedb/routers/permissions.py | 4 +- tests/routers/test_permissions.py | 77 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/mavedb/routers/permissions.py b/src/mavedb/routers/permissions.py index 8d9a17b2..21cfe3ef 100644 --- a/src/mavedb/routers/permissions.py +++ b/src/mavedb/routers/permissions.py @@ -35,9 +35,9 @@ class ModelName(str, Enum): status_code=200, response_model=bool ) -async def check_authorization( +async def check_permission( *, - model_name: str, + model_name: ModelName, urn: str, action: Action, db: Session = Depends(deps.get_db), diff --git a/tests/routers/test_permissions.py b/tests/routers/test_permissions.py index 2ab55934..e3bcf79f 100644 --- a/tests/routers/test_permissions.py +++ b/tests/routers/test_permissions.py @@ -4,6 +4,7 @@ change_ownership, create_experiment, create_seq_score_set, + create_seq_score_set_with_variants, ) from mavedb.models.experiment import Experiment as ExperimentDbModel from mavedb.models.experiment_set import ExperimentSet as ExperimentSetDbModel @@ -12,7 +13,7 @@ # Test check_authorization function # Experiment set tests -def test_get_true_authorization_from_own_experiment_set_add_experiment_check(client, setup_router_db): +def test_get_true_permission_from_own_experiment_set_add_experiment_check(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/add_experiment") @@ -20,7 +21,7 @@ def test_get_true_authorization_from_own_experiment_set_add_experiment_check(cli assert response.json() == True -def test_contributor_gets_true_authorization_from_others_experiment_set_add_experiment_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_experiment_set_add_experiment_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) @@ -38,7 +39,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_set_add_expe assert response.json() == True -def test_get_false_authorization_from_other_users_experiment_set_add_experiment_check(session, client, setup_router_db): +def test_get_false_permission_from_others_experiment_set_add_experiment_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) change_ownership(session, experiment["experimentSetUrn"], ExperimentSetDbModel) @@ -49,7 +50,7 @@ def test_get_false_authorization_from_other_users_experiment_set_add_experiment_ assert response.json() == False -def test_cannot_get_authorization_with_wrong_action_in_experiment_set(client, setup_router_db): +def test_cannot_get_permission_with_wrong_action_in_experiment_set(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/{experiment['experimentSetUrn']}/edit") @@ -60,7 +61,7 @@ def test_cannot_get_authorization_with_wrong_action_in_experiment_set(client, se " 'add_role', 'publish'" -def test_cannot_get_authorization_with_non_existing_experiment_set(client, setup_router_db): +def test_cannot_get_permission_with_non_existing_experiment_set(client, setup_router_db): response = client.get(f"/api/v1/permissions/user-is-permitted/experiment-set/invalidUrn/update") assert response.status_code == 404 @@ -69,7 +70,7 @@ def test_cannot_get_authorization_with_non_existing_experiment_set(client, setup # Experiment tests -def test_get_true_authorization_from_own_experiment_update_check(client, setup_router_db): +def test_get_true_permission_from_own_experiment_update_check(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/update") @@ -77,7 +78,7 @@ def test_get_true_authorization_from_own_experiment_update_check(client, setup_r assert response.json() == True -def test_get_true_authorization_from_own_experiment_delete_check(client, setup_router_db): +def test_get_true_permission_from_own_experiment_delete_check(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/delete") @@ -85,7 +86,7 @@ def test_get_true_authorization_from_own_experiment_delete_check(client, setup_r assert response.json() == True -def test_get_true_authorization_from_own_experiment_add_score_set_check(client, setup_router_db): +def test_get_true_permission_from_own_experiment_add_score_set_check(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") @@ -93,7 +94,7 @@ def test_get_true_authorization_from_own_experiment_add_score_set_check(client, assert response.json() == True -def test_contributor_gets_true_authorization_from_others_experiment_update_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_experiment_update_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) add_contributor( @@ -110,7 +111,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_update_check assert response.json() == True -def test_contributor_gets_true_authorization_from_others_experiment_delete_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_experiment_delete_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) add_contributor( @@ -127,7 +128,7 @@ def test_contributor_gets_true_authorization_from_others_experiment_delete_check assert response.json() == True -def test_contributor_gets_true_authorization_from_others_experiment_add_score_set_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_private_experiment_add_score_set_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) add_contributor( @@ -144,18 +145,31 @@ def test_contributor_gets_true_authorization_from_others_experiment_add_score_se assert response.json() == True -# TODO: This one has problem. Need to fix has_permission function from lib/permissions.py first -def test_get_true_authorization_from_other_users_experiment_add_score_set_check(session, client, setup_router_db): +def test_get_false_permission_from_others_private_experiment_add_score_set_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/add_score_set") + assert response.status_code == 200 + assert response.json() == False + + +def test_get_true_permission_from_others_public_experiment_add_score_set_check(session, data_provider, client, setup_router_db, data_files): + experiment = create_experiment(client) + score_set_1 = create_seq_score_set_with_variants( + client, session, data_provider, experiment["urn"], data_files / "scores.csv" + ) + pub_score_set = client.post(f"/api/v1/score-sets/{score_set_1['urn']}/publish").json() + pub_experiment_urn = pub_score_set["experiment"]["urn"] + change_ownership(session, pub_experiment_urn, ExperimentDbModel) + response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{pub_experiment_urn}/add_score_set") + assert response.status_code == 200 assert response.json() == True -def test_get_false_authorization_from_other_users_experiment_update_check(session, client, setup_router_db): +def test_get_false_permission_from_others_experiment_update_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) @@ -165,7 +179,7 @@ def test_get_false_authorization_from_other_users_experiment_update_check(sessio assert response.json() == False -def test_get_false_authorization_from_other_users_experiment_delete_check(session, client, setup_router_db): +def test_get_false_permission_from_other_users_experiment_delete_check(session, client, setup_router_db): experiment = create_experiment(client) change_ownership(session, experiment["urn"], ExperimentDbModel) @@ -175,7 +189,7 @@ def test_get_false_authorization_from_other_users_experiment_delete_check(sessio assert response.json() == False -def test_cannot_get_authorization_with_wrong_action_in_experiment(client, setup_router_db): +def test_cannot_get_permission_with_wrong_action_in_experiment(client, setup_router_db): experiment = create_experiment(client) response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/{experiment['urn']}/invalidAction") @@ -186,7 +200,7 @@ def test_cannot_get_authorization_with_wrong_action_in_experiment(client, setup_ " 'add_role', 'publish'" -def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): +def test_cannot_get_permission_with_non_existing_experiment(client, setup_router_db): response = client.get(f"/api/v1/permissions/user-is-permitted/experiment/invalidUrn/update") assert response.status_code == 404 @@ -195,7 +209,7 @@ def test_cannot_get_authorization_with_non_existing_experiment(client, setup_rou # Score set tests -def test_get_true_authorization_from_own_score_set_update_check(client, setup_router_db): +def test_get_true_permission_from_own_score_set_update_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/update") @@ -204,7 +218,7 @@ def test_get_true_authorization_from_own_score_set_update_check(client, setup_ro assert response.json() == True -def test_get_true_authorization_from_own_score_set_delete_check(client, setup_router_db): +def test_get_true_permission_from_own_score_set_delete_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/delete") @@ -213,7 +227,7 @@ def test_get_true_authorization_from_own_score_set_delete_check(client, setup_ro assert response.json() == True -def test_get_true_authorization_from_own_score_set_publish_check(client, setup_router_db): +def test_get_true_permission_from_own_score_set_publish_check(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/publish") @@ -222,7 +236,7 @@ def test_get_true_authorization_from_own_score_set_publish_check(client, setup_r assert response.json() == True -def test_contributor_gets_true_authorization_from_others_score_set_update_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_score_set_update_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -240,7 +254,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_update_check( assert response.json() == True -def test_contributor_gets_true_authorization_from_others_score_set_delete_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_score_set_delete_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -258,7 +272,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_delete_check( assert response.json() == True -def test_contributor_gets_true_authorization_from_others_score_set_publish_check(session, client, setup_router_db): +def test_contributor_gets_true_permission_from_others_score_set_publish_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -276,7 +290,7 @@ def test_contributor_gets_true_authorization_from_others_score_set_publish_check assert response.json() == True -def test_get_false_authorization_from_other_users_score_set_delete_check(session, client, setup_router_db): +def test_get_false_permission_from_others_score_set_delete_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -287,7 +301,7 @@ def test_get_false_authorization_from_other_users_score_set_delete_check(session assert response.json() == False -def test_get_false_authorization_from_other_users_score_set_update_check(session, client, setup_router_db): +def test_get_false_permission_from_others_score_set_update_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -298,7 +312,7 @@ def test_get_false_authorization_from_other_users_score_set_update_check(session assert response.json() == False -def test_get_false_authorization_from_other_users_score_set_publish_check(session, client, setup_router_db): +def test_get_false_permission_from_others_score_set_publish_check(session, client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) change_ownership(session, score_set["urn"], ScoreSetDbModel) @@ -309,7 +323,7 @@ def test_get_false_authorization_from_other_users_score_set_publish_check(sessio assert response.json() == False -def test_cannot_get_authorization_with_wrong_action_in_score_set(client, setup_router_db): +def test_cannot_get_permission_with_wrong_action_in_score_set(client, setup_router_db): experiment = create_experiment(client) score_set = create_seq_score_set(client, experiment["urn"]) response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/{score_set['urn']}/invalidAction") @@ -321,7 +335,7 @@ def test_cannot_get_authorization_with_wrong_action_in_score_set(client, setup_r " 'add_role', 'publish'" -def test_cannot_get_authorization_with_non_existing_experiment(client, setup_router_db): +def test_cannot_get_permission_with_non_existing_experiment(client, setup_router_db): response = client.get(f"/api/v1/permissions/user-is-permitted/score-set/invalidUrn/update") assert response.status_code == 404 @@ -330,9 +344,10 @@ def test_cannot_get_authorization_with_non_existing_experiment(client, setup_rou # Common invalid test -def test_cannot_get_authorization_with_non_existing_item(client, setup_router_db): +def test_cannot_get_permission_with_non_existing_item(client, setup_router_db): response = client.get(f"/api/v1/permissions/user-is-permitted/invalidModel/invalidUrn/update") - assert response.status_code == 404 + assert response.status_code == 422 response_data = response.json() - assert response_data["detail"] == "invalidModel with URN 'invalidUrn' not found" + assert response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: " \ + "'experiment', 'experiment-set', 'score-set'" From 010c229f4a387ca41826c6ada39d4a249b4e0527 Mon Sep 17 00:00:00 2001 From: EstelleDa Date: Fri, 11 Oct 2024 17:26:43 +1100 Subject: [PATCH 7/7] Change the error message. --- src/mavedb/routers/permissions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mavedb/routers/permissions.py b/src/mavedb/routers/permissions.py index 21cfe3ef..480621a1 100644 --- a/src/mavedb/routers/permissions.py +++ b/src/mavedb/routers/permissions.py @@ -62,4 +62,4 @@ async def check_permission( return permission else: logger.debug(msg="The requested resources does not exist.", extra=logging_context()) - raise HTTPException(status_code=404, detail=f"{model_name} with URN '{urn}' not found") + raise HTTPException(status_code=404, detail=f"{model_name.value} with URN '{urn}' not found")