diff --git a/src/applications/auth/adapter/PhutilFacebookAuthAdapter.php b/src/applications/auth/adapter/PhutilFacebookAuthAdapter.php --- a/src/applications/auth/adapter/PhutilFacebookAuthAdapter.php +++ b/src/applications/auth/adapter/PhutilFacebookAuthAdapter.php @@ -5,13 +5,6 @@ */ final class PhutilFacebookAuthAdapter extends PhutilOAuthAuthAdapter { - private $requireSecureBrowsing; - - public function setRequireSecureBrowsing($require_secure_browsing) { - $this->requireSecureBrowsing = $require_secure_browsing; - return $this; - } - public function getAdapterType() { return 'facebook'; } @@ -61,10 +54,6 @@ return $this->getOAuthAccountData('name'); } - public function getAccountSecuritySettings() { - return $this->getOAuthAccountData('security_settings'); - } - protected function getAuthenticateBaseURI() { return 'https://www.facebook.com/dialog/oauth'; } @@ -79,7 +68,6 @@ 'name', 'email', 'link', - 'security_settings', 'picture', ); @@ -97,17 +85,6 @@ $ex); } - if ($this->requireSecureBrowsing) { - if (empty($data['security_settings']['secure_browsing']['enabled'])) { - throw new Exception( - pht( - 'This Phabricator install requires you to enable Secure Browsing '. - 'on your Facebook account in order to use it to log in to '. - 'Phabricator. For more information, see %s', - 'https://www.facebook.com/help/156201551113407/')); - } - } - return $data; } diff --git a/src/applications/auth/provider/PhabricatorFacebookAuthProvider.php b/src/applications/auth/provider/PhabricatorFacebookAuthProvider.php --- a/src/applications/auth/provider/PhabricatorFacebookAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorFacebookAuthProvider.php @@ -3,14 +3,35 @@ final class PhabricatorFacebookAuthProvider extends PhabricatorOAuth2AuthProvider { - const KEY_REQUIRE_SECURE = 'oauth:facebook:require-secure'; - public function getProviderName() { return pht('Facebook'); } protected function getProviderConfigurationHelp() { $uri = PhabricatorEnv::getProductionURI($this->getLoginURI()); + + $domain = id(new PhutilURI($uri))->getDomain(); + + $table = array( + 'Client OAuth Login' => pht('No'), + 'Web OAuth Login' => pht('Yes'), + 'Enforce HTTPS' => pht('Yes'), + 'Force Web OAuth Reauthentication' => pht('Yes (Optional)'), + 'Embedded Browser OAuth Login' => pht('No'), + 'Use Strict Mode for Redirect URIs' => pht('Yes'), + 'Login from Devices' => pht('No'), + 'Valid OAuth Redirect URIs' => '`'.(string)$uri.'`', + 'App Domains' => '`'.$domain.'`', + ); + + $rows = array(); + foreach ($table as $k => $v) { + $rows[] = sprintf('| %s | %s |', $k, $v); + $rows[] = sprintf('|----| |'); + } + $rows = implode("\n", $rows); + + return pht( 'To configure Facebook OAuth, create a new Facebook Application here:'. "\n\n". @@ -18,29 +39,15 @@ "\n\n". 'You should use these settings in your application:'. "\n\n". - " - **Site URL**: Set this to `%s`\n". - " - **Valid OAuth redirect URIs**: You should also set this to `%s`\n". - " - **Client OAuth Login**: Set this to **OFF**.\n". - " - **Embedded browser OAuth Login**: Set this to **OFF**.\n". + "%s\n". "\n\n". - "Some of these settings may be in the **Advanced** tab.\n\n". "After creating your new application, copy the **App ID** and ". "**App Secret** to the fields above.", - (string)$uri, - (string)$uri); - } - - public function getDefaultProviderConfig() { - return parent::getDefaultProviderConfig() - ->setProperty(self::KEY_REQUIRE_SECURE, 1); + $rows); } protected function newOAuthAdapter() { - $require_secure = $this->getProviderConfig()->getProperty( - self::KEY_REQUIRE_SECURE); - - return id(new PhutilFacebookAuthAdapter()) - ->setRequireSecureBrowsing($require_secure); + return new PhutilFacebookAuthAdapter(); } protected function getLoginIcon() { @@ -55,71 +62,4 @@ ); } - public function readFormValuesFromProvider() { - $require_secure = $this->getProviderConfig()->getProperty( - self::KEY_REQUIRE_SECURE); - - return parent::readFormValuesFromProvider() + array( - self::KEY_REQUIRE_SECURE => $require_secure, - ); - } - - public function readFormValuesFromRequest(AphrontRequest $request) { - return parent::readFormValuesFromRequest($request) + array( - self::KEY_REQUIRE_SECURE => $request->getBool(self::KEY_REQUIRE_SECURE), - ); - } - - public function extendEditForm( - AphrontRequest $request, - AphrontFormView $form, - array $values, - array $issues) { - - parent::extendEditForm($request, $form, $values, $issues); - - $key_require = self::KEY_REQUIRE_SECURE; - $v_require = idx($values, $key_require); - - $form - ->appendChild( - id(new AphrontFormCheckboxControl()) - ->addCheckbox( - $key_require, - $v_require, - pht( - "%s ". - "Require users to enable 'secure browsing' on Facebook in order ". - "to use Facebook to authenticate with Phabricator. This ". - "improves security by preventing an attacker from capturing ". - "an insecure Facebook session and escalating it into a ". - "Phabricator session. Enabling it is recommended.", - phutil_tag('strong', array(), pht('Require Secure Browsing:'))))); - } - - public function renderConfigPropertyTransactionTitle( - PhabricatorAuthProviderConfigTransaction $xaction) { - - $author_phid = $xaction->getAuthorPHID(); - $old = $xaction->getOldValue(); - $new = $xaction->getNewValue(); - $key = $xaction->getMetadataValue( - PhabricatorAuthProviderConfigTransaction::PROPERTY_KEY); - - switch ($key) { - case self::KEY_REQUIRE_SECURE: - if ($new) { - return pht( - '%s turned "Require Secure Browsing" on.', - $xaction->renderHandleLink($author_phid)); - } else { - return pht( - '%s turned "Require Secure Browsing" off.', - $xaction->renderHandleLink($author_phid)); - } - } - - return parent::renderConfigPropertyTransactionTitle($xaction); - } - }