Overview

In #3452581: [META] XB Permissions we describe a Content Creator role.

For being able to access the XB UI without a pre-existing content, we need an access check for any XB-enabled entity.
This is required e.g. for adding the first page (#3529836: Enable starting with an empty XB UI (so without first having to create an entity with a component tree)).

Proposed resolution

User interface changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

penyaskito created an issue. See original summary.

penyaskito’s picture

Title: Add permission for "Use Experience Builder" » [PP-1] Add permission for "Use Experience Builder"
Status: Active » Postponed
wim leers’s picture

🤔 Shouldn't this be the other way around, i.e. this before #3529836? Because of:

Should this permission apply to every XB route?

Devil's advocate: why this permission? Only for #3529836? It's not one of the permissions @lauriii prescribed?

penyaskito’s picture

I doubted to be honest. But without #3529836 this will be hard to write tests for, while #3529836 can still use 'access administration pages' as we are doing now.

Why a new permission? Because we could use "Create Pages (for Page content entities)", but we might have XB enabled for other content entities and a user might not have permission to create pages but to edit e.g. articles. And I'm assuming here that the XB UI will be able to create any enabled XB content type from blank.

An alternative is another route access check that checks if I have permissions for any XB-enabled content.

penyaskito’s picture

Title: [PP-1] Add permission for "Use Experience Builder" » [PP-1] Add access check for using Experience Builder
Issue summary: View changes

An alternative is another route access check that checks if I have permissions for any XB-enabled content.

This will be probably the solution we will go for, so changed the title + IS

wim leers’s picture

Title: [PP-1] Add access check for using Experience Builder » [later phase] [PP-2] Add access check for using Experience Builder
Issue tags: +stable blocker

I think we have an alternative solution for #4 over at #3529836-7: Enable starting with an empty XB UI (so without first having to create an entity with a component tree) 😊

I'm pretty sure @lauriii specifically wanted to AVOID this kind of permission.

So then this is the generalization of #3529836-7: Enable starting with an empty XB UI (so without first having to create an entity with a component tree), for much later!

wim leers’s picture

Title: [later phase] [PP-2] Add access check for using Experience Builder » Add access check for using Experience Builder at all: if >=1 content entity type with an XB field can be created or edited.
Assigned: Unassigned » wim leers
Priority: Normal » Major
Status: Postponed » Active
Related issues: +#3529895: Provide the client with `create` operation access information similar to #3516657, +#3516432: Update all XB routes to respect content entity update/field edit access of edited XB field

@penyaskito's work at #3529895: Provide the client with `create` operation access information similar to #3516657 inspired me and made me realize a connection: the \Drupal\Core\Entity\EntityFieldManagerInterface::getFieldMapByFieldType()-based logic he wrote for \Drupal\experience_builder\Controller\ExperienceBuilderController::getContentEntityCreateOperations() is what #3529836: Enable starting with an empty XB UI (so without first having to create an entity with a component tree) needs as the long-term solution, rather than the pragmatic interim linked in #7.

Combine the pattern of #3529895 with the logic in ComponentTreeEditAccessCheck (from #3516432: Update all XB routes to respect content entity update/field edit access of edited XB field), and I think we have a solution!

wim leers’s picture

Component: Page builder » Internal HTTP API
Assigned: wim leers » penyaskito
Issue summary: View changes
Status: Active » Needs review
Issue tags: +Needs tests

Response for /xb/api/v0/config/component when accessing as not just the anonymous user, but also an authenticated user that for example can only edit Media entities, but not Pages nor article nodes:

{
  "errors": [
    "Requires >=1 content entity type with an XB field that can be created or edited."
  ]
}
penyaskito’s picture

@Wim at #8: This is exactly what I was envisioning in #3452581-54: [META] XB Permissions!!!

wim leers’s picture

Hah! 😄 Great minds … 🥸

penyaskito’s picture

penyaskito’s picture

Assigned: penyaskito » larowlan
Issue summary: View changes
Issue tags: -Needs tests
wim leers’s picture

Assigned: larowlan » Unassigned
Status: Needs review » Needs work
wim leers’s picture

Note that while reviewing #3522488: Follow-up for #3518292: `ApiContentControllers::list()`: search should exclude entity query matches for entities with auto-save data, I noticed this MR should also update

experience_builder.api.content.list:
  path: '/xb/api/v0/content/{entity_type}'
  defaults:
    _controller: 'Drupal\experience_builder\Controller\ApiContentControllers::list'
  requirements:
    _permission: 'edit xb_page'
  methods: [GET]
  options:
    _format: 'json'

Because

    _permission: 'edit xb_page'

makes no sense — it was a useful interim step, but what this issue is doing is more appropriate :)

wim leers’s picture

Discussed with @effulgentsia and @penyaskito — @effulgentsia agrees this should be beta-blocking.

penyaskito’s picture

Assigned: Unassigned » penyaskito
penyaskito’s picture

Assigned: penyaskito » wim leers
Status: Needs work » Needs review

Assuming the last failure is a cypress random failure, which I'm retrying, this should be ready for review.

wim leers’s picture

Assigned: wim leers » penyaskito
Status: Needs review » Needs work

Maybe I'm missing something, but these changes don't quite make sense to me? 😅

wim leers’s picture

isholgueras’s picture

Assigned: penyaskito » isholgueras

working on the reroll

isholgueras’s picture

Assigned: isholgueras » Unassigned

Reroll done.

wim leers’s picture

Assigned: Unassigned » penyaskito
penyaskito’s picture

Assigned: penyaskito » wim leers
Status: Needs work » Needs review

Fixed logic.

          // Grant access if the current user can:
          // 1. create such a content entity (and set the XB field)
          // 2. edit such a content entity (and update the XB field)
          // 3. edit code components, as there might a "component developer role"

There is one cypress test that requires the permission to pass, but I can't reproduce locally (but test fails locally too).

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, found one minor nit and self-addressed it.

wim leers’s picture

wim leers’s picture

Assigned: wim leers » Unassigned

Found only one problem: half a dozen remaining occurrences of the access administration pages permission. Refactored them all away. Was trivial thanks to the infrastructure (and examples!) in this MR 😊👍

Echo'ing @larowlan's RTBC — this is so long overdue, and feels great to finally get XB to this point! 😊 Thanks, @penyaskito!

wim leers’s picture

Got navigation.cy.js to green — it had previously implicitly (and inappropriately) been relying on the access administration pages permission, simply to use /admin/config as the "last visited URL" for the "Exit XB" functionality.

Merging at last… 🚢🥳

  • wim leers committed 87c252f7 on 0.x
    Issue #3529924 by penyaskito, wim leers, larowlan, effulgentsia: Add...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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