Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15451939
D9767.id23449.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D9767.id23449.diff
View Options
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');
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sun, Mar 30, 12:44 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7713573
Default Alt Text
D9767.id23449.diff (11 KB)
Attached To
Mode
D9767: Introduce CAN_EDIT for ExternalAccount, and make CAN_VIEW more liberal
Attached
Detach File
Event Timeline
Log In to Comment