From a3b4162841ee16f117a36e68ed801bc23f69cf01 Mon Sep 17 00:00:00 2001
From: Paul Selkirk <paul@painless-security.com>
Date: Sun, 5 Nov 2023 02:09:07 -0800
Subject: [PATCH] fix: Don't redirect user to the login page when logging in
 (#6570)

* fix: Don't redirect user to the login page when logging in (#5876)

(Embrace and extend c4bf508cd8.)

* test: Add test case for login button

* 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 | 12 +++++------
 ietf/ietfauth/tests.py                | 31 ++++++++++++++++++++++++++-
 ietf/templates/base.html              |  4 ++--
 ietf/templates/base/menu_user.html    |  4 ++--
 4 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/ietf/doc/templatetags/ietf_filters.py b/ietf/doc/templatetags/ietf_filters.py
index 9b4700bfb..c0ea94ab7 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,9 +409,9 @@ 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='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
@@ -419,8 +419,8 @@ def removesuffix(value, suffix):
     HTML-unsafe output.
     """
     base = str(value)
-    if base.endswith(suffix):
-        return base[:-len(suffix)]
+    if base.startswith(prefix):
+        return base[len(prefix):]
     else:
         return base
 
diff --git a/ietf/ietfauth/tests.py b/ietf/ietfauth/tests.py
index 0e5fcb3c4..ec085ed81 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,35 @@ class IetfAuthTests(TestCase):
         self.assertEqual(r.status_code, 302)
         self.assertEqual(urlsplit(r["Location"])[2], "/foobar")
 
+    def test_login_button(self):
+        PersonFactory(user__username='plain')
+
+        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], 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')
         email = EmailFactory(person=person)
diff --git a/ietf/templates/base.html b/ietf/templates/base.html
index bc315dd56..7dc552268 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 #}
 <!DOCTYPE html>
 {% load analytical %}
 {% load ietf_filters static %}
@@ -60,7 +60,7 @@
                     {% if not user.is_authenticated %}
                         <a class="btn me-1 {% if server_mode and server_mode == "production" %} btn-warning {% else %} btn-info {% endif %} d-none d-sm-block"
                            rel="nofollow"
-                           href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removesuffix:'accounts/logout/'|urlencode }}">
+                           href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removeprefix:'/accounts/logout'|removeprefix:'/accounts/login/?next='|urlencode }}">
                             Sign in
                         </a>
                     {% endif %}
diff --git a/ietf/templates/base/menu_user.html b/ietf/templates/base/menu_user.html
index e731307f2..8245ece71 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 @@
             <li>
                 <a class="dropdown-item {% if flavor != 'top' %} text-wrap{% endif %}"
                    rel="nofollow"
-                   href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|urlencode }}">
+                   href="{% url 'ietf.ietfauth.views.login' %}?next={{ request.get_full_path|removeprefix:'/accounts/login/?next='|urlencode }}">
                     Sign in
                 </a>
             </li>