From 87309734cc5c8ab3be532e21279199450264019d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 8 Sep 2011 14:16:59 -0700
Subject: [PATCH] Nuke sessions from the database when users logout

Summary:
@tomo ran into an issue where he had some non-SSL-only cookie or whatever, so
"Logout" had no apparent effect. Make sure "Logout" really works by destroying
the session.

I originally kept the sessions around to be able to debug session stuff, but we
have a fairly good session log now and no reprorted session bugs except for all
the cookie stuff. It's also slightly more secure to actually destroy sessions,
since it means "logout" breaks any cookies that attackers somehow stole (e.g.,
by reading your requests off a public wifi network).

Test Plan: Commented out the cookie clear and logged out. I was logged out and
given a useful error message about clearing my cookies.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: tomo, aran, epriestley

Differential Revision: 911
---
 .../controller/logout/PhabricatorLogoutController.php  |  8 ++++++++
 .../people/storage/user/PhabricatorUser.php            | 10 ++++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/applications/auth/controller/logout/PhabricatorLogoutController.php b/src/applications/auth/controller/logout/PhabricatorLogoutController.php
index ddf31ad76b..3db12be594 100644
--- a/src/applications/auth/controller/logout/PhabricatorLogoutController.php
+++ b/src/applications/auth/controller/logout/PhabricatorLogoutController.php
@@ -39,7 +39,15 @@ class PhabricatorLogoutController extends PhabricatorAuthController {
         PhabricatorUserLog::ACTION_LOGOUT);
       $log->save();
 
+      // Destroy the user's session in the database so logout works even if
+      // their cookies have some issues. We'll detect cookie issues when they
+      // try to login again and tell them to clear any junk.
+      $phsid = $request->getCookie('phsid');
+      if ($phsid) {
+        $user->destroySession($phsid);
+      }
       $request->clearCookie('phsid');
+
       return id(new AphrontRedirectResponse())
         ->setURI('/login/');
     }
diff --git a/src/applications/people/storage/user/PhabricatorUser.php b/src/applications/people/storage/user/PhabricatorUser.php
index 043ee96e2f..7a21d30594 100644
--- a/src/applications/people/storage/user/PhabricatorUser.php
+++ b/src/applications/people/storage/user/PhabricatorUser.php
@@ -284,6 +284,16 @@ class PhabricatorUser extends PhabricatorUserDAO {
     return $session_key;
   }
 
+  public function destroySession($session_key) {
+    $conn_w = $this->establishConnection('w');
+    queryfx(
+      $conn_w,
+      'DELETE FROM %T WHERE userPHID = %s AND sessionKey = %s',
+      self::SESSION_TABLE,
+      $this->getPHID(),
+      $session_key);
+  }
+
   private function generateEmailToken($offset = 0) {
     return $this->generateToken(
       time() + ($offset * self::EMAIL_CYCLE_FREQUENCY),