From e7783fe6cca8698c7a4b63e4f2f4ae31e9bbdf15 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Sat, 26 Apr 2025 05:19:02 +0400 Subject: [PATCH] requested changes & edge cases --- .../src/server/routes/v1/dashboard-router.ts | 8 ++- .../secret-reminder-recipients-types.ts | 8 +++ .../secret-v2-bridge/secret-v2-bridge-dal.ts | 54 +++++++++++++++++++ .../secret-v2-bridge/secret-v2-bridge-fns.ts | 3 ++ .../secret-v2-bridge-service.ts | 3 +- backend/src/services/secret/secret-fns.ts | 1 + backend/src/services/secret/secret-queue.ts | 16 +++++- backend/src/services/secret/secret-service.ts | 1 + frontend/src/hooks/api/secrets/queries.tsx | 1 + frontend/src/hooks/api/secrets/types.ts | 10 ++++ .../SecretListView/CreateReminderForm.tsx | 16 ++---- .../SecretListView/SecretDetailSidebar.tsx | 11 ++++ .../SecretListView/SecretListView.tsx | 12 ++++- 13 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 backend/src/services/secret-reminder-recipients/secret-reminder-recipients-types.ts diff --git a/backend/src/server/routes/v1/dashboard-router.ts b/backend/src/server/routes/v1/dashboard-router.ts index 5a52d4748..54da97682 100644 --- a/backend/src/server/routes/v1/dashboard-router.ts +++ b/backend/src/server/routes/v1/dashboard-router.ts @@ -1,7 +1,7 @@ import { ForbiddenError } from "@casl/ability"; import { z } from "zod"; -import { SecretFoldersSchema, SecretImportsSchema } from "@app/db/schemas"; +import { SecretFoldersSchema, SecretImportsSchema, UsersSchema } from "@app/db/schemas"; import { EventType, UserAgentType } from "@app/ee/services/audit-log/audit-log-types"; import { ProjectPermissionSecretActions } from "@app/ee/services/permission/project-permission"; import { SecretRotationV2Schema } from "@app/ee/services/secret-rotation-v2/secret-rotation-v2-union-schema"; @@ -594,6 +594,12 @@ export const registerDashboardRouter = async (server: FastifyZodProvider) => { .optional(), secrets: secretRawSchema .extend({ + secretReminderRecipients: z + .object({ + user: UsersSchema.pick({ id: true, email: true, username: true }), + id: z.string() + }) + .array(), secretValueHidden: z.boolean(), secretPath: z.string().optional(), secretMetadata: ResourceMetadataSchema.optional(), diff --git a/backend/src/services/secret-reminder-recipients/secret-reminder-recipients-types.ts b/backend/src/services/secret-reminder-recipients/secret-reminder-recipients-types.ts new file mode 100644 index 000000000..49eb9bf0f --- /dev/null +++ b/backend/src/services/secret-reminder-recipients/secret-reminder-recipients-types.ts @@ -0,0 +1,8 @@ +export type TSecretReminderRecipient = { + user: { + id: string; + username: string; + email?: string | null; + }; + id: string; +}; diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts index a9c909899..f54fab361 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-dal.ts @@ -95,7 +95,17 @@ export const secretV2BridgeDALFactory = ({ db, keyStore }: TSecretV2DalArg) => { `${TableName.SecretV2}.id`, `${TableName.SecretRotationV2SecretMapping}.secretId` ) + .leftJoin( + TableName.SecretReminderRecipients, + `${TableName.SecretV2}.id`, + `${TableName.SecretReminderRecipients}.secretId` + ) + .leftJoin(TableName.Users, `${TableName.SecretReminderRecipients}.userId`, `${TableName.Users}.id`) .select(selectAllTableCols(TableName.SecretV2)) + .select(db.ref("id").withSchema(TableName.SecretReminderRecipients).as("reminderRecipientId")) + .select(db.ref("username").withSchema(TableName.Users).as("reminderRecipientUsername")) + .select(db.ref("email").withSchema(TableName.Users).as("reminderRecipientEmail")) + .select(db.ref("id").withSchema(TableName.Users).as("reminderRecipientUserId")) .select(db.ref("id").withSchema(TableName.SecretTag).as("tagId")) .select(db.ref("color").withSchema(TableName.SecretTag).as("tagColor")) .select(db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug")) @@ -119,6 +129,23 @@ export const secretV2BridgeDALFactory = ({ db, keyStore }: TSecretV2DalArg) => { slug, name: slug }) + }, + { + key: "reminderRecipientId", + label: "secretReminderRecipients" as const, + mapper: ({ + reminderRecipientId, + reminderRecipientUsername, + reminderRecipientEmail, + reminderRecipientUserId + }) => ({ + user: { + id: reminderRecipientUserId, + username: reminderRecipientUsername, + email: reminderRecipientEmail + }, + id: reminderRecipientId + }) } ] }); @@ -607,6 +634,12 @@ export const secretV2BridgeDALFactory = ({ db, keyStore }: TSecretV2DalArg) => { `${TableName.SecretV2JnTag}.${TableName.SecretTag}Id`, `${TableName.SecretTag}.id` ) + .leftJoin( + TableName.SecretReminderRecipients, + `${TableName.SecretV2}.id`, + `${TableName.SecretReminderRecipients}.secretId` + ) + .leftJoin(TableName.Users, `${TableName.SecretReminderRecipients}.userId`, `${TableName.Users}.id`) .leftJoin(TableName.ResourceMetadata, `${TableName.SecretV2}.id`, `${TableName.ResourceMetadata}.secretId`) .leftJoin( TableName.SecretRotationV2SecretMapping, @@ -635,6 +668,10 @@ export const secretV2BridgeDALFactory = ({ db, keyStore }: TSecretV2DalArg) => { }) as rank` ) ) + .select(db.ref("id").withSchema(TableName.SecretReminderRecipients).as("reminderRecipientId")) + .select(db.ref("username").withSchema(TableName.Users).as("reminderRecipientUsername")) + .select(db.ref("email").withSchema(TableName.Users).as("reminderRecipientEmail")) + .select(db.ref("id").withSchema(TableName.Users).as("reminderRecipientUserId")) .select(db.ref("id").withSchema(TableName.SecretTag).as("tagId")) .select(db.ref("color").withSchema(TableName.SecretTag).as("tagColor")) .select(db.ref("slug").withSchema(TableName.SecretTag).as("tagSlug")) @@ -679,6 +716,23 @@ export const secretV2BridgeDALFactory = ({ db, keyStore }: TSecretV2DalArg) => { isRotatedSecret: Boolean(el.rotationId) }), childrenMapper: [ + { + key: "reminderRecipientId", + label: "secretReminderRecipients" as const, + mapper: ({ + reminderRecipientId, + reminderRecipientUsername, + reminderRecipientEmail, + reminderRecipientUserId + }) => ({ + user: { + id: reminderRecipientUserId, + username: reminderRecipientUsername, + email: reminderRecipientEmail + }, + id: reminderRecipientId + }) + }, { key: "tagId", label: "tags" as const, diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts index 3b35ab41a..56e9e91f5 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-fns.ts @@ -10,6 +10,7 @@ import { TProjectEnvDALFactory } from "../project-env/project-env-dal"; import { ResourceMetadataDTO } from "../resource-metadata/resource-metadata-schema"; import { INFISICAL_SECRET_VALUE_HIDDEN_MASK } from "../secret/secret-fns"; import { TSecretFolderDALFactory } from "../secret-folder/secret-folder-dal"; +import { TSecretReminderRecipient } from "../secret-reminder-recipients/secret-reminder-recipients-types"; import { TSecretV2BridgeDALFactory } from "./secret-v2-bridge-dal"; import { TFnSecretBulkDelete, TFnSecretBulkInsert, TFnSecretBulkUpdate } from "./secret-v2-bridge-types"; @@ -668,6 +669,7 @@ export const reshapeBridgeSecret = ( secretMetadata?: ResourceMetadataDTO; isRotatedSecret?: boolean; rotationId?: string; + secretReminderRecipients?: TSecretReminderRecipient[]; }, secretValueHidden: boolean ) => ({ @@ -699,6 +701,7 @@ export const reshapeBridgeSecret = ( updatedAt: secret.updatedAt, isRotatedSecret: secret.isRotatedSecret, rotationId: secret.rotationId, + secretReminderRecipients: secret.secretReminderRecipients || [], ...(secretValueHidden ? { secretValue: INFISICAL_SECRET_VALUE_HIDDEN_MASK, diff --git a/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts b/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts index 7e232491a..e9352c8ac 100644 --- a/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts +++ b/backend/src/services/secret-v2-bridge/secret-v2-bridge-service.ts @@ -539,7 +539,8 @@ export const secretV2BridgeServiceFactory = ({ oldSecret: { id: secret.id, secretReminderNote: secret.reminderNote, - secretReminderRepeatDays: secret.reminderRepeatDays + secretReminderRepeatDays: secret.reminderRepeatDays, + secretReminderRecipients: secret.secretReminderRecipients?.map((el) => el.user.id) }, projectId }); diff --git a/backend/src/services/secret/secret-fns.ts b/backend/src/services/secret/secret-fns.ts index 2574dc13e..c295be7e7 100644 --- a/backend/src/services/secret/secret-fns.ts +++ b/backend/src/services/secret/secret-fns.ts @@ -396,6 +396,7 @@ export const decryptSecretRaw = ( id: secret.id, user: secret.userId, tags: secret.tags?.map((el) => ({ ...el, name: el.slug })), + secretReminderRecipients: [], skipMultilineEncoding: secret.skipMultilineEncoding, secretReminderRepeatDays: secret.secretReminderRepeatDays, secretReminderNote: secret.secretReminderNote, diff --git a/backend/src/services/secret/secret-queue.ts b/backend/src/services/secret/secret-queue.ts index f66f9a57b..a768a4fe5 100644 --- a/backend/src/services/secret/secret-queue.ts +++ b/backend/src/services/secret/secret-queue.ts @@ -259,6 +259,8 @@ export const secretQueueFactory = ({ }); } + const existingRecipients = await secretReminderRecipientsDAL.findUsersBySecretId(newSecret.id); + if (recipients?.length) { await secretReminderRecipientsDAL.transaction(async (tx) => { await secretReminderRecipientsDAL.delete({ secretId: newSecret.id }, tx); @@ -271,6 +273,8 @@ export const secretQueueFactory = ({ tx ); }); + } else if (existingRecipients) { + await secretReminderRecipientsDAL.delete({ secretId: newSecret.id }); } await queueService.queue( @@ -306,12 +310,20 @@ export const secretQueueFactory = ({ }; const handleSecretReminder = async ({ newSecret, oldSecret, projectId }: THandleReminderDTO) => { - const { secretReminderRepeatDays, secretReminderNote } = newSecret; + const { secretReminderRepeatDays, secretReminderNote, secretReminderRecipients } = newSecret; + + const recipientsUpdated = + secretReminderRecipients?.some( + (newId) => !oldSecret.secretReminderRecipients?.find((oldId) => newId === oldId) + ) || + (secretReminderRecipients?.length !== oldSecret.secretReminderRecipients?.length && + secretReminderRepeatDays !== null); if (newSecret.type !== SecretType.Personal && secretReminderRepeatDays !== undefined) { if ( (secretReminderRepeatDays && oldSecret.secretReminderRepeatDays !== secretReminderRepeatDays) || - (secretReminderNote && oldSecret.secretReminderNote !== secretReminderNote) + (secretReminderNote && oldSecret.secretReminderNote !== secretReminderNote) || + recipientsUpdated ) { await addSecretReminder({ oldSecret, diff --git a/backend/src/services/secret/secret-service.ts b/backend/src/services/secret/secret-service.ts index 662919f70..d69d587d2 100644 --- a/backend/src/services/secret/secret-service.ts +++ b/backend/src/services/secret/secret-service.ts @@ -685,6 +685,7 @@ export const secretServiceFactory = ({ ...secret, workspace: projectId, environment, + secretReminderRecipients: [], secretPath: groupedPaths[secret.folderId][0].path })) }; diff --git a/frontend/src/hooks/api/secrets/queries.tsx b/frontend/src/hooks/api/secrets/queries.tsx index b7370a29a..10796c6e2 100644 --- a/frontend/src/hooks/api/secrets/queries.tsx +++ b/frontend/src/hooks/api/secrets/queries.tsx @@ -80,6 +80,7 @@ export const mergePersonalSecrets = (rawSecrets: SecretV3Raw[]) => { comment: el.secretComment || "", reminderRepeatDays: el.secretReminderRepeatDays, reminderNote: el.secretReminderNote, + secretReminderRecipients: el.secretReminderRecipients, createdAt: el.createdAt, updatedAt: el.updatedAt, version: el.version, diff --git a/frontend/src/hooks/api/secrets/types.ts b/frontend/src/hooks/api/secrets/types.ts index 1ee339389..fa597169f 100644 --- a/frontend/src/hooks/api/secrets/types.ts +++ b/frontend/src/hooks/api/secrets/types.ts @@ -7,6 +7,14 @@ export enum SecretType { Personal = "personal" } +export type SecretReminderRecipient = { + user: { + id: string; + username: string; + email: string; + }; + id: string; +}; export type EncryptedSecret = { id: string; version: number; @@ -56,6 +64,7 @@ export type SecretV3RawSanitized = { secretMetadata?: { key: string; value: string }[]; isReminderEvent?: boolean; isRotatedSecret?: boolean; + secretReminderRecipients?: SecretReminderRecipient[]; rotationId?: string; }; @@ -81,6 +90,7 @@ export type SecretV3Raw = { updatedAt: string; isRotatedSecret?: boolean; rotationId?: string; + secretReminderRecipients?: SecretReminderRecipient[]; }; export type SecretV3RawResponse = { diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/CreateReminderForm.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/CreateReminderForm.tsx index 679b5da11..5f95d7149 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/CreateReminderForm.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/CreateReminderForm.tsx @@ -58,7 +58,6 @@ export const CreateReminderForm = ({ const { register, control, - reset, setValue, handleSubmit, formState: { isSubmitting } @@ -74,19 +73,10 @@ export const CreateReminderForm = ({ onOpenChange(false, data); }; - useEffect(() => { - if (isOpen) { - reset({ - days: repeatDays || undefined, - note: note || "" - }); - } - }, [isOpen]); - useEffect(() => { // On initial load, filter the members to only include the recipients if (members.length) { - const filteredMembers = members.filter((m) => recipients?.includes(m.id)); + const filteredMembers = members.filter((m) => recipients?.find((r) => r === m.user.id)); setValue( "recipients", filteredMembers.map((m) => ({ @@ -95,7 +85,7 @@ export const CreateReminderForm = ({ })) ); } - }, [members, isOpen]); + }, [members, isOpen, recipients]); return ( @@ -122,6 +112,7 @@ export const CreateReminderForm = ({ onChange={(el) => setValue("days", parseInt(el.target.value, 10))} type="number" placeholder="31" + defaultValue={repeatDays || undefined} value={field.value || undefined} /> @@ -144,6 +135,7 @@ export const CreateReminderForm = ({ placeholder="Remember to rotate the AWS secret every month." className="border border-mineshaft-600 text-sm" rows={8} + defaultValue={note || ""} reSize="none" cols={30} {...register("note")} diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx index 1f4149972..ac8c79e21 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx @@ -1,3 +1,4 @@ +import { useEffect } from "react"; import { Controller, useFieldArray, useForm } from "react-hook-form"; import { subject } from "@casl/ability"; import { faCircleQuestion, faEye } from "@fortawesome/free-regular-svg-icons"; @@ -235,6 +236,16 @@ export const SecretDetailSidebar = ({ const secretReminderRepeatDays = watch("reminderRepeatDays"); const secretReminderNote = watch("reminderNote"); const secretReminderRecipients = watch("reminderRecipients"); + useEffect(() => { + setValue( + "reminderRecipients", + secret?.secretReminderRecipients?.map((el) => el.user.id), + { + shouldDirty: false + } + ); + }, [secret?.secretReminderRecipients]); + const getModifiedByIcon = (userType: string | undefined | null) => { switch (userType) { case ActorType.USER: diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx index 07563a72d..5c70b91e4 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretListView.tsx @@ -49,6 +49,8 @@ export const SecretListView = ({ isProtectedBranch = false, importedBy }: Props) => { + console.log("secretssssss", secrets); + const queryClient = useQueryClient(); const { popUp, handlePopUpToggle, handlePopUpOpen, handlePopUpClose } = usePopUp([ "deleteSecret", @@ -184,6 +186,12 @@ export const SecretListView = ({ const tagIds = tags?.map(({ id }) => id); const oldTagIds = (orgSecret?.tags || []).map(({ id }) => id); const isSameTags = JSON.stringify(tagIds) === JSON.stringify(oldTagIds); + + const isSameRecipients = + !reminderRecipients?.some( + (newId) => !orgSecret.secretReminderRecipients?.find((oldId) => newId === oldId.user.id) + ) && reminderRecipients?.length === orgSecret.secretReminderRecipients?.length; + const isSharedSecUnchanged = ( [ @@ -196,7 +204,9 @@ export const SecretListView = ({ "reminderRecipients", "secretMetadata" ] as const - ).every((el) => orgSecret[el] === modSecret[el]) && isSameTags; + ).every((el) => orgSecret[el] === modSecret[el]) && + isSameTags && + isSameRecipients; try { // personal secret change