From f42ec84d0c6b8d2868da72c9324e4a34be804207 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 27 Apr 2014 17:31:11 -0700 Subject: [PATCH] Add "High Security" mode to support multi-factor auth Summary: Ref T4398. This is roughly a "sudo" mode, like GitHub has for accessing SSH keys, or Facebook has for managing credit cards. GitHub actually calls theirs "sudo" mode, but I think that's too technical for big parts of our audience. I've gone with "high security mode". This doesn't actually get exposed in the UI yet (and we don't have any meaningful auth factors to prompt the user for) but the workflow works overall. I'll go through it in a comment, since I need to arrange some screenshots. Test Plan: See guided walkthrough. Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4398 Differential Revision: https://secure.phabricator.com/D8851 --- .../autopatches/20140423.session.1.hisec.sql | 2 + src/__phutil_library_map__.php | 5 + src/aphront/AphrontRequest.php | 2 + ...AphrontDefaultApplicationConfiguration.php | 43 ++++++ .../PhabricatorApplicationAuth.php | 2 + ...bricatorAuthDowngradeSessionController.php | 52 +++++++ .../data/PhabricatorAuthHighSecurityToken.php | 5 + .../engine/PhabricatorAuthSessionEngine.php | 142 ++++++++++++++++-- ...catorAuthHighSecurityRequiredException.php | 16 ++ .../auth/storage/PhabricatorAuthSession.php | 1 + .../people/storage/PhabricatorUser.php | 14 ++ .../panel/PhabricatorSettingsPanelSSHKeys.php | 12 ++ .../PhabricatorSettingsPanelSessions.php | 21 +++ src/view/page/PhabricatorStandardPageView.php | 16 ++ webroot/rsrc/css/aphront/notification.css | 5 + .../js/core/behavior-high-security-warning.js | 19 +++ 16 files changed, 346 insertions(+), 11 deletions(-) create mode 100644 resources/sql/autopatches/20140423.session.1.hisec.sql create mode 100644 src/applications/auth/controller/PhabricatorAuthDowngradeSessionController.php create mode 100644 src/applications/auth/data/PhabricatorAuthHighSecurityToken.php create mode 100644 src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php create mode 100644 webroot/rsrc/js/core/behavior-high-security-warning.js diff --git a/resources/sql/autopatches/20140423.session.1.hisec.sql b/resources/sql/autopatches/20140423.session.1.hisec.sql new file mode 100644 index 0000000000..4b49d1b563 --- /dev/null +++ b/resources/sql/autopatches/20140423.session.1.hisec.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_user.phabricator_session + ADD highSecurityUntil INT UNSIGNED; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index af7b53c894..85c7847d25 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1206,7 +1206,10 @@ phutil_register_library_map(array( 'PhabricatorAuthController' => 'applications/auth/controller/PhabricatorAuthController.php', 'PhabricatorAuthDAO' => 'applications/auth/storage/PhabricatorAuthDAO.php', 'PhabricatorAuthDisableController' => 'applications/auth/controller/config/PhabricatorAuthDisableController.php', + 'PhabricatorAuthDowngradeSessionController' => 'applications/auth/controller/PhabricatorAuthDowngradeSessionController.php', 'PhabricatorAuthEditController' => 'applications/auth/controller/config/PhabricatorAuthEditController.php', + 'PhabricatorAuthHighSecurityRequiredException' => 'applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php', + 'PhabricatorAuthHighSecurityToken' => 'applications/auth/data/PhabricatorAuthHighSecurityToken.php', 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', 'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php', 'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php', @@ -3947,7 +3950,9 @@ phutil_register_library_map(array( 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorAuthDAO' => 'PhabricatorLiskDAO', 'PhabricatorAuthDisableController' => 'PhabricatorAuthProviderConfigController', + 'PhabricatorAuthDowngradeSessionController' => 'PhabricatorAuthController', 'PhabricatorAuthEditController' => 'PhabricatorAuthProviderConfigController', + 'PhabricatorAuthHighSecurityRequiredException' => 'Exception', 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index 8c4b66c3ec..c83746c5e5 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -18,6 +18,7 @@ final class AphrontRequest { const TYPE_WORKFLOW = '__wflow__'; const TYPE_CONTINUE = '__continue__'; const TYPE_PREVIEW = '__preview__'; + const TYPE_HISEC = '__hisec__'; private $host; private $path; @@ -263,6 +264,7 @@ final class AphrontRequest { final public function isFormPost() { $post = $this->getExists(self::TYPE_FORM) && + !$this->getExists(self::TYPE_HISEC) && $this->isHTTPPost(); if (!$post) { diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index a642823356..8da9b22b23 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -123,6 +123,49 @@ class AphrontDefaultApplicationConfiguration return $response; } + if ($ex instanceof PhabricatorAuthHighSecurityRequiredException) { + + $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( + $user, + $request); + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->setTitle(pht('Entering High Security')) + ->setShortTitle(pht('Security Checkpoint')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->addHiddenInput(AphrontRequest::TYPE_HISEC, true) + ->setErrors( + array( + pht( + 'You are taking an action which requires you to enter '. + 'high security.'), + )) + ->appendParagraph( + pht( + 'High security mode helps protect your account from security '. + 'threats, like session theft or someone messing with your stuff '. + 'while you\'re grabbing a coffee. To enter high security mode, '. + 'confirm your credentials.')) + ->appendChild($form->buildLayoutView()) + ->appendParagraph( + pht( + 'Your account will remain in high security mode for a short '. + 'period of time. When you are finished taking sensitive '. + 'actions, you should leave high security.')) + ->setSubmitURI($request->getPath()) + ->addCancelButton($ex->getCancelURI()) + ->addSubmitButton(pht('Enter High Security')); + + foreach ($request->getPassthroughRequestParameters() as $key => $value) { + $dialog->addHiddenInput($key, $value); + } + + $response = new AphrontDialogResponse(); + $response->setDialog($dialog); + return $response; + } + if ($ex instanceof PhabricatorPolicyException) { if (!$user->isLoggedIn()) { diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index 28d7209fda..ec05fa594b 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -88,6 +88,8 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { => 'PhabricatorAuthConfirmLinkController', 'session/terminate/(?P[^/]+)/' => 'PhabricatorAuthTerminateSessionController', + 'session/downgrade/' + => 'PhabricatorAuthDowngradeSessionController', ), '/oauth/(?P\w+)/login/' diff --git a/src/applications/auth/controller/PhabricatorAuthDowngradeSessionController.php b/src/applications/auth/controller/PhabricatorAuthDowngradeSessionController.php new file mode 100644 index 0000000000..c7feabc606 --- /dev/null +++ b/src/applications/auth/controller/PhabricatorAuthDowngradeSessionController.php @@ -0,0 +1,52 @@ +getRequest(); + $viewer = $request->getUser(); + + $panel_uri = '/settings/panel/sessions/'; + + $session = $viewer->getSession(); + if ($session->getHighSecurityUntil() < time()) { + return $this->newDialog() + ->setTitle(pht('Normal Security Restored')) + ->appendParagraph( + pht('Your session is no longer in high security.')) + ->addCancelButton($panel_uri, pht('Continue')); + } + + if ($request->isFormPost()) { + + queryfx( + $session->establishConnection('w'), + 'UPDATE %T SET highSecurityUntil = NULL WHERE id = %d', + $session->getTableName(), + $session->getID()); + + return id(new AphrontRedirectResponse()) + ->setURI($this->getApplicationURI('session/downgrade/')); + } + + return $this->newDialog() + ->setTitle(pht('Leaving High Security')) + ->appendParagraph( + pht( + 'Leave high security and return your session to normal '. + 'security levels?')) + ->appendParagraph( + pht( + 'If you leave high security, you will need to authenticate '. + 'again the next time you try to take a high security action.')) + ->appendParagraph( + pht( + 'On the plus side, that purple notification bubble will '. + 'disappear.')) + ->addSubmitButton(pht('Leave High Security')) + ->addCancelButton($panel_uri, pht('Stay in High Security')); + } + + +} diff --git a/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php b/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php new file mode 100644 index 0000000000..715e9ced47 --- /dev/null +++ b/src/applications/auth/data/PhabricatorAuthHighSecurityToken.php @@ -0,0 +1,5 @@ +establishConnection('r'); + $session_key = PhabricatorHash::digest($session_token); // NOTE: We're being clever here because this happens on every page load, - // and by joining we can save a query. + // and by joining we can save a query. This might be getting too clever + // for its own good, though... $info = queryfx_one( $conn_r, - 'SELECT s.sessionExpires AS _sessionExpires, s.id AS _sessionID, u.* + 'SELECT + s.id AS s_id, + s.sessionExpires AS s_sessionExpires, + s.sessionStart AS s_sessionStart, + s.highSecurityUntil AS s_highSecurityUntil, + u.* FROM %T u JOIN %T s ON u.phid = s.userPHID AND s.type = %s AND s.sessionKey = %s', $user_table->getTableName(), $session_table->getTableName(), $session_type, - PhabricatorHash::digest($session_token)); + $session_key); if (!$info) { return null; } - $expires = $info['_sessionExpires']; - $id = $info['_sessionID']; - unset($info['_sessionExpires']); - unset($info['_sessionID']); + $session_dict = array( + 'userPHID' => $info['phid'], + 'sessionKey' => $session_key, + 'type' => $session_type, + ); + foreach ($info as $key => $value) { + if (strncmp($key, 's_', 2) === 0) { + unset($info[$key]); + $session_dict[substr($key, 2)] = $value; + } + } + $session = id(new PhabricatorAuthSession())->loadFromArray($session_dict); $ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type); @@ -107,19 +125,21 @@ final class PhabricatorAuthSessionEngine extends Phobject { // TTL back up to the full duration. The idea here is that sessions are // good forever if used regularly, but get GC'd when they fall out of use. - if (time() + (0.80 * $ttl) > $expires) { + if (time() + (0.80 * $ttl) > $session->getSessionExpires()) { $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $conn_w = $session_table->establishConnection('w'); queryfx( $conn_w, 'UPDATE %T SET sessionExpires = UNIX_TIMESTAMP() + %d WHERE id = %d', - $session_table->getTableName(), + $session->getTableName(), $ttl, - $id); + $session->getID()); unset($unguarded); } - return $user_table->loadFromArray($info); + $user = $user_table->loadFromArray($info); + $user->attachSession($session); + return $user; } @@ -182,4 +202,104 @@ final class PhabricatorAuthSessionEngine extends Phobject { return $session_key; } + + /** + * Require high security, or prompt the user to enter high security. + * + * If the user's session is in high security, this method will return a + * token. Otherwise, it will throw an exception which will eventually + * be converted into a multi-factor authentication workflow. + * + * @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. + */ + public function requireHighSecuritySession( + PhabricatorUser $viewer, + AphrontRequest $request, + $cancel_uri) { + + if (!$viewer->hasSession()) { + throw new Exception( + pht('Requiring a high-security session from a user with no session!')); + } + + $session = $viewer->getSession(); + + $token = $this->issueHighSecurityToken($session); + if ($token) { + return $token; + } + + if ($request->isHTTPPost()) { + $request->validateCSRF(); + if ($request->getExists(AphrontRequest::TYPE_HISEC)) { + + // TODO: Actually verify that the user provided some multi-factor + // auth credentials here. For now, we just let you enter high + // security. + + $until = time() + phutil_units('15 minutes in seconds'); + $session->setHighSecurityUntil($until); + + queryfx( + $session->establishConnection('w'), + 'UPDATE %T SET highSecurityUntil = %d WHERE id = %d', + $session->getTableName(), + $until, + $session->getID()); + } + } + + $token = $this->issueHighSecurityToken($session); + if ($token) { + return $token; + } + + throw id(new PhabricatorAuthHighSecurityRequiredException()) + ->setCancelURI($cancel_uri); + } + + + /** + * Issue a high security token for a session, if authorized. + * + * @param PhabricatorAuthSession Session to issue a token for. + * @return PhabricatorAuthHighSecurityToken|null Token, if authorized. + */ + private function issueHighSecurityToken(PhabricatorAuthSession $session) { + $until = $session->getHighSecurityUntil(); + if ($until > time()) { + return new PhabricatorAuthHighSecurityToken(); + } + return null; + } + + + /** + * Render a form for providing relevant multi-factor credentials. + * + * @param PhabricatorUser Viewing user. + * @param AphrontRequest Current request. + * @return AphrontFormView Renderable form. + */ + public function renderHighSecurityForm( + PhabricatorUser $viewer, + AphrontRequest $request) { + + // TODO: This is stubbed. + + $form = id(new AphrontFormView()) + ->setUser($viewer) + ->appendRemarkupInstructions('') + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel(pht('Secret Stuff'))) + ->appendRemarkupInstructions(''); + + return $form; + } + + } diff --git a/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php b/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php new file mode 100644 index 0000000000..16022e53b8 --- /dev/null +++ b/src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php @@ -0,0 +1,16 @@ +cancelURI = $cancel_uri; + return $this; + } + + public function getCancelURI() { + return $this->cancelURI; + } + +} diff --git a/src/applications/auth/storage/PhabricatorAuthSession.php b/src/applications/auth/storage/PhabricatorAuthSession.php index f9d2e41f6e..f78a06baec 100644 --- a/src/applications/auth/storage/PhabricatorAuthSession.php +++ b/src/applications/auth/storage/PhabricatorAuthSession.php @@ -11,6 +11,7 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO protected $sessionKey; protected $sessionStart; protected $sessionExpires; + protected $highSecurityUntil; private $identityObject = self::ATTACHABLE; diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 29527f99bd..a62be09fe7 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -42,6 +42,7 @@ final class PhabricatorUser private $customFields = self::ATTACHABLE; private $alternateCSRFString = self::ATTACHABLE; + private $session = self::ATTACHABLE; protected function readField($field) { switch ($field) { @@ -178,6 +179,19 @@ final class PhabricatorUser return $result; } + public function attachSession(PhabricatorAuthSession $session) { + $this->session = $session; + return $this; + } + + public function getSession() { + return $this->assertAttached($this->session); + } + + public function hasSession() { + return ($this->session !== self::ATTACHABLE); + } + private function generateConduitCertificate() { return Filesystem::readRandomCharacters(255); } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php index ca4bbc236e..3af78bd169 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php @@ -38,6 +38,18 @@ final class PhabricatorSettingsPanelSSHKeys return $this->renderKeyListView($request); } + /* + + NOTE: Uncomment this to test hisec. + TOOD: Implement this fully once hisec does something useful. + + $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( + $viewer, + $request, + '/settings/panel/ssh/'); + + */ + $id = nonempty($edit, $delete); if ($id && is_numeric($id)) { diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php b/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php index 4c98efc27f..9968be768b 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php @@ -66,10 +66,15 @@ final class PhabricatorSettingsPanelSessions pht('Terminate')); } + $hisec = ($session->getHighSecurityUntil() - time()); + $rows[] = array( $handles[$session->getUserPHID()]->renderLink(), substr($session->getSessionKey(), 0, 6), $session->getType(), + ($hisec > 0) + ? phabricator_format_relative_time($hisec) + : null, phabricator_datetime($session->getSessionStart(), $viewer), phabricator_date($session->getSessionExpires(), $viewer), $button, @@ -84,6 +89,7 @@ final class PhabricatorSettingsPanelSessions pht('Identity'), pht('Session'), pht('Type'), + pht('HiSec'), pht('Created'), pht('Expires'), pht(''), @@ -95,6 +101,7 @@ final class PhabricatorSettingsPanelSessions '', 'right', 'right', + 'right', 'action', )); @@ -113,6 +120,20 @@ final class PhabricatorSettingsPanelSessions ->setHeader(pht('Active Login Sessions')) ->addActionLink($terminate_button); + $hisec = ($viewer->getSession()->getHighSecurityUntil() - time()); + if ($hisec > 0) { + $hisec_icon = id(new PHUIIconView()) + ->setSpriteSheet(PHUIIconView::SPRITE_ICONS) + ->setSpriteIcon('lock'); + $hisec_button = id(new PHUIButtonView()) + ->setText(pht('Leave High Security')) + ->setHref('/auth/session/downgrade/') + ->setTag('a') + ->setWorkflow(true) + ->setIcon($hisec_icon); + $header->addActionLink($hisec_button); + } + $panel = id(new PHUIObjectBoxView()) ->setHeader($header) ->appendChild($table); diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index f4cd10be14..ae32cf8250 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -168,6 +168,22 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { Javelin::initBehavior('device'); + if ($user->hasSession()) { + $hisec = ($user->getSession()->getHighSecurityUntil() - time()); + if ($hisec > 0) { + $remaining_time = phabricator_format_relative_time($hisec); + Javelin::initBehavior( + 'high-security-warning', + array( + 'uri' => '/auth/session/downgrade/', + 'message' => pht( + 'Your session is in high security mode. When you '. + 'finish using it, click here to leave.', + $remaining_time), + )); + } + } + if ($console) { require_celerity_resource('aphront-dark-console-css'); diff --git a/webroot/rsrc/css/aphront/notification.css b/webroot/rsrc/css/aphront/notification.css index 8cb810c327..406f3a3143 100644 --- a/webroot/rsrc/css/aphront/notification.css +++ b/webroot/rsrc/css/aphront/notification.css @@ -47,6 +47,11 @@ border: 1px solid {$red}; } +.jx-notification-security { + background: {$lightviolet}; + border: 1px solid {$violet}; +} + .jx-notification-container .phabricator-notification { padding: 0; } diff --git a/webroot/rsrc/js/core/behavior-high-security-warning.js b/webroot/rsrc/js/core/behavior-high-security-warning.js new file mode 100644 index 0000000000..bbbbd1f945 --- /dev/null +++ b/webroot/rsrc/js/core/behavior-high-security-warning.js @@ -0,0 +1,19 @@ +/** + * @provides javelin-behavior-high-security-warning + * @requires javelin-behavior + * javelin-uri + * phabricator-notification + */ + +JX.behavior('high-security-warning', function(config) { + + var n = new JX.Notification() + .setContent(config.message) + .setDuration(0) + .alterClassName('jx-notification-security', true); + + n.listen('activate', function() { JX.$U(config.uri).go(); }); + + n.show(); + +});