From 0fcff782533aa446dedf6de2b074852a03c40d9a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 15 Jan 2019 07:57:32 -0800 Subject: [PATCH] Convert user MFA factors to point at configurable "MFA Providers", not raw "MFA Factors" Summary: Ref T13222. Users configure "Factor Configs", which say "I have an entry on my phone for TOTP secret key XYZ". Currently, these point at raw implementations -- always "TOTP" in practice. To support configuring available MFA types (like "no MFA") and adding MFA types that need some options set (like "Duo", which needs API keys), bind "Factor Configs" to a "Factor Provider" instead. In the future, several "Factors" will be available (TOTP, SMS, Duo, Postal Mail, ...). Administrators configure zero or more "MFA Providers" they want to use (e.g., "Duo" + here's my API key). Then users can add configs for these providers (e.g., "here's my Duo account"). Upshot: - Factor: a PHP subclass, implements the technical details of a type of MFA factor (TOTP, SMS, Duo, etc). - FactorProvider: a storage object, owned by administrators, configuration of a Factor that says "this should be available on this install", plus provides API keys, a human-readable name, etc. - FactorConfig: a storage object, owned by a user, says "I have a factor for provider X on my phone/whatever with secret key Q / my duo account is X / my address is Y". Couple of things not covered here: - Statuses for providers ("Disabled", "Deprecated") don't do anything yet, but you can't edit them anyway. - Some `bin/auth` tools need to be updated. - When no providers are configured, the MFA panel should probably vanish. - Documentation. Test Plan: - Ran migration with providers, saw configs point at the first provider. - Ran migration without providers, saw a provider created and configs pointed at it. - Added/removed factors and providers. Passed MFA gates. Spot-checked database for general sanity. Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D19975 --- .../autopatches/20190115.mfa.01.provider.sql | 2 + .../autopatches/20190115.mfa.02.migrate.php | 72 ++++++++ .../autopatches/20190115.mfa.03.factorkey.sql | 2 + src/__phutil_library_map__.php | 7 +- .../engine/PhabricatorAuthSessionEngine.php | 27 ++- .../auth/factor/PhabricatorAuthFactor.php | 4 +- .../auth/factor/PhabricatorTOTPAuthFactor.php | 1 + .../PhabricatorAuthFactorConfigQuery.php | 88 +++++++++ .../PhabricatorAuthFactorProviderQuery.php | 27 +++ .../storage/PhabricatorAuthFactorConfig.php | 51 ++++-- .../storage/PhabricatorAuthFactorProvider.php | 23 +++ .../PhabricatorMultiFactorSettingsPanel.php | 171 +++++++++--------- 12 files changed, 364 insertions(+), 111 deletions(-) create mode 100644 resources/sql/autopatches/20190115.mfa.01.provider.sql create mode 100644 resources/sql/autopatches/20190115.mfa.02.migrate.php create mode 100644 resources/sql/autopatches/20190115.mfa.03.factorkey.sql create mode 100644 src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php diff --git a/resources/sql/autopatches/20190115.mfa.01.provider.sql b/resources/sql/autopatches/20190115.mfa.01.provider.sql new file mode 100644 index 0000000000..52e818f8d8 --- /dev/null +++ b/resources/sql/autopatches/20190115.mfa.01.provider.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_auth.auth_factorconfig + ADD factorProviderPHID VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20190115.mfa.02.migrate.php b/resources/sql/autopatches/20190115.mfa.02.migrate.php new file mode 100644 index 0000000000..95a60789c3 --- /dev/null +++ b/resources/sql/autopatches/20190115.mfa.02.migrate.php @@ -0,0 +1,72 @@ +establishConnection('w'); + +$provider_table = new PhabricatorAuthFactorProvider(); +$provider_phid = null; +$iterator = new LiskRawMigrationIterator($conn, $table->getTableName()); +$totp_key = 'totp'; +foreach ($iterator as $row) { + + // This wasn't a TOTP factor, so skip it. + if ($row['factorKey'] !== $totp_key) { + continue; + } + + // This factor already has an associated provider. + if (strlen($row['factorProviderPHID'])) { + continue; + } + + // Find (or create) a suitable TOTP provider. Note that we can't "save()" + // an object or this migration will break if the object ever gets new + // columns; just INSERT the raw fields instead. + + if ($provider_phid === null) { + $provider_row = queryfx_one( + $conn, + 'SELECT phid FROM %R WHERE providerFactorKey = %s LIMIT 1', + $provider_table, + $totp_key); + + if ($provider_row) { + $provider_phid = $provider_row['phid']; + } else { + $provider_phid = $provider_table->generatePHID(); + queryfx( + $conn, + 'INSERT INTO %R + (phid, providerFactorKey, name, status, properties, + dateCreated, dateModified) + VALUES (%s, %s, %s, %s, %s, %d, %d)', + $provider_table, + $provider_phid, + $totp_key, + '', + 'active', + '{}', + PhabricatorTime::getNow(), + PhabricatorTime::getNow()); + } + } + + queryfx( + $conn, + 'UPDATE %R SET factorProviderPHID = %s WHERE id = %d', + $table, + $provider_phid, + $row['id']); +} diff --git a/resources/sql/autopatches/20190115.mfa.03.factorkey.sql b/resources/sql/autopatches/20190115.mfa.03.factorkey.sql new file mode 100644 index 0000000000..619787a838 --- /dev/null +++ b/resources/sql/autopatches/20190115.mfa.03.factorkey.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_auth.auth_factorconfig + DROP factorKey; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8ee28d39a7..0800434676 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2207,6 +2207,7 @@ phutil_register_library_map(array( 'PhabricatorAuthEditController' => 'applications/auth/controller/config/PhabricatorAuthEditController.php', 'PhabricatorAuthFactor' => 'applications/auth/factor/PhabricatorAuthFactor.php', 'PhabricatorAuthFactorConfig' => 'applications/auth/storage/PhabricatorAuthFactorConfig.php', + 'PhabricatorAuthFactorConfigQuery' => 'applications/auth/query/PhabricatorAuthFactorConfigQuery.php', 'PhabricatorAuthFactorProvider' => 'applications/auth/storage/PhabricatorAuthFactorProvider.php', 'PhabricatorAuthFactorProviderController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderController.php', 'PhabricatorAuthFactorProviderEditController' => 'applications/auth/controller/mfa/PhabricatorAuthFactorProviderEditController.php', @@ -7888,7 +7889,11 @@ phutil_register_library_map(array( 'PhabricatorAuthDowngradeSessionController' => 'PhabricatorAuthController', 'PhabricatorAuthEditController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthFactor' => 'Phobject', - 'PhabricatorAuthFactorConfig' => 'PhabricatorAuthDAO', + 'PhabricatorAuthFactorConfig' => array( + 'PhabricatorAuthDAO', + 'PhabricatorPolicyInterface', + ), + 'PhabricatorAuthFactorConfigQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorAuthFactorProvider' => array( 'PhabricatorAuthDAO', 'PhabricatorApplicationTransactionInterface', diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php index 66a3e9e8fb..cef2323209 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngine.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngine.php @@ -462,10 +462,14 @@ final class PhabricatorAuthSessionEngine extends Phobject { return $token; } - // Load the multi-factor auth sources attached to this account. - $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( - 'userPHID = %s', - $viewer->getPHID()); + // Load the multi-factor auth sources attached to this account. Note that + // we order factors from oldest to newest, which is not the default query + // ordering but makes the greatest sense in context. + $factors = id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($viewer->getPHID())) + ->setOrderVector(array('-id')) + ->execute(); // If the account has no associated multi-factor auth, just issue a token // without putting the session into high security mode. This is generally @@ -516,7 +520,8 @@ final class PhabricatorAuthSessionEngine extends Phobject { foreach ($factors as $factor) { $factor_phid = $factor->getPHID(); $issued_challenges = idx($challenge_map, $factor_phid, array()); - $impl = $factor->requireImplementation(); + $provider = $factor->getFactorProvider(); + $impl = $provider->getFactor(); $new_challenges = $impl->getNewIssuedChallenges( $factor, @@ -552,7 +557,9 @@ final class PhabricatorAuthSessionEngine extends Phobject { // Limit factor verification rates to prevent brute force attacks. $any_attempt = false; foreach ($factors as $factor) { - $impl = $factor->requireImplementation(); + $provider = $factor->getFactorProvider(); + $impl = $provider->getFactor(); + if ($impl->getRequestHasChallengeResponse($factor, $request)) { $any_attempt = true; break; @@ -577,7 +584,8 @@ final class PhabricatorAuthSessionEngine extends Phobject { $issued_challenges = idx($challenge_map, $factor_phid, array()); - $impl = $factor->requireImplementation(); + $provider = $factor->getFactorProvider(); + $impl = $provider->getFactor(); $validation_result = $impl->getResultFromChallengeResponse( $factor, @@ -716,7 +724,10 @@ final class PhabricatorAuthSessionEngine extends Phobject { foreach ($factors as $factor) { $result = $validation_results[$factor->getPHID()]; - $factor->requireImplementation()->renderValidateFactorForm( + $provider = $factor->getFactorProvider(); + $impl = $provider->getFactor(); + + $impl->renderValidateFactorForm( $factor, $form, $viewer, diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php index ef6ab5d04b..f797ce3a15 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -7,6 +7,7 @@ abstract class PhabricatorAuthFactor extends Phobject { abstract public function getFactorCreateHelp(); abstract public function getFactorDescription(); abstract public function processAddFactorForm( + PhabricatorAuthFactorProvider $provider, AphrontFormView $form, AphrontRequest $request, PhabricatorUser $user); @@ -32,8 +33,7 @@ abstract class PhabricatorAuthFactor extends Phobject { protected function newConfigForUser(PhabricatorUser $user) { return id(new PhabricatorAuthFactorConfig()) - ->setUserPHID($user->getPHID()) - ->setFactorKey($this->getFactorKey()); + ->setUserPHID($user->getPHID()); } protected function newResult() { diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 3632ca5c45..7f48455f4d 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -26,6 +26,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { } public function processAddFactorForm( + PhabricatorAuthFactorProvider $provider, AphrontFormView $form, AphrontRequest $request, PhabricatorUser $user) { diff --git a/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php b/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php new file mode 100644 index 0000000000..f40c12e48a --- /dev/null +++ b/src/applications/auth/query/PhabricatorAuthFactorConfigQuery.php @@ -0,0 +1,88 @@ +ids = $ids; + return $this; + } + + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function withUserPHIDs(array $user_phids) { + $this->userPHIDs = $user_phids; + return $this; + } + + public function newResultObject() { + return new PhabricatorAuthFactorConfig(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->phids !== null) { + $where[] = qsprintf( + $conn, + 'phid IN (%Ls)', + $this->phids); + } + + if ($this->userPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'userPHID IN (%Ls)', + $this->userPHIDs); + } + + return $where; + } + + protected function willFilterPage(array $configs) { + $provider_phids = mpull($configs, 'getFactorProviderPHID'); + + $providers = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($provider_phids) + ->execute(); + $providers = mpull($providers, null, 'getPHID'); + + foreach ($configs as $key => $config) { + $provider = idx($providers, $config->getFactorProviderPHID()); + + if (!$provider) { + unset($configs[$key]); + $this->didRejectResult($config); + continue; + } + + $config->attachFactorProvider($provider); + } + + return $configs; + } + + public function getQueryApplicationClass() { + return 'PhabricatorAuthApplication'; + } + +} diff --git a/src/applications/auth/query/PhabricatorAuthFactorProviderQuery.php b/src/applications/auth/query/PhabricatorAuthFactorProviderQuery.php index f4ce60773b..57b554885c 100644 --- a/src/applications/auth/query/PhabricatorAuthFactorProviderQuery.php +++ b/src/applications/auth/query/PhabricatorAuthFactorProviderQuery.php @@ -5,6 +5,8 @@ final class PhabricatorAuthFactorProviderQuery private $ids; private $phids; + private $statuses; + private $providerFactorKeys; public function withIDs(array $ids) { $this->ids = $ids; @@ -15,6 +17,17 @@ final class PhabricatorAuthFactorProviderQuery $this->phids = $phids; return $this; } + + public function withProviderFactorKeys(array $keys) { + $this->providerFactorKeys = $keys; + return $this; + } + + public function withStatuses(array $statuses) { + $this->statuses = $statuses; + return $this; + } + public function newResultObject() { return new PhabricatorAuthFactorProvider(); } @@ -40,6 +53,20 @@ final class PhabricatorAuthFactorProviderQuery $this->phids); } + if ($this->statuses !== null) { + $where[] = qsprintf( + $conn, + 'status IN (%Ls)', + $this->statuses); + } + + if ($this->providerFactorKeys !== null) { + $where[] = qsprintf( + $conn, + 'providerFactorKey IN (%Ls)', + $this->providerFactorKeys); + } + return $where; } diff --git a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php index 2bed939402..6b04b9eeab 100644 --- a/src/applications/auth/storage/PhabricatorAuthFactorConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorConfig.php @@ -1,14 +1,17 @@ true, self::CONFIG_COLUMN_SCHEMA => array( - 'factorKey' => 'text64', 'factorName' => 'text', 'factorSecret' => 'text', ), @@ -29,26 +31,18 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO { ) + parent::getConfiguration(); } - public function generatePHID() { - return PhabricatorPHID::generateNewPHID( - PhabricatorAuthAuthFactorPHIDType::TYPECONST); + public function getPHIDType() { + return PhabricatorAuthAuthFactorPHIDType::TYPECONST; } - public function getImplementation() { - return idx(PhabricatorAuthFactor::getAllFactors(), $this->getFactorKey()); + public function attachFactorProvider( + PhabricatorAuthFactorProvider $provider) { + $this->factorProvider = $provider; + return $this; } - public function requireImplementation() { - $impl = $this->getImplementation(); - if (!$impl) { - throw new Exception( - pht( - 'Attempting to operate on multi-factor auth which has no '. - 'corresponding implementation (factor key is "%s").', - $this->getFactorKey())); - } - - return $impl; + public function getFactorProvider() { + return $this->assertAttached($this->factorProvider); } public function setSessionEngine(PhabricatorAuthSessionEngine $engine) { @@ -64,4 +58,23 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO { return $this->sessionEngine; } + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } + + public function getPolicy($capability) { + return $this->getUserPHID(); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + } diff --git a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php index 13140395df..e61740b4d7 100644 --- a/src/applications/auth/storage/PhabricatorAuthFactorProvider.php +++ b/src/applications/auth/storage/PhabricatorAuthFactorProvider.php @@ -78,6 +78,29 @@ final class PhabricatorAuthFactorProvider return $this->getFactor()->getFactorName(); } + public function newIconView() { + return $this->getFactor()->newIconView(); + } + + public function getDisplayDescription() { + return $this->getFactor()->getFactorDescription(); + } + + public function processAddFactorForm( + AphrontFormView $form, + AphrontRequest $request, + PhabricatorUser $user) { + + $factor = $this->getFactor(); + + $config = $factor->processAddFactorForm($this, $form, $request, $user); + if ($config) { + $config->setFactorProviderPHID($this->getPHID()); + } + + return $config; + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php index 5fada0bbed..06f1547afa 100644 --- a/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorMultiFactorSettingsPanel.php @@ -16,7 +16,7 @@ final class PhabricatorMultiFactorSettingsPanel } public function processRequest(AphrontRequest $request) { - if ($request->getExists('new')) { + if ($request->getExists('new') || $request->getExists('providerPHID')) { return $this->processNew($request); } @@ -31,22 +31,18 @@ final class PhabricatorMultiFactorSettingsPanel $user = $this->getUser(); $viewer = $request->getUser(); - $factors = id(new PhabricatorAuthFactorConfig())->loadAllWhere( - 'userPHID = %s', - $user->getPHID()); + $factors = id(new PhabricatorAuthFactorConfigQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($user->getPHID())) + ->setOrderVector(array('-id')) + ->execute(); $rows = array(); $rowc = array(); $highlight_id = $request->getInt('id'); foreach ($factors as $factor) { - - $impl = $factor->getImplementation(); - if ($impl) { - $type = $impl->getFactorName(); - } else { - $type = $factor->getFactorKey(); - } + $provider = $factor->getFactorProvider(); if ($factor->getID() == $highlight_id) { $rowc[] = 'highlighted'; @@ -62,7 +58,7 @@ final class PhabricatorMultiFactorSettingsPanel 'sigil' => 'workflow', ), $factor->getFactorName()), - $type, + $provider->getDisplayName(), phabricator_datetime($factor->getDateCreated(), $viewer), javelin_tag( 'a', @@ -128,88 +124,101 @@ final class PhabricatorMultiFactorSettingsPanel $viewer = $request->getUser(); $user = $this->getUser(); + $cancel_uri = $this->getPanelURI(); + + // Check that we have providers before we send the user through the MFA + // gate, so you don't authenticate and then immediately get roadblocked. + $providers = id(new PhabricatorAuthFactorProviderQuery()) + ->setViewer($viewer) + ->withStatuses(array(PhabricatorAuthFactorProvider::STATUS_ACTIVE)) + ->execute(); + if (!$providers) { + return $this->newDialog() + ->setTitle(pht('No MFA Providers')) + ->appendParagraph( + pht( + 'There are no active MFA providers. At least one active provider '. + 'must be available to add new MFA factors.')) + ->addCancelButton($cancel_uri); + } + $providers = mpull($providers, null, 'getPHID'); + $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( $viewer, $request, - $this->getPanelURI()); + $cancel_uri); - $factors = PhabricatorAuthFactor::getAllFactors(); + $selected_phid = $request->getStr('providerPHID'); + if (empty($providers[$selected_phid])) { + $selected_provider = null; + } else { + $selected_provider = $providers[$selected_phid]; + } + + if (!$selected_provider) { + $menu = id(new PHUIObjectItemListView()) + ->setViewer($viewer) + ->setBig(true) + ->setFlush(true); + + foreach ($providers as $provider_phid => $provider) { + $provider_uri = id(new PhutilURI($this->getPanelURI())) + ->setQueryParam('providerPHID', $provider_phid); + + $item = id(new PHUIObjectItemView()) + ->setHeader($provider->getDisplayName()) + ->setHref($provider_uri) + ->setClickable(true) + ->setImageIcon($provider->newIconView()) + ->addAttribute($provider->getDisplayDescription()); + + $menu->addItem($item); + } + + return $this->newDialog() + ->setTitle(pht('Choose Factor Type')) + ->appendChild($menu) + ->addCancelButton($cancel_uri); + } $form = id(new AphrontFormView()) - ->setUser($viewer); + ->setViewer($viewer); - $type = $request->getStr('type'); - if (empty($factors[$type]) || !$request->isFormPost()) { - $factor = null; - } else { - $factor = $factors[$type]; + $config = $selected_provider->processAddFactorForm( + $form, + $request, + $user); + + if ($config) { + $config->save(); + + $log = PhabricatorUserLog::initializeNewLog( + $viewer, + $user->getPHID(), + PhabricatorUserLog::ACTION_MULTI_ADD); + $log->save(); + + $user->updateMultiFactorEnrollment(); + + // Terminate other sessions so they must log in and survive the + // multi-factor auth check. + + id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( + $user, + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); + + return id(new AphrontRedirectResponse()) + ->setURI($this->getPanelURI('?id='.$config->getID())); } - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) - ->addHiddenInput('new', true); - - if ($factor === null) { - $choice_control = id(new AphrontFormRadioButtonControl()) - ->setName('type') - ->setValue(key($factors)); - - foreach ($factors as $available_factor) { - $choice_control->addButton( - $available_factor->getFactorKey(), - $available_factor->getFactorName(), - $available_factor->getFactorDescription()); - } - - $dialog->appendParagraph( - pht( - 'Adding an additional authentication factor improves the security '. - 'of your account. Choose the type of factor to add:')); - - $form - ->appendChild($choice_control); - - } else { - $dialog->addHiddenInput('type', $type); - - $config = $factor->processAddFactorForm( - $form, - $request, - $user); - - if ($config) { - $config->save(); - - $log = PhabricatorUserLog::initializeNewLog( - $viewer, - $user->getPHID(), - PhabricatorUserLog::ACTION_MULTI_ADD); - $log->save(); - - $user->updateMultiFactorEnrollment(); - - // Terminate other sessions so they must log in and survive the - // multi-factor auth check. - - id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( - $user, - new PhutilOpaqueEnvelope( - $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); - - return id(new AphrontRedirectResponse()) - ->setURI($this->getPanelURI('?id='.$config->getID())); - } - } - - $dialog + return $this->newDialog() + ->addHiddenInput('providerPHID', $selected_provider->getPHID()) ->setWidth(AphrontDialogView::WIDTH_FORM) ->setTitle(pht('Add Authentication Factor')) ->appendChild($form->buildLayoutView()) ->addSubmitButton(pht('Continue')) - ->addCancelButton($this->getPanelURI()); - - return id(new AphrontDialogResponse()) - ->setDialog($dialog); + ->addCancelButton($cancel_uri); } private function processEdit(AphrontRequest $request) {