✅♻️🩹 backend: add company_id to editing_session_key (#525)

tdrive/backend/node/src/services/documents/entities/drive-file.ts

@@ -4,6 +4,7 @@ import { Column, Entity } from "../../../core/platform/services/database/service
 import { DriveFileAccessLevel, publicAccessLevel } from "../types";
 import { FileVersion } from "./file-version";
 import search from "./drive-file.search";
+import * as UUIDTools from "../../../utils/uuid";
 
 export const TYPE = "drive_files";
 export type DriveScope = "personal" | "shared";
@@ -96,7 +97,8 @@ export class DriveFile {
    * If this field is non-null, then an editing session is in progress (probably in OnlyOffice).
    * Use {@see EditingSessionKeyFormat} to generate and interpret it.
    * Values should ensure that sorting lexicographically is chronological (assuming perfect clocks everywhere),
-   * and that the application and user that started the edit session are retrievable.
+   * and that the application, company and user that started the edit session are retrievable.
+   * It is not encrypted.
    */
   @Type(() => String)
   @Column("editing_session_key", "string")
@@ -122,32 +124,45 @@ export class DriveFile {
   scope: DriveScope;
 }
 
+const OnlyOfficeSafeDocKeyBase64 = {
+  // base64 uses `+/`, but base64url uses `-_` instead. Both use `=` as padding,
+  // which conflicts with EditingSessionKeyFormat so using `.` instead.
+  fromBuffer(buffer: Buffer) {
+    return buffer.toString("base64url").replace(/=/g, ".");
+  },
+  toBuffer(base64: string) {
+    return Buffer.from(base64.replace(/\./g, "="), "base64url");
+  },
+};
+
 /** Reference implementation for generating then parsing the {@link DriveFile.editing_session_key} field */
 export const EditingSessionKeyFormat = {
-  // OnlyOffice key limits: 128 chars, [0-9a-zA-z=_-]
+  // OnlyOffice key limits: 128 chars, [0-9a-zA-Z.=_-]
+  // See https://api.onlyoffice.com/editors/config/document#key
   // This is specific to it, but the constraint seems strict enough
   // that any other system needing such a unique identifier would find
   // this compatible. This value must be ensured to be the strictest
   // common denominator to all plugin/interop systems. Plugins that
   // require something even stricter have the option of maintaining
   // a look up table to an acceptable value.
-  generate(applicationId: string, userId: string) {
+  generate(applicationId: string, companyId: string, userId: string, overrideTimeStamp?: Date) {
     if (!/^[0-9a-zA-Z_-]+$/m.test(applicationId))
       throw new Error(
         `Invalid applicationId string (${JSON.stringify(
           applicationId,
         )}). Must be short and only alpha numeric`,
       );
-    const isoUTCDateNoSpecialCharsNoMS = new Date()
+    const isoUTCDateNoSpecialCharsNoMS = (overrideTimeStamp ?? new Date())
       .toISOString()
       .replace(/\..+$/, "")
       .replace(/[ZT:-]/g, "");
-    const newKey = [
-      isoUTCDateNoSpecialCharsNoMS,
-      applicationId,
-      userId.replace(/-+/g, ""),
-      randomUUID().replace(/-+/g, ""),
-    ].join("=");
+    const userIdBuffer = UUIDTools.bufferFromUUIDString(userId) as unknown as Uint8Array;
+    const companyIdBuffer = UUIDTools.bufferFromUUIDString(companyId) as unknown as Uint8Array;
+    const entropyBuffer = UUIDTools.bufferFromUUIDString(randomUUID()) as unknown as Uint8Array;
+    const idsString = OnlyOfficeSafeDocKeyBase64.fromBuffer(
+      Buffer.concat([companyIdBuffer, userIdBuffer, entropyBuffer]),
+    );
+    const newKey = [isoUTCDateNoSpecialCharsNoMS, applicationId, idsString].join("=");
     if (newKey.length > 128 || !/^[0-9a-zA-Z=_-]+$/m.test(newKey))
       throw new Error(
         `Invalid generated editingSessionKey (${JSON.stringify(
@@ -159,14 +174,14 @@ export const EditingSessionKeyFormat = {
 
   parse(editingSessionKey: string) {
     const parts = editingSessionKey.split("=");
-    const expectedParts = 4;
+    const expectedParts = 3;
     if (parts.length !== expectedParts)
       throw new Error(
         `Invalid editingSessionKey (${JSON.stringify(
           editingSessionKey,
         )}). Expected ${expectedParts} parts`,
       );
-    const [timestampStr, appId, userId, _random] = parts;
+    const [timestampStr, appId, idsOOBase64String] = parts;
     const timestampMatch = timestampStr.match(
       /^(?<year>\d{4})(?<month>\d\d)(?<day>\d\d)(?<hour>\d\d)(?<minute>\d\d)(?<second>\d\d)$/,
     );
@@ -177,22 +192,16 @@ export const EditingSessionKeyFormat = {
         )}). Didn't start with valid timestamp`,
       );
     const { year, month, day, hour, minute, second } = timestampMatch.groups!;
-    const userIdMatch = userId.match(
-      /^([a-z0-f]{8})([a-z0-f]{4})([a-z0-f]{4})([a-z0-f]{4})([a-z0-f]{12})$/i,
-    );
-    if (!userIdMatch)
-      throw new Error(
-        `Invalid editingSessionKey (${JSON.stringify(
-          editingSessionKey,
-        )}). UserID has wrong number of digits`,
-      );
-    const [, userIdPart1, userIdPart2, userIdPart3, userIdPart4, userIdPart5] = userIdMatch;
+    const idsBuffer = OnlyOfficeSafeDocKeyBase64.toBuffer(idsOOBase64String);
+    const companyId = UUIDTools.formattedUUIDInBufferArray(idsBuffer, 0);
+    const userId = UUIDTools.formattedUUIDInBufferArray(idsBuffer, 1);
     return {
       timestamp: new Date(
         Date.parse(`${[year, month, day].join("-")}T${[hour, minute, second].join(":")}Z`),
       ),
       applicationId: appId,
-      userId: [userIdPart1, userIdPart2, userIdPart3, userIdPart4, userIdPart5].join("-"),
+      companyId,
+      userId,
     };
   },
 };

tdrive/backend/node/src/services/documents/services/index.ts

@@ -976,7 +976,11 @@ export class DocumentsService {
 
     let newKey: string;
     try {
-      newKey = EditingSessionKeyFormat.generate(editorApplicationId, context.user.id);
+      newKey = EditingSessionKeyFormat.generate(
+        editorApplicationId,
+        context.company.id,
+        context.user.id,
+      );
     } catch (e) {
       CrudException.throwMe(e, new CrudException("Error generating new editing_session_key", 500));
     }

tdrive/backend/node/src/utils/uuid.ts

@@ -12,3 +12,35 @@ export function timeuuidToDate(time_str: string): number {
 
   return parseInt(time_string, 16);
 }
+
+/** Remove `-`s from a formatted UUID to get a hex a string */
+export function hexFromFormatted(uuid: string) {
+  const result = uuid.replace(/-+/g, "");
+  if (result.length !== 32)
+    throw new Error(`Invalid UUID (${JSON.stringify(uuid)}). Wrong number of digits`);
+  return result;
+}
+
+/** Add `-`s back in a hex string to make a formatted UUID */
+export function formattedFromHex(hex: string) {
+  const idMatch = hex.match(
+    /^([a-z0-f]{8})([a-z0-f]{4})([a-z0-f]{4})([a-z0-f]{4})([a-z0-f]{12})$/i,
+  );
+  if (!idMatch)
+    throw new Error(`Invalid UUID hex (${JSON.stringify(hex)}). Wrong number of digits`);
+  const [, ...parts] = idMatch;
+  return parts.join("-");
+}
+
+/** Convert a UUID formatted or hex string into a binary buffer */
+export function bufferFromUUIDString(uuidOrHex: string) {
+  return Buffer.from(hexFromFormatted(uuidOrHex), "hex");
+}
+
+/** In a buffer with concatenated UUIDs in binary, extract the `index`th and return as formatted UUID */
+export function formattedUUIDInBufferArray(buffer: Buffer, index: number) {
+  if (buffer.length < (index + 1) * 16)
+    throw new Error(`Cannot get UUID ${JSON.stringify(index)} because the buffer is too small`);
+  const slice = buffer.subarray(index * 16, (index + 1) * 16);
+  return formattedFromHex(slice.toString("hex").toLowerCase());
+}

tdrive/backend/node/test/unit/core/services/documents/drive-file-editing-sessions-key.test.ts

@@ -0,0 +1,53 @@
+import "reflect-metadata";
+import { expect, jest, test, describe, beforeEach, afterEach } from "@jest/globals";
+import { randomUUID } from "crypto";
+import { EditingSessionKeyFormat } from "../../../../../src/services/documents/entities/drive-file";
+
+describe('DriveFile EditingSessionKeyFormat', () => {
+  const mockAppId = 'tdrive_random_application_id';
+  const mockCompanyId = randomUUID();
+  const mockUserId = randomUUID();
+  const mockTimestamp = new Date();
+  const mockTimestampWith0MS = mockTimestamp.getTime() - (mockTimestamp.getTime() % 1000);
+
+  const checkIsUUID = (value: string) => {
+    expect(value).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i);
+    expect(value).not.toMatch(/^0{8}-0{4}-0{4}-0{4}-0{12}$/i);
+  };
+
+  const checkKeyIsOOCompatible = (key: string) => {
+    // OnlyOffice key limits: see https://api.onlyoffice.com/editors/config/document#key
+    expect(key).toMatch(/^[0-9a-zA-Z._=-]{1,128}$/);
+  };
+
+  test('generates a valid value that can be parsed', async () => {
+    checkIsUUID(mockUserId);
+    checkIsUUID(mockCompanyId);
+    const key = EditingSessionKeyFormat.generate(mockAppId, mockCompanyId, mockUserId, mockTimestamp);
+    checkKeyIsOOCompatible(key);
+    const parsed = EditingSessionKeyFormat.parse(key);
+    expect(parsed.applicationId).toBe(mockAppId);
+    expect(parsed.userId).toBe(mockUserId);
+    expect(parsed.companyId).toBe(mockCompanyId);
+    expect(parsed.timestamp.getTime()).toBe(mockTimestampWith0MS);
+  });
+
+  test('generates unique values', async () => {
+    const key = EditingSessionKeyFormat.generate(mockAppId, mockCompanyId, mockUserId, mockTimestamp);
+    const key2 = EditingSessionKeyFormat.generate(mockAppId, mockCompanyId, mockUserId, mockTimestamp);
+    expect(key).not.toBe(key2);
+  });
+
+  test('checks the appId', async () => {
+    expect(() => {
+      EditingSessionKeyFormat.generate('invalid app id !', mockCompanyId, mockUserId);
+    }).toThrow('Invalid applicationId string');
+  });
+
+  test('checks final length', async () => {
+    expect(() => {
+      const tooLongAppID = new Array(100).join('x');
+      EditingSessionKeyFormat.generate(tooLongAppID, mockCompanyId, mockUserId);
+    }).toThrow('Must be <128 chars,');
+  });
+});

tdrive/connectors/onlyoffice-connector/src/app.ts

@@ -41,7 +41,7 @@ class App {
     });
 
     routes.forEach(route => {
-      this.app.use(SERVER_PREFIX, route.router);
+      this.app.use(route.path ?? '/', route.router);
     });
 
     this.app.get('/health', (_req, res) => {