From 3b9ccf11f246594b30d4658622ed502a2d9bd7d6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 20 Jun 2013 11:18:11 -0700 Subject: [PATCH] Drive auth config with the database Summary: Ref T1536. This is the last major migration. Moves us over to the DB and drops all the config stuff. Test Plan: - Ran the migration. - Saw all my old config brought forward and respected, with accurate settings. - Ran LDAP import. - Grepped for all removed config options. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran, wez Maniphest Tasks: T1536 Differential Revision: https://secure.phabricator.com/D6243 --- conf/default.conf.php | 156 ---------- resources/sql/patches/20130619.authconf.php | 158 ++++++++++ src/__phutil_library_map__.php | 15 - .../PhabricatorApplicationAuth.php | 8 +- .../auth/ldap/PhabricatorLDAPProvider.php | 292 ------------------ .../PhabricatorLDAPUnknownUserException.php | 4 - .../auth/provider/PhabricatorAuthProvider.php | 82 ++--- .../provider/PhabricatorAuthProviderLDAP.php | 94 +++--- .../provider/PhabricatorAuthProviderOAuth.php | 49 +-- .../PhabricatorAuthProviderOAuthDisqus.php | 36 --- .../PhabricatorAuthProviderOAuthFacebook.php | 57 +--- .../PhabricatorAuthProviderOAuthGitHub.php | 35 --- .../PhabricatorAuthProviderOAuthGoogle.php | 34 -- .../PhabricatorAuthProviderPassword.php | 9 - .../check/PhabricatorSetupCheckFacebook.php | 37 --- .../option/PhabricatorDisqusConfigOptions.php | 38 --- .../PhabricatorFacebookConfigOptions.php | 76 ----- .../option/PhabricatorGitHubConfigOptions.php | 57 ---- .../option/PhabricatorGoogleConfigOptions.php | 57 ---- .../option/PhabricatorLDAPConfigOptions.php | 76 ----- ...abricatorPhabricatorOAuthConfigOptions.php | 53 ---- .../PhabricatorPeopleController.php | 2 +- .../PhabricatorPeopleLdapController.php | 89 +++--- src/applications/phame/storage/PhamePost.php | 2 +- src/applications/phame/view/PhamePostView.php | 2 +- src/infrastructure/env/PhabricatorEnv.php | 17 + .../patch/PhabricatorBuiltinPatchList.php | 4 + 27 files changed, 323 insertions(+), 1216 deletions(-) create mode 100644 resources/sql/patches/20130619.authconf.php delete mode 100644 src/applications/auth/ldap/PhabricatorLDAPProvider.php delete mode 100644 src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php delete mode 100644 src/applications/config/check/PhabricatorSetupCheckFacebook.php delete mode 100644 src/applications/config/option/PhabricatorFacebookConfigOptions.php delete mode 100644 src/applications/config/option/PhabricatorGitHubConfigOptions.php delete mode 100644 src/applications/config/option/PhabricatorGoogleConfigOptions.php delete mode 100644 src/applications/config/option/PhabricatorLDAPConfigOptions.php delete mode 100644 src/applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php diff --git a/conf/default.conf.php b/conf/default.conf.php index 6b8b3b2476..d061e79a5e 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -616,162 +616,6 @@ return array( 'account.minimum-password-length' => 8, -// -- Facebook OAuth -------------------------------------------------------- // - - // Can users use Facebook credentials to login to Phabricator? - 'facebook.auth-enabled' => false, - - // Can users use Facebook credentials to create new Phabricator accounts? - 'facebook.registration-enabled' => true, - - // Are Facebook accounts permanently linked to Phabricator accounts, or can - // the user unlink them? - 'facebook.auth-permanent' => false, - - // The Facebook "Application ID" to use for Facebook API access. - 'facebook.application-id' => null, - - // The Facebook "Application Secret" to use for Facebook API access. - 'facebook.application-secret' => null, - - // Should Phabricator reject requests made by users with - // Secure Browsing disabled? - 'facebook.require-https-auth' => false, - -// -- GitHub OAuth ---------------------------------------------------------- // - - // Can users use GitHub credentials to login to Phabricator? - 'github.auth-enabled' => false, - - // Can users use GitHub credentials to create new Phabricator accounts? - 'github.registration-enabled' => true, - - // Are GitHub accounts permanently linked to Phabricator accounts, or can - // the user unlink them? - 'github.auth-permanent' => false, - - // The GitHub "Client ID" to use for GitHub API access. - 'github.application-id' => null, - - // The GitHub "Secret" to use for GitHub API access. - 'github.application-secret' => null, - - -// -- Google OAuth ---------------------------------------------------------- // - - // Can users use Google credentials to login to Phabricator? - 'google.auth-enabled' => false, - - // Can users use Google credentials to create new Phabricator accounts? - 'google.registration-enabled' => true, - - // Are Google accounts permanently linked to Phabricator accounts, or can - // the user unlink them? - 'google.auth-permanent' => false, - - // The Google "Client ID" to use for Google API access. - 'google.application-id' => null, - - // The Google "Client Secret" to use for Google API access. - 'google.application-secret' => null, - -// -- LDAP Auth ----------------------------------------------------- // - // Enable ldap auth - 'ldap.auth-enabled' => false, - - // The LDAP server hostname - 'ldap.hostname' => null, - - // The LDAP server port - 'ldap.port' => 389, - - // The LDAP base domain name - 'ldap.base_dn' => null, - - // The attribute to be regarded as 'username'. Has to be unique - 'ldap.search_attribute' => null, - - // Perform a search to find a user - // Many LDAP installations do not have the username in the dn, if this is - // true for you set this to true and configure the username_attribute below - 'ldap.search-first' => false, - - // The attribute to search for if you have to search for a user - 'ldap.username-attribute' => null, - - // The attribute(s) to be regarded as 'real name'. - // If more then one attribute is supplied the values of the attributes in - // the array will be joined - 'ldap.real_name_attributes' => array(), - - // A domain name to use when authenticating against Active Directory - // (e.g. 'example.com') - 'ldap.activedirectory_domain' => null, - - // The LDAP version - 'ldap.version' => 3, - - // LDAP Referrals Option - // Whether referrals should be followed by the client - // Should be set to 0 if you use Windows 2003 AD - 'ldap.referrals' => true, - - // The anonymous user name to use before searching a user. - // Many LDAP installations require login even before searching a user, set - // this option to enable it. - 'ldap.anonymous-user-name' => null, - - // The password of the LDAP anonymous user. - 'ldap.anonymous-user-password' => null, - - // Whether to use STARTTLS - 'ldap.start-tls' => false, - - -// -- Disqus OAuth ---------------------------------------------------------- // - - // Can users use Disqus credentials to login to Phabricator? - 'disqus.auth-enabled' => false, - - // Can users use Disqus credentials to create new Phabricator accounts? - 'disqus.registration-enabled' => true, - - // Are Disqus accounts permanently linked to Phabricator accounts, or can - // the user unlink them? - 'disqus.auth-permanent' => false, - - // The Disqus "Client ID" to use for Disqus API access. - 'disqus.application-id' => null, - - // The Disqus "Client Secret" to use for Disqus API access. - 'disqus.application-secret' => null, - - -// -- Phabricator OAuth ----------------------------------------------------- // - - // Meta-town -- Phabricator is itself an OAuth Provider - // TODO -- T887 -- make this support multiple Phabricator instances! - - // The URI of the Phabricator instance to use as an OAuth server. - 'phabricator.oauth-uri' => null, - - // Can users use Phabricator credentials to login to Phabricator? - 'phabricator.auth-enabled' => false, - - // Can users use Phabricator credentials to create new Phabricator accounts? - 'phabricator.registration-enabled' => true, - - // Are Phabricator accounts permanently linked to Phabricator accounts, or can - // the user unlink them? - 'phabricator.auth-permanent' => false, - - // The Phabricator "Client ID" to use for Phabricator API access. - 'phabricator.application-id' => null, - - // The Phabricator "Client Secret" to use for Phabricator API access. - 'phabricator.application-secret' => null, - - // -- Recaptcha ------------------------------------------------------------- // // Is Recaptcha enabled? If disabled, captchas will not appear. You should diff --git a/resources/sql/patches/20130619.authconf.php b/resources/sql/patches/20130619.authconf.php new file mode 100644 index 0000000000..4482520c11 --- /dev/null +++ b/resources/sql/patches/20130619.authconf.php @@ -0,0 +1,158 @@ + array( + 'enabled' => 'ldap.auth-enabled', + 'registration' => true, + 'type' => 'ldap', + 'domain' => 'self', + ), + 'PhabricatorAuthProviderOAuthDisqus' => array( + 'enabled' => 'disqus.auth-enabled', + 'registration' => 'disqus.registration-enabled', + 'permanent' => 'disqus.auth-permanent', + 'oauth.id' => 'disqus.application-id', + 'oauth.secret' => 'disqus.application-secret', + 'type' => 'disqus', + 'domain' => 'disqus.com', + ), + 'PhabricatorAuthProviderOAuthFacebook' => array( + 'enabled' => 'facebook.auth-enabled', + 'registration' => 'facebook.registration-enabled', + 'permanent' => 'facebook.auth-permanent', + 'oauth.id' => 'facebook.application-id', + 'oauth.secret' => 'facebook.application-secret', + 'type' => 'facebook', + 'domain' => 'facebook.com', + ), + 'PhabricatorAuthProviderOAuthGitHub' => array( + 'enabled' => 'github.auth-enabled', + 'registration' => 'github.registration-enabled', + 'permanent' => 'github.auth-permanent', + 'oauth.id' => 'github.application-id', + 'oauth.secret' => 'github.application-secret', + 'type' => 'github', + 'domain' => 'github.com', + ), + 'PhabricatorAuthProviderOAuthGoogle' => array( + 'enabled' => 'google.auth-enabled', + 'registration' => 'google.registration-enabled', + 'permanent' => 'google.auth-permanent', + 'oauth.id' => 'google.application-id', + 'oauth.secret' => 'google.application-secret', + 'type' => 'google', + 'domain' => 'google.com', + ), + 'PhabricatorAuthProviderPassword' => array( + 'enabled' => 'auth.password-auth-enabled', + 'registration' => false, + 'type' => 'password', + 'domain' => 'self', + ), +); + +foreach ($config_map as $provider_class => $spec) { + $enabled_key = idx($spec, 'enabled'); + $enabled = PhabricatorEnv::getEnvConfigIfExists($enabled_key); + + if (!$enabled) { + echo pht("Skipping %s (not enabled).\n", $provider_class); + // This provider was not previously enabled, so we can skip migrating it. + continue; + } else { + echo pht("Migrating %s...\n", $provider_class); + } + + $registration_key = idx($spec, 'registration'); + if ($registration_key === true) { + $registration = 1; + } else if ($registration_key === false) { + $registration = 0; + } else { + $registration = (int)PhabricatorEnv::getEnvConfigIfExists( + $registration_key, + true); + } + + $unlink_key = idx($spec, 'permanent'); + if (!$unlink_key) { + $unlink = 1; + } else { + $unlink = (int)(!PhabricatorEnv::getEnvConfigIfExists($unlink_key)); + } + + $config = id(new PhabricatorAuthProviderConfig()) + ->setIsEnabled(1) + ->setShouldAllowLogin(1) + ->setShouldAllowRegistration($registration) + ->setShouldAllowLink(1) + ->setShouldAllowUnlink($unlink) + ->setProviderType(idx($spec, 'type')) + ->setProviderDomain(idx($spec, 'domain')) + ->setProviderClass($provider_class); + + if (isset($spec['oauth.id'])) { + $config->setProperty( + PhabricatorAuthProviderOAuth::PROPERTY_APP_ID, + PhabricatorEnv::getEnvConfigIfExists(idx($spec, 'oauth.id'))); + $config->setProperty( + PhabricatorAuthProviderOAuth::PROPERTY_APP_SECRET, + PhabricatorEnv::getEnvConfigIfExists(idx($spec, 'oauth.secret'))); + } + + switch ($provider_class) { + case 'PhabricatorAuthProviderOAuthFacebook': + $config->setProperty( + PhabricatorAuthProviderOAuthFacebook::KEY_REQUIRE_SECURE, + (int)PhabricatorEnv::getEnvConfigIfExists( + 'facebook.require-https-auth')); + break; + case 'PhabricatorAuthProviderLDAP': + + $ldap_map = array( + PhabricatorAuthProviderLDAP::KEY_HOSTNAME + => 'ldap.hostname', + PhabricatorAuthProviderLDAP::KEY_PORT + => 'ldap.port', + PhabricatorAuthProviderLDAP::KEY_DISTINGUISHED_NAME + => 'ldap.base_dn', + PhabricatorAuthProviderLDAP::KEY_SEARCH_ATTRIBUTE + => 'ldap.search_attribute', + PhabricatorAuthProviderLDAP::KEY_USERNAME_ATTRIBUTE + => 'ldap.username-attribute', + PhabricatorAuthProviderLDAP::KEY_REALNAME_ATTRIBUTES + => 'ldap.real_name_attributes', + PhabricatorAuthProviderLDAP::KEY_VERSION + => 'ldap.version', + PhabricatorAuthProviderLDAP::KEY_REFERRALS + => 'ldap.referrals', + PhabricatorAuthProviderLDAP::KEY_START_TLS + => 'ldap.start-tls', + PhabricatorAuthProviderLDAP::KEY_ANONYMOUS_USERNAME + => 'ldap.anonymous-user-name', + PhabricatorAuthProviderLDAP::KEY_ANONYMOUS_PASSWORD + => 'ldap.anonymous-user-password', + PhabricatorAuthProviderLDAP::KEY_SEARCH_FIRST + => 'ldap.search-first', + PhabricatorAuthProviderLDAP::KEY_ACTIVEDIRECTORY_DOMAIN + => 'ldap.activedirectory_domain', + ); + + $defaults = array( + 'ldap.version' => 3, + 'ldap.port' => 389, + ); + + foreach ($ldap_map as $pkey => $ckey) { + $default = idx($defaults, $ckey); + $config->setProperty( + $pkey, + PhabricatorEnv::getEnvConfigIfExists($ckey, $default)); + } + break; + } + + $config->save(); +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0d8f66607c..8278634593 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1012,7 +1012,6 @@ phutil_register_library_map(array( 'PhabricatorExtendingPhabricatorConfigOptions' => 'applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php', 'PhabricatorExternalAccount' => 'applications/people/storage/PhabricatorExternalAccount.php', 'PhabricatorExternalAccountQuery' => 'applications/auth/query/PhabricatorExternalAccountQuery.php', - 'PhabricatorFacebookConfigOptions' => 'applications/config/option/PhabricatorFacebookConfigOptions.php', 'PhabricatorFactAggregate' => 'applications/fact/storage/PhabricatorFactAggregate.php', 'PhabricatorFactChartController' => 'applications/fact/controller/PhabricatorFactChartController.php', 'PhabricatorFactController' => 'applications/fact/controller/PhabricatorFactController.php', @@ -1103,10 +1102,8 @@ phutil_register_library_map(array( 'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php', 'PhabricatorGestureExample' => 'applications/uiexample/examples/PhabricatorGestureExample.php', 'PhabricatorGitGraphStream' => 'applications/repository/daemon/PhabricatorGitGraphStream.php', - 'PhabricatorGitHubConfigOptions' => 'applications/config/option/PhabricatorGitHubConfigOptions.php', 'PhabricatorGlobalLock' => 'infrastructure/util/PhabricatorGlobalLock.php', 'PhabricatorGlobalUploadTargetView' => 'applications/files/view/PhabricatorGlobalUploadTargetView.php', - 'PhabricatorGoogleConfigOptions' => 'applications/config/option/PhabricatorGoogleConfigOptions.php', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/PhabricatorHandleObjectSelectorDataView.php', 'PhabricatorHash' => 'infrastructure/util/PhabricatorHash.php', 'PhabricatorHashTestCase' => 'infrastructure/util/__tests__/PhabricatorHashTestCase.php', @@ -1127,9 +1124,6 @@ phutil_register_library_map(array( 'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/PhabricatorJavelinLinter.php', 'PhabricatorJumpNavHandler' => 'applications/search/engine/PhabricatorJumpNavHandler.php', 'PhabricatorKeyValueDatabaseCache' => 'applications/cache/PhabricatorKeyValueDatabaseCache.php', - 'PhabricatorLDAPConfigOptions' => 'applications/config/option/PhabricatorLDAPConfigOptions.php', - 'PhabricatorLDAPProvider' => 'applications/auth/ldap/PhabricatorLDAPProvider.php', - 'PhabricatorLDAPUnknownUserException' => 'applications/auth/ldap/PhabricatorLDAPUnknownUserException.php', 'PhabricatorLintEngine' => 'infrastructure/lint/PhabricatorLintEngine.php', 'PhabricatorLipsumArtist' => 'applications/lipsum/image/PhabricatorLipsumArtist.php', 'PhabricatorLipsumGenerateWorkflow' => 'applications/lipsum/management/PhabricatorLipsumGenerateWorkflow.php', @@ -1301,7 +1295,6 @@ phutil_register_library_map(array( 'PhabricatorPeopleQuery' => 'applications/people/query/PhabricatorPeopleQuery.php', 'PhabricatorPeopleSearchEngine' => 'applications/people/query/PhabricatorPeopleSearchEngine.php', 'PhabricatorPeopleTestDataGenerator' => 'applications/people/lipsum/PhabricatorPeopleTestDataGenerator.php', - 'PhabricatorPhabricatorOAuthConfigOptions' => 'applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php', 'PhabricatorPhameConfigOptions' => 'applications/phame/config/PhabricatorPhameConfigOptions.php', 'PhabricatorPholioConfigOptions' => 'applications/pholio/config/PhabricatorPholioConfigOptions.php', 'PhabricatorPholioMockTestDataGenerator' => 'applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php', @@ -1462,7 +1455,6 @@ phutil_register_library_map(array( 'PhabricatorSetupCheckDatabase' => 'applications/config/check/PhabricatorSetupCheckDatabase.php', 'PhabricatorSetupCheckExtensions' => 'applications/config/check/PhabricatorSetupCheckExtensions.php', 'PhabricatorSetupCheckExtraConfig' => 'applications/config/check/PhabricatorSetupCheckExtraConfig.php', - 'PhabricatorSetupCheckFacebook' => 'applications/config/check/PhabricatorSetupCheckFacebook.php', 'PhabricatorSetupCheckFileinfo' => 'applications/config/check/PhabricatorSetupCheckFileinfo.php', 'PhabricatorSetupCheckGD' => 'applications/config/check/PhabricatorSetupCheckGD.php', 'PhabricatorSetupCheckImagemagick' => 'applications/config/check/PhabricatorSetupCheckImagemagick.php', @@ -2884,7 +2876,6 @@ phutil_register_library_map(array( 1 => 'PhabricatorPolicyInterface', ), 'PhabricatorExternalAccountQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', - 'PhabricatorFacebookConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorFactAggregate' => 'PhabricatorFactDAO', 'PhabricatorFactChartController' => 'PhabricatorFactController', 'PhabricatorFactController' => 'PhabricatorController', @@ -2979,10 +2970,8 @@ phutil_register_library_map(array( 'PhabricatorGarbageCollectorConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorGarbageCollectorDaemon' => 'PhabricatorDaemon', 'PhabricatorGestureExample' => 'PhabricatorUIExample', - 'PhabricatorGitHubConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorGlobalLock' => 'PhutilLock', 'PhabricatorGlobalUploadTargetView' => 'AphrontView', - 'PhabricatorGoogleConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorHashTestCase' => 'PhabricatorTestCase', 'PhabricatorHeaderView' => 'AphrontView', 'PhabricatorHelpController' => 'PhabricatorController', @@ -2999,8 +2988,6 @@ phutil_register_library_map(array( 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorJavelinLinter' => 'ArcanistLinter', 'PhabricatorKeyValueDatabaseCache' => 'PhutilKeyValueCache', - 'PhabricatorLDAPConfigOptions' => 'PhabricatorApplicationConfigOptions', - 'PhabricatorLDAPUnknownUserException' => 'Exception', 'PhabricatorLintEngine' => 'PhutilLintEngine', 'PhabricatorLipsumGenerateWorkflow' => 'PhabricatorLipsumManagementWorkflow', 'PhabricatorLipsumManagementWorkflow' => 'PhutilArgumentWorkflow', @@ -3174,7 +3161,6 @@ phutil_register_library_map(array( 'PhabricatorPeopleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorPeopleSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorPeopleTestDataGenerator' => 'PhabricatorTestDataGenerator', - 'PhabricatorPhabricatorOAuthConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhameConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPholioConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPholioMockTestDataGenerator' => 'PhabricatorTestDataGenerator', @@ -3331,7 +3317,6 @@ phutil_register_library_map(array( 'PhabricatorSetupCheckDatabase' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckExtensions' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckExtraConfig' => 'PhabricatorSetupCheck', - 'PhabricatorSetupCheckFacebook' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckFileinfo' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckGD' => 'PhabricatorSetupCheck', 'PhabricatorSetupCheckImagemagick' => 'PhabricatorSetupCheck', diff --git a/src/applications/auth/application/PhabricatorApplicationAuth.php b/src/applications/auth/application/PhabricatorApplicationAuth.php index 8b2b60f914..26c1bf3c2a 100644 --- a/src/applications/auth/application/PhabricatorApplicationAuth.php +++ b/src/applications/auth/application/PhabricatorApplicationAuth.php @@ -34,15 +34,13 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { return $items; } - public function shouldAppearInLaunchView() { - return false; + public function getApplicationGroup() { + return self::GROUP_ADMIN; } public function getRoutes() { return array( '/auth/' => array( -/* - '' => 'PhabricatorAuthListController', 'config/' => array( 'new/' => 'PhabricatorAuthNewController', @@ -51,8 +49,6 @@ final class PhabricatorApplicationAuth extends PhabricatorApplication { '(?Penable|disable)/(?P\d+)/' => 'PhabricatorAuthDisableController', ), - -*/ 'login/(?P[^/]+)/' => 'PhabricatorAuthLoginController', 'register/(?:(?P[^/]+)/)?' => 'PhabricatorAuthRegisterController', 'start/' => 'PhabricatorAuthStartController', diff --git a/src/applications/auth/ldap/PhabricatorLDAPProvider.php b/src/applications/auth/ldap/PhabricatorLDAPProvider.php deleted file mode 100644 index 52b932c03c..0000000000 --- a/src/applications/auth/ldap/PhabricatorLDAPProvider.php +++ /dev/null @@ -1,292 +0,0 @@ -connection)) { - ldap_unbind($this->connection); - } - } - - public function isProviderEnabled() { - return PhabricatorEnv::getEnvConfig('ldap.auth-enabled'); - } - - public function getHostname() { - return PhabricatorEnv::getEnvConfig('ldap.hostname'); - } - - public function getPort() { - return PhabricatorEnv::getEnvConfig('ldap.port'); - } - - public function getBaseDN() { - return PhabricatorEnv::getEnvConfig('ldap.base_dn'); - } - - public function getSearchAttribute() { - return PhabricatorEnv::getEnvConfig('ldap.search_attribute'); - } - - public function getUsernameAttribute() { - return PhabricatorEnv::getEnvConfig('ldap.username-attribute'); - } - - public function getLDAPVersion() { - return PhabricatorEnv::getEnvConfig('ldap.version'); - } - - public function getLDAPReferrals() { - return PhabricatorEnv::getEnvConfig('ldap.referrals'); - } - - public function getLDAPStartTLS() { - return PhabricatorEnv::getEnvConfig('ldap.start-tls'); - } - - public function bindAnonymousUserEnabled() { - return strlen(trim($this->getAnonymousUserName())) > 0; - } - - public function getAnonymousUserName() { - return PhabricatorEnv::getEnvConfig('ldap.anonymous-user-name'); - } - - public function getAnonymousUserPassword() { - return PhabricatorEnv::getEnvConfig('ldap.anonymous-user-password'); - } - - public function retrieveUserEmail() { - return $this->userData['mail'][0]; - } - - public function retrieveUserRealName() { - return $this->retrieveUserRealNameFromData($this->userData); - } - - public function retrieveUserRealNameFromData($data) { - $name_attributes = PhabricatorEnv::getEnvConfig( - 'ldap.real_name_attributes'); - - $real_name = ''; - if (is_array($name_attributes)) { - foreach ($name_attributes AS $attribute) { - if (isset($data[$attribute][0])) { - $real_name .= $data[$attribute][0].' '; - } - } - - trim($real_name); - } else if (isset($data[$name_attributes][0])) { - $real_name = $data[$name_attributes][0]; - } - - if ($real_name == '') { - return null; - } - - return $real_name; - } - - public function retrieveUsername() { - $key = nonempty( - $this->getUsernameAttribute(), - $this->getSearchAttribute()); - return $this->userData[$key][0]; - } - - public function getConnection() { - if (!isset($this->connection)) { - $this->connection = ldap_connect($this->getHostname(), $this->getPort()); - - if (!$this->connection) { - throw new Exception('Could not connect to LDAP host at '. - $this->getHostname().':'.$this->getPort()); - } - - ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, - $this->getLDAPVersion()); - ldap_set_option($this->connection, LDAP_OPT_REFERRALS, - $this->getLDAPReferrals()); - - if ($this->getLDAPStartTLS()) { - if (!ldap_start_tls($this->getConnection())) { - throw new Exception('Unabled to initialize STARTTLS for LDAP host at '. - $this->getHostname().':'.$this->getPort()); - } - } - } - - return $this->connection; - } - - public function getUserData() { - return $this->userData; - } - - private function invalidLDAPUserErrorMessage($errno, $errmsg) { - return "LDAP Error #".$errno.": ".$errmsg; - } - - public function auth($username, PhutilOpaqueEnvelope $password) { - if (strlen(trim($username)) == 0) { - throw new Exception('Username can not be empty'); - } - - if (PhabricatorEnv::getEnvConfig('ldap.search-first')) { - // To protect against people phishing for accounts we catch the - // exception and present the default exception that would be presented - // in the case of a failed bind. - try { - $user = $this->getUser($this->getUsernameAttribute(), $username); - $username = $user[$this->getSearchAttribute()][0]; - } catch (PhabricatorLDAPUnknownUserException $e) { - throw new Exception( - $this->invalidLDAPUserErrorMessage( - self::LDAP_INVALID_CREDENTIALS, - ldap_err2str(self::LDAP_INVALID_CREDENTIALS))); - } - } - - $conn = $this->getConnection(); - - $activeDirectoryDomain = - PhabricatorEnv::getEnvConfig('ldap.activedirectory_domain'); - - if ($activeDirectoryDomain) { - $dn = $username.'@'.$activeDirectoryDomain; - } else { - if (isset($user)) { - $dn = $user['dn']; - } else { - $dn = ldap_sprintf( - '%Q=%s,%Q', - $this->getSearchAttribute(), - $username, - $this->getBaseDN()); - } - } - - // NOTE: It is very important we suppress any messages that occur here, - // because it logs passwords if it reaches an error log of any sort. - DarkConsoleErrorLogPluginAPI::enableDiscardMode(); - $result = @ldap_bind($conn, $dn, $password->openEnvelope()); - DarkConsoleErrorLogPluginAPI::disableDiscardMode(); - - if (!$result) { - throw new Exception( - $this->invalidLDAPUserErrorMessage( - ldap_errno($conn), - ldap_error($conn))); - } - - $this->userData = $this->getUser($this->getSearchAttribute(), $username); - return $this->userData; - } - - private function getUser($attribute, $username) { - $conn = $this->getConnection(); - - if ($this->bindAnonymousUserEnabled()) { - // NOTE: It is very important we suppress any messages that occur here, - // because it logs passwords if it reaches an error log of any sort. - DarkConsoleErrorLogPluginAPI::enableDiscardMode(); - $result = ldap_bind( - $conn, - $this->getAnonymousUserName(), - $this->getAnonymousUserPassword()); - DarkConsoleErrorLogPluginAPI::disableDiscardMode(); - - if (!$result) { - throw new Exception('Bind anonymous account failed. '. - $this->invalidLDAPUserErrorMessage( - ldap_errno($conn), - ldap_error($conn))); - } - } - - $query = ldap_sprintf( - '%Q=%S', - $attribute, - $username); - - $result = ldap_search($conn, $this->getBaseDN(), $query); - - if (!$result) { - throw new Exception('Search failed. '. - $this->invalidLDAPUserErrorMessage( - ldap_errno($conn), - ldap_error($conn))); - } - - $entries = ldap_get_entries($conn, $result); - - if ($entries === false) { - throw new Exception('Could not get entries'); - } - - if ($entries['count'] > 1) { - throw new Exception('Found more then one user with this '. - $attribute); - } - - if ($entries['count'] == 0) { - throw new PhabricatorLDAPUnknownUserException('Could not find user'); - } - - return $entries[0]; - } - - public function search($query) { - $result = ldap_search($this->getConnection(), $this->getBaseDN(), - $query); - - if (!$result) { - throw new Exception('Search failed. Please check your LDAP and HTTP '. - 'logs for more information.'); - } - - $entries = ldap_get_entries($this->getConnection(), $result); - - if ($entries === false) { - throw new Exception('Could not get entries'); - } - - if ($entries['count'] == 0) { - throw new Exception('No results found'); - } - - - $rows = array(); - - for ($i = 0; $i < $entries['count']; $i++) { - $row = array(); - $entry = $entries[$i]; - - // Get username, email and realname - $username = $entry[$this->getSearchAttribute()][0]; - if (empty($username)) { - continue; - } - $row[] = $username; - $row[] = $entry['mail'][0]; - $row[] = $this->retrieveUserRealNameFromData($entry); - - - $rows[] = $row; - } - - return $rows; - - } -} diff --git a/src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php b/src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php deleted file mode 100644 index 7013267a84..0000000000 --- a/src/applications/auth/ldap/PhabricatorLDAPUnknownUserException.php +++ /dev/null @@ -1,4 +0,0 @@ -execute(); $providers = array(); - if ($configs) { - foreach ($configs as $config) { - if (!isset($objects[$config->getProviderClass()])) { - // This configuration is for a provider which is not installed. - continue; - } - - $object = clone $objects[$config->getProviderClass()]; - $object->attachProviderConfig($config); - - $key = $object->getProviderKey(); - if (isset($providers[$key])) { - throw new Exception( - pht( - "Two authentication providers use the same provider key ". - "('%s'). Each provider must be identified by a unique ". - "key.", - $key)); - } - $providers[$key] = $object; + foreach ($configs as $config) { + if (!isset($objects[$config->getProviderClass()])) { + // This configuration is for a provider which is not installed. + continue; } - } else { - // TODO: Remove this once we transition to be completely database - // driven. - $from_class_map = array(); - foreach ($objects as $object) { - $from_class = get_class($object); - $object_providers = $object->createProviders(); - assert_instances_of($object_providers, 'PhabricatorAuthProvider'); - foreach ($object_providers as $provider) { - $key = $provider->getProviderKey(); - if (isset($providers[$key])) { - $first_class = $from_class_map[$key]; - throw new Exception( - "PhabricatorAuthProviders '{$first_class}' and ". - "'{$from_class}' both created authentication providers ". - "identified by key '{$key}'. Provider keys must be unique."); - } - $providers[$key] = $provider; - $from_class_map[$key] = $from_class; - } + + $object = clone $objects[$config->getProviderClass()]; + $object->attachProviderConfig($config); + + $key = $object->getProviderKey(); + if (isset($providers[$key])) { + throw new Exception( + pht( + "Two authentication providers use the same provider key ". + "('%s'). Each provider must be identified by a unique ". + "key.", + $key)); } + $providers[$key] = $object; } } @@ -141,38 +118,23 @@ abstract class PhabricatorAuthProvider { abstract public function getAdapter(); public function isEnabled() { - if ($this->hasProviderConfig()) { - return $this->getProviderConfig()->getIsEnabled(); - } - return true; + return $this->getProviderConfig()->getIsEnabled(); } public function shouldAllowLogin() { - if ($this->hasProviderConfig()) { - return $this->getProviderConfig()->getShouldAllowLogin(); - } - return true; + return $this->getProviderConfig()->getShouldAllowLogin(); } public function shouldAllowRegistration() { - if ($this->hasProviderConfig()) { - return $this->getProviderConfig()->getShouldAllowRegistration(); - } - return true; + return $this->getProviderConfig()->getShouldAllowRegistration(); } public function shouldAllowAccountLink() { - if ($this->hasProviderConfig()) { - return $this->getProviderConfig()->getShouldAllowLink(); - } - return true; + return $this->getProviderConfig()->getShouldAllowLink(); } public function shouldAllowAccountUnlink() { - if ($this->hasProviderConfig()) { - return $this->getProviderConfig()->getShouldAllowUnlink(); - } - return true; + return $this->getProviderConfig()->getShouldAllowUnlink(); } public function buildLoginForm( diff --git a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php index 9f21b8c9af..5b6e97b47d 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php @@ -21,38 +21,37 @@ final class PhabricatorAuthProviderLDAP ->setProperty(self::KEY_VERSION, 3); } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('ldap.auth-enabled'); - } - public function getAdapter() { if (!$this->adapter) { + $conf = $this->getProviderConfig(); $adapter = id(new PhutilAuthAdapterLDAP()) - ->setHostname(PhabricatorEnv::getEnvConfig('ldap.hostname')) - ->setPort(PhabricatorEnv::getEnvConfig('ldap.port')) - ->setBaseDistinguishedName(PhabricatorEnv::getEnvConfig('ldap.base_dn')) + ->setHostname( + $conf->getProperty(self::KEY_HOSTNAME)) + ->setPort( + $conf->getProperty(self::KEY_PORT)) + ->setBaseDistinguishedName( + $conf->getProperty(self::KEY_DISTINGUISHED_NAME)) ->setSearchAttribute( - PhabricatorEnv::getEnvConfig('ldap.search_attribute')) + $conf->getProperty(self::KEY_SEARCH_ATTRIBUTE)) ->setUsernameAttribute( - PhabricatorEnv::getEnvConfig('ldap.username-attribute')) + $conf->getProperty(self::KEY_USERNAME_ATTRIBUTE)) ->setRealNameAttributes( - PhabricatorEnv::getEnvConfig('ldap.real_name_attributes')) - ->setLDAPVersion(PhabricatorEnv::getEnvConfig('ldap.version')) - ->setLDAPReferrals(PhabricatorEnv::getEnvConfig('ldap.referrals')) - ->setLDAPStartTLS(PhabricatorEnv::getEnvConfig('ldap.start-tls')) + $conf->getProperty(self::KEY_REALNAME_ATTRIBUTES, array())) + ->setLDAPVersion( + $conf->getProperty(self::KEY_VERSION)) + ->setLDAPReferrals( + $conf->getProperty(self::KEY_REFERRALS)) + ->setLDAPStartTLS( + $conf->getProperty(self::KEY_START_TLS)) ->setAnonymousUsername( - PhabricatorEnv::getEnvConfig('ldap.anonymous-user-name')) + $conf->getProperty(self::KEY_ANONYMOUS_USERNAME)) ->setAnonymousPassword( - new PhutilOpaqueEnvelope( - PhabricatorEnv::getEnvConfig('ldap.anonymous-user-password'))) - ->setSearchFirst(PhabricatorEnv::getEnvConfig('ldap.search-first')) + new PhutilOpaqueEnvelope( + $conf->getProperty(self::KEY_ANONYMOUS_PASSWORD))) + ->setSearchFirst( + $conf->getProperty(self::KEY_SEARCH_FIRST)) ->setActiveDirectoryDomain( - PhabricatorEnv::getEnvConfig('ldap.activedirectory_domain')); + $conf->getProperty(self::KEY_ACTIVEDIRECTORY_DOMAIN)); $this->adapter = $adapter; } return $this->adapter; @@ -217,34 +216,11 @@ final class PhabricatorAuthProviderLDAP } public function readFormValuesFromProvider() { - return array( - self::KEY_HOSTNAME => - PhabricatorEnv::getEnvConfig('ldap.hostname'), - self::KEY_PORT => - PhabricatorEnv::getEnvConfig('ldap.port'), - self::KEY_DISTINGUISHED_NAME => - PhabricatorEnv::getEnvConfig('ldap.base_dn'), - self::KEY_SEARCH_ATTRIBUTE => - PhabricatorEnv::getEnvConfig('ldap.search_attribute'), - self::KEY_USERNAME_ATTRIBUTE => - PhabricatorEnv::getEnvConfig('ldap.username-attribute'), - self::KEY_REALNAME_ATTRIBUTES => - PhabricatorEnv::getEnvConfig('ldap.real_name_attributes'), - self::KEY_VERSION => - PhabricatorEnv::getEnvConfig('ldap.version'), - self::KEY_REFERRALS => - PhabricatorEnv::getEnvConfig('ldap.referrals'), - self::KEY_START_TLS => - PhabricatorEnv::getEnvConfig('ldap.start-tls'), - self::KEY_ANONYMOUS_USERNAME => - PhabricatorEnv::getEnvConfig('ldap.anonymous-user-name'), - self::KEY_ANONYMOUS_PASSWORD => - PhabricatorEnv::getEnvConfig('ldap.anonymous-user-password'), - self::KEY_SEARCH_FIRST => - PhabricatorEnv::getEnvConfig('ldap.search-first'), - self::KEY_ACTIVEDIRECTORY_DOMAIN => - PhabricatorEnv::getEnvConfig('ldap.activedirectory_domain'), - ); + $properties = array(); + foreach ($this->getPropertyLabels() as $key => $ignored) { + $properties[$key] = $this->getProviderConfig()->getProperty($key); + } + return $properties; } public function readFormValuesFromRequest(AphrontRequest $request) { @@ -252,7 +228,7 @@ final class PhabricatorAuthProviderLDAP foreach ($this->getPropertyKeys() as $key) { switch ($key) { case self::KEY_REALNAME_ATTRIBUTES: - $values[$key] = $request->getStrList($key); + $values[$key] = $request->getStrList($key, array()); break; default: $values[$key] = $request->getStr($key); @@ -337,7 +313,7 @@ final class PhabricatorAuthProviderLDAP ->setName($key) ->setLabel($label) ->setCaption($caption) - ->setValue(implode(', ', $value)); + ->setValue($value ? implode(', ', $value) : null); break; case 'password': $control = id(new AphrontFormPasswordControl()) @@ -405,4 +381,16 @@ final class PhabricatorAuthProviderLDAP return parent::renderConfigPropertyTransactionTitle($xaction); } + public static function getLDAPProvider() { + $providers = self::getAllEnabledProviders(); + + foreach ($providers as $provider) { + if ($provider instanceof PhabricatorAuthProviderLDAP) { + return $provider; + } + } + + return null; + } + } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php index 883cd9099e..6ae04d59e9 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php @@ -4,8 +4,6 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider { protected $adapter; - abstract protected function getOAuthClientID(); - abstract protected function getOAuthClientSecret(); abstract protected function newOAuthAdapter(); public function getDescriptionForCreate() { @@ -21,34 +19,12 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider { return $this->adapter; } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - $this->getOAuthClientID() && - $this->getOAuthClientSecret(); - } - protected function configureAdapter(PhutilAuthAdapterOAuth $adapter) { - - if ($this->hasProviderConfig()) { - $config = $this->getProviderConfig(); - $adapter->setClientID($config->getProperty(self::PROPERTY_APP_ID)); - $adapter->setClientSecret( - new PhutilOpaqueEnvelope( - $config->getProperty(self::PROPERTY_APP_SECRET))); - } else { - if ($this->getOAuthClientID()) { - $adapter->setClientID($this->getOAuthClientID()); - } - - if ($this->getOAuthClientSecret()) { - $adapter->setClientSecret($this->getOAuthClientSecret()); - } - } - + $config = $this->getProviderConfig(); + $adapter->setClientID($config->getProperty(self::PROPERTY_APP_ID)); + $adapter->setClientSecret( + new PhutilOpaqueEnvelope( + $config->getProperty(self::PROPERTY_APP_SECRET))); $adapter->setRedirectURI($this->getLoginURI()); return $adapter; } @@ -187,18 +163,9 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider { const PROPERTY_APP_SECRET = 'oauth:app:secret'; public function readFormValuesFromProvider() { - - if ($this->hasProviderConfig()) { - $config = $this->getProviderConfig(); - $id = $config->getProperty(self::PROPERTY_APP_ID); - $secret = $config->getProperty(self::PROPERTY_APP_SECRET); - } else { - $id = $this->getOAuthClientID(); - $secret = $this->getOAuthClientSecret(); - if ($secret) { - $secret = $secret->openEnvelope(); - } - } + $config = $this->getProviderConfig(); + $id = $config->getProperty(self::PROPERTY_APP_ID); + $secret = $config->getProperty(self::PROPERTY_APP_SECRET); return array( self::PROPERTY_APP_ID => $id, diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php index 3f4a88ed09..acc039d832 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthDisqus.php @@ -15,40 +15,4 @@ final class PhabricatorAuthProviderOAuthDisqus return 'Disqus'; } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('disqus.auth-enabled'); - } - - protected function getOAuthClientID() { - return PhabricatorEnv::getEnvConfig('disqus.application-id'); - } - - protected function getOAuthClientSecret() { - $secret = PhabricatorEnv::getEnvConfig('disqus.application-secret'); - if ($secret) { - return new PhutilOpaqueEnvelope($secret); - } - return null; - } - - public function shouldAllowRegistration() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowRegistration(); - } - return PhabricatorEnv::getEnvConfig('disqus.registration-enabled'); - } - - public function shouldAllowAccountUnlink() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowAccountUnlink(); - } - - return !PhabricatorEnv::getEnvConfig('disqus.auth-permanent'); - } - } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php index cfb8d84a6b..eeb25ed5d6 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthFacebook.php @@ -15,55 +15,20 @@ final class PhabricatorAuthProviderOAuthFacebook } protected function newOAuthAdapter() { - $secure_only = PhabricatorEnv::getEnvConfig('facebook.require-https-auth'); + $require_secure = $this->getProviderConfig()->getProperty( + self::KEY_REQUIRE_SECURE); + return id(new PhutilAuthAdapterOAuthFacebook()) - ->setRequireSecureBrowsing($secure_only); + ->setRequireSecureBrowsing($require_secure); } protected function getLoginIcon() { return 'Facebook'; } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('facebook.auth-enabled'); - } - - protected function getOAuthClientID() { - return PhabricatorEnv::getEnvConfig('facebook.application-id'); - } - - protected function getOAuthClientSecret() { - $secret = PhabricatorEnv::getEnvConfig('facebook.application-secret'); - if ($secret) { - return new PhutilOpaqueEnvelope($secret); - } - return null; - } - - public function shouldAllowRegistration() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowRegistration(); - } - return PhabricatorEnv::getEnvConfig('facebook.registration-enabled'); - } - - public function shouldAllowAccountUnlink() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowAccountUnlink(); - } - 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. + $require_secure = $this->getProviderConfig()->getProperty( + self::KEY_REQUIRE_SECURE); return parent::readFormValuesFromProvider() + array( self::KEY_REQUIRE_SECURE => $require_secure, @@ -130,5 +95,15 @@ final class PhabricatorAuthProviderOAuthFacebook return parent::renderConfigPropertyTransactionTitle($xaction); } + public static function getFacebookApplicationID() { + $providers = PhabricatorAuthProvider::getAllProviders(); + $fb_provider = idx($providers, 'facebook:facebook.com'); + if (!$fb_provider) { + return null; + } + + return $fb_provider->getProperty( + PhabricatorAuthProviderOAuth::PROPERTY_APP_ID); + } } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php index f0c09fb25a..f56f202606 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGitHub.php @@ -15,39 +15,4 @@ final class PhabricatorAuthProviderOAuthGitHub return 'Github'; } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('github.auth-enabled'); - } - - protected function getOAuthClientID() { - return PhabricatorEnv::getEnvConfig('github.application-id'); - } - - protected function getOAuthClientSecret() { - $secret = PhabricatorEnv::getEnvConfig('github.application-secret'); - if ($secret) { - return new PhutilOpaqueEnvelope($secret); - } - return null; - } - - public function shouldAllowRegistration() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowRegistration(); - } - return PhabricatorEnv::getEnvConfig('github.registration-enabled'); - } - - public function shouldAllowAccountUnlink() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowAccountUnlink(); - } - return !PhabricatorEnv::getEnvConfig('github.auth-permanent'); - } - } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php index 70ea28fa7e..a20aec5df2 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuthGoogle.php @@ -15,40 +15,6 @@ final class PhabricatorAuthProviderOAuthGoogle return 'Google'; } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('google.auth-enabled'); - } - - protected function getOAuthClientID() { - return PhabricatorEnv::getEnvConfig('google.application-id'); - } - - protected function getOAuthClientSecret() { - $secret = PhabricatorEnv::getEnvConfig('google.application-secret'); - if ($secret) { - return new PhutilOpaqueEnvelope($secret); - } - return null; - } - - public function shouldAllowRegistration() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowRegistration(); - } - return PhabricatorEnv::getEnvConfig('google.registration-enabled'); - } - - public function shouldAllowAccountUnlink() { - if ($this->hasProviderConfig()) { - return parent::shouldAllowAccountUnlink(); - } - return !PhabricatorEnv::getEnvConfig('google.auth-permanent'); - } - public function getLoginURI() { // TODO: Clean this up. See PhabricatorAuthOldOAuthRedirectController. return PhabricatorEnv::getURI('/oauth/google/login/'); diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php index 6795ad64a0..3f315f676c 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -14,15 +14,6 @@ final class PhabricatorAuthProviderPassword 'Allow users to login or register using a username and password.'); } - public function isEnabled() { - if ($this->hasProviderConfig()) { - return parent::isEnabled(); - } - - return parent::isEnabled() && - PhabricatorEnv::getEnvConfig('auth.password-auth-enabled'); - } - public function getAdapter() { if (!$this->adapter) { $adapter = new PhutilAuthAdapterEmpty(); diff --git a/src/applications/config/check/PhabricatorSetupCheckFacebook.php b/src/applications/config/check/PhabricatorSetupCheckFacebook.php deleted file mode 100644 index 3f5c7d6511..0000000000 --- a/src/applications/config/check/PhabricatorSetupCheckFacebook.php +++ /dev/null @@ -1,37 +0,0 @@ -newIssue('config.facebook.application-id') - ->setName(pht("Facebook Application ID Not Set")) - ->setMessage($message) - ->addRelatedPhabricatorConfig('facebook.auth-enabled') - ->addPhabricatorConfig('facebook.application-id'); - } - - if (!PhabricatorEnv::getEnvConfig('facebook.application-secret')) { - $message = pht( - 'You have enabled Facebook authentication, but have not provided a '. - 'Facebook Application Secret. Provide one or disable Facebook '. - 'authentication.'); - - $this->newIssue('config.facebook.application-secret') - ->setName(pht("Facebook Application Secret Not Set")) - ->setMessage($message) - ->addRelatedPhabricatorConfig('facebook.auth-enabled') - ->addPhabricatorConfig('facebook.application-secret'); - } - } -} diff --git a/src/applications/config/option/PhabricatorDisqusConfigOptions.php b/src/applications/config/option/PhabricatorDisqusConfigOptions.php index 592362f1db..d3a06eba36 100644 --- a/src/applications/config/option/PhabricatorDisqusConfigOptions.php +++ b/src/applications/config/option/PhabricatorDisqusConfigOptions.php @@ -13,44 +13,6 @@ final class PhabricatorDisqusConfigOptions public function getOptions() { return array( - $this->newOption('disqus.auth-enabled', 'bool', false) - ->setBoolOptions( - array( - pht("Enable Disqus Authentication"), - pht("Disable Disqus Authentication"), - )) - ->setDescription( - pht( - 'Allow users to login to Phabricator using Disqus credentials.')), - $this->newOption('disqus.registration-enabled', 'bool', true) - ->setBoolOptions( - array( - pht("Enable Disqus Registration"), - pht("Disable Disqus Registration"), - )) - ->setDescription( - pht( - 'Allow users to create new Phabricator accounts using Disqus '. - 'credentials.')), - $this->newOption('disqus.auth-permanent', 'bool', false) - ->setBoolOptions( - array( - pht("Permanently Bind Disqus Accounts"), - pht("Allow Disqus Account Unlinking"), - )) - ->setDescription( - pht( - 'Are Phabricator accounts permanently bound to Disqus '. - 'accounts?')), - $this->newOption('disqus.application-id', 'string', null) - ->setDescription( - pht( - 'Disqus "Client ID" to use for Disqus API access.')), - $this->newOption('disqus.application-secret', 'string', null) - ->setMasked(true) - ->setDescription( - pht( - 'Disqus "Secret" to use for Diqsus API access.')), $this->newOption('disqus.shortname', 'string', null) ->setSummary(pht("Shortname for Disqus comment widget.")) ->setDescription( diff --git a/src/applications/config/option/PhabricatorFacebookConfigOptions.php b/src/applications/config/option/PhabricatorFacebookConfigOptions.php deleted file mode 100644 index 13a3069dc4..0000000000 --- a/src/applications/config/option/PhabricatorFacebookConfigOptions.php +++ /dev/null @@ -1,76 +0,0 @@ -newOption('facebook.auth-enabled', 'bool', false) - ->setBoolOptions( - array( - pht("Enable Facebook Authentication"), - pht("Disable Facebook Authentication"), - )) - ->setDescription( - pht( - 'Allow users to login to Phabricator using Facebook credentials.')), - $this->newOption('facebook.registration-enabled', 'bool', true) - ->setBoolOptions( - array( - pht("Enable Facebook Registration"), - pht("Disable Facebook Registration"), - )) - ->setDescription( - pht( - 'Allow users to create new Phabricator accounts using Facebook '. - 'credentials.')), - $this->newOption('facebook.auth-permanent', 'bool', false) - ->setBoolOptions( - array( - pht("Permanently Bind Facebook Accounts"), - pht("Allow Facebook Account Unlinking"), - )) - ->setDescription( - pht( - 'Are Phabricator accounts permanently bound to Facebook '. - 'accounts?')), - $this->newOption('facebook.application-id', 'string', null) - ->setDescription( - pht( - 'Facebook "Application ID" to use for Facebook API access.')), - $this->newOption('facebook.application-secret', 'string', null) - ->setMasked(true) - ->setDescription( - pht( - 'Facebook "Application Secret" to use for Facebook API access.')), - $this->newOption('facebook.require-https-auth', 'bool', false) - ->setBoolOptions( - array( - pht("Require HTTPS"), - pht("Do Not Require HTTPS"), - )) - ->setSummary( - pht( - 'Reject Facebook logins from accounts that do not have Facebook '. - 'configured in HTTPS-only mode.')) - ->setDescription( - pht( - 'You can require users logging in via Facebook auth have Facebook '. - 'set to HTTPS-only, which ensures their Facebook cookies are '. - 'SSL-only. This makes it more difficult for an attacker to '. - 'escalate a cookie-sniffing attack which captures Facebook '. - 'credentials into Phabricator access, but will require users '. - 'change their Facebook settings if they do not have this mode '. - 'enabled.')), - ); - } - -} diff --git a/src/applications/config/option/PhabricatorGitHubConfigOptions.php b/src/applications/config/option/PhabricatorGitHubConfigOptions.php deleted file mode 100644 index fc24c1d2e8..0000000000 --- a/src/applications/config/option/PhabricatorGitHubConfigOptions.php +++ /dev/null @@ -1,57 +0,0 @@ -newOption('github.auth-enabled', 'bool', false) - ->setBoolOptions( - array( - pht("Enable GitHub Authentication"), - pht("Disable GitHub Authentication"), - )) - ->setDescription( - pht( - 'Allow users to login to Phabricator using GitHub credentials.')), - $this->newOption('github.registration-enabled', 'bool', true) - ->setBoolOptions( - array( - pht("Enable GitHub Registration"), - pht("Disable GitHub Registration"), - )) - ->setDescription( - pht( - 'Allow users to create new Phabricator accounts using GitHub '. - 'credentials.')), - $this->newOption('github.auth-permanent', 'bool', false) - ->setBoolOptions( - array( - pht("Permanently Bind GitHub Accounts"), - pht("Allow GitHub Account Unlinking"), - )) - ->setDescription( - pht( - 'Are Phabricator accounts permanently bound to GitHub '. - 'accounts?')), - $this->newOption('github.application-id', 'string', null) - ->setDescription( - pht( - 'GitHub "Client ID" to use for GitHub API access.')), - $this->newOption('github.application-secret', 'string', null) - ->setMasked(true) - ->setDescription( - pht( - 'GitHub "Secret" to use for GitHub API access.')), - ); - } - -} diff --git a/src/applications/config/option/PhabricatorGoogleConfigOptions.php b/src/applications/config/option/PhabricatorGoogleConfigOptions.php deleted file mode 100644 index 544539b6f0..0000000000 --- a/src/applications/config/option/PhabricatorGoogleConfigOptions.php +++ /dev/null @@ -1,57 +0,0 @@ -newOption('google.auth-enabled', 'bool', false) - ->setBoolOptions( - array( - pht("Enable Google Authentication"), - pht("Disable Google Authentication"), - )) - ->setDescription( - pht( - 'Allow users to login to Phabricator using Google credentials.')), - $this->newOption('google.registration-enabled', 'bool', true) - ->setBoolOptions( - array( - pht("Enable Google Registration"), - pht("Disable Google Registration"), - )) - ->setDescription( - pht( - 'Allow users to create new Phabricator accounts using Google '. - 'credentials.')), - $this->newOption('google.auth-permanent', 'bool', false) - ->setBoolOptions( - array( - pht("Permanently Bind Google Accounts"), - pht("Allow Google Account Unlinking"), - )) - ->setDescription( - pht( - 'Are Phabricator accounts permanently bound to Google '. - 'accounts?')), - $this->newOption('google.application-id', 'string', null) - ->setDescription( - pht( - 'Google "Client ID" to use for Google API access.')), - $this->newOption('google.application-secret', 'string', null) - ->setMasked(true) - ->setDescription( - pht( - 'Google "Secret" to use for Google API access.')), - ); - } - -} diff --git a/src/applications/config/option/PhabricatorLDAPConfigOptions.php b/src/applications/config/option/PhabricatorLDAPConfigOptions.php deleted file mode 100644 index f4ac83d58c..0000000000 --- a/src/applications/config/option/PhabricatorLDAPConfigOptions.php +++ /dev/null @@ -1,76 +0,0 @@ -newOption('ldap.auth-enabled', 'bool', false) - ->setBoolOptions( - array( - pht("Enable LDAP Authentication"), - pht("Disable LDAP Authentication"), - )) - ->setDescription( - pht('Enable LDAP for authentication and registration.')), - $this->newOption('ldap.hostname', 'string', null) - ->setDescription(pht('LDAP server host name.')), - $this->newOption('ldap.port', 'int', 389) - ->setDescription(pht('LDAP server port.')), - $this->newOption('ldap.anonymous-user-name', 'string', null) - ->setDescription( - pht('Username to login to LDAP server with.')), - $this->newOption('ldap.anonymous-user-password', 'string', null) - ->setMasked(true) - ->setDescription( - pht('Password to login to LDAP server with.')), - - // TODO: I have only a vague understanding of what these options do; - // improve the documentation here and provide examples. - - $this->newOption('ldap.base_dn', 'string', null) - ->setDescription(pht('LDAP base domain name.')), - $this->newOption('ldap.search_attribute', 'string', null), - $this->newOption('ldap.search-first', 'bool', false) - ->setBoolOptions( - array( - pht("Enabled"), - pht("Disabled"), - )), - $this->newOption('ldap.username-attribute', 'string', null), - $this->newOption('ldap.real_name_attributes', 'list', array()) - ->setDescription( - pht( - "Attribute or attributes to use as the user's real name. If ". - "multiple attributes are provided, they will be joined with ". - "spaces.")), - $this->newOption('ldap.activedirectory_domain', 'string', null), - $this->newOption('ldap.version', 'int', 3), - $this->newOption('ldap.referrals', 'bool', true) - ->setBoolOptions( - array( - pht("Follow referrals"), - pht("Do not follow referrals"), - )) - ->setDescription( - pht("You may need to disable this if you use Windows 2003 ". - "Active Directory.")), - $this->newOption('ldap.start-tls', 'bool', false) - ->setBoolOptions( - array( - pht("Use STARTTLS"), - pht("Do not use STARTTLS"), - )) - ->setDescription(pht("Should LDAP use STARTTLS?")) - ); - } - -} diff --git a/src/applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php b/src/applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php deleted file mode 100644 index acc9163d02..0000000000 --- a/src/applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php +++ /dev/null @@ -1,53 +0,0 @@ -newOption('phabricator.oauth-uri', 'string', null) - ->setDescription( - pht( - "The URI of the Phabricator instance to use as an OAuth server.")) - ->addExample('https://phabricator.example.com/', pht('Valid Setting')), - $this->newOption('phabricator.auth-enabled', 'bool', false) - ->setDescription( - pht( - "Can users use Phabricator credentials to login to Phabricator?")), - $this->newOption('phabricator.registration-enabled', 'bool', true) - ->setDescription( - pht( - "Can users use Phabricator credentials to create new Phabricator ". - "accounts?")), - $this->newOption('phabricator.auth-permanent', 'bool', false) - ->setBoolOptions( - array( - pht("Permanent"), - pht("Able to be unlinked"), - )) - ->setDescription( - pht( - "Are Phabricator accounts permanently linked to Phabricator ". - "accounts, or can the user unlink them?")), - $this->newOption('phabricator.application-id', 'string', null) - ->setDescription( - pht( - "The Phabricator 'Client ID' to use for Phabricator API access.")), - $this->newOption('phabricator.application-secret', 'string', null) - ->setMasked(true) - ->setDescription( - pht( - "The Phabricator 'Client Secret' to use for Phabricator API ". - "access.")), - ); - } - -} diff --git a/src/applications/people/controller/PhabricatorPeopleController.php b/src/applications/people/controller/PhabricatorPeopleController.php index 1e8aa69a19..954e40b47f 100644 --- a/src/applications/people/controller/PhabricatorPeopleController.php +++ b/src/applications/people/controller/PhabricatorPeopleController.php @@ -18,7 +18,7 @@ abstract class PhabricatorPeopleController extends PhabricatorController { if ($viewer->getIsAdmin()) { $nav->addLabel(pht('User Administration')); - if (PhabricatorEnv::getEnvConfig('ldap.auth-enabled') === true) { + if (PhabricatorAuthProviderLDAP::getLDAPProvider()) { $nav->addFilter('ldap', pht('Import from LDAP')); } diff --git a/src/applications/people/controller/PhabricatorPeopleLdapController.php b/src/applications/people/controller/PhabricatorPeopleLdapController.php index 1a7307301f..a2f02fc687 100644 --- a/src/applications/people/controller/PhabricatorPeopleLdapController.php +++ b/src/applications/people/controller/PhabricatorPeopleLdapController.php @@ -129,50 +129,65 @@ final class PhabricatorPeopleLdapController private function processSearchRequest($request) { $panel = new AphrontPanelView(); - $admin = $request->getUser(); - $username = $request->getStr('username'); - $password = $request->getStr('password'); - $search = $request->getStr('query'); + $search = $request->getStr('query'); - try { - $ldap_provider = new PhabricatorLDAPProvider(); - $envelope = new PhutilOpaqueEnvelope($password); - $ldap_provider->auth($username, $envelope); - $results = $ldap_provider->search($search); - foreach ($results as $key => $result) { - $results[$key][] = $this->renderUserInputs($result); + $ldap_provider = PhabricatorAuthProviderLDAP::getLDAPProvider(); + if (!$ldap_provider) { + throw new Exception("No LDAP provider enabled!"); + } + + $ldap_adapter = $ldap_provider->getAdapter(); + $ldap_adapter->setLoginUsername($request->getStr('username')); + $ldap_adapter->setLoginPassword( + new PhutilOpaqueEnvelope($request->getStr('password'))); + + // This causes us to connect and bind. + // TODO: Clean up this discard mode stuff. + DarkConsoleErrorLogPluginAPI::enableDiscardMode(); + $ldap_adapter->getAccountID(); + DarkConsoleErrorLogPluginAPI::disableDiscardMode(); + + $results = $ldap_adapter->searchLDAP('%Q', $search); + + foreach ($results as $key => $record) { + $account_id = $ldap_adapter->readLDAPRecordAccountID($record); + if (!$account_id) { + unset($results[$key]); + continue; } - $form = id(new AphrontFormView()) - ->setUser($admin); - - $table = new AphrontTableView($results); - $table->setHeaders( - array( - pht('Username'), - pht('Email'), - pht('Real Name'), - pht('Import?'), - )); - $form->appendChild($table); - $form->setAction($request->getRequestURI() - ->alter('import', 'true')->alter('search', null)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Import'))); - - - $panel->appendChild($form); - } catch (Exception $ex) { - $error_view = new AphrontErrorView(); - $error_view->setTitle(pht('LDAP Search Failed')); - $error_view->setErrors(array($ex->getMessage())); - return $error_view; + $info = array( + $account_id, + $ldap_adapter->readLDAPRecordEmail($record), + $ldap_adapter->readLDAPRecordRealName($record), + ); + $results[$key] = $info; + $results[$key][] = $this->renderUserInputs($info); } - return $panel; + $form = id(new AphrontFormView()) + ->setUser($admin); + + $table = new AphrontTableView($results); + $table->setHeaders( + array( + pht('Username'), + pht('Email'), + pht('Real Name'), + pht('Import?'), + )); + $form->appendChild($table); + $form->setAction($request->getRequestURI() + ->alter('import', 'true')->alter('search', null)) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Import'))); + + $panel->appendChild($form); + + return $panel; } private function renderUserInputs($user) { diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php index 3809679c94..6ff00ae041 100644 --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -96,7 +96,7 @@ final class PhamePost extends PhameDAO $options = array(); if ($current == 'facebook' || - PhabricatorEnv::getEnvConfig('facebook.application-id')) { + PhabricatorAuthProviderOAuthFacebook::getFacebookApplicationID()) { $options['facebook'] = 'Facebook'; } if ($current == 'disqus' || diff --git a/src/applications/phame/view/PhamePostView.php b/src/applications/phame/view/PhamePostView.php index 4f3b59fad8..0737efb109 100644 --- a/src/applications/phame/view/PhamePostView.php +++ b/src/applications/phame/view/PhamePostView.php @@ -150,7 +150,7 @@ final class PhamePostView extends AphrontView { } private function renderFacebookComments() { - $fb_id = PhabricatorEnv::getEnvConfig('facebook.application-id'); + $fb_id = PhabricatorAuthProviderOAuthFacebook::getFacebookApplicationID(); if (!$fb_id) { return null; } diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index a9ed3571b0..d6044e046d 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -247,6 +247,23 @@ final class PhabricatorEnv { } + /** + * Get the current configuration setting for a given key. If the key + * does not exist, return a default value instead of throwing. This is + * primarily useful for migrations involving keys which are slated for + * removal. + * + * @task read + */ + public static function getEnvConfigIfExists($key, $default = null) { + try { + return self::getEnvConfig($key); + } catch (Exception $ex) { + return $default; + } + } + + /** * Get the fully-qualified URI for a path. * diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 9fe259e33c..7311f4ae6b 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1374,6 +1374,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130613.authdb.sql'), ), + '20130619.authconf.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130619.authconf.php'), + ), ); } }