Conversation

vmcj

We had this request both during EUC and this would help during next BAPC prelims where we probably ask a BAPC jury member to upload the problems already.

For the 2nd case I would personally trust that person with admin access but others might see that differently. Also only giving the needed access is always a good thing and given that this is easy to test I don't think this would be a real feature creep.

@vmcjvmcj force-pushed the api_admin_actions branch from c13fa94 to 200619a Compare July 2, 2024 06:11
@vmcjvmcj force-pushed the api_admin_actions branch from 11c08dc to 0ddd1ad Compare July 2, 2024 16:44
@vmcjvmcj requested a review from nickygerritsen July 2, 2024 18:26
@vmcjvmcj force-pushed the api_admin_actions branch from fc3fa75 to 9c9f1f0 Compare July 2, 2024 18:37
@vmcjvmcj force-pushed the api_admin_actions branch from 74635a6 to e95c10d Compare July 3, 2024 17:34
@@ -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]
Copy link
Member

Choose a reason for hiding this comment

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

I think editor would be a better suffix than change. Also, is ROLE_API_CONTEST_CHANGE a superset of privileges over ROLE_API_PROBLEM_CHANGE?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, a contest is a different thing from a problem as we allow problems without a contest.

final class Version20240629154640 extends AbstractMigration
{
private const NEW_ROLES = ['api_problem_change' => 'API Problem Changer',
'api_contest_change' => 'API Contest Changer'];
Copy link
Member

Choose a reason for hiding this comment

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

Could we have some more detailed descriptions of these roles? How do admin/API writer, contest changer and problem changer relate?

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.
@vmcjvmcj force-pushed the api_admin_actions branch 2 times, most recently from b78b35b to 80d5b5e Compare July 21, 2024 08:59
@vmcjvmcj force-pushed the api_admin_actions branch from 80d5b5e to 11d2823 Compare July 21, 2024 15:33
public function getDescription(): string
{
return "Add new roles to the database.
Problem editor can add/delete/edit anything related to problems; files, testcases.

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 .

@meisterT

Two questions:

  • Do we need the split in two separate editor roles or is one enough?
  • Is the mechanism over which the change happens (Web vs API) relevant?

If the answer is no to both of these questions, then one role CONTEST_EDITOR who can edit contests including problems and testdata both via web and API should be enough.

@vmcj

Two questions:

* Do we need the split in two separate editor roles or is one enough?

I think BAPCtools also wants to set the contest, and the judges of EUC wanted to only import the problems and nothing else? I'm fine with merging them as its already better than admin and we can always decide to split them later if there is a real usecase.

* Is the mechanism over which the change happens (Web vs API) relevant?

It was for me, I rather not also have to do this in code and if you have the zips already you can also just give them to an admin. I think is is only relevant for automated tools and during the contest I wouldn't want jury members to be able to change things which they might not be aware what are the differences (testcase order etc.).

So its both laziness for the amount of code changed and only giving the rights which are needed. The same distinction was made for locking a contest.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.