Re: Bug during logout - Mailing list pgsql-www

Hello!

Magnus Hagander <magnus@hagander.net> writes:
> So a few things on the patch:
>
> 1. This would be the *only* test in the codebase :)  I think fixing that scenario should be kept to a separate patch
-it clearly
 
> would be good to have them, buth aving *one* makes not much sense. Also, what's up with enabling ESI on the tests --
then
> you require Varnish to run the tests properly. 

Well, having the first test it's the starting point, and patch should
always carry their own when is possible, this helps to avoid making the
same mistake in the future. Well, having *one* tests it's the starting
point, nothing make sense without the first one right? About the ESI,
yeah if you run the tests with a Varnish in front to test the full
infrastructure it will make sense, but clearly not the case, I remove it.

> 2. It seems you assign a "post" variable to the sitenav structure, whichi s then never used because you instead
hardcodethe
 
> URL in the template? (FWIW I do think using the variable is the right thing, but then it should be used)

Yeah! It was in between the work I was doing and forgot the change, yeah
the idea is that any thing that it's a post go with a form, but, later
my thought was "ok the logout", so, there will be anything else except
the logout that it's going to be a post? anyway, yes I changed it!

> 3. This does not cover the case of "distributed logout". That is, there are GET links in the community auth system
thatdoes the
 
> same. If we actually care about not being able to log out that way, we'd need to fix that too? 

Uhmm, when you mean "distributed logout", you mean when people use the
OAuth capabilities provided by postgresql.org right? Well, I think that
logging out from a website it goes in the website, but if we want to
have a logout control it should be in the panel that mange the session
of the PostgreSQL OAuth system, so that panel should have the session
per site, and from there it should logout other systems, so the webpage
logout only that, the webpage.

But now I'm curious, there's any panel that manage the session in other
sites like 2026.pgconf.eu, etc ? In that case, probably we can do
something on that side in another patch!

Regards!
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.comFrom 23ab8702c06dae2ebd81760ad2d30fd649fab117 Mon Sep 17 00:00:00 2001
From: "Jonathan Gonzalez V." <jonathan@abdiel.eu>
Date: Mon, 10 Aug 2026 11:30:40 +0200
Subject: [PATCH v2 1/1] Turn the logout link into a POST method following
 Django 5.2 rules.

---
 media/css/main.css       | 12 ++++++++++++
 pgweb/account/tests.py   | 24 ++++++++++++++++++++++++
 pgweb/util/contexts.py   |  2 +-
 templates/base/page.html |  9 ++++++++-
 4 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 pgweb/account/tests.py

diff --git a/media/css/main.css b/media/css/main.css
index 7e9ebf18..4861a348 100644
--- a/media/css/main.css
+++ b/media/css/main.css
@@ -847,6 +847,18 @@ input#navbar-toggler {
   color: var(--pg-sidenav-a-fg-color);
   font-weight: normal;
 }
+#pgSideNav .pg-sidenav-link {
+  background: none;
+  border: 0;
+  color: var(--pg-sidenav-a-fg-color);
+  cursor: pointer;
+  font: inherit;
+  padding: 0;
+}
+
+#pgSideNav .pg-sidenav-link:hover {
+  text-decoration: underline;
+}
 
 #pgSideNav ul {
   list-style-type: circle;
diff --git a/pgweb/account/tests.py b/pgweb/account/tests.py
new file mode 100644
index 00000000..1e32b850
--- /dev/null
+++ b/pgweb/account/tests.py
@@ -0,0 +1,24 @@
+from django.contrib.auth import SESSION_KEY, get_user_model
+from django.test import TestCase, override_settings
+
+class LogoutTests(TestCase):
+    def setUp(self):
+        self.user = get_user_model().objects.create_user(
+            username='logout-test',
+            email='logout-test@example.com',
+        )
+        self.client.force_login(self.user)
+
+    def test_account_navigation_submits_logout_with_post(self):
+        response = self.client.get('/account/')
+
+        self.assertContains(
+            response,
+            '<form action="/account/logout/" method="post">',
+        )
+
+    def test_logout_post_clears_session(self):
+        response = self.client.post('/account/logout/')
+
+        self.assertRedirects(response, '/', fetch_redirect_response=False)
+        self.assertNotIn(SESSION_KEY, self.client.session)
diff --git a/pgweb/util/contexts.py b/pgweb/util/contexts.py
index ca493714..95750c39 100644
--- a/pgweb/util/contexts.py
+++ b/pgweb/util/contexts.py
@@ -89,7 +89,7 @@ sitenav = {
             {'title': 'Organisations', 'link': '/account/edit/organisations/'},
         ]},
         {'title': 'Change password', 'link': '/account/changepwd/'},
-        {'title': 'Logout', 'link': '/account/logout/'},
+        {'title': 'Logout', 'link': '/account/logout/', 'method': 'post'},
     ],
 }
 
diff --git a/templates/base/page.html b/templates/base/page.html
index 523b53d8..5bdf5ea3 100644
--- a/templates/base/page.html
+++ b/templates/base/page.html
@@ -11,7 +11,14 @@
          <ul>
            {%for m in navmenu%}
            {%if not forloop.first %}</li>{%endif%}
-           <li{%if forloop.last%} class="last-child"{%endif%}><a href="{{m.link}}">{{m.title}}</a>
+           <li{%if forloop.last%} class="last-child"{%endif%}>
+            {%if m.method == 'post'%}
+             <form action="{{m.link}}" method="post">{%csrf_token%}
+              <button class="pg-sidenav-link" type="submit">{{m.title}}</button>
+             </form>
+            {%else%}
+             <a href="{{m.link}}">{{m.title}}</a>
+            {%endif%}
             {%if m.submenu %}
              <ul>
              {%for sm in m.submenu %}
-- 
2.53.0