Skip to content

Commit

Permalink
Merge branch 'release-2024.4.1' into estelle/userAuthorizationCheck
Browse files Browse the repository at this point in the history
  • Loading branch information
@EstelleDa
EstelleDa authored Oct 2, 2024
2 parents c65eac6 + 99116b4 commit 3b5f328
Show file tree
Hide file tree
Showing 7 changed files with 426 additions and 83 deletions.
10 changes: 9 additions & 1 deletion src/mavedb/routers/experiment_sets.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

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
Expand Down Expand Up @@ -64,7 +65,9 @@ async def check_experiment_set_authorization(
response_model=experiment_set.ExperimentSet,
responses={404: {}},
)
def fetch_experiment_set(*, urn: str, db: Session = Depends(deps.get_db)) -> Any:
def fetch_experiment_set(
*, urn: str, db: Session = Depends(deps.get_db), user_data: UserData = Depends(get_current_user)
) -> Any:
"""
Fetch a single experiment set by URN.
"""
Expand All @@ -80,4 +83,9 @@ def fetch_experiment_set(*, urn: str, db: Session = Depends(deps.get_db)) -> Any
else:
item.experiments.sort(key=attrgetter("urn"))

has_permission(user_data, item, Action.READ)

# Filter experiment sub-resources to only those experiments readable by the requesting user.
item.experiments[:] = [exp for exp in item.experiments if has_permission(user_data, exp, Action.READ).permitted]

