diff --git a/arkindex/ponos/api.py b/arkindex/ponos/api.py index 4cdee3e3bebb4b3d76386a47f57150f6ad63ce35..5db15ab4d6bdf6cb5426becda7f49bb60ccf9693 100644 --- a/arkindex/ponos/api.py +++ b/arkindex/ponos/api.py @@ -92,7 +92,7 @@ class PublicKeyEndpoint(APIView): description=dedent(""" Retrieve a Ponos task. - Requires **guest** access on the task's process or Ponos agent authentication. + Requires **guest** access on the task's process, Ponos agent authentication, or authentication as the Ponos task to retrieve. """), ), put=extend_schema( @@ -125,6 +125,7 @@ class TaskDetailsFromAgent(RetrieveUpdateAPIView): 'process__revision__repo', ) authentication_classes = ( + TaskAuthentication, AgentAuthentication, TokenAuthentication, SessionAuthentication, diff --git a/arkindex/ponos/permissions.py b/arkindex/ponos/permissions.py index c27e8d3b5e6e4a04e4b87149c5d279c38846fda8..f86fad22f2b34f09f8862e5ae0390996eab19c79 100644 --- a/arkindex/ponos/permissions.py +++ b/arkindex/ponos/permissions.py @@ -107,7 +107,10 @@ class IsAgentOrTaskGuest(ProcessACLMixin, IsAuthenticated): task == request.auth or require_agent_or_admin(request, view) or ( + # Users with a verified email getattr(request.user, 'verified_email', False) + # Not authenticated through Ponos task authentication + and not isinstance(request.auth, Task) # process_access_level can return None if there is no access at all and (self.process_access_level(task.process) or 0) >= Role.Guest.value ) diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py index 0e4717cc0386b6ca65712cfe7e2c734ff7ba5bf2..646778873c9ab1ddc2aa10ef6748b4c888cafd40 100644 --- a/arkindex/ponos/tests/test_api.py +++ b/arkindex/ponos/tests/test_api.py @@ -78,13 +78,61 @@ class TestAPI(FixtureAPITestCase): resp = self.client.get(reverse("api:task-details", args=[self.task1.id])) self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) - def test_task_details_task_forbidden(self): - with self.assertNumQueries(0): + @patch("arkindex.ponos.aws.s3") + @patch("arkindex.ponos.models.s3") + def test_task_details_own_task(self, s3_mock, aws_s3_mock): + s3_mock.Object.return_value.bucket_name = "ponos" + s3_mock.Object.return_value.key = "somelog" + s3_mock.Object.return_value.get.return_value = { + "Body": BytesIO(b"Failed successfully") + } + aws_s3_mock.meta.client.generate_presigned_url.return_value = "http://somewhere" + + with self.assertNumQueries(3): resp = self.client.get( reverse("api:task-details", args=[self.task1.id]), HTTP_AUTHORIZATION=f'Ponos {self.task1.token}', ) - self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + self.assertDictEqual( + resp.json(), + { + "id": str(self.task1.id), + "run": 0, + "depth": 0, + "slug": "initialisation", + "state": "unscheduled", + "parents": [], + "tags": [], + "logs": "Failed successfully", + "url": resp.wsgi_request.build_absolute_uri( + reverse("api:task-details", args=[self.task1.id]) + ), + "full_log": "http://somewhere", + "extra_files": {}, + "agent": None, + "gpu": None, + "shm_size": None, + }, + ) + + self.assertEqual(s3_mock.Object.call_count, 1) + self.assertEqual(s3_mock.Object().get.call_count, 1) + self.assertEqual(s3_mock.Object().get.call_args, call(Range="bytes=-42")) + self.assertEqual(aws_s3_mock.meta.client.generate_presigned_url.call_count, 1) + self.assertEqual( + aws_s3_mock.meta.client.generate_presigned_url.call_args, + call("get_object", Params={"Bucket": "ponos", "Key": "somelog"}), + ) + + def test_task_details_other_task_forbidden(self): + with self.assertNumQueries(2): + resp = self.client.get( + reverse("api:task-details", args=[self.task2.id]), + HTTP_AUTHORIZATION=f'Ponos {self.task1.token}', + ) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) def test_task_details_requires_verified(self): self.user.verified_email = False @@ -646,13 +694,13 @@ class TestAPI(FixtureAPITestCase): self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) def test_update_task_from_agent_forbids_task(self): - with self.assertNumQueries(0): + with self.assertNumQueries(1): resp = self.client.put( reverse("api:task-details", args=[self.task1.id]), data={"state": State.Completed.value}, HTTP_AUTHORIZATION=f'Ponos {self.task1.token}', ) - self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) def test_update_task_from_agent_unassigned(self): self.assertNotEqual(self.task1.agent, self.agent) @@ -933,13 +981,13 @@ class TestAPI(FixtureAPITestCase): self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) def test_partial_update_task_from_agent_forbids_task(self): - with self.assertNumQueries(0): + with self.assertNumQueries(1): resp = self.client.patch( reverse("api:task-details", args=[self.task1.id]), data={"state": State.Completed.value}, HTTP_AUTHORIZATION=f'Ponos {self.task1.token}', ) - self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) def test_partial_update_task_from_agent_unassigned(self): self.assertNotEqual(self.task1.agent, self.agent) diff --git a/arkindex/ponos/tests/test_artifacts_api.py b/arkindex/ponos/tests/test_artifacts_api.py index 6dfa397b57ff1f007e952716981eae98b0a3996e..928f4bd96b40cc7e10d1ae4e9effcf6fa14d5657 100644 --- a/arkindex/ponos/tests/test_artifacts_api.py +++ b/arkindex/ponos/tests/test_artifacts_api.py @@ -227,7 +227,7 @@ class TestAPI(FixtureAPITestCase): ) def test_list_other_task(self): - with self.assertNumQueries(5): + with self.assertNumQueries(2): response = self.client.get( reverse("api:task-artifacts", args=[self.task1.id]), HTTP_AUTHORIZATION=f'Ponos {self.task2.token}', @@ -412,7 +412,7 @@ class TestAPI(FixtureAPITestCase): self.assertEqual(response.json(), {"size": [expected_error]}) def test_create_other_task(self): - with self.assertNumQueries(5): + with self.assertNumQueries(2): response = self.client.post( reverse("api:task-artifacts", args=[self.task1.id]), data={ @@ -577,16 +577,9 @@ class TestAPI(FixtureAPITestCase): ) def test_download_other_task(self): - with self.assertNumQueries(5): + with self.assertNumQueries(2): response = self.client.get( reverse("api:task-artifact-download", args=[self.task1.id, "path/to/file.json"]), HTTP_AUTHORIZATION=f'Ponos {self.task2.token}', ) - self.assertEqual(response.status_code, status.HTTP_302_FOUND) - - self.assertTrue(response.has_header("Location")) - self.assertTrue( - response["Location"].startswith( - f"http://somewhere/ponos-artifacts/{self.task1.id}/path/to/file.json" - ) - ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)