swsnu / swppfall2021

Learning Software Engineering By Building Web Services
28 stars 19 forks source link

Question about safe/unsafe HTTP methods treating #138

Open veldic opened 2 years ago

veldic commented 2 years ago

When we were working on our backend code, we created(actually copy & paste of hw4 skeleton code) token providing function.

@ensure_csrf_cookie
def token(request):
    if request.method == 'GET':
        return HttpResponse(status=204)
    else:
        return HttpResponseNotAllowed(['GET'])

But SonarCloud says

to us with Quality Gate Failed.

I think we can solve this warning by adding @require_GET annotation to this function (SonarCloud recommended us to do so). But I don't have any idea how to deal with the function that will use safe('GET') and unsafe('POST', 'DELETE', ...) methods together.

Is there any proper way to treat this warning?

kdh0102 commented 2 years ago

Sorry for the belated response. The proper way to handle the warnings is to follow the coding practices suggested in the recommendation tab, which you already have seen. the function with both safe('GET') and unsafe('POST', 'DELETE', ...) methods itself violates the warning: Allowing both safe and unsafe HTTP methods is security-sensitive.

clarkindy commented 2 years ago

@kdh0102, I do not think the answer solves the original question:

But I don't have any idea how to deal with the function that will use safe('GET') and unsafe('POST', 'DELETE', ...) methods together.

Our team has the same problem, and I think other teams are having the same issue too.

We want our back-end application to handle requests to a URL differently based on what HTTP method was used. For example, HW4 specifies us to do so:

  • GET /api/article/: Response with a JSON having a list of dictionaries for each article's title, content, and author. The value of the author must be the id of the author but not her username. You don't need to include the id of each article to the response.
  • POST /api/article/: Create an article with the information given by request JSON body and response with 201. Posted article (with it's assigned id) should be included in response's content as JSON format.

Therefore, we have to allow multiple methods in one view function. We can see this in the code of our Django practice session repository:

@csrf_exempt
def hero_list(request):
    if request.method == "GET":
        ...
    elif request.method == "POST":
        ...
    else: 
        return HttpResponseNotAllowed(["GET", "POST"])

The problem is, as you mentioned, SonarCloud does not allow a view function to accept both safe and unsafe methods. I think this could be a problem if the view function does the same thing regardless of which HTTP method was used. This is not our case, since we strictly divide our function's behavior with if-else statements. However, SonarCloud does not seem to care about our view function's internal structure, and only cares about whether the function accepts both safe and unsafe methods.

There are potential solutions:

So I want to ask whether there is any way to handle this issue only using what we learned in our class.

kdh0102 commented 2 years ago

@clarkindy Thanks for sharing your experiences and opinions.

clarkindy commented 2 years ago

Thank you for your answer.

The hotspots tell you to review whether to apply the fix. It is your decision how to handle the issues.

Is there any way to pass quality gate without handling security hotspots?

By the way, class-based views are a way to generate views by making sub-classes of django.views.View. You can find more information in Django documentation.

kdh0102 commented 2 years ago

You can manually review each hotspot in the SonarCloud page.

clarkindy commented 2 years ago

Oh, I did not know that. Thank you!