Skip to content

Commit dbf7c74

Browse files
committed
Fix #43: Ensure fallback validator is called with correct value.
This also fixes the tests for hte fallback behavior, which were incorrect and as a result masked the fact that the fallback was being used incorrectly in the sync path. Thanks Aguswahyudi Steven for noticing the bug with the fallback, and for the initial patch.
1 parent cb0f978 commit dbf7c74

File tree

3 files changed

+35
-15
lines changed

3 files changed

+35
-15
lines changed

src/pwned_passwords_django/middleware.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def _scan_payload_sync(request: http.HttpRequest) -> typing.List[str]:
8383
"Falling back to Django CommonPasswordValidator due "
8484
"to error contacting Pwned Passwords."
8585
)
86-
return [key for key in keys_to_search if _fallback(key)]
86+
return [key for key in keys_to_search if _fallback(request.POST[key])]
8787

8888

8989
@sync_and_async_middleware
@@ -195,7 +195,7 @@ def middleware(request: http.HttpRequest) -> http.HttpResponse:
195195
containing likely passwords against the Pwned Passwords database.
196196
197197
"""
198-
request.pwned_passwords = {}
198+
request.pwned_passwords = []
199199
if request.method == "POST":
200200
request.pwned_passwords = _scan_payload_sync(request)
201201
response = get_response(request)

tests/test_middleware.py

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ async def test_custom_regex_async(self):
241241

242242
def test_error_handler(self):
243243
"""
244-
The middleware will catch a PwnedPasswordsError and set
245-
``request.pwned_passwords`` based on CommonPasswordValidator.
244+
If the sync middleware fails and the submitted password is not in
245+
Django's common passwords list, request.pwned_passwords will be empty.
246246
247247
"""
248248
sync_mock, _ = self.api_error_mocks()
@@ -251,14 +251,38 @@ def test_error_handler(self):
251251
self.test_clean, data={"password": get_random_string(length=20)}
252252
)
253253

254+
def test_error_handler_bad_password(self):
255+
"""
256+
If the sync mdidleware fails and the submitted password is in Django's
257+
common passwords list, the request.pwned_passwords list is still
258+
correctly populated.
259+
260+
"""
261+
sync_mock, _ = self.api_error_mocks()
262+
with mock.patch("pwned_passwords_django.api.check_password", sync_mock):
263+
self.client.post(self.test_breach, data={"password": "password"})
264+
254265
async def test_error_handler_async(self):
255266
"""
256-
The async middleware will catch a PwnedPasswordsError and set
257-
``request.pwned_passwords`` to an empty dictionary.
267+
If the async middleware fails and the submitted password is not in
268+
Django's common passwords list, request.pwned_passwords will be empty.
258269
259270
"""
260-
async_mock, _ = self.api_error_mocks()
271+
_, async_mock = self.api_error_mocks()
261272
with mock.patch("pwned_passwords_django.api.check_password_async", async_mock):
262273
await self.async_client.post(
263274
self.test_clean_async, data={"password": get_random_string(length=20)}
264275
)
276+
277+
async def test_error_handler_async_bad_password(self):
278+
"""
279+
If the async mdidleware fails and the submitted password is in Django's
280+
common passwords list, the request.pwned_passwords list is still
281+
correctly populated.
282+
283+
"""
284+
_, async_mock = self.api_error_mocks()
285+
with mock.patch("pwned_passwords_django.api.check_password_async", async_mock):
286+
await self.async_client.post(
287+
self.test_breach_async, data={"password": "password"}
288+
)

tests/urls.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ async def async_view(request):
2727
return HttpResponse("Content.")
2828

2929

30-
def breach_count(request, field):
30+
def breach(request, field):
3131
"""
3232
A view which asserts that it received a compromised password, in the given
3333
``field``.
@@ -39,7 +39,7 @@ def breach_count(request, field):
3939
return HttpResponse("Content.")
4040

4141

42-
async def async_breach_count(request, field):
42+
async def async_breach(request, field):
4343
"""
4444
An async view which asserts that it received a compromised password, in the
4545
given ``field``.
@@ -84,14 +84,10 @@ async def async_clean(request):
8484
async_view,
8585
name="pwned-middleware-async",
8686
),
87-
path(
88-
"pwned-passwords-django/tests/<str:field>/",
89-
breach_count,
90-
name="pwned-breach",
91-
),
87+
path("pwned-passwords-django/tests/<str:field>/", breach, name="pwned-breach"),
9288
path(
9389
"pwned-passwords-django/tests/async/<str:field>/",
94-
async_breach_count,
90+
async_breach,
9591
name="pwned-breach-async",
9692
),
9793
path(

0 commit comments

Comments
 (0)