From b65842f5c1068cbfeaa410cd7cb4cb646c912543 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Tue, 1 Oct 2024 00:16:18 +0400 Subject: [PATCH] fix: requested changes --- .../20240925100349_managed-secret-sharing.ts | 11 +++++---- ...20240930134623_secret-sharing-string-id.ts | 23 ------------------ backend/src/db/schemas/secret-sharing.ts | 2 +- .../server/routes/v1/secret-sharing-router.ts | 2 +- .../secret-sharing/secret-sharing-service.ts | 24 +++++++------------ .../secret-sharing/secret-sharing-types.ts | 2 +- .../src/hooks/api/secretSharing/queries.ts | 8 +++---- .../ViewSecretPublicPage.tsx | 19 ++++----------- 8 files changed, 28 insertions(+), 63 deletions(-) delete mode 100644 backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts diff --git a/backend/src/db/migrations/20240925100349_managed-secret-sharing.ts b/backend/src/db/migrations/20240925100349_managed-secret-sharing.ts index 25871455a..56784d314 100644 --- a/backend/src/db/migrations/20240925100349_managed-secret-sharing.ts +++ b/backend/src/db/migrations/20240925100349_managed-secret-sharing.ts @@ -10,6 +10,11 @@ export async function up(knex: Knex): Promise { t.string("encryptedValue").nullable().alter(); t.binary("encryptedSecret").nullable(); + t.string("hashedHex").nullable().alter(); + + t.string("identifier", 64).nullable(); + t.unique("identifier"); + t.index("identifier"); }); } } @@ -17,11 +22,9 @@ 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) => { - t.string("iv").notNullable().alter(); - t.string("tag").notNullable().alter(); - t.string("encryptedValue").notNullable().alter(); - t.dropColumn("encryptedSecret"); + + t.dropColumn("identifier"); }); } } diff --git a/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts b/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts deleted file mode 100644 index 10d0da660..000000000 --- a/backend/src/db/migrations/20240930134623_secret-sharing-string-id.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { Knex } from "knex"; - -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) => { - t.string("identifier", 36).nullable(); - - t.unique("identifier"); - t.index("identifier"); - }); - } -} - -export async function down(knex: Knex): Promise { - if (await knex.schema.hasTable(TableName.SecretSharing)) { - await knex.schema.alterTable(TableName.SecretSharing, (t) => { - // 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 0b78b036f..d47f288b2 100644 --- a/backend/src/db/schemas/secret-sharing.ts +++ b/backend/src/db/schemas/secret-sharing.ts @@ -14,7 +14,7 @@ export const SecretSharingSchema = z.object({ encryptedValue: z.string().nullable().optional(), iv: z.string().nullable().optional(), tag: z.string().nullable().optional(), - hashedHex: z.string(), + hashedHex: z.string().nullable().optional(), expiresAt: z.date(), userId: z.string().uuid().nullable().optional(), orgId: z.string().uuid().nullable().optional(), diff --git a/backend/src/server/routes/v1/secret-sharing-router.ts b/backend/src/server/routes/v1/secret-sharing-router.ts index 214993138..fa4e9bf6a 100644 --- a/backend/src/server/routes/v1/secret-sharing-router.ts +++ b/backend/src/server/routes/v1/secret-sharing-router.ts @@ -58,7 +58,7 @@ export const registerSecretSharingRouter = async (server: FastifyZodProvider) => id: z.string() }), body: z.object({ - hashedHex: z.string().min(1), + hashedHex: z.string().min(1).optional(), password: z.string().optional() }), response: { diff --git a/backend/src/services/secret-sharing/secret-sharing-service.ts b/backend/src/services/secret-sharing/secret-sharing-service.ts index 59908f97e..9f2404370 100644 --- a/backend/src/services/secret-sharing/secret-sharing-service.ts +++ b/backend/src/services/secret-sharing/secret-sharing-service.ts @@ -72,10 +72,7 @@ export const secretSharingServiceFactory = ({ const encryptedSecret = encryptWithRoot(Buffer.from(secretValue)); - // This will be 36 characters long, due to encoding it to base64. - const id = crypto.randomBytes(27).toString("base64url"); - - const hashedHex = crypto.createHash("sha256").update(id).digest("base64url").substring(0, 13); + const id = crypto.randomBytes(32).toString("hex"); const hashedPassword = password ? await bcrypt.hash(password, 10) : null; const newSharedSecret = await secretSharingDAL.create({ @@ -84,7 +81,6 @@ export const secretSharingServiceFactory = ({ tag: null, encryptedValue: null, encryptedSecret, - hashedHex, name, password: hashedPassword, expiresAt: new Date(expiresAt), @@ -94,7 +90,9 @@ export const secretSharingServiceFactory = ({ accessType }); - return { id: `${newSharedSecret.identifier}${hashedHex}` }; + const idToReturn = `${Buffer.from(newSharedSecret.identifier!, "hex").toString("base64url")}`; + + return { id: idToReturn }; }; const createPublicSharedSecret = async ({ @@ -124,8 +122,7 @@ export const secretSharingServiceFactory = ({ const encryptWithRoot = kmsService.encryptWithRootKey(); const encryptedSecret = encryptWithRoot(Buffer.from(secretValue)); - const id = crypto.randomBytes(27).toString("base64url"); - const hashedHex = crypto.createHash("sha256").update(id).digest("base64url").substring(0, 13); + const id = crypto.randomBytes(32).toString("hex"); const hashedPassword = password ? await bcrypt.hash(password, 10) : null; const newSharedSecret = await secretSharingDAL.create({ @@ -133,16 +130,14 @@ export const secretSharingServiceFactory = ({ encryptedValue: null, iv: null, tag: null, - hashedHex, encryptedSecret, - password: hashedPassword, expiresAt: new Date(expiresAt), expiresAfterViews, accessType }); - return { id: `${newSharedSecret.identifier}${hashedHex}` }; + return { id: `${Buffer.from(newSharedSecret.identifier!, "hex").toString("base64url")}` }; }; const getSharedSecrets = async ({ @@ -220,8 +215,7 @@ export const secretSharingServiceFactory = ({ hashedHex }) : await secretSharingDAL.findOne({ - hashedHex, - identifier: sharedSecretId + identifier: Buffer.from(sharedSecretId, "base64url").toString("hex") }); if (!sharedSecret) @@ -295,12 +289,12 @@ export const secretSharingServiceFactory = ({ 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 }); + const deletedSharedSecret = await secretSharingDAL.deleteById(sharedSecretId); + if (sharedSecret.orgId && sharedSecret.orgId !== orgId) throw new ForbiddenRequestError({ message: "User does not have permission to delete shared secret" }); diff --git a/backend/src/services/secret-sharing/secret-sharing-types.ts b/backend/src/services/secret-sharing/secret-sharing-types.ts index 9469e8de5..1d9efa1e3 100644 --- a/backend/src/services/secret-sharing/secret-sharing-types.ts +++ b/backend/src/services/secret-sharing/secret-sharing-types.ts @@ -28,7 +28,7 @@ export type TCreatePublicSharedSecretDTO = { export type TGetActiveSharedSecretByIdDTO = { sharedSecretId: string; - hashedHex: string; + hashedHex?: string; orgId?: string; password?: string; }; diff --git a/frontend/src/hooks/api/secretSharing/queries.ts b/frontend/src/hooks/api/secretSharing/queries.ts index c349d39f5..479f3ec16 100644 --- a/frontend/src/hooks/api/secretSharing/queries.ts +++ b/frontend/src/hooks/api/secretSharing/queries.ts @@ -8,7 +8,7 @@ export const secretSharingKeys = { allSharedSecrets: () => ["sharedSecrets"] as const, specificSharedSecrets: ({ offset, limit }: { offset: number; limit: number }) => [...secretSharingKeys.allSharedSecrets(), { offset, limit }] as const, - getSecretById: (arg: { id: string; hashedHex: string; password?: string }) => [ + getSecretById: (arg: { id: string; hashedHex: string | null; password?: string }) => [ "shared-secret", arg ] @@ -46,7 +46,7 @@ export const useGetActiveSharedSecretById = ({ password }: { sharedSecretId: string; - hashedHex: string; + hashedHex: string | null; password?: string; }) => { return useQuery( @@ -55,7 +55,7 @@ export const useGetActiveSharedSecretById = ({ const { data } = await apiRequest.post( `/api/v1/secret-sharing/public/${sharedSecretId}`, { - hashedHex, + ...(hashedHex && { hashedHex }), password } ); @@ -63,7 +63,7 @@ export const useGetActiveSharedSecretById = ({ return data; }, { - enabled: Boolean(sharedSecretId) && Boolean(hashedHex) + enabled: Boolean(sharedSecretId) } ); }; diff --git a/frontend/src/views/ViewSecretPublicPage/ViewSecretPublicPage.tsx b/frontend/src/views/ViewSecretPublicPage/ViewSecretPublicPage.tsx index f2309860b..95475ad4a 100644 --- a/frontend/src/views/ViewSecretPublicPage/ViewSecretPublicPage.tsx +++ b/frontend/src/views/ViewSecretPublicPage/ViewSecretPublicPage.tsx @@ -15,14 +15,6 @@ const extractDetailsFromUrl = (router: NextRouter) => { const idString = id as string; - if (!idString) { - return { - id: "", - hashedHex: "", - key: null - }; - } - if (urlEncodedKey) { const [hashedHex, key] = urlEncodedKey ? urlEncodedKey.toString().split("-") : ["", ""]; @@ -33,13 +25,9 @@ const extractDetailsFromUrl = (router: NextRouter) => { }; } - // get the first 36 characters as id and the rest as hex - const idPart = idString.substring(0, 36); - const hexPart = idString.substring(36); - return { - id: idPart || "", - hashedHex: hexPart || "", + id: idString, + hashedHex: null, key: null }; }; @@ -65,6 +53,9 @@ export const ViewSecretPublicPage = () => { ((error as AxiosError)?.response?.data as { message: string })?.message === "Invalid credentials"; + console.log("data", fetchSecret); + console.log("err", error); + const shouldShowPasswordPrompt = isInvalidCredential || (fetchSecret?.isPasswordProtected && !fetchSecret.secret); const isValidatingPassword = Boolean(password) && isFetching;