Differential D13626 Diff 33033 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 {

				public function handleRequest(AphrontRequest $request) {
				$viewer = $request->getViewer();
				$id = $request->getURIData('id');

				$badge = id(new PhabricatorBadgesQuery())
				->setViewer($viewer)
				epriestleyUnsubmitted Done Inline Actions Remove now that you're using getURIData(). epriestley: Remove now that you're using getURIData().
				->withIDs(array($id))
				->needRecipients(true)
				->requireCapabilities(
				array(
				epriestleyUnsubmitted Done Inline Actions handleRequest / getViewer (see below) epriestley: handleRequest / getViewer (see below)
				PhabricatorPolicyCapability::CAN_EDIT,
				PhabricatorPolicyCapability::CAN_VIEW,
				))
				->executeOne();
				if (!$badge) {
				return new Aphront404Response();
				}

				$recipient_phids = $badge->getRecipientPHIDs();

				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 ($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($viewer)
				->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);

				$state = array();
				foreach ($handles as $handle) {
				$state[] = array(
				'phid' => $handle->getPHID(),
				'name' => $handle->getFullName(),
				);
				epriestleyUnsubmitted Done Inline Actions Prefer `$handles = $viewer->loadHandles($phids)` in modern code. epriestley: Prefer `$handles = $viewer->loadHandles($phids)` in modern code.
				}
				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).

				$can_edit = PhabricatorPolicyFilter::hasCapability(
				$viewer,
				$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($viewer)
				->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($viewer);

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

				$crumbs = $this->buildApplicationCrumbs();
				$crumbs->addTextCrumb($badge->getName(), $badge_url);
				$crumbs->addTextCrumb(pht('Recipients'));

				return $this->buildApplicationPage(
				array(
				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,
				$form_box,
				$recipient_list,
				),
				array(
				'title' => $title,
				));
				}

				}