From 9cdb4dcde9ce11b39ec2df2d1d1188a825b46b62 Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Wed, 19 Feb 2025 16:52:48 -0800 Subject: [PATCH] improvement: address feedback --- .../aws/aws-connection-service.ts | 31 ++- .../aws-parameter-store-sync-fns.ts | 83 +++--- .../aws-parameter-store-sync-schemas.ts | 13 +- docs/integrations/secret-syncs/overview.mdx | 7 + .../forms/CreateSecretSyncForm.tsx | 253 ++++++++++-------- .../AwsParameterStoreSyncOptionsFields.tsx | 25 ++ .../AwsParameterStoreSyncReviewFields.tsx | 6 +- .../src/hooks/api/appConnections/aws/types.ts | 8 - .../SecretSyncsTab/SecretSyncsTab.tsx | 2 +- .../SecretSyncDetailsByIDPage.tsx | 2 +- .../AwsParameterStoreSyncOptionsSection.tsx | 2 +- 11 files changed, 269 insertions(+), 163 deletions(-) diff --git a/backend/src/services/app-connection/aws/aws-connection-service.ts b/backend/src/services/app-connection/aws/aws-connection-service.ts index d722d727e..689608b81 100644 --- a/backend/src/services/app-connection/aws/aws-connection-service.ts +++ b/backend/src/services/app-connection/aws/aws-connection-service.ts @@ -24,9 +24,25 @@ const listAwsKmsKeys = async ( region }); - const { Aliases = [] } = await awsKms.listAliases({ Limit: 100 }).promise(); + const aliasEntries: AWS.KMS.AliasList = []; + let aliasMarker: string | undefined; + do { + // eslint-disable-next-line no-await-in-loop + const response = await awsKms.listAliases({ Limit: 100, Marker: aliasMarker }).promise(); + aliasEntries.push(...(response.Aliases || [])); + aliasMarker = response.NextMarker; + } while (aliasMarker); - const aliasEntries = Aliases.filter((aliasEntry) => { + const keyMetadataRecord: Record = {}; + for await (const aliasEntry of aliasEntries) { + if (aliasEntry.TargetKeyId) { + const keyDescription = await awsKms.describeKey({ KeyId: aliasEntry.TargetKeyId }).promise(); + + keyMetadataRecord[aliasEntry.TargetKeyId] = keyDescription.KeyMetadata; + } + } + + const validAliasEntries = aliasEntries.filter((aliasEntry) => { if (!aliasEntry.TargetKeyId) return false; if (destination === SecretSync.AWSParameterStore && aliasEntry.AliasName === "alias/aws/ssm") return true; @@ -36,13 +52,18 @@ const listAwsKmsKeys = async ( if (aliasEntry.AliasName?.includes("alias/aws/")) return false; + const keyMetadata = keyMetadataRecord[aliasEntry.TargetKeyId]; + + if (!keyMetadata || keyMetadata.KeyUsage !== "ENCRYPT_DECRYPT" || keyMetadata.KeySpec !== "SYMMETRIC_DEFAULT") + return false; + return true; }); - const kmsKeys = aliasEntries.map((alias) => { + const kmsKeys = validAliasEntries.map((aliasEntry) => { return { - id: alias.TargetKeyId!, - alias: alias.AliasName! + id: aliasEntry.TargetKeyId!, + alias: aliasEntry.AliasName! }; }); diff --git a/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-fns.ts b/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-fns.ts index e74f6e451..9fa25ba76 100644 --- a/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-fns.ts +++ b/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-fns.ts @@ -137,10 +137,9 @@ const getParameterMetadataByPath = async (ssm: AWS.SSM, path: string): Promise => { + needsTagsPermissions: boolean +): Promise<{ shouldManageTags: boolean; awsParameterStoreTagsRecord: TAWSParameterStoreTagsRecord }> => { const awsParameterStoreTagsRecord: TAWSParameterStoreTagsRecord = {}; for await (const entry of Object.entries(awsParameterStoreSecretsRecord)) { @@ -163,15 +162,23 @@ const getParameterStoreTagsRecord = async ( } catch (e) { // users aren't required to provide tag permissions to use sync so we handle gracefully if unauthorized // and they aren't trying to configure tags - if ((e as AWSError).code === "AccessDeniedException" && !areTagsPresent) { - return {}; + if ((e as AWSError).code === "AccessDeniedException") { + if (!needsTagsPermissions) { + return { shouldManageTags: false, awsParameterStoreTagsRecord: {} }; + } + + throw new SecretSyncError({ + message: + "IAM role has inadequate permissions to manage resource tags. Ensure the following polices are present: ssm:ListTagsForResource, ssm:AddTagsToResource, and ssm:RemoveTagsFromResource", + shouldRetry: false + }); } throw e; } } - return awsParameterStoreTagsRecord; + return { shouldManageTags: true, awsParameterStoreTagsRecord }; }; const processParameterTags = ({ @@ -294,13 +301,11 @@ export const AwsParameterStoreSyncFns = { const awsParameterStoreMetadataRecord = await getParameterMetadataByPath(ssm, destinationConfig.path); - const awsParameterStoreTagsRecord = await getParameterStoreTagsRecord( + const { shouldManageTags, awsParameterStoreTagsRecord } = await getParameterStoreTagsRecord( ssm, - destinationConfig.path, awsParameterStoreSecretsRecord, - Boolean(syncOptions.tags || syncOptions.syncSecretMetadataAsTags) + Boolean(syncOptions.tags?.length || syncOptions.syncSecretMetadataAsTags) ); - const syncTagsRecord = Object.fromEntries(syncOptions.tags?.map((tag) => [tag.key, tag.value]) ?? []); for await (const entry of Object.entries(secretMap)) { @@ -316,7 +321,7 @@ export const AwsParameterStoreSyncFns = { if ( !(key in awsParameterStoreSecretsRecord) || value !== awsParameterStoreSecretsRecord[key].Value || - syncOptions.keyId !== awsParameterStoreMetadataRecord[key]?.KeyId + (syncOptions.keyId ?? "alias/aws/ssm") !== awsParameterStoreMetadataRecord[key]?.KeyId ) { try { await putParameter(ssm, { @@ -334,28 +339,44 @@ export const AwsParameterStoreSyncFns = { } } - const { tagsToAdd, tagKeysToRemove } = processParameterTags({ - syncTagsRecord: { - // configured sync tags take preference over secret metadata - ...(syncOptions.syncSecretMetadataAsTags && - Object.fromEntries(secretMetadata?.map((tag) => [tag.key, tag.value]) ?? [])), - ...syncTagsRecord - }, - awsTagsRecord: awsParameterStoreTagsRecord[key] ?? {} - }); - - if (tagsToAdd.length) { - await addTagsToParameter(ssm, { - ResourceId: `${destinationConfig.path}${key}`, - Tags: tagsToAdd + if (shouldManageTags) { + const { tagsToAdd, tagKeysToRemove } = processParameterTags({ + syncTagsRecord: { + // configured sync tags take preference over secret metadata + ...(syncOptions.syncSecretMetadataAsTags && + Object.fromEntries(secretMetadata?.map((tag) => [tag.key, tag.value]) ?? [])), + ...syncTagsRecord + }, + awsTagsRecord: awsParameterStoreTagsRecord[key] ?? {} }); - } - if (tagKeysToRemove.length) { - await removeTagsFromParameter(ssm, { - ResourceId: `${destinationConfig.path}${key}`, - TagKeys: tagKeysToRemove - }); + if (tagsToAdd.length) { + try { + await addTagsToParameter(ssm, { + ResourceId: `${destinationConfig.path}${key}`, + Tags: tagsToAdd + }); + } catch (error) { + throw new SecretSyncError({ + error, + secretKey: key + }); + } + } + + if (tagKeysToRemove.length) { + try { + await removeTagsFromParameter(ssm, { + ResourceId: `${destinationConfig.path}${key}`, + TagKeys: tagKeysToRemove + }); + } catch (error) { + throw new SecretSyncError({ + error, + secretKey: key + }); + } + } } } diff --git a/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-schemas.ts b/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-schemas.ts index 3ed07a678..c42a44a7c 100644 --- a/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-schemas.ts +++ b/backend/src/services/secret-sync/aws-parameter-store/aws-parameter-store-sync-schemas.ts @@ -35,14 +35,17 @@ const AwsParameterStoreSyncOptionsSchema = z.object({ .string() .regex( /^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$/u, - "Tag keys can only contain Unicode letters, digits, white space and any of the following: _." + "Invalid resource tag key: keys can only contain Unicode letters, digits, white space and any of the following: _.:/=+@-" ) - .min(1, "AWS tag key required") - .max(128, "AWS tag name cannot exceed 128 characters"), + .min(1, "Resource tag key required") + .max(128, "Resource tag name cannot exceed 128 characters"), value: z .string() - .regex(/^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$/u, "Invalid AWS tag value") - .max(256, "Tag values can only contain Unicode letters, digits, white space and any of the following: _.:/=+@-") + .regex( + /^([\p{L}\p{Z}\p{N}_.:/=+\-@]*)$/u, + "Invalid resource tag value: tag values can only contain Unicode letters, digits, white space and any of the following: _.:/=+@-" + ) + .max(256, "Resource tag value cannot exceed 256 characters") }) .array() .max(50) diff --git a/docs/integrations/secret-syncs/overview.mdx b/docs/integrations/secret-syncs/overview.mdx index cc93d8036..7b92a55e3 100644 --- a/docs/integrations/secret-syncs/overview.mdx +++ b/docs/integrations/secret-syncs/overview.mdx @@ -77,6 +77,13 @@ via the UI or API for the third-party service you intend to sync secrets to. - Destination: The App Connection to utilize and the destination endpoint to deploy secrets to. These can vary between services. - Options: Customize how secrets should be synced. Examples include adding a suffix or prefix to your secrets, or importing secrets from the destination on the initial sync. + + Secret Syncs are the source of truth for connected third-party services. Any secret, + including associated data, not present or imported in Infisical before syncing will be + overwritten, and changes directly in the connected service outside of infisical may also + be overwritten by future syncs. + + Some third-party services do not support importing secrets. diff --git a/frontend/src/components/secret-syncs/forms/CreateSecretSyncForm.tsx b/frontend/src/components/secret-syncs/forms/CreateSecretSyncForm.tsx index efd7f412c..0a56714c5 100644 --- a/frontend/src/components/secret-syncs/forms/CreateSecretSyncForm.tsx +++ b/frontend/src/components/secret-syncs/forms/CreateSecretSyncForm.tsx @@ -1,11 +1,13 @@ import { useState } from "react"; import { Controller, FormProvider, useForm } from "react-hook-form"; +import { faInfoCircle } from "@fortawesome/free-solid-svg-icons"; +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { Tab } from "@headlessui/react"; import { zodResolver } from "@hookform/resolvers/zod"; import { twMerge } from "tailwind-merge"; import { createNotification } from "@app/components/notifications"; -import { Button, Checkbox, FormControl, Switch } from "@app/components/v2"; +import { Button, FormControl, Modal, ModalContent, Switch } from "@app/components/v2"; import { useWorkspace } from "@app/context"; import { SECRET_SYNC_MAP } from "@app/helpers/secretSyncs"; import { @@ -42,8 +44,9 @@ export const CreateSecretSyncForm = ({ destination, onComplete, onCancel }: Prop const { currentWorkspace } = useWorkspace(); const { name: destinationName } = SECRET_SYNC_MAP[destination]; + const [showConfirmation, setShowConfirmation] = useState(false); + const [selectedTabIndex, setSelectedTabIndex] = useState(0); - const [confirmOverwrite, setConfirmOverwrite] = useState(false); const { syncOption } = useSecretSyncOption(destination); @@ -77,6 +80,7 @@ export const CreateSecretSyncForm = ({ destination, onComplete, onCancel }: Prop onComplete(secretSync); } catch (err: any) { console.error(err); + setShowConfirmation(false); createNotification({ title: `Failed to add ${destinationName} Sync`, text: err.message, @@ -94,7 +98,7 @@ export const CreateSecretSyncForm = ({ destination, onComplete, onCancel }: Prop setSelectedTabIndex((prev) => prev - 1); }; - const { handleSubmit, trigger, watch, control } = formMethods; + const { handleSubmit, trigger, control } = formMethods; const isStepValid = async (index: number) => trigger(FORM_TABS[index].fields); @@ -102,7 +106,7 @@ export const CreateSecretSyncForm = ({ destination, onComplete, onCancel }: Prop const handleNext = async () => { if (isFinalStep) { - handleSubmit(onSubmit)(); + setShowConfirmation(true); return; } @@ -123,113 +127,146 @@ export const CreateSecretSyncForm = ({ destination, onComplete, onCancel }: Prop return isEnabled; }; - const initialSyncBehavior = watch("syncOptions.initialSyncBehavior"); + if (showConfirmation) + return ( + <> +
+
+ + Secret Sync Behavior +
+

+ Secret Syncs are the source of truth for connected third-party services. Any secret, + including associated data, not present or imported in Infisical before syncing will be + overwritten, and changes directly in the connected service outside of infisical may also + be overwritten by future syncs. +

+
+
+ + + +
+ + ); return ( -
- - - - {FORM_TABS.map((tab, index) => ( - { - e.preventDefault(); - const isEnabled = await isTabEnabled(index); - setSelectedTabIndex((prev) => (isEnabled ? index : prev)); - }} - className={({ selected }) => - `w-30 -mb-[0.14rem] ${index > selectedTabIndex ? "opacity-30" : ""} px-4 py-2 text-sm font-medium outline-none disabled:opacity-60 ${ - selected - ? "border-b-2 border-mineshaft-300 text-mineshaft-200" - : "text-bunker-300" - }` - } - key={tab.key} - > - {index + 1}. {tab.name} - - ))} - - - - - - - - - - - { - return ( - - + + + + + {FORM_TABS.map((tab, index) => ( + { + e.preventDefault(); + const isEnabled = await isTabEnabled(index); + setSelectedTabIndex((prev) => (isEnabled ? index : prev)); + }} + className={({ selected }) => + `w-30 -mb-[0.14rem] ${index > selectedTabIndex ? "opacity-30" : ""} px-4 py-2 text-sm font-medium outline-none disabled:opacity-60 ${ + selected + ? "border-b-2 border-mineshaft-300 text-mineshaft-200" + : "text-bunker-300" + }` + } + key={tab.key} + > + {index + 1}. {tab.name} + + ))} + + + + + + + + + + + { + return ( + -

Auto-Sync {value ? "Enabled" : "Disabled"}

- - - ); - }} - /> - - - - - - - - - - - {isFinalStep && - initialSyncBehavior === SecretSyncInitialSyncBehavior.OverwriteDestination && ( - setConfirmOverwrite(Boolean(isChecked))} - > -

- I understand all secrets present in the configured {destinationName} destination will - be removed if they are not present within Infisical. -

- - )} -
- - {selectedTabIndex > 0 && ( - - )} -
-
+ {selectedTabIndex > 0 && ( + + )} + + + + +
+
+ + Secret Sync Behavior +
+

