diff --git a/src/applications/auth/controller/PhabricatorAuthController.php b/src/applications/auth/controller/PhabricatorAuthController.php --- a/src/applications/auth/controller/PhabricatorAuthController.php +++ b/src/applications/auth/controller/PhabricatorAuthController.php @@ -143,12 +143,19 @@ // be logged in yet, and because we want to tailor an error message to // distinguish between "not usable" and "does not exist". We do explicit // checks later on to make sure this account is valid for the intended - // operation. + // operation. This requires edit permission for completeness and consistency + // but it won't actually be meaningfully checked because we're using the + // ominpotent user. $account = id(new PhabricatorExternalAccountQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withAccountSecrets(array($account_key)) ->needImages(true) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$account) { diff --git a/src/applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php b/src/applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php --- a/src/applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php +++ b/src/applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php @@ -36,7 +36,12 @@ $viewer = $this->getViewer(); $query = id(new PhabricatorExternalAccountQuery()) - ->setViewer($viewer); + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )); $username = $args->getArg('user'); if (strlen($username)) { diff --git a/src/applications/auth/query/PhabricatorExternalAccountQuery.php b/src/applications/auth/query/PhabricatorExternalAccountQuery.php --- a/src/applications/auth/query/PhabricatorExternalAccountQuery.php +++ b/src/applications/auth/query/PhabricatorExternalAccountQuery.php @@ -1,5 +1,15 @@ <?php +/** + * NOTE: When loading ExternalAccounts for use in an authetication context (that + * is, you're going to act as the account or link identities or anything like + * that) you should require CAN_EDIT capability even if you aren't actually + * editing the ExternalAccount. + * + * ExternalAccounts have a permissive CAN_VIEW policy (like users) because they + * interact directly with objects and can leave comments, sign documents, etc. + * However, CAN_EDIT is restricted to users who own the accounts. + */ final class PhabricatorExternalAccountQuery extends PhabricatorCursorPagedPolicyAwareQuery { diff --git a/src/applications/differential/landing/DifferentialLandingToGitHub.php b/src/applications/differential/landing/DifferentialLandingToGitHub.php --- a/src/applications/differential/landing/DifferentialLandingToGitHub.php +++ b/src/applications/differential/landing/DifferentialLandingToGitHub.php @@ -101,6 +101,11 @@ ->withUserPHIDs(array($viewer->getPHID())) ->withAccountTypes(array('github')) ->withAccountDomains(array($repo_domain)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$this->account) { diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php --- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php @@ -37,6 +37,11 @@ ->withUserPHIDs(array($viewer->getPHID())) ->withAccountTypes(array($provider->getProviderType())) ->withAccountDomains(array($provider->getProviderDomain())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); if (!$accounts) { diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php --- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php +++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php @@ -31,6 +31,11 @@ ->setViewer($viewer) ->withUserPHIDs(array($viewer->getPHID())) ->withAccountTypes(array($provider->getProviderType())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); if (!$accounts) { diff --git a/src/applications/doorkeeper/option/PhabricatorAsanaConfigOptions.php b/src/applications/doorkeeper/option/PhabricatorAsanaConfigOptions.php --- a/src/applications/doorkeeper/option/PhabricatorAsanaConfigOptions.php +++ b/src/applications/doorkeeper/option/PhabricatorAsanaConfigOptions.php @@ -59,6 +59,11 @@ ->withUserPHIDs(array($viewer->getPHID())) ->withAccountTypes(array($provider->getProviderType())) ->withAccountDomains(array($provider->getProviderDomain())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$account) { return null; diff --git a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php --- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php +++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php @@ -524,6 +524,11 @@ ->withUserPHIDs($all_phids) ->withAccountTypes(array($provider->getProviderType())) ->withAccountDomains(array($provider->getProviderDomain())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); foreach ($accounts as $account) { @@ -549,6 +554,11 @@ ->withUserPHIDs($user_phids) ->withAccountTypes(array($provider->getProviderType())) ->withAccountDomains(array($provider->getProviderDomain())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); // Reorder accounts in the original order. diff --git a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerJIRA.php b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerJIRA.php --- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerJIRA.php +++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerJIRA.php @@ -63,6 +63,11 @@ ->withUserPHIDs($try_users) ->withAccountTypes(array($provider->getProviderType())) ->withAccountDomains(array($domain)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); // Reorder accounts in the original order. // TODO: This needs to be adjusted if/when we allow you to link multiple diff --git a/src/applications/metamta/receiver/PhabricatorMailReceiver.php b/src/applications/metamta/receiver/PhabricatorMailReceiver.php --- a/src/applications/metamta/receiver/PhabricatorMailReceiver.php +++ b/src/applications/metamta/receiver/PhabricatorMailReceiver.php @@ -107,6 +107,11 @@ ->withAccountTypes(array('email')) ->withAccountDomains(array($from_obj->getDomainName(), 'self')) ->withAccountIDs(array($from_obj->getAddress())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->loadOneOrCreate(); return $xuser->getPhabricatorUser(); } else { diff --git a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php --- a/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php @@ -125,6 +125,11 @@ ->setViewer($viewer) ->withUserPHIDs(array($user->getPHID())) ->needImages(true) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); foreach ($accounts as $account) { diff --git a/src/applications/people/storage/PhabricatorExternalAccount.php b/src/applications/people/storage/PhabricatorExternalAccount.php --- a/src/applications/people/storage/PhabricatorExternalAccount.php +++ b/src/applications/people/storage/PhabricatorExternalAccount.php @@ -84,6 +84,25 @@ return true; } + public function getDisplayName() { + if (strlen($this->displayName)) { + return $this->displayName; + } + + // TODO: Figure out how much identifying information we're going to show + // to users about external accounts. For now, just show a string which is + // clearly not an error, but don't disclose any identifying information. + + $map = array( + 'email' => pht('Email User'), + ); + + $type = $this->getAccountType(); + + return idx($map, $type, pht('"%s" User', $type)); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -91,11 +110,17 @@ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } public function getPolicy($capability) { - return PhabricatorPolicies::POLICY_NOONE; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicies::getMostOpenPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + return PhabricatorPolicies::POLICY_NOONE; + } } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { @@ -103,8 +128,13 @@ } public function describeAutomaticCapability($capability) { - // TODO: (T603) This is complicated. - return null; + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return null; + case PhabricatorPolicyCapability::CAN_EDIT: + return pht( + 'External accounts can only be edited by the account owner.'); + } } } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php b/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php --- a/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelExternalAccounts.php @@ -28,6 +28,11 @@ ->setViewer($viewer) ->withUserPHIDs(array($viewer->getPHID())) ->needImages(true) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); $linked_head = id(new PHUIHeaderView()) diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php b/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php --- a/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelSessions.php @@ -25,6 +25,11 @@ $accounts = id(new PhabricatorExternalAccountQuery()) ->setViewer($viewer) ->withUserPHIDs(array($viewer->getPHID())) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->execute(); $identity_phids = mpull($accounts, 'getPHID');