Problem/Motivation
Since #3611317: Address PHPSTAN issues (commit 917ad7f8, first released in 3.3.7), the Duplicate operation link on the product variations overview (/product/{commerce_product}/variations) is rendered for every user who can reach that page, regardless of whether they have access to duplicate the variation.
That issue refactored ProductVariationListBuilder::getDefaultOperations() to accept and populate cacheability metadata. In doing so, the access check was changed from a boolean to an AccessResultInterface object, but the if () condition was left as-is:
// Before 917ad7f8: if ($entity->access('create') && $entity->hasLinkTemplate('duplicate-form')) { // After 917ad7f8 — modules/product/src/ProductVariationListBuilder.php:296-300 $access = $entity->access('create', NULL, TRUE); if ($cacheability instanceof CacheableMetadata) { $cacheability->addCacheableDependency($access); } if ($access && $entity->hasLinkTemplate('duplicate-form')) {
AccessibleInterface::access() is documented as ($return_as_object is true ? \Drupal\Core\Access\AccessResultInterface : bool), so if ($access) evaluates an object and is unconditionally TRUE. The access result is now used only to register cacheability - it no longer influences the visibility decision at all.
Notably, the very same commit applied this refactor correctly everywhere else. OrderListBuilder::getDefaultOperations() calls ->isAllowed() on all four of its access results (lines 95, 103, 111, 119). ProductVariationListBuilder is the only place where ->isAllowed() was omitted.
Scope of impact: this is not a privilege escalation. Following the link is still correctly blocked by route access on the duplicate-form link template, so no unauthorised duplication is possible. The impact is a misleading UI: users are offered an operation they cannot perform and get an "Access denied" on click. It also means the duplicate column is populated for roles that were deliberately granted read-only overview access.
Steps to reproduce
- Install Commerce 3.3.7, 3.3.8 or current 3.x-dev.
- Create a product type that allows multiple variations, and a product with at least one variation.
- Create a role granted
access commerce_product overview(plusview commerce_product), and not grantedadminister commerce_productnor anymanage <type> commerce_product_variationpermission. PerProductVariationCollectionAccessCheck,access commerce_product overviewalone is enough to view the variations listing. - As a user with that role, visit
/product/1/variations.
Expected: no Duplicate operation link, since the user has no create access.
Actual: the Duplicate link is shown for every variation row. Clicking it results in "Access denied".
On 3.3.6 and earlier, the link is correctly hidden.
Proposed resolution
Evaluate the access result instead of the object, matching the pattern already used in OrderListBuilder:
if ($access->isAllowed() && $entity->hasLinkTemplate('duplicate-form')) {Remaining tasks
- Implement the proposed resolution
- Add test coverage asserting the Duplicate operation is absent for a user without create access. The absence of such a test is why this regression shipped unnoticed.
User interface changes
The Duplicate operation link is again hidden from users who lack access to duplicate a variation, restoring pre-3.3.7 behaviour.
API changes
None.
Data model changes
None.
Issue fork commerce-3618077
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
Comment #3
jsacksick commentedComment #5
jsacksick commentedGood catch, opened an MR.
Comment #7
jsacksick commented