Open

Splitoff admin only roles for API #2616

Show file tree
Hide file tree
Changes from all commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
265be80
Add new roles for problem/contest changes via API
vmcjJun 30, 2024
11d2823
Implement Jaap his feedback
vmcjJul 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Failed to load files.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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_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
password_hashers:
App\Entity\User:
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
<?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_editor' => 'API Problem Editor',
'api_contest_editor' => 'API Contest Editor'];

public function getDescription(): string
{
return "Add new roles to the database.
Problem editor can add/delete/edit anything related to problems; files, testcases.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think any (end) user will read this and I think @eldering his feedback is that people now don't know what the roles do without checking the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. What the roles do should ideally be clear from the role constant names, but at least from the role descriptions .

Contest editor can add/delete/edit the time & connected problems, 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
{
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;
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,7 +74,7 @@ public function __construct(
* Add a new contest.
* @throws BadRequestHttpException
*/
#[IsGranted('ROLE_ADMIN')]
#[IsGranted('ROLE_API_CONTEST_EDITOR')]
#[Rest\Post('')]
#[OA\RequestBody(
required: true,
Expand DownExpand Up@@ -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_EDITOR')]
#[Rest\Delete('/{cid}/banner', name: 'delete_contest_banner')]
#[OA\Response(response: 204, description: 'Deleting banner succeeded')]
#[OA\Parameter(ref: '#/components/parameters/cid')]
Expand All@@ -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_EDITOR')]
#[Rest\Post("/{cid}/banner", name: 'post_contest_banner')]
#[Rest\Put("/{cid}/banner", name: 'put_contest_banner')]
#[OA\RequestBody(
Expand DownExpand Up@@ -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_EDITOR')]
#[Rest\Delete('/{cid}/problemset', name: 'delete_contest_problemset')]
#[OA\Response(response: 204, description: 'Deleting problemset document succeeded')]
#[OA\Parameter(ref: '#/components/parameters/cid')]
Expand All@@ -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_EDITOR')]
#[Rest\Post("/{cid}/problemset", name: 'post_contest_problemset')]
#[Rest\Put("/{cid}/problemset", name: 'put_contest_problemset')]
#[OA\RequestBody(
Expand DownExpand Up@@ -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_EDITOR')"))]
#[Rest\('/{cid}')]
#[OA\RequestBody(
required: true,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -61,7 +61,7 @@ public function __construct(
* @throws BadRequestHttpException
* @throws NonUniqueResultException
*/
#[IsGranted('ROLE_ADMIN')]
#[IsGranted('ROLE_API_PROBLEM_EDITOR')]
#[Rest\Post('/add-data')]
#[OA\RequestBody(
required: true,
Expand DownExpand Up@@ -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_EDITOR')]
#[Rest\Post('')]
#[OA\RequestBody(
required: true,
Expand DownExpand Up@@ -237,7 +237,7 @@ public function addProblemAction(Request $request): array
/**
* Unlink a problem from this contest.
*/
#[IsGranted('ROLE_ADMIN')]
#[IsGranted('ROLE_API_PROBLEM_EDITOR')]
#[Rest\Delete('/{id}')]
#[OA\Response(response: 204, description: 'Problem unlinked from contest succeeded')]
#[OA\Parameter(ref: '#/components/parameters/id')]
Expand DownExpand Up@@ -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_EDITOR')]
#[Rest\Put('/{id}')]
#[OA\Response(
response: 200,
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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_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]))) {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -373,4 +374,10 @@ public function provideSingleNotFound(): Generator
yield [$id];
}
}

protected function provideAllowedUsers(): Generator
{
yield ['admin', ['admin']];
yield ['team', [static::$testedRole]];
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,6 +21,7 @@
class ContestControllerAdminTest extends ContestControllerTest
{
protected ?string $apiUser = 'admin';
protected static string $testedRole = 'api_contest_editor';

private function parseSortYaml(string $yamlString): array
{
Expand All@@ -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
Expand DownExpand Up@@ -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);
Expand All@@ -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
{
Expand All@@ -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);
Expand All@@ -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) {
Expand DownExpand Up@@ -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) {
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -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_editor']];
}

/**
* @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
Expand All@@ -322,6 +350,8 @@ public function testActivateTimeContestYaml(
id: anothereruption
EOF;

$this->roles = $newRoles;
self::setUp();
if ($setActivate) {
$yaml = "activate_time: ".$activateTime."\n".$yaml;
}
Expand Down
Loading
Loading