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

How to remove/prevent parameters inherited" from ViewSet #399

Closed
valentijnscholten opened this issue May 21, 2021 · 6 comments
Closed

How to remove/prevent parameters inherited" from ViewSet #399

valentijnscholten opened this issue May 21, 2021 · 6 comments

Comments

@valentijnscholten
Copy link

valentijnscholten commented May 21, 2021

I am migrating from drf_yasg to drf-spectacular and I think the below is not answered yet anywhere. I did read the docs :-)

I have some ViewSets to handle models via DRF, for example:

class EngagementViewSet(mixins.ListModelMixin,
                        mixins.RetrieveModelMixin,
                        mixins.UpdateModelMixin,
                        mixins.DestroyModelMixin,
                        mixins.CreateModelMixin,
                        ra_api.AcceptedRisksMixin,
                        viewsets.GenericViewSet):
    serializer_class = serializers.EngagementSerializer
    queryset = Engagement.objects.none()
    filter_backends = (DjangoFilterBackend,)
    filter_class = ApiEngagementFilter
    permission_classes = (IsAuthenticated, permissions.UserHasEngagementPermission)

    @swagger_auto_schema(
        request_body=no_body, responses={status.HTTP_200_OK: ""}
    )
    @extend_schema(
        request=no_body,
        responses={status.HTTP_200_OK: ""}
    )
    @action(detail=True, methods=["post"])
    def close(self, request, pk=None):
        eng = self.get_object()
        close_engagement(eng)
        return HttpResponse()

As you can see there is a custom Mixin ra.api.AcceptedRisksMixin to provides an extra action method:

class AcceptedFindingsMixin(ABC):

    @swagger_auto_schema(
        request_body=AcceptedRiskSerializer(many=True),
        responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)},
    )
    @extend_schema(
        request=AcceptedRiskSerializer(many=True),
        responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)},
    )
    @action(methods=['post'], detail=False, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer)
    def accept_risks(self, request):
        serializer = AcceptedRiskSerializer(data=request.data, many=True)
        if serializer.is_valid():
            accepted_risks = serializer.save()
        else:
            return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST)
        owner = request.user
        accepted_result = []
        for engagement in get_authorized_engagements(Permissions.Engagement_View):
            base_findings = engagement.unaccepted_open_findings
            accepted = _accept_risks(accepted_risks, base_findings, owner)
            engagement.accept_risks(accepted)
            accepted_result.extend(accepted)
        result = RiskAcceptanceSerializer(instance=accepted_result, many=True)
        return Response(status=201, data=result.data)

For drf-yasg it works as expected/desired, getting two parameters the id and request body in the schema:

       "/api/v2/engagements/{id}/accept_risks/": {
            "post": {
                "operationId": "engagements_accept_risks_create",
                "description": "",
                "parameters": [
                    {
                        "in": "path",
                        "name": "id",
                        "schema": {
                            "type": "integer"
                        },
                        "description": "A unique integer value identifying this engagement.",
                        "required": true
                    }
                ],
                "tags": [
                    "engagements"
                ],
                "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "$ref": "#/components/schemas/AcceptedRisk"
                            }
                        },

For drf-spectacular I am getting a lot more parameters. I think these are "inherited" or discovered from the EngagementViewset where there is a filter backend that accepts all these parameters.

      "/api/v2/engagements/{id}/accept_risks/": {
            "post": {
                "operationId": "engagements_accept_risks_create",
                "description": "",
                "parameters": [
                    {
                        "in": "query",
                        "name": "active",
                        "schema": {
                            "type": "boolean"
                        }
                    },
                    {
                        "in": "query",
                        "name": "api_test",
                        "schema": {
                            "type": "boolean"
                        }
                    },
                    {
                        "in": "path",
                        "name": "id",
                        "schema": {
                            "type": "integer"
                        },
                        "description": "A unique integer value identifying this engagement.",
                        "required": true
                    },
                    {
                        "in": "query",
                        "name": "id",
                        "schema": {
                            "type": "integer"
                        }
                    },
                    {
                        "name": "limit",
                        "required": false,
                        "in": "query",
                        "description": "Number of results to return per page.",
                        "schema": {
                            "type": "integer"
                        }
                    },

....


               "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "type": "array",
                                "items": {
                                    "$ref": "#/components/schemas/AcceptedRisk"
                                }
                            }
                        },
           

I also notice pagination related schema elements in the response:

                "responses": {
                    "201": {
                        "content": {
                            "application/json": {
                                "schema": {
                                    "$ref": "#/components/schemas/PaginatedRiskAcceptanceList"
                                }
                            }
                        },
                        "description": ""
                    }
                }

Is the expected / intended behaviour? Without stating which one is correct, I think the drf-yasg scheme is what I was expecting / looks the most intuitive.

I tried using @extend_schema, but it looks like that can only be used to add extra parameters, not to remove superfluous ones.

Is there anyway to prevent this "inheritance" of parameters and other elemens into the method from the MixIn?

I am not seeing all these "extra parameters" on @action methods declared directly in the ViewSet, such as close in the above example.

Full code is at: https://github.com/valentijnscholten/django-DefectDojo/blob/cd89dac2d92f5734c7c717edce39130caf6278f3/dojo/api_v2/views.py#L115

@tfranzel
Copy link
Owner

hi, thats a lot to process 😄

technically the action filterset class is inherited from the view. you can get rid of it by forcing it to None. then the extra parameters should dissapear.

    @action(
        methods=['post'], detail=False, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer, 
        filterset_class=None, pagination_class=None
    )
    def accept_risks(self, request):

same goes for the pagination. you invoke the lurking pagination in the view by forcing the action response to many=True. disable the pagination on the action and this should dissappear.

the reason this is implemented like that is that it would be very hard to get the reverse situation on purpose, i.e. getting a paginated/filtered action response.

second option is to use @extend_schema_view(accept_risks=extend_schema(...)) on the view and not use @extend_schema in the mixin. then you have no bleed-through, but i think above solution is cleaner.

@tfranzel
Copy link
Owner

one more thing. parameter are additive as this is the most common use-case. you can however remove parameters by force with extend_schema:

OpenApiParameter('limit', exclude=True),

so 3 options but the first would be preferable. if i understood you correctly, this is indeed the intended behavior. the solution from drf-yasg is likely more intuitive, but that "heuristic" closes some doors in terms of functionality (reverse situation outlined above).

@valentijnscholten
Copy link
Author

Thanks for the quick and elaborate response. Looks like it's filter_backends that I need to set to None. With filterset_class I'm getting an exception in as_view() later on.

This works:

    @extend_schema(
        request=AcceptedRiskSerializer(many=True),
        responses={status.HTTP_201_CREATED: RiskAcceptanceSerializer(many=True)},
    )
    @action(methods=['post'], detail=True, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer,
            filter_backends=None, pagination_class=None)
    def accept_risks(self, request, pk=None):

@tfranzel
Copy link
Owner

yes that makes sense. i wrote that example from memory. django-filter depends on both classes. perfect!

@valentijnscholten
Copy link
Author

Small tweak to use empty list needed as filter_backends=None caused some issues down the road:

    @action(methods=['post'], detail=True, permission_classes=[IsAdminUser], serializer_class=AcceptedRiskSerializer,
            filter_backends=[], pagination_class=None)

@tfranzel
Copy link
Owner

closing this issue for now. feel free to comment if anything is missing or not working and we will follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants