Problem/Motivation

A security review identified multiple locations throughout the CRM module where accessCheck(FALSE) is used in entity queries. This disables Drupal's entity-level access control system, which could expose data to users who shouldn't have access to it.

Security Impact: While most affected routes are protected by administrative permissions (e.g., "administer crm"), this creates a defense-in-depth issue. Entity queries should respect entity-level access controls in addition to route-level permissions to prevent information disclosure if:

  • Additional access restrictions are added via hook_entity_access()
  • Custom permissions are configured on entities
  • The code is reused in contexts with different route permissions

Current behavior: Users with route-level access can see counts and data for ALL entities, even those they shouldn't have entity-level access to view.

Affected locations:

  • src/Controller/RelationshipController.php (lines 110, 134)
  • src/Access/ContactAccessControlHandler.php (line 145)
  • src/ListBuilder/ContactListBuilder.php (line 48)
  • src/ListBuilder/ContactMethodListBuilder.php (lines 79, 133)
  • src/ListBuilder/RelationshipListBuilder.php (line 48)
  • src/Service/RelationshipStatisticsService.php (lines 152, 216)
  • src/Plugin/Validation/Constraint/RelationshipLimitConstraintValidator.php (line 128)
  • src/Form/RelationshipTypeForm.php (line 367)
  • src/Hook/ContactHooks.php (line 185)

Steps to reproduce

  1. Install the CRM module
  2. Create a custom access handler or use hook_entity_access() to restrict certain contacts
  3. Create a user with "administer crm" permission but restricted entity-level access
  4. Log in as that user and navigate to /admin/crm/contacts
  5. Observe that the entity counts include contacts the user shouldn't be able to view
  6. Navigate to relationship statistics pages
  7. Observe statistics include relationships for contacts the user cannot access

Expected result: Entity queries should only return entities the current user has permission to view, respecting both route-level AND entity-level access controls.

Actual result: Entity queries with accessCheck(FALSE) return all entities regardless of entity-level access restrictions, potentially exposing sensitive data.

Proposed resolution

Review all instances of accessCheck(FALSE) and categorize them into three groups, handling each appropriately:

Implementation approach:

  1. Controllers and List Builders: Change to accessCheck(TRUE) to respect entity-level permissions. Example:
// Before:
$query = $this->entityTypeManager
  ->getStorage('crm_contact')
  ->getQuery()
  ->accessCheck(FALSE);

// After:
$query = $this->entityTypeManager
  ->getStorage('crm_contact')
  ->getQuery()
  ->accessCheck(TRUE);
  1. Internal Services: Keep accessCheck(FALSE) for system-level statistics where access has already been verified at the route level. Add inline comments:
// Deliberately bypassing access checks because this is a system-level
// calculation. Route-level access is enforced by the 'administer crm'
// permission on the calling controller.
$query = $this->entityTypeManager
  ->getStorage('crm_relationship')
  ->getQuery()
  ->accessCheck(FALSE);
  1. Validation Constraints: Keep accessCheck(FALSE) as validators run in a system context. Add comments explaining this.
  2. Access Handlers: Review case-by-case, as these are part of the access determination logic itself.

Backward compatibility: This change will reduce the entity counts shown to users with limited permissions. This is the correct behavior and shouldn't break any functionality, but should be noted in the release notes.

Remaining tasks

  • Review and update src/Controller/RelationshipController.php
  • Review and update src/ListBuilder/ContactListBuilder.php
  • Review and update src/ListBuilder/ContactMethodListBuilder.php
  • Review and update src/ListBuilder/RelationshipListBuilder.php
  • Review src/Form/RelationshipTypeForm.php
  • Review src/Hook/ContactHooks.php
  • Review src/Access/ContactAccessControlHandler.php
  • Add explanatory comments to src/Service/RelationshipStatisticsService.php
  • Add explanatory comments to src/Plugin/Validation/Constraint/RelationshipLimitConstraintValidator.php
  • Write automated tests verifying access control enforcement with restricted users
  • Write tests for legitimate accessCheck(FALSE) usage

User interface changes

No direct UI changes. List builders may show different entity counts for users with limited permissions after the fix.

API changes

None. This is an internal security fix that doesn't change public APIs.

Data model changes

None.

Issue fork crm-3571870

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

bluegeek9 created an issue. See original summary.

bluegeek9’s picture

//www.flaticon.com/free-icons/thank-you Thank you for your contribution! Your continued support makes this project sustainable.
There are multiple ways to show appreciation for the work contributed to this project including:
  • Triage issues and adding more context to existing issues.
  • Flagging CRM as a favorite on the project page to help others discover it and show your support.
  • Review the Developer Docs for accuracy and clarity.
bluegeek9’s picture

Assigned: bluegeek9 » Unassigned
Status: Active » 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.

Status: Fixed » Closed (fixed)

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