Fix GHSA-59cf-4f4p-752w

KKT handlers (and similar) had permission to manage anonymous questions and community service.
The code sometime only checked for the STUDENT_COUNCIL role and did not check if the user was an elected member of the student council. STUDENT_COUNCIL role is given to many people (including KKT handlers, committee members).

This commit should fix these issues, a warning is also placed in
`Role.php` to decrease the likelihood of making this mistake again.

Author: horcsinbalint

app/Exports/UsersExport.php

@@ -31,7 +31,7 @@ public function sheets(): array
 
         if(user()->can('viewSemesterEvaluation', User::class)) {
             $sheets[] = new SemesterEvaluationExport($this->includedUsers);
-            if(user()->hasRole(Role::STUDENT_COUNCIL)) {
+            if(user()->hasRole([Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS])) {
                 $sheets[] = new StudentsCouncilFeedback($this->includedUsers);
             }
         }

app/Models/Role.php

@@ -45,6 +45,12 @@ class Role extends Model
     public const DIRECTOR = 'director';
     public const STAFF = 'staff';
     public const LOCALE_ADMIN = 'locale-admin';
+    /**
+     * Danger: STUDENT_COUNCIL role is given to many users who ARE NOT actually part of the elected part of the student council (like KKT handlers, committee members). 
+     * It is generally a mistake if you simply check for the existence of this role. You usually need to check for the RoleObject as well.
+     * You most likely would like to use STUDENT_COUNCIL => STUDENT_COUNCIL_LEADERS or STUDENT_COUNCIL => STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS
+     * instead of STUDENT_COUNCIL in policies.
+     */
     public const STUDENT_COUNCIL = 'student-council';
     public const STUDENT_COUNCIL_SECRETARY = 'student-council-secretary';
     public const BOARD_OF_TRUSTEES_MEMBER = 'board-of-trustees-member';
@@ -81,6 +87,15 @@ class Role extends Model
         self::COMMUNICATION_LEADER,
         self::SPORT_LEADER,
     ];
+    public const STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS = [
+        self::PRESIDENT,
+        self::SCIENCE_VICE_PRESIDENT,
+        self::ECONOMIC_VICE_PRESIDENT,
+        self::CULTURAL_LEADER,
+        self::COMMUNITY_LEADER,
+        self::COMMUNICATION_LEADER,
+        self::SPORT_LEADER,
+    ];
     public const COMMITTEE_REFERENTS = [
         self::CULTURAL_REFERENT,
         self::COMMUNITY_REFERENT,

app/Policies/AnswerSheetPolicy.php

@@ -18,6 +18,6 @@ class AnswerSheetPolicy
     public function administer(User $user): bool
     {
         return $user->isAdmin()
-          || $user->hasRole(Role::STUDENT_COUNCIL);
+          || $user->hasRole([Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS]);
     }
 }

app/Policies/CommunityServicePolicy.php

@@ -40,7 +40,7 @@ public function create(User $user)
      */
     public function approveAny(User $user)
     {
-        return $user->hasRole([Role::STUDENT_COUNCIL]);
+        return $user->hasRole([Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS]);
     }
 
     /**

app/Policies/ReservableItemPolicy.php

@@ -42,7 +42,7 @@ public function requestReservationForType(User $user, ReservableItemType $type):
                     return $user->hasRole([Role::COLLEGIST, Role::TENANT]);
                 case ReservableItemType::ROOM:
                     return config('custom.room_reservation_open')
-                        && $user->hasRole([Role::WORKSHOP_LEADER, Role::WORKSHOP_ADMINISTRATOR, Role::STUDENT_COUNCIL]);
+                        && $user->hasRole([Role::WORKSHOP_LEADER, Role::WORKSHOP_ADMINISTRATOR, Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS]);
                 default:
                     throw new \Exception("unknown ReservableItemType");
             }

app/Policies/UserPolicy.php

@@ -35,7 +35,7 @@ public function viewAll(User $user): bool
             Role::SECRETARY,
             Role::DIRECTOR,
             Role::STUDENT_COUNCIL_SECRETARY,
-            Role::STUDENT_COUNCIL => array_merge(Role::STUDENT_COUNCIL_LEADERS, Role::COMMITTEE_LEADERS),
+            Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS,
         ]);
     }
 
@@ -68,7 +68,7 @@ public function viewAny(User $user): bool
                 Role::WORKSHOP_ADMINISTRATOR,
                 Role::WORKSHOP_LEADER,
                 Role::STUDENT_COUNCIL_SECRETARY,
-                Role::STUDENT_COUNCIL => array_merge(Role::STUDENT_COUNCIL_LEADERS, Role::COMMITTEE_LEADERS),
+                Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS,
             ]);
     }
 
@@ -102,7 +102,7 @@ public function view(User $user, User $target): bool
                 return $user->hasRole([
                     Role::SECRETARY,
                     Role::DIRECTOR,
-                    Role::STUDENT_COUNCIL => array_merge(Role::STUDENT_COUNCIL_LEADERS, Role::COMMITTEE_LEADERS),
+                    Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS,
                     Role::STUDENT_COUNCIL_SECRETARY,
                 ]);
             })) || $target->workshops
@@ -128,7 +128,7 @@ public function updateAnyPermission(User $user, User $target, Role $role = null)
         if (!isset($role)) {
             return $user->hasRole([
                 Role::SECRETARY,
-                Role::STUDENT_COUNCIL => array_merge(Role::STUDENT_COUNCIL_LEADERS, Role::COMMITTEE_LEADERS),
+                Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS,
                 Role::STUDENT_COUNCIL_SECRETARY,
                 Role::WORKSHOP_ADMINISTRATOR,
                 Role::WORKSHOP_LEADER
@@ -182,7 +182,7 @@ public function updateAnyPermission(User $user, User $target, Role $role = null)
 
         if ($role->name == Role::STUDENT_COUNCIL) {
             return $user->hasRole([
-                Role::STUDENT_COUNCIL => array_merge(Role::STUDENT_COUNCIL_LEADERS, Role::COMMITTEE_LEADERS),
+                Role::STUDENT_COUNCIL => Role::STUDENT_COUNCIL_LEADERS_AND_COMMITTEE_LEADERS,
                 Role::STUDENT_COUNCIL_SECRETARY
             ]);
         }