🐛 Remove /admin prefix from group and event management routes

[dehy]

Summary

Routes for group management and event management were incorrectly prefixed with /admin, making them inaccessible or inconsistent with the rest of the application's routing conventions. These controllers already enforce their own access control via denyAccessUnlessGranted / voters, so the /admin path prefix was both redundant and incorrect.

Changes

EventManagementController — removed /admin prefix from all routes:

• /admin/events → /events
• /admin/events/create → /events/create
• /admin/events/create/{type} → /events/create/{type}
• /admin/events/{id}/edit → /events/{id}/edit
• /admin/events/{id}/delete → /events/{id}/delete

GroupManagementController — removed /admin prefix from all routes:

• /admin/groups/{id}/manage → /groups/{id}/manage
• /admin/groups/{id}/add-member → /groups/{id}/add-member
• /admin/groups/{id}/remove-member → /groups/{id}/remove-member
• /admin/groups/create → /groups/create

templates/group/manage.html.twig — updated route references accordingly.

GroupManagementControllerTest — updated test URL paths to match new routes.

Testing

• Updated functional tests pass with the corrected paths.
• No security regression: access control is enforced at the controller level via voters, independent of the URL prefix.

[Copilot]

Pull request overview

This PR removes the /admin URL prefix from group and event management routes to align them with the rest of the app’s routing, relying on controller-level authorization (IsGranted / voters) instead of a path prefix.

Changes:

• Updated GroupManagementController routes from /admin/groups/... to /groups/... and aligned functional tests.
• Updated group management template AJAX endpoints to call the new /groups/... paths.
• Updated EventManagementController routes from /admin/events... to /events....

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
tests/Functional/Controller/GroupManagementControllerTest.php	Updates tested URLs to match the new /groups/... routes and centralizes them as constants.
templates/group/manage.html.twig	Updates AJAX endpoints to the new /groups/... paths.
src/Controller/GroupManagementController.php	Removes /admin prefix from group management routes.
src/Controller/Admin/EventManagementController.php	Removes /admin prefix from event management routes (but introduces path collisions with existing public event routes).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (4)

src/Controller/Management/GroupManagementController.php:128

• New behavior: remove-member now enforces CSRF validation. Consider adding a functional test that explicitly covers the missing/invalid _token path (403 + JSON error) so this security requirement remains enforced.
  src/Controller/Management/EventManagementController.php:36
• PR description lists event management routes as /events, /events/create, etc., but the implementation uses /events/manage, /events/manage/create, etc. Please align either the route paths here or the PR description so they match (and to avoid confusion for reviewers/deploy notes).
  src/Controller/Management/GroupManagementController.php:83
• New behavior: add-member now rejects requests with an invalid/missing CSRF token. There isn’t currently functional test coverage asserting the CSRF failure case (e.g., POST without _token returns 403 with the expected JSON error), which would help prevent regressions for this security check.
  templates/group/manage.html.twig:168
• The showAlert function builds HTML with alert.innerHTML and directly injects the message string into the DOM. These messages are composed on the server from licensee->getFullname() and group->getName() in GroupManagementController, so an attacker who controls these fields can inject arbitrary HTML/JavaScript and achieve XSS when a club admin manages group members. To fix this, treat message as plain text on the client (e.g. using textContent or explicit text nodes) or ensure the server-side message is safely escaped before insertion into the DOM instead of concatenated into HTML.

            // Fonction pour afficher des alertes
            function showAlert(message, type = 'success') {
                const alert = document.createElement('div');
                alert.className = `alert alert-${type} alert-dismissible fade show`;
                alert.innerHTML = `
                    ${message}
                    <button type="button" class="btn-close" data-bs-dismiss="alert"></button>

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/Controller/Management/EventManagementController.php:36

• /events/manage can be shadowed by the generic #[Route('/events/{slug}')] in EventController (e.g. slug=manage), making the management index route unreachable depending on route loading order. Use a non-conflicting prefix (e.g. /management/events), or set an explicit priority on the management routes / lower priority on the show route and/or add a {slug} requirement that excludes reserved segments like manage.
  src/Controller/Management/EventManagementController.php:36
• The PR description says event management routes move from /admin/events... to /events..., but the implementation uses /events/manage.... If /events/manage is the intended new convention, please update the PR description (and any docs) to match; otherwise adjust the routes to the documented paths.
  templates/management/group/manage.html.twig:156
• Two separate forms of the same type (GroupMemberActionType) are rendered with the same default form name, which will generate duplicate HTML id attributes for the inner fields (e.g. both will likely render id="group_member_action_licenseeId" / _token). Consider creating them with distinct names (e.g. createNamed('add_member_action', ...) / createNamed('remove_member_action', ...)) to avoid duplicate IDs and make DOM interactions more robust.
  templates/management/group/manage.html.twig:216
• The AJAX handlers read JSON responses and later pass data.message into an alert helper that writes directly to innerHTML. Since these messages contain Licensee::getFullname() and Group::getName(), which are user-editable fields, a licensee can inject HTML/JS into their name and have it executed in a club admin’s browser when they are added/removed from a group (stored DOM XSS). To mitigate this, avoid rendering untrusted strings via innerHTML (e.g. use textContent or HTML-escape the message first) and ensure any server-side messages that interpolate user data are encoded for the HTML context.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
90.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud