Bug #7817

QubitAcl buildAcl method should not scan permissions belonging to other users

Added by Jesús García Crespo over 7 years ago. Updated about 7 years ago.

Status:VerifiedStart date:01/14/2015
Priority:HighDue date:
Assignee:Jesús García Crespo% Done:

0%

Category:Access Control
Target version:Release 2.2.0
Google Code Legacy ID: Tested version:2.1, 2.1.1
Sponsored:No Requires documentation:

Description

How to reproduce

  • Create group A
  • Assign some permissions to the group, including ALLOW permissions for a specific repository
  • Create user A1
  • Add permission ALLOW TRANSLATIONS TO FRENCH to A1
  • Create user A2
  • Log in as A2

Error

Role '312' not found

The problem seems to be in buildAcl (QubitAcl class).

There is a foreach that iterates over the user permissions including those belonging to its group (because of the way the SQL query is made). However the query seems also to be matching permissions assigned to a different user.
When $roleId is defined in #L377 for a permission that belongs to a different user, it takes the value of the other user:

$roleId = (isset($permission->userId)) ? $permission->userId : $permission->groupId;

Zend/Acl breaks because the ID of the other user is not recognized.

Solution

We could either update the $c1 criterion to avoid including perms from other users or ignore the iteration within the foreach like follows:

-        $roleId = (isset($permission->userId)) ? $permission->userId : $permission->groupId;
+
+        if (isset($permission->userId))
+        {
+          // Ignore this permission as it doesn't belong to the current user
+          // TODO: Update query, i.e. $c1 criterion should have user_id limited to current user or NULL?
+          if ($permission->userId != $this->_user->getUserID())
+          {
+            continue;
+          }
+
+          $roleId = $permission->userId;
+        }
+        else
+        {
+          $roleId = $permission->groupId;
+        }

Related issues

Related to Access to Memory (AtoM) - Bug #8115: Allowed languages for translation not working when they a... Verified 03/18/2015

History

#2 Updated by Jesús García Crespo over 7 years ago

  • Subject changed from QubitAcl issue to QubitAcl buildAcl method should not scan permissions belonging to other users

#3 Updated by Sarah Romkey over 7 years ago

  • Assignee changed from Jesús García Crespo to José Raddaoui Marín

#4 Updated by Sarah Romkey about 7 years ago

  • Target version deleted (Release 2.2.0)

#5 Updated by José Raddaoui Marín about 7 years ago

  • Related to Bug #8115: Allowed languages for translation not working when they are set creating a NEW user added

#6 Updated by José Raddaoui Marín about 7 years ago

  • Status changed from New to QA/Review
  • Assignee changed from José Raddaoui Marín to Jesús García Crespo

The problem in #7811 was a row in the "acl_permission" table with the "user_id" and the "group_id" columns populated. I've tried to reproduce this adding all the permissions I could find for groups and users and checking the database, but all the permissions are saved with one id or the other, none was saved with both ids.

I'm not sure if we should mark this as "Won't fix" or if we should improve the query, just in case I'm missing something.

#7 Updated by Dan Gillean about 7 years ago

  • Target version set to Release 2.2.0

This has been affecting a client site, and Jesus apparently already has a patch, and feels like we should resolve this in 2.2. He will take a look soon.

#8 Updated by Dan Gillean about 7 years ago

  • Status changed from QA/Review to In progress

#9 Updated by Jesús García Crespo about 7 years ago

  • Status changed from In progress to Verified

I couldn't reproduce neither, but it definitely happened with at least one existing database so I decided to add the extra check just in case.

Also available in: Atom PDF