return item
27 changes: 9 additions & 18 deletions src/mavedb/routers/experiments.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
)
from mavedb.lib.logging import LoggedRoute
from mavedb.lib.logging.context import logging_context, save_to_logging_context
from mavedb.lib.permissions import assert_permission, Action
from mavedb.lib.permissions import assert_permission, has_permission, Action
from mavedb.lib.validation.exceptions import ValidationError
from mavedb.lib.validation.keywords import validate_keyword_list
from mavedb.lib.keywords import search_keyword
Expand Down Expand Up @@ -200,24 +200,15 @@ def get_experiment_score_sets(

assert_permission(user_data, experiment, Action.READ)

# If there is a current user with score sets associated with this experiment, return all of them. Otherwise, only show
# the public / published score sets.
#
# TODO(#182): A side effect of this implementation is that only the user who has created the experiment may view all the Score sets
# associated with a given experiment. This could be solved with user impersonation for certain user roles.
score_sets = (
db.query(ScoreSet).filter(ScoreSet.experiment_id == experiment.id).filter(~ScoreSet.superseding_score_set.has())
score_set_result = (
db.query(ScoreSet)
.filter(ScoreSet.experiment_id == experiment.id)
.filter(~ScoreSet.superseding_score_set.has())
.all()
)
if user_data is not None:
score_set_result = score_sets.filter(
or_(
ScoreSet.private.is_(False),
and_(ScoreSet.private.is_(True), ScoreSet.created_by == user_data.user),
)
).all()
else:
score_set_result = score_sets.filter(ScoreSet.private.is_(False)).all()
logger.debug(msg="User is anonymous; Filtering only public score sets will be shown.", extra=logging_context())
score_set_result[:] = [
score_set for score_set in score_set_result if has_permission(user_data, score_set, Action.READ).permitted
]

if not score_set_result:
save_to_logging_context({"associated_resources": []})
Expand Down
113 changes: 72 additions & 41 deletions tests/routers/test_experiments.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,15 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination1(client, s
"key": "Variant Library Creation Method",
"value": "Endogenous locus library method",
"special": False,
"description": "Description"
"description": "Description",
},
},
{
"keyword": {
"key": "In Vitro Construct Library Method System",
"value": "Oligo-directed mutagenic PCR",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -151,9 +151,11 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination1(client, s
response = client.post("/api/v1/experiments/", json=experiment)
assert response.status_code == 422
response_data = response.json()
assert response_data["detail"] == \
"If 'Variant Library Creation Method' is 'Endogenous locus library method', both 'Endogenous Locus " \
"Library Method System' and 'Endogenous Locus Library Method Mechanism' must be present."
assert (
response_data["detail"]
== "If 'Variant Library Creation Method' is 'Endogenous locus library method', both 'Endogenous Locus "
"Library Method System' and 'Endogenous Locus Library Method Mechanism' must be present."
)


def test_cannot_create_experiment_that_keywords_has_wrong_combination2(client, setup_router_db):
Expand All @@ -165,15 +167,15 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination2(client, s
"key": "Variant Library Creation Method",
"value": "In vitro construct library method",
"special": False,
"description": "Description"
"description": "Description",
},
},
{
"keyword": {
"key": "Endogenous Locus Library Method System",
"value": "SaCas9",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -182,9 +184,11 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination2(client, s
response = client.post("/api/v1/experiments/", json=experiment)
assert response.status_code == 422
response_data = response.json()
assert response_data["detail"] == \
"If 'Variant Library Creation Method' is 'In vitro construct library method', both 'In Vitro Construct " \
"Library Method System' and 'In Vitro Construct Library Method Mechanism' must be present."
assert (
response_data["detail"]
== "If 'Variant Library Creation Method' is 'In vitro construct library method', both 'In Vitro Construct "
"Library Method System' and 'In Vitro Construct Library Method Mechanism' must be present."
)


def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, setup_router_db):
Expand All @@ -199,16 +203,16 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, s
"key": "Variant Library Creation Method",
"value": "Other",
"special": False,
"description": "Description"
"description": "Description",
},
"description": "Description"
"description": "Description",
},
{
"keyword": {
"key": "Endogenous Locus Library Method System",
"value": "SaCas9",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -217,10 +221,12 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, s
response = client.post("/api/v1/experiments/", json=experiment)
assert response.status_code == 422
response_data = response.json()
assert response_data["detail"] == \
"If 'Variant Library Creation Method' is 'Other', none of 'Endogenous Locus Library Method System', " \
"'Endogenous Locus Library Method Mechanism', 'In Vitro Construct Library Method System', or 'In Vitro " \
"Construct Library Method Mechanism' should be present."
assert (
response_data["detail"]
== "If 'Variant Library Creation Method' is 'Other', none of 'Endogenous Locus Library Method System', "
"'Endogenous Locus Library Method Mechanism', 'In Vitro Construct Library Method System', or 'In Vitro "
"Construct Library Method Mechanism' should be present."
)


def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, setup_router_db):
Expand All @@ -235,16 +241,16 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, s
"key": "Variant Library Creation Method",
"value": "Other",
"special": False,
"description": "Description"
"description": "Description",
},
"description": "Description"
"description": "Description",
},
{
"keyword": {
"key": "In Vitro Construct Library Method System",
"value": "Error-prone PCR",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -253,10 +259,12 @@ def test_cannot_create_experiment_that_keywords_has_wrong_combination3(client, s
response = client.post("/api/v1/experiments/", json=experiment)
assert response.status_code == 422
response_data = response.json()
assert response_data["detail"] == \
"If 'Variant Library Creation Method' is 'Other', none of 'Endogenous Locus Library Method System', " \
"'Endogenous Locus Library Method Mechanism', 'In Vitro Construct Library Method System', or 'In Vitro " \
"Construct Library Method Mechanism' should be present."
assert (
response_data["detail"]
== "If 'Variant Library Creation Method' is 'Other', none of 'Endogenous Locus Library Method System', "
"'Endogenous Locus Library Method Mechanism', 'In Vitro Construct Library Method System', or 'In Vitro "
"Construct Library Method Mechanism' should be present."
)


def test_cannot_create_experiment_that_keyword_value_is_other_without_description(client, setup_router_db):
Expand All @@ -271,17 +279,17 @@ def test_cannot_create_experiment_that_keyword_value_is_other_without_descriptio
"key": "Variant Library Creation Method",
"value": "Other",
"special": False,
"description": "Description"
"description": "Description",
},
"description": None
"description": None,
},
]
}
experiment = {**TEST_MINIMAL_EXPERIMENT, **invalid_keywords}
response = client.post("/api/v1/experiments/", json=experiment)
assert response.status_code == 422
response_data = response.json()
error_messages = [error['msg'] for error in response_data["detail"]]
error_messages = [error["msg"] for error in response_data["detail"]]
assert "Other option does not allow empty description." in error_messages


Expand All @@ -294,16 +302,16 @@ def test_cannot_create_experiment_that_keywords_have_duplicate_keys(client, setu
"key": "Variant Library Creation Method",
"value": "Other",
"special": False,
"description": "Description"
"description": "Description",
},
"description": "Description"
"description": "Description",
},
{
"keyword": {
"key": "Variant Library Creation Method",
"value": "In vitro construct library method",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -327,15 +335,15 @@ def test_cannot_create_experiment_that_keywords_have_duplicate_values(client, se
"key": "Delivery method",
"value": "In vitro construct library method",
"special": False,
"description": "Description"
"description": "Description",
},
},
{
"keyword": {
"key": "Variant Library Creation Method",
"value": "In vitro construct library method",
"special": False,
"description": "Description"
"description": "Description",
},
},
]
Expand All @@ -359,18 +367,13 @@ def test_create_experiment_that_keywords_have_duplicate_others(client, setup_rou
"key": "Variant Library Creation Method",
"value": "Other",
"special": False,
"description": "Description"
"description": "Description",
},
"description": "Description"
"description": "Description",
},
{
"keyword": {
"key": "Delivery method",
"value": "Other",
"special": False,
"description": "Description"
},
"description": "Description"
"keyword": {"key": "Delivery method", "value": "Other", "special": False, "description": "Description"},
"description": "Description",
},
]
}
Expand Down Expand Up @@ -1019,6 +1022,34 @@ def test_search_score_sets_for_experiments(session, client, setup_router_db, dat
assert response.json()[0]["urn"] == published_score_set["urn"]


def test_search_score_sets_for_contributor_experiments(session, client, setup_router_db, data_files, data_provider):
experiment = create_experiment(client)
score_set_pub = create_seq_score_set_with_variants(
client, session, data_provider, experiment["urn"], data_files / "scores.csv"
)
# make the unpublished score set owned by some other user. This shouldn't appear in the results.
score_set_unpub = create_seq_score_set(client, experiment["urn"], update={"title": "Unpublished Score Set"})
published_score_set = client.post(f"/api/v1/score-sets/{score_set_pub['urn']}/publish").json()
change_ownership(session, score_set_unpub["urn"], ScoreSetDbModel)
add_contributor(
session,
score_set_unpub["urn"],
ScoreSetDbModel,
TEST_USER["username"],
TEST_USER["first_name"],
TEST_USER["last_name"],
)

# On score set publication, the experiment will get a new urn
experiment_urn = published_score_set["experiment"]["urn"]
response = client.get(f"/api/v1/experiments/{experiment_urn}/score-sets")
assert response.status_code == 200
response_urns = [score_set["urn"] for score_set in response.json()]
assert len(response_urns) == 2
assert published_score_set["urn"] in response_urns
assert score_set_unpub["urn"] in response_urns


def test_search_score_sets_for_my_experiments(session, client, setup_router_db, data_files, data_provider):
experiment = create_experiment(client)
score_set_pub = create_seq_score_set_with_variants(
Expand Down
35 changes: 35 additions & 0 deletions tests/routers/test_score_set.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,41 @@ def test_search_score_sets_match(session, data_provider, client, setup_router_db
assert response.json()[0]["title"] == score_set_1_1["title"]


def test_search_score_sets_urn_match(session, data_provider, client, setup_router_db, data_files):
experiment_1 = create_experiment(client)
score_set_1_1 = create_seq_score_set_with_variants(
client,
session,
data_provider,
experiment_1["urn"],
data_files / "scores.csv"
)

search_payload = {"urn": score_set_1_1['urn']}
response = client.post("/api/v1/score-sets/search", json=search_payload)
assert response.status_code == 200
assert len(response.json()) == 1
assert response.json()[0]["urn"] == score_set_1_1["urn"]


# There is space in the end of test urn. The search result returned nothing before.
def test_search_score_sets_urn_with_space_match(session, data_provider, client, setup_router_db, data_files):
experiment_1 = create_experiment(client)
score_set_1_1 = create_seq_score_set_with_variants(
client,
session,
data_provider,
experiment_1["urn"],
data_files / "scores.csv"
)
urn_with_space = score_set_1_1['urn'] + " "
search_payload = {"urn": urn_with_space}
response = client.post("/api/v1/score-sets/search", json=search_payload)
assert response.status_code == 200
assert len(response.json()) == 1
assert response.json()[0]["urn"] == score_set_1_1["urn"]


def test_anonymous_cannot_delete_other_users_private_scoreset(
session, data_provider, client, setup_router_db, data_files, anonymous_app_overrides
):
Expand Down
Loading

0 comments on commit 3b5f328

Please sign in to comment.