Skip to content
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

Login process is reset when accessing sprites #3643

Closed
tandrieuxx opened this issue Feb 14, 2024 · 7 comments
Closed

Login process is reset when accessing sprites #3643

tandrieuxx opened this issue Feb 14, 2024 · 7 comments

Comments

@tandrieuxx
Copy link

Hi,

We use allauth and allauth.mfa along with Wagtail, and our login templates use SVG sprites.

Through the login process, more precisely just after the email/password input and before the MFA code input, the login data gets stripped from the session, because accessing the sprites causes the function _check_dangling_login to execute request.session.pop("account_login") in allauth.account.middleware.
Our SVG sprites do not pass the condition in _should_check_dangling_login since they're text/html and not statics.

As a result, users can see the MFA input page but no matter what they enter, they're redirected to the login page because account_login is no longer in the session object, so the login process is reset.

Would it be possible to extend the "favicon.ico, robots.txt, humans.txt" whitelist via settings?

def _should_check_dangling_login(request, response):
    content_type = response.headers.get("content-type")
    if content_type:
        content_type = content_type.partition(";")[0]
    if content_type and content_type != "text/html":
        return False
    if request.path.startswith(settings.STATIC_URL) or request.path in [
        "/favicon.ico",
        "/robots.txt",
        "/humans.txt",
    ]:
        return False
    if response.status_code // 100 != 2:
        return False
    return True


def _check_dangling_login(request):
    if not getattr(request, "_account_login_accessed", False):
        if "account_login" in request.session:
            request.session.pop("account_login")
@pennersr
Copy link
Owner

Why are SVG files served as text/html ? Shouldn't that be image/svg+xml ?

@tandrieuxx
Copy link
Author

tandrieuxx commented Feb 14, 2024

The file in question is served by Wagtail with this content type, it's actually a collection of SVGs

@SebCorbin
Copy link
Contributor

Question is raised in Wagtail's discussion wagtail/wagtail#11649

@pennersr
Copy link
Owner

The file in question is served by Wagtail with this content type, it's actually a collection of SVGs

Do you perhaps have a public link to such content? I would like to have a look at what we're dealing with here...

@SebCorbin
Copy link
Contributor

See https://torchbox.com/admin/login/ for the login page, and https://torchbox.com/admin/sprite-44dc8f37/ for a sprite. I would say the wagtail's way of handling this sprite is quite funky (as it's not a valid HTML document): it seems that Wagtail is using a script to store the icon in localStorage

@pennersr
Copy link
Owner

I have added a refinement to the middleware to deal with this gracefully, see: 551f523

@SebCorbin
Copy link
Contributor

SebCorbin commented May 16, 2024

For any person stumbling upon this: 551f523 is using Sec-Fetch-Dest which is a header only sent by the browser in a secured context (i.e. HTTPS or on 127.0.0.1/*.localhost host domains).

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

No branches or pull requests

3 participants