- Issue created by @mglaman
- ๐ซ๐ฎFinland lauriii Finland
Do we need a generic "Use Experience Builder" permission which we'd use for the XB route and would move the components behind it? Otherwise it is pretty complicated to figure out when users should have access to the components.
- ๐ช๐ธSpain isholgueras
In the ticket ๐ Provide granular permissions for pages Active there were these 3 permissions (create, edit and delete).
As I didn't have any option for overview or a general "administer", I could only use "edit xb_page" as the permission to use whenever a editor wants to edit it.
Maybe I was mistaken or maybe it's not clear.
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
โ Config entity access control handler
The
Component
config entity type uses\Drupal\experience_builder\EntityHandlers\ContentCreatorVisibleXbConfigEntityAccessControlHandler
, which has:// We always allow viewing these entities. 'view' => AccessResult::allowed(),
(since ๐ Add access control for "code components" and "asset libraries", special case: instantiated code components must be accessible to *all* Active )
โ Route definition's requirements
The culprit is
experience_builder.api.config.list: path: '/xb/api/config/{xb_config_entity_type_id}' โฆ requirements: _permission: 'administer themes'
Choices
Either:
- Remove the route requirements altogether and rely on its individually checking access:
// As config entities do not use sql-storage, we need explicit access check // per https://www.drupal.org/node/3201242. $config_entities = array_filter($config_entities, fn(XbHttpApiEligibleConfigEntityInterface $config_entity) => $config_entity->access('view'));
- Or defining a
collection_permission
, which is why theBlock
config entity for example has
collection_permission: 'access block library',
I don't think we want any net new permissions though. ๐
- Remove the route requirements altogether and rely on its individually checking access:
- ๐ง๐ชBelgium wim leers Ghent ๐ง๐ช๐ช๐บ
IMHO the simplest approach is this:
diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index fde9ce0a3..cb10b697d 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -130,7 +130,6 @@ experience_builder.api.config.list: defaults: _controller: 'Drupal\experience_builder\Controller\ApiConfigControllers::list' requirements: - _permission: 'administer themes' # Any internal HTTP API-eligible XB config entity type. _xb_http_eligible_config_entity: 'TRUE' # But deter users from trying to access other config entities via this route: this ensures 404 responses rather than
Because
# The UI client can fetch all the components the current user has access too, regardless of which kind of XB-enabled # entity you're editing. _user_is_logged_in: 'TRUE'
already ensures no anonymous users can access it. That was added by ๐ Add access control for "code components" and "asset libraries", special case: instantiated code components must be accessible to *all* Active and it seems a simple oversight that that the
_permission
part was not removed? - First commit to issue fork.
- Merge request !1038Issue #3518836: experience_builder.api.config.list route should not require... โ (Open) created by meghasharma
- ๐ฎ๐ณIndia meghasharma
Iโve created a merge request that removes the `_permission: 'administer themes'` requirement from the `experience_builder.api.config.list` route, based on Wim Leersโ suggestion in comment #10.
This allows content editors with the proper `xb_page` permissions to view and place components without needing the `administer themes` permission.
The route is still protected by `_user_is_logged_in: 'TRUE'` and per-entity `access('view')` checks, so there are no security risks introduced.Tested and confirmed:
Before fix: Component list was not visible for content editors.
After fix: Component list is visible and components can be placed.Attaching screenshots of before and after the fix for reference.
- ๐ฎ๐ณIndia amangrover90
Only thing I'm thinking about is now that we've removed this permission. The authors(who don't have administer code components) cannot see any code components. They can just see the components. If the code components are added to the components, it'll start throwing 403 for such users.
So maybe we should update JavascriptComponent to also use ContentCreatorVisibleXbConfigEntityAccessControlHandler access handler.
- ๐บ๐ธUnited States mglaman WI, USA
+1 use ContentCreatorVisibleXbConfigEntityAccessControlHandler, that organically happened in ๐ ContentCreatorVisibleXbConfigEntityAccessControlHandler's `view` access must refuse access to disabled config entities Active
- ๐ช๐ธSpain penyaskito Seville ๐, Spain ๐ช๐ธ, UTC+2 ๐ช๐บ
I think this should be PPed on ๐ ContentCreatorVisibleXbConfigEntityAccessControlHandler's `view` access must refuse access to disabled config entities Active (which should be ready to land if RTBCed).
Re #9: A `collection_permission` might be unavoidable. See #3525572-11: Implement OAuth2 authentication for API endpoints related to code components โ where Bรกlint has issues with oauth, and we might want to remove the
_user_is_logged_in
- ๐ณ๐ฑNetherlands balintbrews Amsterdam, NL
Thanks, @penyaskito, for highlighting this issue for me. After our discussions, I'm now unblocked with regards to the
_user_is_logged_in
access checker and other permissions โ see my summary here: #3525572-13: Implement OAuth2 authentication for API endpoints related to code components โ .