fix: Move collection index validation logic to a context assert function (#2116)

* Abstract validation logic for readability

* Add index validation in collections.move

* Add tests

server/api/collections.js

@@ -67,12 +67,10 @@ router.post("collections.create", auth(), async (ctx) => {
   });
 
   if (index) {
-    const allowedASCII = new RegExp(/^[\x21-\x7E]+$/);
-    if (!allowedASCII.test(index)) {
-      throw new ValidationError(
+    ctx.assertIndexCharacters(
+      index,
       "Index characters must be between x21 to x7E ASCII"
     );
-    }
   } else {
     index = fractionalIndex(
       null,
@@ -664,6 +662,10 @@ router.post("collections.move", auth(), async (ctx) => {
   let index = ctx.body.index;
 
   ctx.assertPresent(index, "index is required");
+  ctx.assertIndexCharacters(
+    index,
+    "Index characters must be between x21 to x7E ASCII"
+  );
   ctx.assertUuid(id, "id must be a uuid");
 
   const user = ctx.state.user;

server/api/collections.test.js

@@ -162,6 +162,15 @@ describe("#collections.move", () => {
     expect(body.success).toBe(true);
   });
 
+  it("should return error when index is not valid", async () => {
+    const { admin, collection } = await seed();
+    const res = await server.post("/api/collections.move", {
+      body: { token: admin.getJwtToken(), id: collection.id, index: "يونيكود" },
+    });
+
+    expect(res.status).toEqual(400);
+  });
+
   it("if index collision occurs, should updated index of other collection", async () => {
     const { user, admin, collection } = await seed();
     const createdCollectionResponse = await server.post(
@@ -1019,6 +1028,14 @@ describe("#collections.create", () => {
     expect(body.policies[0].abilities.export).toBeTruthy();
   });
 
+  it("should error when index is invalid", async () => {
+    const user = await buildUser();
+    const res = await server.post("/api/collections.create", {
+      body: { token: user.getJwtToken(), name: "Test", index: "يونيكود" },
+    });
+    expect(res.status).toEqual(400);
+  });
+
   it("should allow setting sharing to false", async () => {
     const { user } = await seed();
     const res = await server.post("/api/collections.create", {

server/middlewares/validation.js

@@ -2,6 +2,7 @@
 import { type Context } from "koa";
 import validator from "validator";
 import { validateColorHex } from "../../shared/utils/color";
+import { validateIndexCharacters } from "../../shared/utils/indexCharacters";
 import { ParamRequiredError, ValidationError } from "../errors";
 
 export default function validation() {
@@ -60,6 +61,11 @@ export default function validation() {
       }
     };
 
+    ctx.assertIndexCharacters = (value, message) => {
+      if (!validateIndexCharacters(value)) {
+        throw new ValidationError(message);
+      }
+    };
     return next();
   };
 }

shared/utils/indexCharacters.js

@@ -0,0 +1,4 @@
+// @flow
+
+export const validateIndexCharacters = (index: string) =>
+  /^[\x21-\x7E]+$/i.test(index);