Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15410836
D8922.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
17 KB
Referenced Files
None
Subscribers
None
D8922.diff
View Options
diff --git a/resources/sql/autopatches/20140430.auth.1.partial.sql b/resources/sql/autopatches/20140430.auth.1.partial.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20140430.auth.1.partial.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_user.phabricator_session
+ ADD isPartial BOOL NOT NULL DEFAULT 0;
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1213,6 +1213,7 @@
'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php',
'PhabricatorAuthFactorTOTP' => 'applications/auth/factor/PhabricatorAuthFactorTOTP.php',
'PhabricatorAuthFactorTOTPTestCase' => 'applications/auth/factor/__tests__/PhabricatorAuthFactorTOTPTestCase.php',
+ 'PhabricatorAuthFinishController' => 'applications/auth/controller/PhabricatorAuthFinishController.php',
'PhabricatorAuthHighSecurityRequiredException' => 'applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php',
'PhabricatorAuthHighSecurityToken' => 'applications/auth/data/PhabricatorAuthHighSecurityToken.php',
'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php',
@@ -3980,6 +3981,7 @@
'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO',
'PhabricatorAuthFactorTOTP' => 'PhabricatorAuthFactor',
'PhabricatorAuthFactorTOTPTestCase' => 'PhabricatorTestCase',
+ 'PhabricatorAuthFinishController' => 'PhabricatorAuthController',
'PhabricatorAuthHighSecurityRequiredException' => 'Exception',
'PhabricatorAuthLinkController' => 'PhabricatorAuthController',
'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController',
diff --git a/src/aphront/console/DarkConsoleController.php b/src/aphront/console/DarkConsoleController.php
--- a/src/aphront/console/DarkConsoleController.php
+++ b/src/aphront/console/DarkConsoleController.php
@@ -16,6 +16,10 @@
return !PhabricatorEnv::getEnvConfig('darkconsole.always-on');
}
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
public function processRequest() {
$request = $this->getRequest();
$user = $request->getUser();
diff --git a/src/aphront/console/DarkConsoleDataController.php b/src/aphront/console/DarkConsoleDataController.php
--- a/src/aphront/console/DarkConsoleDataController.php
+++ b/src/aphront/console/DarkConsoleDataController.php
@@ -15,6 +15,10 @@
return !PhabricatorEnv::getEnvConfig('darkconsole.always-on');
}
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
public function willProcessRequest(array $data) {
$this->key = $data['key'];
}
diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php
--- a/src/applications/auth/application/PhabricatorApplicationAuth.php
+++ b/src/applications/auth/application/PhabricatorApplicationAuth.php
@@ -83,6 +83,7 @@
'register/(?:(?P<akey>[^/]+)/)?' => 'PhabricatorAuthRegisterController',
'start/' => 'PhabricatorAuthStartController',
'validate/' => 'PhabricatorAuthValidateController',
+ 'finish/' => 'PhabricatorAuthFinishController',
'unlink/(?P<pkey>[^/]+)/' => 'PhabricatorAuthUnlinkController',
'(?P<action>link|refresh)/(?P<pkey>[^/]+)/'
=> 'PhabricatorAuthLinkController',
diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php
--- a/src/applications/auth/controller/PhabricatorAuthController.php
+++ b/src/applications/auth/controller/PhabricatorAuthController.php
@@ -82,7 +82,7 @@
$should_login = $event->getValue('shouldLogin');
if ($should_login) {
$session_key = id(new PhabricatorAuthSessionEngine())
- ->establishSession($session_type, $user->getPHID());
+ ->establishSession($session_type, $user->getPHID(), $partial = true);
// NOTE: We allow disabled users to login and roadblock them later, so
// there's no check for users being disabled here.
diff --git a/src/applications/auth/controller/PhabricatorAuthFinishController.php b/src/applications/auth/controller/PhabricatorAuthFinishController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/auth/controller/PhabricatorAuthFinishController.php
@@ -0,0 +1,71 @@
+<?php
+
+final class PhabricatorAuthFinishController
+ extends PhabricatorAuthController {
+
+ public function shouldRequireLogin() {
+ return false;
+ }
+
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
+ public function processRequest() {
+ $request = $this->getRequest();
+ $viewer = $request->getUser();
+
+ // If the user already has a full session, just kick them out of here.
+ $has_partial_session = $viewer->hasSession() &&
+ $viewer->getSession()->getIsPartial();
+ if (!$has_partial_session) {
+ return id(new AphrontRedirectResponse())->setURI('/');
+ }
+
+ $engine = new PhabricatorAuthSessionEngine();
+
+ try {
+ $token = $engine->requireHighSecuritySession(
+ $viewer,
+ $request,
+ '/logout/');
+ } catch (PhabricatorAuthHighSecurityRequiredException $ex) {
+ $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm(
+ $ex->getFactors(),
+ $ex->getFactorValidationResults(),
+ $viewer,
+ $request);
+
+ return $this->newDialog()
+ ->setTitle(pht('Provide Multi-Factor Credentials'))
+ ->setShortTitle(pht('Multi-Factor Login'))
+ ->setWidth(AphrontDialogView::WIDTH_FORM)
+ ->addHiddenInput(AphrontRequest::TYPE_HISEC, true)
+ ->appendParagraph(
+ pht(
+ 'Welcome, %s. To complete the login process, provide your '.
+ 'multi-factor credentials.',
+ phutil_tag('strong', array(), $viewer->getUsername())))
+ ->appendChild($form->buildLayoutView())
+ ->setSubmitURI($request->getPath())
+ ->addCancelButton($ex->getCancelURI())
+ ->addSubmitButton(pht('Continue'));
+ }
+
+ // Upgrade the partial session to a full session.
+ $engine->upgradePartialSession($viewer);
+
+ // TODO: It might be nice to add options like "bind this session to my IP"
+ // here, even for accounts without multi-factor auth attached to them.
+
+ $next = PhabricatorCookies::getNextURICookie($request);
+ $request->clearCookie(PhabricatorCookies::COOKIE_NEXTURI);
+
+ if (!PhabricatorEnv::isValidLocalWebResource($next)) {
+ $next = '/';
+ }
+
+ return id(new AphrontRedirectResponse())->setURI($next);
+ }
+
+}
diff --git a/src/applications/auth/controller/PhabricatorAuthValidateController.php b/src/applications/auth/controller/PhabricatorAuthValidateController.php
--- a/src/applications/auth/controller/PhabricatorAuthValidateController.php
+++ b/src/applications/auth/controller/PhabricatorAuthValidateController.php
@@ -7,6 +7,10 @@
return false;
}
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
public function processRequest() {
$request = $this->getRequest();
$viewer = $request->getUser();
@@ -54,14 +58,8 @@
return $this->renderErrors($failures);
}
- $next = PhabricatorCookies::getNextURICookie($request);
- $request->clearCookie(PhabricatorCookies::COOKIE_NEXTURI);
-
- if (!PhabricatorEnv::isValidLocalWebResource($next)) {
- $next = '/';
- }
-
- return id(new AphrontRedirectResponse())->setURI($next);
+ $finish_uri = $this->getApplicationURI('finish/');
+ return id(new AphrontRedirectResponse())->setURI($finish_uri);
}
private function renderErrors(array $messages) {
diff --git a/src/applications/auth/controller/PhabricatorLogoutController.php b/src/applications/auth/controller/PhabricatorLogoutController.php
--- a/src/applications/auth/controller/PhabricatorLogoutController.php
+++ b/src/applications/auth/controller/PhabricatorLogoutController.php
@@ -17,6 +17,10 @@
return false;
}
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
public function processRequest() {
$request = $this->getRequest();
$user = $request->getUser();
diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
--- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
+++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php
@@ -94,6 +94,7 @@
s.sessionExpires AS s_sessionExpires,
s.sessionStart AS s_sessionStart,
s.highSecurityUntil AS s_highSecurityUntil,
+ s.isPartial AS s_isPartial,
u.*
FROM %T u JOIN %T s ON u.phid = s.userPHID
AND s.type = %s AND s.sessionKey = %s',
@@ -159,9 +160,10 @@
* @{class:PhabricatorAuthSession}).
* @param phid|null Identity to establish a session for, usually a user
* PHID. With `null`, generates an anonymous session.
+ * @param bool True to issue a partial session.
* @return string Newly generated session key.
*/
- public function establishSession($session_type, $identity_phid) {
+ public function establishSession($session_type, $identity_phid, $partial) {
// Consume entropy to generate a new session key, forestalling the eventual
// heat death of the universe.
$session_key = Filesystem::readRandomCharacters(40);
@@ -176,26 +178,32 @@
// This has a side effect of validating the session type.
$session_ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type);
+ $digest_key = PhabricatorHash::digest($session_key);
+
// Logging-in users don't have CSRF stuff yet, so we have to unguard this
// write.
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
id(new PhabricatorAuthSession())
->setUserPHID($identity_phid)
->setType($session_type)
- ->setSessionKey(PhabricatorHash::digest($session_key))
+ ->setSessionKey($digest_key)
->setSessionStart(time())
->setSessionExpires(time() + $session_ttl)
+ ->setIsPartial($partial ? 1 : 0)
->save();
$log = PhabricatorUserLog::initializeNewLog(
null,
$identity_phid,
- PhabricatorUserLog::ACTION_LOGIN);
+ ($partial
+ ? PhabricatorUserLog::ACTION_LOGIN_PARTIAL
+ : PhabricatorUserLog::ACTION_LOGIN));
+
$log->setDetails(
array(
'session_type' => $session_type,
));
- $log->setSession($session_key);
+ $log->setSession($digest_key);
$log->save();
unset($unguarded);
@@ -287,6 +295,12 @@
new PhabricatorAuthTryFactorAction(),
-1);
+ if ($session->getIsPartial()) {
+ // If we have a partial session, just issue a token without
+ // putting it in high security mode.
+ return $this->issueHighSecurityToken($session, true);
+ }
+
$until = time() + phutil_units('15 minutes in seconds');
$session->setHighSecurityUntil($until);
@@ -303,9 +317,6 @@
PhabricatorUserLog::ACTION_ENTER_HISEC);
$log->save();
} else {
-
-
-
$log = PhabricatorUserLog::initializeNewLog(
$viewer,
$viewer->getPHID(),
@@ -331,11 +342,15 @@
* Issue a high security token for a session, if authorized.
*
* @param PhabricatorAuthSession Session to issue a token for.
+ * @param bool Force token issue.
* @return PhabricatorAuthHighSecurityToken|null Token, if authorized.
*/
- private function issueHighSecurityToken(PhabricatorAuthSession $session) {
+ private function issueHighSecurityToken(
+ PhabricatorAuthSession $session,
+ $force = false) {
+
$until = $session->getHighSecurityUntil();
- if ($until > time()) {
+ if ($until > time() || $force) {
return new PhabricatorAuthHighSecurityToken();
}
return null;
@@ -390,4 +405,42 @@
$log->save();
}
+
+ /**
+ * Upgrade a partial session to a full session.
+ *
+ * @param PhabricatorAuthSession Session to upgrade.
+ * @return void
+ */
+ public function upgradePartialSession(PhabricatorUser $viewer) {
+ if (!$viewer->hasSession()) {
+ throw new Exception(
+ pht('Upgrading partial session of user with no session!'));
+ }
+
+ $session = $viewer->getSession();
+
+ if (!$session->getIsPartial()) {
+ throw new Exception(pht('Session is not partial!'));
+ }
+
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+ $session->setIsPartial(0);
+
+ queryfx(
+ $session->establishConnection('w'),
+ 'UPDATE %T SET isPartial = %d WHERE id = %d',
+ $session->getTableName(),
+ 0,
+ $session->getID());
+
+ $log = PhabricatorUserLog::initializeNewLog(
+ $viewer,
+ $viewer->getPHID(),
+ PhabricatorUserLog::ACTION_LOGIN_FULL);
+ $log->save();
+ unset($unguarded);
+
+ }
+
}
diff --git a/src/applications/auth/storage/PhabricatorAuthSession.php b/src/applications/auth/storage/PhabricatorAuthSession.php
--- a/src/applications/auth/storage/PhabricatorAuthSession.php
+++ b/src/applications/auth/storage/PhabricatorAuthSession.php
@@ -12,6 +12,7 @@
protected $sessionStart;
protected $sessionExpires;
protected $highSecurityUntil;
+ protected $isPartial;
private $identityObject = self::ATTACHABLE;
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -20,6 +20,10 @@
return false;
}
+ public function shouldAllowPartialSessions() {
+ return false;
+ }
+
public function shouldRequireEmailVerification() {
return PhabricatorUserEmail::isEmailVerificationRequired();
}
@@ -53,7 +57,8 @@
// session. This is used to provide CSRF protection to logged-out users.
$phsid = $session_engine->establishSession(
PhabricatorAuthSession::TYPE_WEB,
- null);
+ null,
+ $partial = false);
// This may be a resource request, in which case we just don't set
// the cookie.
@@ -133,14 +138,24 @@
return $this->delegateToController($checker_controller);
}
+ $auth_class = 'PhabricatorApplicationAuth';
+ $auth_application = PhabricatorApplication::getByClass($auth_class);
+
+ // Require partial sessions to finish login before doing anything.
+ if (!$this->shouldAllowPartialSessions()) {
+ if ($user->hasSession() &&
+ $user->getSession()->getIsPartial()) {
+ $login_controller = new PhabricatorAuthFinishController($request);
+ $this->setCurrentApplication($auth_application);
+ return $this->delegateToController($login_controller);
+ }
+ }
+
if ($this->shouldRequireLogin()) {
// This actually means we need either:
// - a valid user, or a public controller; and
// - permission to see the application.
- $auth_class = 'PhabricatorApplicationAuth';
- $auth_application = PhabricatorApplication::getByClass($auth_class);
-
$allow_public = $this->shouldAllowPublic() &&
PhabricatorEnv::getEnvConfig('policy.allow-public');
diff --git a/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php b/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php
--- a/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php
+++ b/src/applications/conduit/method/ConduitAPI_conduit_connect_Method.php
@@ -145,10 +145,10 @@
if ($valid != $signature) {
throw new ConduitException('ERR-INVALID-CERTIFICATE');
}
- $session_key = id(new PhabricatorAuthSessionEngine())
- ->establishSession(
- PhabricatorAuthSession::TYPE_CONDUIT,
- $user->getPHID());
+ $session_key = id(new PhabricatorAuthSessionEngine())->establishSession(
+ PhabricatorAuthSession::TYPE_CONDUIT,
+ $user->getPHID(),
+ $partial = false);
} else {
throw new ConduitException('ERR-NO-CERTIFICATE');
}
diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php
--- a/src/applications/people/storage/PhabricatorUserLog.php
+++ b/src/applications/people/storage/PhabricatorUserLog.php
@@ -4,6 +4,8 @@
implements PhabricatorPolicyInterface {
const ACTION_LOGIN = 'login';
+ const ACTION_LOGIN_PARTIAL = 'login-partial';
+ const ACTION_LOGIN_FULL = 'login-full';
const ACTION_LOGOUT = 'logout';
const ACTION_LOGIN_FAILURE = 'login-fail';
const ACTION_RESET_PASSWORD = 'reset-pass';
@@ -46,7 +48,9 @@
public static function getActionTypeMap() {
return array(
self::ACTION_LOGIN => pht('Login'),
- self::ACTION_LOGIN_FAILURE => pht('Login Failure'),
+ self::ACTION_LOGIN_PARTIAL => pht('Login: Partial Login'),
+ self::ACTION_LOGIN_FULL => pht('Login: Upgrade to Full'),
+ self::ACTION_LOGIN_FAILURE => pht('Login: Failure'),
self::ACTION_LOGOUT => pht('Logout'),
self::ACTION_RESET_PASSWORD => pht('Reset Password'),
self::ACTION_CREATE => pht('Create Account'),
diff --git a/src/infrastructure/celerity/CelerityResourceController.php b/src/infrastructure/celerity/CelerityResourceController.php
--- a/src/infrastructure/celerity/CelerityResourceController.php
+++ b/src/infrastructure/celerity/CelerityResourceController.php
@@ -14,6 +14,10 @@
return false;
}
+ public function shouldAllowPartialSessions() {
+ return true;
+ }
+
abstract public function getCelerityResourceMap();
protected function serveResource($path, $package_hash = null) {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 20, 8:32 AM (2 d, 44 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7669279
Default Alt Text
D8922.diff (17 KB)
Attached To
Mode
D8922: Require multiple auth factors to establish web sessions
Attached
Detach File
Event Timeline
Log In to Comment