From 24544b1a2f24f9d09798c346930fc4644d963bad Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 23 Jan 2014 14:03:44 -0800 Subject: [PATCH] Straighten out absolute/relative URIs in login providers Summary: Ref T4339. Login providers use absolute URIs, but the ones that rely on local form submits should not, because we want to include CSRF tokens where applicable. Instead, make the default be relative URIs and turn them into absolute ones for the callback proivders. Test Plan: Clicked, like, every login button. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4339 Differential Revision: https://secure.phabricator.com/D8045 --- src/applications/auth/provider/PhabricatorAuthProvider.php | 3 +-- .../auth/provider/PhabricatorAuthProviderOAuth.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuth1.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuth1JIRA.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuth1Twitter.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuthAmazon.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuthAsana.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuthDisqus.php | 2 +- .../auth/provider/PhabricatorAuthProviderOAuthGitHub.php | 4 ++-- .../auth/provider/PhabricatorAuthProviderOAuthGoogle.php | 4 ++-- .../auth/provider/PhabricatorAuthProviderOAuthTwitch.php | 2 +- .../auth/provider/PhabricatorAuthProviderPersona.php | 2 +- 12 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index 5e228ebca8..ac56803e1b 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -259,8 +259,7 @@ abstract class PhabricatorAuthProvider { public function getLoginURI() { $app = PhabricatorApplication::getByClass('PhabricatorApplicationAuth'); - $uri = $app->getApplicationURI('/login/'.$this->getProviderKey().'/'); - return PhabricatorEnv::getURI($uri); + return $app->getApplicationURI('/login/'.$this->getProviderKey().'/'); } public function getSettingsURI() { diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php index 548ff0bfe0..cf74ff8f46 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php @@ -25,7 +25,7 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider { $adapter->setClientSecret( new PhutilOpaqueEnvelope( $config->getProperty(self::PROPERTY_APP_SECRET))); - $adapter->setRedirectURI($this->getLoginURI()); + $adapter->setRedirectURI(PhabricatorEnv::getURI($this->getLoginURI())); return $adapter; } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1.php index 30c82cd65f..3173c13aeb 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1.php @@ -30,7 +30,7 @@ abstract class PhabricatorAuthProviderOAuth1 extends PhabricatorAuthProvider { if (strlen($secret)) { $adapter->setConsumerSecret(new PhutilOpaqueEnvelope($secret)); } - $adapter->setCallbackURI($this->getLoginURI()); + $adapter->setCallbackURI(PhabricatorEnv::getURI($this->getLoginURI())); return $adapter; } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php index faa21f6c3d..9c835790d9 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1JIRA.php @@ -21,7 +21,7 @@ final class PhabricatorAuthProviderOAuth1JIRA "**Step 1 of 2**: Provide the name and URI for your JIRA install.\n\n". "In the next step, you will configure JIRA."); } else { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "**Step 2 of 2**: In this step, you will configure JIRA.\n\n". "**Create a JIRA Application**: Log into JIRA and go to ". diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php index c1cdd0ded9..a6df0295a4 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth1Twitter.php @@ -8,7 +8,7 @@ final class PhabricatorAuthProviderOAuth1Twitter } public function getConfigurationHelp() { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure Twitter OAuth, create a new application here:". diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php index 99f7fccd29..d01f216eef 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthAmazon.php @@ -8,7 +8,7 @@ final class PhabricatorAuthProviderOAuthAmazon } public function getConfigurationHelp() { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); $uri = new PhutilURI(PhabricatorEnv::getProductionURI('/')); $https_note = null; diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php index a929c8f26e..183bcc221a 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthAsana.php @@ -9,7 +9,7 @@ final class PhabricatorAuthProviderOAuthAsana public function getConfigurationHelp() { $app_uri = PhabricatorEnv::getProductionURI('/'); - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure Asana OAuth, create a new application here:". diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php index 4c374be64c..58c55ccaab 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php @@ -8,7 +8,7 @@ final class PhabricatorAuthProviderOAuthDisqus } public function getConfigurationHelp() { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure Disqus OAuth, create a new application here:". diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php index fe57b7d96a..9c0eb7c907 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php @@ -9,7 +9,7 @@ final class PhabricatorAuthProviderOAuthGitHub public function getConfigurationHelp() { $uri = PhabricatorEnv::getProductionURI('/'); - $callback_uri = $this->getLoginURI(); + $callback_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure GitHub OAuth, create a new GitHub Application here:". @@ -38,7 +38,7 @@ final class PhabricatorAuthProviderOAuthGitHub public function getLoginURI() { // TODO: Clean this up. See PhabricatorAuthOldOAuthRedirectController. - return PhabricatorEnv::getURI('/oauth/github/login/'); + return '/oauth/github/login/'; } } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php index bd10f13afd..6815032e6f 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php @@ -8,7 +8,7 @@ final class PhabricatorAuthProviderOAuthGoogle } public function getConfigurationHelp() { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure Google OAuth, create a new 'API Project' here:". @@ -38,7 +38,7 @@ final class PhabricatorAuthProviderOAuthGoogle public function getLoginURI() { // TODO: Clean this up. See PhabricatorAuthOldOAuthRedirectController. - return PhabricatorEnv::getURI('/oauth/google/login/'); + return '/oauth/google/login/'; } } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthTwitch.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthTwitch.php index db4b12730f..8ab03e06a4 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthTwitch.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthTwitch.php @@ -8,7 +8,7 @@ final class PhabricatorAuthProviderOAuthTwitch } public function getConfigurationHelp() { - $login_uri = $this->getLoginURI(); + $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); return pht( "To configure Twitch.tv OAuth, create a new application here:". diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPersona.php b/src/applications/auth/provider/PhabricatorAuthProviderPersona.php index e96c8d96d1..dca32ed3dd 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPersona.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPersona.php @@ -29,7 +29,7 @@ final class PhabricatorAuthProviderPersona Javelin::initBehavior( 'persona-login', array( - 'loginURI' => $this->getLoginURI(), + 'loginURI' => PhabricatorEnv::getURI($this->getLoginURI()), )); return $this->renderStandardLoginButton(