Fixing bug in notification role testing (and better documenting the c…

…ode to avoid such mistakes).
ReCodEx · May 22, 2019 · e9584cb · e9584cb
1 parent 309309c
commit e9584cb
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 14 deletions.
diff --git a/app/V1Module/presenters/NotificationsPresenter.php b/app/V1Module/presenters/NotificationsPresenter.php
@@ -63,7 +63,7 @@ function (Notification $notification) {
         return $this->notificationAcl->canViewDetail($notification);
       });
 
-    $this->sendSuccessResponse($notifications);
+    $this->sendSuccessResponse(array_values($notifications));
   }
 
   public function checkAll() {
@@ -83,7 +83,7 @@ function (Notification $notification) {
         return $this->notificationAcl->canViewDetail($notification);
       });
 
-    $this->sendSuccessResponse($notifications);
+    $this->sendSuccessResponse(array_values($notifications));
   }
 
   public function checkCreate() {

diff --git a/app/V1Module/security/Authorizator.php b/app/V1Module/security/Authorizator.php
@@ -48,7 +48,7 @@ protected function checkPermissionsForRoleList($roleList, $resource, $privilege)
     return false;
   }
 
-  protected function isInRole($target, $role): bool {
-    return $this->roles->isInRole($target, $role);
+  protected function isInRole(string $actualTestedRole, string $minimalRequestedRole): bool {
+    return $this->roles->isInRole($actualTestedRole, $minimalRequestedRole);
   }
 }
diff --git a/app/V1Module/security/Policies/NotificationPermissionPolicy.php b/app/V1Module/security/Policies/NotificationPermissionPolicy.php
@@ -27,7 +27,7 @@ public function hasRole(Identity $identity, Notification $notification) {
       return false;
     }
 
-    return $this->roles->isInRole($notification->getRole(), $user->getRole());
+    return $this->roles->isInRole($user->getRole(), $notification->getRole());
   }
 
   public function isGlobal(Identity $identity, Notification $notification) {

diff --git a/app/V1Module/security/Roles.php b/app/V1Module/security/Roles.php
@@ -14,23 +14,32 @@ abstract class Roles
   public const EMPOWERED_SUPERVISOR_ROLE = "empowered-supervisor";
   public const SUPERADMIN_ROLE = "superadmin";
 
-  protected $roles = [];
+  /**
+   * @var array
+   * Indices are role names, values holds a list of all parents (from which a role inherits permissions).
+   */
+  protected $rolesParents = [];
 
 
   public abstract function setup();
 
-  protected function addRole($role, $parents) {
-    $this->roles[$role] = $parents;
+  protected function addRole(string $role, array $parents) {
+    $this->rolesParents[$role] = $parents;
   }
 
-  public function isInRole($target, $role): bool {
-    if ($target === $role) {
+  /**
+   * Verify whether given actual role has at least the permissions of minimal requested role.
+   * @param string $actualTestedRole
+   * @param string $minimalRequestedRole
+   */
+  public function isInRole(string $actualTestedRole, string $minimalRequestedRole): bool {
+    if ($actualTestedRole === $minimalRequestedRole) {
       return true;
     }
 
-    if (array_key_exists($target, $this->roles)) {
-      foreach ($this->roles[$target] as $parent) {
-        if ($this->isInRole($parent, $role)) {
+    if (array_key_exists($actualTestedRole, $this->rolesParents)) {
+      foreach ($this->rolesParents[$actualTestedRole] as $parent) {
+        if ($this->isInRole($parent, $minimalRequestedRole)) {
           return true;
         }
       }
@@ -45,7 +54,7 @@ public function isInRole($target, $role): bool {
    * @return bool true if given role is valid
    */
   public function validateRole(string $role): bool {
-    if (array_key_exists($role, $this->roles)) {
+    if (array_key_exists($role, $this->rolesParents)) {
       return true;
     }