Skip to content

Fix: Handle AttributeError in IntrospectTokenView #1562

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Fix: Handle AttributeError in IntrospectTokenView #1562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a97350a
Fix: Handle AttributeError in IntrospectTokenView
dawidwolski-identt Apr 9, 2025
d8e7efa
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2025
700264f
Merge branch 'master' into handle_attribute_error_in_get_token_response
dawidwolski-identt May 5, 2025
d1a5bdf
Merge branch 'master' into handle_attribute_error_in_get_token_response
dawidwolski-identt May 12, 2025
ad927b6
Return HTTP 400 if no token is provided
dawidwolski-identt May 22, 2025
c5c7b9c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 22, 2025
a3f6030
json response
dawidwolski-identt May 22, 2025
02884ad
Merge branch 'handle_attribute_error_in_get_token_response' of github…
dawidwolski-identt May 22, 2025
e35e876
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions oauth2_provider/views/introspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ def get_token_response(token_value=None):
)
except ObjectDoesNotExist:
return JsonResponse({"active": False}, status=200)
except AttributeError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@dopry dopry Jun 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return JsonResponse(
{"error": "invalid_request", "error_description": "Token parameter is missing."},
status=400,
)
else:
if token.is_valid():
data = {
Expand Down
14 changes: 14 additions & 0 deletions tests/test_introspection_view.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ def test_view_post_notexisting_token(self):
},
)

def test_view_post_no_token(self):
"""
Test that when you pass no token HTTP 400 is returned
"""
auth_headers = {
"HTTP_AUTHORIZATION": "Bearer " + self.resource_server_token.token,
}
response = self.client.post(reverse("oauth2_provider:introspect"), **auth_headers)

self.assertEqual(response.status_code, 400)
content = response.json()
self.assertIsInstance(content, dict)
self.assertEqual(content["error"], "invalid_request")

def test_view_post_valid_client_creds_basic_auth(self):
"""Test HTTP basic auth working"""
auth_headers = get_basic_auth_header(self.application.client_id, CLEARTEXT_SECRET)
Expand Down
11 changes: 6 additions & 5 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ envlist =
docs,
lint,
sphinxlint,
py{38,39,310,311,312}-dj42,
py{310,311,312}-dj50,
py{310,311,312}-dj51,
py{310,311,312}-djmain,
py{38,39,310,311,312,313}-dj42,
py{310,311,312,313}-dj50,
py{310,311,312,313}-dj51,
py{310,311,312,313}-djmain,
py39-multi-db-dj-42

[gh-actions]
Expand All @@ -18,6 +18,7 @@ python =
3.10: py310
3.11: py311
3.12: py312
3.13: py313

[gh-actions:env]
DJANGO =
Expand Down Expand Up @@ -54,7 +55,7 @@ deps =
passenv =
PYTEST_ADDOPTS

[testenv:py{310,311,312}-djmain]
[testenv:py{310,311,312,313}-djmain]
ignore_errors = true
ignore_outcome = true

Expand Down