Merge pull request #84 from copenhagentruckwash/fix-pos-url-order-reassignment-vulnerability

Prevent implicit POS order department changes

src/components/shop/POSDepartmentProcess.vue

@@ -320,20 +320,26 @@ export const ensureCurrentOrderDepartment = async (targetDepartmentId = null) =>
   }
 
   try {
-    let currentDepartmentId = null;
-    try {
-      currentDepartmentId = await getCurrentOrderDepartmentId(normalizedOrderId);
-    } catch (error) {
-      currentDepartmentId = null;
-    }
+    const currentDepartmentId = await getCurrentOrderDepartmentId(normalizedOrderId);
     if (currentDepartmentId === normalizedDepartmentId) {
       department_id.value = normalizedDepartmentId;
       return true;
     }
 
-    await SessionUser.objects.orders.set.department_id(normalizedOrderId, normalizedDepartmentId);
-    department_id.value = normalizedDepartmentId;
-    return true;
+    parseError(
+      {
+        response: {
+          data: {
+            data: {
+              message: "Ordren tilhører ikke den valgte afdeling",
+            },
+            message: "Ordren tilhører ikke den valgte afdeling",
+          },
+        },
+      },
+      "stepError"
+    );
+    return false;
   } catch (error) {
     parseError(error, "stepError");
     return false;
@@ -727,9 +733,9 @@ export const restoreStoredPosOrderId = async (
     if (selectedDepartmentId && toPositiveInteger(storedOrder?.department_id) !== selectedDepartmentId) {
       if (options.syncDepartment === true) {
         order_id.value = storedOrderId;
-        const didSyncDepartment = await ensureCurrentOrderDepartment(selectedDepartmentId);
-        if (!didSyncDepartment) {
-          throw new Error("Stored order department could not be changed to current department");
+        const isCurrentDepartment = await ensureCurrentOrderDepartment(selectedDepartmentId);
+        if (!isCurrentDepartment) {
+          throw new Error("Stored order department does not match current department");
         }
       } else {
         throw new Error("Stored order department does not match current department");

tests/unit/pos-department-process.spec.js

@@ -389,7 +389,7 @@ describe("POSDepartmentProcess.restoreStoredPosOrderId", () => {
     expect(localStorage.getItem("pos_order_id")).toBeNull();
   });
 
-  it("moves a stored current order to the selected department when requested", async () => {
+  it("rejects a stored current order from a different department instead of moving it", async () => {
     localStorage.setItem("pos_order_id", "51211");
     department_id.value = 2;
     SessionUser.objects.orders.get.single.mockResolvedValue({
@@ -399,7 +399,6 @@ describe("POSDepartmentProcess.restoreStoredPosOrderId", () => {
       completed_at: null,
     });
     SessionUser.objects.orders.functions.get_department_id.mockResolvedValue(88);
-    SessionUser.objects.orders.set.department_id.mockResolvedValue({ data: { success: true } });
 
     await expect(
       restoreStoredPosOrderId({
@@ -408,12 +407,11 @@ describe("POSDepartmentProcess.restoreStoredPosOrderId", () => {
         departmentId: 2,
         syncDepartment: true,
       })
-    ).resolves.toBe(51211);
+    ).resolves.toBeNull();
 
-    expect(SessionUser.objects.orders.set.department_id).toHaveBeenCalledWith(51211, 2);
-    expect(order_id.value).toBe(51211);
-    expect(department_id.value).toBe(2);
-    expect(localStorage.getItem("pos_order_id")).toBe("51211");
+    expect(SessionUser.objects.orders.set.department_id).not.toHaveBeenCalled();
+    expect(order_id.value).toBeNull();
+    expect(localStorage.getItem("pos_order_id")).toBeNull();
   });
 });
 
@@ -429,15 +427,25 @@ describe("POSDepartmentProcess.createOrder department sync", () => {
     localStorage.setItem("token", "test-token");
   });
 
-  it("updates an existing current order to the selected department instead of creating a new order", async () => {
+  it("rejects an existing current order from a different department instead of updating it", async () => {
     order_id.value = 9201;
     department_id.value = 7;
     SessionUser.objects.orders.functions.get_department_id.mockResolvedValue(3);
-    SessionUser.objects.orders.set.department_id.mockResolvedValue({ data: { success: true } });
+
+    await expect(createOrder({ isMobile: true })).resolves.toBe(false);
+
+    expect(SessionUser.objects.orders.set.department_id).not.toHaveBeenCalled();
+    expect(axios.post).not.toHaveBeenCalled();
+  });
+
+  it("reuses an existing current order when it already belongs to the selected department", async () => {
+    order_id.value = 9201;
+    department_id.value = 7;
+    SessionUser.objects.orders.functions.get_department_id.mockResolvedValue(7);
 
     await expect(createOrder({ isMobile: true })).resolves.toBe(true);
 
-    expect(SessionUser.objects.orders.set.department_id).toHaveBeenCalledWith(9201, 7);
+    expect(SessionUser.objects.orders.set.department_id).not.toHaveBeenCalled();
     expect(axios.post).not.toHaveBeenCalled();
   });