From 1a0896475cf9d2af5cd03b85159ccbb2fe0a3d52 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Mon, 30 Sep 2024 19:03:21 +0400 Subject: [PATCH] fix: added new identifier field for non-uuid IDs --- ...20240930134623_secret-sharing-string-id.ts | 58 ++---------------- backend/src/db/schemas/secret-sharing.ts | 3 +- .../secret-sharing/secret-sharing-dal.ts | 12 +--- .../secret-sharing/secret-sharing-service.ts | 60 +++++++++++++++---- 4 files changed, 55 insertions(+), 78 deletions(-) diff --git a/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts b/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts index 69b8f7a83..10d0da660 100644 --- a/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts +++ b/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/ban-ts-comment */ import { Knex } from "knex"; import { TableName } from "../schemas"; @@ -6,31 +5,10 @@ import { TableName } from "../schemas"; export async function up(knex: Knex): Promise { if (await knex.schema.hasTable(TableName.SecretSharing)) { await knex.schema.alterTable(TableName.SecretSharing, (t) => { - // Add a new column - t.string("new_id", 36).nullable(); - }); + t.string("identifier", 36).nullable(); - // Copy data from old column to new column - await knex(TableName.SecretSharing).update({ - // @ts-ignore - new_id: knex.raw("id::text") - }); - - await knex.schema.alterTable(TableName.SecretSharing, (t) => { - // Make the new column not nullable - t.string("new_id", 36).notNullable().alter(); - - // Drop the old primary key - t.dropPrimary(); - - // Drop the old id column - t.dropColumn("id"); - - // Rename the new column to 'id' - t.renameColumn("new_id", "id"); - - // Set the new column as primary key - t.primary(["id"]); + t.unique("identifier"); + t.index("identifier"); }); } } @@ -38,34 +16,8 @@ export async function up(knex: Knex): Promise { export async function down(knex: Knex): Promise { if (await knex.schema.hasTable(TableName.SecretSharing)) { await knex.schema.alterTable(TableName.SecretSharing, (t) => { - // Add a new UUID column - t.uuid("new_id").nullable(); - }); - - // Copy data from string id to UUID, ensuring valid UUID format - await knex(TableName.SecretSharing).update({ - // @ts-ignore - new_id: knex.raw("id::uuid") - }); - - await knex.schema.alterTable(TableName.SecretSharing, (t) => { - // Make the new column not nullable - t.uuid("new_id").notNullable().alter(); - - // Drop the old primary key - t.dropPrimary(); - - // Drop the old id column - t.dropColumn("id"); - - // Rename the new column to 'id' - t.renameColumn("new_id", "id"); - - // Set the new column as primary key - t.primary(["id"]); - - // Set the id column to use a UUID default - t.uuid("id").defaultTo(knex.raw("gen_random_uuid()")).notNullable().alter(); + // If rolled back, all secrets created with this new structure will stop working. + t.dropColumn("identifier"); }); } } diff --git a/backend/src/db/schemas/secret-sharing.ts b/backend/src/db/schemas/secret-sharing.ts index 7490269d7..0b78b036f 100644 --- a/backend/src/db/schemas/secret-sharing.ts +++ b/backend/src/db/schemas/secret-sharing.ts @@ -10,6 +10,7 @@ import { zodBuffer } from "@app/lib/zod"; import { TImmutableDBKeys } from "./models"; export const SecretSharingSchema = z.object({ + id: z.string().uuid(), encryptedValue: z.string().nullable().optional(), iv: z.string().nullable().optional(), tag: z.string().nullable().optional(), @@ -25,7 +26,7 @@ export const SecretSharingSchema = z.object({ lastViewedAt: z.date().nullable().optional(), password: z.string().nullable().optional(), encryptedSecret: zodBuffer.nullable().optional(), - id: z.string() + identifier: z.string().nullable().optional() }); export type TSecretSharing = z.infer; diff --git a/backend/src/services/secret-sharing/secret-sharing-dal.ts b/backend/src/services/secret-sharing/secret-sharing-dal.ts index f6108b3ff..5c690b266 100644 --- a/backend/src/services/secret-sharing/secret-sharing-dal.ts +++ b/backend/src/services/secret-sharing/secret-sharing-dal.ts @@ -82,21 +82,11 @@ export const secretSharingDALFactory = (db: TDbClient) => { } }; - const create = async (data: Omit, tx?: Knex) => { - try { - const [res] = await (tx || db)(TableName.SecretSharing).insert(data).returning("*"); - return res; - } catch (error) { - throw new DatabaseError({ error, name: "Create Shared Secret" }); - } - }; - return { ...sharedSecretOrm, countAllUserOrgSharedSecrets, pruneExpiredSharedSecrets, softDeleteById, - findActiveSharedSecrets, - create + findActiveSharedSecrets }; }; diff --git a/backend/src/services/secret-sharing/secret-sharing-service.ts b/backend/src/services/secret-sharing/secret-sharing-service.ts index 0aa89dda1..3509fc87f 100644 --- a/backend/src/services/secret-sharing/secret-sharing-service.ts +++ b/backend/src/services/secret-sharing/secret-sharing-service.ts @@ -1,6 +1,7 @@ import crypto from "node:crypto"; import bcrypt from "bcrypt"; +import { z } from "zod"; import { TSecretSharing } from "@app/db/schemas"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; @@ -27,6 +28,8 @@ type TSecretSharingServiceFactoryDep = { export type TSecretSharingServiceFactory = ReturnType; +const isUuidV4 = (uuid: string) => z.string().uuid().safeParse(uuid).success; + export const secretSharingServiceFactory = ({ permissionService, secretSharingDAL, @@ -76,7 +79,7 @@ export const secretSharingServiceFactory = ({ const hashedPassword = password ? await bcrypt.hash(password, 10) : null; const newSharedSecret = await secretSharingDAL.create({ - id, + identifier: id, iv: null, tag: null, encryptedValue: null, @@ -91,7 +94,7 @@ export const secretSharingServiceFactory = ({ accessType }); - return { id: `${newSharedSecret.id}${hashedHex}` }; + return { id: `${newSharedSecret.identifier}${hashedHex}` }; }; const createPublicSharedSecret = async ({ @@ -126,7 +129,7 @@ export const secretSharingServiceFactory = ({ const hashedPassword = password ? await bcrypt.hash(password, 10) : null; const newSharedSecret = await secretSharingDAL.create({ - id, + identifier: id, encryptedValue: null, iv: null, tag: null, @@ -139,7 +142,7 @@ export const secretSharingServiceFactory = ({ accessType }); - return { id: `${newSharedSecret.id}${hashedHex}` }; + return { id: `${newSharedSecret.identifier}${hashedHex}` }; }; const getSharedSecrets = async ({ @@ -183,22 +186,44 @@ export const secretSharingServiceFactory = ({ const $decrementSecretViewCount = async (sharedSecret: TSecretSharing, sharedSecretId: string) => { const { expiresAfterViews } = sharedSecret; + const isUuid = isUuidV4(sharedSecretId); + if (expiresAfterViews) { // decrement view count if view count expiry set - await secretSharingDAL.updateById(sharedSecretId, { $decr: { expiresAfterViews: 1 } }); + + if (isUuid) { + await secretSharingDAL.updateById(sharedSecretId, { $decr: { expiresAfterViews: 1 } }); + } else { + await secretSharingDAL.update({ identifier: sharedSecretId }, { $decr: { expiresAfterViews: 1 } }); + } } - await secretSharingDAL.updateById(sharedSecretId, { - lastViewedAt: new Date() - }); + if (isUuid) { + await secretSharingDAL.updateById(sharedSecretId, { + lastViewedAt: new Date() + }); + } else { + await secretSharingDAL.update( + { identifier: sharedSecretId }, + { + lastViewedAt: new Date() + } + ); + } }; - /** Get's passwordless secret. validates all secret's requested (must be fresh). */ + /** Get's password-less secret. validates all secret's requested (must be fresh). */ const getSharedSecretById = async ({ sharedSecretId, hashedHex, orgId, password }: TGetActiveSharedSecretByIdDTO) => { - const sharedSecret = await secretSharingDAL.findOne({ - id: sharedSecretId, - hashedHex - }); + const sharedSecret = isUuidV4(sharedSecretId) + ? await secretSharingDAL.findOne({ + id: sharedSecretId, + hashedHex + }) + : await secretSharingDAL.findOne({ + hashedHex, + identifier: sharedSecretId + }); + if (!sharedSecret) throw new NotFoundError({ message: "Shared secret not found" @@ -269,7 +294,16 @@ export const secretSharingServiceFactory = ({ const { actor, actorId, orgId, actorAuthMethod, actorOrgId, sharedSecretId } = deleteSharedSecretInput; const { permission } = await permissionService.getOrgPermission(actor, actorId, orgId, actorAuthMethod, actorOrgId); if (!permission) throw new ForbiddenRequestError({ name: "User does not belong to the specified organization" }); + const deletedSharedSecret = await secretSharingDAL.deleteById(sharedSecretId); + + const sharedSecret = isUuidV4(sharedSecretId) + ? await secretSharingDAL.findById(sharedSecretId) + : await secretSharingDAL.findOne({ identifier: sharedSecretId }); + + if (sharedSecret.orgId && sharedSecret.orgId !== orgId) + throw new ForbiddenRequestError({ message: "User does not have permission to delete shared secret" }); + return deletedSharedSecret; };