Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions springfield/cms/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# file, You can obtain one at https://mozilla.org/MPL/2.0/.
import logging
from collections import defaultdict
from contextvars import ContextVar
from http import HTTPStatus

from django.conf import settings
Expand All @@ -16,6 +17,8 @@

logger = logging.getLogger(__name__)

_is_admin_request = ContextVar("is_wagtail_admin_request", default=False)


class CMSLocaleFallbackMiddleware:
"""Middleware that handles two locale-fallback concerns for 404 responses:
Expand Down Expand Up @@ -164,3 +167,35 @@ def _has_null_byte(self, request):
# This gets called as a 404, so let's just treat it as Not Found
return True
return False


def is_wagtail_admin_request():
"""True while the current execution context is handling a Wagtail-admin request."""
return _is_admin_request.get()


class WagtailAdminRequestMiddleware:
"""
Set the admin-request flag for the duration of a Wagtail-admin request.

Backed by a ``ContextVar`` so it is correct under both sync (WSGI) and async
(ASGI) workers: the value is isolated per execution context and restored in a
``finally``, so it cannot leak between requests sharing a worker thread.
"""

def __init__(self, get_response):
self.get_response = get_response

def __call__(self, request):
# Match the Wagtail admin mount exactly: the prefix followed by "/" (so a
# public path such as "/cms-administrator/" can't be mistaken for the
# admin), plus the bare prefix itself (which redirects to the
# trailing-slash URL).
admin_prefix = "/" + settings.WAGTAIL_ADMIN_URL_PREFIX.strip("/")
path = request.path
is_admin_request = path == admin_prefix or path.startswith(admin_prefix + "/")
token = _is_admin_request.set(is_admin_request)
try:
return self.get_response(request)
finally:
_is_admin_request.reset(token)
25 changes: 25 additions & 0 deletions springfield/cms/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from lib import l10n_utils
from springfield.base.i18n import normalize_language
from springfield.cms.middleware import is_wagtail_admin_request
from springfield.cms.utils import compute_cms_page_locales


Expand Down Expand Up @@ -172,6 +173,30 @@ def localized(self):

return localized

def get_url_parts(self, request=None):
"""
While handling a Wagtail-admin request, build a page's URL from the
page's *own* locale rather than the editor's active UI language.

Avoids this bug:
1. an editor sets their Wagtail admin language to Portuguese, so
Wagtail activates 'pt-pt' for admin responses
(get_localized_response)
2. they open a published pt-BR page in the editor
3. the "PUBLISHED" ("PUBLICADA") badge links to /pt-PT/, because
core promotes the pt-BR prefix to the active 'pt-pt'.

