fix: #IMPULS-6155, enforce objectstorage compliance

tests/src/test/js/it/scenarios/storage/_utils.ts

@@ -0,0 +1,93 @@
+import { describe } from "https://jslib.k6.io/k6chaijs/4.3.4.0/index.js";
+import http from "k6/http";
+import { FormData } from "https://jslib.k6.io/formdata/0.0.2/index.js";
+
+import {
+  BASE_URL,
+  getHeaders,
+  authenticateWeb,
+  Session,
+  Structure,
+  UserInfo,
+  createAndSetRole,
+  linkRoleToUsers,
+  createUserAndGetData,
+  createEmptyStructure,
+  activateUsers,
+} from '../../../node_modules/edifice-k6-commons/dist/index.js';
+
+/**
+ * Shared fixture and the route wrappers that stay local to the storage scenarios.
+ *
+ * These scenarios exercise org.entcore.common.storage.impl.S3Storage through the only surface k6 can reach:
+ * the workspace, directory and archive routes that call it. The workspace and archive wrappers now live in
+ * edifice-k6-commons — createFolderOrFail, copyDocument, copyDocuments, copiedIds, deleteDocument,
+ * deleteDocuments, getDocumentBase64, the responseType argument of downloadFile and the timeout argument of
+ * verifyExportFiles. What is left here is the fixture and the two directory routes, which have no helper
+ * there.
+ */
+
+export type StorageInitData = {
+  head: Structure;
+  user: UserInfo;
+}
+
+/** A structure with one activated teacher holding the workspace and archive workflows. */
+export function initStorageFixture(schoolName: string): StorageInitData {
+  let structure: Structure | null = null;
+  let user: UserInfo | null = null;
+  describe("[Storage-Init] Initialize data", () => {
+    <Session>authenticateWeb(__ENV.ADMC_LOGIN, __ENV.ADMC_PASSWORD);
+    structure = createEmptyStructure(`${schoolName}`, true);
+    user = createUserAndGetData({
+      firstName: "Storage " + Date.now(),
+      lastName: "User",
+      "type": "Teacher",
+      structureId: structure.id,
+      birthDate: "2020-01-01",
+      positionIds: []
+    });
+    activateUsers(structure);
+    const roles = [
+      createAndSetRole('Espace documentaire'),
+      createAndSetRole('Archive'),
+    ];
+    const groups = [`Teachers from group ${structure.name}.`];
+    for (const role of roles) {
+      linkRoleToUsers(structure, role, groups);
+    }
+  });
+  return { head: structure, user };
+}
+
+/** PUT /directory/avatar/:userId — every call goes through cleanAvatarCache, hence findByFilenameEndingWith. */
+export function updateAvatar(userId: string, documentId: string) {
+  return http.put(`${BASE_URL}/directory/avatar/${userId}`,
+      JSON.stringify({ picture: `/workspace/document/${documentId}` }),
+      { headers: getHeaders("application/json") });
+}
+
+/** GET /userbook/avatar/:id — serves the cached avatar, or redirects to the default one. */
+export function getAvatar(userId: string) {
+  return http.get(`${BASE_URL}/userbook/avatar/${userId}`,
+      { headers: getHeaders(), redirects: 0, responseType: "binary" });
+}
+
+/**
+ * POST /directory/massmessaging/column/mapping — uploads a CSV and asks for its column mapping. The service
+ * pulls the uploaded directory back from storage with copyDirectoryToFs before parsing it, so a storage
+ * failure surfaces as the "Failed to copy import files from storage to FS" error rather than as a mapping.
+ *
+ * The form fields mirror the ones edifice-k6-commons posts to /directory/wizard/import, which is a payload
+ * known to pass ImportInfos.validate.
+ */
+export function postMassMessagingColumnMapping(structureId: string, structureName: string, csv: ArrayBuffer) {
+  const form = new FormData();
+  form.append("type", "CSV");
+  form.append("structureName", structureName);
+  form.append("structureId", structureId);
+  form.append("Teacher", http.file(csv, "enseignants.csv"));
+  const headers = getHeaders();
+  headers["Content-Type"] = "multipart/form-data; boundary=" + form.boundary;
+  return http.post(`${BASE_URL}/directory/massmessaging/column/mapping`, form.body(), { headers });
+}

