From 8c8ab25fa1aa67cde705965bdf8399b393dca8b9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 18 Jun 2013 15:52:01 -0700 Subject: [PATCH] Restore/respect "require secure browsing" for Facebook (phabricator) Summary: Ref T1536. Because Facebook publishes data from Phabricator to user profiles and that data is sensitive, it wants to require secure browsing to be enabled in order to login. Respect the existing option, and support it in the UI. The UI part isn't reachable yet. Test Plan: {F46723} Reviewers: chad, btrahan Reviewed By: chad CC: arice, wez, aran Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6224 --- .../PhabricatorAuthProviderOAuthFacebook.php | 78 ++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php index d30215df66..607e5aff9d 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php @@ -3,12 +3,16 @@ final class PhabricatorAuthProviderOAuthFacebook extends PhabricatorAuthProviderOAuth { + const KEY_REQUIRE_SECURE = 'oauth:facebook:require-secure'; + public function getProviderName() { return pht('Facebook'); } protected function newOAuthAdapter() { - return new PhutilAuthAdapterOAuthFacebook(); + $secure_only = PhabricatorEnv::getEnvConfig('facebook.require-https-auth'); + return id(new PhutilAuthAdapterOAuthFacebook()) + ->setRequireSecureBrowsing($secure_only); } protected function getLoginIcon() { @@ -48,4 +52,76 @@ final class PhabricatorAuthProviderOAuthFacebook return !PhabricatorEnv::getEnvConfig('facebook.auth-permanent'); } + public function readFormValuesFromProvider() { + $require_secure = PhabricatorEnv::getEnvConfig( + 'facebook.require-https-auth'); + + // TODO: When we read from config, default this on for new providers. + + 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.", + hsprintf( + '%s', + 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); + } + + }