Closed (fixed)
Project:
Workspace
Version:
8.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
24 Oct 2015 at 11:58 UTC
Updated:
22 Jun 2016 at 14:30 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
jeqq commentedWorkspaces are now content entity types and we have the
administer workspacespermission for workspaces management, also we use this permission to restrict access to the entity paths and forms.Comment #3
dixon_What we need here is access control for switching and viewing workspaces.
Here are some thoughts:
administer workspaces- Allows to a user to do everything with all workspace entities.create workspaceview any workspace- Applies to switching and viewing workspaces. Content entity access within workspaces is handled as usual.edit any workspace- Applies to the workspace entity. Content entity access within workspaces is handled as usual.delete any workspace- Applies to the workspace entity. Content entity access within workspaces is handled as usual.view own workspace- Allows users to switch to workspaces created by themselves. Content entity access within the workspace is handled as usual.edit own workspace- Allows users to edit its own workspace (e.g. the workspace entity itself). Entity access within the workspace is handled as usual.delete own workspace- Allows users to delete its own workspaces (e.g. the workspace entity itself and all content in it).bypass content access control in own workspace- Similar to the "bypass content access control" but specific to only the user's own workspaces. This will allow users to edit any content in own workspace. If the user does not have this permission, then regular content entity access rules would apply.view $id workspaceedit $id workspacedelete $id workspacebypass content access control in $id workspaceComment #4
dixon_Comment #5
dixon_To implement the above we'll need to implement
hook_entity_access().Comment #6
dixon_Comment #7
Crell commentedMINE!
Comment #8
Crell commentedWork in progress snapshot, courtesy of the new 2600382-permissions branch.
The general create, edit any, edit own permissions seem to work here, and include BrowserTests (yay). Incidentally, BTB is terrible for debugging. :-(
Next step is the per-workspace permissions. I'll work on that tomorrow.
Related question: How exactly would we properly test the "view*" permissions? They probably work now, but we should get a test to verify that. I'm not sure exactly what that looks like here. Does it really mean "switch to", in which case we need to add a permission check to the switch-to logic? If so, should it be called "Switch to" rather than "view"?
Comment #9
dixon_@Crell this looks really good so far! As for the "view" permission, I think we need a couple of things
\Drupal\multiversion\Workspace\*into Workspace module, because:\Drupal\workspace\EntityAccessintoWorkspaceManagerWorkspaceManager::getActiveWorkspace()and throw some exception if the current user doesn't have accessWorkspaceManager::getActiveWorkspace()which will throw the above exception on access deniedWe should be able to test the following pseudo code:
Comment #13
Crell commentedI don't know that point 1 is feasible, since the WorkspaceManager is used in a lot of places outside of that namespace. However, anywhere we have a workspace entity we can simply do $entity->access('view', $account), which will give us a boolean. That doesn't require a hard dependency on Workspace module (although it does likely mean that without it, you'll always get access denied).
Comment #14
Crell commentedAfter much fighting with core, here's view access control!
There's no delete UI at all at the moment, so I am not testing that. The permissions are created and probably work, I think? :-)
Next step: The per-workspace permissions. Although, are those useful? Should we encourage that sort of long-lived workspace? Or will workspaces be short-lived enough that they don't matter? Hum...
Note: All tests in this module pass for me locally. I have no idea what testbot is whining about.
Comment #15
Crell commentedAlso, you'll probably want this patch for Multiversion: #2692527: Don't allow switching to an inaccessible workspace
Comment #16
Crell commentedAnd here we go, now with 100% more per-workspace permissions!
Tixon, back over to you for review.
Comment #19
timmillwoodok, tests pass locally, so think we're good on this.
Now back to #2692527: Don't allow switching to an inaccessible workspace.
@Crell will open a new issue for the
bypass content access controlpermissions.Comment #20
Crell commentedSee you in #2693867: Allow users to bypass node access controls in selected workspaces.
Comment #27
timmillwood