tests/src/test/js/it/scenarios/storage/avatar.ts

@@ -0,0 +1,160 @@
+import { describe } from "https://jslib.k6.io/k6chaijs/4.3.4.0/index.js";
+import { check, sleep } from "k6";
+import crypto from "k6/crypto";
+
+import {
+  authenticateWeb,
+  getConnectedUserId,
+  uploadFile,
+} from '../../../node_modules/edifice-k6-commons/dist/index.js';
+import {
+  StorageInitData,
+  initStorageFixture,
+  updateAvatar,
+  getAvatar,
+} from './_utils.ts';
+
+/**
+ * Storage.findByFilenameEndingWith, and the Storage.removeFiles that follows it.
+ *
+ * On S3Storage this is the only caller of S3Client.getObjectsEndingWith, which issues a ListObjectsV2 —
+ * "list-type=2&prefix=..." — so it is the one route that signs a request carrying several query parameters.
+ * That is exactly what IMPULS-6155 touches: the values used to go through URLEncoder rather than an RFC 3986
+ * encoder, and the parameter ordering was left to the caller.
+ *
+ * The trigger is an avatar update: DefaultUserBookService.update calls cleanAvatarCache whenever the payload
+ * carries a "picture", which lists the cached thumbnails of that user and removes them.
+ *
+ * How the listing gets pinned despite cleanAvatarCache swallowing its failures: the second update has to
+ * drop the avatars cached by the first one before caching its own. If the listing came back empty when it
+ * should not have, nothing would be removed and the route would keep serving the first picture — so the
+ * check that the served bytes changed between the two updates is what actually proves the listing found
+ * its keys. The route answering 200 alone would prove nothing.
+ *
+ * Caveat worth knowing before reading a failure: Directory builds the avatar storage as an S3Storage only
+ * when the directory config holds an "s3avatars" block, and falls back to FileStorage otherwise. Without
+ * that block this scenario still passes, but it exercises FileStorage — it says nothing about SigV4.
+ */
+
+const maxDuration = __ENV.MAX_DURATION || "5m";
+const schoolName = __ENV.DATA_SCHOOL_NAME || "General - One user - Storage avatar";
+const gracefulStop = parseInt(__ENV.GRACEFUL_STOP || "2s");
+
+export const options = {
+  setupTimeout: "1h",
+  thresholds: {
+    checks: ["rate == 1.00"],
+  },
+  scenarios: {
+    testFindByFilenameEndingWith: {
+      executor: "per-vu-iterations",
+      exec: "testFindByFilenameEndingWith",
+      vus: 1,
+      maxDuration: maxDuration,
+      gracefulStop,
+    },
+  }
+};
+
+const dataRootPath = __ENV.DATA_ROOT_PATH || "../../../../resources/data";
+
+let firstPicture: ArrayBuffer;
+let secondPicture: ArrayBuffer;
+try {
+  firstPicture = open(`${dataRootPath}/workspace/small.png`, "b");
+  secondPicture = open(`${dataRootPath}/workspace/big-picture.jpg`, "b");
+} catch (e) {
+  firstPicture = open(`${dataRootPath}/data/workspace/small.png`, "b");
+  secondPicture = open(`${dataRootPath}/data/workspace/big-picture.jpg`, "b");
+}
+
+/** The size of a k6 response body, whichever shape it came back in. */
+function bodyLength(body: any): number {
+  if (body == null) {
+    return 0;
+  }
+  if (typeof body === "string") {
+    return body.length;
+  }
+  return typeof body.byteLength === "number" ? body.byteLength : 0;
+}
+
+/**
+ * Polls the avatar route until it serves something, and until the extra condition holds. The caching runs
+ * off the request, through the image resizer, so how long it takes is not ours to know — a fixed sleep
+ * either flakes or wastes time.
+ */
+function awaitAvatar(userId: string, until?: (r: any) => boolean, timeoutSeconds = 30): any {
+  for (let waited = 0; waited < timeoutSeconds; waited++) {
+    const res = getAvatar(userId);
+    if (res.status === 200 && bodyLength(res.body) > 0 && (until == null || until(res))) {
+      return res;
+    }
+    sleep(1);
+  }
+  console.error(`Avatar never settled for ${userId} within ${timeoutSeconds}s`);
+  return null;
+}
+
+export function setup(): StorageInitData {
+  return initStorageFixture(schoolName);
+}
+
+export function testFindByFilenameEndingWith(data: StorageInitData) {
+  describe('[Storage] List and drop the cached avatars of a user', () => {
+    authenticateWeb(data.user.login);
+    const userId = getConnectedUserId() as string;
+
+    // First update: the cache is empty, so the listing matches nothing and only the caching runs.
+    const uploaded = uploadFile(firstPicture, "avatar.png");
+    let res = updateAvatar(userId, uploaded._id);
+    let ok = check(res, {
+      "the first avatar update should be ok": (r) => r.status === 200,
+      "the first avatar update should echo the picture": (r) => {
+        const picture = r.json("picture");
+        return typeof picture === "string" && picture.indexOf(uploaded._id) >= 0;
+      },
+    });
+    if (!ok) {
+      console.error(`First avatar update failed: ${res.status} - ${res.body}`);
+      return;
+    }
+
+    // The thumbnails are cached asynchronously, so poll rather than sleep a fixed amount.
+    const first = awaitAvatar(userId);
+    check(first, {
+      "the avatar should be served after the first update": (r) => r != null && r.status === 200,
+      "the served avatar should have content": (r) => r != null && bodyLength(r.body) > 0,
+    });
+
+    // Second update: this is the interesting one. The cache now holds objects whose keys end with the user
+    // id, so findByFilenameEndingWith has to list them and removeFiles has to drop them. A ListObjectsV2
+    // that came back rejected leaves the response a 200 all the same, so what the check below pins is that
+    // the update completed and the avatar is still served afterwards.
+    const replacement = uploadFile(secondPicture, "avatar-2.jpg");
+    res = updateAvatar(userId, replacement._id);
+    ok = check(res, {
+      "the second avatar update should be ok": (r) => r.status === 200,
+      "the second avatar update should echo the new picture": (r) => {
+        const picture = r.json("picture");
+        return typeof picture === "string" && picture.indexOf(replacement._id) >= 0;
+      },
+    });
+    if (!ok) {
+      console.error(`Second avatar update failed: ${res.status} - ${res.body}`);
+      return;
+    }
+
+    const second = awaitAvatar(userId, (r) =>
+        first == null || crypto.sha256(r.body as ArrayBuffer, "hex") !==
+            crypto.sha256(first.body as ArrayBuffer, "hex"));
+    check(second, {
+      "the avatar should still be served after the replacement": (r) => r != null && r.status === 200,
+      // The pin on findByFilenameEndingWith: the first picture had to be listed and removed for this to
+      // be the second one. A listing that came back empty would leave the first avatar in place.
+      "the served avatar should be the replacement, not the first one": (r) =>
+          r != null && first != null &&
+          crypto.sha256(r.body as ArrayBuffer, "hex") !== crypto.sha256(first.body as ArrayBuffer, "hex"),
+    });
+  });
+}

