User roles: Use roles instead of status

config/packages/security.yaml

@@ -14,12 +14,13 @@ security:
                 class: Chamilo\CoreBundle\Entity\User
                 property: username
     role_hierarchy:
-        ROLE_STUDENT: [ROLE_USER, ROLE_STUDENT]
+        ROLE_STUDENT: [ROLE_USER]
         ROLE_ADMIN:
             - ROLE_USER
+            - ROLE_STUDENT
+            - ROLE_TEACHER
             - ROLE_QUESTION_MANAGER
             - ROLE_SESSION_MANAGER
-            - ROLE_TEACHER
             - ROLE_CURRENT_COURSE_TEACHER
             - ROLE_CURRENT_COURSE_SESSION_TEACHER
             - ROLE_CURRENT_COURSE_GROUP_TEACHER
@@ -27,9 +28,10 @@ security:
         ROLE_GLOBAL_ADMIN: [ROLE_ADMIN, ROLE_ALLOWED_TO_SWITCH] # The user that installed the platform.
         ROLE_TEACHER: [ROLE_STUDENT]
         ROLE_RRHH: [ROLE_TEACHER, ROLE_ALLOWED_TO_SWITCH]
-        ROLE_QUESTION_MANAGER: [ROLE_STUDENT, ROLE_QUESTION_MANAGER]
-        ROLE_SESSION_MANAGER: [ROLE_STUDENT, ROLE_SESSION_MANAGER, ROLE_ALLOWED_TO_SWITCH]
+        ROLE_QUESTION_MANAGER: [ROLE_STUDENT]
+        ROLE_SESSION_MANAGER: [ROLE_STUDENT, ROLE_ALLOWED_TO_SWITCH]
         ROLE_STUDENT_BOSS: [ROLE_STUDENT]
+        ROLE_INVITEE: [ROLE_STUDENT]
 
         ROLE_CURRENT_COURSE_STUDENT: [ROLE_CURRENT_COURSE_STUDENT] # Set in the CourseListener
         ROLE_CURRENT_COURSE_TEACHER: [ROLE_CURRENT_COURSE_TEACHER, ROLE_CURRENT_COURSE_STUDENT] # Set in the course listener

public/main/inc/lib/api.lib.php

