Page MenuHomePhabricator
Files F14411657

D19889.id47518.diff
No OneTemporary
Actions

D19889.id47518.diff
View Options

diff --git a/resources/sql/autopatches/20181214.auth.01.workflowkey.sql b/resources/sql/autopatches/20181214.auth.01.workflowkey.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20181214.auth.01.workflowkey.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_auth.auth_challenge
+ ADD workflowKey VARCHAR(255) NOT NULL COLLATE {$COLLATE_TEXT};
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
@@ -46,6 +46,26 @@
const ONETIME_USERNAME = 'rename';
+ private $workflowKey;
+
+ public function setWorkflowKey($workflow_key) {
+ $this->workflowKey = $workflow_key;
+ return $this;
+ }
+
+ public function getWorkflowKey() {
+
+ // TODO: A workflow key should become required in order to issue an MFA
+ // challenge, but allow things to keep working for now until we can update
+ // callsites.
+ if ($this->workflowKey === null) {
+ return 'legacy';
+ }
+
+ return $this->workflowKey;
+ }
+
+
/**
* Get the session kind (e.g., anonymous, user, external account) from a
* session token. Returns a `KIND_` constant.
@@ -473,6 +493,10 @@
return $this->issueHighSecurityToken($session, true);
}
+ foreach ($factors as $factor) {
+ $factor->setSessionEngine($this);
+ }
+
// Check for a rate limit without awarding points, so the user doesn't
// get partway through the workflow only to get blocked.
PhabricatorSystemActionEngine::willTakeAction(
diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorAuthFactor.php
@@ -43,10 +43,13 @@
PhabricatorAuthFactorConfig $config,
PhabricatorUser $viewer) {
+ $engine = $config->getSessionEngine();
+
return id(new PhabricatorAuthChallenge())
->setUserPHID($viewer->getPHID())
->setSessionPHID($viewer->getSession()->getPHID())
- ->setFactorPHID($config->getPHID());
+ ->setFactorPHID($config->getPHID())
+ ->setWorkflowKey($engine->getWorkflowKey());
}
final public function getNewIssuedChallenges(
diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
--- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
+++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
@@ -225,6 +225,9 @@
$session_phid = $viewer->getSession()->getPHID();
+ $engine = $config->getSessionEngine();
+ $workflow_key = $engine->getWorkflowKey();
+
foreach ($challenges as $challenge) {
$challenge_timestep = (int)$challenge->getChallengeKey();
@@ -249,6 +252,17 @@
'again.',
new PhutilNumber($wait_duration)));
}
+
+ if ($challenge->getWorkflowKey() !== $workflow_key) {
+ return $this->newResult()
+ ->setIsWait(true)
+ ->setErrorMessage(
+ pht(
+ 'This factor recently issued a challenge for a different '.
+ 'workflow. Wait %s seconds for the code to cycle, then try '.
+ 'again.',
+ new PhutilNumber($wait_duration)));
+ }
}
return null;
diff --git a/src/applications/auth/storage/PhabricatorAuthChallenge.php b/src/applications/auth/storage/PhabricatorAuthChallenge.php
--- a/src/applications/auth/storage/PhabricatorAuthChallenge.php
+++ b/src/applications/auth/storage/PhabricatorAuthChallenge.php
@@ -7,6 +7,7 @@
protected $userPHID;
protected $factorPHID;
protected $sessionPHID;
+ protected $workflowKey;
protected $challengeKey;
protected $challengeTTL;
protected $properties = array();
@@ -20,6 +21,7 @@
self::CONFIG_COLUMN_SCHEMA => array(
'challengeKey' => 'text255',
'challengeTTL' => 'epoch',
+ 'workflowKey' => 'text255',
),
self::CONFIG_KEY_SCHEMA => array(
'key_issued' => array(
diff --git a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php
--- a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php
+++ b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php
@@ -8,6 +8,8 @@
protected $factorSecret;
protected $properties = array();
+ private $sessionEngine;
+
protected function getConfiguration() {
return array(
self::CONFIG_SERIALIZATION => array(
@@ -49,4 +51,17 @@
return $impl;
}
+ public function setSessionEngine(PhabricatorAuthSessionEngine $engine) {
+ $this->sessionEngine = $engine;
+ return $this;
+ }
+
+ public function getSessionEngine() {
+ if (!$this->sessionEngine) {
+ throw new PhutilInvalidStateException('setSessionEngine');
+ }
+
+ return $this->sessionEngine;
+ }
+
}
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
@@ -154,7 +154,12 @@
// Require two-factor auth to sign legal documents.
if ($viewer->isLoggedIn()) {
+ $workflow_key = sprintf(
+ 'legalpad.sign(%s)',
+ $document->getPHID());
+
$hisec_token = id(new PhabricatorAuthSessionEngine())
+ ->setWorkflowKey($workflow_key)
->requireHighSecurityToken(
$viewer,
$request,

File Metadata

Mime Type
text/plain
Expires
Wed, Dec 25, 12:00 PM (1 h, 7 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6926121
Default Alt Text
D19889.id47518.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility