Problem/Motivation
1. At some point we check for "administer nodes" permission, which is not OK for a generic access check implementation. We should rely on $entity_type->getAdminPermission() instead.
2. if "list" operation is being checked and we end up passing it on to the entity access unpredictable results can happen (even fatal in my - CRM core - case). "list" doesn't seem to be standard operation and entity access handlers don't handle it in general. We should either pass something else instead of list of communicate clear expectation that we expect access handlers to be able to handle that.
Proposed resolution
1. Rely on admin permission.
2. When "list" operation is checked use "view" when checking access with the entity access handler.
We should update tests to cover this cases.
Comment | File | Size | Author |
---|---|---|---|
#3 | 2729889-3.patch | 3.65 KB | dawehner |
#3 | 2729889-test.patch | 1.19 KB | dawehner |
#2 | 2729889_2.patch | 2.39 KB | slashrsm |
|
Comments
Comment #2
slashrsm CreditAttribution: slashrsm at MD Systems GmbH commentedHad 5 mins to look into this. We still need tests. Any feedback on the approach would be awesome.
Comment #3
dawehnerThank you @slashrsm
This patch totally makes sense. Here is a test to prove that things actually work.
Comment #4
dawehnerComment #6
dawehnerWell the branch test are broken due to still using php 5.5
Sorry again for not seeing the issue earlier. Thank you @slashrsm!