Task #12474 (closed)
Bug: Object permissions object does not reflect true permissions
Reported by: | drussell-x | Owned by: | jamoore |
---|---|---|---|
Priority: | critical | Milestone: | 5.1.3 |
Component: | Queries | Version: | 5.0.2 |
Keywords: | n.a. | Cc: | java@…, cxallan |
Resources: | n.a. | Referenced By: | n.a. |
References: | n.a. | Remaining Time: | 0.0d |
Sprint: | n.a. |
Description
All these queries are run as the user 'memberonly':
id | login | first name | last name | email | active | admin | member of | owner of -----+------------+------------+-----------+--------------------------------+--------+-------+-----------+---------- 152 | memberonly | Member | Only | | Yes | | 103 |
This is an example of a read-only group:
id | name | perms | # of owners | # of members -----+--------------------+--------+-------------+-------------- 103 | ReadOnlyGroup | rwr--- | 1 | 2
Here is an image that belongs to the owner of that group, root:
$ omero hql "select image.id, exp.omeName, grp.name from Image image join image.details.owner exp join image.details.group grp where image.id=452" Using session 85f1a5b3-1029-4bc7-97ea-3b95f62d4baf (memberonly@localhost:4064). Idle timeout: 10.0 min. Current group: ReadOnlyGroup # | Col1 | Col2 | Col3 ---+------+------+--------------- 0 | 452 | root | ReadOnlyGroup
The permissions when getting that image:
$ omero hql "select image.details.permissions from Image image where image.id=452" Using session 85f1a5b3-1029-4bc7-97ea-3b95f62d4baf (memberonly@localhost:4064). Idle timeout: 10.0 min. Current group: ReadOnlyGroup # | Col1 ---+---------------------------------------------------------------------------------------------- 0 | {'canLink': True, 'canEdit': True, 'canDelete': True, 'perm': 'rwr---', 'canAnnotate': True} (1 row)
As it is a read-only group, the memberonly user should not be showing these permissions. This is just an example, it seems the permissions are wrong in all cases except where the querying user is also the owner or an administrator, in which case I guess it is correct, but probably in the same way that a stopped clock is right twice a day.
Change History (14)
comment:1 Changed 10 years ago by jamoore
- Cc java@… cxallan added
- Milestone changed from Unscheduled to 5.0.4
- Priority changed from critical to blocker
comment:2 Changed 10 years ago by jamoore
Bah. @Parent annotation looked promising then I ran into https://hibernate.atlassian.net/browse/HHH-3868
comment:3 Changed 10 years ago by jamoore
Note: this bug was not detected due to the format of the assertion in Python:
assert "%s mismatch!" % x, found == expected
Tests fixed in https://github.com/openmicroscopy/openmicroscopy/pull/2849
comment:4 Changed 10 years ago by jmoore <josh@…>
(In [cd023fcd7fcdaf212f4a3f09f2b0bd066b6d9f24/ome.git] on branch develop) Fix assert error which led to this bug (See #12474)
assert STRING, CONDITION will always pass
since a non-empty STRING is a valid CONDITION.
comment:5 Changed 10 years ago by jmoore <josh@…>
(In [70eec7c528816d54e41ff3e4da0889c81cb3ac5a/ome.git] on branch develop) Add a passing workaround test (See #12474)
As a workaround, it's possible to load a single object
per (group, owner) pair and cache the permissions from
that object. With this commit, the test outcomes are:
`
test/integration/test_permissions.py xxXxxXxxXXXxXXxXXxXXxXXxXXxXXXXXXXXX....................................
30 tests deselected by '-ktestProjectionPermissions'
36 passed, 30 deselected, 12 xfailed, 24 xpassed in 45.35 seconds
`
comment:6 Changed 10 years ago by jamoore
PR opened against hibernate: https://github.com/hibernate/hibernate-orm/pull/823
Conceivably I could backport this to the existing version.
comment:7 Changed 10 years ago by drussell-x
Your Hibernate fix seems to have been merged which is great, what do you reckon on the chances of it being in their release and then in our Hibernate upgrade? Or a backport as you say would also solve this.
comment:8 Changed 10 years ago by jburel
- Milestone changed from 5.1.0-m3 to 5.1.0-m4
comment:9 Changed 10 years ago by jamoore
- Milestone changed from 5.1.0 to 5.1.1
comment:10 Changed 9 years ago by jamoore
- Milestone changed from 5.1.1 to 5.1.2
- Priority changed from blocker to critical
I will not be able to get a PR in for 5.1.1 freeze. Pushing.
comment:11 Changed 9 years ago by jamoore
- Milestone changed from 5.1.2 to 5.1.3
Sorry, I didn't manage Hibernate 3.6 for 5.1.2.
comment:12 Changed 9 years ago by jamoore
- Resolution set to fixed
- Status changed from new to closed
I couldn't find a solution for this in pure Hibernate (see the extended discussion under https://trello.com/c/ahpjb7IY/6-hibernate-3-6)
But https://github.com/openmicroscopy/openmicroscopy/pull/3910 contains a workaround which clients can use in their queries: select new ome.util.PermDetails(d) rather than select d.details.permissions.
comment:13 Changed 9 years ago by jmoore <josh@…>
- Remaining Time set to 0
(In [fe086fdbad8d8e10dcdec6d0d9803f255ab003c2/ome.git] on branch develop) Add wrapper class for loading permission info (Fix #12474)
Since no solution for properly loading a Permissions object
when the target of a project query (select d.details.permissions),
this workaround is being added which will allow wrapping a
object in HQL (select new ome.util.PermDetails(d)) to produce
the desired effect.
comment:14 Changed 9 years ago by Sébastien Besson <seb.besson@…>
(In [00a886e14daf4170f3e7e22953a73845597e11b1/ome.git] on branch develop) Merge pull request #3910 from joshmoore/12474-perm-wrapper
Add wrapper class for loading permission info (Fix #12474)
In the way of explanation, what's happening is that since Permissions are not IObjects, the if instanceof IObject statement in ProxyCleanupFilter of course fails, and acl.postProcess((IObject) f) can never be called, meaning permissions.copyRestrictions(allow) can never be called, meaning the restrictions are null. When this feature was developed, I have *no* clue how it got passed testing since it is broken as designed. The only workaround I can think of (at the moment) is if Hibernate gives me some kind of hook to know which row matches which permission object, at which point I'll then have to load the object (or at least its owner/group details). Easier may ultimately be to map all of this information directly via a pgsql function.