Comments (13)
Yes, that does seem pretty weird. I could think of a situation like the following:
- User attempts to log-in with username/password.
- User gets to the 2FA page and realizes they don't have their phone (or whatever TOTP device they use).
- User closes tab.
On a "shared" computer, someone else could re-open the page and enter the token without knowing the original password.
I think it is an edge-case, but is unexpected behavior. Expiring the session quickly is a probably a reasonable solution.
from django-allauth-2fa.
So I've spent two days hunting down a bug related to this one. The behavior was:
- Go to sign in page.
- Enter username and password.
- Redirected to 2FA prompt.
- Enter 2FA (TOTP) code.
- Redirected to sign in page.
- Repeat from 2.
I managed to track it down to AllauthTwoFactorMiddleware.process_request()
deleting allauth_2fa_user_id
from the session. And I just found out why; this is what the runserver
output typically looks like for that flow:
[04/Nov/2019 09:36:04] "GET /accounts/login/ HTTP/1.1" 200 20822
[04/Nov/2019 09:36:04] "GET /static/CACHE/css/cookies.968d7e16bfd2.css HTTP/1.1" 200 720
[04/Nov/2019 09:36:04] "GET /static/CACHE/css/project.06d33650b1ef.css HTTP/1.1" 200 378
[04/Nov/2019 09:36:04] "GET /static/CACHE/css/mwm.42f545798894.css HTTP/1.1" 200 121339
[04/Nov/2019 09:36:32] "POST /accounts/login/ HTTP/1.1" 302 0
[04/Nov/2019 09:36:33] "GET /accounts/two-factor-authenticate HTTP/1.1" 200 19551
[04/Nov/2019 09:36:33] "GET /static/CACHE/css/project.06d33650b1ef.css HTTP/1.1" 200 378
[04/Nov/2019 09:36:33] "GET /static/CACHE/css/cookies.968d7e16bfd2.css HTTP/1.1" 200 720
[04/Nov/2019 09:36:33] "GET /static/CACHE/css/mwm.42f545798894.css HTTP/1.1" 200 121339
[04/Nov/2019 09:36:33] "GET /apple-touch-icon.png HTTP/1.1" 302 0
[04/Nov/2019 09:36:58] "POST /accounts/two-factor-authenticate HTTP/1.1" 302 0
[04/Nov/2019 09:36:58] "GET /accounts/login/ HTTP/1.1" 200 20822
[04/Nov/2019 09:36:58] "GET /static/CACHE/css/project.06d33650b1ef.css HTTP/1.1" 200 378
[04/Nov/2019 09:36:58] "GET /static/CACHE/css/cookies.968d7e16bfd2.css HTTP/1.1" 200 720
[04/Nov/2019 09:36:58] "GET /static/CACHE/css/mwm.42f545798894.css HTTP/1.1" 200 121339
That GET /apple-touch-icon.png
is what's wiping the key. Why is that GET there at all? It's a redirect to the static path; because of legacy reasons, browsers expect your icons at the root level. So I have a bunch of path(..., RedirectView.as_view(url=...))
entries in my site urlpatterns
.
I'm not sure what the problem with "they're half-logged-in" is that requires no other HTTP activity at all until the POST /accounts/two-factor-authenticate
step.
I get the "On a "shared" computer, someone else could re-open the page and enter the token without knowing the original password" case from the prior comment, but that trick works one and only if the bad guy has control of the TOTP device. In my case it's a non-issue, because my session cookie expiration is set to 0 so the cookie is dead once the tab is closed. I guess someone can restore the cookie by reopening the tab from history?
So I'm not sure how to fix this (I can replace AllauthTwoFactorMiddleware
with my own code, but the question is what that code should be), but the current behavior is broken for me. It seems the "half-logged-in" state needs better definition. Maybe it needs to be passed a list of "safe" URL name patterns (which don't wipe the login) in a setting?
from django-allauth-2fa.
Looking through django-otp
, there's a OTPAuthenticationFormMixin
in forms
that could put everything on the login page and possibly fix this, but I'm at a loss as to where to mix it in.
from django-allauth-2fa.
Looking through
django-otp
, there's aOTPAuthenticationFormMixin
informs
that could put everything on the login page and possibly fix this, but I'm at a loss as to where to mix it in.
Since the login forms aren't provided by django-allauth-2fa that could be hard. It could always be documented, of course.
Is there a reduced set of steps to reproduce this, by the way?
from django-allauth-2fa.
I think the minimum set of steps to reproduce would be something like:
- Set up a minimal Django project with django-allauth-2fa and its dependents. Leave the
socialaccount
stuff out ofallauth
. Also set it up to userunserver
and to serve its own static data, e.g.STATIC_ROOT = '/static/'
. This last bit is part of the trigger for the bug. - Create your superuser.
- Add some graphics and stuff to the basic page template that's used for all your pages. A stie logo in the menu bar, for example. Also add some favicon files. This is the second part of the trigger.
- Fire up
runserver
, create a new user, and set up Google Authenticator (or equivalent) TOTP for it. - Logout superuser and login your new user.
- After the username/password page and before the enter token page, you'll see several accesses to
/static/
. These are the accesses that discard the session data that the username/password step created. - Entering the TOTP token sends you back to the username/password page, since the session data was discarded.
from django-allauth-2fa.
The /static/
stuff might not be the trigger. Safari, at least, looks for /apple-touch-icon.png
, though there are equivalent Android and other files that can trigger it the same way.
Here's the part of my urls.py
that handles all the favicons:
from django.conf import settings
from django.urls import include, path, reverse
from django.conf.urls.static import static
from django.contrib import admin, sitemaps
from django.contrib.sitemaps.views import sitemap
from django.views.generic import TemplateView, RedirectView
# ...
def favicon_path(name, pattern=None):
target = name if pattern is None else pattern
url = f'{settings.STATIC_URL}images/favicons/{target}'
return path(name, RedirectView.as_view(url=url))
# ...
urlpatterns = [
# ...
path("accounts/", include("allauth_2fa.urls")),
path("accounts/", include("allauth.urls")),
# ...
# Favicons
favicon_path('android-chrome-<dims>.png', 'android-chrome-%(dims)s.png'),
favicon_path('apple-touch-icon<dims>.png', 'apple-touch-icon%(dims)s.png'),
favicon_path('apple-touch-icon.png', 'apple-touch-icon.png'),
favicon_path('browserconfig.xml'),
favicon_path('favicon-16x16.png'),
favicon_path('favicon-32x32.png'),
favicon_path('favicon.ico'),
favicon_path('mstile<dims>.png', 'mstile%(dims)s.png'),
favicon_path('safari-pinned-tab.svg'),
favicon_path('site.webmanifest'),
] + static(
settings.MEDIA_URL, document_root=settings.MEDIA_ROOT
)
from django-allauth-2fa.
As noted last November, it's AllauthTwoFactorMiddleware.process_request()
making the mistake in line 23:
if not match.url_name or not match.url_name.startswith(
'two-factor-authenticate'):
There needs to be a check here for "harmless" paths that shouldn't reset the login flow. The question is how to get these URLs to this test:
- a setting with a list of additional "startswith" patterns for the URL name,
- a setting with a list of regular expressions,
- a function (or path to a function) to be called to provide the additional test, or
- require the module user to wrap or monkeypatch this middleware with that test (requiring
resolve()
to be called twice).
from django-allauth-2fa.
i have the same BUG, but with
- javascript-catalog
- and my own javascript-settings.
We must find a solution to ignore specific views from the check.
Because some view are legit and part of the process.
The middleware is loaded when the TwoFactorAuthenticate view is loaded, no normally that others view are loaded in the process.
from django-allauth-2fa.
If a developer would tell me which of the four options I gave were acceptable, I could provide a pull request.
But I'm not going to go to that effort just to have it ignored.
from django-allauth-2fa.
for me the more easy is to have like an exception list.
one is to have some "internal utility views" and another is to have a full functional view redirect.
In first instance, the "utility views" are not redirected and load directly
from django-allauth-2fa.
most fast solution with exception list
def process_request(self, request):
match = resolve(request.path)
except_list = getattr(settings, 'EXCEPT_VIEWS_FROM_CHECK', ())
except_list += ('two-factor-authenticate',)
if not match.url_name or not match.url_name.startswith(except_list):
try:
del request.session['allauth_2fa_user_id']
except KeyError:
log.info(match.url_name)
from django-allauth-2fa.
@SoulRaven Use the < >
button in the toolbar to mark off code blocks. I can't read your comment.
Also, I'm not sure "must" is the word you wanted, and can't tell what you really meant.
from django-allauth-2fa.
@SoulRaven Use the
< >
button in the toolbar to mark off code blocks. I can't read your comment.Also, I'm not sure "must" is the word you wanted, and can't tell what you really meant.
fix
from django-allauth-2fa.
Related Issues (20)
- Plan for a 1.0 release HOT 8
- Rename main branch to 'main' HOT 2
- Redirect-to-next breaks with certain view classes
- OTP login breaks with subclassed OTPAdapter
- TypeError at /account/two-factor-authenticate - exists again in 0.9 HOT 2
- Join jazzband? HOT 11
- When usinging allauth with email login the login screen does not navigate to totp entry HOT 1
- can someone point how to enable 2FA using email for all users? HOT 1
- Configurable redirect URLs HOT 1
- Not compatible with function-based login view HOT 4
- Disabling 2FA does not work with a backup token HOT 2
- Success social login directing to failed social login page HOT 2
- ALLAUTH_2FA_SETUP_SUCCESS_URL and ALLAUTH_2FA_REMOVE_SUCCESS_URL not used in views
- RestFul api use HOT 1
- Make it possible to disable 2fa without a token.
- Templates are not included in 0.11.0 wheel distribution HOT 1
- Missing dependabot config HOT 3
- When using with the latest version of django-allauth, 2FAย is bypassed HOT 5
- Consider where django-allauth-2fa stands now django-allauth has MFA built-in HOT 4
- user_logged_in signal from Django is executed twice
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. ๐๐๐
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google โค๏ธ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from django-allauth-2fa.