diff --git a/backend/src/db/migrations/20250218020306_backfill-secret-permissions-with-readvalue.ts b/backend/src/db/migrations/20250218020306_backfill-secret-permissions-with-readvalue.ts deleted file mode 100644 index 9e567cefe..000000000 --- a/backend/src/db/migrations/20250218020306_backfill-secret-permissions-with-readvalue.ts +++ /dev/null @@ -1,313 +0,0 @@ -import { MongoAbility, RawRuleOf } from "@casl/ability"; -import { PackRule, packRules, unpackRules } from "@casl/ability/extra"; -import { Knex } from "knex"; -import { z } from "zod"; - -import { selectAllTableCols } from "@app/lib/knex"; - -import { TableName } from "../schemas"; - -enum ProjectPermissionSub { - Secrets = "secrets" -} - -enum SecretActions { - Read = "read", - ReadValue = "readValue" -} - -const UnpackedPermissionSchema = z.object({ - subject: z - .union([z.string().min(1), z.string().array()]) - .transform((el) => (typeof el !== "string" ? el[0] : el)) - .optional(), - action: z.union([z.string().min(1), z.string().array()]).transform((el) => (typeof el === "string" ? [el] : el)), - conditions: z.unknown().optional(), - inverted: z.boolean().optional() -}); - -const $unpackPermissions = (permissions: unknown) => - UnpackedPermissionSchema.array().parse(unpackRules((permissions || []) as PackRule>[])); - -const $updatePermissionsUp = (permissions: unknown) => { - const parsedPermissions = $unpackPermissions(permissions); - let shouldUpdate = false; - - for (let i = 0; i < parsedPermissions.length; i += 1) { - const parsedPermission = parsedPermissions[i]; - const { subject, action } = parsedPermission; - - if (subject === ProjectPermissionSub.Secrets) { - if (action.includes(SecretActions.Read) && !action.includes(SecretActions.ReadValue)) { - action.push(SecretActions.ReadValue); - parsedPermissions[i] = { ...parsedPermission, action }; - shouldUpdate = true; - } - } - } - - return { - parsedPermissions, - shouldUpdate - }; -}; - -const $updatePermissionsDown = (permissions: unknown) => { - const parsedPermissions = $unpackPermissions(permissions); - - let shouldUpdate = false; - for (let i = 0; i < parsedPermissions.length; i += 1) { - const parsedPermission = parsedPermissions[i]; - - const { subject, action } = parsedPermission; - - if (subject === ProjectPermissionSub.Secrets) { - const readValueIndex = action.indexOf(SecretActions.ReadValue); - - if (action.includes(SecretActions.ReadValue) && readValueIndex !== -1) { - action.splice(readValueIndex, 1); - parsedPermissions[i] = { ...parsedPermission, action }; - - shouldUpdate = true; - } - } - } - - const repackedPermissions = packRules(parsedPermissions); - - return { - repackedPermissions, - shouldUpdate - }; -}; - -const CHUNK_SIZE = 1000; - -export async function up(knex: Knex): Promise { - const projectRoles = await knex(TableName.ProjectRoles).select(selectAllTableCols(TableName.ProjectRoles)); - const projectIdentityAdditionalPrivileges = await knex(TableName.IdentityProjectAdditionalPrivilege).select( - selectAllTableCols(TableName.IdentityProjectAdditionalPrivilege) - ); - const projectUserAdditionalPrivileges = await knex(TableName.ProjectUserAdditionalPrivilege).select( - selectAllTableCols(TableName.ProjectUserAdditionalPrivilege) - ); - - const serviceTokens = await knex(TableName.ServiceToken).select(selectAllTableCols(TableName.ServiceToken)); - - const updatedServiceTokens = serviceTokens.reduce((acc, serviceToken) => { - const { permissions } = serviceToken; // Service tokens are special, and include an array of actions only. - - if (permissions.includes(SecretActions.Read) && !permissions.includes(SecretActions.ReadValue)) { - permissions.push(SecretActions.ReadValue); - acc.push({ - ...serviceToken, - permissions - }); - } - return acc; - }, []); - - if (updatedServiceTokens.length > 0) { - for (let i = 0; i < updatedServiceTokens.length; i += CHUNK_SIZE) { - const chunk = updatedServiceTokens.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ServiceToken) - .whereIn( - "id", - chunk.map((t) => t.id) - ) - .update({ - // @ts-expect-error -- raw query - permissions: knex.raw( - `CASE id - ${chunk.map((t) => `WHEN '${t.id}' THEN ?::text[]`).join(" ")} - END`, - chunk.map((t) => t.permissions) - ) - }); - } - } - - const updatedRoles = projectRoles.reduce((acc, projectRole) => { - const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(projectRole.permissions); - - if (shouldUpdate) { - acc.push({ - ...projectRole, - permissions: JSON.stringify(packRules(parsedPermissions)) - }); - } - return acc; - }, []); - - const updatedIdentityAdditionalPrivileges = projectIdentityAdditionalPrivileges.reduce< - typeof projectIdentityAdditionalPrivileges - >((acc, identityAdditionalPrivilege) => { - const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(identityAdditionalPrivilege.permissions); - - if (shouldUpdate) { - acc.push({ - ...identityAdditionalPrivilege, - permissions: JSON.stringify(packRules(parsedPermissions)) - }); - } - return acc; - }, []); - - const updatedUserAdditionalPrivileges = projectUserAdditionalPrivileges.reduce< - typeof projectUserAdditionalPrivileges - >((acc, userAdditionalPrivilege) => { - const { shouldUpdate, parsedPermissions } = $updatePermissionsUp(userAdditionalPrivilege.permissions); - - if (shouldUpdate) { - acc.push({ - ...userAdditionalPrivilege, - permissions: JSON.stringify(packRules(parsedPermissions)) - }); - } - return acc; - }, []); - - if (updatedRoles.length > 0) { - for (let i = 0; i < updatedRoles.length; i += CHUNK_SIZE) { - const chunk = updatedRoles.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ProjectRoles).insert(chunk).onConflict("id").merge(["permissions"]); - } - } - - if (updatedIdentityAdditionalPrivileges.length > 0) { - for (let i = 0; i < updatedIdentityAdditionalPrivileges.length; i += CHUNK_SIZE) { - const chunk = updatedIdentityAdditionalPrivileges.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.IdentityProjectAdditionalPrivilege).insert(chunk).onConflict("id").merge(["permissions"]); - } - } - - if (updatedUserAdditionalPrivileges.length > 0) { - for (let i = 0; i < updatedUserAdditionalPrivileges.length; i += CHUNK_SIZE) { - const chunk = updatedUserAdditionalPrivileges.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ProjectUserAdditionalPrivilege).insert(chunk).onConflict("id").merge(["permissions"]); - } - } -} - -export async function down(knex: Knex): Promise { - const projectRoles = await knex(TableName.ProjectRoles).select(selectAllTableCols(TableName.ProjectRoles)); - const identityAdditionalPrivileges = await knex(TableName.IdentityProjectAdditionalPrivilege).select( - selectAllTableCols(TableName.IdentityProjectAdditionalPrivilege) - ); - const userAdditionalPrivileges = await knex(TableName.ProjectUserAdditionalPrivilege).select( - selectAllTableCols(TableName.ProjectUserAdditionalPrivilege) - ); - const serviceTokens = await knex(TableName.ServiceToken).select(selectAllTableCols(TableName.ServiceToken)); - - const updatedServiceTokens = serviceTokens.reduce((acc, serviceToken) => { - const { permissions } = serviceToken; - - if (permissions.includes(SecretActions.ReadValue)) { - permissions.splice(permissions.indexOf(SecretActions.ReadValue), 1); - acc.push({ - ...serviceToken, - permissions - }); - } - return acc; - }, []); - - if (updatedServiceTokens.length > 0) { - for (let i = 0; i < updatedServiceTokens.length; i += CHUNK_SIZE) { - const chunk = updatedServiceTokens.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ServiceToken) - .whereIn( - "id", - chunk.map((t) => t.id) - ) - .update({ - // @ts-expect-error -- raw query - permissions: knex.raw( - `CASE id - ${chunk.map((t) => `WHEN '${t.id}' THEN ?::text[]`).join(" ")} - END`, - chunk.map((t) => t.permissions) - ) - }); - } - } - - const updatedRoles = projectRoles.reduce((acc, projectRole) => { - const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(projectRole.permissions); - - if (shouldUpdate) { - acc.push({ - ...projectRole, - permissions: JSON.stringify(repackedPermissions) - }); - } - return acc; - }, []); - - const updatedIdentityAdditionalPrivileges = identityAdditionalPrivileges.reduce( - (acc, identityAdditionalPrivilege) => { - const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(identityAdditionalPrivilege.permissions); - - if (shouldUpdate) { - acc.push({ - ...identityAdditionalPrivilege, - permissions: JSON.stringify(repackedPermissions) - }); - } - return acc; - }, - [] - ); - - const updatedUserAdditionalPrivileges = userAdditionalPrivileges.reduce( - (acc, userAdditionalPrivilege) => { - const { shouldUpdate, repackedPermissions } = $updatePermissionsDown(userAdditionalPrivilege.permissions); - - if (shouldUpdate) { - acc.push({ - ...userAdditionalPrivilege, - permissions: JSON.stringify(repackedPermissions) - }); - } - return acc; - }, - [] - ); - - if (updatedRoles.length > 0) { - for (let i = 0; i < updatedRoles.length; i += CHUNK_SIZE) { - const chunk = updatedRoles.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ProjectRoles).insert(chunk).onConflict("id").merge(["permissions"]); - } - } - - if (updatedIdentityAdditionalPrivileges.length > 0) { - for (let i = 0; i < updatedIdentityAdditionalPrivileges.length; i += CHUNK_SIZE) { - const chunk = updatedIdentityAdditionalPrivileges.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.IdentityProjectAdditionalPrivilege).insert(chunk).onConflict("id").merge(["permissions"]); - } - } - - if (updatedUserAdditionalPrivileges.length > 0) { - for (let i = 0; i < updatedUserAdditionalPrivileges.length; i += CHUNK_SIZE) { - const chunk = updatedUserAdditionalPrivileges.slice(i, i + CHUNK_SIZE); - - // eslint-disable-next-line no-await-in-loop - await knex(TableName.ProjectUserAdditionalPrivilege).insert(chunk).onConflict("id").merge(["permissions"]); - } - } -} diff --git a/backend/src/ee/routes/v2/project-role-router.ts b/backend/src/ee/routes/v2/project-role-router.ts index 2d3b1984d..c3b3c11f2 100644 --- a/backend/src/ee/routes/v2/project-role-router.ts +++ b/backend/src/ee/routes/v2/project-role-router.ts @@ -2,6 +2,7 @@ import { packRules } from "@casl/ability/extra"; import { z } from "zod"; import { ProjectMembershipRole, ProjectRolesSchema } from "@app/db/schemas"; +import { checkForInvalidPermissionCombination } from "@app/ee/services/permission/permission-fns"; import { ProjectPermissionV2Schema } from "@app/ee/services/permission/project-permission"; import { PROJECT_ROLE } from "@app/lib/api-docs"; import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; @@ -92,7 +93,10 @@ export const registerProjectRoleRouter = async (server: FastifyZodProvider) => { .describe(PROJECT_ROLE.UPDATE.slug), name: z.string().trim().optional().describe(PROJECT_ROLE.UPDATE.name), description: z.string().trim().nullish().describe(PROJECT_ROLE.UPDATE.description), - permissions: ProjectPermissionV2Schema.array().describe(PROJECT_ROLE.UPDATE.permissions).optional() + permissions: ProjectPermissionV2Schema.array() + .describe(PROJECT_ROLE.UPDATE.permissions) + .optional() + .superRefine(checkForInvalidPermissionCombination) }), response: { 200: z.object({ diff --git a/backend/src/ee/services/permission/permission-fns.ts b/backend/src/ee/services/permission/permission-fns.ts index 80a58db0a..97113a562 100644 --- a/backend/src/ee/services/permission/permission-fns.ts +++ b/backend/src/ee/services/permission/permission-fns.ts @@ -1,7 +1,108 @@ +/* eslint-disable no-nested-ternary */ +import { ForbiddenError, MongoAbility, subject } from "@casl/ability"; +import { z } from "zod"; + import { TOrganizations } from "@app/db/schemas"; -import { ForbiddenRequestError, UnauthorizedError } from "@app/lib/errors"; +import { BadRequestError, ForbiddenRequestError, UnauthorizedError } from "@app/lib/errors"; import { ActorAuthMethod, AuthMethod } from "@app/services/auth/auth-type"; +import { + ProjectPermissionSecretActions, + ProjectPermissionSet, + ProjectPermissionSub, + ProjectPermissionV2Schema, + SecretSubjectFields +} from "./project-permission"; + +export function CheckForbiddenErrorSecretsSubject( + permission: MongoAbility, + action: Extract< + ProjectPermissionSecretActions, + ProjectPermissionSecretActions.ReadValue | ProjectPermissionSecretActions.DescribeSecret + >, + subjectFields?: SecretSubjectFields +) { + try { + if (subjectFields) { + ForbiddenError.from(permission).throwUnlessCan(action, subject(ProjectPermissionSub.Secrets, subjectFields)); + } else { + ForbiddenError.from(permission).throwUnlessCan(action, ProjectPermissionSub.Secrets); + } + } catch { + if (subjectFields) { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretActions.DescribeAndReadValue, + subject(ProjectPermissionSub.Secrets, subjectFields) + ); + } else { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionSecretActions.DescribeAndReadValue, + ProjectPermissionSub.Secrets + ); + } + } +} + +export function CheckCanSecretsSubject( + permission: MongoAbility, + action: Extract< + ProjectPermissionSecretActions, + ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue + >, + subjectFields?: SecretSubjectFields +) { + let canNewPermission = false; + let canOldPermission = false; + + if (subjectFields) { + canNewPermission = permission.can(action, subject(ProjectPermissionSub.Secrets, subjectFields)); + canOldPermission = permission.can( + ProjectPermissionSecretActions.DescribeAndReadValue, + subject(ProjectPermissionSub.Secrets, subjectFields) + ); + } else { + canNewPermission = permission.can(action, ProjectPermissionSub.Secrets); + canOldPermission = permission.can( + ProjectPermissionSecretActions.DescribeAndReadValue, + ProjectPermissionSub.Secrets + ); + } + + return canNewPermission || canOldPermission; +} + +const OptionalArrayPermissionSchema = ProjectPermissionV2Schema.array().optional(); +export function checkForInvalidPermissionCombination(permissions: z.infer) { + if (!permissions) return; + + for (const permission of permissions) { + if (permission.subject === ProjectPermissionSub.Secrets) { + if (permission.action.includes(ProjectPermissionSecretActions.DescribeAndReadValue)) { + const hasReadValue = permission.action.includes(ProjectPermissionSecretActions.ReadValue); + const hasDescribeSecret = permission.action.includes(ProjectPermissionSecretActions.DescribeSecret); + + if (!hasReadValue && !hasDescribeSecret) return; + + const hasBothDescribeAndReadValue = + permission.action.includes(ProjectPermissionSecretActions.DescribeSecret) && + permission.action.includes(ProjectPermissionSecretActions.ReadValue); + + throw new BadRequestError({ + message: `You have selected Full Read Access, and ${ + hasBothDescribeAndReadValue + ? "both Read Value and Describe Secret" + : hasReadValue + ? "Read Value" + : hasDescribeSecret + ? "Describe Secret" + : "" + }. You cannot select Read Value or Describe Secret if you have selected Full Read Access.` + }); + } + } + } +} + function isAuthMethodSaml(actorAuthMethod: ActorAuthMethod) { if (!actorAuthMethod) return false; diff --git a/backend/src/ee/services/permission/project-permission.ts b/backend/src/ee/services/permission/project-permission.ts index 3f69c481a..6256c9965 100644 --- a/backend/src/ee/services/permission/project-permission.ts +++ b/backend/src/ee/services/permission/project-permission.ts @@ -18,7 +18,8 @@ export enum ProjectPermissionActions { } export enum ProjectPermissionSecretActions { - DescribeSecret = "read", + DescribeAndReadValue = "read", + DescribeSecret = "describeSecret", ReadValue = "readValue", Create = "create", Edit = "edit", @@ -564,6 +565,7 @@ const buildAdminPermissionRules = () => { can( [ + // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSecretActions.ReadValue, ProjectPermissionSecretActions.Create, @@ -632,6 +634,7 @@ const buildMemberPermissionRules = () => { can( [ + // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSecretActions.ReadValue, ProjectPermissionSecretActions.Edit, @@ -808,6 +811,7 @@ export const projectMemberPermissions = buildMemberPermissionRules(); const buildViewerPermissionRules = () => { const { can, rules } = new AbilityBuilder>(createMongoAbility); + // not adding DescribeAndReadValue, because it's already covered by DescribeSecret and ReadValue can(ProjectPermissionSecretActions.DescribeSecret, ProjectPermissionSub.Secrets); can(ProjectPermissionSecretActions.ReadValue, ProjectPermissionSub.Secrets); can(ProjectPermissionActions.Read, ProjectPermissionSub.SecretFolders); @@ -852,7 +856,6 @@ export const buildServiceTokenProjectPermission = ( ) => { const canWrite = permission.includes("write"); const canRead = permission.includes("read"); - const canReadValue = permission.includes("readValue"); const { can, build } = new AbilityBuilder>(createMongoAbility); scopes.forEach(({ secretPath, environment }) => { @@ -876,7 +879,7 @@ export const buildServiceTokenProjectPermission = ( environment }); } - if (canRead) { + if (canRead && subject !== ProjectPermissionSub.Secrets) { can(ProjectPermissionActions.Read, subject, { // @ts-expect-error type secretPath: { $glob: secretPath }, @@ -884,12 +887,18 @@ export const buildServiceTokenProjectPermission = ( }); } - if (subject === ProjectPermissionSub.Secrets && canReadValue) { + if (subject === ProjectPermissionSub.Secrets && canRead) { // @ts-expect-error type can(ProjectPermissionSecretActions.ReadValue, subject as ProjectPermissionSub.Secrets, { secretPath: { $glob: secretPath }, environment }); + + // @ts-expect-error type + can(ProjectPermissionSecretActions.DescribeSecret, subject as ProjectPermissionSub.Secrets, { + secretPath: { $glob: secretPath }, + environment + }); } } ); diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts index 8efae75e6..9f3ec7111 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-service.ts @@ -77,6 +77,7 @@ import { TSecretApprovalDetailsDTO, TStatusChangeDTO } from "./secret-approval-request-types"; +import { CheckForbiddenErrorSecretsSubject } from "../permission/permission-fns"; type TSecretApprovalRequestServiceFactoryDep = { permissionService: Pick; @@ -917,10 +918,11 @@ export const secretApprovalRequestServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.SecretManager }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath }) - ); + + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath + }); await projectDAL.checkProjectUpgradeStatus(projectId); diff --git a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts index 397824904..6afc6bca3 100644 --- a/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts +++ b/backend/src/ee/services/secret-snapshot/secret-snapshot-service.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-argument */ // akhilmhdh: I did this, quite strange bug with eslint. Everything do have a type stil has this error -import { ForbiddenError, subject } from "@casl/ability"; +import { ForbiddenError } from "@casl/ability"; import { ActionProjectType, TableName, TSecretTagJunctionInsert, TSecretV2TagJunctionInsert } from "@app/db/schemas"; import { decryptSymmetric128BitHexKeyUTF8 } from "@app/lib/crypto"; @@ -12,6 +12,7 @@ import { TKmsServiceFactory } from "@app/services/kms/kms-service"; import { KmsDataKey } from "@app/services/kms/kms-types"; import { TProjectBotServiceFactory } from "@app/services/project-bot/project-bot-service"; import { TSecretDALFactory } from "@app/services/secret/secret-dal"; +import { INFISICAL_SECRET_VALUE_HIDDEN_MASK } from "@app/services/secret/secret-fns"; import { TSecretVersionDALFactory } from "@app/services/secret/secret-version-dal"; import { TSecretVersionTagDALFactory } from "@app/services/secret/secret-version-tag-dal"; import { TSecretFolderDALFactory } from "@app/services/secret-folder/secret-folder-dal"; @@ -22,6 +23,7 @@ import { TSecretVersionV2DALFactory } from "@app/services/secret-v2-bridge/secre import { TSecretVersionV2TagDALFactory } from "@app/services/secret-v2-bridge/secret-version-tag-dal"; import { TLicenseServiceFactory } from "../license/license-service"; +import { CheckCanSecretsSubject, CheckForbiddenErrorSecretsSubject } from "../permission/permission-fns"; import { TPermissionServiceFactory } from "../permission/permission-service"; import { ProjectPermissionActions, @@ -39,7 +41,6 @@ import { TSnapshotFolderDALFactory } from "./snapshot-folder-dal"; import { TSnapshotSecretDALFactory } from "./snapshot-secret-dal"; import { TSnapshotSecretV2DALFactory } from "./snapshot-secret-v2-dal"; import { getFullFolderPath } from "./snapshot-service-fns"; -import { INFISICAL_SECRET_VALUE_HIDDEN_MASK } from "@app/services/secret/secret-fns"; type TSecretSnapshotServiceFactoryDep = { snapshotDAL: TSnapshotDALFactory; @@ -102,10 +103,10 @@ export const secretSnapshotServiceFactory = ({ ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); // We need to check if the user has access to the secrets in the folder. If we don't do this, a user could theoretically access snapshot secret values even if they don't have read access to the secrets in the folder. - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath: path + }); const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) { @@ -139,10 +140,10 @@ export const secretSnapshotServiceFactory = ({ ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretRollback); // We need to check if the user has access to the secrets in the folder. If we don't do this, a user could theoretically access snapshot secret values even if they don't have read access to the secrets in the folder. - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath: path + }); const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) @@ -186,15 +187,12 @@ export const secretSnapshotServiceFactory = ({ snapshotDetails = { ...encryptedSnapshotDetails, secretVersions: encryptedSnapshotDetails.secretVersions.map((el) => { - const canReadValue = permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: encryptedSnapshotDetails.environment.slug, - secretPath: fullFolderPath, - secretName: el.key, - secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined - }) - ); + const canReadValue = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: encryptedSnapshotDetails.environment.slug, + secretPath: fullFolderPath, + secretName: el.key, + secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined + }); let secretValue = ""; if (canReadValue) { @@ -238,15 +236,12 @@ export const secretSnapshotServiceFactory = ({ key: botKey }); - const canReadValue = permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: encryptedSnapshotDetails.environment.slug, - secretPath: fullFolderPath, - secretName: secretKey, - secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined - }) - ); + const canReadValue = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: encryptedSnapshotDetails.environment.slug, + secretPath: fullFolderPath, + secretName: secretKey, + secretTags: el.tags.length ? el.tags.map((tag) => tag.slug) : undefined + }); let secretValue = ""; diff --git a/backend/src/server/routes/v2/service-token-router.ts b/backend/src/server/routes/v2/service-token-router.ts index 1a84c9123..fb10f17db 100644 --- a/backend/src/server/routes/v2/service-token-router.ts +++ b/backend/src/server/routes/v2/service-token-router.ts @@ -94,7 +94,7 @@ export const registerServiceTokenRouter = async (server: FastifyZodProvider) => iv: z.string().trim(), tag: z.string().trim(), expiresIn: z.number().nullable(), - permissions: z.enum(["read", "write", "readValue"]).array() + permissions: z.enum(["read", "write"]).array() }), response: { 200: z.object({ diff --git a/backend/src/services/integration/integration-service.ts b/backend/src/services/integration/integration-service.ts index f72357964..3f7c55896 100644 --- a/backend/src/services/integration/integration-service.ts +++ b/backend/src/services/integration/integration-service.ts @@ -1,6 +1,7 @@ -import { ForbiddenError, subject } from "@casl/ability"; +import { ForbiddenError } from "@casl/ability"; import { ActionProjectType } from "@app/db/schemas"; +import { CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, @@ -95,13 +96,10 @@ export const integrationServiceFactory = ({ }); ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Create, ProjectPermissionSub.Integrations); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: sourceEnvironment, - secretPath - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: sourceEnvironment, + secretPath + }); const folder = await folderDAL.findBySecretPath(integrationAuth.projectId, sourceEnvironment, secretPath); if (!folder) { @@ -178,13 +176,10 @@ export const integrationServiceFactory = ({ const newSecretPath = secretPath || integration.secretPath; if (environment || secretPath) { - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: newEnvironment, - secretPath: newSecretPath - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: newEnvironment, + secretPath: newSecretPath + }); } const folder = await folderDAL.findBySecretPath(integration.projectId, newEnvironment, newSecretPath); diff --git a/backend/src/services/project/project-service.ts b/backend/src/services/project/project-service.ts index 5367a70cd..b21cc99fe 100644 --- a/backend/src/services/project/project-service.ts +++ b/backend/src/services/project/project-service.ts @@ -10,6 +10,7 @@ import { } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; import { OrgPermissionActions, OrgPermissionSubjects } from "@app/ee/services/permission/org-permission"; +import { CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, @@ -764,10 +765,7 @@ export const projectServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.Any }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - ProjectPermissionSub.Secrets - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret); const project = await projectDAL.findProjectById(projectId); diff --git a/backend/src/services/secret-import/secret-import-service.ts b/backend/src/services/secret-import/secret-import-service.ts index 6c412edc0..27ed12c84 100644 --- a/backend/src/services/secret-import/secret-import-service.ts +++ b/backend/src/services/secret-import/secret-import-service.ts @@ -4,6 +4,7 @@ import { ForbiddenError, subject } from "@casl/ability"; import { ActionProjectType, TableName } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; +import { CheckCanSecretsSubject, CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, @@ -93,13 +94,11 @@ export const secretImportServiceFactory = ({ ); // check if user has permission to import from target path - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment: data.environment, - secretPath: data.path - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment: data.environment, + secretPath: data.path + }); + if (isReplication) { const plan = await licenseService.getPlan(actorOrgId); if (!plan.secretApproval) { @@ -405,13 +404,10 @@ export const secretImportServiceFactory = ({ if (!secretImportDoc.isReplication) throw new BadRequestError({ message: "Import is not in replication mode" }); // check if user has permission to import from target path - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment: secretImportDoc.importEnv.slug, - secretPath: secretImportDoc.importPath - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment: secretImportDoc.importEnv.slug, + secretPath: secretImportDoc.importPath + }); await projectDAL.checkProjectUpgradeStatus(projectId); @@ -599,14 +595,12 @@ export const secretImportServiceFactory = ({ // so anything based on this order will also be in right position const secretImports = await secretImportDAL.find({ folderId: folder.id, isReplication: false }); const allowedImports = secretImports.filter((el) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: el.importEnv.slug, - secretPath: el.importPath - }) - ) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: el.importEnv.slug, + secretPath: el.importPath + }) ); + return fnSecretsFromImports({ allowedImports, folderDAL, secretDAL, secretImportDAL }); }; @@ -651,16 +645,14 @@ export const secretImportServiceFactory = ({ secretImportDAL, decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : ""), hasSecretAccess: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }) }); + return importedSecrets; } @@ -671,13 +663,10 @@ export const secretImportServiceFactory = ({ }); const allowedImports = secretImports.filter((el) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: el.importEnv.slug, - secretPath: el.importPath - }) - ) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: el.importEnv.slug, + secretPath: el.importPath + }) ); const importedSecrets = await fnSecretsFromImports({ allowedImports, diff --git a/backend/src/services/secret-sync/secret-sync-service.ts b/backend/src/services/secret-sync/secret-sync-service.ts index 7b92a463c..d74ea120e 100644 --- a/backend/src/services/secret-sync/secret-sync-service.ts +++ b/backend/src/services/secret-sync/secret-sync-service.ts @@ -1,6 +1,7 @@ -import { ForbiddenError, subject } from "@casl/ability"; +import { ForbiddenError } from "@casl/ability"; import { ActionProjectType } from "@app/db/schemas"; +import { CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionSecretActions, @@ -178,13 +179,10 @@ export const secretSyncServiceFactory = ({ ProjectPermissionSub.SecretSyncs ); - ForbiddenError.from(projectPermission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath - }) - ); + CheckForbiddenErrorSecretsSubject(projectPermission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath + }); const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); @@ -269,13 +267,10 @@ export const secretSyncServiceFactory = ({ if (!updatedEnvironment || !updatedSecretPath) throw new BadRequestError({ message: "Must specify both source environment and secret path" }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: updatedEnvironment, - secretPath: updatedSecretPath - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: updatedEnvironment, + secretPath: updatedSecretPath + }); const newFolder = await folderDAL.findBySecretPath(secretSync.projectId, updatedEnvironment, updatedSecretPath); 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 3b7f04907..8ebc0a582 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 @@ -10,6 +10,7 @@ import { TableName, TSecretsV2 } from "@app/db/schemas"; +import { CheckCanSecretsSubject, CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, @@ -540,17 +541,14 @@ export const secretV2BridgeServiceFactory = ({ }); } - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName: inputSecret.secretName, - ...(tagsToCheck.length && { - secretTags: tagsToCheck.map((el) => el.slug) - }) + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName: inputSecret.secretName, + ...(tagsToCheck.length && { + secretTags: tagsToCheck.map((el) => el.slug) }) - ); + }); return reshapeBridgeSecret( projectId, @@ -651,15 +649,12 @@ export const secretV2BridgeServiceFactory = ({ projectId }); - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName: secretToDelete.key, - secretTags: secretToDelete.tags?.map((el) => el.slug) - }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName: secretToDelete.key, + secretTags: secretToDelete.tags?.map((el) => el.slug) + }); return reshapeBridgeSecret( projectId, @@ -702,11 +697,7 @@ export const secretV2BridgeServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.SecretManager }); - - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - ProjectPermissionSub.Secrets - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret); } const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environments, path); @@ -752,11 +743,7 @@ export const secretV2BridgeServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.SecretManager }); - - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - ProjectPermissionSub.Secrets - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret); const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) return 0; @@ -791,8 +778,20 @@ export const secretV2BridgeServiceFactory = ({ }); const decryptedSecrets = secrets - .filter((el) => - projectPermission.can( + .filter((el) => { + if ( + filterByAction === ProjectPermissionSecretActions.ReadValue || + filterByAction === ProjectPermissionSecretActions.DescribeSecret + ) { + return CheckCanSecretsSubject(projectPermission, filterByAction, { + environment: groupedFolderMappings[el.folderId][0].environment, + secretPath: groupedFolderMappings[el.folderId][0].path, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }); + } + + return projectPermission.can( filterByAction, subject(ProjectPermissionSub.Secrets, { environment: groupedFolderMappings[el.folderId][0].environment, @@ -800,19 +799,16 @@ export const secretV2BridgeServiceFactory = ({ secretName: el.key, secretTags: el.tags.map((i) => i.slug) }) - ) - ) + ); + }) .map((secret) => { // Note(Daniel): This is only relevant if the filterAction isn't set to ReadValue. This is needed for the frontend. - const secretValueHidden = !projectPermission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: groupedFolderMappings[secret.folderId][0].environment, - secretPath: groupedFolderMappings[secret.folderId][0].path, - secretName: secret.key, - secretTags: secret.tags.map((i) => i.slug) - }) - ); + const secretValueHidden = !CheckCanSecretsSubject(projectPermission, ProjectPermissionSecretActions.ReadValue, { + environment: groupedFolderMappings[secret.folderId][0].environment, + secretPath: groupedFolderMappings[secret.folderId][0].path, + secretName: secret.key, + secretTags: secret.tags.map((i) => i.slug) + }); return reshapeBridgeSecret( projectId, @@ -858,10 +854,7 @@ export const secretV2BridgeServiceFactory = ({ actionProjectType: ActionProjectType.SecretManager }); if (!isInternal) { - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - ProjectPermissionSub.Secrets - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret); } const folders = await folderDAL.findBySecretPathMultiEnv(projectId, environments, path); @@ -913,15 +906,11 @@ export const secretV2BridgeServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.SecretManager }); - - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: path, - secretTags: params.tagSlugs - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath: path, + secretTags: params.tagSlugs + }); let paths: { folderId: string; path: string }[] = []; @@ -960,15 +949,12 @@ export const secretV2BridgeServiceFactory = ({ const decryptedSecrets = secrets .filter((el) => { - const canDescribeSecret = permission.can( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: groupedPaths[el.folderId][0].path, - secretName: el.key, - secretTags: el.tags.map((i) => i.slug) - }) - ); + const canDescribeSecret = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath: groupedPaths[el.folderId][0].path, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }); if (!canDescribeSecret) { return false; @@ -977,14 +963,15 @@ export const secretV2BridgeServiceFactory = ({ if (viewSecretValue) { // Recursive secret, should be filtered out if (groupedPaths[el.folderId][0].path !== path) { - const canReadRecursiveSecretValue = permission.can( + const canReadRecursiveSecretValue = CheckCanSecretsSubject( + permission, ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { + { environment, secretPath: groupedPaths[el.folderId][0].path, secretName: el.key, secretTags: el.tags.map((i) => i.slug) - }) + } ); if (!canReadRecursiveSecretValue) { @@ -993,15 +980,12 @@ export const secretV2BridgeServiceFactory = ({ } if (throwOnMissingReadValuePermission) { - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: groupedPaths[el.folderId][0].path, - secretName: el.key, - secretTags: el.tags.map((i) => i.slug) - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: groupedPaths[el.folderId][0].path, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }); } // Else, we do nothing. Because we don't want to filter out the secret, OR throw an error. // If the user doesn't have access to read the value, in the below map function, we mask the secret value and return the secret with a hidden value. @@ -1014,15 +998,12 @@ export const secretV2BridgeServiceFactory = ({ const secretValueHidden = !viewSecretValue || - !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: groupedPaths[secret.folderId][0].path, - secretName: secret.key, - secretTags: secret.tags.map((i) => i.slug) - }) - ); + !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: groupedPaths[secret.folderId][0].path, + secretName: secret.key, + secretTags: secret.tags.map((i) => i.slug) + }); return reshapeBridgeSecret( projectId, @@ -1047,15 +1028,12 @@ export const secretV2BridgeServiceFactory = ({ secretDAL, decryptSecretValue: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : undefined), canExpandValue: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }) }); if (shouldExpandSecretReferences) { @@ -1095,25 +1073,19 @@ export const secretV2BridgeServiceFactory = ({ expandSecretReferences, decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : ""), hasSecretAccess: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => { - const canDescribe = permission.can( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ); + const canDescribe = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }); - const canReadValue = permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ); + const canReadValue = CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }); return viewSecretValue ? canDescribe && canReadValue : canDescribe; } @@ -1264,15 +1236,12 @@ export const secretV2BridgeServiceFactory = ({ }) )); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: path, - secretName, - secretTags: (secret?.tags || []).map((el) => el.slug) - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath: path, + secretName, + secretTags: (secret?.tags || []).map((el) => el.slug) + }); // this will throw if the user doesn't have read value permission no matter what // because if its an expansion, it will fully depend on the value. @@ -1282,15 +1251,12 @@ export const secretV2BridgeServiceFactory = ({ secretDAL, decryptSecretValue: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : undefined), canExpandValue: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => { - return permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ); + return CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }); } }); @@ -1310,15 +1276,12 @@ export const secretV2BridgeServiceFactory = ({ decryptor: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : ""), expandSecretReferences: shouldExpandSecretReferences ? expandSecretReferences : undefined, hasSecretAccess: (expandEnvironment, expandSecretPath, expandSecretKey, expandSecretTags) => { - return permission.can( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretKey, - secretTags: expandSecretTags - }) - ); + return CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretKey, + secretTags: expandSecretTags + }); } }); @@ -1330,15 +1293,12 @@ export const secretV2BridgeServiceFactory = ({ if (viewSecretValue) { if ( - !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: importedSecret.environment, - secretPath: importedSecrets[i].secretPath, - secretName: importedSecret.key, - secretTags: (importedSecret.secretTags || []).map((el) => el.slug) - }) - ) && + !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: importedSecret.environment, + secretPath: importedSecrets[i].secretPath, + secretName: importedSecret.key, + secretTags: (importedSecret.secretTags || []).map((el) => el.slug) + }) && secretType !== SecretType.Personal ) { throw new ForbiddenRequestError({ @@ -1386,15 +1346,12 @@ export const secretV2BridgeServiceFactory = ({ if (viewSecretValue) { if ( - !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: path, - secretName, - secretTags: (secret?.tags || []).map((el) => el.slug) - }) - ) && + !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path, + secretName, + secretTags: (secret?.tags || []).map((el) => el.slug) + }) && secretType !== SecretType.Personal ) { throw new ForbiddenRequestError({ @@ -1562,15 +1519,12 @@ export const secretV2BridgeServiceFactory = ({ }); return newSecrets.map((el) => { - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName: el.key, - secretTags: el.tags?.map((i) => i.slug) - }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName: el.key, + secretTags: el.tags?.map((i) => i.slug) + }); return reshapeBridgeSecret( projectId, @@ -1899,15 +1853,12 @@ export const secretV2BridgeServiceFactory = ({ ); return updatedSecrets.map((el) => { - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: el.secretPath, - secretName: el.key, - secretTags: el.tags.map((i) => i.slug) - }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: el.secretPath, + secretName: el.key, + secretTags: el.tags.map((i) => i.slug) + }); return { ...reshapeBridgeSecret( @@ -2032,15 +1983,12 @@ export const secretV2BridgeServiceFactory = ({ const secretValueHidden = !secretToDeleteMatch || - !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName: el.key, - secretTags: secretToDeleteMatch.tags?.map((i) => i.slug) - }) - ); + !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName: el.key, + secretTags: secretToDeleteMatch.tags?.map((i) => i.slug) + }); return reshapeBridgeSecret( projectId, @@ -2097,17 +2045,15 @@ export const secretV2BridgeServiceFactory = ({ sort: [["createdAt", "desc"]] }); return secretVersions.map((el) => { - const secretValueHidden = permission.cannot( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: folder.environment.envSlug, - secretPath: folderWithPath.path, - secretName: el.key, - ...(el.tags?.length && { - secretTags: el.tags.map((tag) => tag.slug) - }) + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: folder.environment.envSlug, + secretPath: folderWithPath.path, + secretName: el.key, + ...(el.tags?.length && { + secretTags: el.tags.map((tag) => tag.slug) }) - ); + }); + return reshapeBridgeSecret( folder.projectId, folder.environment.envSlug, @@ -2224,15 +2170,27 @@ export const secretV2BridgeServiceFactory = ({ sourceSecrets.forEach((secret) => { for (const sourceAction of sourceActions) { - ForbiddenError.from(permission).throwUnlessCan( - sourceAction, - subject(ProjectPermissionSub.Secrets, { + if ( + sourceAction === ProjectPermissionSecretActions.DescribeSecret || + sourceAction === ProjectPermissionSecretActions.ReadValue + ) { + CheckForbiddenErrorSecretsSubject(permission, sourceAction, { environment: sourceEnvironment, secretPath: sourceSecretPath, secretName: secret.key, secretTags: secret.tags.map((el) => el.slug) - }) - ); + }); + } else { + ForbiddenError.from(permission).throwUnlessCan( + sourceAction, + subject(ProjectPermissionSub.Secrets, { + environment: sourceEnvironment, + secretPath: sourceSecretPath, + secretName: secret.key, + secretTags: secret.tags.map((el) => el.slug) + }) + ); + } } }); @@ -2555,10 +2513,10 @@ export const secretV2BridgeServiceFactory = ({ actionProjectType: ActionProjectType.SecretManager }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { environment, secretPath }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath + }); const folder = await folderDAL.findBySecretPath(projectId, environment, secretPath); if (!folder) @@ -2579,15 +2537,12 @@ export const secretV2BridgeServiceFactory = ({ type: SecretType.Shared }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName, - secretTags: (secret?.tags || []).map((el) => el.slug) - }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath, + secretName, + secretTags: (secret?.tags || []).map((el) => el.slug) + }); const decryptedSecretValue = secret.encryptedValue ? secretManagerDecryptor({ cipherTextBlob: secret.encryptedValue }).toString() @@ -2599,27 +2554,21 @@ export const secretV2BridgeServiceFactory = ({ secretDAL, decryptSecretValue: (value) => (value ? secretManagerDecryptor({ cipherTextBlob: value }).toString() : undefined), canExpandValue: (expandEnvironment, expandSecretPath, expandSecretName, expandSecretTags) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: expandEnvironment, - secretPath: expandSecretPath, - secretName: expandSecretName, - secretTags: expandSecretTags - }) - ) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: expandEnvironment, + secretPath: expandSecretPath, + secretName: expandSecretName, + secretTags: expandSecretTags + }) }); if ( - !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName, - secretTags: (secret?.tags || []).map((el) => el.slug) - }) - ) + !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath, + secretName, + secretTags: (secret?.tags || []).map((el) => el.slug) + }) ) { throw new ForbiddenRequestError({ message: `Unable to get secret reference tree for secret with key '${secretName}', because you don't have permission to view secret value.` diff --git a/backend/src/services/secret/secret-fns.ts b/backend/src/services/secret/secret-fns.ts index 4556528a8..ddaa054ab 100644 --- a/backend/src/services/secret/secret-fns.ts +++ b/backend/src/services/secret/secret-fns.ts @@ -1,5 +1,4 @@ /* eslint-disable no-await-in-loop */ -import { subject } from "@casl/ability"; import path from "path"; import { @@ -12,8 +11,9 @@ import { TSecretFolders, TSecrets } from "@app/db/schemas"; +import { CheckCanSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; -import { ProjectPermissionSecretActions, ProjectPermissionSub } from "@app/ee/services/permission/project-permission"; +import { ProjectPermissionSecretActions } from "@app/ee/services/permission/project-permission"; import { getConfig } from "@app/lib/config/env"; import { buildSecretBlindIndexFromName, @@ -191,13 +191,10 @@ export const recursivelyGetSecretPaths = ({ // Filter out paths that the user does not have permission to access, and paths that are not in the current path const allowedPaths = paths.filter( (folder) => - permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: folder.path - }) - ) && folder.path.startsWith(currentPath === "/" ? "" : currentPath) + CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: folder.path + }) && folder.path.startsWith(currentPath === "/" ? "" : currentPath) ); return allowedPaths; diff --git a/backend/src/services/secret/secret-service.ts b/backend/src/services/secret/secret-service.ts index 8af0deb2b..e3eb75bda 100644 --- a/backend/src/services/secret/secret-service.ts +++ b/backend/src/services/secret/secret-service.ts @@ -13,6 +13,7 @@ import { SecretType } from "@app/db/schemas"; import { TLicenseServiceFactory } from "@app/ee/services/license/license-service"; +import { CheckCanSecretsSubject, CheckForbiddenErrorSecretsSubject } from "@app/ee/services/permission/permission-fns"; import { TPermissionServiceFactory } from "@app/ee/services/permission/permission-service"; import { ProjectPermissionActions, @@ -452,13 +453,10 @@ export const secretServiceFactory = ({ }); } - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath: path - }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); return { ...updatedSecret[0], @@ -562,10 +560,10 @@ export const secretServiceFactory = ({ }); } - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); return { ...deletedSecret[0], @@ -622,10 +620,10 @@ export const secretServiceFactory = ({ paths = deepPaths.map(({ folderId, path: p }) => ({ folderId, path: p })); } else { - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) return { secrets: [], imports: [] }; @@ -647,13 +645,10 @@ export const secretServiceFactory = ({ // if its service token allow full access over imported one actor === ActorType.SERVICE ? true - : permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: importEnv.slug, - secretPath: importPath - }) - ) + : CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: importEnv.slug, + secretPath: importPath + }) ); const importedSecrets = await fnSecretsFromImports({ allowedImports, @@ -704,10 +699,11 @@ export const secretServiceFactory = ({ actorOrgId, actionProjectType: ActionProjectType.SecretManager }); - ForbiddenError.from(permission).throwUnlessCan( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + CheckForbiddenErrorSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); + const folder = await folderDAL.findBySecretPath(projectId, environment, path); if (!folder) throw new NotFoundError({ @@ -754,14 +750,12 @@ export const secretServiceFactory = ({ // if its service token allow full access over imported one actor === ActorType.SERVICE ? true - : permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: importEnv.slug, - secretPath: importPath - }) - ) + : CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: importEnv.slug, + secretPath: importPath + }) ); + const importedSecrets = await fnSecretsFromImports({ allowedImports, secretDAL, @@ -975,10 +969,10 @@ export const secretServiceFactory = ({ secretVersionTagDAL }); - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); return updatedSecrets.map((secret) => ({ ...secret, @@ -1069,11 +1063,10 @@ export const secretServiceFactory = ({ }); } } - - const secretValueHidden = !permission.can( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { environment, secretPath: path }) - ); + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment, + secretPath: path + }); return secrets.map((secret) => ({ ...secret, @@ -1259,8 +1252,20 @@ export const secretServiceFactory = ({ ProjectPermissionSecretActions.Delete, ProjectPermissionSecretActions.Create, ProjectPermissionSecretActions.Edit - ].filter((action) => - entityPermission.permission.can( + ].filter((action) => { + if ( + action === ProjectPermissionSecretActions.DescribeSecret || + action === ProjectPermissionSecretActions.ReadValue + ) { + return CheckCanSecretsSubject(entityPermission.permission, action, { + environment, + secretPath, + secretName, + secretTags: secret?.tags?.map((el) => el.slug) + }); + } + + return entityPermission.permission.can( action, subject(ProjectPermissionSub.Secrets, { environment, @@ -1268,8 +1273,8 @@ export const secretServiceFactory = ({ secretName, secretTags: secret?.tags?.map((el) => el.slug) }) - ) - ); + ); + }); return { ...entityPermission, @@ -2424,17 +2429,14 @@ export const secretServiceFactory = ({ key: botKey }); - const secretValueHidden = permission.cannot( - ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { - environment: folder.environment.envSlug, - secretPath: folderWithPath.path, - secretName: secretKey, - ...(el.tags?.length && { - secretTags: el.tags.map((tag) => tag.slug) - }) + const secretValueHidden = !CheckCanSecretsSubject(permission, ProjectPermissionSecretActions.ReadValue, { + environment: folder.environment.envSlug, + secretPath: folderWithPath.path, + secretName: secretKey, + ...(el.tags?.length && { + secretTags: el.tags.map((tag) => tag.slug) }) - ); + }); return decryptSecretRaw( { @@ -2833,13 +2835,23 @@ export const secretServiceFactory = ({ } for (const sourceAction of sourceActions) { - ForbiddenError.from(permission).throwUnlessCan( - sourceAction, - subject(ProjectPermissionSub.Secrets, { + if ( + sourceAction === ProjectPermissionSecretActions.ReadValue || + sourceAction === ProjectPermissionSecretActions.DescribeSecret + ) { + CheckForbiddenErrorSecretsSubject(permission, sourceAction, { environment: sourceEnvironment, secretPath: sourceSecretPath - }) - ); + }); + } else { + ForbiddenError.from(permission).throwUnlessCan( + sourceAction, + subject(ProjectPermissionSub.Secrets, { + environment: sourceEnvironment, + secretPath: sourceSecretPath + }) + ); + } } return { diff --git a/backend/src/services/service-token/service-token-types.ts b/backend/src/services/service-token/service-token-types.ts index d06d90a9d..f1a700908 100644 --- a/backend/src/services/service-token/service-token-types.ts +++ b/backend/src/services/service-token/service-token-types.ts @@ -7,7 +7,7 @@ export type TCreateServiceTokenDTO = { iv: string; tag: string; expiresIn?: number | null; - permissions: ("read" | "write" | "readValue")[]; + permissions: ("read" | "write")[]; } & TProjectPermission; export type TGetServiceTokenInfoDTO = Omit; diff --git a/frontend/src/context/ProjectPermissionContext/types.ts b/frontend/src/context/ProjectPermissionContext/types.ts index 260ffce09..495055977 100644 --- a/frontend/src/context/ProjectPermissionContext/types.ts +++ b/frontend/src/context/ProjectPermissionContext/types.ts @@ -8,7 +8,8 @@ export enum ProjectPermissionActions { } export enum ProjectPermissionSecretActions { - DescribeSecret = "read", + DescribeAndReadValue = "read", + DescribeSecret = "describeSecret", ReadValue = "readValue", Create = "create", Edit = "edit", diff --git a/frontend/src/lib/fn/permission.ts b/frontend/src/lib/fn/permission.ts new file mode 100644 index 000000000..f2077d76b --- /dev/null +++ b/frontend/src/lib/fn/permission.ts @@ -0,0 +1,36 @@ +import { MongoAbility, subject } from "@casl/ability"; + +import { ProjectPermissionSet } from "@app/context/ProjectPermissionContext"; +import { + ProjectPermissionSecretActions, + ProjectPermissionSub, + SecretSubjectFields +} from "@app/context/ProjectPermissionContext/types"; + +export function secretsPermissionCan( + permission: MongoAbility, + action: Extract< + ProjectPermissionSecretActions, + ProjectPermissionSecretActions.DescribeSecret | ProjectPermissionSecretActions.ReadValue + >, + subjectFields?: SecretSubjectFields +) { + let canNewPermission = false; + let canOldPermission = false; + + if (subjectFields) { + canNewPermission = permission.can(action, subject(ProjectPermissionSub.Secrets, subjectFields)); + canOldPermission = permission.can( + ProjectPermissionSecretActions.DescribeAndReadValue, + subject(ProjectPermissionSub.Secrets, subjectFields) + ); + } else { + canNewPermission = permission.can(action, ProjectPermissionSub.Secrets); + canOldPermission = permission.can( + ProjectPermissionSecretActions.DescribeAndReadValue, + ProjectPermissionSub.Secrets + ); + } + + return canNewPermission || canOldPermission; +} diff --git a/frontend/src/pages/project/AccessControlPage/components/ServiceTokenTab/components/ServiceTokenSection/AddServiceTokenModal.tsx b/frontend/src/pages/project/AccessControlPage/components/ServiceTokenTab/components/ServiceTokenSection/AddServiceTokenModal.tsx index a669a8360..fa9500a53 100644 --- a/frontend/src/pages/project/AccessControlPage/components/ServiceTokenTab/components/ServiceTokenSection/AddServiceTokenModal.tsx +++ b/frontend/src/pages/project/AccessControlPage/components/ServiceTokenTab/components/ServiceTokenSection/AddServiceTokenModal.tsx @@ -303,13 +303,9 @@ export const AddServiceTokenModal = ({ popUp, handlePopUpToggle }: Props) => { render={({ field: { onChange, value }, fieldState: { error } }) => { const options = [ { - label: "Describe Secret (default)", + label: "Read (default)", value: "read" }, - { - label: "Read Value (optional)", - value: "readValue" - }, { label: "Write (optional)", value: "write" diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx index 8aca40a34..a6e472976 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/GeneralPermissionPolicies.tsx @@ -35,12 +35,13 @@ export const GeneralPermissionPolicies = ) => { - const { control } = useFormContext(); + const { control, watch } = useFormContext(); const items = useFieldArray({ control, name: `permissions.${subject}` }); const [isOpen, setIsOpen] = useToggle(); + // const [hideFullReadAccess, setHideFullReadAccess] = useState(false); if (!items.fields.length) return
; @@ -71,119 +72,138 @@ export const GeneralPermissionPolicies = {isOpen && (
- {items.fields.map((el, rootIndex) => ( -
- {isConditionalSubjects(subject) && ( -
-
Permission
-
- ( - - )} - /> -
-
- -

- Whether to allow or forbid the selected actions when the following - conditions (if any) are met. -

-

Forbid rules must come after allow rules.

- - } - > - - -
-
- )} -
-
Actions
-
- {actions.map(({ label, value }) => { - if (typeof value !== "string") return undefined; + {items.fields.map((el, rootIndex) => { + let isFullReadAccessEnabled = false; - return ( + if (subject === ProjectPermissionSub.Secrets) { + isFullReadAccessEnabled = watch(`permissions.${subject}.${rootIndex}.read` as any); + } + + return ( +
+ {isConditionalSubjects(subject) && ( +
+
Permission
+
{ - return ( -
- - {label} - -
- ); - }} + defaultValue={false as any} + name={`permissions.${subject}.${rootIndex}.inverted`} + render={({ field }) => ( + + )} /> - ); +
+
+ +

+ Whether to allow or forbid the selected actions when the following + conditions (if any) are met. +

+

Forbid rules must come after allow rules.

+ + } + > + + +
+
+ )} +
+
Actions
+
+ {actions.map(({ label, value }, index) => { + if (typeof value !== "string") return undefined; + + if ( + subject === ProjectPermissionSub.Secrets && + value === "read" && + !isFullReadAccessEnabled + ) { + return null; + } + + return ( + { + return ( +
+ + {label} + +
+ ); + }} + /> + ); + })} +
+
+ {children && + cloneElement(children, { + position: rootIndex })} +
+ {!isDisabled && isConditionalSubjects(subject) && ( + + )} + {!isDisabled && ( + + )}{" "}
- {children && - cloneElement(children, { - position: rootIndex - })} -
- {!isDisabled && isConditionalSubjects(subject) && ( - - )} - {!isDisabled && ( - - )}{" "} -
-
- ))} + ); + })}
)}
diff --git a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx index e26ec8bc4..9f13b53a8 100644 --- a/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx +++ b/frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx @@ -1,5 +1,9 @@ +import { ReactNode } from "react"; +import { faWarning } from "@fortawesome/free-solid-svg-icons"; +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { z } from "zod"; +import { Tooltip } from "@app/components/v2"; import { ProjectPermissionActions, ProjectPermissionCmekActions, @@ -24,11 +28,12 @@ const GeneralPolicyActionSchema = z.object({ }); const SecretPolicyActionSchema = z.object({ - read: z.boolean().optional(), // describe secret - edit: z.boolean().optional(), - delete: z.boolean().optional(), - create: z.boolean().optional(), - readValue: z.boolean().optional() + [ProjectPermissionSecretActions.DescribeAndReadValue]: z.boolean().optional(), // existing read, gives both describe and read value + [ProjectPermissionSecretActions.DescribeSecret]: z.boolean().optional(), // describe secret, cannot read value + [ProjectPermissionSecretActions.ReadValue]: z.boolean().optional(), // read value + [ProjectPermissionSecretActions.Edit]: z.boolean().optional(), // edit secret + [ProjectPermissionSecretActions.Delete]: z.boolean().optional(), // delete secret + [ProjectPermissionSecretActions.Create]: z.boolean().optional() // create secret }); const CmekPolicyActionSchema = z.object({ @@ -294,19 +299,24 @@ export const rolePermission2Form = (permissions: TProjectPermission[] = []) => { } if (subject === ProjectPermissionSub.Secrets) { - const canRead = action.includes(ProjectPermissionSecretActions.DescribeSecret); + const canDescribeAndReadValue = action.includes( + ProjectPermissionSecretActions.DescribeAndReadValue + ); + const canDescribe = action.includes(ProjectPermissionSecretActions.DescribeSecret); + const canReadValue = action.includes(ProjectPermissionSecretActions.ReadValue); + const canEdit = action.includes(ProjectPermissionSecretActions.Edit); const canDelete = action.includes(ProjectPermissionSecretActions.Delete); const canCreate = action.includes(ProjectPermissionSecretActions.Create); - const canReadValue = action.includes(ProjectPermissionSecretActions.ReadValue); // from above statement we are sure it won't be undefined formVal[subject]!.push({ - read: canRead, + describeSecret: canDescribe, + read: canDescribeAndReadValue, + readValue: canReadValue, create: canCreate, edit: canEdit, delete: canDelete, - readValue: canReadValue, conditions: conditions ? convertCaslConditionToFormOperator(conditions) : [], inverted }); @@ -501,7 +511,7 @@ export type TProjectPermissionObject = { [K in ProjectPermissionSub]: { title: string; actions: { - label: string; + label: string | ReactNode; value: keyof Omit< NonNullable[K]>[number], "conditions" | "inverted" @@ -514,11 +524,35 @@ export const PROJECT_PERMISSION_OBJECT: TProjectPermissionObject = { [ProjectPermissionSub.Secrets]: { title: "Secrets", actions: [ - { label: "Describe Secret", value: "read" }, - { label: "Create", value: "create" }, - { label: "Read Value", value: "readValue" }, - { label: "Modify", value: "edit" }, - { label: "Remove", value: "delete" } + { + label: ( +
+

+ Read (legacy) +

+ + This is a legacy action and will be removed in the future. +
+
You should instead use the{" "} + Describe Secret and{" "} + Read Value actions. +
+ } + > + + +
+ ), + value: ProjectPermissionSecretActions.DescribeAndReadValue + }, + { label: "Describe Secret", value: ProjectPermissionSecretActions.DescribeSecret }, + { label: "Read Value", value: ProjectPermissionSecretActions.ReadValue }, + { label: "Modify", value: ProjectPermissionSecretActions.Edit }, + { label: "Remove", value: ProjectPermissionSecretActions.Delete }, + { label: "Create", value: ProjectPermissionSecretActions.Create } ] }, [ProjectPermissionSub.SecretFolders]: { diff --git a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx index 3b05f10fa..536f75542 100644 --- a/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx +++ b/frontend/src/pages/secret-manager/OverviewPage/components/SecretOverviewTableRow/SecretRenameRow.tsx @@ -15,6 +15,7 @@ import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionCo import { useToggle } from "@app/hooks"; import { useUpdateSecretV3 } from "@app/hooks/api"; import { SecretType, SecretV3RawSanitized } from "@app/hooks/api/types"; +import { secretsPermissionCan } from "@app/lib/fn/permission"; enum SecretActionType { Created = "created", @@ -51,8 +52,11 @@ function SecretRenameRow({ environments, getSecretByKey, secretKey, secretPath } secretTags: (secretDetails?.tags || []).map((i) => i.slug) }); const isSecretInEnvReadOnly = - permission.can(ProjectPermissionSecretActions.DescribeSecret, secretPermissionSubject) && - permission.cannot(ProjectPermissionSecretActions.Edit, secretPermissionSubject); + secretsPermissionCan( + permission, + ProjectPermissionSecretActions.DescribeSecret, + secretPermissionSubject + ) && permission.cannot(ProjectPermissionSecretActions.Edit, secretPermissionSubject); if (isSecretInEnvReadOnly) { return true; } diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx index 5cbefe9c3..4dba1c2a8 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/SecretDashboardPage.tsx @@ -38,6 +38,7 @@ import { useGetProjectSecretsDetails } from "@app/hooks/api/dashboard"; import { DashboardSecretsOrderBy } from "@app/hooks/api/dashboard/types"; import { OrderByDirection } from "@app/hooks/api/generic/types"; import { ProjectType } from "@app/hooks/api/workspace/types"; +import { secretsPermissionCan } from "@app/lib/fn/permission"; import { SecretTableResourceCount } from "../OverviewPage/components/SecretTableResourceCount"; import { SecretV2MigrationSection } from "../OverviewPage/components/SecretV2MigrationSection"; @@ -103,23 +104,27 @@ const Page = () => { const workspaceId = currentWorkspace?.id || ""; const projectSlug = currentWorkspace?.slug || ""; const secretPath = (routerQueryParams.secretPath as string) || "/"; - const canReadSecret = permission.can( + + const canReadSecret = secretsPermissionCan( + permission, ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { + { environment, secretPath, secretName: "*", secretTags: ["*"] - }) + } ); - const canReadSecretValue = permission.can( + + const canReadSecretValue = secretsPermissionCan( + permission, ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { + { environment, secretPath, secretName: "*", secretTags: ["*"] - }) + } ); const canReadSecretImports = permission.can( 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 d6da791bb..3ad7159d7 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretDetailSidebar.tsx @@ -57,6 +57,7 @@ import { ActorType } from "@app/hooks/api/auditLogs/enums"; import { useGetSecretAccessList } from "@app/hooks/api/secrets/queries"; import { SecretV3RawSanitized, WsTag } from "@app/hooks/api/types"; import { ProjectType } from "@app/hooks/api/workspace/types"; +import { secretsPermissionCan } from "@app/lib/fn/permission"; import { CreateReminderForm } from "./CreateReminderForm"; import { formSchema, SecretActionType, TFormSchema } from "./SecretListView.utils"; @@ -140,26 +141,24 @@ export const SecretDetailSidebar = ({ }) ); - const cannotReadSecretValue = permission.cannot( + const cannotReadSecretValue = !secretsPermissionCan( + permission, ProjectPermissionSecretActions.ReadValue, - subject(ProjectPermissionSub.Secrets, { + { environment, secretPath, secretName: secretKey, secretTags: selectTagSlugs - }) + } ); const isReadOnly = - permission.can( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName: secretKey, - secretTags: selectTagSlugs - }) - ) && + secretsPermissionCan(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath, + secretName: secretKey, + secretTags: selectTagSlugs + }) && cannotEditSecret && cannotReadSecretValue; diff --git a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretItem.tsx b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretItem.tsx index 02f7a0aa9..0271bffef 100644 --- a/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretItem.tsx +++ b/frontend/src/pages/secret-manager/SecretDashboardPage/components/SecretListView/SecretItem.tsx @@ -47,6 +47,7 @@ import { import { ProjectPermissionSecretActions } from "@app/context/ProjectPermissionContext/types"; import { Blur } from "@app/components/v2/Blur"; +import { secretsPermissionCan } from "@app/lib/fn/permission"; import { FontAwesomeSpriteName, formSchema, @@ -131,15 +132,12 @@ export const SecretItem = memo( }); const isReadOnly = - permission.can( - ProjectPermissionSecretActions.DescribeSecret, - subject(ProjectPermissionSub.Secrets, { - environment, - secretPath, - secretName, - secretTags: selectedTagSlugs - }) - ) && + secretsPermissionCan(permission, ProjectPermissionSecretActions.DescribeSecret, { + environment, + secretPath, + secretName, + secretTags: selectedTagSlugs + }) && permission.cannot( ProjectPermissionSecretActions.Edit, subject(ProjectPermissionSub.Secrets, {