Use APIView.permission_denied to handle 401 and 403 http errors

[hashlash]

Reference: https://www.django-rest-framework.org/api-guide/authentication/#unauthorized-and-forbidden-responses

I change the test viewset to use BasicAuthentication because the DRF default will always return 403 since the first auth class is SessionAuthentication.

[dfunckt]

Is permission_denied an attribute of DRF's ViewSet? Is this possible to result in an AttributeError?

Also, what are the implications of switching to the new behavior for users that depend on PermissionDenied be raised and is there a way to get the previous behavior?

[hashlash]

Is permission_denied an attribute of DRF's ViewSet? Is this possible to result in an AttributeError?

permission_denied is defined in APIView, and since ViewSet is a subclass of APIView, the function should always works.

[hashlash]

Also, what are the implications of switching to the new behavior for users that depend on PermissionDenied be raised ...?

The view will return the status code correctly based on whether the request is authenticated (the credentials are valid, but the user doesn't have the correct permission) or not (the credentials is not provided or invalid).

Quoting from DRF docs:

HTTP 401 responses must always include a WWW-Authenticate header, that instructs the client how to authenticate. HTTP 403 responses do not include the WWW-Authenticate header.

The kind of response that will be used depends on the authentication scheme. Although multiple authentication schemes may be in use, only one scheme may be used to determine the type of response. The first authentication class set on the view is used when determining the type of response.

Note that when a request may successfully authenticate, but still be denied permission to perform the request, in which case a 403 Permission Denied response will always be used, regardless of the authentication scheme.

[hashlash]

... is there a way to get the previous behavior?

For current implementation, the users needs to override the initial() method and replace the NotAuthenticated exception with PermissionDenied (using try except block).

As an alternative implementation, we could allow users to opt in/out of this change using a Django setting.

[dfunckt]

The view will return the status code correctly based on whether the request is authenticated (the credentials are valid, but the user doesn't have the correct permission) or not (the credentials is not provided or invalid).

I understand that, what I mean is that this is changing a behavior that existing users may (?) depend on in a way that might cause crashes at runtime so I'm trying to understand the (potential) impact this change may have.

[hashlash]

Hmm, I could only think of a mismatch of HTTP status code checks (most likely on client side) led to unhandled (or incorrect) condition?

Something like this:

if status_code == 200:
    ...
elif status_code == 404:
    ...
elif status_code == 403:
    ...
else:
    return "Unhandled message"

I don't think that would lead to a crash.

[hashlash]

Hi @dfunckt, would you prefer to let the user opt-in for this patch for now? I was thinking that you might consider this a breaking change, so enforcing it on the next major release might be more suitable. But still, I think this is a bug, so it should be applied with a patch release (or at least a minor release).