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

  1. Install Commerce 3.3.7, 3.3.8 or current 3.x-dev.
  2. Create a product type that allows multiple variations, and a product with at least one variation.
  3. Create a role granted access commerce_product overview (plus view commerce_product), and not granted administer commerce_product nor any manage <type> commerce_product_variation permission. Per ProductVariationCollectionAccessCheck, access commerce_product overview alone is enough to view the variations listing.
  4. 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

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

svendecabooter created an issue. See original summary.

jsacksick made their first commit to this issue’s fork.

jsacksick’s picture

Priority: Major » Normal

jsacksick’s picture

Status: Active » Needs review

Good catch, opened an MR.

  • jsacksick committed acad71b9 on 3.x
    fix: #3618077 Regression: Duplicate operation always shown on product...
    
jsacksick’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.