Issue #0: Improve access to example controller (f3a82430) · Commits · project / xray_audit

src/Controller/XrayAuditDisplayModeExampleController.php

+34 −20

Changes for src/Controller/XrayAuditDisplayModeExampleController.php: 34 added lines, 20 removed lines.
Original line number	Diff line number	Diff line
		@@ -5,6 +5,7 @@ namespace Drupal\xray_audit\Controller;
		use Drupal\Core\Controller\ControllerBase;
		use Drupal\Core\Database\Connection;
		use Drupal\Core\Database\StatementInterface;
		use Drupal\Core\Entity\EntityInterface;
		use Drupal\Core\Url;
		use Symfony\Component\DependencyInjection\ContainerInterface;
		use Symfony\Component\HttpFoundation\RedirectResponse;
		@@ -18,6 +19,11 @@ use Symfony\Component\HttpFoundation\RedirectResponse;
		*/
		final class XrayAuditDisplayModeExampleController extends ControllerBase {

		/**
		* Maximum candidates checked before giving up on finding a viewable example.
		*/
		protected const MAX_EXAMPLE_CANDIDATES = 10;

		/**
		* Database connection.
		*
		@@ -43,24 +49,23 @@ final class XrayAuditDisplayModeExampleController extends ControllerBase {
		}

		/**
		* Display a entity in a given view mode.
		* Display an entity in a given view mode.
		*
		* @param string $entity_type
		* The view mode.
		* @param string $entity_id
		* The entity type.
		* Entity type validation, loading and view access are enforced by the route.
		*
		* @param \Drupal\Core\Entity\EntityInterface $entity
		* The entity to display.
		* @param string $view_mode
		* The entity id.
		* The view mode.
		*
		* @return array
		* Render array of the entity.
		*/
		public function displayEntity(string $entity_type, string $entity_id, string $view_mode) {
		$response = [];
		if ($entity = $this->entityTypeManager()->getStorage($entity_type)->load($entity_id)) {
		$response = $this->entityTypeManager()
		->getViewBuilder($entity_type)
		public function displayEntity(EntityInterface $entity, string $view_mode) {
		return $this->entityTypeManager()
		->getViewBuilder($entity->getEntityTypeId())
		->view($entity, $view_mode);
		}
		return $response;
		}

		/**
		* Display an entity in a given view mode.
		@@ -95,7 +100,7 @@ final class XrayAuditDisplayModeExampleController extends ControllerBase {
		$build['#attributes']['class'] = ['xray-audit--iframe'];
		$build['#iframe_url'] = Url::fromRoute('xray_audit.display_mode_example', [
		'entity_type' => $entity_type,
		'entity_id' => $id,
		'entity' => $id,
		'view_mode' => $view_mode,
		]);
		}
		@@ -111,7 +116,7 @@ final class XrayAuditDisplayModeExampleController extends ControllerBase {
		* Entity type name.
		*/
		protected function getExampleId(string $entity_type, string $entity_bundle) {
		$query = $this->entityTypeManager()->getStorage($entity_type)->getQuery();
		$query = $this->entityTypeManager()->getStorage($entity_type)->getQuery()->accessCheck(TRUE);
		$bundle_column = 'type';
		$sort_column = 'changed';

		@@ -126,21 +131,30 @@ final class XrayAuditDisplayModeExampleController extends ControllerBase {
		$sort_column = 'created';
		}

		$query->accessCheck(TRUE)
		->condition($bundle_column, $entity_bundle);
		$query->condition($bundle_column, $entity_bundle);

		if ($entity_type === 'paragraph') {
		$query->condition('id', $this->getParagraphsUsedOnCurrentRevisions(), 'IN');
		}

		$ids = $query->range(0, 100)
		->accessCheck(TRUE)
		->sort($sort_column, 'DESC')
		->execute();

		if (count($ids) > 0) {
		return $ids[array_rand($ids)];
		// Entity query access does not always match the entity access handler:
		// paragraphs delegate view access to their parent, which the query cannot
		// express. Returning an id the user cannot view would render a 403 page
		// inside the example iframe.
		$storage = $this->entityTypeManager()->getStorage($entity_type);
		$candidates = array_values($ids);
		shuffle($candidates);
		foreach (array_slice($candidates, 0, static::MAX_EXAMPLE_CANDIDATES) as $id) {
		$entity = $storage->load($id);
		if ($entity instanceof EntityInterface && $entity->access('view')) {
		return $id;
		}
		}

		return FALSE;
		}

tests/src/Functional/Controller/XrayAuditDisplayModeExampleControllerTest.php

0 → 100644

+144 −0

Changes for tests/src/Functional/Controller/XrayAuditDisplayModeExampleControllerTest.php: 144 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		<?php

		namespace Drupal\Tests\xray_audit\Functional\Controller;

		use Drupal\Tests\BrowserTestBase;

		/**
		* Tests the XrayAuditDisplayModeExampleController security fix.
		*
		* @group xray_audit
		* @group critical
		*/
		class XrayAuditDisplayModeExampleControllerTest extends BrowserTestBase {

		/**
		* {@inheritdoc}
		*/
		protected $defaultTheme = 'stark';

		/**
		* {@inheritdoc}
		*/
		protected static $modules = [
		'node',
		'field',
		'text',
		'filter',
		'user',
		'system',
		'xray_audit',
		];

		/**
		* Marker unique to the unpublished node's body, used to detect disclosure.
		*/
		protected const UNPUBLISHED_MARKER = 'SECRET-XRAY-UNPUB-9001';

		/**
		* Marker unique to the published node's body.
		*/
		protected const PUBLISHED_MARKER = 'PUBLIC-XRAY-PUB-9002';

		/**
		* Unpublished article, must never leak to a user without content access.
		*
		* @var \Drupal\node\NodeInterface
		*/
		protected $unpublishedNode;

		/**
		* Published article, used as the control case.
		*
		* @var \Drupal\node\NodeInterface
		*/
		protected $publishedNode;

		/**
		* {@inheritdoc}
		*/
		protected function setUp(): void {
		parent::setUp();

		$this->drupalCreateContentType(['type' => 'article', 'name' => 'Article']);

		$this->unpublishedNode = $this->drupalCreateNode([
		'type' => 'article',
		'title' => 'Xray unpublished example',
		'body' => [['value' => static::UNPUBLISHED_MARKER]],
		'status' => 0,
		]);

		$this->publishedNode = $this->drupalCreateNode([
		'type' => 'article',
		'title' => 'Xray published example',
		'body' => [['value' => static::PUBLISHED_MARKER]],
		'status' => 1,
		]);
		}

		/**
		* Anonymous users must not reach the example route at all.
		*
		* Regression for the advisory: the route previously required only
		* "access content", letting anonymous users render any entity by id.
		*/
		public function testAnonymousCannotDiscloseUnpublishedNode() {
		$this->drupalGet('/xray-audit/node/' . $this->unpublishedNode->id() . '/full/example');
		$this->assertSession()->statusCodeEquals(403);
		$this->assertSession()->responseNotContains(static::UNPUBLISHED_MARKER);
		}

		/**
		* The route now gates on "xray_audit access", which anonymous never has.
		*/
		public function testAnonymousIsDeniedRegardlessOfNodeStatus() {
		$this->drupalGet('/xray-audit/node/' . $this->publishedNode->id() . '/full/example');
		$this->assertSession()->statusCodeEquals(403);

		$this->drupalGet('/xray-audit/node/article/full/popup');
		$this->assertSession()->statusCodeEquals(403);
		}

		/**
		* A user with the module permission but not "bypass node access".
		*
		* Entity-level access is still required beyond the route permission.
		*/
		public function testAuditorWithoutBypassRespectsEntityAccess() {
		$auditor = $this->drupalCreateUser(['xray_audit access', 'access content']);
		$this->drupalLogin($auditor);

		$this->drupalGet('/xray-audit/node/' . $this->unpublishedNode->id() . '/full/example');
		$this->assertSession()->statusCodeEquals(403);
		$this->assertSession()->responseNotContains(static::UNPUBLISHED_MARKER);

		$this->drupalGet('/xray-audit/node/' . $this->publishedNode->id() . '/full/example');
		$this->assertSession()->statusCodeEquals(200);
		$this->assertSession()->responseContains(static::PUBLISHED_MARKER);
		}

		/**
		* A user with "bypass node access" sees entities the normal handler hides.
		*/
		public function testAuditorWithBypassCanViewUnpublishedNode() {
		$auditor = $this->drupalCreateUser(['xray_audit access', 'access content', 'bypass node access']);
		$this->drupalLogin($auditor);

		$this->drupalGet('/xray-audit/node/' . $this->unpublishedNode->id() . '/full/example');
		$this->assertSession()->statusCodeEquals(200);
		$this->assertSession()->responseContains(static::UNPUBLISHED_MARKER);
		}

		/**
		* A missing entity id yields a 404.
		*/
		public function testMissingEntityYields404() {
		$auditor = $this->drupalCreateUser(['xray_audit access', 'access content', 'bypass node access']);
		$this->drupalLogin($auditor);

		$this->drupalGet('/xray-audit/node/999999/full/example');
		$this->assertSession()->statusCodeEquals(404);
		}

		}

xray_audit.module

+4 −0

Changes for xray_audit.module: 4 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -59,11 +59,15 @@ function xray_audit_preprocess_html__xray_audit(&$variables) {
		$variables['page_bottom']['#access'] = FALSE;
		}

		// Error responses on these paths reach this hook before any html class is
		// set on the page.
		if (is_array($variables['attributes']['class'] ?? NULL)) {
		foreach ($variables['attributes']['class'] as &$item) {
		if (str_contains($item, 'toolbar-')) {
		$item = '';
		}
		}
		}

		$variables['#attached']['library'][] = 'xray_audit/xray_audit_examples_preview';
		}

xray_audit.routing.yml

+6 −5

Changes for xray_audit.routing.yml: 6 added lines, 5 removed lines.
Original line number	Diff line number	Diff line
		@@ -28,17 +28,18 @@ xray_audit.batch_process:
		type: batch_process

		xray_audit.display_mode_example:
		path: '/xray-audit/{entity_type}/{entity_id}/{view_mode}/example'
		path: '/xray-audit/{entity_type}/{entity}/{view_mode}/example'
		defaults:
		_controller: '\Drupal\xray_audit\Controller\XrayAuditDisplayModeExampleController::displayEntity'
		requirements:
		_permission: 'access content'
		_permission: 'xray_audit access'
		_entity_access: 'entity.view'
		options:
		parameters:
		entity_type:
		type: string
		entity_id:
		type: string
		entity:
		type: entity:{entity_type}
		view_mode:
		type: string

		@@ -47,7 +48,7 @@ xray_audit.example_popup:
		defaults:
		_controller: '\Drupal\xray_audit\Controller\XrayAuditDisplayModeExampleController::displayEntityPopupContent'
		requirements:
		_permission: 'access content'
		_permission: 'xray_audit access'
		options:
		parameters:
		entity_type: