From 80d5b5e0c7695d271f4e2ff94f13ec7ab91fb03b Mon Sep 17 00:00:00 2001 From: Michael Vasseur <14887731+vmcj@users.noreply.github.com> Date: Sun, 21 Jul 2024 10:51:58 +0200 Subject: [PATCH] Implement Jaap his feedback --- webapp/config/packages/security.yaml | 2 +- webapp/migrations/Version20240629154640.php | 11 ++++++++--- webapp/src/Controller/API/ContestController.php | 12 ++++++------ webapp/src/Controller/API/ProblemController.php | 8 ++++---- webapp/src/DataFixtures/DefaultData/RoleFixture.php | 4 ++-- .../Controller/API/ContestControllerAdminTest.php | 4 ++-- .../Controller/API/ProblemControllerAdminTest.php | 2 +- 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/webapp/config/packages/security.yaml b/webapp/config/packages/security.yaml index eaae1a40df6..10542592c9c 100644 --- a/webapp/config/packages/security.yaml +++ b/webapp/config/packages/security.yaml @@ -4,7 +4,7 @@ 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_API_PROBLEM_CHANGE, ROLE_API_CONTEST_CHANGE] + ROLE_API_PROBLEM_EDITOR, ROLE_API_CONTEST_EDITOR] ROLE_SUPER_ADMIN: [ROLE_ADMIN, ROLE_ALLOWED_TO_SWITCH] # https://symfony.com/doc/current/security.html#registering-the-user-hashing-passwords diff --git a/webapp/migrations/Version20240629154640.php b/webapp/migrations/Version20240629154640.php index b2446e32d31..7ed3c7eab0f 100644 --- a/webapp/migrations/Version20240629154640.php +++ b/webapp/migrations/Version20240629154640.php @@ -9,12 +9,17 @@ final class Version20240629154640 extends AbstractMigration { - private const NEW_ROLES = ['api_problem_change' => 'API Problem Changer', - 'api_contest_change' => 'API Contest Changer']; + private const NEW_ROLES = ['api_problem_editor' => 'API Problem Editor', + 'api_contest_editor' => 'API Contest Editor']; public function getDescription(): string { - return 'Add new roles to the database.'; + return "Add new roles to the database. + Problem editor can add/delete/edit anything related to problems; files, testcases. + Contest editor can add/delete/edit the time & connected probems, but not the files + or testcases of those problems. + They are a subset of the ADMIN role in the API but not a proper superset of the API_WRITER + as that also has access to push teams etc."; } public function up(Schema $schema): void diff --git a/webapp/src/Controller/API/ContestController.php b/webapp/src/Controller/API/ContestController.php index cef0f4352fd..b61dfb807dc 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_API_CONTEST_CHANGE')] + #[IsGranted('ROLE_API_CONTEST_EDITOR')] #[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_API_CONTEST_CHANGE')] + #[IsGranted('ROLE_API_CONTEST_EDITOR')] #[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_API_CONTEST_CHANGE')] + #[IsGranted('ROLE_API_CONTEST_EDITOR')] #[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_API_CONTEST_CHANGE')] + #[IsGranted('ROLE_API_CONTEST_EDITOR')] #[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_API_CONTEST_CHANGE')] + #[IsGranted('ROLE_API_CONTEST_EDITOR')] #[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(new Expression("is_granted('ROLE_API_WRITER') or is_granted('ROLE_API_CONTEST_CHANGE')"))] + #[IsGranted(new Expression("is_granted('ROLE_API_WRITER') or is_granted('ROLE_API_CONTEST_EDITOR')"))] #[Rest\Patch('/{cid}')] #[OA\RequestBody( required: true, diff --git a/webapp/src/Controller/API/ProblemController.php b/webapp/src/Controller/API/ProblemController.php index 90b54ee1af2..7b68ddba17e 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_API_PROBLEM_CHANGE')] + #[IsGranted('ROLE_API_PROBLEM_EDITOR')] #[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_API_PROBLEM_CHANGE')] + #[IsGranted('ROLE_API_PROBLEM_EDITOR')] #[Rest\Post('')] #[OA\RequestBody( required: true, @@ -237,7 +237,7 @@ public function addProblemAction(Request $request): array /** * Unlink a problem from this contest. */ - #[IsGranted('ROLE_API_PROBLEM_CHANGE')] + #[IsGranted('ROLE_API_PROBLEM_EDITOR')] #[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_API_PROBLEM_CHANGE')] + #[IsGranted('ROLE_API_PROBLEM_EDITOR')] #[Rest\Put('/{id}')] #[OA\Response( response: 200, diff --git a/webapp/src/DataFixtures/DefaultData/RoleFixture.php b/webapp/src/DataFixtures/DefaultData/RoleFixture.php index 5ec8c63de5c..d055ea8c785 100644 --- a/webapp/src/DataFixtures/DefaultData/RoleFixture.php +++ b/webapp/src/DataFixtures/DefaultData/RoleFixture.php @@ -29,8 +29,8 @@ public function load(ObjectManager $manager): void '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' + 'api_problem_editor' => 'API Problem Editor', + 'api_contest_editor' => 'API Contest Editor' ]; foreach ($roles as $roleName => $description) { if (!($role = $manager->getRepository(Role::class)->findOneBy(['dj_role' => $roleName]))) { diff --git a/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php b/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php index 8ca2a9e94a0..1f46ebadf2a 100644 --- a/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php +++ b/webapp/tests/Unit/Controller/API/ContestControllerAdminTest.php @@ -21,7 +21,7 @@ class ContestControllerAdminTest extends ContestControllerTest { protected ?string $apiUser = 'admin'; - protected static string $testedRole = 'api_contest_change'; + protected static string $testedRole = 'api_contest_editor'; private function parseSortYaml(string $yamlString): array { @@ -326,7 +326,7 @@ public function provideChangeTimes(): Generator // 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']]; + yield [['id' => 1, 'scoreboard_thaw_time' => '-13 seconds'], 200, 'Demo contest', [], true, true, ['api_contest_editor']]; } /** diff --git a/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php b/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php index 822269faaa3..80bb1e06520 100644 --- a/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php +++ b/webapp/tests/Unit/Controller/API/ProblemControllerAdminTest.php @@ -11,7 +11,7 @@ class ProblemControllerAdminTest extends ProblemControllerTest { protected ?string $apiUser = 'admin'; - protected static string $testedRole = 'api_problem_change'; + protected static string $testedRole = 'api_problem_editor'; protected function setUp(): void {