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

fix(chord): fix bad logic for checking if dataset project changed #544

Merged
merged 1 commit into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions chord_metadata_service/authz/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ def one_no_authz_put(self, url: str, *args, **kwargs):
"""
return self._one_authz_put(False, url, *args, **kwargs)

def _one_authz_patch(self, authz_res: bool, url: str, *args, **kwargs):
with aioresponses() as m:
mock_authz_eval_one_result(m, authz_res)
return self.client.patch(url, *args, content_type="application/json", **kwargs)

def one_authz_patch(self, url: str, *args, **kwargs):
"""
Mocks a single True response from the authorization service and executes a JSON PATCH request.
"""
return self._one_authz_patch(True, url, *args, **kwargs)

def _one_authz_delete(self, authz_res: bool, url: str, *args, **kwargs):
with aioresponses() as m:
mock_authz_eval_one_result(m, authz_res)
Expand Down
2 changes: 1 addition & 1 deletion chord_metadata_service/chord/api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ async def update(self, request, *args, **kwargs):
return forbidden(request) # side effect: sets authz done flag

# Do not allow datasets to change project
if request.data["project"] != dataset_project_id:
if "project" in request.data and request.data["project"] != dataset_project_id:
return bad_request(request, "Dataset project ID cannot change")

authz.mark_authz_done(request)
Expand Down
41 changes: 26 additions & 15 deletions chord_metadata_service/chord/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,17 @@ def setUp(self):

def test_update_dataset(self):
r = self.one_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_200_OK
self.assertEqual(r.status_code, status.HTTP_200_OK)
self.dataset.refresh_from_db()
self.assertEqual(self.dataset.title, self.valid_update["title"])

def test_update_dataset_partial(self):
r = self.one_authz_patch(
f"/api/datasets/{self.dataset.identifier}", data=json.dumps({"title": self.valid_update["title"]})
)
self.assertEqual(r.status_code, status.HTTP_200_OK)
self.dataset.refresh_from_db()
assert self.dataset.title == self.valid_update["title"]
self.assertEqual(self.dataset.title, self.valid_update["title"])

def test_update_dataset_changed_project(self):
r = self.one_authz_put(
Expand All @@ -248,50 +256,53 @@ def test_update_dataset_changed_project(self):
"project": str(self.project_2.identifier),
})
)
assert r.status_code == status.HTTP_400_BAD_REQUEST
self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
res = r.json()
assert res["message"] == "Bad Request"
assert res["errors"][0]["message"] == "Dataset project ID cannot change"
self.assertEqual(res["message"], "Bad Request")
self.assertEqual(res["errors"][0]["message"], "Dataset project ID cannot change")

def test_update_dataset_bad_dats_json(self):
r = self.one_authz_put(
f"/api/datasets/{self.dataset.identifier}",
data=json.dumps({**self.valid_update, "dats_file": "asdf"}), # asdf is not JSON
)
assert r.status_code == status.HTTP_400_BAD_REQUEST
self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
res = r.json()
assert res["message"] == "Bad Request"
assert res["errors"][0]["message"] == (
"Submitted dataset.dats_file data is not a valid JSON string. Make sure the string value is JSON "
"compatible, or submit dats_file as a JSON object."
self.assertEqual(res["message"], "Bad Request")
self.assertEqual(
res["errors"][0]["message"],
(
"Submitted dataset.dats_file data is not a valid JSON string. Make sure the string value is JSON "
"compatible, or submit dats_file as a JSON object."
)
)

def test_update_dataset_forbidden(self):
r = self.one_no_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_403_FORBIDDEN
self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)

def test_update_dataset_not_found(self):
r = self.one_authz_put(f"/api/datasets/{uuid.uuid4()}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_404_NOT_FOUND
self.assertEqual(r.status_code, status.HTTP_404_NOT_FOUND)


class DeleteDatasetTest(AuthzAPITestCase, ProjectTestCase):

def test_delete_dataset(self):
r = self.one_authz_delete(f"/api/datasets/{self.dataset.identifier}")
assert r.status_code == status.HTTP_204_NO_CONTENT
self.assertEqual(r.status_code, status.HTTP_204_NO_CONTENT)

with self.assertRaises(Dataset.DoesNotExist): # must not exist in DB anymore
self.dataset.refresh_from_db()

def test_delete_dataset_forbidden(self):
r = self.one_no_authz_delete(f"/api/datasets/{self.dataset.identifier}")
assert r.status_code == status.HTTP_403_FORBIDDEN
self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
self.dataset.refresh_from_db() # must not raise DoesNotExist

def test_delete_dataset_not_found(self):
r = self.client.delete(f"/api/datasets/{uuid.uuid4()}")
assert r.status_code == status.HTTP_404_NOT_FOUND
self.assertEqual(r.status_code, status.HTTP_404_NOT_FOUND)


class CreateProjectJsonSchema(AuthzAPITestCaseWithProjectJSON):
Expand Down