The logic here can't depend on the request parameter, because the
edit-page badge renders ``<a href="{{ page.url }}">``, and Page.url`
calls get_url_parts() with request=None by construction.
Instead, we use the is_wagtail_admin_request() helper to ensure that
our logic only runs for requests for Wagtail admin pages.
"""
if is_wagtail_admin_request() and normalize_language(translation.get_language()) != self.locale.language_code:
with translation.override(self.locale.language_code):
return super().get_url_parts(request=request)
return super().get_url_parts(request=request)

def get_active_locale_url(self, request=None):
"""
Replace the URLs locale with the active locale if the page is a fallback
Expand Down
27 changes: 27 additions & 0 deletions springfield/cms/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from wagtail.models import Locale, Page, Site

from springfield.cms.fixtures.base_fixtures import get_placeholder_images, get_test_index_page
from springfield.cms.middleware import _is_admin_request
from springfield.cms.tests.factories import LocaleFactory, SimpleRichTextPageFactory

User = get_user_model()
Expand Down Expand Up @@ -345,3 +346,29 @@ def site_with_en_de_fr_it_homepages_and_some_translations(site_with_en_de_fr_it_
it_translation = fr_page.copy_for_translation(it_locale)
it_translation.title = "Italian Translation"
it_translation.save_revision().publish()


@pytest.fixture
def admin_request_middleware_admin_request():
"""
Simulate the effects of WagtailAdminRequestMiddleware on a request.
"""
token = _is_admin_request.set(True)
try:
yield
finally:
_is_admin_request.reset(token)


@pytest.fixture
def pt_pt_test_page(tiny_localized_site):
"""
A pt-PT translation of the en-US test-page (with a live pt-PT root).
"""
pt_pt_locale = LocaleFactory(language_code="pt-PT")
site = Site.objects.get(is_default_site=True)
site.root_page.copy_for_translation(pt_pt_locale).save_revision().publish()
en_test_page = Page.objects.get(locale__language_code="en-US", slug="test-page")
pt_pt_page = en_test_page.copy_for_translation(pt_pt_locale)
pt_pt_page.save_revision().publish()
return pt_pt_page.specific
54 changes: 54 additions & 0 deletions springfield/cms/tests/test_get_url_parts_admin_e2e.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

from django.conf import settings
from django.contrib.auth import get_user_model
from django.test import override_settings

import pytest
from bs4 import BeautifulSoup
from wagtail.models import Page
from wagtail.users.models import UserProfile

from springfield.cms.tests.factories import LocaleFactory

pytestmark = [pytest.mark.django_db]

User = get_user_model()


@override_settings(
# The project is SSO-only by default; enable conventional auth so the test
# client can log in (mirrors springfield/cms/tests/test_auth.py).
AUTHENTICATION_BACKENDS=("django.contrib.auth.backends.ModelBackend",),
USE_SSO_AUTH=False,
WAGTAIL_ENABLE_ADMIN=True,
FALLBACK_LOCALES={"pt-PT": "pt-BR"},
)
def test_published_badge_for_pt_br_page_points_to_pt_br_for_pt_pt_editor(client, tiny_localized_site):
"""
If a user with a preferred pt-pt locale edits a pt-BR page, page's "PUBLISHED" badge should be the pt-BR URL.
"""
LocaleFactory(language_code="pt-PT")
pt_br_page = Page.objects.get(locale__language_code="pt-BR", slug="child-page")

editor = User.objects.create_superuser(username="editor", email="editor@example.com", password="admin12345")
profile = UserProfile.get_for_user(editor)
profile.preferred_language = "pt-pt" # Wagtail's Portuguese (Portugal) admin language
profile.save()
client.force_login(editor)

response = client.get(f"/{settings.WAGTAIL_ADMIN_URL_PREFIX}/pages/{pt_br_page.id}/edit/")

assert response.status_code == 200

# Scope the assertion to the PUBLISHED badge anchor itself (rendered by
# page_status_tag_new.html with class "page-status-tag"), so an unrelated
# widget elsewhere on the edit page can't make this pass or fail spuriously.
soup = BeautifulSoup(response.content, "html.parser")
badge_hrefs = {a.get("href") for a in soup.select("a.page-status-tag")}
assert badge_hrefs, "expected the PUBLISHED badge anchor in the edit header"
# The badge links to the page's own canonical pt-BR URL, never the
# editor-UI-language pt-PT URL (the bug).
assert badge_hrefs == {"/pt-BR/test-page/child-page/"}
42 changes: 41 additions & 1 deletion springfield/cms/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import pytest
from wagtail.models import Locale, Page, PageViewRestriction, Site

from springfield.cms.middleware import CMSLocaleFallbackMiddleware
from springfield.cms.middleware import CMSLocaleFallbackMiddleware, WagtailAdminRequestMiddleware, is_wagtail_admin_request
from springfield.cms.tests.factories import LocaleFactory, SimpleRichTextPageFactory

pytestmark = [pytest.mark.django_db]
Expand Down Expand Up @@ -614,3 +614,43 @@ def test_CMSLocaleFallbackMiddleware_alias_locale_view_restricted_page_redirects
# Redirects to canonical URL so Wagtail's restriction enforcement fires there
assert response.status_code == 302
assert response.headers["Location"] == pt_br_child.url


def test_wagtail_admin_request_middleware(rf):
"""
WagtailAdminRequestMiddleware marks /cms-admin/ requests True, and others as False.

Also, it should always clears its mark after the request is processed.
"""

flag_during_request = []

def get_response(request):
flag_during_request.append(is_wagtail_admin_request())
return HttpResponse("ok")

middleware = WagtailAdminRequestMiddleware(get_response)

prefix = settings.WAGTAIL_ADMIN_URL_PREFIX.strip("/")
cases = [
(f"/{prefix}/pages/1/edit/", True), # a real admin path
(f"/{prefix}/", True), # the admin root, with trailing slash
(f"/{prefix}", True), # the bare prefix (redirects to trailing-slash URL)
("/pt-BR/some-page/", False), # an ordinary content path
(f"/{prefix}istrator/", False), # a look-alike that must NOT match the prefix
]

for path, _expected in cases:
middleware(rf.get(path))
assert is_wagtail_admin_request() is False # cleared after each request

assert flag_during_request == [expected for _path, expected in cases]
assert is_wagtail_admin_request() is False # cleared after each request

# The flag is cleared even if the inner handler raises.
def server_error(request):
raise ValueError("server error")

with pytest.raises(ValueError):
WagtailAdminRequestMiddleware(server_error)(rf.get(f"/{settings.WAGTAIL_ADMIN_URL_PREFIX}/someotherurl/"))
assert is_wagtail_admin_request() is False
102 changes: 102 additions & 0 deletions springfield/cms/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,108 @@ def test_get_active_locale_url_returns_original_url_if_active_language_not_in_fa
assert "/pt-BR/" not in url


_PT_BR_CHILD_URL = "/pt-BR/test-page/child-page/"
_PT_PT_CHILD_URL = "/pt-PT/test-page/child-page/"


def _pt_br_child():
return Page.objects.get(locale__language_code="pt-BR", slug="child-page").specific


@pytest.mark.parametrize("enable_admin", [True, False])
def test_get_url_parts_does_not_affect_non_admin_urls(tiny_localized_site, enable_admin):
"""
Assert get_url_parts() does not affect non-admin URLs, regardless of the WAGTAIL_ENABLE_ADMIN.
"""
with override_settings(WAGTAIL_ENABLE_ADMIN=enable_admin, FALLBACK_LOCALES={"pt-PT": "pt-BR"}):
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("pt-pt"):
# For non-admin requests, Wagtail promotes pt-BR to the active pt-pt (/pt-PT/) locale.
assert page.get_url() == _PT_PT_CHILD_URL
assert page.get_url_parts()[2] == _PT_PT_CHILD_URL
# Our get_url_parts() override returns exactly what Wagtail's
# Page.get_url_parts()[2] returns.
assert Page.get_url_parts(page, None)[2] == _PT_PT_CHILD_URL


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_points_pt_br_page_to_own_locale(admin_request_middleware_admin_request, tiny_localized_site):
"""
If a user with a preferred pt-pt locale edits a pt-BR page, page's URL should be the pt-BR URL.
"""
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("pt-pt"):
assert page.get_url() == _PT_BR_CHILD_URL
assert page.get_url_parts()[2] == _PT_BR_CHILD_URL


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_page_url_property(admin_request_middleware_admin_request, tiny_localized_site):
"""
If user with preferred pt-pt locale gets a pt-BR page's URL, page's URL should be the pt-BR URL.
"""
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("pt-pt"):
assert page.url == _PT_BR_CHILD_URL


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_pt_pt_page_keeps_pt_pt(admin_request_middleware_admin_request, pt_pt_test_page):
"""
If a user with a preferred pt-pt locale edits a pt-PT page, page's URL should be the pt-PT URL.
"""
with translation.override("pt-pt"):
url = pt_pt_test_page.get_url()
assert "/pt-PT/" in url
assert "/pt-BR/" not in url
assert url == "/pt-PT/test-page/"


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_english_pt_br_page(admin_request_middleware_admin_request, tiny_localized_site):
"""
If a user with a preferred en-us locale edits a pt-BR page, page's URL should be the pt-BR URL.
"""
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("en-us"):
assert page.get_url() == _PT_BR_CHILD_URL
assert page.get_url_parts()[2] == _PT_BR_CHILD_URL


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_lowercase_own_locale(admin_request_middleware_admin_request, tiny_localized_site):
"""
If a user with a preferred pt-br locale edits a pt-BR page, page's URL should be the pt-BR URL.
"""
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("pt-br"):
assert page.get_url() == _PT_BR_CHILD_URL
assert page.get_url_parts()[2] == _PT_BR_CHILD_URL


@override_settings(FALLBACK_LOCALES={"pt-PT": "pt-BR"})
def test_admin_request_get_active_locale_url_unchanged(admin_request_middleware_admin_request, tiny_localized_site):
"""
If a user with a preferred pt-pt locale gets the active-locale URL for a pt-BR page, it should be the
pt-PT alias URL.

This test asserts that the get_url_parts() override does not affect get_active_locale_url():
get_active_locale_url() still rewrites the pinned /pt-BR/ prefix back to the active /pt-PT/ alias.
"""
LocaleFactory(language_code="pt-PT")
page = _pt_br_child()
with translation.override("pt-pt"):
url = page.get_active_locale_url()
assert "/pt-PT/" in url
assert "/pt-BR/" not in url
assert url == _PT_PT_CHILD_URL


def test_whats_new_index_page_redirects_to_latest_whats_new(
minimal_site,
rf,
Expand Down
Loading
Loading