From 53d06361935ed24bad26807c741c861483e2a715 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:12:01 +0000 Subject: [PATCH 1/8] Fix limited-backoffice permissions and add department access restrictions --- .../classes/limited_backoffice_service.php | 38 ++ services/nginx/app/routes/orderItemsRoute.php | 17 +- services/nginx/app/routes/ordersRoute.php | 13 +- .../tests/Api/LimitedBackofficeApiTest.php | 523 ++++++++++++++++++ .../nginx/app/tests/Api/OrderItemsApiTest.php | 4 +- .../nginx/app/tests/Api/OrdersApiTest.php | 10 +- 6 files changed, 596 insertions(+), 9 deletions(-) diff --git a/services/nginx/app/classes/limited_backoffice_service.php b/services/nginx/app/classes/limited_backoffice_service.php index 67aa895f..416d3bac 100644 --- a/services/nginx/app/classes/limited_backoffice_service.php +++ b/services/nginx/app/classes/limited_backoffice_service.php @@ -47,6 +47,7 @@ class limited_backoffice_service 'permissions' => [ 'user', 'permissions_list_own', + 'list_departments', 'list_orders', 'fetch_order', 'add_order', @@ -89,6 +90,11 @@ class limited_backoffice_service 'add_bookings', 'complete_bookings', 'resend_booking_confirmations', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', + 'statistics_orders_new', + 'statistics_bookings_new', ], ], 'booking_coordinator' => [ @@ -97,6 +103,7 @@ class limited_backoffice_service 'permissions' => [ 'user', 'permissions_list_own', + 'list_departments', 'list_orders', 'list_bookings', 'list_own_bookings', @@ -108,6 +115,11 @@ class limited_backoffice_service 'department_timebookings_entries_get', 'department_timebookings_entries_post', 'department_timebookings_entries_put', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', + 'statistics_orders_new', + 'statistics_bookings_new', ], ], 'operations_lead' => [ @@ -116,6 +128,7 @@ class limited_backoffice_service 'permissions' => [ 'user', 'permissions_list_own', + 'list_departments', 'list_orders', 'fetch_order', 'add_order', @@ -159,6 +172,9 @@ class limited_backoffice_service 'add_bookings', 'complete_bookings', 'resend_booking_confirmations', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', 'statistics_orders_new', 'statistics_bookings_new', ], @@ -169,6 +185,7 @@ class limited_backoffice_service 'permissions' => [ 'user', 'permissions_list_own', + 'list_departments', 'list_orders', 'fetch_order', 'add_order', @@ -212,6 +229,9 @@ class limited_backoffice_service 'add_bookings', 'complete_bookings', 'resend_booking_confirmations', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', 'statistics_orders_new', 'statistics_bookings_new', self::PERMISSION_ACCESS, @@ -417,6 +437,22 @@ class limited_backoffice_service 'group' => 'time_bookings', 'capability' => 'edit_time_booking_entries', ], + 'list_departments' => [ + 'group' => 'departments', + 'capability' => 'view_departments', + ], + 'list_department_daily_reports' => [ + 'group' => 'departments', + 'capability' => 'view_daily_reports', + ], + 'list_notifications' => [ + 'group' => 'notifications', + 'capability' => 'view_notifications', + ], + 'list_own_notifications' => [ + 'group' => 'notifications', + 'capability' => 'view_notifications', + ], 'statistics_orders_new' => [ 'group' => 'reports', 'capability' => 'view_order_statistics', @@ -444,6 +480,7 @@ class limited_backoffice_service */ private const ROLE_PERMISSION_GROUP_ORDER = [ 'account', + 'departments', 'orders', 'products', 'customers', @@ -452,6 +489,7 @@ class limited_backoffice_service 'scanner', 'bookings', 'time_bookings', + 'notifications', 'reports', 'limited_backoffice', ]; diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index c6786290..75dc1e15 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -75,6 +75,8 @@ class orderItemsRoute if (!$order->exists()) { $response->error('Order not found', 404); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$order->department_id->value()); $product = (new products_o())->getProductById((int)$data['product_id']); if (!$product->exists()) { $response->error('Product not found', 404); @@ -173,7 +175,7 @@ class orderItemsRoute $this->delete('/order/items', function () { // Require the user to be logged in - global $response; + global $response, $db; $this->requirePermission('delete_order_items'); // Get the user object $user = (new authentication())->get_user(); @@ -185,6 +187,17 @@ class orderItemsRoute if (!isset($data['id'])) { $response->error('Order Item ID is required', 400); } + // Look up the order item to check department access + $orderItemContext = $db->query( + 'SELECT oi.order_id FROM order_items oi WHERE oi.id = ' . (int)$data['id'] . ' LIMIT 1' + ); + $orderItemRow = $orderItemContext ? $orderItemContext->fetch_assoc() : null; + if ($orderItemRow !== null) { + $orderForAccess = (new orders_o())->getOrderById((int)$orderItemRow['order_id']); + if ($orderForAccess->exists()) { + self::requireDepartmentAccess((string)(int)$orderForAccess->department_id->value()); + } + } // Delete the order item (new order_items_o())->removeOrderItem((int)$data['id']); // Return the list of departments @@ -261,6 +274,8 @@ class orderItemsRoute if (!$order->exists()) { $response->error('Order not found', 404); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$order->department_id->value()); $canAccessAllOrderItems = $this->hasPermission('list_order_items'); if (!$canAccessAllOrderItems && !$order->isOwnOrder((int)$user->customer_number->value())) { diff --git a/services/nginx/app/routes/ordersRoute.php b/services/nginx/app/routes/ordersRoute.php index d8681fd2..de71173b 100644 --- a/services/nginx/app/routes/ordersRoute.php +++ b/services/nginx/app/routes/ordersRoute.php @@ -172,6 +172,8 @@ class ordersRoute if (!(new departments_o())->getDepartmentById((int)$data['department_id'])) { $response->error('Department not found', 400); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$data['department_id']); // Make sure the customer number set is valid $targetUser = (new users_o())->getUserByCustomerNumber((int)$data['customer_id']); if (!$targetUser->exists()) { @@ -472,6 +474,8 @@ class ordersRoute if (!$order->exists()) { $response->error('Order not found', 400); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$order->department_id->value()); // Get the base64 file $base64_file = (string)$this->getParameter('base64_file'); $attachment_store = new attachment_store(); @@ -530,6 +534,8 @@ class ordersRoute if (!$order->exists()) { $response->error('Order not found', 400); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$order->department_id->value()); // Delete the attachment $order->removeAttachment((int)$attachment_id); // Log the incident @@ -568,6 +574,8 @@ class ordersRoute if (!$order->exists()) { $response->error('Order not found', 400); } + // Check if the user has access to the department + self::requireDepartmentAccess((string)(int)$order->department_id->value()); // Mark the order as completed $order->markAsCompleted((string)$user->display_name->value()); // Log the incident @@ -1154,7 +1162,8 @@ class ordersRoute } // Admin/department path (requires edit_order) self::requirePermission($permission_other); - /** Departmental access */ + /** Departmental access — user must have access to the order's current department */ + self::requireDepartmentAccess((string)(int)$order->department_id->value()); $originalCustomerNumber = (int)$order->customer_id->value(); $newCustomerNumber = $originalCustomerNumber; $shouldAutoReassignInvoiceCollection = false; @@ -1219,6 +1228,8 @@ class ordersRoute if (!(new departments_o())->getDepartmentById((int)$data['department_id'])) { $response->error('Department not found', 400); } + // Check if the user has access to the target department + self::requireDepartmentAccess((string)(int)$data['department_id']); $order->department_id->set((int)$data['department_id']); } // If the booking ID is set, validate it diff --git a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php index 8223c0db..2c6e904b 100644 --- a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php +++ b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php @@ -1606,3 +1606,526 @@ it('rejects invalid limited backoffice employee contact details', function (): v ->assertSuccess(false) ->assertMessage('Phone number must be 4-15 digits.'); }); + +it('includes list_departments in all active limited backoffice role presets', function (): void { + foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { + $permissions = limited_backoffice_role_preset_permissions($roleKey); + expect($permissions) + ->toContain('list_departments', "$roleKey must include list_departments"); + } +}); + +it('includes list_department_daily_reports (dagsopgørelse) in all active limited backoffice role presets', function (): void { + foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { + $permissions = limited_backoffice_role_preset_permissions($roleKey); + expect($permissions) + ->toContain('list_department_daily_reports', "$roleKey must include list_department_daily_reports"); + } +}); + +it('includes list_notifications and list_own_notifications (Notifikationer) in all active limited backoffice role presets', function (): void { + foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { + $permissions = limited_backoffice_role_preset_permissions($roleKey); + expect($permissions) + ->toContain('list_notifications', "$roleKey must include list_notifications") + ->toContain('list_own_notifications', "$roleKey must include list_own_notifications"); + } +}); + +it('includes statistics_orders_new and statistics_bookings_new (Overblik) in all active limited backoffice role presets', function (): void { + foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { + $permissions = limited_backoffice_role_preset_permissions($roleKey); + expect($permissions) + ->toContain('statistics_orders_new', "$roleKey must include statistics_orders_new") + ->toContain('statistics_bookings_new', "$roleKey must include statistics_bookings_new"); + } +}); + +it('assigns list_departments to managed employees and enforces list_departments permission on GET /departments', function (): void { + api_test_covers('GET /departments', 'limited backoffice employee'); + + $department = api_fixtures()->createDepartment(['name' => 'LB Dept List Test']); + $manager = limited_backoffice_manager_session([(int)$department['id']]); + + $result = api_client()->post('/limited-backoffice/employees', [ + 'role_key' => 'cashier', + 'department_ids' => [(int)$department['id']], + 'display_name' => 'Dept List Test Employee', + 'email' => 'dept-list-test@example.test', + 'password' => 'Secret123!', + ], $manager['headers']); + + $result->assertStatus(200)->assertEnvelope()->assertSuccess(); + $employeeId = (int)($result->data()['id'] ?? 0); + expect($employeeId)->toBeGreaterThan(0); + limited_backoffice_cleanup_created_employee($employeeId); + + $employeeRow = api_test_runtime()->queryOne( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + ); + $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); + expect($managedGroupId)->toBeGreaterThan(0); + + $permRow = api_test_runtime()->queryOne( + 'SELECT 1 FROM `groups_permissions` + WHERE `group_id` = ' . $managedGroupId . " AND `permission` = 'list_departments' LIMIT 1" + ); + expect($permRow)->not->toBeNull('Managed employee must have list_departments permission'); +}); + +it('assigns list_department_daily_reports to managed employees', function (): void { + $department = api_fixtures()->createDepartment(['name' => 'LB Daily Report Perm Test']); + $manager = limited_backoffice_manager_session([(int)$department['id']]); + + $result = api_client()->post('/limited-backoffice/employees', [ + 'role_key' => 'cashier', + 'department_ids' => [(int)$department['id']], + 'display_name' => 'Daily Report Perm Employee', + 'email' => 'daily-report-perm@example.test', + 'password' => 'Secret123!', + ], $manager['headers']); + + $result->assertStatus(200)->assertEnvelope()->assertSuccess(); + $employeeId = (int)($result->data()['id'] ?? 0); + expect($employeeId)->toBeGreaterThan(0); + limited_backoffice_cleanup_created_employee($employeeId); + + $employeeRow = api_test_runtime()->queryOne( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + ); + $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); + expect($managedGroupId)->toBeGreaterThan(0); + + $permRow = api_test_runtime()->queryOne( + 'SELECT 1 FROM `groups_permissions` + WHERE `group_id` = ' . $managedGroupId . " AND `permission` = 'list_department_daily_reports' LIMIT 1" + ); + expect($permRow)->not->toBeNull('Managed employee must have list_department_daily_reports permission'); +}); + +it('assigns list_notifications and list_own_notifications to managed employees', function (): void { + $department = api_fixtures()->createDepartment(['name' => 'LB Notifications Perm Test']); + $manager = limited_backoffice_manager_session([(int)$department['id']]); + + $result = api_client()->post('/limited-backoffice/employees', [ + 'role_key' => 'cashier', + 'department_ids' => [(int)$department['id']], + 'display_name' => 'Notifications Perm Employee', + 'email' => 'notifications-perm@example.test', + 'password' => 'Secret123!', + ], $manager['headers']); + + $result->assertStatus(200)->assertEnvelope()->assertSuccess(); + $employeeId = (int)($result->data()['id'] ?? 0); + expect($employeeId)->toBeGreaterThan(0); + limited_backoffice_cleanup_created_employee($employeeId); + + $employeeRow = api_test_runtime()->queryOne( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + ); + $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); + expect($managedGroupId)->toBeGreaterThan(0); + + foreach (['list_notifications', 'list_own_notifications'] as $perm) { + $permRow = api_test_runtime()->queryOne( + 'SELECT 1 FROM `groups_permissions` + WHERE `group_id` = ' . $managedGroupId . " AND `permission` = '" . $perm . "' LIMIT 1" + ); + expect($permRow)->not->toBeNull("Managed employee must have $perm permission"); + } +}); + +it('assigns statistics permissions (Overblik) to all active limited backoffice role presets including cashier and booking_coordinator', function (): void { + $department = api_fixtures()->createDepartment(['name' => 'LB Stats Perm Test']); + $manager = limited_backoffice_manager_session([(int)$department['id']]); + + foreach (['cashier', 'booking_coordinator'] as $roleKey) { + $result = api_client()->post('/limited-backoffice/employees', [ + 'role_key' => $roleKey, + 'department_ids' => [(int)$department['id']], + 'display_name' => 'Stats Perm ' . $roleKey, + 'email' => 'stats-perm-' . $roleKey . '@example.test', + 'password' => 'Secret123!', + ], $manager['headers']); + + $result->assertStatus(200)->assertEnvelope()->assertSuccess(); + $employeeId = (int)($result->data()['id'] ?? 0); + expect($employeeId)->toBeGreaterThan(0); + limited_backoffice_cleanup_created_employee($employeeId); + + $employeeRow = api_test_runtime()->queryOne( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + ); + $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); + expect($managedGroupId)->toBeGreaterThan(0); + + foreach (['statistics_orders_new', 'statistics_bookings_new'] as $perm) { + $permRow = api_test_runtime()->queryOne( + 'SELECT 1 FROM `groups_permissions` + WHERE `group_id` = ' . $managedGroupId . " AND `permission` = '" . $perm . "' LIMIT 1" + ); + expect($permRow)->not->toBeNull("$roleKey managed employee must have $perm permission"); + } + } +}); + +it('enforces department access when creating an order via POST /orders', function (): void { + api_test_covers('POST /orders', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Create Allowed Dept']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Create Denied Dept']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Create Customer']); + + $session = api_fixtures()->createUserSession([ + 'add_order', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should succeed for accessible department + $created = api_client()->post('/orders', [ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + 'reference' => 'DEPT-ACCESS-TEST', + 'notes' => '', + 'reg_1' => 'ABCD111', + ], $session['headers']); + $created->assertStatus(200)->assertEnvelope()->assertSuccess(); + + // Should fail for inaccessible department + api_client()->post('/orders', [ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + 'reference' => 'DEPT-ACCESS-DENIED', + 'notes' => '', + 'reg_1' => 'ABCD222', + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); +}); + +it('enforces department access on the existing order when editing via PUT /order', function (): void { + api_test_covers('PUT /order', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Edit Allowed Dept']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Edit Denied Dept']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Edit Customer']); + + $orderInAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderInDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + + $session = api_fixtures()->createUserSession([ + 'edit_order', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should succeed editing order in accessible department + api_client()->put('/order', [ + 'id' => (int)$orderInAllowed['id'], + 'notes' => 'updated', + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + // Should fail editing order in inaccessible department + api_client()->put('/order', [ + 'id' => (int)$orderInDenied['id'], + 'notes' => 'should be denied', + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); +}); + +it('enforces department access when moving an order to a new department via PUT /order', function (): void { + api_test_covers('PUT /order', 'department access move'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Move Allowed Dept']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Move Denied Dept']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Move Customer']); + + $order = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + + $sessionBothDepts = api_fixtures()->createUserSession([ + 'edit_order', + 'department_access_' . (int)$allowedDepartment['id'], + 'department_access_' . (int)$deniedDepartment['id'], + ]); + + $sessionOnlyAllowed = api_fixtures()->createUserSession([ + 'edit_order', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should fail when moving to inaccessible department (user only has access to allowedDepartment) + api_client()->put('/order', [ + 'id' => (int)$order['id'], + 'department_id' => (int)$deniedDepartment['id'], + ], $sessionOnlyAllowed['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed when user has access to both departments + api_client()->put('/order', [ + 'id' => (int)$order['id'], + 'department_id' => (int)$deniedDepartment['id'], + ], $sessionBothDepts['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); + +it('enforces department access when marking an order as completed via POST /orders/mark_as_completed', function (): void { + api_test_covers('POST /orders/mark_as_completed', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Complete Allowed Dept']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Complete Denied Dept']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Complete Customer']); + + $orderAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + + $session = api_fixtures()->createUserSession([ + 'mark_order_as_completed', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should fail for inaccessible department + api_client()->post('/orders/mark_as_completed', [ + 'id' => (int)$orderDenied['id'], + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed for accessible department + api_client()->post('/orders/mark_as_completed', [ + 'id' => (int)$orderAllowed['id'], + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); + +it('enforces department access when deleting order attachments via DELETE /orders/attachments', function (): void { + api_test_covers('DELETE /orders/attachments', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Del Attach Allowed']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Del Attach Denied']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Del Attach Customer']); + + $orderAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + $attachmentAllowed = api_fixtures()->createOrderAttachment(['order_id' => (int)$orderAllowed['id']]); + $attachmentDenied = api_fixtures()->createOrderAttachment(['order_id' => (int)$orderDenied['id']]); + + $session = api_fixtures()->createUserSession([ + 'delete_order_attachments', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should fail for inaccessible department + api_client()->delete('/orders/attachments', [ + 'order_id' => (int)$orderDenied['id'], + 'attachment_id' => (int)$attachmentDenied['id'], + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed for accessible department + api_client()->delete('/orders/attachments', [ + 'order_id' => (int)$orderAllowed['id'], + 'attachment_id' => (int)$attachmentAllowed['id'], + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); + +it('enforces department access when adding order items via POST /order/items', function (): void { + api_test_covers('POST /order/items', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Add Allowed']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Add Denied']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Item Add Customer']); + $category = api_fixtures()->createCategory(['name' => 'Order Item Dept Access Category']); + $product = api_fixtures()->createProduct(['name' => 'Order Item Dept Access Product', 'category' => $category['id']]); + + $orderAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + + $session = api_fixtures()->createUserSession([ + 'add_order_items', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should fail for inaccessible department + api_client()->post('/order/items', [ + 'order_id' => (int)$orderDenied['id'], + 'product_id' => (int)$product['id'], + 'quantity' => 1, + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed for accessible department + api_client()->post('/order/items', [ + 'order_id' => (int)$orderAllowed['id'], + 'product_id' => (int)$product['id'], + 'quantity' => 1, + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); + +it('enforces department access when deleting order items via DELETE /order/items', function (): void { + api_test_covers('DELETE /order/items', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Del Allowed']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Del Denied']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Item Del Customer']); + $category = api_fixtures()->createCategory(['name' => 'Order Item Del Category']); + $product = api_fixtures()->createProduct(['name' => 'Order Item Del Product', 'category' => $category['id']]); + + $orderAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + + $session = api_fixtures()->createUserSession([ + 'delete_order_items', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + $superuserSession = api_fixtures()->createUserSession([], ['group_id' => 1]); + $itemAllowedResponse = api_client()->post('/order/items', [ + 'order_id' => (int)$orderAllowed['id'], + 'product_id' => (int)$product['id'], + 'quantity' => 1, + ], $superuserSession['headers']); + $itemAllowed = $itemAllowedResponse->data(); + + $itemDeniedResponse = api_client()->post('/order/items', [ + 'order_id' => (int)$orderDenied['id'], + 'product_id' => (int)$product['id'], + 'quantity' => 1, + ], $superuserSession['headers']); + $itemDenied = $itemDeniedResponse->data(); + + // Should fail for inaccessible department + api_client()->delete('/order/items', ['id' => (int)$itemDenied['id']], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed for accessible department + api_client()->delete('/order/items', ['id' => (int)$itemAllowed['id']], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); + +it('enforces department access when editing order items via PUT /order/items', function (): void { + api_test_covers('PUT /order/items', 'department access'); + + $allowedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Edit Allowed']); + $deniedDepartment = api_fixtures()->createDepartment(['name' => 'Order Item Edit Denied']); + $customer = api_fixtures()->createUser(['display_name' => 'Order Item Edit Customer']); + $category = api_fixtures()->createCategory(['name' => 'Order Item Edit Category']); + $product = api_fixtures()->createProduct(['name' => 'Order Item Edit Product', 'category' => $category['id']]); + + $orderAllowed = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$allowedDepartment['id'], + ]); + $orderDenied = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => (int)$deniedDepartment['id'], + ]); + $itemAllowed = api_fixtures()->createOrderItem([ + 'order_id' => (int)$orderAllowed['id'], + 'product_id' => (int)$product['id'], + 'cashier_id' => 1, + 'price' => 100, + ]); + $itemDenied = api_fixtures()->createOrderItem([ + 'order_id' => (int)$orderDenied['id'], + 'product_id' => (int)$product['id'], + 'cashier_id' => 1, + 'price' => 200, + ]); + + $session = api_fixtures()->createUserSession([ + 'edit_order_items', + 'list_order_items', + 'department_access_' . (int)$allowedDepartment['id'], + ]); + + // Should fail for inaccessible department + api_client()->put('/order/items', [ + 'id' => (int)$itemDenied['id'], + 'price' => 999, + 'notes' => '', + 'reference' => '', + 'quantity' => 1, + ], $session['headers']) + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMissingPermissions(['department_access_' . (int)$deniedDepartment['id']]); + + // Should succeed for accessible department + api_client()->put('/order/items', [ + 'id' => (int)$itemAllowed['id'], + 'price' => 150, + 'notes' => '', + 'reference' => '', + 'quantity' => 1, + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); +}); diff --git a/services/nginx/app/tests/Api/OrderItemsApiTest.php b/services/nginx/app/tests/Api/OrderItemsApiTest.php index 0088ee9a..e02fc464 100644 --- a/services/nginx/app/tests/Api/OrderItemsApiTest.php +++ b/services/nginx/app/tests/Api/OrderItemsApiTest.php @@ -150,7 +150,7 @@ it('uses a product fixed price instead of the best discount when adding an order 'cashier_id' => $cashier['id'], 'reference' => 'FIXED-PRICE', ]); - $session = api_fixtures()->createUserSession(['add_order_items', 'list_products']); + $session = api_fixtures()->createUserSession(['add_order_items', 'list_products', 'department_access_' . $department['id']]); $productResponse = api_client()->get( '/products?final_price=true&id=' . $product['id'] . '&customer_id=' . $customer['customer_number'], @@ -281,7 +281,7 @@ it('does not allow clearing notes for order items whose product requires notes', 'quantity' => 1, 'notes' => 'Initial note', ]); - $session = api_fixtures()->createUserSession(['edit_order_items', 'list_order_items']); + $session = api_fixtures()->createUserSession(['edit_order_items', 'list_order_items', 'department_access_' . $department['id']]); api_client() ->put('/order/items', [ diff --git a/services/nginx/app/tests/Api/OrdersApiTest.php b/services/nginx/app/tests/Api/OrdersApiTest.php index 1560658c..22b702a2 100644 --- a/services/nginx/app/tests/Api/OrdersApiTest.php +++ b/services/nginx/app/tests/Api/OrdersApiTest.php @@ -73,7 +73,7 @@ it('creates orders through the orders endpoint', function (): void { $customer = api_fixtures()->createUser(['display_name' => 'Order Create Customer']); $department = api_fixtures()->createDepartment(['name' => 'Order Create Department']); - $session = api_fixtures()->createUserSession(['add_order']); + $session = api_fixtures()->createUserSession(['add_order', 'department_access_' . $department['id']]); $response = api_client()->post('/orders', [ 'customer_id' => $customer['customer_number'], @@ -128,7 +128,7 @@ it('defaults order PO only from a matching active booking', function (): void { 'po' => 'DELETED-BOOKING-PO', 'deleted_at' => date('Y-m-d H:i:s'), ]); - $session = api_fixtures()->createUserSession(['add_order', 'edit_order'], [ + $session = api_fixtures()->createUserSession(['add_order', 'edit_order', 'department_access_' . $department['id']], [ 'customer_number' => $customer['customer_number'], ]); @@ -149,7 +149,7 @@ it('defaults order PO only from a matching active booking', function (): void { $matchingOrderId = (int)($createResponse->data()['id'] ?? 0); expect($createResponse->data()['po'] ?? null)->toBe('MATCHING-BOOKING-PO'); - $unauthorizedSession = api_fixtures()->createUserSession(['add_order'], [ + $unauthorizedSession = api_fixtures()->createUserSession(['add_order', 'department_access_' . $department['id']], [ 'customer_number' => $otherCustomer['customer_number'], ]); $unauthorizedResponse = api_client()->post('/orders', [ @@ -233,7 +233,7 @@ it('rejects invalid order creation requests', function (): void { $customer = api_fixtures()->createUser(); $department = api_fixtures()->createDepartment(); - $session = api_fixtures()->createUserSession(['add_order']); + $session = api_fixtures()->createUserSession(['add_order', 'department_access_' . $department['id']]); api_client()->post('/orders', [ 'customer_id' => $customer['customer_number'], @@ -262,7 +262,7 @@ it('updates orders through the primary and legacy endpoints', function (): void 'notes' => 'Before update', 'reg_1' => 'BEFORE1', ]); - $session = api_fixtures()->createUserSession(['edit_order']); + $session = api_fixtures()->createUserSession(['edit_order', 'department_access_' . $department['id']]); api_client()->put('/orders', [ 'id' => $order['id'], From 1d25cbe21cac4523b2d92e664c72b6dcd3585afe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:14:34 +0000 Subject: [PATCH 2/8] Fix SQL injection concerns: use prepared statements in orderItemsRoute and tests --- services/nginx/app/routes/orderItemsRoute.php | 14 ++++++++++---- .../app/tests/Api/LimitedBackofficeApiTest.php | 18 ++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index 75dc1e15..2e5ba3b4 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -188,10 +188,16 @@ class orderItemsRoute $response->error('Order Item ID is required', 400); } // Look up the order item to check department access - $orderItemContext = $db->query( - 'SELECT oi.order_id FROM order_items oi WHERE oi.id = ' . (int)$data['id'] . ' LIMIT 1' - ); - $orderItemRow = $orderItemContext ? $orderItemContext->fetch_assoc() : null; + $itemId = (int)$data['id']; + $stmt = $db->prepare('SELECT oi.order_id FROM order_items oi WHERE oi.id = ? LIMIT 1'); + if ($stmt !== false) { + $stmt->bind_param('i', $itemId); + $stmt->execute(); + $orderItemRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); + } else { + $orderItemRow = null; + } if ($orderItemRow !== null) { $orderForAccess = (new orders_o())->getOrderById((int)$orderItemRow['order_id']); if ($orderForAccess->exists()) { diff --git a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php index 2c6e904b..4d60484c 100644 --- a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php +++ b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php @@ -1727,10 +1727,13 @@ it('assigns list_notifications and list_own_notifications to managed employees', expect($managedGroupId)->toBeGreaterThan(0); foreach (['list_notifications', 'list_own_notifications'] as $perm) { - $permRow = api_test_runtime()->queryOne( - 'SELECT 1 FROM `groups_permissions` - WHERE `group_id` = ' . $managedGroupId . " AND `permission` = '" . $perm . "' LIMIT 1" + $stmt = api_test_runtime()->db()->prepare( + 'SELECT 1 FROM `groups_permissions` WHERE `group_id` = ? AND `permission` = ? LIMIT 1' ); + $stmt->bind_param('is', $managedGroupId, $perm); + $stmt->execute(); + $permRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); expect($permRow)->not->toBeNull("Managed employee must have $perm permission"); } }); @@ -1760,10 +1763,13 @@ it('assigns statistics permissions (Overblik) to all active limited backoffice r expect($managedGroupId)->toBeGreaterThan(0); foreach (['statistics_orders_new', 'statistics_bookings_new'] as $perm) { - $permRow = api_test_runtime()->queryOne( - 'SELECT 1 FROM `groups_permissions` - WHERE `group_id` = ' . $managedGroupId . " AND `permission` = '" . $perm . "' LIMIT 1" + $stmt = api_test_runtime()->db()->prepare( + 'SELECT 1 FROM `groups_permissions` WHERE `group_id` = ? AND `permission` = ? LIMIT 1' ); + $stmt->bind_param('is', $managedGroupId, $perm); + $stmt->execute(); + $permRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); expect($permRow)->not->toBeNull("$roleKey managed employee must have $perm permission"); } } From d0f94ac549ade70536efae4f96cf65497a5149fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:15:59 +0000 Subject: [PATCH 3/8] Use prepared statements for all new DB queries in tests and route --- .../tests/Api/LimitedBackofficeApiTest.php | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php index 4d60484c..be5c1b9b 100644 --- a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php +++ b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php @@ -1660,16 +1660,24 @@ it('assigns list_departments to managed employees and enforces list_departments expect($employeeId)->toBeGreaterThan(0); limited_backoffice_cleanup_created_employee($employeeId); - $employeeRow = api_test_runtime()->queryOne( - 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + $stmt = api_test_runtime()->db()->prepare( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ? LIMIT 1' ); + $stmt->bind_param('i', $employeeId); + $stmt->execute(); + $employeeRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); expect($managedGroupId)->toBeGreaterThan(0); - $permRow = api_test_runtime()->queryOne( - 'SELECT 1 FROM `groups_permissions` - WHERE `group_id` = ' . $managedGroupId . " AND `permission` = 'list_departments' LIMIT 1" + $permStmt = api_test_runtime()->db()->prepare( + 'SELECT 1 FROM `groups_permissions` WHERE `group_id` = ? AND `permission` = ? LIMIT 1' ); + $listDepts = 'list_departments'; + $permStmt->bind_param('is', $managedGroupId, $listDepts); + $permStmt->execute(); + $permRow = $permStmt->get_result()->fetch_assoc(); + $permStmt->close(); expect($permRow)->not->toBeNull('Managed employee must have list_departments permission'); }); @@ -1690,16 +1698,24 @@ it('assigns list_department_daily_reports to managed employees', function (): vo expect($employeeId)->toBeGreaterThan(0); limited_backoffice_cleanup_created_employee($employeeId); - $employeeRow = api_test_runtime()->queryOne( - 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + $stmt = api_test_runtime()->db()->prepare( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ? LIMIT 1' ); + $stmt->bind_param('i', $employeeId); + $stmt->execute(); + $employeeRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); expect($managedGroupId)->toBeGreaterThan(0); - $permRow = api_test_runtime()->queryOne( - 'SELECT 1 FROM `groups_permissions` - WHERE `group_id` = ' . $managedGroupId . " AND `permission` = 'list_department_daily_reports' LIMIT 1" + $permStmt = api_test_runtime()->db()->prepare( + 'SELECT 1 FROM `groups_permissions` WHERE `group_id` = ? AND `permission` = ? LIMIT 1' ); + $dailyReports = 'list_department_daily_reports'; + $permStmt->bind_param('is', $managedGroupId, $dailyReports); + $permStmt->execute(); + $permRow = $permStmt->get_result()->fetch_assoc(); + $permStmt->close(); expect($permRow)->not->toBeNull('Managed employee must have list_department_daily_reports permission'); }); @@ -1720,9 +1736,13 @@ it('assigns list_notifications and list_own_notifications to managed employees', expect($employeeId)->toBeGreaterThan(0); limited_backoffice_cleanup_created_employee($employeeId); - $employeeRow = api_test_runtime()->queryOne( - 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + $stmt = api_test_runtime()->db()->prepare( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ? LIMIT 1' ); + $stmt->bind_param('i', $employeeId); + $stmt->execute(); + $employeeRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); expect($managedGroupId)->toBeGreaterThan(0); @@ -1756,9 +1776,13 @@ it('assigns statistics permissions (Overblik) to all active limited backoffice r expect($employeeId)->toBeGreaterThan(0); limited_backoffice_cleanup_created_employee($employeeId); - $employeeRow = api_test_runtime()->queryOne( - 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ' . $employeeId . ' LIMIT 1' + $stmtEmp = api_test_runtime()->db()->prepare( + 'SELECT `managed_group_id` FROM `limited_backoffice_employees` WHERE `user_id` = ? LIMIT 1' ); + $stmtEmp->bind_param('i', $employeeId); + $stmtEmp->execute(); + $employeeRow = $stmtEmp->get_result()->fetch_assoc(); + $stmtEmp->close(); $managedGroupId = (int)($employeeRow['managed_group_id'] ?? 0); expect($managedGroupId)->toBeGreaterThan(0); From 734cd13c8751d8011b2bab248bf98f9cc1e6e383 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:16:55 +0000 Subject: [PATCH 4/8] Handle prepared statement failure with error response in DELETE /order/items --- services/nginx/app/routes/orderItemsRoute.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index 2e5ba3b4..ca1baa3e 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -190,14 +190,14 @@ class orderItemsRoute // Look up the order item to check department access $itemId = (int)$data['id']; $stmt = $db->prepare('SELECT oi.order_id FROM order_items oi WHERE oi.id = ? LIMIT 1'); - if ($stmt !== false) { - $stmt->bind_param('i', $itemId); - $stmt->execute(); - $orderItemRow = $stmt->get_result()->fetch_assoc(); - $stmt->close(); - } else { - $orderItemRow = null; + if ($stmt === false) { + (new logs_o())->add('order_items', 'global', 1, 0, 'DELETE_ORDER_ITEMS', 'Failed to prepare department access check query'); + $response->error('Failed to validate department access', 500); } + $stmt->bind_param('i', $itemId); + $stmt->execute(); + $orderItemRow = $stmt->get_result()->fetch_assoc(); + $stmt->close(); if ($orderItemRow !== null) { $orderForAccess = (new orders_o())->getOrderById((int)$orderItemRow['order_id']); if ($orderForAccess->exists()) { From e4465d9d91d82a50cfeb693ed9fc4c0f4ab5fbf9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:17:50 +0000 Subject: [PATCH 5/8] Improve DELETE /order/items: clearer error message, 404 when order not found --- services/nginx/app/routes/orderItemsRoute.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index ca1baa3e..6dc7d174 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -191,8 +191,8 @@ class orderItemsRoute $itemId = (int)$data['id']; $stmt = $db->prepare('SELECT oi.order_id FROM order_items oi WHERE oi.id = ? LIMIT 1'); if ($stmt === false) { - (new logs_o())->add('order_items', 'global', 1, 0, 'DELETE_ORDER_ITEMS', 'Failed to prepare department access check query'); - $response->error('Failed to validate department access', 500); + (new logs_o())->add('order_items', 'global', 1, 0, 'DELETE_ORDER_ITEMS', 'Database error while preparing department access check query'); + $response->error('Database error while checking department access', 500); } $stmt->bind_param('i', $itemId); $stmt->execute(); @@ -200,9 +200,10 @@ class orderItemsRoute $stmt->close(); if ($orderItemRow !== null) { $orderForAccess = (new orders_o())->getOrderById((int)$orderItemRow['order_id']); - if ($orderForAccess->exists()) { - self::requireDepartmentAccess((string)(int)$orderForAccess->department_id->value()); + if (!$orderForAccess->exists()) { + $response->error('Order not found', 404); } + self::requireDepartmentAccess((string)(int)$orderForAccess->department_id->value()); } // Delete the order item (new order_items_o())->removeOrderItem((int)$data['id']); From d9fbba3130c7382a50ddfe4e918f4785646d9ec6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:19:00 +0000 Subject: [PATCH 6/8] Return 404 when order item not found in DELETE /order/items --- services/nginx/app/routes/orderItemsRoute.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index 6dc7d174..13f1665c 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -204,6 +204,8 @@ class orderItemsRoute $response->error('Order not found', 404); } self::requireDepartmentAccess((string)(int)$orderForAccess->department_id->value()); + } else { + $response->error('Order item not found', 404); } // Delete the order item (new order_items_o())->removeOrderItem((int)$data['id']); From 57bcbaf72a0584d6802e6028411764fe2d42f6fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Jul 2026 00:31:43 +0000 Subject: [PATCH 7/8] Fix 11 failing API tests across 4 files --- .../classes/limited_backoffice_service.php | 20 +++++++++++++++++++ services/nginx/app/routes/orderItemsRoute.php | 14 ++++++------- services/nginx/app/routes/ordersRoute.php | 6 +----- .../tests/Api/LimitedBackofficeApiTest.php | 14 ++++++------- 4 files changed, 34 insertions(+), 20 deletions(-) diff --git a/services/nginx/app/classes/limited_backoffice_service.php b/services/nginx/app/classes/limited_backoffice_service.php index 416d3bac..90072899 100644 --- a/services/nginx/app/classes/limited_backoffice_service.php +++ b/services/nginx/app/classes/limited_backoffice_service.php @@ -29,6 +29,21 @@ class limited_backoffice_service self::PERMISSION_PUBLIC_EMPLOYEE_DATA, ]; + /** + * Permissions that are always granted to managed employees when present in a role preset, + * regardless of whether the creating manager holds those permissions themselves. + * + * @var array + */ + private const ROLE_UNCONDITIONAL_PERMISSIONS = [ + 'list_departments', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', + 'statistics_orders_new', + 'statistics_bookings_new', + ]; + /** * @var array}> */ @@ -1638,6 +1653,11 @@ class limited_backoffice_service continue; } + if (in_array($permission, self::ROLE_UNCONDITIONAL_PERMISSIONS, true)) { + $permissions[] = $permission; + continue; + } + if ($manager->hasPermission($permission)) { $permissions[] = $permission; } diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index 13f1665c..e3a51fc0 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -81,9 +81,6 @@ class orderItemsRoute if (!$product->exists()) { $response->error('Product not found', 404); } - if ($product->requiresOrderItemNote() && trim((string)($notes ?? '')) === '') { - $response->error('Notes is required for this product', 400); - } $customerRuleViolation = (new customer_product_rule_service()) ->firstViolationForOrderItem((int)$data['order_id'], (int)$data['product_id'], $related_item_id); if ($customerRuleViolation !== null) { @@ -97,6 +94,9 @@ class orderItemsRoute ); $response->error($customerRuleViolation['message'], 400); } + if ($product->requiresOrderItemNote() && trim((string)($notes ?? '')) === '') { + $response->error('Notes is required for this product', 400); + } // Add the order item to the order This is done individually, to make the notes to the individual order items possible $order_items = (new order_items_o()); @@ -181,12 +181,12 @@ class orderItemsRoute $user = (new authentication())->get_user(); // Check if the request was successful if ($user) { - // Get the query data - $data = $_GET; - // Check if the required fields are set - if (!isset($data['id'])) { + // Get the order item id from the query string or request body + $itemIdRaw = $this->fromRequest('id'); + if ($itemIdRaw === null || $itemIdRaw === '') { $response->error('Order Item ID is required', 400); } + $data = ['id' => $itemIdRaw]; // Look up the order item to check department access $itemId = (int)$data['id']; $stmt = $db->prepare('SELECT oi.order_id FROM order_items oi WHERE oi.id = ? LIMIT 1'); diff --git a/services/nginx/app/routes/ordersRoute.php b/services/nginx/app/routes/ordersRoute.php index de71173b..7625c5e4 100644 --- a/services/nginx/app/routes/ordersRoute.php +++ b/services/nginx/app/routes/ordersRoute.php @@ -1373,11 +1373,7 @@ class ordersRoute { try { $user = (new authentication())->get_user(); - if ($user !== false && isset($user->customer_number) && (int)$user->customer_number->value() === $customerNumber) { - return true; - } - - return $this->hasDepartmentAccess((string)$departmentId); + return $user !== false && isset($user->customer_number) && (int)$user->customer_number->value() === $customerNumber; } catch (\Throwable) { return false; } diff --git a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php index be5c1b9b..f1d0253f 100644 --- a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php +++ b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php @@ -1610,16 +1610,14 @@ it('rejects invalid limited backoffice employee contact details', function (): v it('includes list_departments in all active limited backoffice role presets', function (): void { foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { $permissions = limited_backoffice_role_preset_permissions($roleKey); - expect($permissions) - ->toContain('list_departments', "$roleKey must include list_departments"); + expect($permissions)->toContain('list_departments'); } }); it('includes list_department_daily_reports (dagsopgørelse) in all active limited backoffice role presets', function (): void { foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { $permissions = limited_backoffice_role_preset_permissions($roleKey); - expect($permissions) - ->toContain('list_department_daily_reports', "$roleKey must include list_department_daily_reports"); + expect($permissions)->toContain('list_department_daily_reports'); } }); @@ -1627,8 +1625,8 @@ it('includes list_notifications and list_own_notifications (Notifikationer) in a foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { $permissions = limited_backoffice_role_preset_permissions($roleKey); expect($permissions) - ->toContain('list_notifications', "$roleKey must include list_notifications") - ->toContain('list_own_notifications', "$roleKey must include list_own_notifications"); + ->toContain('list_notifications') + ->toContain('list_own_notifications'); } }); @@ -1636,8 +1634,8 @@ it('includes statistics_orders_new and statistics_bookings_new (Overblik) in all foreach (['cashier', 'booking_coordinator', 'operations_lead', 'department_admin'] as $roleKey) { $permissions = limited_backoffice_role_preset_permissions($roleKey); expect($permissions) - ->toContain('statistics_orders_new', "$roleKey must include statistics_orders_new") - ->toContain('statistics_bookings_new', "$roleKey must include statistics_bookings_new"); + ->toContain('statistics_orders_new') + ->toContain('statistics_bookings_new'); } }); From 0b342a7780d8b20d4d7de53b954996f7f9b642ee Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Tue, 7 Jul 2026 02:47:49 +0200 Subject: [PATCH 8/8] Align limited backoffice permission cap tests --- .../tests/Api/LimitedBackofficeApiTest.php | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php index f1d0253f..01ed883a 100644 --- a/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php +++ b/services/nginx/app/tests/Api/LimitedBackofficeApiTest.php @@ -25,6 +25,7 @@ function limited_backoffice_all_role_permissions(): array return [ 'user', 'permissions_list_own', + 'list_departments', 'list_orders', 'fetch_order', 'add_order', @@ -71,6 +72,9 @@ function limited_backoffice_all_role_permissions(): array 'department_timebookings_entries_get', 'department_timebookings_entries_post', 'department_timebookings_entries_put', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', 'statistics_orders_new', 'statistics_bookings_new', ]; @@ -887,6 +891,7 @@ it('creates updates lists and deactivates scoped employees without exposing raw }); foreach ([ 'list_orders', + 'list_departments', 'fetch_order', 'add_order', 'edit_order', @@ -928,6 +933,9 @@ it('creates updates lists and deactivates scoped employees without exposing raw 'department_timebookings_entries_get', 'department_timebookings_entries_post', 'department_timebookings_entries_put', + 'list_department_daily_reports', + 'list_notifications', + 'list_own_notifications', 'statistics_orders_new', 'statistics_bookings_new', 'limited_backoffice_access', @@ -1075,7 +1083,7 @@ it('creates updates lists and deactivates scoped employees without exposing raw }); }); -it('caps limited employee permissions to the manager permissions and selected departments', function (): void { +it('caps manager-gated limited employee permissions while keeping baseline role permissions', function (): void { api_test_covers('POST /limited-backoffice/employees', 'auth'); api_test_covers('GET /limited-backoffice/roles', 'auth'); @@ -1088,9 +1096,11 @@ it('caps limited employee permissions to the manager permissions and selected de $rolesByKey = array_column($roles->data(), null, 'key'); $operationsLeadGroups = array_column($rolesByKey['operations_lead']['permission_groups'] ?? [], 'capabilities', 'key'); expect($operationsLeadGroups['account'] ?? null)->toBe(['sign_in', 'view_own_permissions']); + expect($operationsLeadGroups['departments'] ?? null)->toBe(['view_departments', 'view_daily_reports']); expect($operationsLeadGroups['orders'] ?? null)->toBe(['view_orders']); + expect($operationsLeadGroups['notifications'] ?? null)->toBe(['view_notifications']); + expect($operationsLeadGroups['reports'] ?? null)->toBe(['view_order_statistics', 'view_booking_statistics']); expect($roles->body)->not->toContain('create_orders'); - expect($roles->body)->not->toContain('view_order_statistics'); $created = api_client()->post('/limited-backoffice/employees', [ 'display_name' => 'Limited Capped Lead', @@ -1122,7 +1132,13 @@ it('caps limited employee permissions to the manager permissions and selected de ->toContain('user') ->toContain('permissions_list_own') ->toContain('employee_public_data') + ->toContain('list_departments') ->toContain('list_orders') + ->toContain('list_department_daily_reports') + ->toContain('list_notifications') + ->toContain('list_own_notifications') + ->toContain('statistics_orders_new') + ->toContain('statistics_bookings_new') ->toContain('department_access_' . (int)$department['id']) ->not->toContain('add_order') ->not->toContain('fetch_order') @@ -1130,7 +1146,6 @@ it('caps limited employee permissions to the manager permissions and selected de ->not->toContain('search_customers') ->not->toContain('add_bookings') ->not->toContain('delete_order') - ->not->toContain('statistics_orders_new') ->not->toContain(limited_backoffice_service::PERMISSION_MANAGE_EMPLOYEES); });