From 9fc8cee0424d57d8965b4a198746b4fe78a40ae4 Mon Sep 17 00:00:00 2001 From: Paul Selkirk Date: Tue, 31 Oct 2023 23:48:07 -0700 Subject: [PATCH 1/3] fix: Don't redirect user to the login page when logging in (#5876) (Embrace and extend c4bf508cd8.) --- ietf/doc/templatetags/ietf_filters.py | 12 ++++-------- ietf/templates/base.html | 4 ++-- ietf/templates/base/menu_user.html | 4 ++-- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index 9b4700bfb1..29d39bc073 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -409,20 +409,16 @@ def startswith(x, y): return str(x).startswith(y) -@register.filter(name='removesuffix', is_safe=False) -def removesuffix(value, suffix): - """Remove an exact-match suffix +@register.filter(name='removepath', is_safe=False) +def removepath(value, suffix): + """Remove an exact-match path The is_safe flag is False because indiscriminate use of this could result in non-safe output. See https://docs.djangoproject.com/en/2.2/howto/custom-template-tags/#filters-and-auto-escaping which describes the possibility that removing characters from an escaped string may introduce HTML-unsafe output. """ - base = str(value) - if base.endswith(suffix): - return base[:-len(suffix)] - else: - return base + return str(value).replace(suffix, '') @register.filter diff --git a/ietf/templates/base.html b/ietf/templates/base.html index 77a2fa9d96..4de70b6351 100644 --- a/ietf/templates/base.html +++ b/ietf/templates/base.html @@ -1,4 +1,4 @@ -{# Copyright The IETF Trust 2015-2022, All Rights Reserved #} +{# Copyright The IETF Trust 2015-2023, All Rights Reserved #} {% load analytical %} {% load ietf_filters static %} @@ -60,7 +60,7 @@ {% if not user.is_authenticated %} + href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removepath:'/accounts/logout'|removepath:'/accounts/login/?next='|urlencode }}"> Sign in {% endif %} diff --git a/ietf/templates/base/menu_user.html b/ietf/templates/base/menu_user.html index 9e6bbde56a..c52945c87b 100644 --- a/ietf/templates/base/menu_user.html +++ b/ietf/templates/base/menu_user.html @@ -1,4 +1,4 @@ -{# Copyright The IETF Trust 2015, All Rights Reserved #} +{# Copyright The IETF Trust 2015-2023, All Rights Reserved #} {% load origin %} {% origin %} {% load ietf_filters %} @@ -87,7 +87,7 @@
  • + href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removepath:'/accounts/login/?next='|urlencode }}"> Sign in
    • From 4001100878a588b399771ec277000b5bab435533 Mon Sep 17 00:00:00 2001 From: Paul Selkirk Date: Wed, 1 Nov 2023 11:24:46 -0700 Subject: [PATCH 2/3] test: Add test case for login button --- ietf/ietfauth/tests.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index 0e5fcb3c40..ee369c2365 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2009-2022, All Rights Reserved +# Copyright The IETF Trust 2009-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -111,6 +111,26 @@ def test_login_and_logout(self): self.assertEqual(r.status_code, 302) self.assertEqual(urlsplit(r["Location"])[2], "/foobar") + def test_login_button(self): + PersonFactory(user__username='plain') + + # try mashing the sign-in button repeatedly + r = self.client.get("/") + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + login_url = q("a:Contains('Sign in')").attr("href") + self.assertTrue(login_url.endswith("accounts/login/?next=/")) + r = self.client.get(login_url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + login_url = q("a:Contains('Sign in')").attr("href") + self.assertTrue(login_url.endswith("accounts/login/?next=/")) + + # try logging in with the provided next + r = self.client.post(login_url, {"username":"plain", "password":"plain+password"}) + self.assertEqual(r.status_code, 302) + self.assertEqual(urlsplit(r["Location"])[2], "/") + def test_login_with_different_email(self): person = PersonFactory(user__username='plain') email = EmailFactory(person=person) From ae986d21d5d33e7a8a1891cae4e6e1e29b746489 Mon Sep 17 00:00:00 2001 From: Paul Selkirk Date: Wed, 1 Nov 2023 18:59:48 -0700 Subject: [PATCH 3/3] refactor: The template filter just strips off a path prefix, so rename/recode accordingly Also test with a non-trivial redirect target. --- ietf/doc/templatetags/ietf_filters.py | 14 ++++++---- ietf/ietfauth/tests.py | 39 ++++++++++++++++----------- ietf/templates/base.html | 2 +- ietf/templates/base/menu_user.html | 2 +- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py index 29d39bc073..c0ea94ab71 100644 --- a/ietf/doc/templatetags/ietf_filters.py +++ b/ietf/doc/templatetags/ietf_filters.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2007-2020, All Rights Reserved +# Copyright The IETF Trust 2007-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -409,16 +409,20 @@ def startswith(x, y): return str(x).startswith(y) -@register.filter(name='removepath', is_safe=False) -def removepath(value, suffix): - """Remove an exact-match path +@register.filter(name='removeprefix', is_safe=False) +def removeprefix(value, prefix): + """Remove an exact-match prefix The is_safe flag is False because indiscriminate use of this could result in non-safe output. See https://docs.djangoproject.com/en/2.2/howto/custom-template-tags/#filters-and-auto-escaping which describes the possibility that removing characters from an escaped string may introduce HTML-unsafe output. """ - return str(value).replace(suffix, '') + base = str(value) + if base.startswith(prefix): + return base[len(prefix):] + else: + return base @register.filter diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py index ee369c2365..ec085ed813 100644 --- a/ietf/ietfauth/tests.py +++ b/ietf/ietfauth/tests.py @@ -114,22 +114,31 @@ def test_login_and_logout(self): def test_login_button(self): PersonFactory(user__username='plain') - # try mashing the sign-in button repeatedly - r = self.client.get("/") - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - login_url = q("a:Contains('Sign in')").attr("href") - self.assertTrue(login_url.endswith("accounts/login/?next=/")) - r = self.client.get(login_url) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - login_url = q("a:Contains('Sign in')").attr("href") - self.assertTrue(login_url.endswith("accounts/login/?next=/")) + def _test_login(url): + # try mashing the sign-in button repeatedly + r = self.client.get(url) + if r.status_code == 302: + r = self.client.get(r["Location"]) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + login_url = q("a:Contains('Sign in')").attr("href") + self.assertEqual(login_url, "/accounts/login/?next=" + url) + r = self.client.get(login_url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + login_url = q("a:Contains('Sign in')").attr("href") + self.assertEqual(login_url, "/accounts/login/?next=" + url) - # try logging in with the provided next - r = self.client.post(login_url, {"username":"plain", "password":"plain+password"}) - self.assertEqual(r.status_code, 302) - self.assertEqual(urlsplit(r["Location"])[2], "/") + # try logging in with the provided next + r = self.client.post(login_url, {"username":"plain", "password":"plain+password"}) + self.assertEqual(r.status_code, 302) + self.assertEqual(urlsplit(r["Location"])[2], url) + self.client.logout() + + # try with a trivial next + _test_login("/") + # try with a next that requires login + _test_login(urlreverse(ietf.ietfauth.views.profile)) def test_login_with_different_email(self): person = PersonFactory(user__username='plain') diff --git a/ietf/templates/base.html b/ietf/templates/base.html index 4de70b6351..0668ba6318 100644 --- a/ietf/templates/base.html +++ b/ietf/templates/base.html @@ -60,7 +60,7 @@ {% if not user.is_authenticated %} + href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removeprefix:'/accounts/logout'|removeprefix:'/accounts/login/?next='|urlencode }}"> Sign in {% endif %} diff --git a/ietf/templates/base/menu_user.html b/ietf/templates/base/menu_user.html index c52945c87b..70ace5cd46 100644 --- a/ietf/templates/base/menu_user.html +++ b/ietf/templates/base/menu_user.html @@ -87,7 +87,7 @@
  • + href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removeprefix:'/accounts/login/?next='|urlencode }}"> Sign in