Skip to content

Commit f42ec84

Browse files
author
epriestley
committed
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
1 parent c453e98 commit f42ec84

16 files changed

+346
-11
lines changed

‎resources/sql/autopatches/20140423.session.1.hisec.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE {$NAMESPACE}_user.phabricator_session
2+
ADD highSecurityUntil INT UNSIGNED;

‎src/__phutil_library_map__.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,10 @@
12061206
'PhabricatorAuthController' => 'applications/auth/controller/PhabricatorAuthController.php',
12071207
'PhabricatorAuthDAO' => 'applications/auth/storage/PhabricatorAuthDAO.php',
12081208
'PhabricatorAuthDisableController' => 'applications/auth/controller/config/PhabricatorAuthDisableController.php',
1209+
'PhabricatorAuthDowngradeSessionController' => 'applications/auth/controller/PhabricatorAuthDowngradeSessionController.php',
12091210
'PhabricatorAuthEditController' => 'applications/auth/controller/config/PhabricatorAuthEditController.php',
1211+
'PhabricatorAuthHighSecurityRequiredException' => 'applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php',
1212+
'PhabricatorAuthHighSecurityToken' => 'applications/auth/data/PhabricatorAuthHighSecurityToken.php',
12101213
'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php',
12111214
'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php',
12121215
'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php',
@@ -3947,7 +3950,9 @@
39473950
'PhabricatorAuthController' => 'PhabricatorController',
39483951
'PhabricatorAuthDAO' => 'PhabricatorLiskDAO',
39493952
'PhabricatorAuthDisableController' => 'PhabricatorAuthProviderConfigController',
3953+
'PhabricatorAuthDowngradeSessionController' => 'PhabricatorAuthController',
39503954
'PhabricatorAuthEditController' => 'PhabricatorAuthProviderConfigController',
3955+
'PhabricatorAuthHighSecurityRequiredException' => 'Exception',
39513956
'PhabricatorAuthLinkController' => 'PhabricatorAuthController',
39523957
'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController',
39533958
'PhabricatorAuthLoginController' => 'PhabricatorAuthController',

