Skip to content

fix: add resolvability check before redirecting to prevent insecure redirects after publishing #436

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

Merged
merged 11 commits into from
Jan 15, 2025
23 changes: 20 additions & 3 deletions djangocms_versioning/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,7 @@
f"admin:{self.model._meta.app_label}_{self.model._meta.model_name}_compare",
args=(queryset[0].pk,),
)
url += "?compare_to=%d" % queryset[1].pk

Check failure on line 919 in djangocms_versioning/admin.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (UP031)

djangocms_versioning/admin.py:919:16: UP031 Use format specifiers instead of percent format

return redirect(url)

Expand Down Expand Up @@ -1031,19 +1031,20 @@
)

requested_redirect = request.GET.get("next", None)

if conf.ON_PUBLISH_REDIRECT in ("preview", "published"):
redirect_url=get_preview_url(version.content)
else:
redirect_url=version_list_url(version.content)

if not version.can_be_published():
self.message_user(request, _("Version cannot be published"), messages.ERROR)
return redirect(requested_redirect or redirect_url)
return self._internal_redirect(requested_redirect, redirect_url)
try:
version.check_publish(request.user)
except ConditionFailed as e:
self.message_user(request, force_str(e), messages.ERROR)
return redirect(requested_redirect or redirect_url)
return self._internal_redirect(requested_redirect, redirect_url)

# Publish the version
version.publish(request.user)
Expand All @@ -1056,7 +1057,23 @@
if hasattr(version.content, "get_absolute_url"):
redirect_url = version.content.get_absolute_url() or redirect_url

return redirect(requested_redirect or redirect_url)
return self._internal_redirect(requested_redirect, redirect_url)


def _internal_redirect(self, url, fallback):
"""Helper function to check if the give URL is resolvable
If resolvable, return the URL; otherwise, returns the fallback URL.
"""
if not url:
return redirect(fallback)

try:
resolve(url)
except Resolver404:
return redirect(fallback)

return redirect(url)


def unpublish_view(self, request, object_id):
"""Unpublishes the specified version and redirects back to the
Expand Down
40 changes: 40 additions & 0 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,46 @@

conf.ON_PUBLISH_REDIRECT = original_setting


def test_publish_resolvable_redirect_url(self):
from djangocms_versioning import conf

original_setting = conf.ON_PUBLISH_REDIRECT
conf.ON_PUBLISH_REDIRECT = "published"

user = self.get_superuser()
poll_version = factories.PollVersionFactory(state=constants.DRAFT)

# when there is no requested redirect
url = self.get_admin_url(
self.versionable.version_model_proxy, "publish", poll_version.pk
)

with self.login_user_context(user):
response = self.client.post(url)

self.assertEqual(poll_version.content.get_absolute_url(), response.url)

# when the requested url is resolvable
resolvable_url = url + "?next=" + helpers.get_preview_url(poll_version.content)

with self.login_user_context(user):
response = self.client.post(resolvable_url)

self.assertEqual(response.url, helpers.get_preview_url(poll_version.content))

# when the requested url is not resolvable, should default to version list url
not_resolvable_url = url + "?next=http://example.com"

with self.login_user_context(user):
response = self.client.post(not_resolvable_url)

self.assertEqual(response.url, helpers.get_preview_url(poll_version.content))

conf.ON_PUBLISH_REDIRECT = original_setting



def test_published_view_sets_modified_time(self):
poll_version = factories.PollVersionFactory(state=constants.DRAFT)
url = self.get_admin_url(
Expand Down Expand Up @@ -2156,7 +2196,7 @@
url = self.get_admin_url(
self.versionable.version_model_proxy, "compare", versions[0].pk
)
url += "?compare_to=%d" % versions[1].pk

Check failure on line 2199 in tests/test_admin.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (UP031)

tests/test_admin.py:2199:16: UP031 Use format specifiers instead of percent format
user = self.get_staff_user_with_no_permissions()

with self.login_user_context(user):
Expand Down
Loading