+ Secret Syncs are the source of truth for connected third-party services. Any secret + not present or imported in Infisical before syncing will be overwritten, and changes + made directly in the connected service may also be overwritten by future syncs from + Infisical. +

+
+ + + ); }; diff --git a/frontend/src/components/secret-syncs/forms/SecretSyncOptionsFields/AwsParameterStoreSyncOptionsFields.tsx b/frontend/src/components/secret-syncs/forms/SecretSyncOptionsFields/AwsParameterStoreSyncOptionsFields.tsx index be4394ef7..c35f4d31a 100644 --- a/frontend/src/components/secret-syncs/forms/SecretSyncOptionsFields/AwsParameterStoreSyncOptionsFields.tsx +++ b/frontend/src/components/secret-syncs/forms/SecretSyncOptionsFields/AwsParameterStoreSyncOptionsFields.tsx @@ -63,6 +63,31 @@ export const AwsParameterStoreSyncOptionsFields = () => { onChange={(option) => onChange((option as SingleValue)?.alias ?? null) } + // eslint-disable-next-line react/no-unstable-nested-components + noOptionsMessage={({ inputValue }) => + inputValue ? undefined : ( +

+ To configure a KMS key, ensure the following permissions are present on the + selected IAM role:{" "} + + "kms:ListKeys" + + ,{" "} + + "kms:ListAliases" + + ,{" "} + + "kms:Encrypt" + + ,{" "} + + "kms:Decrypt" + + . +

+ ) + } options={kmsKeys} placeholder="Leave blank to use default KMS key" getOptionLabel={(option) => diff --git a/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/AwsParameterStoreSyncReviewFields.tsx b/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/AwsParameterStoreSyncReviewFields.tsx index 54e9df9b1..1b005e91c 100644 --- a/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/AwsParameterStoreSyncReviewFields.tsx +++ b/frontend/src/components/secret-syncs/forms/SecretSyncReviewFields/AwsParameterStoreSyncReviewFields.tsx @@ -18,8 +18,8 @@ export const AwsParameterStoreSyncOptionsReviewFields = () => { return ( <> {keyId && {keyId}} - {tags?.length && ( - + {tags && tags.length > 0 && ( + { )} {syncSecretMetadataAsTags && ( - + Enabled )} diff --git a/frontend/src/hooks/api/appConnections/aws/types.ts b/frontend/src/hooks/api/appConnections/aws/types.ts index b49111455..7661b131d 100644 --- a/frontend/src/hooks/api/appConnections/aws/types.ts +++ b/frontend/src/hooks/api/appConnections/aws/types.ts @@ -1,13 +1,5 @@ import { SecretSync } from "@app/hooks/api/secretSyncs"; -export type TDatabricksSecretScope = { - name: string; -}; - -export type TDatabricksConnectionListSecretScopesResponse = { - secretScopes: TDatabricksSecretScope[]; -}; - export type TListAwsConnectionKmsKeys = { connectionId: string; region: string; diff --git a/frontend/src/pages/secret-manager/IntegrationsListPage/components/SecretSyncsTab/SecretSyncsTab.tsx b/frontend/src/pages/secret-manager/IntegrationsListPage/components/SecretSyncsTab/SecretSyncsTab.tsx index f44ea439b..d9c237a3d 100644 --- a/frontend/src/pages/secret-manager/IntegrationsListPage/components/SecretSyncsTab/SecretSyncsTab.tsx +++ b/frontend/src/pages/secret-manager/IntegrationsListPage/components/SecretSyncsTab/SecretSyncsTab.tsx @@ -19,7 +19,7 @@ export const SecretSyncsTab = () => { const { data: secretSyncs = [], isPending: isSecretSyncsPending } = useListSecretSyncs( currentWorkspace.id, { - refetchInterval: 4000 + refetchInterval: 30000 } ); diff --git a/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/SecretSyncDetailsByIDPage.tsx b/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/SecretSyncDetailsByIDPage.tsx index 8d65871cf..3b837257f 100644 --- a/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/SecretSyncDetailsByIDPage.tsx +++ b/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/SecretSyncDetailsByIDPage.tsx @@ -37,7 +37,7 @@ const PageContent = () => { const { handlePopUpToggle, popUp, handlePopUpOpen } = usePopUp(["editSync"] as const); const { data: secretSync, isPending } = useGetSecretSync(destination, syncId, { - refetchInterval: 4000 + refetchInterval: 30000 }); if (isPending) { diff --git a/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/components/SecretSyncOptionsSection/AwsParameterStoreSyncOptionsSection.tsx b/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/components/SecretSyncOptionsSection/AwsParameterStoreSyncOptionsSection.tsx index 12d91d3dd..1b898bc70 100644 --- a/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/components/SecretSyncOptionsSection/AwsParameterStoreSyncOptionsSection.tsx +++ b/frontend/src/pages/secret-manager/SecretSyncDetailsByIDPage/components/SecretSyncOptionsSection/AwsParameterStoreSyncOptionsSection.tsx @@ -17,7 +17,7 @@ export const AwsParameterStoreSyncOptionsSection = ({ secretSync }: Props) => { return ( <> {keyId && {keyId}} - {tags?.length && ( + {tags && tags.length > 0 && (