Summary

CourseVoter, SessionVoter, and GroupVoter currently assign contextual roles (ROLE_CURRENT_COURSE_STUDENT, ROLE_CURRENT_COURSE_TEACHER, ROLE_CURRENT_COURSE_SESSION_*, ROLE_CURRENT_COURSE_GROUP_*) as a side effect of their voteOnAttribute() method by calling $user->addRole(...) directly on the User entity.

This is an architectural antipattern that mixes two responsibilities:

1. Context establishment: "Is the user a member of this course/session/group?"
2. Permission decision: "Can the user view/edit/delete this specific resource?"

A Voter must be a pure function (token, subject, attribute) -> GRANT | DENY | ABSTAIN. Performing side effects inside it breaks Symfony's Security contract.

Current behavior (problematic)

src/CoreBundle/Security/Authorization/Voter/CourseVoter.php performs role assignment at lines 104, 106, 131, 134, 153, 161, 177, 180, 199. Example:

// Inside CourseVoter::voteOnAttribute()
if ($courseUser instanceof CourseRelUser) {
    $user->addRole(ResourceNodeVoter::ROLE_CURRENT_COURSE_STUDENT);
    if (CourseRelUser::TEACHER === $courseUser->getStatus()) {
        $user->addRole(ResourceNodeVoter::ROLE_CURRENT_COURSE_TEACHER);
    }
}

Why this is broken

• Non-deterministic voter order: Symfony does not guarantee CourseVoter runs before other voters that may already expect ROLE_CURRENT_COURSE_* in the token.
• Short-circuit evaluation: With the default affirmative strategy, if any other voter grants first, CourseVoter is never invoked - and the contextual role is silently not assigned. Subsequent is_granted() checks against contextual roles become inconsistent depending on what was checked first in the request.
• Mutates User instead of the token: $user->addRole() modifies the Doctrine-managed entity. The RoleHierarchyVoter reads roles from the token, not from User::getRoles() at every check, so the role may not be visible where it should be.
• Voters become impure: Hard to unit test, hard to reason about, and prevents declarative use of security: expressions on API Platform operations.
• Runs N times per request: every is_granted() re-invokes the Voter and re-evaluates membership, instead of resolving it once.

Proposed architecture

Separate the two concerns into distinct request pipeline stages:

[priority 8]  Symfony Firewall                -> authenticates user, loads into TokenStorage
[priority 30] CidReqListener (existing)        -> resolves cid/sid/gid, exposes Course/Session/CGroup in request attributes
[priority 20] CourseContextRoleListener (NEW)  -> reads context, validates membership, replaces token with contextual roles
[priority 0]  (other listeners)
                  |
                  v
              API Platform AccessCheckerProvider
                  |
                  v
              is_granted('ROLE_CURRENT_COURSE_STUDENT')  <- declarative check on #[ApiResource]
                  |
                  v
              Controller / StateProvider
                  |
                  v
              CourseVoter / SessionVoter / GroupVoter (pure)  -> decide VIEW/EDIT/DELETE on objects only

Benefits

• #[ApiResource] operations declare membership requirements declaratively: security: "is_granted('ROLE_CURRENT_COURSE_STUDENT')".
• Endpoints that omit cid/sid/gid simply do not receive the role -> automatic 403, which is the desired behavior.
• Voters stay pure and unit-testable in isolation.
• Membership validation runs once per request, not once per is_granted() call.
• Aligns with API Platform's intended security model and removes the need for ad-hoc cid validation logic scattered across providers/extensions.

Implementation plan

Recommended order, to minimize blast radius:

1. Create CourseContextRoleListener (~100 lines) that:

   • Listens to kernel.request with priority lower than CidReqListener.
   • Reads Course, Session, CGroup from request attributes.
   • Validates user membership against each context.
   • Builds the list of contextual roles to grant.
   • Replaces the current token in TokenStorage with a new token containing the existing roles plus the contextual ones.
   • Does not touch User::addRole().

2. Do not remove the existing addRole() calls in Voters yet. Roles are momentarily duplicated (listener + voter), but nothing breaks. This lets the new path be validated in isolation.

3. Migrate 1-2 simple #[ApiResource] entities to declare security: using the contextual role. Verify behavior end-to-end (HTTP, GraphQL, and any State Provider).

4. Remove $user->addRole(...) from CourseVoter, SessionVoter, and GroupVoter. Audit consumers that may have depended on the side effect.

5. Progressively migrate remaining #[ApiResource] operations that need contextual access.

6. Add regression tests per contextual role: without context -> 403; with correct context -> 200; with mismatched context -> 403.

Files in scope

• src/CoreBundle/EventListener/CidReqListener.php (existing, no change expected)
• src/CoreBundle/Security/Authorization/Voter/CourseVoter.php (cleanup)
• src/CoreBundle/Security/Authorization/Voter/SessionVoter.php (cleanup)
• src/CoreBundle/Security/Authorization/Voter/GroupVoter.php (cleanup)
• src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php (defines the role constants; should remain the source of truth)
• src/CoreBundle/EventListener/CourseContextRoleListener.php (new)
• #[ApiResource] operations across src/CoreBundle/Entity/ and src/CourseBundle/Entity/ (progressive migration)

Risks to monitor

• Legacy code under public/main/: routes do not pass through API Platform but may depend on the side effect of $user->addRole(). The legacy stack uses its own session-based access checks (api_protect_course_script() and friends), but anywhere it intersects with Symfony security must be verified.
• GraphQL: API Platform GraphQL uses the same state providers and access checker, so it benefits automatically. Coverage must be confirmed via tests.
• CLI commands and Messenger handlers: have no request, therefore no contextual role. Verify that no Voter invoked from a CLI/async context relies on ROLE_CURRENT_COURSE_*.
• Firewall statefulness: the API firewall should be stateless: true so that token replacement does not leak across requests via session storage. Confirm before merging.
• Behat scenarios that assume the legacy role-assignment timing may need adjustment.

Out of scope

• Refactoring permission semantics inside the Voters (the VIEW/EDIT/DELETE decisions themselves). This issue is strictly about where contextual roles are assigned, not how object-level permissions are decided.
• Replacing the existing api_protect_course_script() mechanism in legacy code.
• Changing CidReqListener itself.

Acceptance criteria

• A dedicated request listener assigns ROLE_CURRENT_COURSE_* roles to the security token after CidReqListener resolves the context.
• CourseVoter, SessionVoter, and GroupVoter contain no calls to User::addRole() or any equivalent side effect.
• At least one #[ApiResource] operation declares its course-context requirement via security: using a contextual role, and is covered by a regression test.
• Existing functional tests pass without modification (or are explicitly updated with justification).
• Documentation in docs/security.md explains where contextual roles come from and how to use them in #[ApiResource] declarations.