tests/src/test/js/it/scenarios/storage/copy.ts

@@ -0,0 +1,136 @@
+import { describe } from "https://jslib.k6.io/k6chaijs/4.3.4.0/index.js";
+import { check } from "k6";
+import crypto from "k6/crypto";
+
+import {
+  authenticateWeb,
+  uploadFile,
+  downloadFile,
+  createFolderOrFail,
+  copyDocument,
+  copyDocuments,
+  copiedIds,
+} from '../../../node_modules/edifice-k6-commons/dist/index.js';
+import {
+  StorageInitData,
+  initStorageFixture,
+} from './_utils.ts';
+
+/**
+ * Storage.copyFile — S3Client.copyFile, the CopyObject request carrying x-amz-copy-source.
+ *
+ * That header is the one SUPPORT-4854 fixed (it used to be set after the signature, so S3 rejected the
+ * request naming it as unsigned) and the one IMPULS-6155 now percent encodes. Nothing exercised it end to
+ * end: no k6 scenario copies a document, and the workspace copy route is the only way in.
+ */
+
+const maxDuration = __ENV.MAX_DURATION || "5m";
+const schoolName = __ENV.DATA_SCHOOL_NAME || "General - One user - Storage copy";
+const gracefulStop = parseInt(__ENV.GRACEFUL_STOP || "2s");
+
+export const options = {
+  setupTimeout: "1h",
+  thresholds: {
+    checks: ["rate == 1.00"],
+  },
+  scenarios: {
+    testCopyDocument: {
+      executor: "per-vu-iterations",
+      exec: "testCopyDocument",
+      vus: 1,
+      maxDuration: maxDuration,
+      gracefulStop,
+    },
+    testCopyDocumentsInBulk: {
+      executor: "per-vu-iterations",
+      exec: "testCopyDocumentsInBulk",
+      vus: 1,
+      maxDuration: maxDuration,
+      gracefulStop,
+    },
+  }
+};
+
+const dataRootPath = __ENV.DATA_ROOT_PATH || "../../../../resources/data";
+
+let fileToUpload: ArrayBuffer;
+try {
+  fileToUpload = open(`${dataRootPath}/workspace/small.png`, "b");
+} catch (e) {
+  fileToUpload = open(`${dataRootPath}/data/workspace/small.png`, "b");
+}
+
+export function setup(): StorageInitData {
+  return initStorageFixture(schoolName);
+}
+
+export function testCopyDocument(data: StorageInitData) {
+  describe('[Storage] Copy one document', () => {
+    authenticateWeb(data.user.login);
+
+    const original = uploadFile(fileToUpload, "small.png");
+    const folderId = createFolderOrFail("Copies " + Date.now());
+
+    const copyRes = copyDocument(original._id, folderId);
+    let ok = check(copyRes, {
+      "copy should be ok": (r) => r.status === 200,
+      "copy should return one document": (r) => copiedIds(r).length === 1,
+    });
+    if (!ok) {
+      console.error(`Copy failed: ${copyRes.status} - ${copyRes.body}`);
+      return;
+    }
+    const copyId = copiedIds(copyRes)[0];
+
+    check(copyId, {
+      "the copy should be a distinct document": (id) => id !== original._id,
+    });
+
+    // The point of the scenario: the copy is readable, and holds the very same bytes. A CopyObject that
+    // silently failed, or copied the wrong key, shows up here and nowhere else.
+    const originalBytes = downloadFile(original._id, "", "binary");
+    const copyBytes = downloadFile(copyId, "", "binary");
+    ok = check({ originalBytes, copyBytes }, {
+      "original should still download": (r) => r.originalBytes.status === 200,
+      "copy should download": (r) => r.copyBytes.status === 200,
+      "copy should have the same size": (r) =>
+          (r.copyBytes.body as ArrayBuffer).byteLength === (r.originalBytes.body as ArrayBuffer).byteLength,
+      "copy should be byte for byte identical": (r) =>
+          crypto.sha256(r.copyBytes.body as ArrayBuffer, "hex") ===
+          crypto.sha256(r.originalBytes.body as ArrayBuffer, "hex"),
+    });
+    if (!ok) {
+      console.error(`Original status ${originalBytes.status}, copy status ${copyBytes.status}`);
+    }
+  });
+}
+
+export function testCopyDocumentsInBulk(data: StorageInitData) {
+  describe('[Storage] Copy several documents', () => {
+    authenticateWeb(data.user.login);
+
+    const first = uploadFile(fileToUpload, "first.png");
+    const second = uploadFile(fileToUpload, "second.png");
+    const folderId = createFolderOrFail("Bulk copies " + Date.now());
+
+    const copyRes = copyDocuments([first._id, second._id], folderId);
+    const ok = check(copyRes, {
+      "bulk copy should be ok": (r) => r.status === 200,
+      "bulk copy should return two documents": (r) => copiedIds(r).length === 2,
+    });
+    if (!ok) {
+      console.error(`Bulk copy failed: ${copyRes.status} - ${copyRes.body}`);
+      return;
+    }
+
+    // Each copy triggers its own CopyObject: one signature per file, so a header set after the signature
+    // fails on all of them rather than on the first only.
+    for (const copyId of copiedIds(copyRes)) {
+      const res = downloadFile(copyId, "", "binary");
+      check(res, {
+        "each copy should download": (r) => r.status === 200,
+        "each copy should have content": (r) => (r.body as ArrayBuffer).byteLength > 0,
+      });
+    }
+  });
+}