Skip to content

Commit

Permalink
refactor(account): Drop implicit dropping of login
Browse files Browse the repository at this point in the history
With redirects in place this is not needed.
  • Loading branch information
pennersr committed Sep 20, 2024
1 parent 8d09548 commit 0812ed7
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 90 deletions.
57 changes: 1 addition & 56 deletions allauth/account/middleware.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import os
from types import SimpleNamespace

from django.conf import settings
from django.http import HttpResponseRedirect
from django.urls import NoReverseMatch, reverse
from django.utils.decorators import sync_and_async_middleware

from asgiref.sync import iscoroutinefunction, sync_to_async
from asgiref.sync import iscoroutinefunction

from allauth.account import app_settings
from allauth.account.adapter import get_adapter
from allauth.account.internal import flows
from allauth.core import context
Expand All @@ -26,8 +24,6 @@ async def middleware(request):
request.allauth = SimpleNamespace()
with context.request_context(request):
response = await get_response(request)
if _should_check_dangling_login(request, response):
await _acheck_dangling_login(request)
if _should_redirect_accounts(request, response):
response = await _aredirect_accounts(request)
return response
Expand All @@ -38,8 +34,6 @@ def middleware(request):
request.allauth = SimpleNamespace()
with context.request_context(request):
response = get_response(request)
if _should_check_dangling_login(request, response):
_check_dangling_login(request)
if _should_redirect_accounts(request, response):
response = _redirect_accounts(request)
return response
Expand All @@ -58,55 +52,6 @@ def process_exception(request, exception):
return middleware


def _should_check_dangling_login(request, response):
sec_fetch_dest = request.headers.get("sec-fetch-dest")
if sec_fetch_dest and sec_fetch_dest != "document":
return False
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
# STATIC_URL might be None, as the staticfiles app is not strictly required
if (
settings.STATIC_URL and 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):
from allauth.account.stages import EmailVerificationStage

if not getattr(request, "_account_login_accessed", False):
if login := request.session.get(flows.login.LOGIN_SESSION_KEY):
if isinstance(login, dict): # Deal with fake stages
current_stage = login.get("state", {}).get("stages", {}).get("current")
if (
current_stage == EmailVerificationStage.key
and not app_settings.EMAIL_VERIFICATION_BY_CODE_ENABLED
):
# These days, "email verification by link" is just a regular
# stage. However, "email verification by link" was never
# automatically cancelled. So we need to make an exception
# here.
#
# TODO: Reconsider the overall approach to dangling logins:
# https://github.com/pennersr/django-allauth/issues/4087
return
request.session.pop(flows.login.LOGIN_SESSION_KEY)


async def _acheck_dangling_login(request):
await sync_to_async(_check_dangling_login)(request)


def _should_redirect_accounts(request, response) -> bool:
"""
URLs should be hackable. Yet, assuming allauth is included like this...
Expand Down
34 changes: 0 additions & 34 deletions allauth/account/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -1,48 +1,14 @@
import django
from django.conf import settings
from django.http import HttpResponse
from django.test.client import AsyncClient
from django.urls import path, reverse

import pytest

from allauth.account.internal import flows
from allauth.account.internal.decorators import login_not_required
from allauth.account.middleware import AccountMiddleware
from allauth.core.exceptions import ImmediateHttpResponse


@pytest.mark.parametrize(
"path,status_code,content_type,sec_fetch_dest, login_removed",
[
("/", 200, "text/html", None, True),
("/", 200, "text/html", "empty", False),
("/", 200, "text/html", "document", True),
("/", 200, "text/html; charset=utf8", None, True),
("/", 200, "text/txt", None, False),
("/", 404, "text/html", None, False),
(settings.STATIC_URL, 200, "text/html", None, False),
("/favicon.ico", 200, "image/x-icon", None, False),
("/robots.txt", 200, "text/plain", None, False),
("/robots.txt", 200, "text/html", None, False),
("/humans.txt", 200, "text/plain", None, False),
],
)
def test_remove_dangling_login(
rf, path, status_code, login_removed, content_type, sec_fetch_dest
):
request = rf.get(path)
if sec_fetch_dest:
# rf.get(headers=...) is Django 4.2+.
request.META["HTTP_SEC_FETCH_DEST"] = sec_fetch_dest
request.session = {"account_login": True}
response = HttpResponse(status=status_code)
response["Content-Type"] = content_type
mw = AccountMiddleware(lambda request: response)
mw(request)
assert (flows.login.LOGIN_SESSION_KEY in request.session) is (not login_removed)


@login_not_required
def raise_immediate_http_response(request):
response = HttpResponse(content="raised-response")
Expand Down

0 comments on commit 0812ed7

Please sign in to comment.