Skip to content

Fix argument order in the async middleware lockout response - #1494

Merged
camilonova merged 1 commit into
django-commons:masterfrom
MaxFreedomPollard:fix-async-middleware-lockout-arguments
Oct 30, 2026
Merged

camilonova merged 1 commit into
django-commons:masterfrom
MaxFreedomPollard:fix-async-middleware-lockout-arguments

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

What does this PR do?

AxesMiddleware.__acall__ passes the wrong arguments to axes.helpers.get_lockout_response, so under ASGI every lockout response is built from the credentials dict in the slot meant for the original response, with no credentials at all.

get_lockout_response is declared as get_lockout_response(request, original_response=None, credentials=None) at axes/helpers.py:515. The sync branch calls it correctly at axes/middleware.py:51. The async branch at axes/middleware.py:61-65 still calls (request, credentials), which was right until 04fd39f inserted original_response as the second parameter; that commit updated __call__ and only reformatted __acall__.

Two documented behaviours break as a result. A custom AXES_LOCKOUT_CALLABLE receives the credentials dict as original_response and None as credentials, while docs/4_configuration.rst documents def generate_lockout_response(request: HttpRequest, original_response: HttpResponse, credentials: dict). With no lockout callable configured, get_lockout_response then runs get_client_username(request, None) and get_failure_limit(request, None), so an AXES_USERNAME_CALLABLE that reads the credentials dict raises AttributeError: 'NoneType' object has no attribute 'get' and the client gets a 500 instead of the lockout page.

The fix is one line: the async branch now passes (request, response, credentials) like the sync branch.

__acall__ had no test coverage, so this adds three tests to tests/test_middleware.py. They drive the middleware through async_to_sync(AxesMiddleware(get_response)) with an async get_response, so the real coroutine dispatch in __call__ runs rather than being bypassed.

Verified on Python 3.11 with Django 5.2 against sqlite, the configuration test.yml runs.

pytest gives 380 passed, 19 subtests passed. On an unmodified master checkout it gives 377 passed, 19 subtests passed.

With axes/middleware.py reverted to master and the new tests in place, both regression tests fail: test_async_lockout_response_with_axes_callable_username with AttributeError: 'NoneType' object has no attribute 'get' raised from axes/helpers.py:546, and test_async_lockout_callable_gets_original_response_and_credentials with AssertionError: Lists differ: [({...: 'username'}, None)] != [(<HttpResponse status_code=200, "text/html; charset=utf-8">, {...: 'username'})], which shows the credentials landing in the original_response position.

black --check --diff axes leaves 41 files unchanged, prospector axes finds 0 messages, and mypy axes finds no issues in 41 source files. mypy . reports one error on the pre-existing get_username helper in tests/test_middleware.py; it sits at line 10 on an unmodified checkout and at line 11 here only because this adds an import above it.

No documentation change is needed: docs/4_configuration.rst and docs/5_customization.rst already describe the three argument contract that this restores. I have left CHANGES.rst alone because every entry there landed in a Version x.y.z commit, but I am glad to add one if you would rather contributors did that.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

AxesMiddleware.__call__ passes three arguments to
axes.helpers.get_lockout_response, but AxesMiddleware.__acall__ still
passes two. get_lockout_response is declared in axes/helpers.py as
get_lockout_response(request, original_response=None, credentials=None),
so the async path binds the credentials dict to original_response and
leaves credentials as None.

Under ASGI that breaks two documented behaviours. AXES_LOCKOUT_CALLABLE
receives the credentials dict in the slot where
docs/4_configuration.rst says it receives the original HttpResponse, and
it never receives the credentials at all. With no lockout callable
configured, get_client_username and get_failure_limit are called with
credentials set to None, so an AXES_USERNAME_CALLABLE that reads the
credentials dict raises AttributeError instead of rendering the lockout
page.

The async branch now passes (request, response, credentials) like the
sync branch does. AxesMiddleware.__acall__ had no test coverage, so this
adds three tests for it.
@camilonova
camilonova merged commit 8a8cf96 into django-commons:master Oct 30, 2026
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants