diff --git a/webapp/config/packages/security.yaml b/webapp/config/packages/security.yaml index 33361a36db..eaae1a40df 100644 --- a/webapp/config/packages/security.yaml +++ b/webapp/config/packages/security.yaml @@ -3,10 +3,10 @@ security: role_hierarchy: ROLE_JURY: [ROLE_CLARIFICATION_RW, ROLE_API, ROLE_API_READER, ROLE_API_SOURCE_READER] - ROLE_ADMIN: [ROLE_JURY, ROLE_JUDGEHOST, ROLE_API_WRITER] + ROLE_ADMIN: [ROLE_JURY, ROLE_JUDGEHOST, ROLE_API_WRITER, + ROLE_API_PROBLEM_CHANGE, ROLE_API_CONTEST_CHANGE] ROLE_SUPER_ADMIN: [ROLE_ADMIN, ROLE_ALLOWED_TO_SWITCH] - # https://symfony.com/doc/current/security.html#registering-the-user-hashing-passwords password_hashers: App\Entity\User: diff --git a/webapp/migrations/Version20240629154640.php b/webapp/migrations/Version20240629154640.php new file mode 100644 index 0000000000..b2446e32d3 --- /dev/null +++ b/webapp/migrations/Version20240629154640.php @@ -0,0 +1,41 @@ + 'API Problem Changer', + 'api_contest_change' => 'API Contest Changer']; + + public function getDescription(): string + { + return 'Add new roles to the database.'; + } + + public function up(Schema $schema): void + { + foreach (self::NEW_ROLES as $role => $description) { + $this->addSql( + 'INSERT INTO role (`role`, `description`) VALUES (:role, :desc)', + ['role' => $role, 'desc' => $description] + ); + } + } + + public function down(Schema $schema): void + { + foreach (array_keys(self::NEW_ROLES) as $role) { + $this->addSql('DELETE FROM role WHERE role = ' . $role ); + } + } + + public function isTransactional(): bool + { + return false; + } +} diff --git a/webapp/src/Controller/API/ContestController.php b/webapp/src/Controller/API/ContestController.php index bea5588e4d..cef0f4352f 100644 --- a/webapp/src/Controller/API/ContestController.php +++ b/webapp/src/Controller/API/ContestController.php @@ -74,7 +74,7 @@ public function __construct( * Add a new contest. * @throws BadRequestHttpException */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_CONTEST_CHANGE')] #[Rest\Post('')] #[OA\RequestBody( required: true, @@ -200,7 +200,7 @@ public function bannerAction(Request $request, string $cid): Response /** * Delete the banner for the given contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_CONTEST_CHANGE')] #[Rest\Delete('/{cid}/banner', name: 'delete_contest_banner')] #[OA\Response(response: 204, description: 'Deleting banner succeeded')] #[OA\Parameter(ref: '#/components/parameters/cid')] @@ -220,7 +220,7 @@ public function deleteBannerAction(Request $request, string $cid): Response /** * Set the banner for the given contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_CONTEST_CHANGE')] #[Rest\Post("/{cid}/banner", name: 'post_contest_banner')] #[Rest\Put("/{cid}/banner", name: 'put_contest_banner')] #[OA\RequestBody( @@ -268,7 +268,7 @@ public function setBannerAction(Request $request, string $cid, ValidatorInterfac /** * Delete the problemset document for the given contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_CONTEST_CHANGE')] #[Rest\Delete('/{cid}/problemset', name: 'delete_contest_problemset')] #[OA\Response(response: 204, description: 'Deleting problemset document succeeded')] #[OA\Parameter(ref: '#/components/parameters/cid')] @@ -288,7 +288,7 @@ public function deleteProblemsetAction(Request $request, string $cid): Response /** * Set the problemset document for the given contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_CONTEST_CHANGE')] #[Rest\Post("/{cid}/problemset", name: 'post_contest_problemset')] #[Rest\Put("/{cid}/problemset", name: 'put_contest_problemset')] #[OA\RequestBody( @@ -384,7 +384,7 @@ public function problemsetAction(Request $request, string $cid): Response * Change the start time or unfreeze (thaw) time of the given contest. * @throws NonUniqueResultException */ - #[IsGranted('ROLE_API_WRITER')] + #[IsGranted(new Expression("is_granted('ROLE_API_WRITER') or is_granted('ROLE_API_CONTEST_CHANGE')"))] #[Rest\Patch('/{cid}')] #[OA\RequestBody( required: true, diff --git a/webapp/src/Controller/API/ProblemController.php b/webapp/src/Controller/API/ProblemController.php index ecfab13749..90b54ee1af 100644 --- a/webapp/src/Controller/API/ProblemController.php +++ b/webapp/src/Controller/API/ProblemController.php @@ -61,7 +61,7 @@ public function __construct( * @throws BadRequestHttpException * @throws NonUniqueResultException */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_PROBLEM_CHANGE')] #[Rest\Post('/add-data')] #[OA\RequestBody( required: true, @@ -176,7 +176,7 @@ public function listAction(Request $request): Response * @return array{problem_id: string, messages: array} * @throws NonUniqueResultException */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_PROBLEM_CHANGE')] #[Rest\Post('')] #[OA\RequestBody( required: true, @@ -237,7 +237,7 @@ public function addProblemAction(Request $request): array /** * Unlink a problem from this contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_PROBLEM_CHANGE')] #[Rest\Delete('/{id}')] #[OA\Response(response: 204, description: 'Problem unlinked from contest succeeded')] #[OA\Parameter(ref: '#/components/parameters/id')] @@ -290,7 +290,7 @@ public function unlinkProblemAction(Request $request, string $id): Response /** * Link an existing problem to this contest. */ - #[IsGranted('ROLE_ADMIN')] + #[IsGranted('ROLE_API_PROBLEM_CHANGE')] #[Rest\Put('/{id}')] #[OA\Response( response: 200, diff --git a/webapp/src/DataFixtures/DefaultData/RoleFixture.php b/webapp/src/DataFixtures/DefaultData/RoleFixture.php index 088fe210ec..5ec8c63de5 100644 --- a/webapp/src/DataFixtures/DefaultData/RoleFixture.php +++ b/webapp/src/DataFixtures/DefaultData/RoleFixture.php @@ -20,15 +20,17 @@ public function load(ObjectManager $manager): void { // Mapping from role to description $roles = [ - 'admin' => 'Administrative User', - 'jury' => 'Jury User', - 'team' => 'Team Member', - 'balloon' => 'Balloon runner', - 'judgehost' => '(Internal/System) Judgehost', - 'api_reader' => 'API reader', - 'api_writer' => 'API writer', - 'api_source_reader' => 'Source code reader', - 'clarification_rw' => 'Clarification handler', + 'admin' => 'Administrative User', + 'jury' => 'Jury User', + 'team' => 'Team Member', + 'balloon' => 'Balloon runner', + 'judgehost' => '(Internal/System) Judgehost', + 'api_reader' => 'API reader', + 'api_writer' => 'API writer', + 'api_source_reader' => 'Source code reader', + 'clarification_rw' => 'Clarification handler', + 'api_problem_change' => 'API Problem Changer', + 'api_contest_change' => 'API Contest Changer' ]; foreach ($roles as $roleName => $description) { if (!($role = $manager->getRepository(Role::class)->findOneBy(['dj_role' => $roleName]))) { diff --git a/webapp/tests/Unit/Controller/API/BaseTestCase.php b/webapp/tests/Unit/Controller/API/BaseTestCase.php index 1549cfcb80..b58ef2e3a7 100644 --- a/webapp/tests/Unit/Controller/API/BaseTestCase.php +++ b/webapp/tests/Unit/Controller/API/BaseTestCase.php @@ -12,6 +12,7 @@ abstract class BaseTestCase extends BaseBaseTestCase { protected static array $rootEndpoints = ['contests', 'judgehosts', 'users']; + protected static string $testedRole = 'unset'; /** @var KernelBrowser */ protected KernelBrowser $client; @@ -373,4 +374,10 @@ public function provideSingleNotFound(): Generator yield [$id]; } } + + protected function provideAllowedUsers(): Generator + { + yield ['admin', ['admin']]; + yield ['team', [static::$testedRole]]; + } } diff --git a/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php b/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php index 5655e97c80..8ca2a9e94a 100644 --- a/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php +++ b/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php @@ -21,6 +21,7 @@ class ContestControllerAdminTest extends ContestControllerTest { protected ?string $apiUser = 'admin'; + protected static string $testedRole = 'api_contest_change'; private function parseSortYaml(string $yamlString): array { @@ -29,7 +30,10 @@ private function parseSortYaml(string $yamlString): array return $new; } - public function testAddYaml(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddYaml(string $user, array $newRoles): void { $yaml = <<roles = $newRoles; + self::setUp(); $url = $this->helperGetEndpointURL($this->apiEndpoint); $tempYamlFile = tempnam(sys_get_temp_dir(), "/contest-yaml-"); file_put_contents($tempYamlFile, $yaml); @@ -89,7 +95,10 @@ public function testAddYaml(): void self::assertNull($this->getContest($cid)->getDeactivatetime()); } - public function testAddJson(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddJson(string $user, array $newRoles): void { $json = <<roles = $newRoles; + self::setUp(); $url = $this->helperGetEndpointURL($this->apiEndpoint); $tempJsonFile = tempnam(sys_get_temp_dir(), "/contest-json-"); file_put_contents($tempJsonFile, $json); @@ -121,8 +132,13 @@ protected function getContest(int|string $cid): Contest return static::getContainer()->get(EntityManagerInterface::class)->getRepository(Contest::class)->findOneBy(['externalid' => $cid]); } - public function testBannerManagement(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testBannerManagement(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); // First, make sure we have no banner $id = 1; if ($this->objectClassForExternalId !== null) { @@ -163,8 +179,13 @@ public function testBannerManagement(): void self::assertArrayNotHasKey('banner', $object); } - public function testProblemsetManagement(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testProblemsetManagement(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); // First, make sure we have no problemset document $id = 1; if ($this->objectClassForExternalId !== null) { @@ -233,7 +254,10 @@ public function testChangeTimes( array $extraFixtures = [], bool $checkUnfreezeTime = false, bool $convertRelativeTimes = false, + array $newRoles = [], ): void { + $this->roles = $newRoles; + self::setUp(); $this->loadFixture(DemoPreStartContestFixture::class); $this->loadFixtures($extraFixtures); $id = 1; @@ -299,6 +323,10 @@ public function provideChangeTimes(): Generator yield [['id' => 1, 'scoreboard_thaw_time' => '+15 seconds', 'force' => true], 204, null, [DemoPostUnfreezeContestFixture::class], false, true]; yield [['id' => 1, 'scoreboard_thaw_time' => '+15 seconds'], 204, null, [], false, true]; yield [['id' => 1, 'scoreboard_thaw_time' => '-15 seconds'], 200, 'Demo contest', [], true, true]; + + // Show that this works for both roles + yield [['id' => 1, 'scoreboard_thaw_time' => '-14 seconds'], 200, 'Demo contest', [], true, true, ['admin']]; + yield [['id' => 1, 'scoreboard_thaw_time' => '-13 seconds'], 200, 'Demo contest', [], true, true, ['api_contest_change']]; } /** @@ -306,7 +334,7 @@ public function provideChangeTimes(): Generator */ public function testActivateTimeContestYaml( string $activateTime, string $startTime, ?string $deactivateTime, - bool $setActivate, bool $setDeactivate + bool $setActivate, bool $setDeactivate, array $newRoles = [], ): void { $yaml = <<roles = $newRoles; + self::setUp(); if ($setActivate) { $yaml = "activate_time: ".$activateTime."\n".$yaml; } diff --git a/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php b/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php index 30247b78b2..822269faaa 100644 --- a/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php +++ b/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php @@ -11,6 +11,7 @@ class ProblemControllerAdminTest extends ProblemControllerTest { protected ?string $apiUser = 'admin'; + protected static string $testedRole = 'api_problem_change'; protected function setUp(): void { @@ -21,7 +22,10 @@ protected function setUp(): void parent::setUp(); } - public function testAddJson(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddJson(string $user, array $newRoles): void { $json = <<roles = $newRoles; + self::setUp(); $url = $this->helperGetEndpointURL($this->apiEndpoint) . '/add-data'; $tempJsonFile = tempnam(sys_get_temp_dir(), "/problems-json-"); file_put_contents($tempJsonFile, $json); @@ -86,8 +92,13 @@ public function testAddJson(): void self::assertEquals($expectedProblems, $addedProblems); } - public function testDelete(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testDelete(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); // Check that we can delete the problem $url = $this->helperGetEndpointURL($this->apiEndpoint) . '/fltcmp'; $this->verifyApiJsonResponse('DELETE', $url, 204, $this->apiUser); @@ -98,15 +109,25 @@ public function testDelete(): void self::assertCount(2, $problems); } - public function testDeleteNotFound(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testDeleteNotFound(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); // Check that we can delete the problem $url = $this->helperGetEndpointURL($this->apiEndpoint) . '/4'; $this->verifyApiJsonResponse('DELETE', $url, 404, $this->apiUser); } - public function testAdd(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAdd(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); $this->loadFixture(DummyProblemFixture::class); $body = [ @@ -145,16 +166,26 @@ public function testAdd(): void self::assertCount(4, $problems); } - public function testAddNotFound(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddNotFound(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); // Check that we can delete the problem $url = $this->helperGetEndpointURL($this->apiEndpoint) . '/notfound'; $response = $this->verifyApiJsonResponse('PUT', $url, 404, $this->apiUser, ['label' => 'dummy']); self::assertEquals("Object with ID 'notfound' not found", $response['message']); } - public function testAddExisting(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddExisting(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); $this->loadFixture(DummyProblemFixture::class); // Check that we can not add a problem that is already added @@ -163,8 +194,13 @@ public function testAddExisting(): void self::assertEquals('Problem already linked to contest', $response['message']); } - public function testAddToLocked(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testAddToLocked(string $user, array $newRoles): void { + $this->roles = $newRoles; + self::setUp(); $this->loadFixture(LockedContestFixture::class); $this->loadFixture(DummyProblemFixture::class); @@ -184,9 +220,14 @@ public function testAddToLocked(): void self::assertStringContainsString('Contest is locked', $problemResponse['message']); } - public function testDeleteFromLocked(): void + /** + * @dataProvider provideAllowedUsers + */ + public function testDeleteFromLocked(string $user, array $newRoles): void { $this->loadFixture(LockedContestFixture::class); + $this->roles = $newRoles; + self::setUp(); // Check that we cannot delete the problem. $url = $this->helperGetEndpointURL($this->apiEndpoint) . '/fltcmp';