Workspaces are implemented as config entities which by default doesn't really have a access API. But config entities can probably implement the same API as content entities do.

Along with this we need to decide on how access in general should work. How do we manage access control to workspaces. What will happen if one tries to switch to a workspace you don't have access to, etc.

Comments

dixon_ created an issue. See original summary.

jeqq’s picture

Status: Active » Needs review

Workspaces are now content entity types and we have the administer workspaces permission for workspaces management, also we use this permission to restrict access to the entity paths and forms.

dixon_’s picture

Status: Needs review » Needs work

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 workspace
  • view 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 workspace
  • edit $id workspace
  • delete $id workspace
  • bypass content access control in $id workspace
dixon_’s picture

Priority: Major » Critical
dixon_’s picture

To implement the above we'll need to implement hook_entity_access().

dixon_’s picture

Project: Multiversion » Workspace
Crell’s picture

Assigned: Unassigned » Crell

MINE!

Crell’s picture

Status: Needs work » Needs review
StatusFileSize
new13.06 KB

Work 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"?

dixon_’s picture

@Crell this looks really good so far! As for the "view" permission, I think we need a couple of things

  1. Move everything under \Drupal\multiversion\Workspace\* into Workspace module, because:
    • It probably makes our life easier to implement "view" access checking if Workspace module own these services
    • Multiversion should be kept lightweight and only deal with storage anyhow, so moving them makes sense
  2. Inject the \Drupal\workspace\EntityAccess into WorkspaceManager
  3. Do "view" access checking inside WorkspaceManager::getActiveWorkspace() and throw some exception if the current user doesn't have access
  4. Hook into the request stack somewhere and use WorkspaceManager::getActiveWorkspace() which will throw the above exception on access denied

We should be able to test the following pseudo code:

  1. Create user A who only have access to view her own workspaces
  2. Create workspace Z that is NOT owned by user A
  3. Set workspace Z as the active workspace (with WorkspaceManager)
  4. GET the front page (or ANY other route) as user A
  5. HTTP 403 should be returned

Status: Needs review » Needs work

The last submitted patch, 8: 2600382-permissions.patch, failed testing.

The last submitted patch, 8: 2600382-permissions.patch, failed testing.

The last submitted patch, 8: 2600382-permissions.patch, failed testing.

Crell’s picture

I 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).

Crell’s picture

Status: Needs work » Needs review
StatusFileSize
new21.29 KB
new13.62 KB

After 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.

Crell’s picture

Crell’s picture

StatusFileSize
new28.24 KB
new12.4 KB

And here we go, now with 100% more per-workspace permissions!

Tixon, back over to you for review.

Status: Needs review » Needs work

The last submitted patch, 16: 2600382-permissions.patch, failed testing.

  • timmillwood committed bf50f69 on 8.x-1.x authored by Crell
    Issue #2600382 by Crell: Come up with a simple access/permission system...
timmillwood’s picture

Status: Needs work » Fixed

ok, 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 control permissions.

Crell’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

The last submitted patch, 14: 2600382-permissions.patch, failed testing.

The last submitted patch, 14: 2600382-permissions.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 16: 2600382-permissions.patch, failed testing.

The last submitted patch, 14: 2600382-permissions.patch, failed testing.

The last submitted patch, 16: 2600382-permissions.patch, failed testing.

timmillwood’s picture

Status: Needs work » Closed (fixed)