-
-
Notifications
You must be signed in to change notification settings - Fork 351
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.
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
Support for Django Channels (ASGI) #712
Comments
Hey @Archmonger, thanks for reporting! I think ASGI support would be essential to have as well. This would also be a good target for version 6.0.0. Since we'd need to define a target setup that should work out-of-the-box or otherwise, would you mind posting example settings that should work with Axes and ASGI enabled? |
This is the GitHub repository that I've been attempting to reach compatibility with https://github.com/Archmonger/Conreq. Repo only has python dependencies. |
That's good, let's go forward from there. What other Django packages are you using by the way? It seems like the error is coming from here and the request should just be updated with the |
The none value for await login(
self.scope,
self.scope["user"],
backend="django.contrib.auth.backends.ModelBackend", # <<< This line right here
) So therefore the request never propagates up your Axes stack and all axes request attributes are never populated. Or at least that's my leading theory. I haven't looked at your team's source code and I'm not entirely familiar with the Django request stack. Here's a list of django requirements I pulled out of my requirements.txt awesome-django-timezones==0.3.0
channels==3.0.3
diskcache==5.1.0
django==3.1.5
django-htmlmin==0.11.0
django-solo==1.1.5
django-searchable-encrypted-fields==0.1.9
django-cleanup==5.1.0
daphne==3.0.1
django-project-version[git]==0.13.0
pwned-passwords-django==1.4.1
whitenoise[brotli]==5.2.0 |
Just had a thought. Perhaps the to solution to "only one auth backend can be set" within channels is having the axes backend wrap one or more backends. For example a through a |
@Archmonger wrapping is actually a nice idea, if it can be done cleanly! Would you be interested in inspecting this on the code level? The authentication backend API specification from Django docs would need to be followed and ASGI implementation checked as well. |
Sure. I'll consider a PR on axes as a part of my task list. I have a few higher priority cards so I'll get to it within roughly a month. |
My work schedule slowed down my OSS work. Still aiming to tackle this as a PR but unsure what my timeline looks like. |
Roger that @Archmonger 👍 No need to worry about timelines since Axes is a voluntary FOSS project. |
Just jotting down some ideas I had today in regards to the implementation of this future PR... We can automate the whole
|
Forcibly setting the backend could lead to hard-to-debug problems in some stacks. Could there be alternatives to this approach or could we e.g. introduce a new flag for configuring another authentication backend separately in Axes that we could then invoke from e.g. a new Axes ASGI authentication wrapper backend? |
Forcibly setting the values isn't necessary, I just thought it might be a clever way to automate some setup. The alternative is: AUTHENTICATION_BACKENDS = ['axes.backends.AxesBackend']
AXES_BACKENDS = [ 'example.one', 'example.two' ] Reusing the This wouldn't really improve debug capabilities though, I'd say it's equivalent to the last method except isn't not automating the configuration. I don't really think adding axes in the loop in either scenarios would affect stack traces in any harmful way. The user would just see an axes stack preceding the authentication stack that suffers from an exception. |
I'd rather have an added configuration key in the settings and think that "explicit is better than implicit" in this case. If we are using ASGI and have added the extra Axes settings, it might also be feasible to add system checks in the Django checks framework for making sure that the user has correctly configured the authentication stack i.e. has the |
Agreed, adding some checks would be a good addition. We definitely want to raise |
Currently, Django Axes results in an exception when used with the login functionality within Django-Channels
channels.auth
.If multiple backends are defined in
settings.py
, Django Channels requires the user to define a backend to authenticate with. For example,But when the user defines this backend, it breaks the Axes authentication stack.
Perhaps it's worth discussing with the Django Channels team whether a workaround can be reached with their
login
function.The text was updated successfully, but these errors were encountered: