[MGNLUI-2510] UI shouldn't enable actions for which the user has no permissions Created: 09/Dec/13 Updated: 21/Apr/15 Resolved: 12/Mar/14 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | page editor, pages app |
| Affects Version/s: | 5.2 |
| Fix Version/s: | 5.2.3 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Samuel Staehelin | Assignee: | Mikaël Geljić |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support, ux | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
demo.magnolia-cms.com |
||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||||||||||
| Description |
|
This ticket originally helped us uncover security issues in core (see issue links, first comments), but the UI should also adjust itself correctly to denote correct user permissions, by disabling unauthorized actions, and maintaining proper state according to such permissions. For 5.2.x, AvailabilityDefinition should gain a #writePermissionRequired flag. – ORIGINAL DESCRIPTION – I changed the permission of the editors in a subtree to read only.
|
| Comments |
| Comment by Philipp Bärfuss [ 10/Dec/13 ] |
|
We have to analyze wether this is a security or app related issue. In any case this is what should happen:
|
| Comment by Mikaël Geljić [ 10/Dec/13 ] |
|
First, all actions are available, regardless of such ACLs. By default, with user eric and an ACL to make the whole About subtree read-only, we were able to:
We were not able to:
So far we were not able to reproduce:
Will now run more testing with "colliding" groups or roles. |
| Comment by Mikaël Geljić [ 10/Dec/13 ] |
|
Further tests with "conflicting" roles: we added a new role to eric with an ACL granting RW access to the Large Article page.
|
| Comment by Mikaël Geljić [ 10/Dec/13 ] |
|
debugging the Move use case, the following permission check seems to (wrongly) pass: if (!acMgr.isGranted(childPath, Permission.MODIFY_CHILD_NODE_COLLECTION)) { This is in org.apache.jackrabbit.core.NodeImpl.orderBefore(Element, Element) |
| Comment by Mikaël Geljić [ 11/Dec/13 ] |
|
Repurposing ticket for UI glitches related to permissions, though they do not affect security of data. These are action availability issues (currently they don't take permissions into account) and page-editor state when navigating between RW and RO pages. |
| Comment by Andreas Weder [ 18/Dec/13 ] |
|
In general, I prefer to disable actions over simply not showing them. The list of actions should remain constant. This is particularly true, if you e.g. select different pages in the page tree, for some of which you do have the permission to e.g. edit the page, for others you don't. Disabling an action helps in understanding that an action is actually there, but is just unavailable now and/or to me. So I would prefer to always get the "edit" action, but have it disabled if I lack necessary permissions. In contrast, I would like to be able hide all actions for a specific product feature. To explain this, let's say that we have a section in the Action bar that allowed a user to personalize a page. If I don't have access to the entire personalization feature, I should not see any actions related to that feature - the entire section and its actions should be hidden, not merely disabled. This is different from disabling actions due to access rights. I might have a specific access right for one page, but not for another one: disabling the action is the right thing to do then. If I lack access to an entire product feature, I should not see anything related to it in the action bar, as I won't be able to use it anyway. |
| Comment by Mikaël Geljić [ 21/Jan/14 ] |
|
Stopped progress, now compiling results and input so far into the wiki - requires further discussion. See issue links: wiki page 'Permissions for UI availability'. |
| Comment by Mikaël Geljić [ 29/Jan/14 ] |
|
Results from 'Permissions for UI' meeting
This issue will only track work for 5.2.x. |
| Comment by Andreas Weder [ 20/Feb/14 ] |
|
Added ux label as I'd like to track this in the UX team. |
| Comment by Mikaël Geljić [ 07/Mar/14 ] |
|
Updated description following architecture review of Availability for 5.2.x and 5.3. In the end voters are out of the picture. |
| Comment by Mikaël Geljić [ 07/Mar/14 ] |
|
implemented and added #writePermissionRequired to the following set of pages browser actions: |
| Comment by Roman Kovařík [ 10/Mar/14 ] |
|
| Comment by Mikaël Geljić [ 10/Mar/14 ] |
|
Thanks Roman, |
| Comment by Mikaël Geljić [ 10/Mar/14 ] |
However there's a known general limitation that is worth mentioning on availability: root node availability bypasses all other conditions. It relies on checking null item instead of the actual root node, so we have no workspace and root path info to check availability against. This won't be fixed until availability is revamped in 5.3. |
| Comment by Christoph Meier [ 11/Mar/14 ] |
|
QA-comment: |
| Comment by Milan Divilek [ 11/Mar/14 ] |
|
Reopen: As a superuser I can't edit superuser user and forum_ALL_admin role (not sure if something other I found this two case). |