Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15436462
D8046.id18213.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
7 KB
Referenced Files
None
Subscribers
None
D8046.id18213.diff
View Options
Index: src/aphront/AphrontRequest.php
===================================================================
--- src/aphront/AphrontRequest.php
+++ src/aphront/AphrontRequest.php
@@ -212,22 +212,25 @@
// Add some diagnostic details so we can figure out if some CSRF issues
// are JS problems or people accessing Ajax URIs directly with their
// browsers.
- if ($token) {
- $token_info = "with an invalid CSRF token";
+ $more_info = array();
+
+ if ($this->isAjax()) {
+ $more_info[] = pht('This was an Ajax request.');
} else {
- $token_info = "without a CSRF token";
+ $more_info[] = pht('This was a Web request.');
}
- if ($this->isAjax()) {
- $more_info = "(This was an Ajax request, {$token_info}.)";
+ if ($token) {
+ $more_info[] = pht('This request had an invalid CSRF token.');
} else {
- $more_info = "(This was a web request, {$token_info}.)";
+ $more_info[] = pht('This request had no CSRF token.');
}
// Give a more detailed explanation of how to avoid the exception
// in developer mode.
if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
- $more_info = $more_info .
+ // TODO: Clean this up, see T1921.
+ $more_info[] =
"To avoid this error, use phabricator_form() to construct forms. " .
"If you are already using phabricator_form(), make sure the form " .
"'action' uses a relative URI (i.e., begins with a '/'). Forms " .
@@ -249,14 +252,11 @@
// but give the user some indication of what happened since the workflow
// is incredibly confusing otherwise.
throw new AphrontCSRFException(
- "The form you just submitted did not include a valid CSRF token. ".
- "This token is a technical security measure which prevents a ".
- "certain type of login hijacking attack. However, the token can ".
- "become invalid if you leave a page open for more than six hours ".
- "without a connection to the internet. To fix this problem: reload ".
- "the page, and then resubmit it. All data inserted to the form will ".
- "be lost in some browsers so copy them somewhere before reloading.\n\n".
- $more_info);
+ pht(
+ "You are trying to save some data to Phabricator, but the request ".
+ "your browser made included an incorrect token. Reload the page ".
+ "and try again. You may need to clear your cookies.\n\n%s",
+ implode("\n", $more_info)));
}
return true;
Index: src/applications/auth/provider/PhabricatorAuthProviderPassword.php
===================================================================
--- src/applications/auth/provider/PhabricatorAuthProviderPassword.php
+++ src/applications/auth/provider/PhabricatorAuthProviderPassword.php
@@ -185,14 +185,15 @@
}
if (!$account) {
- $log = PhabricatorUserLog::initializeNewLog(
- null,
- $log_user ? $log_user->getPHID() : null,
- PhabricatorUserLog::ACTION_LOGIN_FAILURE);
- $log->save();
+ if ($request->isFormPost()) {
+ $log = PhabricatorUserLog::initializeNewLog(
+ null,
+ $log_user ? $log_user->getPHID() : null,
+ PhabricatorUserLog::ACTION_LOGIN_FAILURE);
+ $log->save();
+ }
$request->clearCookie(PhabricatorCookies::COOKIE_USERNAME);
- $request->clearCookie(PhabricatorCookies::COOKIE_SESSION);
$response = $controller->buildProviderPageResponse(
$this,
Index: src/applications/base/controller/PhabricatorController.php
===================================================================
--- src/applications/base/controller/PhabricatorController.php
+++ src/applications/base/controller/PhabricatorController.php
@@ -34,14 +34,27 @@
$user = $request->getUser();
} else {
$user = new PhabricatorUser();
+ $session_engine = new PhabricatorAuthSessionEngine();
$phsid = $request->getCookie(PhabricatorCookies::COOKIE_SESSION);
if (strlen($phsid)) {
- $session_user = id(new PhabricatorAuthSessionEngine())
- ->loadUserForSession(PhabricatorAuthSession::TYPE_WEB, $phsid);
+ $session_user = $session_engine->loadUserForSession(
+ PhabricatorAuthSession::TYPE_WEB,
+ $phsid);
if ($session_user) {
$user = $session_user;
}
+ } else {
+ // If the client doesn't have a session token, generate an anonymous
+ // session. This is used to provide CSRF protection to logged-out users.
+ $phsid = $session_engine->establishSession(
+ PhabricatorAuthSession::TYPE_WEB,
+ null);
+ $request->setCookie(PhabricatorCookies::COOKIE_SESSION, $phsid);
+ }
+
+ if (!$user->isLoggedIn()) {
+ $user->attachAlternateCSRFString(PhabricatorHash::digest($phsid));
}
$request->setUser($user);
Index: src/applications/people/storage/PhabricatorUser.php
===================================================================
--- src/applications/people/storage/PhabricatorUser.php
+++ src/applications/people/storage/PhabricatorUser.php
@@ -39,6 +39,8 @@
private $omnipotent = false;
private $customFields = self::ATTACHABLE;
+ private $alternateCSRFString = self::ATTACHABLE;
+
protected function readField($field) {
switch ($field) {
case 'timezoneIdentifier':
@@ -217,12 +219,7 @@
}
public function validateCSRFToken($token) {
- if (!$this->getPHID()) {
- return true;
- }
-
$salt = null;
-
$version = 'plain';
// This is a BREACH-mitigating token. See T3684.
@@ -287,8 +284,15 @@
}
private function generateToken($epoch, $frequency, $key, $len) {
+ if ($this->getPHID()) {
+ $vec = $this->getPHID().$this->getPasswordHash();
+ } else {
+ $vec = $this->getAlternateCSRFString();
+ }
+
$time_block = floor($epoch / $frequency);
- $vec = $this->getPHID().$this->getPasswordHash().$key.$time_block;
+ $vec = $vec.$key.$time_block;
+
return substr(PhabricatorHash::digest($vec), 0, $len);
}
@@ -439,6 +443,15 @@
}
}
+ public function getAlternateCSRFString() {
+ return $this->assertAttached($this->alternateCSRFString);
+ }
+
+ public function attachAlternateCSRFString($string) {
+ $this->alternateCSRFString = $string;
+ return $this;
+ }
+
private static function tokenizeName($name) {
if (function_exists('mb_strtolower')) {
$name = mb_strtolower($name, 'UTF-8');
Index: src/view/page/PhabricatorStandardPageView.php
===================================================================
--- src/view/page/PhabricatorStandardPageView.php
+++ src/view/page/PhabricatorStandardPageView.php
@@ -163,17 +163,6 @@
Javelin::initBehavior('history-install');
Javelin::initBehavior('phabricator-gesture');
- // If the client doesn't have a session token, generate an anonymous
- // session. This is used to provide CSRF protection to logged-out users.
- $session_token = $request->getCookie(PhabricatorCookies::COOKIE_SESSION);
- if (!strlen($session_token)) {
- $anonymous_session = id(new PhabricatorAuthSessionEngine())
- ->establishSession('web', null);
- $request->setCookie(
- PhabricatorCookies::COOKIE_SESSION,
- $anonymous_session);
- }
-
$current_token = null;
if ($user) {
$current_token = $user->getCSRFToken();
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Wed, Mar 26, 2:23 PM (3 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7229552
Default Alt Text
D8046.id18213.diff (7 KB)
Attached To
Mode
D8046: Support CSRF for logged-out users
Attached
Detach File
Event Timeline
Log In to Comment