Changeset View
Standalone View
src/applications/auth/provider/PhabricatorPhabricatorAuthProvider.php
<?php | <?php | ||||
final class PhabricatorPhabricatorAuthProvider | final class PhabricatorPhabricatorAuthProvider | ||||
extends PhabricatorOAuth2AuthProvider { | extends PhabricatorOAuth2AuthProvider { | ||||
const PROPERTY_PHABRICATOR_NAME = 'oauth2:phabricator:name'; | const PROPERTY_PHABRICATOR_NAME = 'oauth2:phabricator:name'; | ||||
const PROPERTY_PHABRICATOR_URI = 'oauth2:phabricator:uri'; | const PROPERTY_PHABRICATOR_URI = 'oauth2:phabricator:uri'; | ||||
public function getProviderName() { | public function getProviderName() { | ||||
return pht('Phabricator'); | return pht('Phabricator'); | ||||
} | } | ||||
public function getConfigurationHelp() { | public function getConfigurationHelp() { | ||||
if ($this->isCreate()) { | if ($this->isCreate()) { | ||||
return pht( | return pht( | ||||
"**Step 1 of 2 - Name Phabricator OAuth Instance**\n\n". | "**Step 1 of 2 - Choose a Service Name**\n\n". | ||||
'Choose a permanent name for the OAuth server instance of '. | 'Choose a permanent name for the remote service. This name is used '. | ||||
'Phabricator. //This// instance of Phabricator uses this name '. | 'internally to keep track of the service, in case the URL changes '. | ||||
'internally to keep track of the OAuth server instance of '. | 'later.'); | ||||
epriestley: Maybe just:
> Choose a Service Name
> Choose a permanent name for the remote service. This… | |||||
'Phabricator, in case the URL changes later.'); | |||||
} | } | ||||
return parent::getConfigurationHelp(); | return parent::getConfigurationHelp(); | ||||
} | } | ||||
protected function getProviderConfigurationHelp() { | protected function getProviderConfigurationHelp() { | ||||
$config = $this->getProviderConfig(); | $config = $this->getProviderConfig(); | ||||
$base_uri = rtrim( | $base_uri = rtrim( | ||||
$config->getProperty(self::PROPERTY_PHABRICATOR_URI), '/'); | $config->getProperty(self::PROPERTY_PHABRICATOR_URI), '/'); | ||||
$login_uri = PhabricatorEnv::getURI($this->getLoginURI()); | $login_uri = PhabricatorEnv::getURI($this->getLoginURI()); | ||||
return pht( | return pht( | ||||
"**Step 2 of 2 - Configure Phabricator OAuth Instance**\n\n". | "**Step 2 of 2 - Configure the platform OAuth Instance**\n\n". | ||||
"To configure Phabricator OAuth, create a new application here:". | "To configure the platform OAuth, create a new application here:". | ||||
"\n\n". | "\n\n". | ||||
"%s/oauthserver/client/create/". | "%s/oauthserver/client/create/". | ||||
"\n\n". | "\n\n". | ||||
"When creating your application, use these settings:". | "When creating your application, use these settings:". | ||||
"\n\n". | "\n\n". | ||||
" - **Redirect URI:** Set this to: `%s`". | " - **Redirect URI:** Set this to: `%s`". | ||||
"\n\n". | "\n\n". | ||||
"After completing configuration, copy the **Client ID** and ". | "After completing configuration, copy the **Client ID** and ". | ||||
"**Client Secret** to the fields above. (You may need to generate the ". | "**Client Secret** to the fields above. (You may need to generate the ". | ||||
"client secret by clicking 'New Secret' first.)", | "client secret by clicking 'New Secret' first.)", | ||||
$base_uri, | $base_uri, | ||||
$login_uri); | $login_uri); | ||||
} | } | ||||
protected function newOAuthAdapter() { | protected function newOAuthAdapter() { | ||||
$config = $this->getProviderConfig(); | $config = $this->getProviderConfig(); | ||||
return id(new PhutilPhabricatorAuthAdapter()) | return id(new PhutilPhabricatorAuthAdapter()) | ||||
->setAdapterDomain($config->getProviderDomain()) | ->setAdapterDomain($config->getProviderDomain()) | ||||
->setPhabricatorBaseURI( | ->setPhabricatorBaseURI( | ||||
$config->getProperty(self::PROPERTY_PHABRICATOR_URI)); | $config->getProperty(self::PROPERTY_PHABRICATOR_URI)); | ||||
} | } | ||||
protected function getLoginIcon() { | protected function getLoginIcon() { | ||||
// TODO: Update icon reference? | |||||
return 'Phabricator'; | return 'Phabricator'; | ||||
} | } | ||||
private function isCreate() { | private function isCreate() { | ||||
return !$this->getProviderConfig()->getID(); | return !$this->getProviderConfig()->getID(); | ||||
} | } | ||||
public function readFormValuesFromProvider() { | public function readFormValuesFromProvider() { | ||||
Show All 36 Lines | if (!$is_setup) { | ||||
$errors = array(); | $errors = array(); | ||||
$issues = array(); | $issues = array(); | ||||
} | } | ||||
$key_name = self::PROPERTY_PHABRICATOR_NAME; | $key_name = self::PROPERTY_PHABRICATOR_NAME; | ||||
$key_uri = self::PROPERTY_PHABRICATOR_URI; | $key_uri = self::PROPERTY_PHABRICATOR_URI; | ||||
if (!strlen($values[$key_name])) { | if (!strlen($values[$key_name])) { | ||||
$errors[] = pht('Phabricator instance name is required.'); | $errors[] = pht('Instance name is required.'); | ||||
Not Done Inline ActionsThese are probably clear enough as just "Instance name is required", etc. epriestley: These are probably clear enough as just "Instance name is required", etc. | |||||
Done Inline ActionsIf this is in reference to the steps above which is now "Choose a Service Name", should this be "Service name is required"? Or should "Choose a Service Name" be changed to "Choose an Instance Name"? cspeckmim: If this is in reference to the steps above which is now "Choose a Service Name", should this be… | |||||
Not Done Inline ActionsI think switching everything to "service" is probably more clear. I'm not sure my use of "instance" is totally obvious to readers, and when Phabricator later added support for actual instancing (via PHABRICATOR_INSTANCE / cluster.instance) that muddied the waters a bit further. Ideally, in modern documentation/UI stuff, I think we should probably use "instance" as a noun only to refer to a particular instantiation of Phabricator using instancing via the cluster.instance mechanism. Phacility is likely the only service in the wild that uses this mechanism. epriestley: I think switching everything to "service" is probably more clear. I'm not sure my use of… | |||||
$issues[$key_name] = pht('Required'); | $issues[$key_name] = pht('Required'); | ||||
} else if (!preg_match('/^[a-z0-9.]+\z/', $values[$key_name])) { | } else if (!preg_match('/^[a-z0-9.]+\z/', $values[$key_name])) { | ||||
$errors[] = pht( | $errors[] = pht( | ||||
'Phabricator instance name must contain only lowercase letters, '. | 'Instance name must contain only lowercase letters, '. | ||||
'digits, and periods.'); | 'digits, and periods.'); | ||||
$issues[$key_name] = pht('Invalid'); | $issues[$key_name] = pht('Invalid'); | ||||
} | } | ||||
if (!strlen($values[$key_uri])) { | if (!strlen($values[$key_uri])) { | ||||
$errors[] = pht('Phabricator base URI is required.'); | $errors[] = pht('Base URI is required.'); | ||||
$issues[$key_uri] = pht('Required'); | $issues[$key_uri] = pht('Required'); | ||||
} else { | } else { | ||||
$uri = new PhutilURI($values[$key_uri]); | $uri = new PhutilURI($values[$key_uri]); | ||||
if (!$uri->getProtocol()) { | if (!$uri->getProtocol()) { | ||||
$errors[] = pht( | $errors[] = pht( | ||||
'Phabricator base URI should include protocol (like "%s").', | 'Base URI should include a protocol (like "%s").', | ||||
'https://'); | 'https://'); | ||||
$issues[$key_uri] = pht('Invalid'); | $issues[$key_uri] = pht('Invalid'); | ||||
} | } | ||||
} | } | ||||
if (!$errors && $is_setup) { | if (!$errors && $is_setup) { | ||||
$config = $this->getProviderConfig(); | $config = $this->getProviderConfig(); | ||||
Show All 22 Lines | public function extendEditForm( | ||||
$v_uri = $values[self::PROPERTY_PHABRICATOR_URI]; | $v_uri = $values[self::PROPERTY_PHABRICATOR_URI]; | ||||
$e_uri = idx($issues, self::PROPERTY_PHABRICATOR_URI, $e_required); | $e_uri = idx($issues, self::PROPERTY_PHABRICATOR_URI, $e_required); | ||||
if ($is_setup) { | if ($is_setup) { | ||||
$form | $form | ||||
->appendChild( | ->appendChild( | ||||
id(new AphrontFormTextControl()) | id(new AphrontFormTextControl()) | ||||
->setLabel(pht('Phabricator Instance Name')) | ->setLabel(pht('Instance Name')) | ||||
->setValue($v_name) | ->setValue($v_name) | ||||
->setName(self::PROPERTY_PHABRICATOR_NAME) | ->setName(self::PROPERTY_PHABRICATOR_NAME) | ||||
->setError($e_name) | ->setError($e_name) | ||||
->setCaption(pht( | ->setCaption(pht( | ||||
'Use lowercase letters, digits, and periods. For example: %s', | 'Use lowercase letters, digits, and periods. For example: %s', | ||||
phutil_tag( | phutil_tag( | ||||
'tt', | 'tt', | ||||
array(), | array(), | ||||
'`phabricator.oauthserver`')))); | '`phabricator.oauthserver`')))); | ||||
} else { | } else { | ||||
$form | $form | ||||
->appendChild( | ->appendChild( | ||||
id(new AphrontFormStaticControl()) | id(new AphrontFormStaticControl()) | ||||
->setLabel(pht('Phabricator Instance Name')) | ->setLabel(pht('Instance Name')) | ||||
->setValue($v_name)); | ->setValue($v_name)); | ||||
} | } | ||||
$form | $form | ||||
->appendChild( | ->appendChild( | ||||
id(new AphrontFormTextControl()) | id(new AphrontFormTextControl()) | ||||
->setLabel(pht('Phabricator Base URI')) | ->setLabel(pht('Base URI')) | ||||
->setValue($v_uri) | ->setValue($v_uri) | ||||
->setName(self::PROPERTY_PHABRICATOR_URI) | ->setName(self::PROPERTY_PHABRICATOR_URI) | ||||
->setCaption( | ->setCaption( | ||||
pht( | pht( | ||||
'The URI where the OAuth server instance of Phabricator is '. | 'The URI where the OAuth server instance for this installation '. | ||||
'installed. For example: %s', | 'is hosted. For example: %s', | ||||
phutil_tag('tt', array(), 'https://phabricator.mycompany.com/'))) | phutil_tag('tt', array(), 'https://platform.mycompany.com/'))) | ||||
->setError($e_uri)); | ->setError($e_uri)); | ||||
if (!$is_setup) { | if (!$is_setup) { | ||||
parent::extendEditForm($request, $form, $values, $issues); | parent::extendEditForm($request, $form, $values, $issues); | ||||
} | } | ||||
} | } | ||||
public function hasSetupStep() { | public function hasSetupStep() { | ||||
Show All 9 Lines |
Maybe just: