Permissions for XB config entity types

Without this, we need to grant anonymous users the administer code components permission for them to be visible 😅

Solid start, thanks! Posted a first round of feedback 😊

Very much on the right track! 🤩 Lots of suggestions, but loving where this is going! 👏🙏

Thanks for the clarifications, @lauriii and @tedbow! 🙏👍

I made a few further clarifications — do you agree?

I'm terribly sorry, @penyaskito, but turns out that pretty much everything you've done on this issue/MR turns out to be unnecessary now that @lauriii has provided precise product requirements 🫣😅

But … I'm pretty sure this issue has helped @lauriii arrive at those clear conclusions!

AFAICT given the refined product requirements for XB permissions (see the new meta: #3452581: [META] XB Permissions) means that this issue can just:

• Use a tiny subclass of default \Drupal\Core\Entity\EntityAccessControlHandler for the Component and Pattern config entity types, aka all XB config entity types that provide the basic building blocks for the XB UI. These must respect the admin_permission, except that viewing must always be allowed.

  For now, simply require the user to be authenticated (i.e. vary by the user.roles:authenticated) cache context).

• Use the default \Drupal\Core\Entity\EntityAccessControlHandler for the JavaScriptComponent, AssetLibrary and PageRegion config entity types, aka all XB config entity types that do not need to be available to all XB users.

  So that's basically just:

  diff --git a/src/Entity/PageRegion.php b/src/Entity/PageRegion.php
  index e384258ed..773f4d2ee 100644
  --- a/src/Entity/PageRegion.php
  +++ b/src/Entity/PageRegion.php
  @@ -23,7 +23,7 @@ use Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItemInstantiat
    *    label_singular = @Translation("page region"),
    *    label_plural = @Translation("page region"),
    *    label_collection = @Translation("Page region"),
  - *    admin_permission = "access administration pages",
  + *    admin_permission = "administer page template",
    *    entity_keys = {
    *      "id" = "id",
    *      "status" = "status",
  diff --git a/src/Entity/Pattern.php b/src/Entity/Pattern.php
  index 12e61d717..fb23d7895 100644
  --- a/src/Entity/Pattern.php
  +++ b/src/Entity/Pattern.php
  @@ -22,7 +22,7 @@ use Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItemInstantiat
    *    label_singular = @Translation("pattern"),
    *    label_plural = @Translation("patterns"),
    *    label_collection = @Translation("Patterns"),
  - *    admin_permission = "access administration pages",
  + *    admin_permission = "administer patterns",
    *    entity_keys = {
    *      "id" = "id",
    *      "label" = "label",
  

• … but then the bulk of the remaining work is in updating the routes/controllers to respect that. Plus all the testCannotDeleteWhenThereAreDependents() work also remains relevant.

@lauriii to confirm

1. The permissions listed in #3452581: [META] XB Permissions do not include Administer Components, so I added that in #3452581: [META] XB Permissions.

   Impact: there is one more permission, which would be required to access the /admin/structure/component/status UI. Please either confirm you agree, or propose an alternative.

2. • Use a tiny subclass of default \Drupal\Core\Entity\EntityAccessControlHandler for the Component and Pattern config entity types, aka all XB config entity types that provide the basic building blocks for the XB UI. These must respect the admin_permission, except that viewing must always be allowed.

     For now, simply require the user to be authenticated (i.e. vary by the user.roles:authenticated) cache context).

   Impact: without this relaxation, the administer patterns and administer components permissions would be required for a regular Content Creator to be able to use XB at all.

   In fact, it could be implemented more simply still:

   •   public function access(EntityInterface $entity, $operation, ?AccountInterface $account = NULL, $return_as_object = FALSE) {
         $account = $this->prepareUser($account);
         if ($operation === 'view') {
           $result = AccessResult::allowed();
           return $return_as_object ? $result : $result->isAllowed();
         }
         return parent::access($entity, $operation, $account, $return_as_object);
     

   • Add the
     

         _user_is_logged_in: 'TRUE'
     

     route requirement to the experience_builder.api.config.list route.

Could we use administer themes permission for this?

Sure, that's fine.

@lauriii to confirm

1. It might make sense to even move this under "Appearance" but that's off-topic for this issue 🤔

   That's a trivial move: instead of being listed at /admin/structure, add 2 new tabs at /admin/appearance. It'd also allow us to clean up the long obsolete Page Builder Components UI label.

   Impact:

   Before

   

   After

   

   (Patch that implements this attached.)

2. Seems fine to allow viewing these to be tied to whatever parent context (e.g., access content).

   There is no parent context when using the XB UI and fetching the list of Component and Pattern config entities. The XB UI just fetches them regardless of which thing you're editing, no matter if it's a Page or Node content entity, or a ContentTypeTemplate config entity (once #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model is implemented).

   Impact: Either:

   1. context-free: it is acceptable that Component and Pattern config entities are retrieved context-free from the server to populate the choices available in the client.
   2. context-aware: the client
      1. first retrieves the edited XB component tree
      2. after it got a (2xx) response, it passes this context to the server again when fetching the Component and Pattern config entities
      3. … while it'd return the exact same API response regardless of the context, it'd perform different access checking

      This means that for now, it's oddly using different access checks to fetch the same data, but in the future it'd make it trivial to add context-aware (specifically, component tree host-aware) filtering logic.

      Note too that this would cause worse front-end performance.

   IMHO the context-free choice is the simplest, sanest choice for now.

#20: ACK, context-free it is 👍 That means you've +1'd the _user_is_logged_in proposal.

Could we add a new tab called "Components" and then add the two tabs under it?

Sure! :)

That works too :)

Managed to do a partial review, will review the rest later today :)

Looking great! 😄 I think we're in the final stretch here now :)

Tantalizingly close! 😄

Looking great now! 😄 But … all PHPUnit CI jobs were failing. It seems to have been a CI infra issue with the DB. Rerunning those CI jobs, feel free to merge if they pass :)

(Please merge using the d.o "merge" button, not the GitLab one. That ensures the proper commit message is applied.)

This unblocked #3516648: Update `ExperienceBuilderController` to pass current user's XB high-level permissions to the client!