Fix product null department permissions
This commit is contained in:
@@ -22,21 +22,23 @@ class productsRoute
|
||||
*/
|
||||
private function getCustomerIfProvided(): ?users_o
|
||||
{
|
||||
global $response;
|
||||
if (self::isParametersSet(['customer_id'])) {
|
||||
$customerId = (int)self::getParameter('customer_id');
|
||||
try {
|
||||
$customerObject = (new users_o())->getUserByCustomerNumber((int)$customerId);
|
||||
if ($customerObject->exists()) {
|
||||
return $customerObject;
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
// Log the incident
|
||||
(new logs_o())->add('products', 'global', 3, 0, 'GET_CUSTOMER_FAILED', 'Failed to get customer with id ' . $customerId . '. Error: ' . $e->getMessage());
|
||||
// Return null
|
||||
return null;
|
||||
}
|
||||
$customerId = $this->getOptionalPositiveIntParameter('customer_id');
|
||||
if ($customerId === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
try {
|
||||
$customerObject = (new users_o())->getUserByCustomerNumber($customerId);
|
||||
if ($customerObject->exists()) {
|
||||
return $customerObject;
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
// Log the incident
|
||||
(new logs_o())->add('products', 'global', 3, 0, 'GET_CUSTOMER_FAILED', 'Failed to get customer with id ' . $customerId . '. Error: ' . $e->getMessage());
|
||||
// Return null
|
||||
return null;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -45,12 +47,49 @@ class productsRoute
|
||||
* @return int|null
|
||||
*/
|
||||
private function getDepartmentIdIfProvided(): ?int
|
||||
{
|
||||
return $this->getOptionalPositiveIntParameter('department_id');
|
||||
}
|
||||
|
||||
private function getOptionalPositiveIntParameter(string $parameter): ?int
|
||||
{
|
||||
global $response;
|
||||
if (self::isParametersSet(['department_id'])) {
|
||||
return (int)self::getParameter('department_id');
|
||||
if (!self::isParametersSet([$parameter])) {
|
||||
return null;
|
||||
}
|
||||
return null;
|
||||
|
||||
$value = self::getParameter($parameter);
|
||||
if ($this->isNullLikeOptionalParameter($value)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$parsed = null;
|
||||
if (is_int($value)) {
|
||||
$parsed = $value;
|
||||
} elseif (is_string($value) && preg_match('/^\d+$/', trim($value)) === 1) {
|
||||
$parsed = (int)trim($value);
|
||||
} else {
|
||||
$response->error('Invalid ' . $parameter, 400);
|
||||
}
|
||||
|
||||
if ($parsed === null || $parsed <= 0) {
|
||||
$response->error('Invalid ' . $parameter, 400);
|
||||
}
|
||||
|
||||
return $parsed;
|
||||
}
|
||||
|
||||
private function isNullLikeOptionalParameter(mixed $value): bool
|
||||
{
|
||||
if ($value === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!is_string($value)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return in_array(strtolower(trim($value)), ['', 'null', 'undefined'], true);
|
||||
}
|
||||
|
||||
private function assertCanUseDepartmentPricing(mixed $user, ?int $departmentId): void
|
||||
@@ -72,11 +111,7 @@ class productsRoute
|
||||
*/
|
||||
private function getCategoryIfProvided(): ?int
|
||||
{
|
||||
global $response;
|
||||
if (self::isParametersSet(['category'])) {
|
||||
return (int)self::getParameter('category');
|
||||
}
|
||||
return null;
|
||||
return $this->getOptionalPositiveIntParameter('category');
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -85,11 +120,7 @@ class productsRoute
|
||||
*/
|
||||
private function getProductIdIfProvided(): ?int
|
||||
{
|
||||
global $response;
|
||||
if (self::isParametersSet(['id'])) {
|
||||
return (int)self::getParameter('id');
|
||||
}
|
||||
return null;
|
||||
return $this->getOptionalPositiveIntParameter('id');
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -210,13 +241,14 @@ class productsRoute
|
||||
// Check if the request was successful
|
||||
if ($user || $isProductDetailsRestricted) {
|
||||
// Define the variables
|
||||
$customer = self::getCustomerIfProvided(); // This is only used if the customer_id parameter is provided
|
||||
$departmentId = self::getDepartmentIdIfProvided(); // This is only used if the department_id parameter is provided
|
||||
$this->assertCanUseDepartmentPricing($user, $departmentId);
|
||||
$category = self::getCategoryIfProvided(); // This is only used if the category parameter is provided (ID of the category)
|
||||
$productId = self::getProductIdIfProvided(); // This is only used if the id parameter is provided (ID of the product)
|
||||
$customer = $this->getCustomerIfProvided(); // This is only used if the customer_id parameter is provided
|
||||
$departmentId = $this->getDepartmentIdIfProvided(); // This is only used if the department_id parameter is provided
|
||||
$category = $this->getCategoryIfProvided(); // This is only used if the category parameter is provided (ID of the category)
|
||||
$productId = $this->getProductIdIfProvided(); // This is only used if the id parameter is provided (ID of the product)
|
||||
$useFinalPrice = self::isParametersSet(['final_price']) && self::getParameter('final_price') === 'true';
|
||||
// Check if the "final_price" parameter is set, and true.
|
||||
if (self::isParametersSet(['final_price']) && self::getParameter('final_price') === 'true') {
|
||||
if ($useFinalPrice) {
|
||||
$this->assertCanUseDepartmentPricing($user, $departmentId);
|
||||
// Determine the products to return
|
||||
if ($category) {
|
||||
// Get products in the category
|
||||
@@ -274,17 +306,17 @@ class productsRoute
|
||||
);
|
||||
}
|
||||
// Check if the category is set in the request
|
||||
$data = $_GET ?? [];
|
||||
// Check if the category is set
|
||||
if (isset($data['category'])) {
|
||||
if ($category !== null) {
|
||||
// Log the incident
|
||||
(new logs_o())->add('products', 'global', 1, $responsibleUserId, 'LIST_PRODUCTS', 'Successfully listed products in category ' . $data['category']);
|
||||
(new logs_o())->add('products', 'global', 1, $responsibleUserId, 'LIST_PRODUCTS', 'Successfully listed products in category ' . $category);
|
||||
// Return the list of products
|
||||
$products = (new products_o())->listObjectsByCategory($data['category']);
|
||||
$products = (new products_o())->listObjectsByCategory($category);
|
||||
// Check if the department_id is set
|
||||
if (isset($data['department_id'])) {
|
||||
if ($departmentId !== null) {
|
||||
$this->assertCanUseDepartmentPricing($user, $departmentId);
|
||||
// Apply the departments unique pricing
|
||||
$products = (new products_o())->applyDepartmentPricing((array)$products, (int)$data['department_id']);
|
||||
$products = (new products_o())->applyDepartmentPricing((array)$products, $departmentId);
|
||||
}
|
||||
$response->success(
|
||||
array_map(function ($product) use ($isProductDetailsRestricted) {
|
||||
@@ -295,9 +327,10 @@ class productsRoute
|
||||
// Log the incident
|
||||
(new logs_o())->add('products', 'global', 1, $responsibleUserId, 'LIST_PRODUCTS', 'Successfully listed products');
|
||||
// Check if the department_id is set
|
||||
if (isset($data['department_id'])) {
|
||||
if ($departmentId !== null) {
|
||||
$this->assertCanUseDepartmentPricing($user, $departmentId);
|
||||
// Get all product ids contained in a category attached to the department
|
||||
$departmentSpecificProducts = (new departments_o())->select((int)$data['department_id'])->getAllProductInDepartmentCategories();
|
||||
$departmentSpecificProducts = (new departments_o())->select($departmentId)->getAllProductInDepartmentCategories();
|
||||
// Get the product ids as an array
|
||||
$departmentSpecificProductIds = array_map(function ($product) {
|
||||
return $product->id;
|
||||
@@ -312,7 +345,7 @@ class productsRoute
|
||||
(new products_o())->forceRestrictFilters([
|
||||
'id' => $departmentSpecificProductIds,
|
||||
])
|
||||
), (int)$data['department_id'])
|
||||
), $departmentId)
|
||||
);
|
||||
}
|
||||
// Return the list of products
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
usesApiSuite();
|
||||
|
||||
it('treats null-like optional product params as omitted for product detail requests', function (): void {
|
||||
api_test_covers('GET /products', 'optional-params');
|
||||
|
||||
$product = api_fixtures()->createProduct([
|
||||
'name' => 'Null Query Product',
|
||||
'price' => 400,
|
||||
]);
|
||||
$session = api_fixtures()->createUserSession([], ['group_id' => 1]);
|
||||
|
||||
$response = api_client()->get(
|
||||
'/products?id=' . (int)$product['id']
|
||||
. '&department_id=null&customer_id=null&category_id=null&final_price=false',
|
||||
$session['headers']
|
||||
);
|
||||
|
||||
$response
|
||||
->assertStatus(200)
|
||||
->assertEnvelope()
|
||||
->assertSuccess();
|
||||
|
||||
expect($response->data())
|
||||
->toBeArray()
|
||||
->toHaveKey('id', (int)$product['id']);
|
||||
expect($response->body)->not->toContain('department_access_0');
|
||||
});
|
||||
|
||||
it('still requires department access when final product pricing uses a real department', function (): void {
|
||||
api_test_covers('GET /products', 'permissions');
|
||||
|
||||
$department = api_fixtures()->createDepartment(['name' => 'Product Pricing Department']);
|
||||
$product = api_fixtures()->createProduct([
|
||||
'name' => 'Department Priced Product',
|
||||
'price' => 500,
|
||||
]);
|
||||
$session = api_fixtures()->createUserSession(['list_products']);
|
||||
|
||||
api_client()->get(
|
||||
'/products?final_price=true&id=' . (int)$product['id']
|
||||
. '&department_id=' . (int)$department['id'],
|
||||
$session['headers']
|
||||
)
|
||||
->assertStatus(403)
|
||||
->assertEnvelope()
|
||||
->assertSuccess(false)
|
||||
->assertMissingPermissions(['department_access_' . (int)$department['id']]);
|
||||
});
|
||||
|
||||
it('rejects invalid department ids without requesting department access zero', function (): void {
|
||||
api_test_covers('GET /products', 'validation');
|
||||
|
||||
$product = api_fixtures()->createProduct([
|
||||
'name' => 'Invalid Department Product',
|
||||
'price' => 600,
|
||||
]);
|
||||
$session = api_fixtures()->createUserSession(['list_products']);
|
||||
|
||||
$response = api_client()->get(
|
||||
'/products?final_price=true&id=' . (int)$product['id'] . '&department_id=0',
|
||||
$session['headers']
|
||||
);
|
||||
|
||||
$response
|
||||
->assertStatus(400)
|
||||
->assertEnvelope()
|
||||
->assertSuccess(false)
|
||||
->assertMessage('Invalid department_id');
|
||||
|
||||
expect($response->body)->not->toContain('department_access_0');
|
||||
});
|
||||
Reference in New Issue
Block a user