revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01:00
Code Issues Releases Wiki Activity

Fix login issue with stale HTTP vs HTTPS cookies

Browse source 
Summary:
In D758, I tightened the scope for which we issue cookies. Instead of setting
them on the whole domain we set them only on the subdomain, and we set them as
HTTPS only if the install is HTTPS.

However, this can leave the user with a stale HTTP cookie which the browser
sends and which never gets cleared. Handle this situation by:

  - Clear all four <domain, https> pairs when clearing cookies ("nuke it from
orbit").
  - Clear 'phsid' cookies when they're invalid.

Test Plan: Applied a hackier version of this patch to secure.phabricator.com and
was able to login with a stale HTTP cookie.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 838
This commit is contained in:
epriestley 2011-08-19 11:43:18 -07:00
parent 51bd08da27
commit ed33e59c5a
2 changed files with 26 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
src/aphront/request/AphrontRequest.php
View file

@ -204,27 +204,32 @@ class AphrontRequest {
if ($value == '') { if ($value == '') {
// NOTE: If we're clearing the cookie, also clear it on the entire // NOTE: If we're clearing the cookie, also clear it on the entire
// domain. This allows us to clear older cookies which we didn't scope // domain and both HTTP/HTTPS versions. This allows us to clear older
// as tightly. // cookies which we didn't scope as tightly. Eventually we could remove
setcookie( // this, although it doesn't really hurt us. Basically, we're just making
$name, // really sure that cookies get cleared when we try to clear them.
$value, $secure_options = array(true, false);
$expire, $domain_options = array('', $base_domain);
$path = '/', } else {
$domain = '', // Otherwise, when setting cookies, set only one tightly-scoped cookie.
$secure = ($base_protocol == 'https'), $is_secure = ($base_protocol == 'https');
$http_only = true); $secure_options = array($is_secure);
$domain_options = array($base_domain);
} }
foreach ($secure_options as $cookie_secure) {
foreach ($domain_options as $cookie_domain) {
setcookie( setcookie(
$name, $name,
$value, $value,
$expire, $expire,
$path = '/', $path = '/',
$base_domain, $cookie_domain,
$secure = ($base_protocol == 'https'), $cookie_secure,
$http_only = true); $http_only = true);
} }
}
}
final public function setUser($user) { final public function setUser($user) {
$this->user = $user; $this->user = $user;

3
src/applications/base/controller/base/PhabricatorController.php
View file

@ -50,6 +50,9 @@ abstract class PhabricatorController extends AphrontController {
$phsid); $phsid);
if ($info) { if ($info) {
$user->loadFromArray($info); $user->loadFromArray($info);
} else {
// The session cookie is invalid, so clear it.
$request->clearCookie('phsid');
} }
} }