From e34f25e752be67b58a03917a16f0e295f3a82d08 Mon Sep 17 00:00:00 2001 From: Keshav Priyadarshi Date: Wed, 13 Dec 2023 21:34:12 +0530 Subject: [PATCH 1/5] Fix swagger API doc for `api/packages/bulk_search/` - Use `extend_schema` to override the request body, which was not being properly discovered. - drf-spectacular relies on `get_serializer_class()` and `get_serializer()`, and it works well for view functions that purely deal with ModelSerializer. For anything else, it gets a bit murky, and it is advised to provide proper overrides in the `extend_schema` decorator. - Override erroneous pagination and filter backend caused due to response containing multiple serializer object https://drf-spectacular.readthedocs.io/en/latest/faq.html#my-action-is-erroneously-paginated-or-has-filter-parameters-that-i-do-not-want Signed-off-by: Keshav Priyadarshi --- vulnerabilities/api.py | 45 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 86f800e71..d83210487 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -11,8 +11,10 @@ from django.db.models import Prefetch from django_filters import rest_framework as filters +from drf_spectacular.utils import extend_schema from packageurl import PackageURL from rest_framework import serializers +from rest_framework import status from rest_framework import viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -272,6 +274,16 @@ def filter_purl(self, queryset, name, value): return self.queryset.filter(**lookups) +class PackageBulkSearchRequestSerializer(serializers.Serializer): + purls = serializers.ListField( + child=serializers.CharField(), + allow_empty=False, + help_text="List of PackageURL strings in canonical form.", + ) + purl_only = serializers.BooleanField(required=False, default=False) + plain_purl = serializers.BooleanField(required=False, default=False) + + class PackageViewSet(viewsets.ReadOnlyModelViewSet): """ Lookup for vulnerable packages by Package URL. @@ -283,21 +295,36 @@ class PackageViewSet(viewsets.ReadOnlyModelViewSet): filterset_class = PackageFilterSet throttle_classes = [StaffUserRateThrottle, AnonRateThrottle] - # TODO: Fix the swagger documentation for this endpoint - @action(detail=False, methods=["post"]) + @extend_schema( + request=PackageBulkSearchRequestSerializer, + responses={ + 200: PackageSerializer(many=True), + }, + ) + @action( + detail=False, + methods=["post"], + serializer_class=PackageBulkSearchRequestSerializer, + filter_backends=[], + pagination_class=None, + ) def bulk_search(self, request): """ Lookup for vulnerable packages using many Package URLs at once. """ - - purls = request.data.get("purls", []) or [] - purl_only = request.data.get("purl_only", False) - plain_purl = request.data.get("plain_purl", False) - if not purls or not isinstance(purls, list): + serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): return Response( - status=400, - data={"Error": "A non-empty 'purls' list of PURLs is required."}, + status=status.HTTP_400_BAD_REQUEST, + data={ + "error": serializer.errors, + "message": "A non-empty 'purls' list of PURLs is required.", + }, ) + validated_data = serializer.validated_data + purls = validated_data.get("purls") + purl_only = validated_data.get("purl_only", False) + plain_purl = validated_data.get("plain_purl", False) if plain_purl: purl_objects = [PackageURL.from_string(purl) for purl in purls] From 3524ec516ef4ea989935bcf1b7a36a5118895f21 Mon Sep 17 00:00:00 2001 From: Keshav Priyadarshi Date: Thu, 14 Dec 2023 22:09:14 +0530 Subject: [PATCH 2/5] Add tests for `bulk_search` Signed-off-by: Keshav Priyadarshi --- vulnerabilities/tests/test_api.py | 34 +++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index 924d6fa2e..dbf1509d6 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -647,6 +647,40 @@ def test_bulk_api_with_purl_only_option(self): assert len(response) == 1 assert response[0] == "pkg:nginx/nginx@1.0.15" + def test_bulk_api_without_purls_list(self): + request_body = { + "purls": None, + } + response = self.csrf_client.post( + "/api/packages/bulk_search", + data=json.dumps(request_body), + content_type="application/json", + ).json() + + expected = { + "error": {"purls": ["This field may not be null."]}, + "message": "A non-empty 'purls' list of PURLs is required.", + } + + self.assertEqual(response, expected) + + def test_bulk_api_without_purls_empty_list(self): + request_body = { + "purls": [], + } + response = self.csrf_client.post( + "/api/packages/bulk_search", + data=json.dumps(request_body), + content_type="application/json", + ).json() + + expected = { + "error": {"purls": ["This list may not be empty."]}, + "message": "A non-empty 'purls' list of PURLs is required.", + } + + self.assertEqual(response, expected) + class BulkSearchAPICPE(TestCase): def setUp(self): From 5b1a76cf57bc9f87aa53c792392674b640e77166 Mon Sep 17 00:00:00 2001 From: Keshav Priyadarshi Date: Thu, 14 Dec 2023 22:18:01 +0530 Subject: [PATCH 3/5] Test `bulk_search` with empty request body Signed-off-by: Keshav Priyadarshi --- vulnerabilities/tests/test_api.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index dbf1509d6..de2d5da4f 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -681,6 +681,21 @@ def test_bulk_api_without_purls_empty_list(self): self.assertEqual(response, expected) + def test_bulk_api_with_empty_request_body(self): + request_body = {} + response = self.csrf_client.post( + "/api/packages/bulk_search", + data=json.dumps(request_body), + content_type="application/json", + ).json() + + expected = { + "error": {"purls": ["This field is required."]}, + "message": "A non-empty 'purls' list of PURLs is required.", + } + + self.assertEqual(response, expected) + class BulkSearchAPICPE(TestCase): def setUp(self): From 556ca5f7712b3c32167d196fa9cc0e6d4a6e60e3 Mon Sep 17 00:00:00 2001 From: Keshav Priyadarshi Date: Thu, 14 Dec 2023 23:59:16 +0530 Subject: [PATCH 4/5] Fix API doc generation for `api/packages/lookup` Signed-off-by: Keshav Priyadarshi --- vulnerabilities/api.py | 31 ++++++++++++++++++++++++++----- vulnerabilities/tests/test_api.py | 8 +++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index 10698b601..dd9044e33 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -12,6 +12,7 @@ from django.db.models import Prefetch from django_filters import rest_framework as filters from drf_spectacular.utils import extend_schema +from drf_spectacular.utils import inline_serializer from packageurl import PackageURL from rest_framework import serializers from rest_framework import status @@ -284,6 +285,10 @@ class PackageBulkSearchRequestSerializer(serializers.Serializer): plain_purl = serializers.BooleanField(required=False, default=False) +class LookupRequestSerializer(serializers.Serializer): + purl = serializers.CharField(required=True, help_text="PackageURL strings in canonical form.") + + class PackageViewSet(viewsets.ReadOnlyModelViewSet): """ Lookup for vulnerable packages by Package URL. @@ -374,17 +379,33 @@ def all(self, request): vulnerable_purls = [str(package.package_url) for package in vulnerable_packages] return Response(vulnerable_purls) - @action(detail=False, methods=["post"]) + @extend_schema( + request=LookupRequestSerializer, + responses=PackageSerializer(many=True), + ) + @action( + detail=False, + methods=["post"], + serializer_class=LookupRequestSerializer, + filter_backends=[], + pagination_class=None, + ) def lookup(self, request): """ Return the response for exact PackageURL requested for. """ - purl = request.data.get("purl") - if not purl: + serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): return Response( - status=400, - data={"Error": "A 'purl' is required."}, + status=status.HTTP_400_BAD_REQUEST, + data={ + "error": serializer.errors, + "message": "A 'purl' is required.", + }, ) + validated_data = serializer.validated_data + purl = validated_data.get("purl") + return Response( PackageSerializer( Package.objects.for_purls([purl]), many=True, context={"request": request} diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index de2d5da4f..88ac30369 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -817,7 +817,13 @@ def test_lookup_endpoint_failure(self): data=json.dumps(request_body), content_type="application/json", ).json() - assert response == {"Error": "A 'purl' is required."} + + expected = { + "error": {"purl": ["This field may not be null."]}, + "message": "A 'purl' is required.", + } + + self.assertEqual(response, expected) def test_lookup_endpoint(self): request_body = {"purl": "pkg:pypi/microweber/microweber@1.2"} From e7d43534b377074dd2d27c4a6982f9de0bfe25f0 Mon Sep 17 00:00:00 2001 From: Keshav Priyadarshi Date: Fri, 15 Dec 2023 00:16:41 +0530 Subject: [PATCH 5/5] Fix API doc generation for `api/packages/bulk_lookup` Signed-off-by: Keshav Priyadarshi --- vulnerabilities/api.py | 42 +++++++++++++++++++++++-------- vulnerabilities/tests/test_api.py | 15 +++++++++++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/vulnerabilities/api.py b/vulnerabilities/api.py index dd9044e33..3b82a7970 100644 --- a/vulnerabilities/api.py +++ b/vulnerabilities/api.py @@ -275,18 +275,24 @@ def filter_purl(self, queryset, name, value): return self.queryset.filter(**lookups) -class PackageBulkSearchRequestSerializer(serializers.Serializer): +class PackageurlListSerializer(serializers.Serializer): purls = serializers.ListField( child=serializers.CharField(), allow_empty=False, help_text="List of PackageURL strings in canonical form.", ) + + +class PackageBulkSearchRequestSerializer(PackageurlListSerializer): purl_only = serializers.BooleanField(required=False, default=False) plain_purl = serializers.BooleanField(required=False, default=False) class LookupRequestSerializer(serializers.Serializer): - purl = serializers.CharField(required=True, help_text="PackageURL strings in canonical form.") + purl = serializers.CharField( + required=True, + help_text="PackageURL strings in canonical form.", + ) class PackageViewSet(viewsets.ReadOnlyModelViewSet): @@ -302,9 +308,7 @@ class PackageViewSet(viewsets.ReadOnlyModelViewSet): @extend_schema( request=PackageBulkSearchRequestSerializer, - responses={ - 200: PackageSerializer(many=True), - }, + responses={200: PackageSerializer(many=True)}, ) @action( detail=False, @@ -381,7 +385,7 @@ def all(self, request): @extend_schema( request=LookupRequestSerializer, - responses=PackageSerializer(many=True), + responses={200: PackageSerializer(many=True)}, ) @action( detail=False, @@ -412,17 +416,33 @@ def lookup(self, request): ).data ) - @action(detail=False, methods=["post"]) + @extend_schema( + request=PackageurlListSerializer, + responses={200: PackageSerializer(many=True)}, + ) + @action( + detail=False, + methods=["post"], + serializer_class=PackageurlListSerializer, + filter_backends=[], + pagination_class=None, + ) def bulk_lookup(self, request): """ Return the response for exact PackageURLs requested for. """ - purls = request.data.get("purls") or [] - if not purls: + serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): return Response( - status=400, - data={"Error": "A non-empty 'purls' list of PURLs is required."}, + status=status.HTTP_400_BAD_REQUEST, + data={ + "error": serializer.errors, + "message": "A non-empty 'purls' list of PURLs is required.", + }, ) + validated_data = serializer.validated_data + purls = validated_data.get("purls") + return Response( PackageSerializer( Package.objects.for_purls(purls), diff --git a/vulnerabilities/tests/test_api.py b/vulnerabilities/tests/test_api.py index 88ac30369..c6e667280 100644 --- a/vulnerabilities/tests/test_api.py +++ b/vulnerabilities/tests/test_api.py @@ -899,3 +899,18 @@ def test_bulk_lookup_endpoint(self): content_type="application/json", ).json() assert len(response) == 1 + + def test_bulk_lookup_endpoint_failure(self): + request_body = {"purls": None} + response = self.csrf_client.post( + "/api/packages/bulk_lookup", + data=json.dumps(request_body), + content_type="application/json", + ).json() + + expected = { + "error": {"purls": ["This field may not be null."]}, + "message": "A non-empty 'purls' list of PURLs is required.", + } + + self.assertEqual(response, expected)