Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/Controller/PackageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -733,10 +733,17 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn
}
}

$data['addMaintainerForm'] = $this->createAddMaintainerForm($package)->createView();
$data['removeMaintainerForm'] = $this->createRemoveMaintainerForm($package)->createView();
$data['transferPackageForm'] = $this->createTransferPackageForm($package)->createView();
$data['deleteForm'] = $this->createDeletePackageForm($package)->createView();
// The template renders each of these behind the matching voter grant, and building them
// is not free - createView() on the remove-maintainer form materialises an EntityType
// choice list with a DB query - so visitors who cannot see a form must not pay for it.
$data['addMaintainerForm'] = $this->isGranted(PackageActions::AddMaintainer->value, $package)
? $this->createAddMaintainerForm($package)->createView() : null;
$data['removeMaintainerForm'] = $this->isGranted(PackageActions::RemoveMaintainer->value, $package)
? $this->createRemoveMaintainerForm($package)->createView() : null;
$data['transferPackageForm'] = $this->isGranted(PackageActions::TransferPackage->value, $package)
? $this->createTransferPackageForm($package)->createView() : null;
$data['deleteForm'] = $this->isGranted(PackageActions::Delete->value, $package)
? $this->createDeletePackageForm($package)->createView() : null;
} else {
$data['hasVersionSecurityAdvisories'] = [];
$data['hasVersionsFlaggedAsMalware'] = [];
Expand Down
29 changes: 29 additions & 0 deletions tests/Controller/PackageControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,35 @@ public function testView(): void
self::assertStringContainsString('noindex', (string) $auditLink->attr('rel'));
}

public function testPackagePageOmitsManagementFormsForVisitorsWhoCannotUseThem(): void
{
$owner = self::createUser('owner', 'owner@example.org');
// a second maintainer is required for remove_maintainer to be granted at all
$comaintainer = self::createUser('comaintainer', 'comaintainer@example.org');
$package = self::createPackage('test/pkg', 'https://example.com/test/pkg', maintainers: [$owner, $comaintainer]);
$this->store($owner, $comaintainer, $package);

$crawler = $this->client->request('GET', '/packages/test/pkg');
self::assertResponseIsSuccessful();

// The controller skips building these entirely when the visitor lacks the grant, which also
// skips the EntityType choice-list query behind the remove-maintainer form.
self::assertCount(0, $crawler->filter('[name="add_maintainer_form"]'));
self::assertCount(0, $crawler->filter('[name="remove_maintainer_form"]'));
self::assertCount(0, $crawler->filter('[name="transfer_package_form"]'));
self::assertCount(0, $crawler->filter('form.delete.action'));

// ...and still builds them for someone who can, so the skip is keyed on the grant only.
$this->client->loginUser($owner);
$crawler = $this->client->request('GET', '/packages/test/pkg');
self::assertResponseIsSuccessful();

self::assertCount(1, $crawler->filter('[name="add_maintainer_form"]'));
self::assertCount(1, $crawler->filter('[name="remove_maintainer_form"]'));
self::assertCount(1, $crawler->filter('[name="transfer_package_form"]'));
self::assertCount(1, $crawler->filter('form.delete.action'));
}

public function testPackagePageOnlyCountsViewsWhileTheSpamHeuristicCanUseThem(): void
{
$fresh = self::createPackage('test/fresh', 'https://example.com/test/fresh');
Expand Down