Closed solartune closed 1 year ago
Hey @solartune! I think this might have to do with the way DRF handles authentication and it's custom authentication backends. Are you using the defaults with it? Have you checked the integration documentation?
That integration should only have to be done for DRF < 3.7 though? I'm experiencing similar problems, the lockout message is never bubbled up to DRF, using base token authentication views.
Hi @aleksihakli! Thank you for your answer! Yes, as @ckcollab already mentioned, according to the docs it should be the option only for DRF < 3.7 but I'm using DRF 3.9. Do you think we should try something specifically for DRF higher than 3.7?
Have you inspected the DRF token authentication and how it interacts with the stock Django authentication stack?
I think that this has more to do with the DRF and Axes not interacting correctly.
Could you post your full DRF, Axes and Django authentication configuration for reference?
Have you inspected the DRF token authentication and how it interacts with the stock Django authentication stack?
We haven't dug into DRF yet, we have tried djoser instead of DRF token and experience similar problems though. Will dig into DRF throughout next week.
In the mean time, I believe these are all the relevant pieces?
# settings.py
INSTALLED_APPS = (
'axes',
)
AUTHENTICATION_BACKENDS = [
'axes.backends.AxesBackend',
'django.contrib.auth.backends.ModelBackend',
]
MIDDLEWARE = (
...
# AxesMiddleware should be the last middleware in the MIDDLEWARE list.
# It only formats user lockout messages and renders Axes lockout responses
# on failed user authentication attempts from login views.
# If you do not want Axes to override the authentication response
# you can skip installing the middleware and use your own views.
'axes.middleware.AxesMiddleware',
)
AXES_LOCKOUT_TEMPLATE = 'lockout_error.json'
# if you change this cooloff time, update `lockout_error.json`
AXES_COOLOFF_TIME = 4
# !!! Things work fine if AXES_ONLY_USER_FAILURES is left default, False
AXES_ONLY_USER_FAILURES = True
AUTH_USER_MODEL = 'profiles.User'
# urls.py
from rest_framework.authtoken.views import ObtainAuthToken
urlpatterns = [
re_path(r'api/auth/token/login/?$', ObtainAuthToken.as_view(), name="login"),
]
lockout_error.json
template:
{"detail": "Account locked for 4 hours: too many login attempts. Contact an admin to unlock your account."}
# requirements.txt
Django==2.2.10
djangorestframework==3.11.0
django-axes==5.3.1
The problem for us seems to be: AXES_ONLY_USER_FAILURES
which seems to behave properly but does not bubble up the error to the frontend, instead it returns normal error:
{"non_field_errors":["Unable to log in with provided credentials."]}
I don't have time to look into this right now personally, but it would be great to get this sorted out and fixed! If you can look into this feel free to open a PR.
Better integration tests with DRF are on the backlog and would fit this case very well.
This relates to #411, any help with improving the DRF integrations and tests for it would be awesome!
I did some investigation recently. So, django request and drf request aren't the same. It looks like drf request contains attributes of the django request but not vice versa. So, the attributes which set in the package to the drf request aren't visible in the middleware where the django request is used.
Might be I missed smth but this is my current impression.
Took me about 3 hours to realize that the error message is wrong 🙈 Would be happy about any updates happening!
I have found a solution for the problem, btw. But don't have time to make a PR right now. Hopefully, will do that this week.
FYI @aleksihakli
We fixed the problem in another project. We changed the validate()
in the LoginSerializer
like this:
from rest_framework.exceptions import ValidationError as DrfValidationError
def validate(self, attrs):
try:
return super().validate(attrs)
except (ValidationError, DrfValidationError) as e:
try:
# Catch case of ValidationError because of Axes lockout and raise correct validation error
credentials = {
'username': attrs['username']
}
if AxesProxyHandler.is_locked(self._context['request'], credentials):
raise DrfValidationError(settings.CUSTOM_AXES_LOCKOUT_ERROR_MSG)
raise e
# In case username is not defined in the request
except KeyError:
raise e
@solartune Is this the road you took?
I've modified the original middleware :)
It's either possible to use my solution in some project as a custom middleware or to include this change to the package (which is my plan if it will be accepted).
Here is an example how it can look like (please notice that I use a custom get_failures
here but in fact, it was just copied from the original code. My plan is to make it common so it will be written in a single place. So I post it just as an example, not as a proper solution yet.):
from typing import Callable
from django.conf import settings
from axes.helpers import get_lockout_response, get_failure_limit
from django.db.models import Sum
from axes.attempts import get_user_attempts
from axes.handlers.proxy import AxesProxyHandler
def get_failures(request, credentials: dict = None) -> int:
attempts_list = get_user_attempts(request, credentials)
attempt_count = max(
(
attempts.aggregate(Sum("failures_since_start"))[
"failures_since_start__sum"
]
or 0
)
for attempts in attempts_list
)
return attempt_count
class AxesMiddleware:
"""
Middleware that calculates necessary HTTP request attributes for attempt monitoring
and maps lockout signals into readable HTTP 403 Forbidden responses.
This middleware recognizes a logout monitoring flag in the request and
and uses the ``axes.helpers.get_lockout_response`` handler for returning
customizable and context aware lockout message to the end user if necessary.
To customize the lockout handling behaviour further, you can subclass this middleware
and change the ``__call__`` method to your own liking.
Please see the following configuration flags before customizing this handler:
- ``AXES_LOCKOUT_TEMPLATE``,
- ``AXES_LOCKOUT_URL``,
- ``AXES_COOLOFF_MESSAGE``, and
- ``AXES_PERMALOCK_MESSAGE``.
"""
def __init__(self, get_response: Callable):
self.get_response = get_response
def __call__(self, request):
AxesProxyHandler.update_request(request)
response = self.get_response(request)
if hasattr(response, "renderer_context"):
username = response.renderer_context["request"].data["username"]
credentials = {"username": username}
failures_since_start = get_failures(request, credentials)
if (
settings.AXES_LOCK_OUT_AT_FAILURE
and failures_since_start
>= get_failure_limit(request, credentials)
):
request.axes_locked_out = True
if getattr(request, "axes_locked_out", None):
response = get_lockout_response(request) # type: ignore
return response
The idea behind that is that django request has no idea about changes made to DRF request (as I explained it earlier) and hence the original code doesn't work properly with DRF. Here I just do checks which are normally done in a different place if we take a look at the original code.
I haven't tested it from different angles yet but a few tests with DRF showed that this is a working solution. Maybe @aleksihakli you could comment what do you think of this? Shall I continue with the idea?
@solartune Cool, will give it a shot. Would be awesome if this would end up in the package. Take care that the username is often the email field and therefore cannot be set statically.
Update: Didn't get it to work quickly. Hopefully this will make its way into the package 😃
Update: Didn't get it to work quickly. Hopefully this will make its way into the package 😃
Could you please provide details? What doesn't work in your case?
At first the email instead of the username is not present (I did not investigate) and there was an attribute error deep down in some of the used functions. As I said, just gave it a quick shot.
@solartune I think it would be awesome to support DRF correctly as well and I think your approach looks valid. I guess the two things to take into account are that Axes
As long 1. is OK and we don't degrade or "break" current functionality I'm all for trying out different approaches and seeing what works best!
Hi guys,
I've notices the same issue as @solartune with the DRF request and HttpRequest objects not being the same. So even though the axes_locked_out
is correctly applied to the DRF request, this is not transferred to the HTTPRequest which is check in the middleware response path.
Additionally, I'm also having the issue that even though Axes correctly tag the DRF request with axes_locked_out==True
, which subsequently breaks the iteration of backends (Axes-backend being the first), the request is still being passed on to the remaining authentication backends which successfully authenticate the request. This leaves the request in a state where it's both "locked" and authenticated at once. Worth noting that I'm additionally using Djoser, which could be causing additional problems.
Expected behaviour would be that after multiple failed requests that triggers the lockout limit, a request with correct credentials should still not be passed to the subsequent authentication backends, but rather just be locked out independent of the credentials passed.
For DRF I use the following workaround. I do not install AxesMiddleware
and instead connect to user_locked_out
signal
from axes.signals import user_locked_out
from rest_framework.exceptions import PermissionDenied
def deny_access(**kwargs):
raise PermissionDenied('Too many failed login attempts')
user_locked_out.connect(deny_access)
Thanks for the quick reply, @momyc that is seemingly working great with my initial testing, many thanks!
@aleksihakli Is this additional user_locked_out
signal something that should be contributed to the library as well? Could do some additional digging to understand why the lockout flow doesn't seem to work as intended in my case.
The problem is that DRF wraps original request and pass it around, e.g. Axes handler sets that wrapper's attribute while middleware checks original request.
The signal approach looks valid as well. We are using a specialized AxesBackendPermissionDenied
in the project to signify the logout state, maybe emitting that could be a good approach in the handlers?
@solartune and others, please note that this can be resolved in 5.10.0 by attaching the correct signal handler for DRF
@solartune and others, please note that this can be resolved in 5.10.0 by attaching the correct signal handler for DRF
But there is an edge case, if we set AXES_ENABLE_ACCESS_FAILURE_LOG
=True
it will not create the AccessFailureLog
if we raise a PermissionDenied
in the user_locked_out
signal handler function.
Because it stops the execution at https://github.com/jazzband/django-axes/blob/2c4a4f6f8db58648ff8b34d8ca9b06c20e8d1a9c/axes/handlers/database.py#L238 and doesn't go down to https://github.com/jazzband/django-axes/blob/2c4a4f6f8db58648ff8b34d8ca9b06c20e8d1a9c/axes/handlers/database.py#L246
I managed to solve this problem by subscribing to Django's user_login_failed
signal:
@receiver(user_login_failed)
def user_login_failed_handler(*args, **kwargs):
"""Receives the user login failed signal and raise DRF ``PermissionDenied`` exception if the user is locked out."""
if settings.AXES_ENABLED is False:
return
request = kwargs['request']
if AxesProxyHandler.is_locked(request, kwargs['credentials']):
raise PermissionDenied(get_lockout_message())
Hi guys, I’ve tried to use django-axes with django rest framework. For some reason when a user reaches a limit of login attempts he continues receiving 400 code instead of 403. I was able to find a workaround in my view to return 403 but this probably should work out of the box, right?
I followed the documentation when I was installing the package.
After installing I specified a few addition settings:
I use the latest (5.2.2) version of the package. Django version is 2.2.10 DRF version is 3.9.0