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 @@ -345,6 +345,33 @@ /* -( High Security )------------------------------------------------------ */ + /** + * Require the user respond to a high security (MFA) check. + * + * This method differs from @{method:requireHighSecuritySession} in that it + * does not upgrade the user's session as a side effect. This method is + * appropriate for one-time checks. + * + * @param PhabricatorUser User whose session needs to be in high security. + * @param AphrontReqeust Current request. + * @param string URI to return the user to if they cancel. + * @return PhabricatorAuthHighSecurityToken Security token. + * @task hisec + */ + public function requireHighSecurityToken( + PhabricatorUser $viewer, + AphrontRequest $request, + $cancel_uri) { + + return $this->newHighSecurityToken( + $viewer, + $request, + $cancel_uri, + false, + false); + } + + /** * Require high security, or prompt the user to enter high security. * @@ -352,6 +379,11 @@ * token. Otherwise, it will throw an exception which will eventually * be converted into a multi-factor authentication workflow. * + * This method upgrades the user's session to high security for a short + * period of time, and is appropriate if you anticipate they may need to + * take multiple high security actions. To perform a one-time check instead, + * use @{method:requireHighSecurityToken}. + * * @param PhabricatorUser User whose session needs to be in high security. * @param AphrontReqeust Current request. * @param string URI to return the user to if they cancel. @@ -367,11 +399,30 @@ $cancel_uri, $jump_into_hisec = false) { + return $this->newHighSecurityToken( + $viewer, + $request, + $cancel_uri, + false, + true); + } + + private function newHighSecurityToken( + PhabricatorUser $viewer, + AphrontRequest $request, + $cancel_uri, + $jump_into_hisec, + $upgrade_session) { + if (!$viewer->hasSession()) { throw new Exception( pht('Requiring a high-security session from a user with no session!')); } + // TODO: If a user answers a "requireHighSecurityToken()" prompt and hits + // a "requireHighSecuritySession()" prompt a short time later, the one-shot + // token should be good enough to upgrade the session. + $session = $viewer->getSession(); // Check if the session is already in high security mode. @@ -441,6 +492,11 @@ return $this->issueHighSecurityToken($session, true); } + // If we aren't upgrading the session itself, just issue a token. + if (!$upgrade_session) { + return $this->issueHighSecurityToken($session, true); + } + $until = time() + phutil_units('15 minutes in seconds'); $session->setHighSecurityUntil($until); diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignController.php b/src/applications/legalpad/controller/LegalpadDocumentSignController.php --- a/src/applications/legalpad/controller/LegalpadDocumentSignController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php @@ -149,15 +149,16 @@ } $errors = array(); + $hisec_token = null; if ($request->isFormOrHisecPost() && !$has_signed) { // Require two-factor auth to sign legal documents. if ($viewer->isLoggedIn()) { - $engine = new PhabricatorAuthSessionEngine(); - $engine->requireHighSecuritySession( - $viewer, - $request, - '/'.$document->getMonogram()); + $hisec_token = id(new PhabricatorAuthSessionEngine()) + ->requireHighSecurityToken( + $viewer, + $request, + $document->getURI()); } list($form_data, $errors, $field_errors) = $this->readSignatureForm( diff --git a/src/applications/legalpad/storage/LegalpadDocument.php b/src/applications/legalpad/storage/LegalpadDocument.php --- a/src/applications/legalpad/storage/LegalpadDocument.php +++ b/src/applications/legalpad/storage/LegalpadDocument.php @@ -120,7 +120,7 @@ return 'L'.$this->getID(); } - public function getViewURI() { + public function getURI() { return '/'.$this->getMonogram(); }