diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60264ac87..76364fbee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,6 +27,8 @@ jobs: RESOLWE_POSTGRESQL_PORT: 55433 RESOLWE_REDIS_PORT: 56380 RESOLWE_TEST_ONLY_CHANGES_TO: "origin/master" + TOX_COMMAND: "tox --skip-missing-interpreters false" + strategy: matrix: toxenv: ["py312"] @@ -65,9 +67,9 @@ jobs: # NOTE: If we are building the "master" branch or a pull request against the # "master" branch, we allow installing pre-releases with the pip command. - - name: Allow pre-releases for master branch - if: github.ref == 'refs/heads/master' - run: echo "PIP_PRE=1" >> $GITHUB_ENV + - name: Allow pre-releases for PRs and merges with master branch + if: github.event_name == 'pull_request' || github.ref == 'refs/heads/master' + run: echo "TOX_COMMAND=${{ env.TOX_COMMAND }} --pre" >> $GITHUB_ENV - name: Assert PR is up-to-date if: github.event_name == 'pull_request' @@ -83,35 +85,35 @@ jobs: - name: Documentation run: | - tox -e docs + ${{ env.TOX_COMMAND }} -e docs - name: Linters run: | - tox -e linters + ${{ env.TOX_COMMAND }} -e linters - name: Packaging run: | - tox -e packaging + ${{ env.TOX_COMMAND }} -e packaging - name: Extras run: | - tox -e extra + ${{ env.TOX_COMMAND }} -e extra - name: Migrations run: | - tox -e migrations + ${{ env.TOX_COMMAND }} -e migrations - name: Run entire test suite if: github.event_name != 'pull_request' timeout-minutes: 120 run: | - tox -e ${{ matrix.toxenv }} + ${{ env.TOX_COMMAND }} -e ${{ matrix.toxenv }} - name: Unit partial test suite on pull request if: github.event_name == 'pull_request' timeout-minutes: 120 run: | - tox -e ${{ matrix.toxenv }}-partial + ${{ env.TOX_COMMAND }} -e ${{ matrix.toxenv }}-partial build: runs-on: public-docker-runner diff --git a/docs/CHANGELOG.rst b/docs/CHANGELOG.rst index 65e241560..747de7819 100644 --- a/docs/CHANGELOG.rst +++ b/docs/CHANGELOG.rst @@ -13,6 +13,13 @@ Unreleased Added ----- +- Allow staff users to create new ``Variant`` objects through the API endpoint +- Allow authenticated users to create new ``VariantCall`` objects via the API + endpoint +- Allow staff users to create new ``VariantExperiment`` objects through the API + endpoint +- Allow staff users to delet ``Variant`` and ``VariantExperiment`` via API +- Allow users with permissions to delete ``VariantCall`` objects via the API Changed ------- diff --git a/resolwe_bio/variants/models.py b/resolwe_bio/variants/models.py index 19cc9b6f9..2a68b3e6a 100644 --- a/resolwe_bio/variants/models.py +++ b/resolwe_bio/variants/models.py @@ -175,6 +175,10 @@ def permission_proxy(cls) -> str: """Return the permission proxy name.""" return "sample" + def in_container(self): + """Permissions depend on sample object.""" + return True + #: the referenced sample sample = models.ForeignKey( Sample, on_delete=models.CASCADE, related_name="variant_calls" diff --git a/resolwe_bio/variants/serializers.py b/resolwe_bio/variants/serializers.py index e94f97d39..4d0e4da14 100644 --- a/resolwe_bio/variants/serializers.py +++ b/resolwe_bio/variants/serializers.py @@ -27,6 +27,7 @@ class Meta: """Serializer configuration.""" model = Variant + optional_fields = ["annotation"] fields = [ "id", "species", @@ -37,6 +38,7 @@ class Meta: "alternative", "annotation", ] + extra_kwargs = {"annotation": {"required": False}} class VariantTranscriptSerializer(SelectiveFieldMixin, serializers.ModelSerializer): @@ -94,9 +96,9 @@ class Meta: model = VariantCall fields = [ "id", - "sample_id", - "variant_id", - "experiment_id", + "sample", + "variant", + "experiment", "quality", "genotype_quality", "depth", @@ -104,7 +106,7 @@ class Meta: "alternative_allele_depth", "filter", "genotype", - "data_id", + "data", ] diff --git a/resolwe_bio/variants/tests/test_variant.py b/resolwe_bio/variants/tests/test_variant.py index d52f5d1bc..419c21ec9 100644 --- a/resolwe_bio/variants/tests/test_variant.py +++ b/resolwe_bio/variants/tests/test_variant.py @@ -371,9 +371,9 @@ def test_add_variants(self): variant2 = Variant.objects.get(chromosome="chr2") expected_calls = [ { - "sample_id": self.data.entity.pk, - "variant_id": variant1.pk, - "experiment_id": experiment.pk, + "sample": self.data.entity.pk, + "variant": variant1.pk, + "experiment": experiment.pk, "quality": 1.0, "depth_norm_quality": None, "alternative_allele_depth": None, @@ -381,12 +381,12 @@ def test_add_variants(self): "filter": "1", "genotype": "1", "genotype_quality": None, - "data_id": self.data.pk, + "data": self.data.pk, }, { - "sample_id": self.data.entity.pk, - "variant_id": variant2.pk, - "experiment_id": experiment.pk, + "sample": self.data.entity.pk, + "variant": variant2.pk, + "experiment": experiment.pk, "quality": 2.0, "depth_norm_quality": 0.2, "alternative_allele_depth": 2, @@ -394,14 +394,14 @@ def test_add_variants(self): "filter": "2", "genotype": "2", "genotype_quality": 2, - "data_id": self.data.pk, + "data": self.data.pk, }, ] self.assertCountEqual( VariantCall.objects.all().values( - "sample_id", - "variant_id", - "experiment_id", + "sample", + "variant", + "experiment", "quality", "depth_norm_quality", "alternative_allele_depth", @@ -409,7 +409,7 @@ def test_add_variants(self): "filter", "genotype", "genotype_quality", - "data_id", + "data", ), expected_calls, ) @@ -562,9 +562,94 @@ def test_add_variant_annotations(self): class VariantTest(PrepareDataMixin, TestCase): def setUp(self) -> None: - self.view = VariantViewSet.as_view({"get": "list"}) + self.view = VariantViewSet.as_view( + {"get": "list", "post": "create", "delete": "destroy"} + ) return super().setUp() + def test_create_destroy(self): + """Test the Variant create and destroy. + + Only users with staff status are allowed to create Variant objects. + """ + variant_data = { + "species": "Homo Sapiens", + "genome_assembly": "test_create", + "chromosome": "CHR_test_create", + "position": 1, + "reference": "test_create", + "alternative": "alt_test_create", + } + + # Test creation as unauthenticated user. + request = APIRequestFactory().post("/variant", variant_data, format="json") + response = self.view(request) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as non-staff user. + request = APIRequestFactory().post("/variant", variant_data, format="json") + force_authenticate(request, self.contributor) + response = self.view(request) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as staff user. + self.contributor.is_staff = True + self.contributor.save(update_fields=["is_staff"]) + request = APIRequestFactory().post("/variant", variant_data, format="json") + force_authenticate(request, self.contributor) + response = self.view(request) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + # Check that the created object exists. + created = Variant.objects.get(**response.data) + + # Delete as anonymous user. + request = APIRequestFactory().delete( + "/variant", {"pk": created.pk}, format="json" + ) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete as non-staff user. + self.contributor.is_staff = False + self.contributor.save(update_fields=["is_staff"]) + request = APIRequestFactory().delete( + "/variant", {"pk": created.pk}, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete as staff user. + self.contributor.is_staff = True + self.contributor.save(update_fields=["is_staff"]) + request = APIRequestFactory().delete( + "/variant", {"pk": created.pk}, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request, pk=created.pk) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(Variant.DoesNotExist): + created.refresh_from_db() + + self.contributor.is_staff = False + self.contributor.save(update_fields=["is_staff"]) + def test_filter(self): """Test the Variant filter.""" request = APIRequestFactory().get("/variant") @@ -1132,9 +1217,155 @@ def test_ordering(self): class VariantCallTest(PrepareDataMixin, TestCase): def setUp(self) -> None: - self.view = VariantCallViewSet.as_view({"get": "list"}) + self.view = VariantCallViewSet.as_view( + {"get": "list", "post": "create", "delete": "destroy"} + ) return super().setUp() + def test_create_destroy(self): + """Test the VariantCall creation.""" + variant_call_data = { + "sample": self.sample.pk, + "variant": self.variants[0].pk, + "quality": 1.0, + "depth_norm_quality": 0.1, + "alternative_allele_depth": 3, + "depth": 5, + "filter": "test_create", + "genotype": "l", + "genotype_quality": 1, + "experiment": self.experiments[0].pk, + "data": None, + } + + # Test creation as unauthenticated user. + request = APIRequestFactory().post( + "/variantcall", variant_call_data, format="json" + ) + response = self.view(request) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as user without permissions on the sample. + user = User.objects.get_or_create(username="user", email="user@genialis.com")[0] + request = APIRequestFactory().post( + "/variantcall", variant_call_data, format="json" + ) + force_authenticate(request, user) + response = self.view(request) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as user with permissions on the sample. + self.sample.set_permission(Permission.VIEW, user) + request = APIRequestFactory().post( + "/variantcall", variant_call_data, format="json" + ) + force_authenticate(request, user) + response = self.view(request) + created = VariantCall.objects.get(**response.data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual( + response.data, + { + "id": created.id, + **variant_call_data, + }, + ) + + data = Data.objects.create( + contributor=self.contributor, + process=Process.objects.create(contributor=self.contributor), + status=Data.STATUS_PROCESSING, + ) + variant_call_data["data"] = data.pk + + # Test creation as user without permissions on data object. + request = APIRequestFactory().post( + "/variantcall", variant_call_data, format="json" + ) + force_authenticate(request, user) + response = self.view(request) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as user with read permissions on data object. + data.set_permission(Permission.VIEW, user) + request = APIRequestFactory().post( + "/variantcall", variant_call_data, format="json" + ) + force_authenticate(request, user) + response = self.view(request) + created = VariantCall.objects.get(**response.data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual( + response.data, + { + "id": created.id, + **variant_call_data, + }, + ) + + # Delete as anonymous user. + request = APIRequestFactory().delete( + "/variantcall", {"pk": created.pk}, format="json" + ) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete without required permission to data and sample. + self.sample.set_permission(Permission.VIEW, user) + data.set_permission(Permission.VIEW, user) + request = APIRequestFactory().delete( + "/variantcall", {"pk": created.pk}, format="json" + ) + force_authenticate(request, user) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete without required permission to data. + self.sample.set_permission(Permission.EDIT, user) + data.set_permission(Permission.VIEW, user) + request = APIRequestFactory().delete( + "/variantcall", {"pk": created.pk}, format="json" + ) + force_authenticate(request, user) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete with required permission to data. + self.sample.set_permission(Permission.EDIT, user) + data.set_permission(Permission.EDIT, user) + request = APIRequestFactory().delete( + "/variantcall", {"pk": created.pk}, format="json" + ) + force_authenticate(request, user) + response = self.view(request, pk=created.pk) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(VariantCall.DoesNotExist): + created.refresh_from_db() + def test_filter(self): # No filter no permission for public. request = APIRequestFactory().get("/variantcall") @@ -1359,9 +1590,96 @@ def test_ordering(self): class VariantExperimentTest(PrepareDataMixin, TestCase): def setUp(self) -> None: - self.view = VariantExperimentViewSet.as_view({"get": "list"}) + self.view = VariantExperimentViewSet.as_view( + {"get": "list", "post": "create", "delete": "destroy"} + ) return super().setUp() + def test_create_destroy(self): + """Test the VariantExperiment creation and deletion. + + Only users with staff status are allowed to create VariantExperiment objects. + """ + variant_experiment_data = { + "variant_data_source": "test", + "contributor": self.contributor.pk, + } + + # Test creation as unauthenticated user. + request = APIRequestFactory().post( + "/variantexperiment", variant_experiment_data, format="json" + ) + response = self.view(request) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as non-staff user. + request = APIRequestFactory().post( + "/variantexperiment", variant_experiment_data, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Test creation as staff user. + self.contributor.is_staff = True + self.contributor.save(update_fields=["is_staff"]) + request = APIRequestFactory().post( + "/variantexperiment", variant_experiment_data, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + # Check that the created object exists. + created = VariantExperiment.objects.get(**response.data) + self.contributor.is_staff = False + self.contributor.save(update_fields=["is_staff"]) + + # Delete as anonymous user. + request = APIRequestFactory().delete( + "/variantexperiment", {"pk": created.pk}, format="json" + ) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "Authentication credentials were not provided.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete as non-staff user. + request = APIRequestFactory().delete( + "/variantexperiment", {"pk": created.pk}, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request, pk=created.pk) + self.assertContains( + response, + "You do not have permission to perform this action.", + status_code=status.HTTP_403_FORBIDDEN, + ) + + # Delete as staff user. + self.contributor.is_staff = True + self.contributor.save(update_fields=["is_staff"]) + request = APIRequestFactory().delete( + "/variantexperiment", {"pk": created.pk}, format="json" + ) + force_authenticate(request, self.contributor) + response = self.view(request, pk=created.pk) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(VariantExperiment.DoesNotExist): + created.refresh_from_db() + + self.contributor.is_staff = False + self.contributor.save(update_fields=["is_staff"]) + def test_distinct(self): """Test only distynct experiments are returned.""" # Create another VariantCall that points to the first sample and experiment. diff --git a/resolwe_bio/variants/views.py b/resolwe_bio/variants/views.py index 68cbf7dd3..c04851607 100644 --- a/resolwe_bio/variants/views.py +++ b/resolwe_bio/variants/views.py @@ -9,9 +9,12 @@ import logging import django_filters as filters -from rest_framework import mixins, viewsets +from rest_framework import exceptions, mixins, permissions, serializers, viewsets from resolwe.flow.filters import OrderingFilter +from resolwe.flow.views.mixins import ResolweCreateModelMixin +from resolwe.flow.views.utils import IsStaffOrReadOnly +from resolwe.permissions.models import Permission from resolwe_bio.variants.filters import ( VariantAnnotationFilter, @@ -31,12 +34,18 @@ logger = logging.getLogger(__name__) -class VariantViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): +class VariantViewSet( + mixins.ListModelMixin, + ResolweCreateModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): """Variant endpoint.""" queryset = Variant.objects.all() serializer_class = VariantSerializer filter_backends = [filters.rest_framework.DjangoFilterBackend, OrderingFilter] + permission_classes = (IsStaffOrReadOnly,) filterset_class = VariantFilter ordering_fields = ("species", "genome_assembly", "position", "chromosome") @@ -58,7 +67,12 @@ class VariantAnnotationViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): ) -class VariantCallViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): +class VariantCallViewSet( + mixins.ListModelMixin, + ResolweCreateModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): """VariantCall endpoint. The default filter backends are used so permissions are respected. @@ -70,13 +84,45 @@ class VariantCallViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): filterset_class = VariantCallFilter ordering_fields = ("id", "quality", "depth") + permission_classes = (permissions.IsAuthenticatedOrReadOnly,) -class VariantExperimentViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): + def perform_create(self, serializer: serializers.BaseSerializer) -> None: + """Check if user has VIEW permission on the sample.""" + # Check permission on sample. + sample = serializer.validated_data["sample"] + if not sample.has_permission(Permission.VIEW, self.request.user): + raise exceptions.PermissionDenied() + + # Check permission on data (if given). + if data := serializer.validated_data.get("data"): + if not data.has_permission(Permission.VIEW, self.request.user): + raise exceptions.PermissionDenied() + + return super().perform_create(serializer) + + def perform_destroy(self, instance: VariantCall) -> None: + """Check if user has EDIT permission on the sample.""" + if not instance.sample.has_permission(Permission.EDIT, self.request.user): + raise exceptions.PermissionDenied() + if data := instance.data: + if not data.has_permission(Permission.EDIT, self.request.user): + raise exceptions.PermissionDenied() + + return super().perform_destroy(instance) + + +class VariantExperimentViewSet( + mixins.ListModelMixin, + ResolweCreateModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): """VariantExperiment endpoint.""" queryset = VariantExperiment.objects.all() serializer_class = VariantExperimentSerializer filter_backends = [filters.rest_framework.DjangoFilterBackend, OrderingFilter] + permission_classes = (IsStaffOrReadOnly,) filterset_class = VariantExperimentFilter ordering_fields = ("id", "timestamp", "contributor__email")