‎src/aphront/AphrontRequest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ final class AphrontRequest {
1818
const TYPE_WORKFLOW = '__wflow__';
1919
const TYPE_CONTINUE = '__continue__';
2020
const TYPE_PREVIEW = '__preview__';
21+
const TYPE_HISEC = '__hisec__';
2122

2223
private $host;
2324
private $path;
@@ -263,6 +264,7 @@ final public function validateCSRF() {
263264

264265
final public function isFormPost() {
265266
$post = $this->getExists(self::TYPE_FORM) &&
267+
!$this->getExists(self::TYPE_HISEC) &&
266268
$this->isHTTPPost();
267269

268270
if (!$post) {

‎src/aphront/configuration/AphrontDefaultApplicationConfiguration.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,49 @@ public function handleException(Exception $ex) {
123123
return $response;
124124
}
125125

126+
if ($ex instanceof PhabricatorAuthHighSecurityRequiredException) {
127+
128+
$form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm(
129+
$user,
130+
$request);
131+
132+
$dialog = id(new AphrontDialogView())
133+
->setUser($user)
134+
->setTitle(pht('Entering High Security'))
135+
->setShortTitle(pht('Security Checkpoint'))
136+
->setWidth(AphrontDialogView::WIDTH_FORM)
137+
->addHiddenInput(AphrontRequest::TYPE_HISEC, true)
138+
->setErrors(
139+
array(
140+
pht(
141+
'You are taking an action which requires you to enter '.
142+
'high security.'),
143+
))
144+
->appendParagraph(
145+
pht(
146+
'High security mode helps protect your account from security '.
147+
'threats, like session theft or someone messing with your stuff '.
148+
'while you\'re grabbing a coffee. To enter high security mode, '.
149+
'confirm your credentials.'))
150+
->appendChild($form->buildLayoutView())
151+
->appendParagraph(
152+
pht(
153+
'Your account will remain in high security mode for a short '.
154+
'period of time. When you are finished taking sensitive '.
155+
'actions, you should leave high security.'))
156+
->setSubmitURI($request->getPath())
157+
->addCancelButton($ex->getCancelURI())
158+
->addSubmitButton(pht('Enter High Security'));
159+
160+
foreach ($request->getPassthroughRequestParameters() as $key => $value) {
161+
$dialog->addHiddenInput($key, $value);
162+
}
163+
164+
$response = new AphrontDialogResponse();
165+
$response->setDialog($dialog);
166+
return $response;
167+
}
168+
126169
if ($ex instanceof PhabricatorPolicyException) {
127170

128171
if (!$user->isLoggedIn()) {

‎src/applications/auth/application/PhabricatorApplicationAuth.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public function getRoutes() {
8888
=> 'PhabricatorAuthConfirmLinkController',
8989
'session/terminate/(?P<id>[^/]+)/'
9090
=> 'PhabricatorAuthTerminateSessionController',
91+
'session/downgrade/'
92+
=> 'PhabricatorAuthDowngradeSessionController',
9193
),
9294

9395
'/oauth/(?P<provider>\w+)/login/'

‎src/applications/auth/controller/PhabricatorAuthDowngradeSessionController.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
final class PhabricatorAuthDowngradeSessionController
4+
extends PhabricatorAuthController {
5+
6+
public function processRequest() {
7+
$request = $this->getRequest();
8+
$viewer = $request->getUser();
9+
10+
$panel_uri = '/settings/panel/sessions/';
11+
12+
$session = $viewer->getSession();
13+
if ($session->getHighSecurityUntil() < time()) {
14+
return $this->newDialog()
15+
->setTitle(pht('Normal Security Restored'))
16+
->appendParagraph(
17+
pht('Your session is no longer in high security.'))
18+
->addCancelButton($panel_uri, pht('Continue'));
19+
}
20+
21+
if ($request->isFormPost()) {
22+
23+
queryfx(
24+
$session->establishConnection('w'),
25+
'UPDATE %T SET highSecurityUntil = NULL WHERE id = %d',
26+
$session->getTableName(),
27+
$session->getID());
28+
29+
return id(new AphrontRedirectResponse())
30+
->setURI($this->getApplicationURI('session/downgrade/'));
31+
}
32+
33+
return $this->newDialog()
34+
->setTitle(pht('Leaving High Security'))
35+
->appendParagraph(
36+
pht(
37+
'Leave high security and return your session to normal '.
38+
'security levels?'))
39+
->appendParagraph(
40+
pht(
41+
'If you leave high security, you will need to authenticate '.
42+
'again the next time you try to take a high security action.'))
43+
->appendParagraph(
44+
pht(
45+
'On the plus side, that purple notification bubble will '.
46+
'disappear.'))
47+
->addSubmitButton(pht('Leave High Security'))
48+
->addCancelButton($panel_uri, pht('Stay in High Security'));
49+
}
50+
51+
52+
}

‎src/applications/auth/data/PhabricatorAuthHighSecurityToken.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
3+
final class PhabricatorAuthHighSecurityToken {
4+
5+
}

‎src/applications/auth/engine/PhabricatorAuthSessionEngine.php‎

Lines changed: 131 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
<?php
22

3+
/**
4+
* @task hisec High Security Mode
5+
*/
36
final class PhabricatorAuthSessionEngine extends Phobject {
47

58
/**
@@ -78,48 +81,65 @@ public function loadUserForSession($session_type, $session_token) {
7881
$session_table = new PhabricatorAuthSession();
7982
$user_table = new PhabricatorUser();
8083
$conn_r = $session_table->establishConnection('r');
84+
$session_key = PhabricatorHash::digest($session_token);
8185

8286
// NOTE: We're being clever here because this happens on every page load,
83-
// and by joining we can save a query.
87+
// and by joining we can save a query. This might be getting too clever
88+
// for its own good, though...
8489

8590
$info = queryfx_one(
8691
$conn_r,
87-
'SELECT s.sessionExpires AS _sessionExpires, s.id AS _sessionID, u.*
92+
'SELECT
93+
s.id AS s_id,
94+
s.sessionExpires AS s_sessionExpires,
95+
s.sessionStart AS s_sessionStart,
96+
s.highSecurityUntil AS s_highSecurityUntil,
97+
u.*
8898
FROM %T u JOIN %T s ON u.phid = s.userPHID
8999
AND s.type = %s AND s.sessionKey = %s',
90100
$user_table->getTableName(),
91101
$session_table->getTableName(),
92102
$session_type,
93-
PhabricatorHash::digest($session_token));
103+
$session_key);
94104

95105
if (!$info) {
96106
return null;
97107
}
98108

99-
$expires = $info['_sessionExpires'];
100-
$id = $info['_sessionID'];
101-
unset($info['_sessionExpires']);
102-
unset($info['_sessionID']);
109+
$session_dict = array(
110+
'userPHID' => $info['phid'],
111+
'sessionKey' => $session_key,
112+
'type' => $session_type,
113+
);
114+
foreach ($info as $key => $value) {
115+
if (strncmp($key, 's_', 2) === 0) {
116+
unset($info[$key]);
117+
$session_dict[substr($key, 2)] = $value;
118+
}
119+
}
120+
$session = id(new PhabricatorAuthSession())->loadFromArray($session_dict);
103121

104122
$ttl = PhabricatorAuthSession::getSessionTypeTTL($session_type);
105123

106124
// If more than 20% of the time on this session has been used, refresh the
107125
// TTL back up to the full duration. The idea here is that sessions are
108126
// good forever if used regularly, but get GC'd when they fall out of use.
109127

110-
if (time() + (0.80 * $ttl) > $expires) {
128+
if (time() + (0.80 * $ttl) > $session->getSessionExpires()) {
111129
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
112130
$conn_w = $session_table->establishConnection('w');
113131
queryfx(
114132
$conn_w,
115133
'UPDATE %T SET sessionExpires = UNIX_TIMESTAMP() + %d WHERE id = %d',
116-
$session_table->getTableName(),
134+
$session->getTableName(),
117135
$ttl,
118-
$id);
136+
$session->getID());
119137
unset($unguarded);
120138
}
121139

122-
return $user_table->loadFromArray($info);
140+
$user = $user_table->loadFromArray($info);
141+
$user->attachSession($session);
142+
return $user;
123143
}
124144

125145

@@ -182,4 +202,104 @@ public function establishSession($session_type, $identity_phid) {
182202
return $session_key;
183203
}
184204

205+
206+
/**
207+
* Require high security, or prompt the user to enter high security.
208+
*
209+
* If the user's session is in high security, this method will return a
210+
* token. Otherwise, it will throw an exception which will eventually
211+
* be converted into a multi-factor authentication workflow.
212+
*
213+
* @param PhabricatorUser User whose session needs to be in high security.
214+
* @param AphrontReqeust Current request.
215+
* @param string URI to return the user to if they cancel.
216+
* @return PhabricatorAuthHighSecurityToken Security token.
217+
*/
218+
public function requireHighSecuritySession(
219+
PhabricatorUser $viewer,
220+
AphrontRequest $request,
221+
$cancel_uri) {
222+
223+
if (!$viewer->hasSession()) {
224+
throw new Exception(
225+
pht('Requiring a high-security session from a user with no session!'));
226+
}
227+
228+
$session = $viewer->getSession();
229+
230+
$token = $this->issueHighSecurityToken($session);
231+
if ($token) {
232+
return $token;
233+
}
234+
235+
if ($request->isHTTPPost()) {
236+
$request->validateCSRF();
237+
if ($request->getExists(AphrontRequest::TYPE_HISEC)) {
238+
239+
// TODO: Actually verify that the user provided some multi-factor
240+
// auth credentials here. For now, we just let you enter high
241+
// security.
242+
243+
$until = time() + phutil_units('15 minutes in seconds');
244+
$session->setHighSecurityUntil($until);
245+
246+
queryfx(
247+
$session->establishConnection('w'),
248+
'UPDATE %T SET highSecurityUntil = %d WHERE id = %d',
249+
$session->getTableName(),
250+
$until,
251+
$session->getID());
252+
}
253+
}
254+
255+
$token = $this->issueHighSecurityToken($session);
256+
if ($token) {
257+
return $token;
258+
}
259+
260+
throw id(new PhabricatorAuthHighSecurityRequiredException())
261+
->setCancelURI($cancel_uri);
262+
}
263+
264+
265+
/**
266+
* Issue a high security token for a session, if authorized.
267+
*
268+
* @param PhabricatorAuthSession Session to issue a token for.
269+
* @return PhabricatorAuthHighSecurityToken|null Token, if authorized.
270+
*/
271+
private function issueHighSecurityToken(PhabricatorAuthSession $session) {
272+
$until = $session->getHighSecurityUntil();
273+
if ($until > time()) {
274+
return new PhabricatorAuthHighSecurityToken();
275+
}
276+
return null;
277+
}
278+
279+
280+
/**
281+
* Render a form for providing relevant multi-factor credentials.
282+
*
283+
* @param PhabricatorUser Viewing user.
284+
* @param AphrontRequest Current request.
285+
* @return AphrontFormView Renderable form.
286+
*/
287+
public function renderHighSecurityForm(
288+
PhabricatorUser $viewer,
289+
AphrontRequest $request) {
290+
291+
// TODO: This is stubbed.
292+
293+
$form = id(new AphrontFormView())
294+
->setUser($viewer)
295+
->appendRemarkupInstructions('')
296+
->appendChild(
297+
id(new AphrontFormTextControl())
298+
->setLabel(pht('Secret Stuff')))
299+
->appendRemarkupInstructions('');
300+
301+
return $form;
302+
}
303+
304+
185305
}

‎src/applications/auth/exception/PhabricatorAuthHighSecurityRequiredException.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
final class PhabricatorAuthHighSecurityRequiredException extends Exception {
4+
5+
private $cancelURI;
6+
7+
public function setCancelURI($cancel_uri) {
8+
$this->cancelURI = $cancel_uri;
9+
return $this;
10+
}
11+
12+
public function getCancelURI() {
13+
return $this->cancelURI;
14+
}
15+
16+
}

‎src/applications/auth/storage/PhabricatorAuthSession.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ final class PhabricatorAuthSession extends PhabricatorAuthDAO
1111
protected $sessionKey;
1212
protected $sessionStart;
1313
protected $sessionExpires;
14+
protected $highSecurityUntil;
1415

1516
private $identityObject = self::ATTACHABLE;
1617

0 commit comments

Comments
 (0)