From 17c3b59a49bec0d0bf1520bc009ea29482a6cca9 Mon Sep 17 00:00:00 2001 From: Sybille Peters Date: Thu, 30 May 2024 16:15:34 +0200 Subject: [PATCH] [BUGFIX] Better handling of type/authMode with permission checks - always write the type to element_type, if available - only use $GLOBALS['TYPO3_CONF_VARS']['BE']['explicitADmode'] for tt_content, otherwise use $GLOBALS['TCA'][$table]['columns'][$type]['config']['authMode'] - if no autMode is set, do not add any checks for it to EditableRestriction This should fix a known problem, that broken links in tx_domain_model_news records were not displayed for non-admin editors Resolves: #369 --- Classes/LinkAnalyzer.php | 3 +- Classes/Repository/EditableRestriction.php | 86 ++++++++++++++++++++-- 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/Classes/LinkAnalyzer.php b/Classes/LinkAnalyzer.php index 0136b1c99..bfec2a9da 100644 --- a/Classes/LinkAnalyzer.php +++ b/Classes/LinkAnalyzer.php @@ -343,7 +343,8 @@ protected function checkLinks(array $links, array $linkTypes, int $mode = 0): vo $record['flexform_field'] = $entryValue['flexformField'] ?? ''; $record['flexform_field_label'] = $entryValue['flexformFieldLabel'] ?? ''; $typeField = $GLOBALS['TCA'][$table]['ctrl']['type'] ?? false; - if ($entryValue['row'][$typeField] ?? false) { + // type might be '0', e.g. for tx_news_domain_model_news, so use isset instead of if with ?? + if (isset($entryValue['row'][$typeField])) { $record['element_type'] = $entryValue['row'][$typeField]; } $record['exclude_link_targets_pid'] = $this->configuration->getExcludeLinkTargetStoragePid(); diff --git a/Classes/Repository/EditableRestriction.php b/Classes/Repository/EditableRestriction.php index a6f3bd1d6..76d260f48 100644 --- a/Classes/Repository/EditableRestriction.php +++ b/Classes/Repository/EditableRestriction.php @@ -44,11 +44,22 @@ class EditableRestriction implements QueryRestrictionInterface protected $allowedLanguages = []; /** - * Explicit allow fields + * Explicitly allow these types based on authMode (and explicitADmode for tt_content) + * + * Example: + * + * [ + * 'tt_content' => [ + * 'CType' => [ + * 'textmedia', + * ... + * ] + * ] + * ] * * @var array>> */ - protected $explicitAllowFields = []; + protected $allowByFieldBasedOnAuthMode = []; /** * @var QueryBuilder @@ -65,9 +76,24 @@ public function __construct(array $searchFields, QueryBuilder $queryBuilder) $this->allowedFields = $this->getAllowedFieldsForCurrentUser($searchFields); $this->allowedLanguages = $this->getAllowedLanguagesForCurrentUser(); foreach ($searchFields as $table => $fields) { + /** @todo We should look at behaviour of other tables besides tt_content. For tt_content.CType, if the + * value has not been activated for the BE group (or was explicitly denied), it is not possible to edit + * the record at all. However, if we try this with a non-tt_content record, the behaviour is different. + * Needs further research. + */ if ($table !== 'pages' && ($GLOBALS['TCA'][$table]['ctrl']['type'] ?? false)) { $type = $GLOBALS['TCA'][$table]['ctrl']['type']; - $this->explicitAllowFields[$table][$type] = $this->getExplicitAllowFieldsForCurrentUser($table, $type); + + // the value in the type field can depend on the value of a related record. We do not handle this + // at the moment, we only ignore these kind of fields. + if (strpos($type, ':') !== false) { + continue; + } + + $values = $this->getAllowByFieldBasedOnAuthModeForCurrentUser($table, $type); + if ($values !== null) { + $this->allowByFieldBasedOnAuthMode[$table][$type] = $values; + } } } $this->queryBuilder = $queryBuilder; @@ -94,17 +120,30 @@ protected function getAllowedLanguagesForCurrentUser(): array } /** + * Based on authMode / explicitADmode. + * + * If a table contains a 'type' field, it is possible to explicitly allow or deny certain types. The behaviour + * depends on the value of $GLOBALS['TCA'][$table]['columns'][$field]['config']['authMode'] (e.g. explicitAllow). + * For tt_content tables, the behaviour depends on the value of $GLOBALS['TYPO3_CONF_VARS']['BE']['explicitADmode']. + * + * * @param string $table * @param string $field - * @return array + * @return array|null If null is passed, no auth checking for this $table / $field */ - protected function getExplicitAllowFieldsForCurrentUser(string $table, string $field): array + protected function getAllowByFieldBasedOnAuthModeForCurrentUser(string $table, string $field): ?array { $allowDenyOptions = []; $fieldConfig = $GLOBALS['TCA'][$table]['columns'][$field]['config'] ?? []; if (!$fieldConfig) { - return []; + return null; } + + $authMode = $this->getAuthMode($table, $field); + if (!$authMode) { + return null; + } + // Check for items if ($fieldConfig['type'] === 'select' && is_array($fieldConfig['items'] ?? false)) { foreach ($fieldConfig['items'] as $iVal) { @@ -120,6 +159,35 @@ protected function getExplicitAllowFieldsForCurrentUser(string $table, string $f return $allowDenyOptions; } + /** + * @todo in v12, this changes + * @return string Return empty string, if no authMode + */ + protected function getAuthMode(string $table, string $type): string + { + if ($type === 'CType') { + /** + * from documentation about explicitADmode: + * "since v12: The handling of $GLOBALS['TYPO3_CONF_VARS']['BE']['explicitADmode'] has been changed and is + * now set using explicitAllow. Extensions should not assume this global array key is set anymore as of + * TYPO3 Core v12. Extensions that need to stay compatible with v11 and v12 should instead use: + * $GLOBALS['TYPO3_CONF_VARS']['BE']['explicitADmode'] ?? 'explicitAllow'." + */ + return $GLOBALS['TYPO3_CONF_VARS']['BE']['explicitADmode'] ?? 'explicitAllow'; + } + + $authMode = $GLOBALS['TCA'][$table]['columns'][$type]['config']['authMode'] ?? ''; + if ($authMode && $authMode != 'explicitAllow') { + /** since TYPO3 v12, only explicitAllow is supported + * from documentation: + * "The only valid value for TCA config option authMode is now explicitAllow. The values explicitDeny and + * individual are obsolete and no longer evaluated." + */ + $authMode = ''; + } + return $authMode; + } + /** * Get allowed table / fieldnames for current backend user. * Only consider table / fields in $searchFields @@ -209,7 +277,7 @@ public function buildExpression(array $queriedTables, ExpressionBuilder $express $constraints[] = $expressionBuilder->isNull(self::TABLE . '.table_name'); } - foreach ($this->explicitAllowFields as $table => $field) { + foreach ($this->allowByFieldBasedOnAuthMode as $table => $field) { $additionalWhere = []; $additionalWhere[] = $expressionBuilder->and( $expressionBuilder->eq( @@ -224,6 +292,10 @@ public function buildExpression(array $queriedTables, ExpressionBuilder $express ) ) ); + $additionalWhere[] = $expressionBuilder->eq( + self::TABLE . '.element_type', + $this->queryBuilder->createNamedParameter('') + ); $additionalWhere[] = $expressionBuilder->neq( self::TABLE . '.table_name', $this->queryBuilder->createNamedParameter($table)