Differential D13626 Diff 32928 src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php

Changeset View

Standalone View

src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php

This file was added.

				<?php

				final class PhabricatorBadgesEditRecipientsController
				extends PhabricatorBadgesController {

				private $id;

				public function willProcessRequest(array $data) {
				$this->id = $data['id'];
				}

				epriestleyUnsubmitted Done Inline Actions Remove now that you're using getURIData(). epriestley: Remove now that you're using getURIData().
				public function processRequest() {
				$request = $this->getRequest();
				$user = $request->getUser();
				$id = $request->getURIData('id');
				epriestleyUnsubmitted Done Inline Actions handleRequest / getViewer (see below) epriestley: handleRequest / getViewer (see below)

				$badge = id(new PhabricatorBadgesQuery())
				->setViewer($user)
				->withIDs(array($this->id))
				->needRecipients(true)
				->requireCapabilities(
				array(
				PhabricatorPolicyCapability::CAN_VIEW,
				))
				->executeOne();
				epriestleyUnsubmitted Done Inline Actions I think this controller does not enforce policies correctly. Although it changes the form based on the user's edit permissions, it does not enforce that a user must actually have permission in order to perform edits. An attacker can just edit the HTML of the form on the client, then submit whatever parameters they want, removing other users' badges. The actual edits need to have the checks applied to them. (Or maybe I'm misunderstanding, I haven't actually patched this locally.) epriestley: I think this controller does not enforce policies correctly. Although it changes the //form//…
				chadAuthorUnsubmitted Done Inline Actions Worth noting then that this is copy/pastey from Project Membership. Not sure what you want to to change here (iNoob). https://secure.phabricator.com/diffusion/P/browse/master/src/applications/project/controller/PhabricatorProjectMembersEditController.php;ef556090f86d57b3d334f486c412dedd6314d2d8$23-25 chad: Worth noting then that this is copy/pastey from Project Membership. Not sure what you want to…
				if (!$badge) {
				return new Aphront404Response();
				}

				$recipient_phids = $badge->getRecipientPHIDs();

				if ($request->isFormPost()) {
				$recipient_spec = array();

				$remove = $request->getStr('remove');
				if ($remove) {
				$recipient_spec['-'] = array_fuse(array($remove));
				}

				$add_recipients = $request->getArr('phids');
				if ($add_recipients) {
				$recipient_spec['+'] = array_fuse($add_recipients);
				}

				$type_recipient = PhabricatorBadgeHasRecipientEdgeType::EDGECONST;

				$xactions = array();

				$xactions[] = id(new PhabricatorBadgesTransaction())
				->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
				->setMetadataValue('edge:type', $type_recipient)
				->setNewValue($recipient_spec);

				$editor = id(new PhabricatorBadgesEditor($badge))
				->setActor($user)
				->setContentSourceFromRequest($request)
				->setContinueOnNoEffect(true)
				->setContinueOnMissingFields(true)
				->applyTransactions($badge, $xactions);

				return id(new AphrontRedirectResponse())
				->setURI($request->getRequestURI());
				}

				$recipient_phids = array_reverse($recipient_phids);
				$handles = $this->loadViewerHandles($recipient_phids);

				epriestleyUnsubmitted Done Inline Actions Prefer `$handles = $viewer->loadHandles($phids)` in modern code. epriestley: Prefer `$handles = $viewer->loadHandles($phids)` in modern code.
				$state = array();
				chadAuthorUnsubmitted Done Inline Actions I couldn't get that to work (returns different results). chad: I couldn't get that to work (returns different results).
				foreach ($handles as $handle) {
				$state[] = array(
				'phid' => $handle->getPHID(),
				'name' => $handle->getFullName(),
				);
				}

				$can_edit = PhabricatorPolicyFilter::hasCapability(
				$user,
				$badge,
				PhabricatorPolicyCapability::CAN_EDIT);

				$form_box = null;
				$title = pht('Add Recipient');
				if ($can_edit) {
				$header_name = pht('Edit Recipients');
				$view_uri = $this->getApplicationURI('view/'.$badge->getID().'/');

				$form = new AphrontFormView();
				$form
				->setUser($user)
				->appendControl(
				id(new AphrontFormTokenizerControl())
				->setName('phids')
				->setLabel(pht('Add Recipients'))
				->setDatasource(new PhabricatorPeopleDatasource()))
				->appendChild(
				id(new AphrontFormSubmitControl())
				->addCancelButton($view_uri)
				->setValue(pht('Add Recipients')));
				$form_box = id(new PHUIObjectBoxView())
				->setHeaderText($title)
				->setForm($form);
				}

				$recipient_list = id(new PhabricatorBadgesRecipientsListView())
				->setBadge($badge)
				->setHandles($handles)
				->setUser($user);

				$badge_url = $this->getApplicationURI().'view/'.$this->id.'/';

				epriestleyUnsubmitted Done Inline Actions Prefer `$this->getApplicationURI('x')` over `$this->getApplicationURI().'x'`. The former is more flexible, since we can, e.g., correct or verify URIs inside `getApplicationURI(...)`. epriestley: Prefer `$this->getApplicationURI('x')` over `$this->getApplicationURI().'x'`. The former is…
				$crumbs = $this->buildApplicationCrumbs();
				$crumbs->addTextCrumb($badge->getName(), $badge_url);
				$crumbs->addTextCrumb(pht('Recipients'));

				return $this->buildApplicationPage(
				array(
				$crumbs,
				$form_box,
				$recipient_list,
				),
				array(
				'title' => $title,
				));
				}

				}