From 3f654e115daf1a9e79d1b557c05c1c4019e22807 Mon Sep 17 00:00:00 2001 From: Daniel Hougaard Date: Wed, 18 Jun 2025 00:17:39 +0400 Subject: [PATCH] feat(secret-syncs): better permissioning --- .../services/permission/project-permission.ts | 42 +++- .../secret-sync/secret-sync-service.ts | 126 +++++++++--- .../forms/SecretSyncSourceFields.tsx | 32 ++- .../context/ProjectPermissionContext/types.ts | 13 +- .../ProjectRoleModifySection.utils.tsx | 19 +- .../components/RolePermissionsSection.tsx | 5 + .../SecretSyncPermissionConditions.tsx | 186 ++++++++++++++++++ 7 files changed, 381 insertions(+), 42 deletions(-) create mode 100644 frontend/src/pages/project/RoleDetailsBySlugPage/components/SecretSyncPermissionConditions.tsx diff --git a/backend/src/ee/services/permission/project-permission.ts b/backend/src/ee/services/permission/project-permission.ts index f61c4b1a4..f11f703fa 100644 --- a/backend/src/ee/services/permission/project-permission.ts +++ b/backend/src/ee/services/permission/project-permission.ts @@ -211,6 +211,11 @@ export type SecretFolderSubjectFields = { secretPath: string; }; +export type SecretSyncSubjectFields = { + environment: string; + secretPath: string; +}; + export type DynamicSecretSubjectFields = { environment: string; secretPath: string; @@ -267,6 +272,10 @@ export type ProjectPermissionSet = | (ForcedSubject & DynamicSecretSubjectFields) ) ] + | [ + ProjectPermissionSecretSyncActions, + ProjectPermissionSub.SecretSyncs | (ForcedSubject & SecretSyncSubjectFields) + ] | [ ProjectPermissionActions, ( @@ -323,7 +332,6 @@ export type ProjectPermissionSet = | [ProjectPermissionActions, ProjectPermissionSub.SshHostGroups] | [ProjectPermissionActions, ProjectPermissionSub.PkiAlerts] | [ProjectPermissionActions, ProjectPermissionSub.PkiCollections] - | [ProjectPermissionSecretSyncActions, ProjectPermissionSub.SecretSyncs] | [ProjectPermissionKmipActions, ProjectPermissionSub.Kmip] | [ProjectPermissionCmekActions, ProjectPermissionSub.Cmek] | [ProjectPermissionActions.Delete, ProjectPermissionSub.Project] @@ -412,6 +420,23 @@ const DynamicSecretConditionV2Schema = z }) .partial(); +const SecretSyncConditionV2Schema = z + .object({ + environment: z.union([ + z.string(), + z + .object({ + [PermissionConditionOperators.$EQ]: PermissionConditionSchema[PermissionConditionOperators.$EQ], + [PermissionConditionOperators.$NEQ]: PermissionConditionSchema[PermissionConditionOperators.$NEQ], + [PermissionConditionOperators.$IN]: PermissionConditionSchema[PermissionConditionOperators.$IN], + [PermissionConditionOperators.$GLOB]: PermissionConditionSchema[PermissionConditionOperators.$GLOB] + }) + .partial() + ]), + secretPath: SECRET_PATH_PERMISSION_OPERATOR_SCHEMA + }) + .partial(); + const SecretImportConditionSchema = z .object({ environment: z.union([ @@ -671,12 +696,6 @@ const GeneralPermissionSchema = [ "Describe what action an entity can take." ) }), - z.object({ - subject: z.literal(ProjectPermissionSub.SecretSyncs).describe("The entity this permission pertains to."), - action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionSecretSyncActions).describe( - "Describe what action an entity can take." - ) - }), z.object({ subject: z.literal(ProjectPermissionSub.Kmip).describe("The entity this permission pertains to."), action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionKmipActions).describe( @@ -836,6 +855,15 @@ export const ProjectPermissionV2Schema = z.discriminatedUnion("subject", [ "When specified, only matching conditions will be allowed to access given resource." ).optional() }), + z.object({ + subject: z.literal(ProjectPermissionSub.SecretSyncs).describe("The entity this permission pertains to."), + action: CASL_ACTION_SCHEMA_NATIVE_ENUM(ProjectPermissionSecretSyncActions).describe( + "Describe what action an entity can take." + ), + conditions: SecretSyncConditionV2Schema.describe( + "When specified, only matching conditions will be allowed to access given resource." + ).optional() + }), ...GeneralPermissionSchema ]); diff --git a/backend/src/services/secret-sync/secret-sync-service.ts b/backend/src/services/secret-sync/secret-sync-service.ts index b64620827..1bb27c05a 100644 --- a/backend/src/services/secret-sync/secret-sync-service.ts +++ b/backend/src/services/secret-sync/secret-sync-service.ts @@ -1,4 +1,4 @@ -import { ForbiddenError } from "@casl/ability"; +import { ForbiddenError, subject } from "@casl/ability"; import { ActionProjectType } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; @@ -217,13 +217,17 @@ export const secretSyncServiceFactory = ({ ForbiddenError.from(projectPermission).throwUnlessCan( ProjectPermissionSecretSyncActions.Create, - ProjectPermissionSub.SecretSyncs + subject(ProjectPermissionSub.SecretSyncs, { environment, secretPath }) ); - throwIfMissingSecretReadValueOrDescribePermission(projectPermission, ProjectPermissionSecretActions.ReadValue, { - environment, - secretPath - }); + throwIfMissingSecretReadValueOrDescribePermission( + projectPermission, + ProjectPermissionSecretActions.DescribeSecret, + { + environment, + secretPath + } + ); const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); @@ -286,10 +290,38 @@ export const secretSyncServiceFactory = ({ projectId: secretSync.projectId }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretSyncActions.Edit, - ProjectPermissionSub.SecretSyncs - ); + // we always check the permission against the existing environment / secret path + // if no secret path / environment is present on the secret sync, we need to check without conditions + if (secretSync.environment?.slug && secretSync.folder?.path) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.Edit, + subject(ProjectPermissionSub.SecretSyncs, { + environment: secretSync.environment.slug, + secretPath: secretSync.folder.path + }) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.Edit, + ProjectPermissionSub.SecretSyncs + ); + } + + // if the user is updating the secret path or environment, we need to check the permission against the new values + if (secretPath || environment) { + const environmentToCheck = environment || secretSync.environment?.slug || ""; + const secretPathToCheck = secretPath || secretSync.folder?.path || ""; + + if (environmentToCheck && secretPathToCheck) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.Edit, + subject(ProjectPermissionSub.SecretSyncs, { + environment: environmentToCheck, + secretPath: secretPathToCheck + }) + ); + } + } if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination]) throw new BadRequestError({ @@ -315,7 +347,7 @@ export const secretSyncServiceFactory = ({ if (!updatedEnvironment || !updatedSecretPath) throw new BadRequestError({ message: "Must specify both source environment and secret path" }); - throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.ReadValue, { + throwIfMissingSecretReadValueOrDescribePermission(permission, ProjectPermissionSecretActions.DescribeSecret, { environment: updatedEnvironment, secretPath: updatedSecretPath }); @@ -374,10 +406,20 @@ export const secretSyncServiceFactory = ({ projectId: secretSync.projectId }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretSyncActions.Delete, - ProjectPermissionSub.SecretSyncs - ); + if (secretSync.environment?.slug && secretSync.folder?.path) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.Delete, + subject(ProjectPermissionSub.SecretSyncs, { + environment: secretSync.environment.slug, + secretPath: secretSync.folder.path + }) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.Delete, + ProjectPermissionSub.SecretSyncs + ); + } if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination]) throw new BadRequestError({ @@ -441,10 +483,20 @@ export const secretSyncServiceFactory = ({ projectId: secretSync.projectId }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretSyncActions.SyncSecrets, - ProjectPermissionSub.SecretSyncs - ); + if (secretSync.environment?.slug && secretSync.folder?.path) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.SyncSecrets, + subject(ProjectPermissionSub.SecretSyncs, { + environment: secretSync.environment.slug, + secretPath: secretSync.folder.path + }) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.SyncSecrets, + ProjectPermissionSub.SecretSyncs + ); + } if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination]) throw new BadRequestError({ @@ -503,10 +555,20 @@ export const secretSyncServiceFactory = ({ projectId: secretSync.projectId }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretSyncActions.ImportSecrets, - ProjectPermissionSub.SecretSyncs - ); + if (secretSync.environment?.slug && secretSync.folder?.path) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.ImportSecrets, + subject(ProjectPermissionSub.SecretSyncs, { + environment: secretSync.environment.slug, + secretPath: secretSync.folder.path + }) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.ImportSecrets, + ProjectPermissionSub.SecretSyncs + ); + } if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination]) throw new BadRequestError({ @@ -559,10 +621,20 @@ export const secretSyncServiceFactory = ({ projectId: secretSync.projectId }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretSyncActions.RemoveSecrets, - ProjectPermissionSub.SecretSyncs - ); + if (secretSync.environment?.slug && secretSync.folder?.path) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.RemoveSecrets, + subject(ProjectPermissionSub.SecretSyncs, { + environment: secretSync.environment.slug, + secretPath: secretSync.folder.path + }) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretSyncActions.RemoveSecrets, + ProjectPermissionSub.SecretSyncs + ); + } if (secretSync.connection.app !== SECRET_SYNC_CONNECTION_MAP[destination]) throw new BadRequestError({ diff --git a/frontend/src/components/secret-syncs/forms/SecretSyncSourceFields.tsx b/frontend/src/components/secret-syncs/forms/SecretSyncSourceFields.tsx index 7cc19ae97..850ebbc80 100644 --- a/frontend/src/components/secret-syncs/forms/SecretSyncSourceFields.tsx +++ b/frontend/src/components/secret-syncs/forms/SecretSyncSourceFields.tsx @@ -1,17 +1,45 @@ +import { useEffect } from "react"; import { Controller, useFormContext } from "react-hook-form"; +import { subject } from "@casl/ability"; import { FilterableSelect, FormControl } from "@app/components/v2"; import { SecretPathInput } from "@app/components/v2/SecretPathInput"; -import { useWorkspace } from "@app/context"; +import { useProjectPermission, useWorkspace } from "@app/context"; +import { + ProjectPermissionSecretSyncActions, + ProjectPermissionSub +} from "@app/context/ProjectPermissionContext/types"; import { TSecretSyncForm } from "./schemas"; export const SecretSyncSourceFields = () => { - const { control, watch } = useFormContext(); + const { control, watch, setError, clearErrors } = useFormContext(); + const { permission } = useProjectPermission(); const { currentWorkspace } = useWorkspace(); const selectedEnvironment = watch("environment"); + const selectedSecretPath = watch("secretPath"); + + useEffect(() => { + const hasAccessToSource = + selectedEnvironment && + permission.can( + ProjectPermissionSecretSyncActions.Create, + subject(ProjectPermissionSub.SecretSyncs, { + environment: selectedEnvironment.slug, + secretPath: selectedSecretPath + }) + ); + + if (!hasAccessToSource) { + setError("secretPath", { + message: "You do not have permission to create secret syncs in this environment or path." + }); + } else { + clearErrors("secretPath"); + } + }, [selectedEnvironment, selectedSecretPath]); return ( <> diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index f9b2a828a..aa9087b1e 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/frontend/src/context/ProjectPermissionContext/types.ts @@ -263,6 +263,11 @@ export type SecretImportSubjectFields = { secretPath: string; }; +export type SecretSyncSubjectFields = { + environment: string; + secretPath: string; +}; + export type SecretRotationSubjectFields = { environment: string; secretPath: string; @@ -303,6 +308,13 @@ export type ProjectPermissionSet = | (ForcedSubject & DynamicSecretSubjectFields) ) ] + | [ + ProjectPermissionSecretSyncActions, + ( + | ProjectPermissionSub.SecretSyncs + | (ForcedSubject & SecretSyncSubjectFields) + ) + ] | [ ProjectPermissionActions, ( @@ -365,7 +377,6 @@ export type ProjectPermissionSet = ] | [ProjectPermissionActions, ProjectPermissionSub.PkiAlerts] | [ProjectPermissionActions, ProjectPermissionSub.PkiCollections] - | [ProjectPermissionSecretSyncActions, ProjectPermissionSub.SecretSyncs] | [ProjectPermissionActions.Delete, ProjectPermissionSub.Project] | [ProjectPermissionActions.Edit, ProjectPermissionSub.Project] | [ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback] diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx index d7872a223..6d95c403c 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx @@ -291,6 +291,13 @@ export const projectRoleFormSchema = z.object({ }) .array() .default([]), + [ProjectPermissionSub.SecretSyncs]: SecretSyncPolicyActionSchema.extend({ + inverted: z.boolean().optional(), + conditions: ConditionSchema + }) + .array() + .default([]), + [ProjectPermissionSub.Commits]: CommitPolicyActionSchema.array().default([]), [ProjectPermissionSub.Member]: MemberPolicyActionSchema.array().default([]), [ProjectPermissionSub.Groups]: GroupPolicyActionSchema.array().default([]), @@ -342,7 +349,6 @@ export const projectRoleFormSchema = z.object({ .default([]), [ProjectPermissionSub.Kms]: GeneralPolicyActionSchema.array().default([]), [ProjectPermissionSub.Cmek]: CmekPolicyActionSchema.array().default([]), - [ProjectPermissionSub.SecretSyncs]: SecretSyncPolicyActionSchema.array().default([]), [ProjectPermissionSub.Kmip]: KmipPolicyActionSchema.array().default([]), [ProjectPermissionSub.SecretScanningDataSources]: SecretScanningDataSourcePolicyActionSchema.array().default([]), @@ -366,7 +372,8 @@ type TConditionalFields = | ProjectPermissionSub.CertificateTemplates | ProjectPermissionSub.SshHosts | ProjectPermissionSub.SecretRotation - | ProjectPermissionSub.Identity; + | ProjectPermissionSub.Identity + | ProjectPermissionSub.SecretSyncs; export const isConditionalSubjects = ( subject: ProjectPermissionSub @@ -379,7 +386,8 @@ export const isConditionalSubjects = ( subject === ProjectPermissionSub.SshHosts || subject === ProjectPermissionSub.SecretRotation || subject === ProjectPermissionSub.PkiSubscribers || - subject === ProjectPermissionSub.CertificateTemplates; + subject === ProjectPermissionSub.CertificateTemplates || + subject === ProjectPermissionSub.SecretSyncs; const convertCaslConditionToFormOperator = (caslConditions: TPermissionCondition) => { const formConditions: z.infer = []; @@ -484,7 +492,8 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { ProjectPermissionSub.SshCertificateTemplates, ProjectPermissionSub.SshCertificateAuthorities, ProjectPermissionSub.SshCertificates, - ProjectPermissionSub.SshHostGroups + ProjectPermissionSub.SshHostGroups, + ProjectPermissionSub.SecretSyncs ].includes(subject) ) { // from above statement we are sure it won't be undefined @@ -786,7 +795,7 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { const canImportSecrets = action.includes(ProjectPermissionSecretSyncActions.ImportSecrets); const canRemoveSecrets = action.includes(ProjectPermissionSecretSyncActions.RemoveSecrets); - if (!formVal[subject]) formVal[subject] = [{}]; + if (!formVal[subject]) formVal[subject] = [{ conditions: [], inverted: false }]; // from above statement we are sure it won't be undefined if (canRead) formVal[subject]![0][ProjectPermissionSecretSyncActions.Read] = true; diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx index 31f63ac51..09f3920fd 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/RolePermissionsSection.tsx @@ -35,6 +35,7 @@ import { TFormSchema } from "./ProjectRoleModifySection.utils"; import { SecretPermissionConditions } from "./SecretPermissionConditions"; +import { SecretSyncPermissionConditions } from "./SecretSyncPermissionConditions"; import { SshHostPermissionConditions } from "./SshHostPermissionConditions"; type Props = { @@ -69,6 +70,10 @@ export const renderConditionalComponents = ( return ; } + if (subject === ProjectPermissionSub.SecretSyncs) { + return ; + } + return ; } diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/SecretSyncPermissionConditions.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/SecretSyncPermissionConditions.tsx new file mode 100644 index 000000000..c017be65a --- /dev/null +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/SecretSyncPermissionConditions.tsx @@ -0,0 +1,186 @@ +import { Controller, useFieldArray, useFormContext } from "react-hook-form"; +import { faInfoCircle, faPlus, faTrash, faWarning } from "@fortawesome/free-solid-svg-icons"; +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; + +import { + Button, + FormControl, + IconButton, + Input, + Select, + SelectItem, + Tooltip +} from "@app/components/v2"; +import { + PermissionConditionOperators, + ProjectPermissionSub +} from "@app/context/ProjectPermissionContext/types"; + +import { + getConditionOperatorHelperInfo, + renderOperatorSelectItems +} from "./PermissionConditionHelpers"; +import { TFormSchema } from "./ProjectRoleModifySection.utils"; + +type Props = { + position?: number; + isDisabled?: boolean; +}; + +export const SecretSyncPermissionConditions = ({ position = 0, isDisabled }: Props) => { + const { + control, + watch, + setValue, + formState: { errors } + } = useFormContext(); + const items = useFieldArray({ + control, + name: `permissions.${ProjectPermissionSub.SecretSyncs}.${position}.conditions` + }); + + const conditionErrorMessage = + errors?.permissions?.[ProjectPermissionSub.SecretSyncs]?.[position]?.conditions?.message || + errors?.permissions?.[ProjectPermissionSub.SecretSyncs]?.[position]?.conditions?.root?.message; + + return ( +
+

Conditions

+

+ Conditions determine when a policy will be applied (always if no conditions are present). +

+

+ All conditions must evaluate to true for the policy to take effect. +

+
+ {items.fields.map((el, index) => { + const condition = watch( + `permissions.${ProjectPermissionSub.SecretSyncs}.${position}.conditions.${index}` + ) as { + lhs: string; + rhs: string; + operator: string; + }; + return ( +
+
+ ( + + + + )} + /> +
+
+ ( + + + + )} + /> +
+ + + +
+
+
+ ( + + + + )} + /> +
+
+ items.remove(index)} + > + + +
+
+ ); + })} +
+ {conditionErrorMessage && ( +
+ + {conditionErrorMessage} +
+ )} +
+ +
+
+ ); +};