Merge pull request #255 from copenhagentruckwash/fix-system-search-authorization-bypass
Enforce department scoping in system search for generic entities
This commit is contained in:
@@ -54,6 +54,7 @@ class system_search_service
|
||||
$allowedTypes = $this->normalizeTypes((array)($options['allowed_types'] ?? []));
|
||||
$ownOnlyTypes = $this->normalizeTypes((array)($options['own_only_types'] ?? []));
|
||||
$ownCustomerNumber = isset($options['own_customer_number']) ? (int)$options['own_customer_number'] : null;
|
||||
$allowedDepartmentIds = array_values(array_unique(array_map('intval', (array)($options['allowed_department_ids'] ?? []))));
|
||||
$permissionsCatalogAll = (array)($options['permissions_catalog_all'] ?? []);
|
||||
$permissionsCatalogOwn = (array)($options['permissions_catalog_own'] ?? []);
|
||||
$moduleConfigVisibility = (array)($options['module_config_visibility'] ?? []);
|
||||
@@ -107,6 +108,7 @@ class system_search_service
|
||||
'offset' => $offset,
|
||||
'own' => $ownCustomerNumber,
|
||||
'own_only' => $ownOnlyTypes,
|
||||
'dept' => $allowedDepartmentIds,
|
||||
'assoc' => $includeAssociations,
|
||||
'dbg' => $debugIntent,
|
||||
'ctx' => $this->permissionContextFingerprint($permissionsCatalogAll, $permissionsCatalogOwn, $moduleConfigVisibility),
|
||||
@@ -130,7 +132,8 @@ class system_search_service
|
||||
$ownCustomerNumber,
|
||||
$permissionsCatalogAll,
|
||||
$permissionsCatalogOwn,
|
||||
$moduleConfigVisibility
|
||||
$moduleConfigVisibility,
|
||||
$allowedDepartmentIds
|
||||
);
|
||||
|
||||
$intentAssociationHint = false;
|
||||
@@ -180,7 +183,8 @@ class system_search_service
|
||||
$ownCustomerNumber,
|
||||
$permissionsCatalogAll,
|
||||
$permissionsCatalogOwn,
|
||||
$moduleConfigVisibility
|
||||
$moduleConfigVisibility,
|
||||
$allowedDepartmentIds
|
||||
);
|
||||
} else {
|
||||
$intentMeta['status'] = 'fallback';
|
||||
@@ -207,6 +211,24 @@ class system_search_service
|
||||
$activeTypes,
|
||||
$this->associationEntityTypes()
|
||||
));
|
||||
<<<<<<< HEAD
|
||||
foreach ($customerNumbers as $customerNumber) {
|
||||
$associated = $this->executeLexicalSearch(
|
||||
$associationTypes,
|
||||
[(string)$customerNumber],
|
||||
[],
|
||||
$ownOnlyTypes,
|
||||
$ownCustomerNumber,
|
||||
$permissionsCatalogAll,
|
||||
$permissionsCatalogOwn,
|
||||
$moduleConfigVisibility,
|
||||
$allowedDepartmentIds,
|
||||
[$customerNumber]
|
||||
);
|
||||
foreach ($associated as &$item) {
|
||||
if (!isset($item['association_reason'])) {
|
||||
$item['association_reason'] = 'customer:' . $customerNumber;
|
||||
=======
|
||||
$associationTypes = array_values(array_diff($associationTypes, $ownOnlyTypes));
|
||||
if (!empty($associationTypes)) {
|
||||
foreach ($customerNumbers as $customerNumber) {
|
||||
@@ -226,6 +248,7 @@ class system_search_service
|
||||
$item['association_reason'] = 'customer:' . $customerNumber;
|
||||
}
|
||||
$item['score'] = max((int)$item['score'], 35);
|
||||
>>>>>>> origin/master
|
||||
}
|
||||
$initialResults = $this->mergeResults($initialResults, $associated);
|
||||
}
|
||||
@@ -307,6 +330,7 @@ class system_search_service
|
||||
* @param array<string, string> $permissionsCatalogAll
|
||||
* @param array<int, string> $permissionsCatalogOwn
|
||||
* @param array<string, bool> $moduleConfigVisibility
|
||||
* @param array<int, int> $allowedDepartmentIds
|
||||
* @param array<int, int> $forcedCustomerNumbers
|
||||
* @return array<int, array<string, mixed>>
|
||||
*/
|
||||
@@ -319,6 +343,7 @@ class system_search_service
|
||||
array $permissionsCatalogAll,
|
||||
array $permissionsCatalogOwn,
|
||||
array $moduleConfigVisibility,
|
||||
array $allowedDepartmentIds = [],
|
||||
array $forcedCustomerNumbers = []
|
||||
): array {
|
||||
$results = [];
|
||||
@@ -326,6 +351,20 @@ class system_search_service
|
||||
foreach ($activeTypes as $entityType) {
|
||||
$boost = (int)($entityBoost[$entityType] ?? 0);
|
||||
$ownOnly = in_array($entityType, $ownOnlyTypes, true);
|
||||
<<<<<<< HEAD
|
||||
$rows = $this->searchEntity(
|
||||
$entityType,
|
||||
$terms,
|
||||
$boost,
|
||||
$ownOnly,
|
||||
$ownCustomerNumber,
|
||||
$permissionsCatalogAll,
|
||||
$permissionsCatalogOwn,
|
||||
$moduleConfigVisibility,
|
||||
$allowedDepartmentIds,
|
||||
$forcedCustomerNumbers
|
||||
);
|
||||
=======
|
||||
if ($ownOnly && $ownCustomerNumber === null && empty($forcedCustomerNumbers)) {
|
||||
continue;
|
||||
}
|
||||
@@ -365,6 +404,7 @@ class system_search_service
|
||||
$forcedCustomerNumbers
|
||||
);
|
||||
}
|
||||
>>>>>>> origin/master
|
||||
$results = $this->mergeResults($results, $rows);
|
||||
}
|
||||
return $results;
|
||||
@@ -375,6 +415,7 @@ class system_search_service
|
||||
* @param array<string, string> $permissionsCatalogAll
|
||||
* @param array<int, string> $permissionsCatalogOwn
|
||||
* @param array<string, bool> $moduleConfigVisibility
|
||||
* @param array<int, int> $allowedDepartmentIds
|
||||
* @param array<int, int> $forcedCustomerNumbers
|
||||
* @return array<int, array<string, mixed>>
|
||||
*/
|
||||
@@ -387,6 +428,7 @@ class system_search_service
|
||||
array $permissionsCatalogAll,
|
||||
array $permissionsCatalogOwn,
|
||||
array $moduleConfigVisibility,
|
||||
array $allowedDepartmentIds,
|
||||
array $forcedCustomerNumbers
|
||||
): array {
|
||||
if ($this->isGenericEntityType($entityType)) {
|
||||
@@ -396,6 +438,7 @@ class system_search_service
|
||||
$entityBoost,
|
||||
$ownOnly,
|
||||
$ownCustomerNumber,
|
||||
$allowedDepartmentIds,
|
||||
$forcedCustomerNumbers
|
||||
);
|
||||
}
|
||||
@@ -1491,6 +1534,7 @@ class system_search_service
|
||||
|
||||
/**
|
||||
* @param array<int, string> $terms
|
||||
* @param array<int, int> $allowedDepartmentIds
|
||||
* @param array<int, int> $forcedCustomerNumbers
|
||||
* @return array<int, array<string, mixed>>
|
||||
*/
|
||||
@@ -1500,6 +1544,7 @@ class system_search_service
|
||||
int $entityBoost,
|
||||
bool $ownOnly,
|
||||
?int $ownCustomerNumber,
|
||||
array $allowedDepartmentIds = [],
|
||||
array $forcedCustomerNumbers = []
|
||||
): array {
|
||||
if (empty($terms)) {
|
||||
@@ -1604,7 +1649,12 @@ class system_search_service
|
||||
$terms,
|
||||
$customerNumbers,
|
||||
$customerField,
|
||||
<<<<<<< HEAD
|
||||
$allowedDepartmentIds,
|
||||
$departmentField,
|
||||
=======
|
||||
$customerFieldMode,
|
||||
>>>>>>> origin/master
|
||||
$fixedConditions
|
||||
);
|
||||
|
||||
@@ -1799,7 +1849,12 @@ class system_search_service
|
||||
* @param array<int, string> $terms
|
||||
* @param array<int, int> $customerNumbers
|
||||
* @param string|null $customerField
|
||||
<<<<<<< HEAD
|
||||
* @param array<int, int> $departmentIds
|
||||
* @param string|null $departmentField
|
||||
=======
|
||||
* @param string $customerFieldMode
|
||||
>>>>>>> origin/master
|
||||
* @param array<string, mixed> $fixedConditions
|
||||
* @return array<int, array<string, mixed>>
|
||||
*/
|
||||
@@ -1810,7 +1865,12 @@ class system_search_service
|
||||
array $terms,
|
||||
array $customerNumbers = [],
|
||||
?string $customerField = null,
|
||||
<<<<<<< HEAD
|
||||
array $departmentIds = [],
|
||||
?string $departmentField = null,
|
||||
=======
|
||||
string $customerFieldMode = 'default',
|
||||
>>>>>>> origin/master
|
||||
array $fixedConditions = []
|
||||
): array {
|
||||
global $db;
|
||||
@@ -1847,6 +1907,10 @@ class system_search_service
|
||||
}
|
||||
}
|
||||
|
||||
if (!empty($departmentIds) && $departmentField !== null && in_array($departmentField, $fields, true)) {
|
||||
$wheres[] = "`$departmentField` IN (" . implode(',', array_map('intval', $departmentIds)) . ")";
|
||||
}
|
||||
|
||||
$termClauses = [];
|
||||
foreach ($terms as $term) {
|
||||
$escaped = $db->escape_string($term);
|
||||
|
||||
@@ -113,6 +113,7 @@ class systemSearchRoute
|
||||
'allowed_types' => $allowedTypes,
|
||||
'own_only_types' => $ownOnlyTypes,
|
||||
'own_customer_number' => $this->resolveEffectiveCustomerNumber(),
|
||||
'allowed_department_ids' => $this->resolveAllowedDepartmentIds($user),
|
||||
'permissions_catalog_all' => $permissionsCatalogAll,
|
||||
'permissions_catalog_own' => $permissionsCatalogOwn,
|
||||
'module_config_visibility' => $this->buildModuleConfigVisibility(),
|
||||
@@ -507,6 +508,27 @@ class systemSearchRoute
|
||||
return $flat;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param mixed $user
|
||||
* @return array<int, int>
|
||||
*/
|
||||
private function resolveAllowedDepartmentIds(mixed $user): array
|
||||
{
|
||||
if ($user === false) {
|
||||
return [];
|
||||
}
|
||||
|
||||
try {
|
||||
$departments = $user->getGroup()->getDepartments();
|
||||
if (!is_array($departments)) {
|
||||
return [];
|
||||
}
|
||||
return array_values(array_unique(array_map('intval', $departments)));
|
||||
} catch (Throwable) {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
private function parseTypeList(mixed $value): array
|
||||
{
|
||||
$result = [];
|
||||
|
||||
@@ -131,6 +131,7 @@ if (!class_exists('TestableSystemSearchService')) {
|
||||
array $permissionsCatalogAll,
|
||||
array $permissionsCatalogOwn,
|
||||
array $moduleConfigVisibility,
|
||||
array $allowedDepartmentIds = [],
|
||||
array $forcedCustomerNumbers = []
|
||||
): array {
|
||||
$this->lexicalCalls[] = [
|
||||
@@ -139,6 +140,7 @@ if (!class_exists('TestableSystemSearchService')) {
|
||||
'entityBoost' => $entityBoost,
|
||||
'ownOnlyTypes' => $ownOnlyTypes,
|
||||
'ownCustomerNumber' => $ownCustomerNumber,
|
||||
'allowedDepartmentIds' => $allowedDepartmentIds,
|
||||
'forcedCustomerNumbers' => $forcedCustomerNumbers,
|
||||
];
|
||||
if (empty($this->queuedLexicalResults)) {
|
||||
@@ -385,6 +387,25 @@ it('caps AI-driven expanded terms to prevent query amplification', function ():
|
||||
expect($maxLen)->toBeLessThanOrEqual(64);
|
||||
});
|
||||
|
||||
<<<<<<< HEAD
|
||||
|
||||
it('passes allowed department ids into lexical execution context', function (): void {
|
||||
$parser = new FakeSystemSearchIntentParser();
|
||||
|
||||
$service = new TestableSystemSearchService($parser, [[
|
||||
['entity_type' => 'orders', 'entity_id' => '1', 'title' => 'Order #1', 'score' => 70],
|
||||
]]);
|
||||
|
||||
$service->search([
|
||||
'query' => 'order',
|
||||
'allowed_types' => ['orders'],
|
||||
'include_associations' => false,
|
||||
'allowed_department_ids' => [3, '7', 3],
|
||||
]);
|
||||
|
||||
expect(count($service->lexicalCalls))->toBe(1);
|
||||
expect($service->lexicalCalls[0]['allowedDepartmentIds'])->toBe([3, 7]);
|
||||
=======
|
||||
it('does not expand associations for own-only entity types', function (): void {
|
||||
$parser = new FakeSystemSearchIntentParser();
|
||||
$service = new TestableSystemSearchService($parser, [
|
||||
@@ -958,4 +979,5 @@ it('includes the economic customer index in cache dependencies for customer scop
|
||||
$tables = system_search_service_invoke_private($service, 'relevantSourceTables', [['objects', 'orders', 'vehicles']]);
|
||||
|
||||
expect($tables)->toContain(system_search_economic_customer_index::TABLE);
|
||||
>>>>>>> origin/master
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user