@@ -2532,7 +2532,7 @@ function api_get_session_image($sessionId, User $user)
 {
     $sessionId = (int) $sessionId;
     $image = '';
-    if ($user->hasRole('ROLE_STUDENT')) {
+    if (!$user->hasRole('ROLE_STUDENT')) {
         // Check whether is not a student
         if ($sessionId > 0) {
             $image = '  '.Display::return_icon(
@@ -5341,40 +5341,41 @@ function api_is_global_platform_admin($user_id = null)
 
 /**
  * @param int  $admin_id_to_check
- * @param int  $my_user_id
+ * @param int  $userId
  * @param bool $allow_session_admin
  *
  * @return bool
  */
 function api_global_admin_can_edit_admin(
     $admin_id_to_check,
-    $my_user_id = null,
+    $userId = 0,
     $allow_session_admin = false
 ) {
-    if (empty($my_user_id)) {
-        $my_user_id = api_get_user_id();
+    if (empty($userId)) {
+        $userId = api_get_user_id();
     }
 
-    $iam_a_global_admin = api_is_global_platform_admin($my_user_id);
+    $iam_a_global_admin = api_is_global_platform_admin($userId);
     $user_is_global_admin = api_is_global_platform_admin($admin_id_to_check);
 
     if ($iam_a_global_admin) {
         // Global admin can edit everything
         return true;
-    } else {
-        // If i'm a simple admin
-        $is_platform_admin = api_is_platform_admin_by_id($my_user_id);
+    }
 
-        if ($allow_session_admin) {
-            $is_platform_admin = api_is_platform_admin_by_id($my_user_id) || (SESSIONADMIN == api_get_user_status($my_user_id));
-        }
+    // If i'm a simple admin
+    $is_platform_admin = api_is_platform_admin_by_id($userId);
 
-        if ($is_platform_admin) {
-            if ($user_is_global_admin) {
-                return false;
-            } else {
-                return true;
-            }
+    if ($allow_session_admin && !$is_platform_admin) {
+        $user = api_get_user_entity($userId);
+        $is_platform_admin = $user->hasRole('ROLE_SESSION_MANAGER');
+    }
+
+    if ($is_platform_admin) {
+        if ($user_is_global_admin) {
+            return false;
+        } else {
+            return true;
         }
     }
 
@@ -5383,14 +5384,14 @@ function api_global_admin_can_edit_admin(
 
 /**
  * @param int  $admin_id_to_check
- * @param int  $my_user_id
+ * @param int  $userId
  * @param bool $allow_session_admin
  *
  * @return bool|null
  */
-function api_protect_super_admin($admin_id_to_check, $my_user_id = null, $allow_session_admin = false)
+function api_protect_super_admin($admin_id_to_check, $userId = null, $allow_session_admin = false)
 {
-    if (api_global_admin_can_edit_admin($admin_id_to_check, $my_user_id, $allow_session_admin)) {
+    if (api_global_admin_can_edit_admin($admin_id_to_check, $userId, $allow_session_admin)) {
         return true;
     } else {
         api_not_allowed();
@@ -6859,18 +6860,23 @@ function api_register_campus($listCampus = true)
     }
 }
 
+function api_user_has_role(string $role): bool
+{
+    $currentUser = api_get_current_user();
+
+    if (null === $currentUser) {
+        return false;
+    }
+
+    return $currentUser->hasRole($role);
+}
+
 /**
  * Checks whether current user is a student boss.
- *
- * @global array $_user
- *
- * @return bool
  */
-function api_is_student_boss()
+function api_is_student_boss(): bool
 {
-    $_user = api_get_user_info();
-
-    return isset($_user['status']) && STUDENT_BOSS == $_user['status'];
+    return api_user_has_role('ROLE_STUDENT_BOSS');
 }
 
 /**

public/main/inc/lib/usermanager.lib.php

@@ -14,8 +14,6 @@ use ChamiloSession as Session;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 
 /**
- * Class UserManager.
- *
  * This library provides functions for user management.
  * Include/require it in your code to use its functionality.
  *
@@ -289,8 +287,10 @@ class UserManager
             $user->setExpirationDate($expirationDate);
         }
 
+        $user->setRoleFromStatus($status);
+
         // Add user to a group
-        $statusToGroup = [
+        /*$statusToGroup = [
             COURSEMANAGER => 'TEACHER',
             STUDENT => 'STUDENT',
             DRH => 'RRHH',
@@ -304,7 +304,7 @@ class UserManager
             if ($group) {
                 $user->addGroup($group);
             }
-        }
+        }*/
 
         $repo->updateUser($user, true);
 
@@ -1048,10 +1048,7 @@ class UserManager
 
         if (!empty($expiration_date)) {
             $expiration_date = api_get_utc_datetime($expiration_date);
-            $expiration_date = new \DateTime(
-                $expiration_date,
-                new DateTimeZone('UTC')
-            );
+            $expiration_date = new \DateTime($expiration_date, new DateTimeZone('UTC'));
         }
 
         $user
@@ -1075,28 +1072,19 @@ class UserManager
             $user->setPlainPassword($password);
         }
 
-        $statusToGroup = [
-            COURSEMANAGER => 'TEACHER',
-            STUDENT => 'STUDENT',
-            DRH => 'RRHH',
-            SESSIONADMIN => 'SESSION_ADMIN',
-            STUDENT_BOSS => 'STUDENT_BOSS',
-            INVITEE => 'INVITEE',
-        ];
-
-        $group = Container::$container->get(GroupRepository::class)->findOneBy(['code' => $statusToGroup[$status]]);
+        $user->setRoleFromStatus($status);
+        /*$group = Container::$container->get(GroupRepository::class)->findOneBy(['code' => $statusToGroup[$status]]);
         if ($group) {
             $user->addGroup($group);
-        }
+        }*/
 
         $userManager->updateUser($user, true);
         Event::addEvent(LOG_USER_UPDATE, LOG_USER_ID, $user_id);
 
         if (1 == $change_active) {
+            $event_title = LOG_USER_DISABLE;
             if (1 == $active) {
                 $event_title = LOG_USER_ENABLE;
-            } else {
-                $event_title = LOG_USER_DISABLE;
             }
             Event::addEvent($event_title, LOG_USER_ID, $user_id);
         }

src/CoreBundle/Entity/User.php

@@ -1790,6 +1790,21 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso
         return $this->hasRole('ROLE_SUPER_ADMIN');
     }
 
+    public function setRoleFromStatus(int $status): void
+    {
+        $role = match ($status) {
+            COURSEMANAGER => 'ROLE_TEACHER',
+            STUDENT => 'ROLE_STUDENT',
+            DRH => 'ROLE_RRHH',
+            SESSIONADMIN => 'ROLE_SESSION_MANAGER',
+            STUDENT_BOSS => 'ROLE_STUDENT_BOSS',
+            INVITEE => 'ROLE_INVITEE',
+            default => 'ROLE_USER',
+        };
+
+        $this->addRole($role);
+    }
+
     public function hasRole(string $role): bool
     {
         return \in_array(strtoupper($role), $this->getRoles(), true);
@@ -1839,11 +1854,6 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso
         return $this;
     }
 
-    public function isUser(self $user = null): bool
-    {
-        return null !== $user && $this->getId() === $user->getId();
-    }
-
     public function removeRole(string $role): self
     {
         if (false !== $key = array_search(strtoupper($role), $this->roles, true)) {

src/CoreBundle/Migrations/Schema/V200/Version20201212114910.php

@@ -70,6 +70,8 @@ final class Version20201212114910 extends AbstractMigrationChamilo
             $this->write("Migrating user: #$userId");
 
             $userEntity->setUuid(Uuid::v4());
+            $userEntity->setRoleFromStatus($userEntity->getStatus());
+
             $creatorId = $userEntity->getCreatorId();
             $creator = null;
             if (isset($userList[$adminId])) {

src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php

@@ -434,7 +434,7 @@ class ResourceNodeVoter extends Voter
 
         // Admin can do everything
         $acl->allow($admin);
-        $acl->allow($superAdmin);
+        //$acl->allow($superAdmin);
 
         //if ($token instanceof AnonymousToken) {
         if ($token instanceof NullToken) {

tests/CoreBundle/Repository/UserRepositoryTest.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+/* For licensing terms, see /license.txt */
+
+namespace Chamilo\Tests\CoreBundle\Repository;
+
+use Chamilo\CoreBundle\Repository\Node\UserRepository;
+use Chamilo\Tests\AbstractApiTest;
+use Chamilo\Tests\ChamiloTestTrait;
+
+/**
+ * @covers \UserRepository
+ */
+class UserRepositoryTest extends AbstractApiTest
+{
+    use ChamiloTestTrait;
+
+    public function testCreateUser(): void
+    {
+        self::bootKernel();
+        //$em = self::getContainer()->get('doctrine')->getManager();
+
+        $userRepo = self::getContainer()->get(UserRepository::class);
+        $student = $this->createUser('student');
+        $this->assertHasNoEntityViolations($student);
+
+        $this->assertSame(1, \count($student->getRoles()));
+        $this->assertTrue(\in_array('ROLE_USER', $student->getRoles(), true));
+
+        $student->addRole('ROLE_STUDENT');
+        $userRepo->update($student);
+
+        $this->assertSame(2, \count($student->getRoles()));
+
+        $student->addRole('ROLE_STUDENT');
+        $userRepo->update($student);
+
+        $this->assertSame(2, \count($student->getRoles()));
+    }
+}