Log out with a POST form.

The logout link put the CSRF token in the URL, which goes into logs and the browser history.

publish/logout.php

@@ -6,8 +6,8 @@ header( 'Pragma: no-cache' );
 include_once('../session.lib.php');
 require_once('../config.lib.php');
 require_https();
-// Only log out from our own logout link, which carries the CSRF token
-if (!csrf_valid($_GET['token'] ?? '')) {
+// Only log out from our own logout form, which posts the CSRF token
+if (!csrf_valid()) {
   header("Location: /");
   exit;
 }

sidebar.inc.php

@@ -29,7 +29,8 @@ if (preg_match("#^(/$|/index|/article)#", $_SERVER['REQUEST_URI'])) {
       <li><a href="/publish">Add</a>/<a href="/publish/edit" class="sidelink">Edit</a></li>
 <?php if (isset($is_admin) && $is_admin) {
         echo '      <li><a href="/publish/addeditor">Add editor</a></li>'."\n";
-        echo '      <li><a href="/publish/logout?token='.csrf_token().'">Logout</a></li>'."\n";
+        // A form, so that the token stays out of the URL
+        echo '      <li><form action="/publish/logout" method="post">'.csrf_field().'<button type="submit">Logout</button></form></li>'."\n";
 }
       //echo '      <li><a href="/policy">Publishing policy</a></li>';
       ?>

style.css

@@ -146,6 +146,23 @@ footer#bottom p {
     margin-right: 5px;
 }
 
+/* The logout button looks like the links next to it */
+#sidebar .links li form {
+  display: inline;
+}
+#sidebar .links li button {
+  padding: 0;
+  border: 0;
+  background: none;
+  font: inherit;
+  color: inherit;
+  cursor: pointer;
+}
+#sidebar .links li button:hover {
+  color: #FF0000;
+  text-decoration: underline;
+}
+
 #sidebar .links .sideinput {
   padding: 1px;
   padding: 0;

tests/php/Pages/FrontPagesTest.php

@@ -85,7 +85,9 @@ class FrontPagesTest extends PageTestCase {
     $token = $this->loginAs();
     $body = $this->get('/')['body'];
     $this->assertStringContainsString('Secret', $body);
-    $this->assertStringContainsString('<a href="/publish/logout?token=' . $token . '">Logout</a>', $body);
+    $this->assertStringContainsString('<form action="/publish/logout" method="post"><input type="hidden" name="csrf_token" value="' . $token . '" />'
+                                      . '<button type="submit">Logout</button></form>', $body);
+    $this->assertStringNotContainsString('?token=', $body);
     $this->assertStringContainsString('<a href="/publish/addeditor">Add editor</a>', $body);
   }
 

tests/php/Pages/LoginTest.php

@@ -182,7 +182,7 @@ class LoginTest extends PageTestCase {
     // A new browser session with only the cookie
     unset($this->cookies['PHPSESSID']);
     $body = $this->get('/')['body'];
-    $this->assertStringContainsString('/publish/logout?token=', $body);
+    $this->assertStringContainsString('<form action="/publish/logout" method="post">', $body);
     $this->assertSame('ed', $this->session()['editor']);
   }
 
@@ -224,7 +224,15 @@ class LoginTest extends PageTestCase {
 
   public function testLogoutNeedsTheToken(): void {
     $this->loginAs();
-    $this->assertRedirect('/', $this->get('/publish/logout?token=wrong'));
+    $this->assertRedirect('/', $this->post('/publish/logout', array('csrf_token' => 'wrong')));
+    $this->assertSame('ed', $this->session()['editor']);
+  }
+
+  // The token stays out of URLs, which go into logs and the browser history
+  public function testLogoutNeedsAPost(): void {
+    $token = $this->loginAs();
+    $this->assertRedirect('/', $this->get('/publish/logout?token=' . $token));
+    $this->assertRedirect('/', $this->get('/publish/logout?csrf_token=' . $token));
     $this->assertSame('ed', $this->session()['editor']);
   }
 
@@ -236,7 +244,7 @@ class LoginTest extends PageTestCase {
     $this->cookies['PersistentAuth'] = 'ed|key';
     $session = self::$state . '/sessions/sess_' . $this->cookies['PHPSESSID'];
 
-    $answer = $this->get('/publish/logout?token=' . $token);
+    $answer = $this->post('/publish/logout', array('csrf_token' => $token));
     $this->assertRedirect('/article?id=2', $answer);
     $this->assertFileDoesNotExist($session);
     $this->assertArrayNotHasKey('PHPSESSID', $this->cookies);
@@ -247,17 +255,17 @@ class LoginTest extends PageTestCase {
   public function testAForgedHostDoesNotMoveTheLogoutRedirect(): void {
     $token = $this->loginAs('ed', array('referrer' => '/article?id=2'));
     $this->assertRedirect('/article?id=2',
-                          $this->get('/publish/logout?token=' . $token, array('Host' => 'evil.example')));
+                          $this->post('/publish/logout', array('csrf_token' => $token), array('Host' => 'evil.example')));
   }
 
   #[DataProvider('otherSites')]
   public function testLogoutIgnoresALastPageOnAnotherSite(string $referrer): void {
     $token = $this->loginAs('ed', array('referrer' => $referrer));
-    $this->assertRedirect('/', $this->get('/publish/logout?token=' . $token));
+    $this->assertRedirect('/', $this->post('/publish/logout', array('csrf_token' => $token)));
   }
 
   public function testLogoutDoesNotGoBackToTheLogoutPage(): void {
     $token = $this->loginAs('ed', array('referrer' => '/publish/logout'));
-    $this->assertRedirect('/', $this->get('/publish/logout?token=' . $token));
+    $this->assertRedirect('/', $this->post('/publish/logout', array('csrf_token' => $token)));
   }
 }