20after4 has uploaded a new change for review. https://gerrit.wikimedia.org/r/168907
Change subject: Sanitize herald effects so that for tasks submitted to a secure project, the following must be enforced: ...................................................................... Sanitize herald effects so that for tasks submitted to a secure project, the following must be enforced: * CC'd users must be a member of secure project * assignee must be a member of the secure project Change-Id: Iceb26d4fac99eca1d52cacf2431a069ffe52fb84 --- M SecurityPolicyEnforcerAction.php 1 file changed, 79 insertions(+), 29 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/phabricator/extensions refs/changes/07/168907/1 diff --git a/SecurityPolicyEnforcerAction.php b/SecurityPolicyEnforcerAction.php index fde9f61..d599757 100644 --- a/SecurityPolicyEnforcerAction.php +++ b/SecurityPolicyEnforcerAction.php @@ -36,6 +36,7 @@ /** @var ManiphestTask */ $task = $object; + $project = null; $viewer = PhabricatorUser::getOmnipotentUser(); @@ -150,7 +151,8 @@ $enforce = true; //operations group - $project_phids = array($this->getProjectPHID('operations')); + $project = $this->getProjectByName('operations'); + $project_phids = array($project->getPHID()); $policy = $this->createCustomPolicy( $task->getAuthorPHID(), $project_phids); @@ -159,7 +161,8 @@ break; case 'security-bug': - $project_phids = array($this->getProjectPHID('security')); + $project = $this->getProjectByName('security'); + $project_phids = array($project->getPHID()); $policy = $this->createCustomPolicy( $task->getAuthorPHID(), @@ -174,7 +177,9 @@ $transactions = array(); if ($enforce) { - + if ($project !== null) { + $this->sanityCheck($adapter, $project); + } if ($view_policy !== null) { $transactions[] = id(new ManiphestTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) @@ -220,9 +225,16 @@ * look up a project by name */ protected function getProjectPHID($projectName) { - static $phids = array(); - if (isset($phids[$projectName])){ - return $phids[$projectName]; + return $this->getProjectByName($projectName)->getPHID(); + } + + /** + * look up a project by name + */ + protected function getProjectByName($projectName) { + static $projects = array(); + if (isset($projects[$projectName])){ + return $projects[$projectName]; } $query = new PhabricatorProjectQuery(); @@ -234,33 +246,71 @@ return null; } - $phids[$projectName] = $project->getPHID(); - return $phids[$projectName]; + return $projects[$projectName] = $project; } protected function createCustomPolicy($user_phids, $project_phids) { - if (!is_array($user_phids)){ - $user_phids = array($user_phids); - } - if (!is_array($project_phids)) { - $project_phids = array($project_phids); - } + if (!is_array($user_phids)){ + $user_phids = array($user_phids); + } + if (!is_array($project_phids)) { + $project_phids = array($project_phids); + } - $policy = id(new PhabricatorPolicy()) - ->setRules( + $policy = id(new PhabricatorPolicy()) + ->setRules( + array( array( - array( - 'action' => PhabricatorPolicy::ACTION_ALLOW, - 'rule' => 'PhabricatorPolicyRuleUsers', - 'value' => $user_phids, - ), - array( - 'action' => PhabricatorPolicy::ACTION_ALLOW, - 'rule' => 'PhabricatorPolicyRuleProjects', - 'value' => $project_phids, - ), - )) - ->save(); - return $policy; + 'action' => PhabricatorPolicy::ACTION_ALLOW, + 'rule' => 'PhabricatorPolicyRuleUsers', + 'value' => $user_phids, + ), + array( + 'action' => PhabricatorPolicy::ACTION_ALLOW, + 'rule' => 'PhabricatorPolicyRuleProjects', + 'value' => $project_phids, + ), + )) + ->save(); + return $policy; } + +/** + * Sanitize herald effects so that for tasks submitted to a secure project, + * the following will be enforced: + * * CC'd users must be a member of secure project + * * assignee must be a member of the secure project +*/ + protected function sanityCheck( + HeraldManiphestTaskAdapter $adapter, + PhabricatorProject $project) { + + $ccs = $adapter->getCcPHIDs(); + $members = $project->getMemberPHIDs(); + $member_phids = array(); + $sanitized_ccs = array(); + + foreach($members as $key=>$phid) { + $member_phids[$phid]=true; + } + + //CC'd users must be members of secure project + foreach($ccs as $cc) { + if (isset($member_phids[$cc])) { + $sanitized_ccs[] = $cc; + } + } + $adapter->setCcPHIDs($sanitized_ccs); + + //assignee must be a member of secure project: + $assign_phid = $adapter->getAssignPHID(); + if ($assign_phid) { + if (!isset($member_phids[$assign_phid])) { + $adapter->setAssignPHID(null); + } + } + + + } + } -- To view, visit https://gerrit.wikimedia.org/r/168907 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iceb26d4fac99eca1d52cacf2431a069ffe52fb84 Gerrit-PatchSet: 1 Gerrit-Project: phabricator/extensions Gerrit-Branch: master Gerrit-Owner: 20after4 <mmod...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits