Add new roles for problem/contest changes via API

This is for an usecase like EUC where there is an Ad-Hoc group which doesn't know each other yet (or even the system). The responsibility for the upload of the problems lies with one team which does not want admin access to make sure nothing gets broken. The same for changing the contest as BAPCtools does for example. Also extended the tests for admin access to now also check for the new roles while making sure admin also keeps the rights by transitivity.
DOMjudge · Jul 21, 2024 · 265be80 · 265be80
1 parent ab267ec
commit 265be80
Show file tree

Hide file tree

Showing 8 changed files with 155 additions and 34 deletions.
diff --git 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
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20240629154640 extends AbstractMigration
+{
+    private const NEW_ROLES = ['api_problem_change' => '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
@@ -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
@@ -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<string, string[]>}
      * @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
@@ -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
@@ -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
@@ -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 = <<<EOF
 duration: 2:00:00
@@ -69,6 +73,8 @@ public function testAddYaml(): void
 scoreboard_freeze_duration: 0:30:00
 EOF;
 
+        $this->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 = <<<EOF
 {
@@ -103,6 +112,8 @@ public function testAddJson(): void
 }
 EOF;
 
+        $this->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,14 +323,18 @@ 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']];
     }
 
     /**
      * @dataProvider provideNewContest
      */
     public function testActivateTimeContestYaml(
         string $activateTime, string $startTime, ?string $deactivateTime,
-        bool $setActivate, bool $setDeactivate
+        bool $setActivate, bool $setDeactivate, array $newRoles = [],
     ): void {
         $yaml = <<<EOF
 duration: 2:00:00
@@ -322,6 +350,8 @@ public function testActivateTimeContestYaml(
     id: anothereruption
 EOF;
 
+        $this->roles = $newRoles;
+        self::setUp();
         if ($setActivate) {
             $yaml = "activate_time: ".$activateTime."\n".$yaml;
         }