diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 51bc9338d2..cb44dd5585 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'e0cb8094', + 'core.pkg.css' => '7a73ffc5', 'core.pkg.js' => '5c737607', 'differential.pkg.css' => 'b8df73d4', 'differential.pkg.js' => '67c9ea4c', @@ -30,7 +30,7 @@ return array( 'rsrc/css/aphront/notification.css' => '30240bd2', 'rsrc/css/aphront/panel-view.css' => '46923d46', 'rsrc/css/aphront/phabricator-nav-view.css' => 'f8a0c1bf', - 'rsrc/css/aphront/table-view.css' => '76eda3f8', + 'rsrc/css/aphront/table-view.css' => 'daa1f9df', 'rsrc/css/aphront/tokenizer.css' => 'b52d0668', 'rsrc/css/aphront/tooltip.css' => 'e3f2412f', 'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2', @@ -133,7 +133,7 @@ return array( 'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e', 'rsrc/css/phui/object-item/phui-oi-list-view.css' => '909f3844', 'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46', - 'rsrc/css/phui/phui-action-list.css' => 'c1a7631d', + 'rsrc/css/phui/phui-action-list.css' => 'c4972757', 'rsrc/css/phui/phui-action-panel.css' => '6c386cbf', 'rsrc/css/phui/phui-badge.css' => '666e25ad', 'rsrc/css/phui/phui-basic-nav-view.css' => '56ebd66d', @@ -157,7 +157,7 @@ return array( 'rsrc/css/phui/phui-header-view.css' => '93cea4ec', 'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0', 'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec', - 'rsrc/css/phui/phui-icon.css' => '281f964d', + 'rsrc/css/phui/phui-icon.css' => '4cbc684a', 'rsrc/css/phui/phui-image-mask.css' => '62c7f4d2', 'rsrc/css/phui/phui-info-view.css' => '37b8d9ce', 'rsrc/css/phui/phui-invisible-character-view.css' => 'c694c4a4', @@ -519,7 +519,7 @@ return array( 'aphront-list-filter-view-css' => 'feb64255', 'aphront-multi-column-view-css' => 'fbc00ba3', 'aphront-panel-view-css' => '46923d46', - 'aphront-table-view-css' => '76eda3f8', + 'aphront-table-view-css' => 'daa1f9df', 'aphront-tokenizer-control-css' => 'b52d0668', 'aphront-tooltip-css' => 'e3f2412f', 'aphront-typeahead-control-css' => '8779483d', @@ -740,7 +740,7 @@ return array( 'path-typeahead' => 'ad486db3', 'people-picture-menu-item-css' => 'fe8e07cf', 'people-profile-css' => '2ea2daa1', - 'phabricator-action-list-view-css' => 'c1a7631d', + 'phabricator-action-list-view-css' => 'c4972757', 'phabricator-busy' => '5202e831', 'phabricator-chatlog-css' => 'abdc76ee', 'phabricator-content-source-view-css' => 'cdf0d579', @@ -823,7 +823,7 @@ return array( 'phui-hovercard' => '074f0783', 'phui-hovercard-view-css' => '6ca90fa0', 'phui-icon-set-selector-css' => '7aa5f3ec', - 'phui-icon-view-css' => '281f964d', + 'phui-icon-view-css' => '4cbc684a', 'phui-image-mask-css' => '62c7f4d2', 'phui-info-view-css' => '37b8d9ce', 'phui-inline-comment-view-css' => '48acce5b', diff --git a/resources/sql/autopatches/20190206.external.01.legalpad.sql b/resources/sql/autopatches/20190206.external.01.legalpad.sql new file mode 100644 index 0000000000..8afa9dd9ff --- /dev/null +++ b/resources/sql/autopatches/20190206.external.01.legalpad.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_legalpad.legalpad_documentsignature + SET signerPHID = NULL WHERE signerPHID LIKE 'PHID-XUSR-%'; diff --git a/resources/sql/autopatches/20190206.external.02.email.sql b/resources/sql/autopatches/20190206.external.02.email.sql new file mode 100644 index 0000000000..14f5f4791f --- /dev/null +++ b/resources/sql/autopatches/20190206.external.02.email.sql @@ -0,0 +1,2 @@ +DELETE FROM {$NAMESPACE}_user.user_externalaccount + WHERE accountType = 'email'; diff --git a/resources/sql/autopatches/20190207.packages.01.state.sql b/resources/sql/autopatches/20190207.packages.01.state.sql new file mode 100644 index 0000000000..0e74f269ba --- /dev/null +++ b/resources/sql/autopatches/20190207.packages.01.state.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD auditingState VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20190207.packages.02.migrate.sql b/resources/sql/autopatches/20190207.packages.02.migrate.sql new file mode 100644 index 0000000000..60bf364ac1 --- /dev/null +++ b/resources/sql/autopatches/20190207.packages.02.migrate.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_owners.owners_package + SET auditingState = IF(auditingEnabled = 0, 'none', 'audit'); diff --git a/resources/sql/autopatches/20190207.packages.03.drop.sql b/resources/sql/autopatches/20190207.packages.03.drop.sql new file mode 100644 index 0000000000..24d0ce1a4f --- /dev/null +++ b/resources/sql/autopatches/20190207.packages.03.drop.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + DROP auditingEnabled; diff --git a/resources/sql/autopatches/20190207.packages.04.xactions.php b/resources/sql/autopatches/20190207.packages.04.xactions.php new file mode 100644 index 0000000000..5a8609166e --- /dev/null +++ b/resources/sql/autopatches/20190207.packages.04.xactions.php @@ -0,0 +1,41 @@ +establishConnection('w'); +$iterator = new LiskRawMigrationIterator($conn, $table->getTableName()); + +// Migrate "Auditing State" transactions for Owners Packages from old values +// (which were "0" or "1", as JSON integer literals, without quotes) to new +// values (which are JSON strings, with quotes). + +foreach ($iterator as $row) { + if ($row['transactionType'] !== 'owners.auditing') { + continue; + } + + $old_value = (int)$row['oldValue']; + $new_value = (int)$row['newValue']; + + if (!$old_value) { + $old_value = 'none'; + } else { + $old_value = 'audit'; + } + + if (!$new_value) { + $new_value = 'none'; + } else { + $new_value = 'audit'; + } + + $old_value = phutil_json_encode($old_value); + $new_value = phutil_json_encode($new_value); + + queryfx( + $conn, + 'UPDATE %R SET oldValue = %s, newValue = %s WHERE id = %d', + $table, + $old_value, + $new_value, + $row['id']); +} diff --git a/resources/sql/patches/133.imagemacro.sql b/resources/sql/patches/133.imagemacro.sql index 01852c6b48..1477fd879f 100644 --- a/resources/sql/patches/133.imagemacro.sql +++ b/resources/sql/patches/133.imagemacro.sql @@ -1,2 +1,2 @@ -ALTER IGNORE TABLE `{$NAMESPACE}_file`.`file_imagemacro` - ADD UNIQUE `name` (`name`); +ALTER TABLE `{$NAMESPACE}_file`.`file_imagemacro` + ADD UNIQUE KEY `name` (`name`); diff --git a/resources/sql/patches/20130611.migrateoauth.php b/resources/sql/patches/20130611.migrateoauth.php index 3622b2772e..92fe854cfd 100644 --- a/resources/sql/patches/20130611.migrateoauth.php +++ b/resources/sql/patches/20130611.migrateoauth.php @@ -1,66 +1,14 @@ establishConnection('w'); $table_name = 'user_oauthinfo'; -$conn_w = $table->establishConnection('w'); -$xaccount = new PhabricatorExternalAccount(); - -echo pht('Migrating OAuth to %s...', 'ExternalAccount')."\n"; - -$domain_map = array( - 'disqus' => 'disqus.com', - 'facebook' => 'facebook.com', - 'github' => 'github.com', - 'google' => 'google.com', -); - -try { - $phabricator_oauth_uri = new PhutilURI( - PhabricatorEnv::getEnvConfig('phabricator.oauth-uri')); - $domain_map['phabricator'] = $phabricator_oauth_uri->getDomain(); -} catch (Exception $ex) { - // Ignore; this likely indicates that we have removed `phabricator.oauth-uri` - // in some future diff. +foreach (new LiskRawMigrationIterator($conn, $table_name) as $row) { + throw new Exception( + pht( + 'Your Phabricator install has ancient OAuth account data and is '. + 'too old to upgrade directly to a modern version of Phabricator. '. + 'Upgrade to a version released between June 2013 and February 2019 '. + 'first, then upgrade to a modern version.')); } - -$rows = queryfx_all( - $conn_w, - 'SELECT * FROM user_oauthinfo'); -foreach ($rows as $row) { - echo pht('Migrating row ID #%d.', $row['id'])."\n"; - $user = id(new PhabricatorUser())->loadOneWhere( - 'id = %d', - $row['userID']); - if (!$user) { - echo pht('Bad user ID!')."\n"; - continue; - } - - $domain = idx($domain_map, $row['oauthProvider']); - if (empty($domain)) { - echo pht('Unknown OAuth provider!')."\n"; - continue; - } - - - $xaccount = id(new PhabricatorExternalAccount()) - ->setUserPHID($user->getPHID()) - ->setAccountType($row['oauthProvider']) - ->setAccountDomain($domain) - ->setAccountID($row['oauthUID']) - ->setAccountURI($row['accountURI']) - ->setUsername($row['accountName']) - ->setDateCreated($row['dateCreated']); - - try { - $xaccount->save(); - } catch (Exception $ex) { - phlog($ex); - } -} - -echo pht('Done.')."\n"; diff --git a/resources/sql/patches/20130611.nukeldap.php b/resources/sql/patches/20130611.nukeldap.php index 3f225cfa84..0f0b976a58 100644 --- a/resources/sql/patches/20130611.nukeldap.php +++ b/resources/sql/patches/20130611.nukeldap.php @@ -1,41 +1,14 @@ establishConnection('w'); $table_name = 'user_ldapinfo'; -$conn_w = $table->establishConnection('w'); -$xaccount = new PhabricatorExternalAccount(); - -echo pht('Migrating LDAP to %s...', 'ExternalAccount')."\n"; - -$rows = queryfx_all($conn_w, 'SELECT * FROM %T', $table_name); -foreach ($rows as $row) { - echo pht('Migrating row ID #%d.', $row['id'])."\n"; - $user = id(new PhabricatorUser())->loadOneWhere( - 'id = %d', - $row['userID']); - if (!$user) { - echo pht('Bad user ID!')."\n"; - continue; - } - - - $xaccount = id(new PhabricatorExternalAccount()) - ->setUserPHID($user->getPHID()) - ->setAccountType('ldap') - ->setAccountDomain('self') - ->setAccountID($row['ldapUsername']) - ->setUsername($row['ldapUsername']) - ->setDateCreated($row['dateCreated']); - - try { - $xaccount->save(); - } catch (Exception $ex) { - phlog($ex); - } +foreach (new LiskRawMigrationIterator($conn, $table_name) as $row) { + throw new Exception( + pht( + 'Your Phabricator install has ancient LDAP account data and is '. + 'too old to upgrade directly to a modern version of Phabricator. '. + 'Upgrade to a version released between June 2013 and February 2019 '. + 'first, then upgrade to a modern version.')); } - -echo pht('Done.')."\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 324fc3c5de..b704a7cc2e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2272,7 +2272,6 @@ phutil_register_library_map(array( 'PhabricatorAuthLinkController' => 'applications/auth/controller/PhabricatorAuthLinkController.php', 'PhabricatorAuthListController' => 'applications/auth/controller/config/PhabricatorAuthListController.php', 'PhabricatorAuthLoginController' => 'applications/auth/controller/PhabricatorAuthLoginController.php', - 'PhabricatorAuthLoginHandler' => 'applications/auth/handler/PhabricatorAuthLoginHandler.php', 'PhabricatorAuthLoginMessageType' => 'applications/auth/message/PhabricatorAuthLoginMessageType.php', 'PhabricatorAuthLogoutConduitAPIMethod' => 'applications/auth/conduit/PhabricatorAuthLogoutConduitAPIMethod.php', 'PhabricatorAuthMFAEditEngineExtension' => 'applications/auth/engineextension/PhabricatorAuthMFAEditEngineExtension.php', @@ -2335,6 +2334,7 @@ phutil_register_library_map(array( 'PhabricatorAuthProviderConfigTransaction' => 'applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php', 'PhabricatorAuthProviderConfigTransactionQuery' => 'applications/auth/query/PhabricatorAuthProviderConfigTransactionQuery.php', 'PhabricatorAuthProviderController' => 'applications/auth/controller/config/PhabricatorAuthProviderController.php', + 'PhabricatorAuthProviderViewController' => 'applications/auth/controller/config/PhabricatorAuthProviderViewController.php', 'PhabricatorAuthProvidersGuidanceContext' => 'applications/auth/guidance/PhabricatorAuthProvidersGuidanceContext.php', 'PhabricatorAuthProvidersGuidanceEngineExtension' => 'applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php', 'PhabricatorAuthQueryPublicKeysConduitAPIMethod' => 'applications/auth/conduit/PhabricatorAuthQueryPublicKeysConduitAPIMethod.php', @@ -3552,6 +3552,8 @@ phutil_register_library_map(array( 'PhabricatorMetaMTASchemaSpec' => 'applications/metamta/storage/PhabricatorMetaMTASchemaSpec.php', 'PhabricatorMetaMTASendGridReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php', 'PhabricatorMetaMTAWorker' => 'applications/metamta/PhabricatorMetaMTAWorker.php', + 'PhabricatorMetronome' => 'infrastructure/util/PhabricatorMetronome.php', + 'PhabricatorMetronomeTestCase' => 'infrastructure/util/__tests__/PhabricatorMetronomeTestCase.php', 'PhabricatorMetronomicTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorMetronomicTriggerClock.php', 'PhabricatorModularTransaction' => 'applications/transactions/storage/PhabricatorModularTransaction.php', 'PhabricatorModularTransactionType' => 'applications/transactions/storage/PhabricatorModularTransactionType.php', @@ -3666,6 +3668,7 @@ phutil_register_library_map(array( 'PhabricatorOwnerPathQuery' => 'applications/owners/query/PhabricatorOwnerPathQuery.php', 'PhabricatorOwnersApplication' => 'applications/owners/application/PhabricatorOwnersApplication.php', 'PhabricatorOwnersArchiveController' => 'applications/owners/controller/PhabricatorOwnersArchiveController.php', + 'PhabricatorOwnersAuditRule' => 'applications/owners/constants/PhabricatorOwnersAuditRule.php', 'PhabricatorOwnersConfigOptions' => 'applications/owners/config/PhabricatorOwnersConfigOptions.php', 'PhabricatorOwnersConfiguredCustomField' => 'applications/owners/customfield/PhabricatorOwnersConfiguredCustomField.php', 'PhabricatorOwnersController' => 'applications/owners/controller/PhabricatorOwnersController.php', @@ -3897,6 +3900,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleTransactionQuery' => 'applications/people/query/PhabricatorPeopleTransactionQuery.php', 'PhabricatorPeopleUserFunctionDatasource' => 'applications/people/typeahead/PhabricatorPeopleUserFunctionDatasource.php', 'PhabricatorPeopleUserPHIDType' => 'applications/people/phid/PhabricatorPeopleUserPHIDType.php', + 'PhabricatorPeopleUsernameMailEngine' => 'applications/people/mail/PhabricatorPeopleUsernameMailEngine.php', 'PhabricatorPeopleWelcomeController' => 'applications/people/controller/PhabricatorPeopleWelcomeController.php', 'PhabricatorPeopleWelcomeMailEngine' => 'applications/people/mail/PhabricatorPeopleWelcomeMailEngine.php', 'PhabricatorPhabricatorAuthProvider' => 'applications/auth/provider/PhabricatorPhabricatorAuthProvider.php', @@ -5011,6 +5015,7 @@ phutil_register_library_map(array( 'PhortuneCurrencySerializer' => 'applications/phortune/currency/PhortuneCurrencySerializer.php', 'PhortuneCurrencyTestCase' => 'applications/phortune/currency/__tests__/PhortuneCurrencyTestCase.php', 'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php', + 'PhortuneDisplayException' => 'applications/phortune/exception/PhortuneDisplayException.php', 'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php', 'PhortuneInvoiceView' => 'applications/phortune/view/PhortuneInvoiceView.php', 'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php', @@ -8016,7 +8021,6 @@ phutil_register_library_map(array( 'PhabricatorAuthLinkController' => 'PhabricatorAuthController', 'PhabricatorAuthListController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthLoginController' => 'PhabricatorAuthController', - 'PhabricatorAuthLoginHandler' => 'Phobject', 'PhabricatorAuthLoginMessageType' => 'PhabricatorAuthMessageType', 'PhabricatorAuthLogoutConduitAPIMethod' => 'PhabricatorAuthConduitAPIMethod', 'PhabricatorAuthMFAEditEngineExtension' => 'PhabricatorEditEngineExtension', @@ -8092,6 +8096,7 @@ phutil_register_library_map(array( 'PhabricatorAuthProviderConfigTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorAuthProviderController' => 'PhabricatorAuthController', + 'PhabricatorAuthProviderViewController' => 'PhabricatorAuthProviderConfigController', 'PhabricatorAuthProvidersGuidanceContext' => 'PhabricatorGuidanceContext', 'PhabricatorAuthProvidersGuidanceEngineExtension' => 'PhabricatorGuidanceEngineExtension', 'PhabricatorAuthQueryPublicKeysConduitAPIMethod' => 'PhabricatorAuthConduitAPIMethod', @@ -9477,6 +9482,8 @@ phutil_register_library_map(array( 'PhabricatorMetaMTASchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorMetaMTASendGridReceiveController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAWorker' => 'PhabricatorWorker', + 'PhabricatorMetronome' => 'Phobject', + 'PhabricatorMetronomeTestCase' => 'PhabricatorTestCase', 'PhabricatorMetronomicTriggerClock' => 'PhabricatorTriggerClock', 'PhabricatorModularTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorModularTransactionType' => 'Phobject', @@ -9608,6 +9615,7 @@ phutil_register_library_map(array( 'PhabricatorOwnerPathQuery' => 'Phobject', 'PhabricatorOwnersApplication' => 'PhabricatorApplication', 'PhabricatorOwnersArchiveController' => 'PhabricatorOwnersController', + 'PhabricatorOwnersAuditRule' => 'Phobject', 'PhabricatorOwnersConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorOwnersConfiguredCustomField' => array( 'PhabricatorOwnersCustomField', @@ -9891,6 +9899,7 @@ phutil_register_library_map(array( 'PhabricatorPeopleTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorPeopleUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorPeopleUserPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorPeopleUsernameMailEngine' => 'PhabricatorPeopleMailEngine', 'PhabricatorPeopleWelcomeController' => 'PhabricatorPeopleController', 'PhabricatorPeopleWelcomeMailEngine' => 'PhabricatorPeopleMailEngine', 'PhabricatorPhabricatorAuthProvider' => 'PhabricatorOAuth2AuthProvider', @@ -11255,6 +11264,7 @@ phutil_register_library_map(array( 'PhortuneCurrencySerializer' => 'PhabricatorLiskSerializer', 'PhortuneCurrencyTestCase' => 'PhabricatorTestCase', 'PhortuneDAO' => 'PhabricatorLiskDAO', + 'PhortuneDisplayException' => 'Exception', 'PhortuneErrCode' => 'PhortuneConstants', 'PhortuneInvoiceView' => 'AphrontTagView', 'PhortuneLandingController' => 'PhortuneController', diff --git a/src/applications/almanac/storage/AlmanacModularTransaction.php b/src/applications/almanac/storage/AlmanacModularTransaction.php index 6497069241..3e2eb0a3ec 100644 --- a/src/applications/almanac/storage/AlmanacModularTransaction.php +++ b/src/applications/almanac/storage/AlmanacModularTransaction.php @@ -7,8 +7,4 @@ abstract class AlmanacModularTransaction return 'almanac'; } - public function getApplicationTransactionCommentObject() { - return null; - } - } diff --git a/src/applications/auth/application/PhabricatorAuthApplication.php b/src/applications/auth/application/PhabricatorAuthApplication.php index df86595b46..307cab61c8 100644 --- a/src/applications/auth/application/PhabricatorAuthApplication.php +++ b/src/applications/auth/application/PhabricatorAuthApplication.php @@ -48,10 +48,10 @@ final class PhabricatorAuthApplication extends PhabricatorApplication { '' => 'PhabricatorAuthListController', 'config/' => array( 'new/' => 'PhabricatorAuthNewController', - 'new/(?P[^/]+)/' => 'PhabricatorAuthEditController', - 'edit/(?P\d+)/' => 'PhabricatorAuthEditController', + 'edit/(?:(?P\d+)/)?' => 'PhabricatorAuthEditController', '(?Penable|disable)/(?P\d+)/' => 'PhabricatorAuthDisableController', + 'view/(?P\d+)/' => 'PhabricatorAuthProviderViewController', ), 'login/(?P[^/]+)/(?:(?P[^/]+)/)?' => 'PhabricatorAuthLoginController', diff --git a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php index 0cac95f53d..f51f379d2b 100644 --- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php @@ -119,38 +119,9 @@ final class PhabricatorAuthOneTimeLoginController } unset($unguarded); - $next = '/'; - if (!PhabricatorPasswordAuthProvider::getPasswordProvider()) { - $next = '/settings/panel/external/'; - } else { + $next_uri = $this->getNextStepURI($target_user); - // We're going to let the user reset their password without knowing - // the old one. Generate a one-time token for that. - $key = Filesystem::readRandomCharacters(16); - $password_type = - PhabricatorAuthPasswordResetTemporaryTokenType::TOKENTYPE; - - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - id(new PhabricatorAuthTemporaryToken()) - ->setTokenResource($target_user->getPHID()) - ->setTokenType($password_type) - ->setTokenExpires(time() + phutil_units('1 hour in seconds')) - ->setTokenCode(PhabricatorHash::weakDigest($key)) - ->save(); - unset($unguarded); - - $panel_uri = '/auth/password/'; - - $next = (string)id(new PhutilURI($panel_uri)) - ->setQueryParams( - array( - 'key' => $key, - )); - - $request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes'); - } - - PhabricatorCookies::setNextURICookie($request, $next, $force = true); + PhabricatorCookies::setNextURICookie($request, $next_uri, $force = true); $force_full_session = false; if ($link_type === PhabricatorAuthSessionEngine::ONETIME_RECOVER) { @@ -206,4 +177,57 @@ final class PhabricatorAuthOneTimeLoginController return id(new AphrontDialogResponse())->setDialog($dialog); } + + private function getNextStepURI(PhabricatorUser $user) { + $request = $this->getRequest(); + + // If we have password auth, let the user set or reset their password after + // login. + $have_passwords = PhabricatorPasswordAuthProvider::getPasswordProvider(); + if ($have_passwords) { + // We're going to let the user reset their password without knowing + // the old one. Generate a one-time token for that. + $key = Filesystem::readRandomCharacters(16); + $password_type = + PhabricatorAuthPasswordResetTemporaryTokenType::TOKENTYPE; + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + id(new PhabricatorAuthTemporaryToken()) + ->setTokenResource($user->getPHID()) + ->setTokenType($password_type) + ->setTokenExpires(time() + phutil_units('1 hour in seconds')) + ->setTokenCode(PhabricatorHash::weakDigest($key)) + ->save(); + unset($unguarded); + + $panel_uri = '/auth/password/'; + + $request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes'); + + return (string)id(new PhutilURI($panel_uri)) + ->setQueryParams( + array( + 'key' => $key, + )); + } + + $providers = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($user) + ->withIsEnabled(true) + ->execute(); + + // If there are no configured providers and the user is an administrator, + // send them to Auth to configure a provider. This is probably where they + // want to go. You can end up in this state by accidentally losing your + // first session during initial setup, or after restoring exported data + // from a hosted instance. + if (!$providers && $user->getIsAdmin()) { + return '/auth/'; + } + + // If we didn't find anywhere better to send them, give up and just send + // them to the home page. + return '/'; + } + } diff --git a/src/applications/auth/controller/PhabricatorAuthStartController.php b/src/applications/auth/controller/PhabricatorAuthStartController.php index 29fa7e0b9f..0b823098d7 100644 --- a/src/applications/auth/controller/PhabricatorAuthStartController.php +++ b/src/applications/auth/controller/PhabricatorAuthStartController.php @@ -75,6 +75,11 @@ final class PhabricatorAuthStartController } } + $configs = array(); + foreach ($providers as $provider) { + $configs[] = $provider->getProviderConfig(); + } + if (!$providers) { if ($this->isFirstTimeSetup()) { // If this is a fresh install, let the user register their admin @@ -172,23 +177,6 @@ final class PhabricatorAuthStartController $button_columns); } - $handlers = PhabricatorAuthLoginHandler::getAllHandlers(); - - $delegating_controller = $this->getDelegatingController(); - - $header = array(); - foreach ($handlers as $handler) { - $handler = clone $handler; - - $handler->setRequest($request); - - if ($delegating_controller) { - $handler->setDelegatingController($delegating_controller); - } - - $header[] = $handler->getAuthLoginHeaderContent(); - } - $invite_message = null; if ($invite) { $invite_message = $this->renderInviteHeader($invite); @@ -196,16 +184,18 @@ final class PhabricatorAuthStartController $custom_message = $this->newCustomStartMessage(); + $email_login = $this->newEmailLoginView($configs); + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Login')); $crumbs->setBorder(true); $title = pht('Login'); $view = array( - $header, $invite_message, $custom_message, $out, + $email_login, ); return $this->newPage() @@ -329,4 +319,43 @@ final class PhabricatorAuthStartController $remarkup_view); } + private function newEmailLoginView(array $configs) { + assert_instances_of($configs, 'PhabricatorAuthProviderConfig'); + + // Check if password auth is enabled. If it is, the password login form + // renders a "Forgot password?" link, so we don't need to provide a + // supplemental link. + + $has_password = false; + foreach ($configs as $config) { + $provider = $config->getProvider(); + if ($provider instanceof PhabricatorPasswordAuthProvider) { + $has_password = true; + } + } + + if ($has_password) { + return null; + } + + $view = array( + pht('Trouble logging in?'), + ' ', + phutil_tag( + 'a', + array( + 'href' => '/login/email/', + ), + pht('Send a login link to your email address.')), + ); + + return phutil_tag( + 'div', + array( + 'class' => 'auth-custom-message', + ), + $view); + } + + } diff --git a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php index e6e1493e5a..1e3023e5d2 100644 --- a/src/applications/auth/controller/PhabricatorAuthUnlinkController.php +++ b/src/applications/auth/controller/PhabricatorAuthUnlinkController.php @@ -32,8 +32,15 @@ final class PhabricatorAuthUnlinkController } } - // Check that this account isn't the last account which can be used to - // login. We prevent you from removing the last account. + $confirmations = $request->getStrList('confirmations'); + $confirmations = array_fuse($confirmations); + + if (!$request->isFormPost() || !isset($confirmations['unlink'])) { + return $this->renderConfirmDialog($confirmations); + } + + // Check that this account isn't the only account which can be used to + // login. We warn you when you remove your only login account. if ($account->isUsableForLogin()) { $other_accounts = id(new PhabricatorExternalAccount())->loadAllWhere( 'userPHID = %s', @@ -47,22 +54,20 @@ final class PhabricatorAuthUnlinkController } if ($valid_accounts < 2) { - return $this->renderLastUsableAccountErrorDialog(); + if (!isset($confirmations['only'])) { + return $this->renderOnlyUsableAccountConfirmDialog($confirmations); + } } } - if ($request->isDialogFormPost()) { - $account->delete(); + $account->delete(); - id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( - $viewer, - new PhutilOpaqueEnvelope( - $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); + id(new PhabricatorAuthSessionEngine())->terminateLoginSessions( + $viewer, + new PhutilOpaqueEnvelope( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION))); - return id(new AphrontRedirectResponse())->setURI($this->getDoneURI()); - } - - return $this->renderConfirmDialog(); + return id(new AphrontRedirectResponse())->setURI($this->getDoneURI()); } private function getDoneURI() { @@ -97,22 +102,27 @@ final class PhabricatorAuthUnlinkController return id(new AphrontDialogResponse())->setDialog($dialog); } - private function renderLastUsableAccountErrorDialog() { - $dialog = id(new AphrontDialogView()) - ->setUser($this->getRequest()->getUser()) - ->setTitle(pht('Last Valid Account')) - ->appendChild( - pht( - 'You can not unlink this account because you have no other '. - 'valid login accounts. If you removed it, you would be unable '. - 'to log in. Add another authentication method before removing '. - 'this one.')) - ->addCancelButton($this->getDoneURI()); + private function renderOnlyUsableAccountConfirmDialog(array $confirmations) { + $confirmations[] = 'only'; - return id(new AphrontDialogResponse())->setDialog($dialog); + return $this->newDialog() + ->setTitle(pht('Unlink Your Only Login Account?')) + ->addHiddenInput('confirmations', implode(',', $confirmations)) + ->appendParagraph( + pht( + 'This is the only external login account linked to your Phabicator '. + 'account. If you remove it, you may no longer be able to log in.')) + ->appendParagraph( + pht( + 'If you lose access to your account, you can recover access by '. + 'sending yourself an email login link from the login screen.')) + ->addCancelButton($this->getDoneURI()) + ->addSubmitButton(pht('Unlink External Account')); } - private function renderConfirmDialog() { + private function renderConfirmDialog(array $confirmations) { + $confirmations[] = 'unlink'; + $provider_key = $this->providerKey; $provider = PhabricatorAuthProvider::getEnabledProviderByKey($provider_key); @@ -129,9 +139,9 @@ final class PhabricatorAuthUnlinkController 'to Phabricator.'); } - $dialog = id(new AphrontDialogView()) - ->setUser($this->getRequest()->getUser()) + return $this->newDialog() ->setTitle($title) + ->addHiddenInput('confirmations', implode(',', $confirmations)) ->appendParagraph($body) ->appendParagraph( pht( @@ -139,8 +149,6 @@ final class PhabricatorAuthUnlinkController 'other active login sessions.')) ->addSubmitButton(pht('Unlink Account')) ->addCancelButton($this->getDoneURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/auth/controller/PhabricatorEmailLoginController.php b/src/applications/auth/controller/PhabricatorEmailLoginController.php index f57a29b11a..eef30e6989 100644 --- a/src/applications/auth/controller/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/PhabricatorEmailLoginController.php @@ -8,17 +8,13 @@ final class PhabricatorEmailLoginController } public function handleRequest(AphrontRequest $request) { - - if (!PhabricatorPasswordAuthProvider::getPasswordProvider()) { - return new Aphront400Response(); - } + $viewer = $this->getViewer(); $e_email = true; $e_captcha = true; $errors = array(); - $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); - + $v_email = $request->getStr('email'); if ($request->isFormPost()) { $e_email = null; $e_captcha = pht('Again'); @@ -29,8 +25,7 @@ final class PhabricatorEmailLoginController $e_captcha = pht('Invalid'); } - $email = $request->getStr('email'); - if (!strlen($email)) { + if (!strlen($v_email)) { $errors[] = pht('You must provide an email address.'); $e_email = pht('Required'); } @@ -42,7 +37,7 @@ final class PhabricatorEmailLoginController $target_email = id(new PhabricatorUserEmail())->loadOneWhere( 'address = %s', - $email); + $v_email); $target_user = null; if ($target_email) { @@ -81,33 +76,10 @@ final class PhabricatorEmailLoginController } if (!$errors) { - $engine = new PhabricatorAuthSessionEngine(); - $uri = $engine->getOneTimeLoginURI( - $target_user, - null, - PhabricatorAuthSessionEngine::ONETIME_RESET); - - if ($is_serious) { - $body = pht( - "You can use this link to reset your Phabricator password:". - "\n\n %s\n", - $uri); - } else { - $body = pht( - "Condolences on forgetting your password. You can use this ". - "link to reset it:\n\n". - " %s\n\n". - "After you set a new password, consider writing it down on a ". - "sticky note and attaching it to your monitor so you don't ". - "forget again! Choosing a very short, easy-to-remember password ". - "like \"cat\" or \"1234\" might also help.\n\n". - "Best Wishes,\nPhabricator\n", - $uri); - - } + $body = $this->newAccountLoginMailBody($target_user); $mail = id(new PhabricatorMetaMTAMail()) - ->setSubject(pht('[Phabricator] Password Reset')) + ->setSubject(pht('[Phabricator] Account Login Link')) ->setForceDelivery(true) ->addRawTos(array($target_email->getAddress())) ->setBody($body) @@ -123,44 +95,90 @@ final class PhabricatorEmailLoginController } } - $error_view = null; - if ($errors) { - $error_view = new PHUIInfoView(); - $error_view->setErrors($errors); + $form = id(new AphrontFormView()) + ->setViewer($viewer); + + if ($this->isPasswordAuthEnabled()) { + $form->appendRemarkupInstructions( + pht( + 'To reset your password, provide your email address. An email '. + 'with a login link will be sent to you.')); + } else { + $form->appendRemarkupInstructions( + pht( + 'To access your account, provide your email address. An email '. + 'with a login link will be sent to you.')); } - $email_auth = new PHUIFormLayoutView(); - $email_auth->appendChild($error_view); - $email_auth - ->setUser($request->getUser()) - ->setFullWidth(true) - ->appendChild( + $form + ->appendControl( id(new AphrontFormTextControl()) - ->setLabel(pht('Email')) + ->setLabel(pht('Email Address')) ->setName('email') - ->setValue($request->getStr('email')) + ->setValue($v_email) ->setError($e_email)) - ->appendChild( + ->appendControl( id(new AphrontFormRecaptchaControl()) ->setLabel(pht('Captcha')) ->setError($e_captcha)); - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Reset Password')); - $crumbs->setBorder(true); - - $dialog = new AphrontDialogView(); - $dialog->setUser($request->getUser()); - $dialog->setTitle(pht('Forgot Password / Email Login')); - $dialog->appendChild($email_auth); - $dialog->addSubmitButton(pht('Send Email')); - $dialog->setSubmitURI('/login/email/'); - - return $this->newPage() - ->setTitle(pht('Forgot Password')) - ->setCrumbs($crumbs) - ->appendChild($dialog); + if ($this->isPasswordAuthEnabled()) { + $title = pht('Password Reset'); + } else { + $title = pht('Email Login'); + } + return $this->newDialog() + ->setTitle($title) + ->setErrors($errors) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->appendForm($form) + ->addCancelButton('/auth/start/') + ->addSubmitButton(pht('Send Email')); } + private function newAccountLoginMailBody(PhabricatorUser $user) { + $engine = new PhabricatorAuthSessionEngine(); + $uri = $engine->getOneTimeLoginURI( + $user, + null, + PhabricatorAuthSessionEngine::ONETIME_RESET); + + $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business'); + $have_passwords = $this->isPasswordAuthEnabled(); + + if ($have_passwords) { + if ($is_serious) { + $body = pht( + "You can use this link to reset your Phabricator password:". + "\n\n %s\n", + $uri); + } else { + $body = pht( + "Condolences on forgetting your password. You can use this ". + "link to reset it:\n\n". + " %s\n\n". + "After you set a new password, consider writing it down on a ". + "sticky note and attaching it to your monitor so you don't ". + "forget again! Choosing a very short, easy-to-remember password ". + "like \"cat\" or \"1234\" might also help.\n\n". + "Best Wishes,\nPhabricator\n", + $uri); + + } + } else { + $body = pht( + "You can use this login link to regain access to your Phabricator ". + "account:". + "\n\n". + " %s\n", + $uri); + } + + return $body; + } + + private function isPasswordAuthEnabled() { + return (bool)PhabricatorPasswordAuthProvider::getPasswordProvider(); + } } diff --git a/src/applications/auth/controller/config/PhabricatorAuthDisableController.php b/src/applications/auth/controller/config/PhabricatorAuthDisableController.php index 5863aceca9..252f159ec4 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthDisableController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthDisableController.php @@ -6,7 +6,8 @@ final class PhabricatorAuthDisableController public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $viewer = $request->getUser(); + + $viewer = $this->getViewer(); $config_id = $request->getURIData('id'); $action = $request->getURIData('action'); @@ -24,6 +25,7 @@ final class PhabricatorAuthDisableController } $is_enable = ($action === 'enable'); + $done_uri = $config->getURI(); if ($request->isDialogFormPost()) { $xactions = array(); @@ -39,8 +41,7 @@ final class PhabricatorAuthDisableController ->setContinueOnNoEffect(true) ->applyTransactions($config, $xactions); - return id(new AphrontRedirectResponse())->setURI( - $this->getApplicationURI()); + return id(new AphrontRedirectResponse())->setURI($done_uri); } if ($is_enable) { @@ -64,8 +65,9 @@ final class PhabricatorAuthDisableController // account and pop a warning like "YOU WILL NO LONGER BE ABLE TO LOGIN // YOU GOOF, YOU PROBABLY DO NOT MEAN TO DO THIS". None of this is // critical and we can wait to see how users manage to shoot themselves - // in the feet. Shortly, `bin/auth` will be able to recover from these - // types of mistakes. + // in the feet. + + // `bin/auth` can recover from these types of mistakes. $title = pht('Disable Provider?'); $body = pht( @@ -77,14 +79,11 @@ final class PhabricatorAuthDisableController $button = pht('Disable Provider'); } - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) + return $this->newDialog() ->setTitle($title) ->appendChild($body) - ->addCancelButton($this->getApplicationURI()) + ->addCancelButton($done_uri) ->addSubmitButton($button); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 6ff0be4383..d3cd2fef98 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -6,8 +6,9 @@ final class PhabricatorAuthEditController public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $viewer = $request->getUser(); - $provider_class = $request->getURIData('className'); + + $viewer = $this->getViewer(); + $provider_class = $request->getStr('provider'); $config_id = $request->getURIData('id'); if ($config_id) { @@ -155,12 +156,7 @@ final class PhabricatorAuthEditController ->setContinueOnNoEffect(true) ->applyTransactions($config, $xactions); - if ($provider->hasSetupStep() && $is_new) { - $id = $config->getID(); - $next_uri = $this->getApplicationURI('config/edit/'.$id.'/'); - } else { - $next_uri = $this->getApplicationURI(); - } + $next_uri = $config->getURI(); return id(new AphrontRedirectResponse())->setURI($next_uri); } @@ -184,7 +180,7 @@ final class PhabricatorAuthEditController $crumb = pht('Edit Provider'); $title = pht('Edit Auth Provider'); $header_icon = 'fa-pencil'; - $cancel_uri = $this->getApplicationURI(); + $cancel_uri = $config->getURI(); } $header = id(new PHUIHeaderView()) @@ -275,6 +271,7 @@ final class PhabricatorAuthEditController $form = id(new AphrontFormView()) ->setUser($viewer) + ->addHiddenInput('provider', $provider_class) ->appendChild( id(new AphrontFormCheckboxControl()) ->setLabel(pht('Allow')) @@ -346,18 +343,6 @@ final class PhabricatorAuthEditController $crumbs->addTextCrumb($crumb); $crumbs->setBorder(true); - $timeline = null; - if (!$is_new) { - $timeline = $this->buildTransactionTimeline( - $config, - new PhabricatorAuthProviderConfigTransactionQuery()); - $xactions = $timeline->getTransactions(); - foreach ($xactions as $xaction) { - $xaction->setProvider($provider); - } - $timeline->setShouldTerminate(true); - } - $form_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Provider')) ->setFormErrors($errors) @@ -369,7 +354,6 @@ final class PhabricatorAuthEditController ->setFooter(array( $form_box, $footer, - $timeline, )); return $this->newPage() diff --git a/src/applications/auth/controller/config/PhabricatorAuthListController.php b/src/applications/auth/controller/config/PhabricatorAuthListController.php index bb118d798e..f4b05e8adf 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthListController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthListController.php @@ -19,31 +19,18 @@ final class PhabricatorAuthListController $id = $config->getID(); - $edit_uri = $this->getApplicationURI('config/edit/'.$id.'/'); - $enable_uri = $this->getApplicationURI('config/enable/'.$id.'/'); - $disable_uri = $this->getApplicationURI('config/disable/'.$id.'/'); + $view_uri = $config->getURI(); $provider = $config->getProvider(); - if ($provider) { - $name = $provider->getProviderName(); - } else { - $name = $config->getProviderType().' ('.$config->getProviderClass().')'; - } + $name = $provider->getProviderName(); - $item->setHeader($name); + $item + ->setHeader($name) + ->setHref($view_uri); - if ($provider) { - $item->setHref($edit_uri); - } else { - $item->addAttribute(pht('Provider Implementation Missing!')); - } - - $domain = null; - if ($provider) { - $domain = $provider->getProviderDomain(); - if ($domain !== 'self') { - $item->addAttribute($domain); - } + $domain = $provider->getProviderDomain(); + if ($domain !== 'self') { + $item->addAttribute($domain); } if ($config->getShouldAllowRegistration()) { @@ -54,21 +41,9 @@ final class PhabricatorAuthListController if ($config->getIsEnabled()) { $item->setStatusIcon('fa-check-circle green'); - $item->addAction( - id(new PHUIListItemView()) - ->setIcon('fa-times') - ->setHref($disable_uri) - ->setDisabled(!$can_manage) - ->addSigil('workflow')); } else { $item->setStatusIcon('fa-ban red'); $item->addIcon('fa-ban grey', pht('Disabled')); - $item->addAction( - id(new PHUIListItemView()) - ->setIcon('fa-plus') - ->setHref($enable_uri) - ->setDisabled(!$can_manage) - ->addSigil('workflow')); } $list->addItem($item); @@ -123,10 +98,11 @@ final class PhabricatorAuthListController $view = id(new PHUITwoColumnView()) ->setHeader($header) - ->setFooter(array( - $guidance, - $list, - )); + ->setFooter( + array( + $guidance, + $list, + )); $nav = $this->newNavigation() ->setCrumbs($crumbs) diff --git a/src/applications/auth/controller/config/PhabricatorAuthNewController.php b/src/applications/auth/controller/config/PhabricatorAuthNewController.php index dbd43f9ea8..c8fd0ad8a5 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthNewController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthNewController.php @@ -6,44 +6,12 @@ final class PhabricatorAuthNewController public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( AuthManageProvidersCapability::CAPABILITY); - $request = $this->getRequest(); - $viewer = $request->getUser(); + + $viewer = $this->getViewer(); + $cancel_uri = $this->getApplicationURI(); $providers = PhabricatorAuthProvider::getAllBaseProviders(); - $e_provider = null; - $errors = array(); - - if ($request->isFormPost()) { - $provider_string = $request->getStr('provider'); - if (!strlen($provider_string)) { - $e_provider = pht('Required'); - $errors[] = pht('You must select an authentication provider.'); - } else { - $found = false; - foreach ($providers as $provider) { - if (get_class($provider) === $provider_string) { - $found = true; - break; - } - } - if (!$found) { - $e_provider = pht('Invalid'); - $errors[] = pht('You must select a valid provider.'); - } - } - - if (!$errors) { - return id(new AphrontRedirectResponse())->setURI( - $this->getApplicationURI('/config/new/'.$provider_string.'/')); - } - } - - $options = id(new AphrontFormRadioButtonControl()) - ->setLabel(pht('Provider')) - ->setName('provider') - ->setError($e_provider); - $configured = PhabricatorAuthProvider::getAllProviders(); $configured_classes = array(); foreach ($configured as $configured_provider) { @@ -55,57 +23,52 @@ final class PhabricatorAuthNewController $providers = msort($providers, 'getLoginOrder'); $providers = array_diff_key($providers, $configured_classes) + $providers; - foreach ($providers as $provider) { - if (isset($configured_classes[get_class($provider)])) { - $disabled = true; - $description = pht('This provider is already configured.'); + $menu = id(new PHUIObjectItemListView()) + ->setViewer($viewer) + ->setBig(true) + ->setFlush(true); + + foreach ($providers as $provider_key => $provider) { + $provider_class = get_class($provider); + + $provider_uri = id(new PhutilURI('/config/edit/')) + ->setQueryParam('provider', $provider_class); + $provider_uri = $this->getApplicationURI($provider_uri); + + $already_exists = isset($configured_classes[get_class($provider)]); + + $item = id(new PHUIObjectItemView()) + ->setHeader($provider->getNameForCreate()) + ->setImageIcon($provider->newIconView()) + ->addAttribute($provider->getDescriptionForCreate()); + + if (!$already_exists) { + $item + ->setHref($provider_uri) + ->setClickable(true); } else { - $disabled = false; - $description = $provider->getDescriptionForCreate(); + $item->setDisabled(true); } - $options->addButton( - get_class($provider), - $provider->getNameForCreate(), - $description, - $disabled ? 'disabled' : null, - $disabled); + + if ($already_exists) { + $messages = array(); + $messages[] = pht('You already have a provider of this type.'); + + $info = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors($messages); + + $item->appendChild($info); + } + + $menu->addItem($item); } - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild($options) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($this->getApplicationURI()) - ->setValue(pht('Continue'))); - - $form_box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Provider')) - ->setFormErrors($errors) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Add Provider')); - $crumbs->setBorder(true); - - $title = pht('Add Auth Provider'); - - $header = id(new PHUIHeaderView()) - ->setHeader($title) - ->setHeaderIcon('fa-plus-square'); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter(array( - $form_box, - )); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - + return $this->newDialog() + ->setTitle(pht('Add Auth Provider')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->appendChild($menu) + ->addCancelButton($cancel_uri); } } diff --git a/src/applications/auth/controller/config/PhabricatorAuthProviderViewController.php b/src/applications/auth/controller/config/PhabricatorAuthProviderViewController.php new file mode 100644 index 0000000000..532744001c --- /dev/null +++ b/src/applications/auth/controller/config/PhabricatorAuthProviderViewController.php @@ -0,0 +1,119 @@ +requireApplicationCapability( + AuthManageProvidersCapability::CAPABILITY); + + $viewer = $this->getViewer(); + $id = $request->getURIData('id'); + + $config = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withIDs(array($id)) + ->executeOne(); + if (!$config) { + return new Aphront404Response(); + } + + $header = $this->buildHeaderView($config); + $properties = $this->buildPropertiesView($config); + $curtain = $this->buildCurtain($config); + + $timeline = $this->buildTransactionTimeline( + $config, + new PhabricatorAuthProviderConfigTransactionQuery()); + $timeline->setShouldTerminate(true); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->addPropertySection(pht('Details'), $properties) + ->setMainColumn($timeline); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb($config->getObjectName()) + ->setBorder(true); + + return $this->newPage() + ->setTitle(pht('Auth Provider: %s', $config->getDisplayName())) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildHeaderView(PhabricatorAuthProviderConfig $config) { + $viewer = $this->getViewer(); + + $view = id(new PHUIHeaderView()) + ->setViewer($viewer) + ->setHeader($config->getDisplayName()); + + if ($config->getIsEnabled()) { + $view->setStatus('fa-check', 'bluegrey', pht('Enabled')); + } else { + $view->setStatus('fa-ban', 'red', pht('Disabled')); + } + + return $view; + } + + private function buildCurtain(PhabricatorAuthProviderConfig $config) { + $viewer = $this->getViewer(); + $id = $config->getID(); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $config, + PhabricatorPolicyCapability::CAN_EDIT); + + $curtain = $this->newCurtainView($config); + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Auth Provider')) + ->setIcon('fa-pencil') + ->setHref($this->getApplicationURI("config/edit/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit)); + + if ($config->getIsEnabled()) { + $disable_uri = $this->getApplicationURI('config/disable/'.$id.'/'); + $disable_icon = 'fa-ban'; + $disable_text = pht('Disable Provider'); + } else { + $disable_uri = $this->getApplicationURI('config/enable/'.$id.'/'); + $disable_icon = 'fa-check'; + $disable_text = pht('Enable Provider'); + } + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setName($disable_text) + ->setIcon($disable_icon) + ->setHref($disable_uri) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + + return $curtain; + } + + private function buildPropertiesView(PhabricatorAuthProviderConfig $config) { + $viewer = $this->getViewer(); + + $view = id(new PHUIPropertyListView()) + ->setViewer($viewer); + + $view->addProperty( + pht('Provider Type'), + $config->getProvider()->getProviderName()); + + return $view; + } +} diff --git a/src/applications/auth/factor/PhabricatorAuthFactor.php b/src/applications/auth/factor/PhabricatorAuthFactor.php index ec49f7f748..912a2c31c9 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorAuthFactor.php @@ -123,6 +123,7 @@ abstract class PhabricatorAuthFactor extends Phobject { ->setUserPHID($viewer->getPHID()) ->setSessionPHID($viewer->getSession()->getPHID()) ->setFactorPHID($config->getPHID()) + ->setIsNewChallenge(true) ->setWorkflowKey($engine->getWorkflowKey()); } @@ -283,8 +284,11 @@ abstract class PhabricatorAuthFactor extends Phobject { $error = $result->getErrorMessage(); - $icon = id(new PHUIIconView()) - ->setIcon('fa-clock-o', 'red'); + $icon = $result->getIcon(); + if (!$icon) { + $icon = id(new PHUIIconView()) + ->setIcon('fa-clock-o', 'red'); + } return id(new PHUIFormTimerControl()) ->setIcon($icon) @@ -295,8 +299,11 @@ abstract class PhabricatorAuthFactor extends Phobject { private function newAnsweredControl( PhabricatorAuthFactorResult $result) { - $icon = id(new PHUIIconView()) - ->setIcon('fa-check-circle-o', 'green'); + $icon = $result->getIcon(); + if (!$icon) { + $icon = id(new PHUIIconView()) + ->setIcon('fa-check-circle-o', 'green'); + } return id(new PHUIFormTimerControl()) ->setIcon($icon) @@ -309,8 +316,11 @@ abstract class PhabricatorAuthFactor extends Phobject { $error = $result->getErrorMessage(); - $icon = id(new PHUIIconView()) - ->setIcon('fa-times', 'red'); + $icon = $result->getIcon(); + if (!$icon) { + $icon = id(new PHUIIconView()) + ->setIcon('fa-times', 'red'); + } return id(new PHUIFormTimerControl()) ->setIcon($icon) @@ -323,8 +333,11 @@ abstract class PhabricatorAuthFactor extends Phobject { $error = $result->getErrorMessage(); - $icon = id(new PHUIIconView()) - ->setIcon('fa-commenting', 'green'); + $icon = $result->getIcon(); + if (!$icon) { + $icon = id(new PHUIIconView()) + ->setIcon('fa-commenting', 'green'); + } return id(new PHUIFormTimerControl()) ->setIcon($icon) diff --git a/src/applications/auth/factor/PhabricatorAuthFactorResult.php b/src/applications/auth/factor/PhabricatorAuthFactorResult.php index 2282f162a9..f03c3674da 100644 --- a/src/applications/auth/factor/PhabricatorAuthFactorResult.php +++ b/src/applications/auth/factor/PhabricatorAuthFactorResult.php @@ -10,6 +10,7 @@ final class PhabricatorAuthFactorResult private $errorMessage; private $value; private $issuedChallenges = array(); + private $icon; public function setAnsweredChallenge(PhabricatorAuthChallenge $challenge) { if (!$challenge->getIsAnsweredChallenge()) { @@ -92,4 +93,13 @@ final class PhabricatorAuthFactorResult return $this->issuedChallenges; } + public function setIcon(PHUIIconView $icon) { + $this->icon = $icon; + return $this; + } + + public function getIcon() { + return $this->icon; + } + } diff --git a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php index 187e011953..4be4c15ea8 100644 --- a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php @@ -612,7 +612,22 @@ final class PhabricatorDuoAuthFactor return $this->newResult() ->setAnsweredChallenge($challenge); case 'waiting': - // No result yet, we'll render a default state later on. + // If we didn't just issue this challenge, give the user a stronger + // hint that they need to follow the instructions. + if (!$challenge->getIsNewChallenge()) { + return $this->newResult() + ->setIsContinue(true) + ->setIcon( + id(new PHUIIconView()) + ->setIcon('fa-exclamation-triangle', 'yellow')) + ->setErrorMessage( + pht( + 'You must approve the challenge which was sent to your '. + 'phone. Open the Duo application and confirm the challenge, '. + 'then continue.')); + } + + // Otherwise, we'll construct a default message later on. break; default: case 'deny': @@ -666,8 +681,7 @@ final class PhabricatorDuoAuthFactor public function getRequestHasChallengeResponse( PhabricatorAuthFactorConfig $config, AphrontRequest $request) { - $value = $this->getChallengeResponseFromRequest($config, $request); - return (bool)strlen($value); + return false; } protected function newResultFromChallengeResponse( @@ -675,41 +689,7 @@ final class PhabricatorDuoAuthFactor PhabricatorUser $viewer, AphrontRequest $request, array $challenges) { - - $challenge = $this->getChallengeForCurrentContext( - $config, - $viewer, - $challenges); - - $code = $this->getChallengeResponseFromRequest( - $config, - $request); - - $result = $this->newResult() - ->setValue($code); - - if ($challenge->getIsAnsweredChallenge()) { - return $result->setAnsweredChallenge($challenge); - } - - if (phutil_hashes_are_identical($code, $challenge->getChallengeKey())) { - $ttl = PhabricatorTime::getNow() + phutil_units('15 minutes in seconds'); - - $challenge - ->markChallengeAsAnswered($ttl); - - return $result->setAnsweredChallenge($challenge); - } - - if (strlen($code)) { - $error_message = pht('Invalid'); - } else { - $error_message = pht('Required'); - } - - $result->setErrorMessage($error_message); - - return $result; + return $this->newResult(); } private function newDuoFuture(PhabricatorAuthFactorProvider $provider) { diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index ba6613c014..7e77dfc11a 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -128,6 +128,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { ->setLabel(pht('TOTP Code')) ->setName('totpcode') ->setValue($code) + ->setAutofocus(true) ->setError($e_code)); } diff --git a/src/applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php b/src/applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php index ac3fe4d309..d1f67393ca 100644 --- a/src/applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php +++ b/src/applications/auth/guidance/PhabricatorAuthProvidersGuidanceEngineExtension.php @@ -10,6 +10,26 @@ final class PhabricatorAuthProvidersGuidanceEngineExtension } public function generateGuidance(PhabricatorGuidanceContext $context) { + $configs = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIsEnabled(true) + ->execute(); + + $allows_registration = false; + foreach ($configs as $config) { + $provider = $config->getProvider(); + if ($provider->shouldAllowRegistration()) { + $allows_registration = true; + break; + } + } + + // If no provider allows registration, we don't need provide any warnings + // about registration being too open. + if (!$allows_registration) { + return array(); + } + $domains_key = 'auth.email-domains'; $domains_link = $this->renderConfigLink($domains_key); $domains_value = PhabricatorEnv::getEnvConfig($domains_key); diff --git a/src/applications/auth/handler/PhabricatorAuthLoginHandler.php b/src/applications/auth/handler/PhabricatorAuthLoginHandler.php deleted file mode 100644 index eabbf91843..0000000000 --- a/src/applications/auth/handler/PhabricatorAuthLoginHandler.php +++ /dev/null @@ -1,36 +0,0 @@ -delegatingController = $controller; - return $this; - } - - final public function getDelegatingController() { - return $this->delegatingController; - } - - final public function setRequest(AphrontRequest $request) { - $this->request = $request; - return $this; - } - - final public function getRequest() { - return $this->request; - } - - final public static function getAllHandlers() { - return id(new PhutilClassMapQuery()) - ->setAncestorClass(__CLASS__) - ->execute(); - } -} diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index 0525edad54..bcccca5121 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -311,6 +311,12 @@ abstract class PhabricatorAuthProvider extends Phobject { return 'Generic'; } + public function newIconView() { + return id(new PHUIIconView()) + ->setSpriteSheet(PHUIIconView::SPRITE_LOGIN) + ->setSpriteIcon($this->getLoginIcon()); + } + public function isLoginFormAButton() { return false; } diff --git a/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php b/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php index 44b58b85ff..4a4babcc12 100644 --- a/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorLDAPAuthProvider.php @@ -112,6 +112,7 @@ final class PhabricatorLDAPAuthProvider extends PhabricatorAuthProvider { id(new AphrontFormTextControl()) ->setLabel(pht('LDAP Username')) ->setName('ldap_username') + ->setAutofocus(true) ->setValue($v_user) ->setError($e_user)) ->appendChild( diff --git a/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php b/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php index d841f091aa..ec5720e078 100644 --- a/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorPasswordAuthProvider.php @@ -229,6 +229,7 @@ final class PhabricatorPasswordAuthProvider extends PhabricatorAuthProvider { id(new AphrontFormTextControl()) ->setLabel(pht('Username or Email')) ->setName('username') + ->setAutofocus(true) ->setValue($v_user) ->setError($e_user)) ->appendChild( diff --git a/src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php b/src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php index 626c80348f..ee073e3ac1 100644 --- a/src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php +++ b/src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php @@ -6,11 +6,7 @@ final class PhabricatorAuthProviderConfigQuery private $ids; private $phids; private $providerClasses; - - const STATUS_ALL = 'status:all'; - const STATUS_ENABLED = 'status:enabled'; - - private $status = self::STATUS_ALL; + private $isEnabled; public function withPHIDs(array $phids) { $this->phids = $phids; @@ -22,40 +18,26 @@ final class PhabricatorAuthProviderConfigQuery return $this; } - public function withStatus($status) { - $this->status = $status; - return $this; - } - public function withProviderClasses(array $classes) { $this->providerClasses = $classes; return $this; } - public static function getStatusOptions() { - return array( - self::STATUS_ALL => pht('All Providers'), - self::STATUS_ENABLED => pht('Enabled Providers'), - ); + public function withIsEnabled($is_enabled) { + $this->isEnabled = $is_enabled; + return $this; + } + + public function newResultObject() { + return new PhabricatorAuthProviderConfig(); } protected function loadPage() { - $table = new PhabricatorAuthProviderConfig(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T %Q %Q %Q', - $table->getTableName(), - $this->buildWhereClause($conn_r), - $this->buildOrderClause($conn_r), - $this->buildLimitClause($conn_r)); - - return $table->loadAllFromArray($data); + return $this->loadStandardPage($this->newResultObject()); } - protected function buildWhereClause(AphrontDatabaseConnection $conn) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->ids !== null) { $where[] = qsprintf( @@ -78,22 +60,27 @@ final class PhabricatorAuthProviderConfigQuery $this->providerClasses); } - $status = $this->status; - switch ($status) { - case self::STATUS_ALL: - break; - case self::STATUS_ENABLED: - $where[] = qsprintf( - $conn, - 'isEnabled = 1'); - break; - default: - throw new Exception(pht("Unknown status '%s'!", $status)); + if ($this->isEnabled !== null) { + $where[] = qsprintf( + $conn, + 'isEnabled = %d', + (int)$this->isEnabled); } - $where[] = $this->buildPagingClause($conn); + return $where; + } - return $this->formatWhereClause($conn, $where); + protected function willFilterPage(array $configs) { + + foreach ($configs as $key => $config) { + $provider = $config->getProvider(); + if (!$provider) { + unset($configs[$key]); + continue; + } + } + + return $configs; } public function getQueryApplicationClass() { diff --git a/src/applications/auth/query/PhabricatorExternalAccountQuery.php b/src/applications/auth/query/PhabricatorExternalAccountQuery.php index b34199ce60..c4a53c12f8 100644 --- a/src/applications/auth/query/PhabricatorExternalAccountQuery.php +++ b/src/applications/auth/query/PhabricatorExternalAccountQuery.php @@ -168,35 +168,4 @@ final class PhabricatorExternalAccountQuery return 'PhabricatorPeopleApplication'; } - /** - * Attempts to find an external account and if none exists creates a new - * external account with a shiny new ID and PHID. - * - * NOTE: This function assumes the first item in various query parameters is - * the correct value to use in creating a new external account. - */ - public function loadOneOrCreate() { - $account = $this->executeOne(); - if (!$account) { - $account = new PhabricatorExternalAccount(); - if ($this->accountIDs) { - $account->setAccountID(reset($this->accountIDs)); - } - if ($this->accountTypes) { - $account->setAccountType(reset($this->accountTypes)); - } - if ($this->accountDomains) { - $account->setAccountDomain(reset($this->accountDomains)); - } - if ($this->accountSecrets) { - $account->setAccountSecret(reset($this->accountSecrets)); - } - if ($this->userPHIDs) { - $account->setUserPHID(reset($this->userPHIDs)); - } - $account->save(); - } - return $account; - } - } diff --git a/src/applications/auth/storage/PhabricatorAuthChallenge.php b/src/applications/auth/storage/PhabricatorAuthChallenge.php index 8fa07d712f..0b740e5fa7 100644 --- a/src/applications/auth/storage/PhabricatorAuthChallenge.php +++ b/src/applications/auth/storage/PhabricatorAuthChallenge.php @@ -16,6 +16,7 @@ final class PhabricatorAuthChallenge protected $properties = array(); private $responseToken; + private $isNewChallenge; const HTTPKEY = '__hisec.challenges__'; const TOKEN_DIGEST_KEY = 'auth.challenge.token'; @@ -241,6 +242,15 @@ final class PhabricatorAuthChallenge return $this->properties[$key]; } + public function setIsNewChallenge($is_new_challenge) { + $this->isNewChallenge = $is_new_challenge; + return $this; + } + + public function getIsNewChallenge() { + return $this->isNewChallenge; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/auth/storage/PhabricatorAuthPasswordTransaction.php b/src/applications/auth/storage/PhabricatorAuthPasswordTransaction.php index 9d02112dff..a8cb6d10a3 100644 --- a/src/applications/auth/storage/PhabricatorAuthPasswordTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthPasswordTransaction.php @@ -11,10 +11,6 @@ final class PhabricatorAuthPasswordTransaction return PhabricatorAuthPasswordPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhabricatorAuthPasswordTransactionType'; } diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php index 1de34c4077..876a70c2d0 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfig.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfig.php @@ -83,6 +83,27 @@ final class PhabricatorAuthProviderConfig return $this->provider; } + public function getURI() { + return '/auth/config/view/'.$this->getID().'/'; + } + + public function getObjectName() { + return pht('Auth Provider %d', $this->getID()); + } + + public function getDisplayName() { + return $this->getProvider()->getProviderName(); + } + + public function getSortVector() { + return id(new PhutilSortVector()) + ->addString($this->getDisplayName()); + } + + public function newIconView() { + return $this->getProvider()->newIconView(); + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */ diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php index e1453b4383..d5a3588d59 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php @@ -33,10 +33,6 @@ final class PhabricatorAuthProviderConfigTransaction return PhabricatorAuthAuthProviderPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getIcon() { $old = $this->getOldValue(); $new = $this->getNewValue(); diff --git a/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php index bb08310cf3..028be1746d 100644 --- a/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthSSHKeyTransaction.php @@ -15,10 +15,6 @@ final class PhabricatorAuthSSHKeyTransaction return PhabricatorAuthSSHKeyPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 1c8a593a78..932e04db4b 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -84,6 +84,24 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { $issue->addPhabricatorConfig($key); } } + + + if (PhabricatorEnv::getEnvConfig('feed.http-hooks')) { + $this->newIssue('config.deprecated.feed.http-hooks') + ->setShortName(pht('Feed Hooks Deprecated')) + ->setName(pht('Migrate From "feed.http-hooks" to Webhooks')) + ->addPhabricatorConfig('feed.http-hooks') + ->setMessage( + pht( + 'The "feed.http-hooks" option is deprecated in favor of '. + 'Webhooks. This option will be removed in a future version '. + 'of Phabricator.'. + "\n\n". + 'You can configure Webhooks in Herald.'. + "\n\n". + 'To resolve this issue, remove all URIs from "feed.http-hooks".')); + } + } /** diff --git a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php index ce24d48ead..7e6978dfd8 100644 --- a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php +++ b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php @@ -182,6 +182,25 @@ EODOC $mailers_description = $this->deformat(pht(<<deformat(pht(<<setHidden(true) ->setDescription($mailers_description), $this->newOption('metamta.default-address', 'string', null) - ->setDescription(pht('Default "From" address.')), + ->setLocked(true) + ->setSummary(pht('Default address used when generating mail.')) + ->setDescription($default_description), $this->newOption( 'metamta.one-mail-per-recipient', 'bool', diff --git a/src/applications/config/storage/PhabricatorConfigTransaction.php b/src/applications/config/storage/PhabricatorConfigTransaction.php index b7cfb6f495..94272bfb1a 100644 --- a/src/applications/config/storage/PhabricatorConfigTransaction.php +++ b/src/applications/config/storage/PhabricatorConfigTransaction.php @@ -13,10 +13,6 @@ final class PhabricatorConfigTransaction return PhabricatorConfigConfigPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); diff --git a/src/applications/differential/controller/DifferentialDiffCreateController.php b/src/applications/differential/controller/DifferentialDiffCreateController.php index 36f0b86202..284edf49d3 100644 --- a/src/applications/differential/controller/DifferentialDiffCreateController.php +++ b/src/applications/differential/controller/DifferentialDiffCreateController.php @@ -112,7 +112,7 @@ final class DifferentialDiffCreateController extends DifferentialController { $arcanist_link, ), pht( - 'You can also paste a diff below, or upload a file '. + 'You can also paste a diff above, or upload a file '. 'containing a diff (for example, from %s, %s or %s).', phutil_tag('tt', array(), 'svn diff'), phutil_tag('tt', array(), 'git diff'), diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index 54be7dd7f1..6a863a4a92 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -566,11 +566,8 @@ final class DiffusionBrowseController extends DiffusionController { $name = idx($spec, 'name', $auto); $item->addIcon('fa-code', $name); - if ($package->getAuditingEnabled()) { - $item->addIcon('fa-check', pht('Auditing Enabled')); - } else { - $item->addIcon('fa-ban', pht('No Auditing')); - } + $rule = $package->newAuditingRule(); + $item->addIcon($rule->getIconIcon(), $rule->getDisplayName()); if ($package->isArchived()) { $item->setDisabled(true); diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index 05072e07c7..03eb0b9edf 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -202,6 +202,7 @@ final class DiffusionCommitQuery $table = $this->newResultObject(); $conn = $table->establishConnection('r'); + $empty_exception = null; $subqueries = array(); if ($this->responsiblePHIDs) { $base_authors = $this->authorPHIDs; @@ -222,21 +223,33 @@ final class DiffusionCommitQuery $this->authorPHIDs = $all_authors; $this->auditorPHIDs = $base_auditors; - $subqueries[] = $this->buildStandardPageQuery( - $conn, - $table->getTableName()); + try { + $subqueries[] = $this->buildStandardPageQuery( + $conn, + $table->getTableName()); + } catch (PhabricatorEmptyQueryException $ex) { + $empty_exception = $ex; + } $this->authorPHIDs = $base_authors; $this->auditorPHIDs = $all_auditors; - $subqueries[] = $this->buildStandardPageQuery( - $conn, - $table->getTableName()); + try { + $subqueries[] = $this->buildStandardPageQuery( + $conn, + $table->getTableName()); + } catch (PhabricatorEmptyQueryException $ex) { + $empty_exception = $ex; + } } else { $subqueries[] = $this->buildStandardPageQuery( $conn, $table->getTableName()); } + if (!$subqueries) { + throw $empty_exception; + } + if (count($subqueries) > 1) { $unions = null; foreach ($subqueries as $subquery) { @@ -642,10 +655,19 @@ final class DiffusionCommitQuery } if ($this->authorPHIDs !== null) { + $author_phids = $this->authorPHIDs; + if ($author_phids) { + $author_phids = $this->selectPossibleAuthors($author_phids); + if (!$author_phids) { + throw new PhabricatorEmptyQueryException( + pht('Author PHIDs contain no possible authors.')); + } + } + $where[] = qsprintf( $conn, 'commit.authorPHID IN (%Ls)', - $this->authorPHIDs); + $author_phids); } if ($this->epochMin !== null) { @@ -934,5 +956,20 @@ final class DiffusionCommitQuery ) + $parent; } + private function selectPossibleAuthors(array $phids) { + // See PHI1057. Select PHIDs which might possibly be commit authors from + // a larger list of PHIDs. This primarily filters out packages and projects + // from "Responsible Users: ..." queries. Our goal in performing this + // filtering is to improve the performance of the final query. + + foreach ($phids as $key => $phid) { + if (phid_get_type($phid) !== PhabricatorPeopleUserPHIDType::TYPECONST) { + unset($phids[$key]); + } + } + + return $phids; + } + } diff --git a/src/applications/diviner/storage/DivinerLiveBookTransaction.php b/src/applications/diviner/storage/DivinerLiveBookTransaction.php index ae461e751a..f8eb81d1f3 100644 --- a/src/applications/diviner/storage/DivinerLiveBookTransaction.php +++ b/src/applications/diviner/storage/DivinerLiveBookTransaction.php @@ -11,8 +11,4 @@ final class DivinerLiveBookTransaction return DivinerBookPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - } diff --git a/src/applications/feed/config/PhabricatorFeedConfigOptions.php b/src/applications/feed/config/PhabricatorFeedConfigOptions.php index 4b6612f931..29c5a9549b 100644 --- a/src/applications/feed/config/PhabricatorFeedConfigOptions.php +++ b/src/applications/feed/config/PhabricatorFeedConfigOptions.php @@ -20,22 +20,22 @@ final class PhabricatorFeedConfigOptions } public function getOptions() { + $hooks_help = $this->deformat(pht(<<newOption('feed.http-hooks', 'list', array()) ->setLocked(true) - ->setSummary(pht('POST notifications of feed events.')) - ->setDescription( - pht( - "If you set this to a list of HTTP URIs, when a feed story is ". - "published a task will be created for each URI that posts the ". - "story data to the URI. Daemons automagically retry failures 100 ". - "times, waiting `\$fail_count * 60s` between each subsequent ". - "failure. Be sure to keep the daemon console (`%s`) open ". - "while developing and testing your end points. You may need to". - "restart your daemons to start sending HTTP requests.\n\n". - "NOTE: URIs are not validated, the URI must return HTTP status ". - "200 within 30 seconds, and no permission checks are performed.", - '/daemon/')), + ->setSummary(pht('Deprecated.')) + ->setDescription($hooks_help), ); } diff --git a/src/applications/feed/worker/FeedPublisherHTTPWorker.php b/src/applications/feed/worker/FeedPublisherHTTPWorker.php index 4742e52a29..27a869ddef 100644 --- a/src/applications/feed/worker/FeedPublisherHTTPWorker.php +++ b/src/applications/feed/worker/FeedPublisherHTTPWorker.php @@ -26,6 +26,11 @@ final class FeedPublisherHTTPWorker extends FeedPushWorker { 'epoch' => $data->getEpoch(), ); + // NOTE: We're explicitly using "http_build_query()" here because the + // "storyData" parameter may be a nested object with arbitrary nested + // sub-objects. + $post_data = http_build_query($post_data, '', '&'); + id(new HTTPSFuture($uri, $post_data)) ->setMethod('POST') ->setTimeout(30) diff --git a/src/applications/fund/storage/FundBacker.php b/src/applications/fund/storage/FundBacker.php index 87ab342e2a..ebdf39ae17 100644 --- a/src/applications/fund/storage/FundBacker.php +++ b/src/applications/fund/storage/FundBacker.php @@ -76,6 +76,7 @@ final class FundBacker extends FundDAO public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } @@ -91,6 +92,8 @@ final class FundBacker extends FundDAO return $initiative->getPolicy($capability); } return PhabricatorPolicies::POLICY_NOONE; + case PhabricatorPolicyCapability::CAN_EDIT: + return PhabricatorPolicies::POLICY_NOONE; } } diff --git a/src/applications/fund/storage/FundBackerTransaction.php b/src/applications/fund/storage/FundBackerTransaction.php index c24e769eb6..c08958a29a 100644 --- a/src/applications/fund/storage/FundBackerTransaction.php +++ b/src/applications/fund/storage/FundBackerTransaction.php @@ -11,10 +11,6 @@ final class FundBackerTransaction return FundBackerPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'FundBackerTransactionType'; } diff --git a/src/applications/herald/action/HeraldCommentAction.php b/src/applications/herald/action/HeraldCommentAction.php index fa52ba1f5f..f8b8fbe813 100644 --- a/src/applications/herald/action/HeraldCommentAction.php +++ b/src/applications/herald/action/HeraldCommentAction.php @@ -19,12 +19,9 @@ final class HeraldCommentAction extends HeraldAction { } $xaction = $object->getApplicationTransactionTemplate(); - try { - $comment = $xaction->getApplicationTransactionCommentObject(); - if (!$comment) { - return false; - } - } catch (PhutilMethodNotImplementedException $ex) { + + $comment = $xaction->getApplicationTransactionCommentObject(); + if (!$comment) { return false; } diff --git a/src/applications/herald/storage/HeraldWebhookTransaction.php b/src/applications/herald/storage/HeraldWebhookTransaction.php index 03c8cbb776..4f924cd4bb 100644 --- a/src/applications/herald/storage/HeraldWebhookTransaction.php +++ b/src/applications/herald/storage/HeraldWebhookTransaction.php @@ -11,10 +11,6 @@ final class HeraldWebhookTransaction return HeraldWebhookPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'HeraldWebhookTransactionType'; } diff --git a/src/applications/legalpad/controller/LegalpadDocumentSignController.php b/src/applications/legalpad/controller/LegalpadDocumentSignController.php index ab98c0bb78..f09d95af29 100644 --- a/src/applications/legalpad/controller/LegalpadDocumentSignController.php +++ b/src/applications/legalpad/controller/LegalpadDocumentSignController.php @@ -364,16 +364,6 @@ final class LegalpadDocumentSignController extends LegalpadController { if ($email_obj) { return $this->signInResponse(); } - $external_account = id(new PhabricatorExternalAccountQuery()) - ->setViewer($viewer) - ->withAccountTypes(array('email')) - ->withAccountDomains(array($email->getDomainName())) - ->withAccountIDs(array($email->getAddress())) - ->loadOneOrCreate(); - if ($external_account->getUserPHID()) { - return $this->signInResponse(); - } - $signer_phid = $external_account->getPHID(); } } break; diff --git a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php index 9df8d2478d..ea14fd4a2f 100644 --- a/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php +++ b/src/applications/legalpad/query/LegalpadDocumentSignatureSearchEngine.php @@ -226,7 +226,7 @@ final class LegalpadDocumentSignatureSearchEngine $handles[$document->getPHID()]->renderLink(), $signer_phid ? $handles[$signer_phid]->renderLink() - : null, + : phutil_tag('em', array(), pht('None')), $name, phutil_tag( 'a', diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php index 0651068550..01d6f1e218 100644 --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php @@ -27,4 +27,8 @@ final class PhabricatorMetaMTAMailListController return $nav; } + public function buildApplicationMenu() { + return $this->buildSideNav()->getMenu(); + } + } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php b/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php index 019adb338d..af6a6fbb88 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAApplicationEmailTransaction.php @@ -16,8 +16,4 @@ final class PhabricatorMetaMTAApplicationEmailTransaction return PhabricatorMetaMTAApplicationEmailPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - } diff --git a/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php b/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php index b2624dd9a4..acfb88ef48 100644 --- a/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php +++ b/src/applications/oauthserver/storage/PhabricatorOAuthServerTransaction.php @@ -19,10 +19,6 @@ final class PhabricatorOAuthServerTransaction return PhabricatorOAuthServerClientPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); $old = $this->getOldValue(); diff --git a/src/applications/owners/constants/PhabricatorOwnersAuditRule.php b/src/applications/owners/constants/PhabricatorOwnersAuditRule.php new file mode 100644 index 0000000000..32a9bb804b --- /dev/null +++ b/src/applications/owners/constants/PhabricatorOwnersAuditRule.php @@ -0,0 +1,117 @@ +key = $key; + $rule->spec = $spec; + + return $rule; + } + + public function getKey() { + return $this->key; + } + + public function getDisplayName() { + return idx($this->spec, 'name', $this->key); + } + + public function getIconIcon() { + return idx($this->spec, 'icon.icon'); + } + + public static function newSelectControlMap() { + $specs = self::newSpecifications(); + return ipull($specs, 'name'); + } + + public static function getStorageValueFromAPIValue($value) { + $specs = self::newSpecifications(); + + $map = array(); + foreach ($specs as $key => $spec) { + $deprecated = idx($spec, 'deprecated', array()); + if (isset($deprecated[$value])) { + return $key; + } + } + + return $value; + } + + public static function getModernValueMap() { + $specs = self::newSpecifications(); + + $map = array(); + foreach ($specs as $key => $spec) { + $map[$key] = pht('"%s"', $key); + } + + return $map; + } + + public static function getDeprecatedValueMap() { + $specs = self::newSpecifications(); + + $map = array(); + foreach ($specs as $key => $spec) { + $deprecated_map = idx($spec, 'deprecated', array()); + foreach ($deprecated_map as $deprecated_key => $label) { + $map[$deprecated_key] = $label; + } + } + + return $map; + } + + private static function newSpecifications() { + return array( + self::AUDITING_NONE => array( + 'name' => pht('No Auditing'), + 'icon.icon' => 'fa-ban', + 'deprecated' => array( + '' => pht('"" (empty string)'), + '0' => '"0"', + ), + ), + self::AUDITING_UNREVIEWED => array( + 'name' => pht('Audit Unreviewed Commits'), + 'icon.icon' => 'fa-check', + ), + self::AUDITING_NO_OWNER => array( + 'name' => pht('Audit Commits With No Owner Involvement'), + 'icon.icon' => 'fa-check', + 'deprecated' => array( + '1' => '"1"', + ), + ), + self::AUDITING_NO_OWNER_AND_UNREVIEWED => array( + 'name' => pht( + 'Audit Unreviewed Commits and Commits With No Owner Involvement'), + 'icon.icon' => 'fa-check', + ), + self::AUDITING_ALL => array( + 'name' => pht('Audit All Commits'), + 'icon.icon' => 'fa-check', + ), + ); + } + + + +} diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index f71009cf19..e28ae2b3bb 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -194,12 +194,8 @@ final class PhabricatorOwnersDetailController $name = idx($spec, 'name', $auto); $view->addProperty(pht('Auto Review'), $name); - if ($package->getAuditingEnabled()) { - $auditing = pht('Enabled'); - } else { - $auditing = pht('Disabled'); - } - $view->addProperty(pht('Auditing'), $auditing); + $rule = $package->newAuditingRule(); + $view->addProperty(pht('Auditing'), $rule->getDisplayName()); $ignored = $package->getIgnoredPathAttributes(); $ignored = array_keys($ignored); diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php index 044cb8beda..13f896d3f0 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -140,12 +140,8 @@ EOTEXT ->setTransactionType( PhabricatorOwnersPackageAuditingTransaction::TRANSACTIONTYPE) ->setIsCopyable(true) - ->setValue($object->getAuditingEnabled()) - ->setOptions( - array( - '' => pht('Disabled'), - '1' => pht('Enabled'), - )), + ->setValue($object->getAuditingState()) + ->setOptions(PhabricatorOwnersAuditRule::newSelectControlMap()), id(new PhabricatorRemarkupEditField()) ->setKey('description') ->setLabel(pht('Description')) diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 564fc8a28b..b9e91ef958 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -13,7 +13,6 @@ final class PhabricatorOwnersPackage PhabricatorNgramsInterface { protected $name; - protected $auditingEnabled; protected $autoReview; protected $description; protected $status; @@ -21,6 +20,7 @@ final class PhabricatorOwnersPackage protected $editPolicy; protected $dominion; protected $properties = array(); + protected $auditingState; private $paths = self::ATTACHABLE; private $owners = self::ATTACHABLE; @@ -55,7 +55,7 @@ final class PhabricatorOwnersPackage PhabricatorOwnersDefaultEditCapability::CAPABILITY); return id(new PhabricatorOwnersPackage()) - ->setAuditingEnabled(0) + ->setAuditingState(PhabricatorOwnersAuditRule::AUDITING_NONE) ->setAutoReview(self::AUTOREVIEW_NONE) ->setDominion(self::DOMINION_STRONG) ->setViewPolicy($view_policy) @@ -126,7 +126,7 @@ final class PhabricatorOwnersPackage self::CONFIG_COLUMN_SCHEMA => array( 'name' => 'sort', 'description' => 'text', - 'auditingEnabled' => 'bool', + 'auditingState' => 'text32', 'status' => 'text32', 'autoReview' => 'text32', 'dominion' => 'text32', @@ -564,6 +564,10 @@ final class PhabricatorOwnersPackage return '/owners/package/'.$this->getID().'/'; } + public function newAuditingRule() { + return PhabricatorOwnersAuditRule::newFromState($this->getAuditingState()); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -720,17 +724,11 @@ final class PhabricatorOwnersPackage 'label' => $review_label, ); - if ($this->getAuditingEnabled()) { - $audit_value = 'audit'; - $audit_label = pht('Auditing Enabled'); - } else { - $audit_value = 'none'; - $audit_label = pht('No Auditing'); - } + $audit_rule = $this->newAuditingRule(); $audit = array( - 'value' => $audit_value, - 'label' => $audit_label, + 'value' => $audit_rule->getKey(), + 'label' => $audit_rule->getDisplayName(), ); $dominion_value = $this->getDominion(); diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php index 1dfc944f63..66e15b634a 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php @@ -15,8 +15,4 @@ final class PhabricatorOwnersPackageTransaction return 'PhabricatorOwnersPackageTransactionType'; } - public function getApplicationTransactionCommentObject() { - return null; - } - } diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php index df4f0feb01..7c16c850fd 100644 --- a/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php @@ -6,27 +6,62 @@ final class PhabricatorOwnersPackageAuditingTransaction const TRANSACTIONTYPE = 'owners.auditing'; public function generateOldValue($object) { - return (int)$object->getAuditingEnabled(); + return $object->getAuditingState(); } public function generateNewValue($object, $value) { - return (int)$value; + return PhabricatorOwnersAuditRule::getStorageValueFromAPIValue($value); } public function applyInternalEffects($object, $value) { - $object->setAuditingEnabled($value); + $object->setAuditingState($value); } public function getTitle() { - if ($this->getNewValue()) { - return pht( - '%s enabled auditing for this package.', - $this->renderAuthor()); - } else { - return pht( - '%s disabled auditing for this package.', - $this->renderAuthor()); + $old_value = $this->getOldValue(); + $new_value = $this->getNewValue(); + + $old_rule = PhabricatorOwnersAuditRule::newFromState($old_value); + $new_rule = PhabricatorOwnersAuditRule::newFromState($new_value); + + return pht( + '%s changed the audit rule for this package from %s to %s.', + $this->renderAuthor(), + $this->renderValue($old_rule->getDisplayName()), + $this->renderValue($new_rule->getDisplayName())); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + // See PHI1047. This transaction type accepted some weird stuff. Continue + // supporting it for now, but move toward sensible consistency. + + $modern_options = PhabricatorOwnersAuditRule::getModernValueMap(); + $deprecated_options = PhabricatorOwnersAuditRule::getDeprecatedValueMap(); + + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + if (isset($modern_options[$new_value])) { + continue; + } + + if (isset($deprecated_options[$new_value])) { + continue; + } + + $errors[] = $this->newInvalidError( + pht( + 'Package auditing value "%s" is not supported. Supported options '. + 'are: %s. Deprecated options are: %s.', + $new_value, + implode(', ', $modern_options), + implode(', ', $deprecated_options)), + $xaction); } + + return $errors; } } diff --git a/src/applications/passphrase/storage/PassphraseCredentialTransaction.php b/src/applications/passphrase/storage/PassphraseCredentialTransaction.php index b7e4f904ef..bbc3b09668 100644 --- a/src/applications/passphrase/storage/PassphraseCredentialTransaction.php +++ b/src/applications/passphrase/storage/PassphraseCredentialTransaction.php @@ -11,10 +11,6 @@ final class PassphraseCredentialTransaction return PassphraseCredentialPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PassphraseCredentialTransactionType'; } diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index 9238d8da3b..06740be26e 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -51,7 +51,8 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { 'send/' => 'PhabricatorPeopleInviteSendController', ), - 'approve/(?P[1-9]\d*)/' => 'PhabricatorPeopleApproveController', + 'approve/(?P[1-9]\d*)/(?:via/(?P[^/]+)/)?' + => 'PhabricatorPeopleApproveController', '(?Pdisapprove)/(?P[1-9]\d*)/' => 'PhabricatorPeopleDisableController', '(?Pdisable)/(?P[1-9]\d*)/' diff --git a/src/applications/people/controller/PhabricatorPeopleApproveController.php b/src/applications/people/controller/PhabricatorPeopleApproveController.php index 013f4371f6..af08a6fbdc 100644 --- a/src/applications/people/controller/PhabricatorPeopleApproveController.php +++ b/src/applications/people/controller/PhabricatorPeopleApproveController.php @@ -14,7 +14,15 @@ final class PhabricatorPeopleApproveController return new Aphront404Response(); } - $done_uri = $this->getApplicationURI('query/approval/'); + $via = $request->getURIData('via'); + switch ($via) { + case 'profile': + $done_uri = urisprintf('/people/manage/%d/', $user->getID()); + break; + default: + $done_uri = $this->getApplicationURI('query/approval/'); + break; + } if ($user->getIsApproved()) { return $this->newDialog() diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 902b21efcc..91afda123b 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -70,40 +70,53 @@ abstract class PhabricatorPeopleProfileController $profile_icon = PhabricatorPeopleIconSet::getIconIcon($profile->getIcon()); $profile_title = $profile->getDisplayTitle(); - $roles = array(); + + $tag = id(new PHUITagView()) + ->setType(PHUITagView::TYPE_SHADE); + + $tags = array(); if ($user->getIsAdmin()) { - $roles[] = pht('Administrator'); - } - if ($user->getIsDisabled()) { - $roles[] = pht('Disabled'); - } - if (!$user->getIsApproved()) { - $roles[] = pht('Not Approved'); - } - if ($user->getIsSystemAgent()) { - $roles[] = pht('Bot'); - } - if ($user->getIsMailingList()) { - $roles[] = pht('Mailing List'); - } - if (!$user->getIsEmailVerified()) { - $roles[] = pht('Email Not Verified'); + $tags[] = id(clone $tag) + ->setName(pht('Administrator')) + ->setColor('blue'); } - $tag = null; - if ($roles) { - $tag = id(new PHUITagView()) - ->setName(implode(', ', $roles)) - ->addClass('project-view-header-tag') - ->setType(PHUITagView::TYPE_SHADE); + // "Disabled" gets a stronger status tag below. + + if (!$user->getIsApproved()) { + $tags[] = id(clone $tag) + ->setName('Not Approved') + ->setColor('yellow'); + } + + if ($user->getIsSystemAgent()) { + $tags[] = id(clone $tag) + ->setName(pht('Bot')) + ->setColor('orange'); + } + + if ($user->getIsMailingList()) { + $tags[] = id(clone $tag) + ->setName(pht('Mailing List')) + ->setColor('orange'); + } + + if (!$user->getIsEmailVerified()) { + $tags[] = id(clone $tag) + ->setName(pht('Email Not Verified')) + ->setColor('violet'); } $header = id(new PHUIHeaderView()) - ->setHeader(array($user->getFullName(), $tag)) + ->setHeader($user->getFullName()) ->setImage($picture) ->setProfileHeader(true) ->addClass('people-profile-header'); + foreach ($tags as $tag) { + $header->addTag($tag); + } + require_celerity_resource('project-view-css'); if ($user->getIsDisabled()) { diff --git a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php index e9faae3d62..835935f775 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileManageController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileManageController.php @@ -92,6 +92,8 @@ final class PhabricatorPeopleProfileManageController PeopleDisableUsersCapability::CAPABILITY); $can_disable = ($has_disable && !$is_self); + $id = $user->getID(); + $welcome_engine = id(new PhabricatorPeopleWelcomeMailEngine()) ->setSender($viewer) ->setRecipient($user); @@ -103,7 +105,7 @@ final class PhabricatorPeopleProfileManageController id(new PhabricatorActionView()) ->setIcon('fa-pencil') ->setName(pht('Edit Profile')) - ->setHref($this->getApplicationURI('editprofile/'.$user->getID().'/')) + ->setHref($this->getApplicationURI('editprofile/'.$id.'/')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); @@ -111,7 +113,7 @@ final class PhabricatorPeopleProfileManageController id(new PhabricatorActionView()) ->setIcon('fa-picture-o') ->setName(pht('Edit Profile Picture')) - ->setHref($this->getApplicationURI('picture/'.$user->getID().'/')) + ->setHref($this->getApplicationURI('picture/'.$id.'/')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); @@ -137,7 +139,7 @@ final class PhabricatorPeopleProfileManageController ->setName($empower_name) ->setDisabled(!$can_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('empower/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('empower/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) @@ -145,7 +147,7 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Change Username')) ->setDisabled(!$is_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('rename/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('rename/'.$id.'/'))); if ($user->getIsDisabled()) { $disable_icon = 'fa-check-circle-o'; @@ -161,19 +163,34 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Send Welcome Email')) ->setWorkflow(true) ->setDisabled(!$can_welcome) - ->setHref($this->getApplicationURI('welcome/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('welcome/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) ->setType(PhabricatorActionView::TYPE_DIVIDER)); + if (!$user->getIsApproved()) { + $approve_action = id(new PhabricatorActionView()) + ->setIcon('fa-thumbs-up') + ->setName(pht('Approve User')) + ->setWorkflow(true) + ->setDisabled(!$is_admin) + ->setHref("/people/approve/{$id}/via/profile/"); + + if ($is_admin) { + $approve_action->setColor(PhabricatorActionView::GREEN); + } + + $curtain->addAction($approve_action); + } + $curtain->addAction( id(new PhabricatorActionView()) ->setIcon($disable_icon) ->setName($disable_name) ->setDisabled(!$can_disable) ->setWorkflow(true) - ->setHref($this->getApplicationURI('disable/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('disable/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) @@ -181,7 +198,7 @@ final class PhabricatorPeopleProfileManageController ->setName(pht('Delete User')) ->setDisabled(!$can_admin) ->setWorkflow(true) - ->setHref($this->getApplicationURI('delete/'.$user->getID().'/'))); + ->setHref($this->getApplicationURI('delete/'.$id.'/'))); $curtain->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/people/mail/PhabricatorPeopleUsernameMailEngine.php b/src/applications/people/mail/PhabricatorPeopleUsernameMailEngine.php new file mode 100644 index 0000000000..c954b7c38e --- /dev/null +++ b/src/applications/people/mail/PhabricatorPeopleUsernameMailEngine.php @@ -0,0 +1,60 @@ +newUsername = $new_username; + return $this; + } + + public function getNewUsername() { + return $this->newUsername; + } + + public function setOldUsername($old_username) { + $this->oldUsername = $old_username; + return $this; + } + + public function getOldUsername() { + return $this->oldUsername; + } + + public function validateMail() { + return; + } + + protected function newMail() { + $sender = $this->getSender(); + $recipient = $this->getRecipient(); + + $sender_username = $sender->getUsername(); + $sender_realname = $sender->getRealName(); + + $old_username = $this->getOldUsername(); + $new_username = $this->getNewUsername(); + + $body = sprintf( + "%s\n\n %s\n %s\n", + pht( + '%s (%s) has changed your Phabricator username.', + $sender_username, + $sender_realname), + pht( + 'Old Username: %s', + $old_username), + pht( + 'New Username: %s', + $new_username)); + + return id(new PhabricatorMetaMTAMail()) + ->addTos(array($recipient->getPHID())) + ->setSubject(pht('[Phabricator] Username Changed')) + ->setBody($body); + } + +} diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 0b18c292c2..055df8b79e 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -555,55 +555,6 @@ final class PhabricatorUser } } - public function sendUsernameChangeEmail( - PhabricatorUser $admin, - $old_username) { - - $admin_username = $admin->getUserName(); - $admin_realname = $admin->getRealName(); - $new_username = $this->getUserName(); - - $password_instructions = null; - if (PhabricatorPasswordAuthProvider::getPasswordProvider()) { - $engine = new PhabricatorAuthSessionEngine(); - $uri = $engine->getOneTimeLoginURI( - $this, - null, - PhabricatorAuthSessionEngine::ONETIME_USERNAME); - $password_instructions = sprintf( - "%s\n\n %s\n\n%s\n", - pht( - "If you use a password to login, you'll need to reset it ". - "before you can login again. You can reset your password by ". - "following this link:"), - $uri, - pht( - "And, of course, you'll need to use your new username to login ". - "from now on. If you use OAuth to login, nothing should change.")); - } - - $body = sprintf( - "%s\n\n %s\n %s\n\n%s", - pht( - '%s (%s) has changed your Phabricator username.', - $admin_username, - $admin_realname), - pht( - 'Old Username: %s', - $old_username), - pht( - 'New Username: %s', - $new_username), - $password_instructions); - - $mail = id(new PhabricatorMetaMTAMail()) - ->addTos(array($this->getPHID())) - ->setForceDelivery(true) - ->setSubject(pht('[Phabricator] Username Changed')) - ->setBody($body) - ->saveAndSend(); - } - public static function describeValidUsername() { return pht( 'Usernames must contain only numbers, letters, period, underscore and '. @@ -1180,9 +1131,10 @@ final class PhabricatorUser $this->openTransaction(); $this->delete(); - $externals = id(new PhabricatorExternalAccount())->loadAllWhere( - 'userPHID = %s', - $this->getPHID()); + $externals = id(new PhabricatorExternalAccountQuery()) + ->setViewer($engine->getViewer()) + ->withUserPHIDs(array($this->getPHID())) + ->execute(); foreach ($externals as $external) { $external->delete(); } diff --git a/src/applications/people/storage/PhabricatorUserTransaction.php b/src/applications/people/storage/PhabricatorUserTransaction.php index 24edb2f5b5..81ca52a132 100644 --- a/src/applications/people/storage/PhabricatorUserTransaction.php +++ b/src/applications/people/storage/PhabricatorUserTransaction.php @@ -11,10 +11,6 @@ final class PhabricatorUserTransaction return PhabricatorPeopleUserPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhabricatorUserTransactionType'; } diff --git a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php index b6d23b3511..b436b76716 100644 --- a/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php +++ b/src/applications/people/xaction/PhabricatorUserUsernameTransaction.php @@ -18,18 +18,27 @@ final class PhabricatorUserUsernameTransaction } public function applyExternalEffects($object, $value) { + $actor = $this->getActor(); $user = $object; + $old_username = $this->getOldValue(); + $new_username = $this->getNewValue(); + $this->newUserLog(PhabricatorUserLog::ACTION_CHANGE_USERNAME) - ->setOldValue($this->getOldValue()) - ->setNewValue($value) + ->setOldValue($old_username) + ->setNewValue($new_username) ->save(); // The SSH key cache currently includes usernames, so dirty it. See T12554 // for discussion. PhabricatorAuthSSHKeyQuery::deleteSSHKeyCache(); - $user->sendUsernameChangeEmail($this->getActor(), $this->getOldValue()); + id(new PhabricatorPeopleUsernameMailEngine()) + ->setSender($actor) + ->setRecipient($object) + ->setOldUsername($old_username) + ->setNewUsername($new_username) + ->sendMail(); } public function getTitle() { @@ -40,6 +49,15 @@ final class PhabricatorUserUsernameTransaction $this->renderNewValue()); } + public function getTitleForFeed() { + return pht( + '%s renamed %s from %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderOldValue(), + $this->renderNewValue()); + } + public function validateTransactions($object, array $xactions) { $actor = $this->getActor(); $errors = array(); diff --git a/src/applications/phame/storage/PhameBlogTransaction.php b/src/applications/phame/storage/PhameBlogTransaction.php index d3d6a79d0a..c605510d7d 100644 --- a/src/applications/phame/storage/PhameBlogTransaction.php +++ b/src/applications/phame/storage/PhameBlogTransaction.php @@ -15,10 +15,6 @@ final class PhameBlogTransaction return PhabricatorPhameBlogPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhameBlogTransactionType'; } diff --git a/src/applications/phlux/storage/PhluxTransaction.php b/src/applications/phlux/storage/PhluxTransaction.php index 1224caf201..b1624d581a 100644 --- a/src/applications/phlux/storage/PhluxTransaction.php +++ b/src/applications/phlux/storage/PhluxTransaction.php @@ -13,10 +13,6 @@ final class PhluxTransaction extends PhabricatorApplicationTransaction { return PhluxVariablePHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); diff --git a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php index 87bddd4d33..521508e0e8 100644 --- a/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php +++ b/src/applications/phortune/controller/payment/PhortunePaymentMethodCreateController.php @@ -73,6 +73,7 @@ final class PhortunePaymentMethodCreateController $provider = $providers[$provider_id]; $errors = array(); + $display_exception = null; if ($request->isFormPost() && $request->getBool('isProviderForm')) { $method = id(new PhortunePaymentMethod()) ->setAccountPHID($account->getPHID()) @@ -107,14 +108,23 @@ final class PhortunePaymentMethodCreateController } if (!$errors) { - $errors = $provider->createPaymentMethodFromRequest( - $request, - $method, - $client_token); + try { + $provider->createPaymentMethodFromRequest( + $request, + $method, + $client_token); + } catch (PhortuneDisplayException $exception) { + $display_exception = $exception; + } catch (Exception $ex) { + $errors = array( + pht('There was an error adding this payment method:'), + $ex->getMessage(), + ); + } } } - if (!$errors) { + if (!$errors && !$display_exception) { $method->save(); // If we added this method on a cart flow, return to the cart to @@ -133,13 +143,17 @@ final class PhortunePaymentMethodCreateController return id(new AphrontRedirectResponse())->setURI($next_uri); } else { - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) - ->setTitle(pht('Error Adding Payment Method')) - ->appendChild(id(new PHUIInfoView())->setErrors($errors)) - ->addCancelButton($request->getRequestURI()); + if ($display_exception) { + $dialog_body = $display_exception->getView(); + } else { + $dialog_body = id(new PHUIInfoView()) + ->setErrors($errors); + } - return id(new AphrontDialogResponse())->setDialog($dialog); + return $this->newDialog() + ->setTitle(pht('Error Adding Payment Method')) + ->appendChild($dialog_body) + ->addCancelButton($request->getRequestURI()); } } diff --git a/src/applications/phortune/exception/PhortuneDisplayException.php b/src/applications/phortune/exception/PhortuneDisplayException.php new file mode 100644 index 0000000000..7b2bbf6875 --- /dev/null +++ b/src/applications/phortune/exception/PhortuneDisplayException.php @@ -0,0 +1,15 @@ +view = $view; + return $this; + } + + public function getView() { + return $this->view; + } + +} diff --git a/src/applications/phortune/provider/PhortuneStripePaymentProvider.php b/src/applications/phortune/provider/PhortuneStripePaymentProvider.php index bdaa4294b2..0463881016 100644 --- a/src/applications/phortune/provider/PhortuneStripePaymentProvider.php +++ b/src/applications/phortune/provider/PhortuneStripePaymentProvider.php @@ -233,8 +233,6 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { array $token) { $this->loadStripeAPILibraries(); - $errors = array(); - $secret_key = $this->getSecretKey(); $stripe_token = $token['stripeCardToken']; @@ -253,7 +251,15 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { // the card more than once. We create one Customer for each card; // they do not map to PhortuneAccounts because we allow an account to // have more than one active card. - $customer = Stripe_Customer::create($params, $secret_key); + try { + $customer = Stripe_Customer::create($params, $secret_key); + } catch (Stripe_CardError $ex) { + $display_exception = $this->newDisplayExceptionFromCardError($ex); + if ($display_exception) { + throw $display_exception; + } + throw $ex; + } $card = $info->card; @@ -267,8 +273,6 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { 'stripe.customerID' => $customer->id, 'stripe.cardToken' => $stripe_token, )); - - return $errors; } public function renderCreatePaymentMethodForm( @@ -383,4 +387,84 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { require_once $root.'/externals/stripe-php/lib/Stripe.php'; } + + private function newDisplayExceptionFromCardError(Stripe_CardError $ex) { + $body = $ex->getJSONBody(); + if (!$body) { + return null; + } + + $map = idx($body, 'error'); + if (!$map) { + return null; + } + + $view = array(); + + $message = idx($map, 'message'); + + $view[] = id(new PHUIInfoView()) + ->setErrors(array($message)); + + $view[] = phutil_tag( + 'div', + array( + 'class' => 'mlt mlb', + ), + pht('Additional details about this error:')); + + $rows = array(); + + $rows[] = array( + pht('Error Code'), + idx($map, 'code'), + ); + + $rows[] = array( + pht('Error Type'), + idx($map, 'type'), + ); + + $param = idx($map, 'param'); + if (strlen($param)) { + $rows[] = array( + pht('Error Param'), + $param, + ); + } + + $decline_code = idx($map, 'decline_code'); + if (strlen($decline_code)) { + $rows[] = array( + pht('Decline Code'), + $decline_code, + ); + } + + $doc_url = idx($map, 'doc_url'); + if ($doc_url) { + $rows[] = array( + pht('Learn More'), + phutil_tag( + 'a', + array( + 'href' => $doc_url, + 'target' => '_blank', + ), + $doc_url), + ); + } + + $view[] = id(new AphrontTableView($rows)) + ->setColumnClasses( + array( + 'header', + 'wide', + )); + + return id(new PhortuneDisplayException(get_class($ex))) + ->setView($view); + } + + } diff --git a/src/applications/phortune/storage/PhortuneAccountTransaction.php b/src/applications/phortune/storage/PhortuneAccountTransaction.php index 6733cbe879..e333ef4a26 100644 --- a/src/applications/phortune/storage/PhortuneAccountTransaction.php +++ b/src/applications/phortune/storage/PhortuneAccountTransaction.php @@ -11,10 +11,6 @@ final class PhortuneAccountTransaction return PhortuneAccountPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhortuneAccountTransactionType'; } diff --git a/src/applications/phortune/storage/PhortuneCartTransaction.php b/src/applications/phortune/storage/PhortuneCartTransaction.php index 41790011a2..c7a1e36e73 100644 --- a/src/applications/phortune/storage/PhortuneCartTransaction.php +++ b/src/applications/phortune/storage/PhortuneCartTransaction.php @@ -19,10 +19,6 @@ final class PhortuneCartTransaction return PhortuneCartPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function shouldHideForMail(array $xactions) { switch ($this->getTransactionType()) { case self::TYPE_CREATED: diff --git a/src/applications/phortune/storage/PhortuneMerchantTransaction.php b/src/applications/phortune/storage/PhortuneMerchantTransaction.php index 3befb12212..976259c534 100644 --- a/src/applications/phortune/storage/PhortuneMerchantTransaction.php +++ b/src/applications/phortune/storage/PhortuneMerchantTransaction.php @@ -11,10 +11,6 @@ final class PhortuneMerchantTransaction return PhortuneMerchantPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhortuneMerchantTransactionType'; } diff --git a/src/applications/phortune/storage/PhortunePaymentProviderConfigTransaction.php b/src/applications/phortune/storage/PhortunePaymentProviderConfigTransaction.php index 08872d48fd..9241c7ae04 100644 --- a/src/applications/phortune/storage/PhortunePaymentProviderConfigTransaction.php +++ b/src/applications/phortune/storage/PhortunePaymentProviderConfigTransaction.php @@ -17,10 +17,6 @@ final class PhortunePaymentProviderConfigTransaction return PhortunePaymentProviderPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); diff --git a/src/applications/policy/filter/PhabricatorPolicyFilter.php b/src/applications/policy/filter/PhabricatorPolicyFilter.php index fb03936ec2..a5c9f356f4 100644 --- a/src/applications/policy/filter/PhabricatorPolicyFilter.php +++ b/src/applications/policy/filter/PhabricatorPolicyFilter.php @@ -175,9 +175,10 @@ final class PhabricatorPolicyFilter extends Phobject { if (!in_array($capability, $object_capabilities)) { throw new Exception( pht( - "Testing for capability '%s' on an object which does ". - "not have that capability!", - $capability)); + 'Testing for capability "%s" on an object ("%s") which does '. + 'not support that capability.', + $capability, + get_class($object))); } $policy = $this->getObjectPolicy($object, $capability); diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index a8b2bb0d4a..158c2480c0 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -19,10 +19,6 @@ final class PhabricatorProjectTransaction return PhabricatorProjectProjectPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhabricatorProjectTransactionType'; } diff --git a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php index 85c354ba67..8729e462a3 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php +++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php @@ -11,10 +11,6 @@ final class PhabricatorRepositoryTransaction return PhabricatorRepositoryRepositoryPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhabricatorRepositoryTransactionType'; } diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php index 75ae0c9c14..b0c60667a4 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php @@ -132,28 +132,91 @@ final class PhabricatorRepositoryCommitOwnersWorker $author_phid, $revision) { - // Don't trigger an audit if auditing isn't enabled for the package. - if (!$package->getAuditingEnabled()) { - return false; + $audit_uninvolved = false; + $audit_unreviewed = false; + + $rule = $package->newAuditingRule(); + switch ($rule->getKey()) { + case PhabricatorOwnersAuditRule::AUDITING_NONE: + return false; + case PhabricatorOwnersAuditRule::AUDITING_ALL: + return true; + case PhabricatorOwnersAuditRule::AUDITING_NO_OWNER: + $audit_uninvolved = true; + break; + case PhabricatorOwnersAuditRule::AUDITING_UNREVIEWED: + $audit_unreviewed = true; + break; + case PhabricatorOwnersAuditRule::AUDITING_NO_OWNER_AND_UNREVIEWED: + $audit_uninvolved = true; + $audit_unreviewed = true; + break; } - // Trigger an audit if we don't recognize the commit's author. - if (!$author_phid) { - return true; + // If auditing is configured to trigger on unreviewed changes, check if + // the revision was "Accepted" when it landed. If not, trigger an audit. + if ($audit_unreviewed) { + $commit_unreviewed = true; + if ($revision) { + $was_accepted = DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED; + if ($revision->isPublished()) { + if ($revision->getProperty($was_accepted)) { + $commit_unreviewed = false; + } + } + } + + if ($commit_unreviewed) { + return true; + } } + // If auditing is configured to trigger on changes with no involved owner, + // check for an owner. If we don't find one, trigger an audit. + if ($audit_uninvolved) { + $commit_uninvolved = $this->isOwnerInvolved( + $commit, + $package, + $author_phid, + $revision); + if ($commit_uninvolved) { + return true; + } + } + + // We can't find any reason to trigger an audit for this commit. + return false; + } + + private function isOwnerInvolved( + PhabricatorRepositoryCommit $commit, + PhabricatorOwnersPackage $package, + $author_phid, + $revision) { + $owner_phids = PhabricatorOwnersOwner::loadAffiliatedUserPHIDs( array( $package->getID(), )); $owner_phids = array_fuse($owner_phids); - // Don't trigger an audit if the author is a package owner. - if (isset($owner_phids[$author_phid])) { - return false; + // For the purposes of deciding whether the owners were involved in the + // revision or not, consider a review by the package itself to count as + // involvement. This can happen when human reviewers force-accept on + // behalf of packages they don't own but have authority over. + $owner_phids[$package->getPHID()] = $package->getPHID(); + + // If the commit author is identifiable and a package owner, they're + // involved. + if ($author_phid) { + if (isset($owner_phids[$author_phid])) { + return true; + } } - // Trigger an audit of there is no corresponding revision. + // Otherwise, we need to find an owner as a reviewer. + + // If we don't have a revision, this is hopeless: no owners are involved. if (!$revision) { return true; } @@ -168,26 +231,25 @@ final class PhabricatorRepositoryCommitOwnersWorker foreach ($revision->getReviewers() as $reviewer) { $reviewer_phid = $reviewer->getReviewerPHID(); - // If this reviewer isn't a package owner, just ignore them. + // If this reviewer isn't a package owner or the package itself, + // just ignore them. if (empty($owner_phids[$reviewer_phid])) { continue; } - // If this reviewer accepted the revision and owns the package, we're - // all clear and do not need to trigger an audit. + // If this reviewer accepted the revision and owns the package (or is + // the package), we've found an involved owner. if (isset($accepted_statuses[$reviewer->getReviewerStatus()])) { $found_accept = true; break; } } - // Don't trigger an audit if a package owner already reviewed the - // revision. if ($found_accept) { - return false; + return true; } - return true; + return false; } } diff --git a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php index ab4da88420..126f298f1f 100644 --- a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php @@ -70,12 +70,8 @@ final class PhabricatorFulltextIndexEngineExtension private function getCommentVersion($object) { $xaction = $object->getApplicationTransactionTemplate(); - try { - $comment = $xaction->getApplicationTransactionCommentObject(); - if (!$comment) { - return 'none'; - } - } catch (Exception $ex) { + $comment = $xaction->getApplicationTransactionCommentObject(); + if (!$comment) { return 'none'; } diff --git a/src/applications/search/storage/PhabricatorProfileMenuItemConfigurationTransaction.php b/src/applications/search/storage/PhabricatorProfileMenuItemConfigurationTransaction.php index b1d30a5b9d..4624d6f9af 100644 --- a/src/applications/search/storage/PhabricatorProfileMenuItemConfigurationTransaction.php +++ b/src/applications/search/storage/PhabricatorProfileMenuItemConfigurationTransaction.php @@ -20,8 +20,4 @@ final class PhabricatorProfileMenuItemConfigurationTransaction return PhabricatorProfileMenuItemPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - } diff --git a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php index 1215487208..29ef9fa2c7 100644 --- a/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorExternalAccountsSettingsPanel.php @@ -41,13 +41,6 @@ final class PhabricatorExternalAccountsSettingsPanel ->setUser($viewer) ->setNoDataString(pht('You have no linked accounts.')); - $login_accounts = 0; - foreach ($accounts as $account) { - if ($account->isUsableForLogin()) { - $login_accounts++; - } - } - foreach ($accounts as $account) { $item = new PHUIObjectItemView(); @@ -72,8 +65,6 @@ final class PhabricatorExternalAccountsSettingsPanel 'account provider).')); } - $can_unlink = $can_unlink && (!$can_login || ($login_accounts > 1)); - $can_refresh = $provider && $provider->shouldAllowAccountRefresh(); if ($can_refresh) { $item->addAction( @@ -105,26 +96,39 @@ final class PhabricatorExternalAccountsSettingsPanel $accounts = mpull($accounts, null, 'getProviderKey'); - $providers = PhabricatorAuthProvider::getAllEnabledProviders(); - $providers = msort($providers, 'getProviderName'); - foreach ($providers as $key => $provider) { - if (isset($accounts[$key])) { - continue; - } + $configs = id(new PhabricatorAuthProviderConfigQuery()) + ->setViewer($viewer) + ->withIsEnabled(true) + ->execute(); + $configs = msort($configs, 'getSortVector'); + + foreach ($configs as $config) { + $provider = $config->getProvider(); if (!$provider->shouldAllowAccountLink()) { continue; } + // Don't show the user providers they already have linked. + $provider_key = $config->getProvider()->getProviderKey(); + if (isset($accounts[$provider_key])) { + continue; + } + $link_uri = '/auth/link/'.$provider->getProviderKey().'/'; - $item = id(new PHUIObjectItemView()) - ->setHeader($provider->getProviderName()) + $link_button = id(new PHUIButtonView()) + ->setTag('a') + ->setIcon('fa-link') ->setHref($link_uri) - ->addAction( - id(new PHUIListItemView()) - ->setIcon('fa-link') - ->setHref($link_uri)); + ->setColor(PHUIButtonView::GREY) + ->setText(pht('Link External Account')); + + $item = id(new PHUIObjectItemView()) + ->setHeader($config->getDisplayName()) + ->setHref($link_uri) + ->setImageIcon($config->newIconView()) + ->setSideColumn($link_button); $linkable->addItem($item); } diff --git a/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php b/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php index 6378ee29d3..3ef48c01e0 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php +++ b/src/applications/settings/storage/PhabricatorUserPreferencesTransaction.php @@ -11,10 +11,6 @@ final class PhabricatorUserPreferencesTransaction return 'user'; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getApplicationTransactionType() { return PhabricatorUserPreferencesPHIDType::TYPECONST; } diff --git a/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php b/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php index 62913b09e3..e1a1b9df34 100644 --- a/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php +++ b/src/applications/slowvote/controller/PhabricatorSlowvoteVoteController.php @@ -37,6 +37,19 @@ final class PhabricatorSlowvoteVoteController $method = $poll->getMethod(); $is_plurality = ($method == PhabricatorSlowvotePoll::METHOD_PLURALITY); + if (!$votes) { + if ($is_plurality) { + $message = pht('You must vote for something.'); + } else { + $message = pht('You must vote for at least one option.'); + } + + return $this->newDialog() + ->setTitle(pht('Stand For Something')) + ->appendParagraph($message) + ->addCancelButton($poll->getURI()); + } + if ($is_plurality && count($votes) > 1) { throw new Exception( pht('In this poll, you may only vote for one option.')); @@ -52,23 +65,39 @@ final class PhabricatorSlowvoteVoteController } } - foreach ($old_votes as $old_vote) { - if (!idx($votes, $old_vote->getOptionID(), false)) { + $poll->openTransaction(); + $poll->beginReadLocking(); + + $poll->reload(); + + $old_votes = id(new PhabricatorSlowvoteChoice())->loadAllWhere( + 'pollID = %d AND authorPHID = %s', + $poll->getID(), + $viewer->getPHID()); + $old_votes = mpull($old_votes, null, 'getOptionID'); + + foreach ($old_votes as $old_vote) { + if (idx($votes, $old_vote->getOptionID())) { + continue; + } + $old_vote->delete(); } - } - foreach ($votes as $vote) { - if (idx($old_votes, $vote, false)) { - continue; + foreach ($votes as $vote) { + if (idx($old_votes, $vote)) { + continue; + } + + id(new PhabricatorSlowvoteChoice()) + ->setAuthorPHID($viewer->getPHID()) + ->setPollID($poll->getID()) + ->setOptionID($vote) + ->save(); } - id(new PhabricatorSlowvoteChoice()) - ->setAuthorPHID($viewer->getPHID()) - ->setPollID($poll->getID()) - ->setOptionID($vote) - ->save(); - } + $poll->endReadLocking(); + $poll->saveTransaction(); return id(new AphrontRedirectResponse()) ->setURI($poll->getURI()); diff --git a/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php b/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php index 0f50a870f6..bac0ea636f 100644 --- a/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php +++ b/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php @@ -11,10 +11,6 @@ final class PhabricatorSpacesNamespaceTransaction return PhabricatorSpacesNamespacePHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getBaseTransactionClass() { return 'PhabricatorSpacesNamespaceTransactionType'; } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 91825eb73d..bd066e633b 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -360,12 +360,7 @@ abstract class PhabricatorApplicationTransactionEditor } if ($template) { - try { - $comment = $template->getApplicationTransactionCommentObject(); - } catch (PhutilMethodNotImplementedException $ex) { - $comment = null; - } - + $comment = $template->getApplicationTransactionCommentObject(); if ($comment) { $types[] = PhabricatorTransactions::TYPE_COMMENT; } @@ -1120,6 +1115,16 @@ abstract class PhabricatorApplicationTransactionEditor $transaction_open = true; } + // We can technically test any object for CAN_INTERACT, but we can + // run into some issues in doing so (for example, in project unit tests). + // For now, only test for CAN_INTERACT if the object is explicitly a + // lockable object. + + $was_locked = false; + if ($object instanceof PhabricatorEditEngineLockableInterface) { + $was_locked = !PhabricatorPolicyFilter::canInteract($actor, $object); + } + foreach ($xactions as $xaction) { $this->applyInternalEffects($object, $xaction); } @@ -1137,6 +1142,10 @@ abstract class PhabricatorApplicationTransactionEditor } foreach ($xactions as $xaction) { + if ($was_locked) { + $xaction->setIsLockOverrideTransaction(true); + } + $xaction->setObjectPHID($object->getPHID()); if ($xaction->getComment()) { $xaction->setPHID($xaction->generatePHID()); diff --git a/src/applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php index 0d20533798..7d80082cb2 100644 --- a/src/applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php +++ b/src/applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php @@ -23,12 +23,7 @@ final class PhabricatorCommentEditEngineExtension PhabricatorApplicationTransactionInterface $object) { $xaction = $object->getApplicationTransactionTemplate(); - - try { - $comment = $xaction->getApplicationTransactionCommentObject(); - } catch (PhutilMethodNotImplementedException $ex) { - $comment = null; - } + $comment = $xaction->getApplicationTransactionCommentObject(); return (bool)$comment; } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 6d047fc823..d71728a01f 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -76,7 +76,7 @@ abstract class PhabricatorApplicationTransaction } public function getApplicationTransactionCommentObject() { - throw new PhutilMethodNotImplementedException(); + return null; } public function getMetadataValue($key, $default = null) { @@ -169,6 +169,14 @@ abstract class PhabricatorApplicationTransaction return (bool)$this->getMetadataValue('core.mfa', false); } + public function setIsLockOverrideTransaction($override) { + return $this->setMetadataValue('core.lock-override', $override); + } + + public function getIsLockOverrideTransaction() { + return (bool)$this->getMetadataValue('core.lock-override', false); + } + public function attachComment( PhabricatorApplicationTransactionComment $comment) { $this->comment = $comment; @@ -1529,6 +1537,12 @@ abstract class PhabricatorApplicationTransaction return false; } } + + // Don't group lock override and non-override transactions together. + $is_override = $this->getIsLockOverrideTransaction(); + if ($is_override != $xaction->getIsLockOverrideTransaction()) { + return false; + } } return true; @@ -1731,12 +1745,7 @@ abstract class PhabricatorApplicationTransaction PhabricatorDestructionEngine $engine) { $this->openTransaction(); - $comment_template = null; - try { - $comment_template = $this->getApplicationTransactionCommentObject(); - } catch (Exception $ex) { - // Continue; no comments for these transactions. - } + $comment_template = $this->getApplicationTransactionCommentObject(); if ($comment_template) { $comments = $comment_template->loadAllWhere( diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php index a1c44cc003..8cf7fe5b48 100644 --- a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php @@ -23,10 +23,6 @@ final class PhabricatorEditEngineConfigurationTransaction return PhabricatorEditEngineConfigurationPHIDType::TYPECONST; } - public function getApplicationTransactionCommentObject() { - return null; - } - public function getTitle() { $author_phid = $this->getAuthorPHID(); diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php index c2b32aa190..4d738877b8 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -416,7 +416,8 @@ class PhabricatorApplicationTransactionView extends AphrontView { ->setColor($xaction->getColor()) ->setHideCommentOptions($this->getHideCommentOptions()) ->setIsSilent($xaction->getIsSilentTransaction()) - ->setIsMFA($xaction->getIsMFATransaction()); + ->setIsMFA($xaction->getIsMFATransaction()) + ->setIsLockOverride($xaction->getIsLockOverrideTransaction()); list($token, $token_removed) = $xaction->getToken(); if ($token) { diff --git a/src/docs/user/configuration/configuring_outbound_email.diviner b/src/docs/user/configuration/configuring_outbound_email.diviner index 4d18ba0eb2..6f5212680e 100644 --- a/src/docs/user/configuration/configuring_outbound_email.diviner +++ b/src/docs/user/configuration/configuring_outbound_email.diviner @@ -47,18 +47,31 @@ not. For more information on using daemons, see @{article:Managing Daemons with phd}. -Basics -====== +Outbound "From" and "To" Addresses +================================== -Before configuring outbound mail, you should first set up -`metamta.default-address` in Configuration. This determines where mail is sent -"From" by default. +When Phabricator sends outbound mail, it must select some "From" address to +send mail from, since mailers require this. -If your domain is `example.org`, set this to something -like `noreply@example.org`. +When mail only has "CC" recipients, Phabricator generates a dummy "To" address, +since some mailers require this and some users write mail rules that depend +on whether they appear in the "To" or "CC" line. -Ideally, this should be a valid, deliverable address that doesn't bounce if -users accidentally send mail to it. +In both cases, the address should ideally correspond to a valid, deliverable +mailbox that accepts the mail and then simply discards it. If the address is +not valid, some outbound mail will bounce, and users will receive bounces when +they "Reply All" even if the other recipients for the message are valid. In +contrast, if the address is a real user address, that user will receive a lot +of mail they probably don't want. + +If you plan to configure //inbound// mail later, you usually don't need to do +anything. Phabricator will automatically create a `noreply@` mailbox which +works the right way (accepts and discards all mail it receives) and +automatically use it when generating addresses. + +If you don't plan to configure inbound mail, you may need to configure an +address for Phabricator to use. You can do this by setting +`metamta.default-address`. Configuring Mailers diff --git a/src/docs/user/userguide/diviner.diviner b/src/docs/user/userguide/diviner.diviner index e94c33d275..01484be14c 100644 --- a/src/docs/user/userguide/diviner.diviner +++ b/src/docs/user/userguide/diviner.diviner @@ -3,17 +3,28 @@ Using Diviner, a documentation generator. -= Overview = +Overview +======== -NOTE: Diviner is new and not yet generally useful. +Diviner is an application for creating technical documentation. -= Generating Documentation = +This article is maintained in a text file in the Phabricator repository and +generated into the display document you are currently reading using Diviner. + +Beyond generating articles, Diviner can also analyze source code and generate +documentation about classes, methods, and other primitives. + + +Generating Documentation +======================== To generate documentation, run: phabricator/ $ ./bin/diviner generate --book -= .book Files = + +Diviner ".book" Files +===================== Diviner documentation books are configured using JSON `.book` files, which look like this: diff --git a/src/docs/user/userguide/owners.diviner b/src/docs/user/userguide/owners.diviner index 95a3882552..11dee8941a 100644 --- a/src/docs/user/userguide/owners.diviner +++ b/src/docs/user/userguide/owners.diviner @@ -114,16 +114,23 @@ Auditing ======== You can automatically trigger audits on unreviewed code by configuring -**Auditing**. The available settings are: +**Auditing**. The available settings allow you to select behavior based on +these conditions: - - **Disabled**: Do not trigger audits. - - **Enabled**: Trigger audits. + - **No Owner Involvement**: Triggers an audit when the commit author is not + a package owner, and no package owner reviewed an associated revision in + Differential. + - **Unreviewed Commits**: Triggers an audit when a commit has no associated + revision in Differential, or the associated revision in Differential landed + without being "Accepted". -When enabled, audits are triggered for commits which: +For example, the **Audit Commits With No Owner Involvement** option triggers +audits for commits which: - affect code owned by the package; - were not authored by a package owner; and - - were not accepted by a package owner. + - were not accepted (in Differential) by a package owner or the package + itself. Audits do not trigger if the package has been archived. diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php index 5bc83972dd..acbbb4fbda 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php @@ -432,7 +432,7 @@ abstract class PhabricatorStorageManagementWorkflow case 'key': if (($phase == 'drop_keys') && $adjust['exists']) { if ($adjust['name'] == 'PRIMARY') { - $key_name = 'PRIMARY KEY'; + $key_name = qsprintf($conn, 'PRIMARY KEY'); } else { $key_name = qsprintf($conn, 'KEY %T', $adjust['name']); } diff --git a/src/infrastructure/util/PhabricatorMetronome.php b/src/infrastructure/util/PhabricatorMetronome.php new file mode 100644 index 0000000000..24f58127f6 --- /dev/null +++ b/src/infrastructure/util/PhabricatorMetronome.php @@ -0,0 +1,92 @@ +offset = $offset; + + return $this; + } + + public function setFrequency($frequency) { + if (!is_int($frequency)) { + throw new Exception(pht('Metronome frequency must be an integer.')); + } + + if ($frequency < 1) { + throw new Exception(pht('Metronome frequency must be 1 or more.')); + } + + $this->frequency = $frequency; + + return $this; + } + + public function setOffsetFromSeed($seed) { + $offset = PhabricatorHash::digestToRange($seed, 0, PHP_INT_MAX); + return $this->setOffset($offset); + } + + public function getFrequency() { + if ($this->frequency === null) { + throw new PhutilInvalidStateException('setFrequency'); + } + return $this->frequency; + } + + public function getOffset() { + $frequency = $this->getFrequency(); + return ($this->offset % $frequency); + } + + public function getNextTickAfter($epoch) { + $frequency = $this->getFrequency(); + $offset = $this->getOffset(); + + $remainder = ($epoch % $frequency); + + if ($remainder < $offset) { + return ($epoch - $remainder) + $offset; + } else { + return ($epoch - $remainder) + $frequency + $offset; + } + } + + public function didTickBetween($min, $max) { + if ($max < $min) { + throw new Exception( + pht( + 'Maximum tick window must not be smaller than minimum tick window.')); + } + + $next = $this->getNextTickAfter($min); + return ($next <= $max); + } + +} diff --git a/src/infrastructure/util/__tests__/PhabricatorMetronomeTestCase.php b/src/infrastructure/util/__tests__/PhabricatorMetronomeTestCase.php new file mode 100644 index 0000000000..9ad74e2b90 --- /dev/null +++ b/src/infrastructure/util/__tests__/PhabricatorMetronomeTestCase.php @@ -0,0 +1,61 @@ + 44, + 'web002.example.net' => 36, + 'web003.example.net' => 25, + 'web004.example.net' => 25, + 'web005.example.net' => 16, + 'web006.example.net' => 26, + 'web007.example.net' => 35, + 'web008.example.net' => 14, + ); + + $metronome = id(new PhabricatorMetronome()) + ->setFrequency(60); + + foreach ($cases as $input => $expect) { + $metronome->setOffsetFromSeed($input); + + $this->assertEqual( + $expect, + $metronome->getOffset(), + pht('Offset for: %s', $input)); + } + } + + public function testMetronomeTicks() { + $metronome = id(new PhabricatorMetronome()) + ->setFrequency(60) + ->setOffset(13); + + $tick_epoch = strtotime('2000-01-01 11:11:13 AM UTC'); + + // Since the epoch is at "0:13" on the clock, the metronome should tick + // then. + $this->assertEqual( + $tick_epoch, + $metronome->getNextTickAfter($tick_epoch - 1), + pht('Tick at 11:11:13 AM.')); + + // The next tick should be a minute later. + $this->assertEqual( + $tick_epoch + 60, + $metronome->getNextTickAfter($tick_epoch), + pht('Tick at 11:12:13 AM.')); + + + // There's no tick in the next 59 seconds. + $this->assertFalse( + $metronome->didTickBetween($tick_epoch, $tick_epoch + 59)); + + $this->assertTrue( + $metronome->didTickBetween($tick_epoch, $tick_epoch + 60)); + } + + +} diff --git a/src/view/form/control/AphrontFormTextControl.php b/src/view/form/control/AphrontFormTextControl.php index 581f22682d..f7fd117cfd 100644 --- a/src/view/form/control/AphrontFormTextControl.php +++ b/src/view/form/control/AphrontFormTextControl.php @@ -5,6 +5,7 @@ final class AphrontFormTextControl extends AphrontFormControl { private $disableAutocomplete; private $sigil; private $placeholder; + private $autofocus; public function setDisableAutocomplete($disable) { $this->disableAutocomplete = $disable; @@ -24,6 +25,15 @@ final class AphrontFormTextControl extends AphrontFormControl { return $this; } + public function setAutofocus($autofocus) { + $this->autofocus = $autofocus; + return $this; + } + + public function getAutofocus() { + return $this->autofocus; + } + public function getSigil() { return $this->sigil; } @@ -49,6 +59,7 @@ final class AphrontFormTextControl extends AphrontFormControl { 'id' => $this->getID(), 'sigil' => $this->getSigil(), 'placeholder' => $this->getPlaceholder(), + 'autofocus' => ($this->getAutofocus() ? 'autofocus' : null), )); } diff --git a/src/view/form/control/PHUIFormNumberControl.php b/src/view/form/control/PHUIFormNumberControl.php index 26e7e03955..c577bebbd0 100644 --- a/src/view/form/control/PHUIFormNumberControl.php +++ b/src/view/form/control/PHUIFormNumberControl.php @@ -3,6 +3,7 @@ final class PHUIFormNumberControl extends AphrontFormControl { private $disableAutocomplete; + private $autofocus; public function setDisableAutocomplete($disable_autocomplete) { $this->disableAutocomplete = $disable_autocomplete; @@ -13,6 +14,15 @@ final class PHUIFormNumberControl extends AphrontFormControl { return $this->disableAutocomplete; } + public function setAutofocus($autofocus) { + $this->autofocus = $autofocus; + return $this; + } + + public function getAutofocus() { + return $this->autofocus; + } + protected function getCustomControlClass() { return 'phui-form-number'; } @@ -34,6 +44,7 @@ final class PHUIFormNumberControl extends AphrontFormControl { 'disabled' => $this->getDisabled() ? 'disabled' : null, 'autocomplete' => $autocomplete, 'id' => $this->getID(), + 'autofocus' => ($this->getAutofocus() ? 'autofocus' : null), )); } diff --git a/src/view/layout/PhabricatorActionView.php b/src/view/layout/PhabricatorActionView.php index a1d8fe2664..3de60a2374 100644 --- a/src/view/layout/PhabricatorActionView.php +++ b/src/view/layout/PhabricatorActionView.php @@ -25,6 +25,7 @@ final class PhabricatorActionView extends AphrontView { const TYPE_DIVIDER = 'type-divider'; const TYPE_LABEL = 'label'; const RED = 'action-item-red'; + const GREEN = 'action-item-green'; public function setSelected($selected) { $this->selected = $selected; diff --git a/src/view/phui/PHUIIconView.php b/src/view/phui/PHUIIconView.php index 8cc61ba2eb..d907cb3343 100644 --- a/src/view/phui/PHUIIconView.php +++ b/src/view/phui/PHUIIconView.php @@ -19,6 +19,7 @@ final class PHUIIconView extends AphrontTagView { private $iconColor; private $iconBackground; private $tooltip; + private $emblemColor; public function setHref($href) { $this->href = $href; @@ -66,6 +67,15 @@ final class PHUIIconView extends AphrontTagView { return $this; } + public function setEmblemColor($emblem_color) { + $this->emblemColor = $emblem_color; + return $this; + } + + public function getEmblemColor() { + return $this->emblemColor; + } + protected function getTagName() { $tag = 'span'; if ($this->href) { @@ -106,6 +116,10 @@ final class PHUIIconView extends AphrontTagView { $this->appendChild($this->text); } + if ($this->emblemColor) { + $classes[] = 'phui-icon-emblem phui-icon-emblem-'.$this->emblemColor; + } + $sigil = null; $meta = array(); if ($this->tooltip) { diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php index 86628058fe..78a75a2063 100644 --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -31,6 +31,7 @@ final class PHUITimelineEventView extends AphrontView { private $pinboardItems = array(); private $isSilent; private $isMFA; + private $isLockOverride; public function setAuthorPHID($author_phid) { $this->authorPHID = $author_phid; @@ -197,6 +198,15 @@ final class PHUITimelineEventView extends AphrontView { return $this->isMFA; } + public function setIsLockOverride($is_override) { + $this->isLockOverride = $is_override; + return $this; + } + + public function getIsLockOverride() { + return $this->isLockOverride; + } + public function setReallyMajorEvent($me) { $this->reallyMajorEvent = $me; return $this; @@ -597,7 +607,8 @@ final class PHUITimelineEventView extends AphrontView { // not expect to have received any mail or notifications. if ($this->getIsSilent()) { $extra[] = id(new PHUIIconView()) - ->setIcon('fa-bell-slash', 'red') + ->setIcon('fa-bell-slash', 'white') + ->setEmblemColor('red') ->setTooltip(pht('Silent Edit')); } @@ -605,9 +616,17 @@ final class PHUITimelineEventView extends AphrontView { // provide a hint that it was extra authentic. if ($this->getIsMFA()) { $extra[] = id(new PHUIIconView()) - ->setIcon('fa-vcard', 'pink') + ->setIcon('fa-vcard', 'white') + ->setEmblemColor('pink') ->setTooltip(pht('MFA Authenticated')); } + + if ($this->getIsLockOverride()) { + $extra[] = id(new PHUIIconView()) + ->setIcon('fa-chain-broken', 'white') + ->setEmblemColor('violet') + ->setTooltip(pht('Lock Overridden')); + } } $extra = javelin_tag( diff --git a/webroot/rsrc/css/aphront/table-view.css b/webroot/rsrc/css/aphront/table-view.css index 1a7d2eb215..9bc8536054 100644 --- a/webroot/rsrc/css/aphront/table-view.css +++ b/webroot/rsrc/css/aphront/table-view.css @@ -45,16 +45,20 @@ background: inherit; } -.aphront-table-view th { +.aphront-table-view th, +.aphront-table-view td.header { font-weight: bold; white-space: nowrap; color: {$bluetext}; - text-shadow: 0 1px 0 white; font-weight: bold; - border-bottom: 1px solid {$thinblueborder}; + text-shadow: 0 1px 0 white; background-color: {$lightbluebackground}; } +.aphront-table-view th { + border-bottom: 1px solid {$thinblueborder}; +} + th.aphront-table-view-sortable-selected { background-color: {$greybackground}; } @@ -74,12 +78,8 @@ th.aphront-table-view-sortable-selected { } .aphront-table-view td.header { - padding: 4px 8px; - white-space: nowrap; text-align: right; - color: {$bluetext}; - font-weight: bold; - vertical-align: top; + border-right: 1px solid {$thinblueborder}; } .aphront-table-view td { diff --git a/webroot/rsrc/css/phui/phui-action-list.css b/webroot/rsrc/css/phui/phui-action-list.css index e7ee38a8bf..3df4ff1b78 100644 --- a/webroot/rsrc/css/phui/phui-action-list.css +++ b/webroot/rsrc/css/phui/phui-action-list.css @@ -99,11 +99,20 @@ background-color: {$sh-redbackground}; } +.phabricator-action-view.action-item-green { + background-color: {$sh-greenbackground}; +} + .phabricator-action-view.action-item-red .phabricator-action-view-item, .phabricator-action-view.action-item-red .phabricator-action-view-icon { color: {$sh-redtext}; } +.phabricator-action-view.action-item-green .phabricator-action-view-item, +.phabricator-action-view.action-item-green .phabricator-action-view-icon { + color: {$sh-greentext}; +} + .device-desktop .phabricator-action-view.action-item-red:hover .phabricator-action-view-item, .device-desktop .phabricator-action-view.action-item-red:hover @@ -111,6 +120,14 @@ color: {$red}; } +.device-desktop .phabricator-action-view.action-item-green:hover + .phabricator-action-view-item, +.device-desktop .phabricator-action-view.action-item-green:hover + .phabricator-action-view-icon { + color: {$green}; +} + + .phabricator-action-view-label .phabricator-action-view-item, .phabricator-action-view-type-label .phabricator-action-view-item { font-size: {$smallerfontsize}; diff --git a/webroot/rsrc/css/phui/phui-icon.css b/webroot/rsrc/css/phui/phui-icon.css index 4108074b08..5436bb04b1 100644 --- a/webroot/rsrc/css/phui/phui-icon.css +++ b/webroot/rsrc/css/phui/phui-icon.css @@ -183,3 +183,24 @@ a.phui-icon-view.phui-icon-square:hover { text-decoration: none; color: #fff; } + + +.phui-icon-emblem { + border-radius: 4px; +} + +.phui-timeline-extra .phui-icon-emblem { + padding: 4px 6px; +} + +.phui-icon-emblem-violet { + background-color: {$violet}; +} + +.phui-icon-emblem-red { + background-color: {$red}; +} + +.phui-icon-emblem-pink { + background-color: {$pink}; +}