Fix: Handle AttributeError in IntrospectTokenView

[dawidwolski-identt]

Description of the Change

Handle AttributeError in get_token_response. If there is no token parameter in introspect endpoints they raise AttributeError.

Checklist

• PR only contains one change (considered splitting up PR)
• unit-test added
• documentation updated
• CHANGELOG.md updated (only for user relevant changes)
• author name in AUTHORS

[dawidwolski-identt]

Contribution guideline says

Make sure to request a review by assigning Reviewer jazzband/django-oauth-toolkit.

but I do not have permission to set assignee.

[dopry]

I'm not 100% sure this is the correct handling. Per https://datatracker.ietf.org/doc/html/rfc7662#section-2.3,

Note that a properly formed and authorized query for an inactive or
otherwise invalid token (or a token the protected resource is not
allowed to know about) is not considered an error response by this
specification. In these cases, the authorization server MUST instead
respond with an introspection response with the "active" field set to
"false" as described in Section 2.2.

I'm not sure that a missing token would qualify as properly formed. I feel that a 400 might be the proper response in this case since the request is missing the required 'token' parameter. My instinct says a present, but invalid or blank token, i.e. a body of

token=

would return an active: false, but missing token= altogether would be malformed and we should let the caller know they're sending a malformed request, since there is a bug in their implementation. I feel returning a 200 active: false in the case of a malformed request may give consumers/developers confidence in a broken client implementation.

[dawidwolski-identt]

@dopry I agree with you. I've changed the response to follow https://datatracker.ietf.org/doc/html/rfc6749#section-5.2

[dopry]

What if something other than the token raised the attribute error? Is there a way to proactively test the token so we avoid the overhead of exception handling.

[dawidwolski-identt]

I think it would be better to change the error message to be more generic and fix the bug that causes the app to crash. Then, if necessary, specify a message. WDYT?

[dopry]

Is there a way to check if a token was provided before get_token_response is called? Are there existing hooks for validation where this could be caught in advance?

[dawidwolski-identt]

I think it is a proper place to check for the presence of a token. It's the first place where the token is accessed. To move this check one level up, we can use request.GET.get("token", None) without a default, but then we need to handle the missing token twice.

[dopry]

I see both get and post return self.get_token_response(request.[GET|POST].get("token", None))

Why not check if token_value is None: before calculating a checksum, rather than waiting on token_value.encode to raise an AttributeError and avoid the overhead of handling an exception unnecessarily.