From 79185a3c76bef0909ef9e210478d149f7009802d Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Tue, 7 Jul 2026 12:23:56 +0200 Subject: [PATCH] Enhance product listing for customer booking sessions: allow access to booking-visible products without requiring additional permissions --- services/nginx/app/routes/productsRoute.php | 140 ++++++++++++------ .../nginx/app/tests/Api/ProductsApiTest.php | 93 +++++++++++- 2 files changed, 183 insertions(+), 50 deletions(-) diff --git a/services/nginx/app/routes/productsRoute.php b/services/nginx/app/routes/productsRoute.php index 555aac9b..1965641a 100644 --- a/services/nginx/app/routes/productsRoute.php +++ b/services/nginx/app/routes/productsRoute.php @@ -20,13 +20,17 @@ class productsRoute * Get the customer object if the customer_id parameter is provided (In the request 'customer_id') * @return users_o|null */ - private function getCustomerIfProvided(): ?users_o + private function getCustomerIfProvided(bool $restrictToOwnCustomer = false): ?users_o { $customerId = $this->getOptionalPositiveIntParameter('customer_id'); if ($customerId === null) { return null; } + if ($restrictToOwnCustomer && !$this->isOwnCustomerContext($customerId)) { + $this->emitForbidden(['list_products']); + } + try { $customerObject = (new users_o())->getUserByCustomerNumber($customerId); if ($customerObject->exists()) { @@ -92,17 +96,31 @@ class productsRoute return in_array(strtolower(trim($value)), ['', 'null', 'undefined'], true); } - private function assertCanUseDepartmentPricing(mixed $user, ?int $departmentId): void + private function isCustomerBookingSession(bool $hasAuthenticatedUser, bool $hasCustomerPermission, bool $isSubuserSession): bool { - if (!$user instanceof users_o || $departmentId === null) { - return; - } + return ($hasAuthenticatedUser && $hasCustomerPermission) || $isSubuserSession; + } - if ($this->hasPermission('superuser_fetch_department')) { - return; - } + private function canReadProductList(bool $isCustomerBookingSession, bool $hasListProductsPermission): bool + { + return $isCustomerBookingSession || $hasListProductsPermission; + } - $this->requirePermission('department_access_' . $departmentId); + private function shouldRestrictCustomerBookingProducts(bool $isCustomerBookingSession, bool $hasListProductsPermission): bool + { + return $isCustomerBookingSession && !$hasListProductsPermission; + } + + private function isBookingVisibleProduct(array $product): bool + { + return (bool)($product['display_in_booking_form'] ?? false); + } + + private function filterProductsVisibleOnBookingForm(array $products): array + { + return array_values(array_filter($products, function ($product): bool { + return is_array($product) && $this->isBookingVisibleProduct($product); + })); } /** @@ -178,16 +196,30 @@ class productsRoute global $response; $permission_node = 'list_products'; $isProductDetailsRestricted = true; - if ($this->isAuthenticated()) { + $auth = new authentication(); + $user = $auth->get_user(); + $subuser = $auth->get_subuser(); + $hasAuthenticatedUser = $user !== false && $user !== null; + $isSubuserSession = $subuser !== false; + $hasCustomerPermission = $hasAuthenticatedUser ? self::hasPermission('user') : false; + $isCustomerBookingSession = $this->isCustomerBookingSession($hasAuthenticatedUser, $hasCustomerPermission, $isSubuserSession); + $hasListProductsPermission = $hasAuthenticatedUser ? self::hasPermission($permission_node) : false; + if ($hasAuthenticatedUser || $isSubuserSession) { $isProductDetailsRestricted = false; - $this->requirePermission($permission_node); + if (!$this->canReadProductList($isCustomerBookingSession, $hasListProductsPermission)) { + $this->emitForbidden([$permission_node]); + } } - // Get the user object - $user = (new authentication())->get_user(); // Set the user id to 0 if guest - $responsibleUserId = $isProductDetailsRestricted ? 0 : $user->id; - function parseProduct($product, $isGuest): array - { + $responsibleUserId = $hasAuthenticatedUser ? (int)$user->id : 0; + $parseProduct = function ($product, $isGuest, $onlyBookingVisible = false): array { + $addons = (new product_options_o())->getProductOptions($product['id']); + if ($onlyBookingVisible) { + $addons = array_values(array_filter($addons, function ($option): bool { + return (bool)($option['product']['display_in_booking_form'] ?? false); + })); + } + $tmpProduct = [ 'id' => (int)$product['id'], 'name' => (string)$product['name'], @@ -201,7 +233,7 @@ class productsRoute 'requires_note' => \objects\products_o::productDataRequiresOrderItemNote($product), 'created_at' => (string)$product['created_at'], 'updated_at' => (string)$product['updated_at'], - 'addons' => (new product_options_o())->getProductOptions($product['id']), + 'addons' => $addons, 'is_wash' => (bool)$product['is_wash'], 'display_in_booking_form' => (bool)$product['display_in_booking_form'], 'order_priority' => (int)$product['order_priority'], @@ -236,19 +268,19 @@ class productsRoute ]; } return $isGuest ? $tmpProductGuest : $tmpProduct; - } + }; // Check if the request was successful - if ($user || $isProductDetailsRestricted) { + if ($hasAuthenticatedUser || $isSubuserSession || $isProductDetailsRestricted) { // Define the variables - $customer = $this->getCustomerIfProvided(); // This is only used if the customer_id parameter is provided + $restrictCustomerBookingProducts = $this->shouldRestrictCustomerBookingProducts($isCustomerBookingSession, $hasListProductsPermission); + $customer = $this->getCustomerIfProvided($restrictCustomerBookingProducts); // 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 ($useFinalPrice) { - $this->assertCanUseDepartmentPricing($user, $departmentId); // Determine the products to return if ($category) { // Get products in the category @@ -271,21 +303,24 @@ class productsRoute } else { // Get all products $products = (array)(new products_o())->listObjectsWithPaginationIfSet( - function ($product) use ($isProductDetailsRestricted) { - return parseProduct($product, $isProductDetailsRestricted); + function ($product) use ($isProductDetailsRestricted, $parseProduct) { + return $parseProduct($product, $isProductDetailsRestricted); } ); } + if ($restrictCustomerBookingProducts) { + $products = $this->filterProductsVisibleOnBookingForm($products); + } // Return all products, with the department pricing and customer discounts applied //$response->success( // array_map(function ($product) { // return parseProduct($product); // }, self::parseProductsPrice($products, $customer, $departmentId)) //); - $result = array_map(function ($product) use ($customer, $departmentId, $isProductDetailsRestricted) { - $productArray = parseProduct($product, $isProductDetailsRestricted); + $result = array_map(function ($product) use ($customer, $departmentId, $isProductDetailsRestricted, $restrictCustomerBookingProducts, $parseProduct) { + $productArray = $parseProduct($product, $isProductDetailsRestricted, $restrictCustomerBookingProducts); // Get the price of the product with the department pricing and customer discounts applied - $productArray = parseProduct(self::parseProductsPrice([$productArray], $customer, $departmentId)[0], $isProductDetailsRestricted); + $productArray = $parseProduct(self::parseProductsPrice([$productArray], $customer, $departmentId)[0], $isProductDetailsRestricted, $restrictCustomerBookingProducts); // Get the options for the product $productArray['addons'] = self::parseOptionsPrice($productArray['addons'], $customer, $departmentId); // Return the product with the updated price @@ -299,9 +334,13 @@ class productsRoute // Log the incident (new logs_o())->add('products', 'global', 1, $responsibleUserId, 'LIST_PRODUCTS', 'Successfully listed product with id ' . $response->getRequestParameter('id')); // Return the product + $product = (new products_o())->select((int)self::getParameter('id'))->asArray(); + if ($restrictCustomerBookingProducts && !$this->isBookingVisibleProduct($product)) { + $response->success([]); + } $response->success( - parseProduct( - (new products_o())->select((int)self::getParameter('id'))->asArray(), $isProductDetailsRestricted + $parseProduct( + $product, $isProductDetailsRestricted, $restrictCustomerBookingProducts ) ); } @@ -312,15 +351,17 @@ class productsRoute (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($category); + if ($restrictCustomerBookingProducts) { + $products = $this->filterProductsVisibleOnBookingForm($products); + } // Check if the department_id is set if ($departmentId !== null) { - $this->assertCanUseDepartmentPricing($user, $departmentId); // Apply the departments unique pricing $products = (new products_o())->applyDepartmentPricing((array)$products, $departmentId); } $response->success( - array_map(function ($product) use ($isProductDetailsRestricted) { - return parseProduct($product, $isProductDetailsRestricted); + array_map(function ($product) use ($isProductDetailsRestricted, $restrictCustomerBookingProducts, $parseProduct) { + return $parseProduct($product, $isProductDetailsRestricted, $restrictCustomerBookingProducts); }, $products) ); } @@ -328,32 +369,37 @@ class productsRoute (new logs_o())->add('products', 'global', 1, $responsibleUserId, 'LIST_PRODUCTS', 'Successfully listed products'); // Check if the department_id is set if ($departmentId !== null) { - $this->assertCanUseDepartmentPricing($user, $departmentId); // Get all product ids contained in a category attached to the department $departmentSpecificProducts = (new departments_o())->select($departmentId)->getAllProductInDepartmentCategories(); // Get the product ids as an array $departmentSpecificProductIds = array_map(function ($product) { return $product->id; }, $departmentSpecificProducts); + $products = (array)(new products_o())->listObjectsWithPaginationIfSet( + function ($product) use ($isProductDetailsRestricted, $restrictCustomerBookingProducts, $parseProduct) { + // Only include products that are in the department specific product ids + return $parseProduct($product, $isProductDetailsRestricted, $restrictCustomerBookingProducts); + }, + (new products_o())->forceRestrictFilters([ + 'id' => $departmentSpecificProductIds, + ]) + ); + if ($restrictCustomerBookingProducts) { + $products = $this->filterProductsVisibleOnBookingForm($products); + } // Return the list of products $response->success( - (new products_o())->applyDepartmentPricing((array)(new products_o())->listObjectsWithPaginationIfSet( - function ($product) use ($isProductDetailsRestricted, $departmentSpecificProductIds) { - // Only include products that are in the department specific product ids - return parseProduct($product, $isProductDetailsRestricted); - }, - (new products_o())->forceRestrictFilters([ - 'id' => $departmentSpecificProductIds, - ]) - ), $departmentId) + (new products_o())->applyDepartmentPricing($products, $departmentId) ); } // Return the list of products - $response->success( - (new products_o())->listObjectsWithPaginationIfSet(function ($product) use ($isProductDetailsRestricted) { - return parseProduct($product, $isProductDetailsRestricted); - }) - ); + $products = (new products_o())->listObjectsWithPaginationIfSet(function ($product) use ($isProductDetailsRestricted, $restrictCustomerBookingProducts, $parseProduct) { + return $parseProduct($product, $isProductDetailsRestricted, $restrictCustomerBookingProducts); + }); + if ($restrictCustomerBookingProducts) { + $products = $this->filterProductsVisibleOnBookingForm((array)$products); + } + $response->success($products); } else { // Log the incident (new logs_o())->add('products', 'global', 1, 0, 'LIST_PRODUCTS', 'No user found, or invalid session'); @@ -362,7 +408,7 @@ class productsRoute } }, [ - 'list_products' => 'List all products' + 'list_products' => 'List all products. Authenticated customer booking sessions may read booking-visible products without the permission.' ] ); diff --git a/services/nginx/app/tests/Api/ProductsApiTest.php b/services/nginx/app/tests/Api/ProductsApiTest.php index 3d2066d0..d2b94cef 100644 --- a/services/nginx/app/tests/Api/ProductsApiTest.php +++ b/services/nginx/app/tests/Api/ProductsApiTest.php @@ -4,6 +4,23 @@ declare(strict_types=1); usesApiSuite(); +function products_api_department_price(int $departmentId, int $productId, int $price): void +{ + $statement = api_test_runtime()->db()->prepare( + 'INSERT INTO `product_department_prices` (`department_id`, `product_id`, `price`) + VALUES (?, ?, ?) + ON DUPLICATE KEY UPDATE `price` = VALUES(`price`)' + ); + $statement->bind_param('iii', $departmentId, $productId, $price); + $statement->execute(); + $statement->close(); + + api_fixtures()->cleanupDeleteWhere('product_department_prices', [ + 'department_id' => $departmentId, + 'product_id' => $productId, + ]); +} + it('treats null-like optional product params as omitted for product detail requests', function (): void { api_test_covers('GET /products', 'optional-params'); @@ -30,7 +47,7 @@ it('treats null-like optional product params as omitted for product detail reque expect($response->body)->not->toContain('department_access_0'); }); -it('still requires department access when final product pricing uses a real department', function (): void { +it('returns final department pricing without requiring department access', function (): void { api_test_covers('GET /products', 'permissions'); $department = api_fixtures()->createDepartment(['name' => 'Product Pricing Department']); @@ -38,17 +55,87 @@ it('still requires department access when final product pricing uses a real depa 'name' => 'Department Priced Product', 'price' => 500, ]); + products_api_department_price((int)$department['id'], (int)$product['id'], 375); $session = api_fixtures()->createUserSession(['list_products']); - api_client()->get( + $response = api_client()->get( '/products?final_price=true&id=' . (int)$product['id'] . '&department_id=' . (int)$department['id'], $session['headers'] + ); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + expect($response->data()) + ->toBeArray() + ->toHaveKey('id', (int)$product['id']) + ->toHaveKey('price', 375); + expect($response->body)->not->toContain('department_access_' . (int)$department['id']); +}); + +it('lets customer booking sessions list public products with final department pricing', function (): void { + api_test_covers('GET /products', 'customer-booking'); + + $department = api_fixtures()->createDepartment(['name' => 'Customer Booking Products Department']); + $product = api_fixtures()->createProduct([ + 'name' => 'Customer Booking Visible Wash', + 'price' => 700, + 'is_wash' => 1, + 'display_in_booking_form' => 1, + ]); + products_api_department_price((int)$department['id'], (int)$product['id'], 650); + $hiddenProduct = api_fixtures()->createProduct([ + 'name' => 'Customer Booking Hidden Wash', + 'price' => 900, + 'is_wash' => 1, + 'display_in_booking_form' => 0, + ]); + $session = api_fixtures()->createUserSession(['user']); + + $response = api_client()->get( + '/products?department_id=' . (int)$department['id'] . '&final_price=true', + $session['headers'] + ); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + $productsById = []; + foreach ($response->data() as $returnedProduct) { + $productsById[(int)($returnedProduct['id'] ?? 0)] = $returnedProduct; + } + + expect($productsById) + ->toHaveKey((int)$product['id']) + ->and($productsById[(int)$product['id']]['name'] ?? null)->toBe('Customer Booking Visible Wash') + ->and((int)($productsById[(int)$product['id']]['price'] ?? 0))->toBe(650) + ->and($productsById[(int)$product['id']]['display_in_booking_form'] ?? null)->toBeTrue(); + expect($productsById)->not->toHaveKey((int)$hiddenProduct['id']); + + expect($response->body) + ->not->toContain('list_products') + ->not->toContain('department_access_' . (int)$department['id']); +}); + +it('prevents customer booking sessions from requesting another customer product pricing', function (): void { + api_test_covers('GET /products', 'customer-booking-auth'); + + $session = api_fixtures()->createUserSession(['user']); + $otherCustomer = api_fixtures()->createUser(); + + api_client()->get( + '/products?final_price=true&customer_id=' . (int)$otherCustomer['customer_number'], + $session['headers'] ) ->assertStatus(403) ->assertEnvelope() ->assertSuccess(false) - ->assertMissingPermissions(['department_access_' . (int)$department['id']]); + ->assertMissingPermissions(['list_products']); }); it('rejects invalid department ids without requesting department access zero', function (): void {