From acfb4693ee969133493ac5e46d7e0e6ee9df8b16 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 7 Oct 2024 23:57:39 +0530 Subject: [PATCH] feat: backend fixed bug in permission change --- backend/src/lib/knex/dynamic.ts | 31 +++++++------------ backend/src/lib/knex/index.ts | 2 +- backend/src/server/plugins/error-handler.ts | 2 +- .../secret-import/secret-import-fns.ts | 7 ++--- .../secret-import/secret-import-service.ts | 2 +- .../secret-v2-bridge-service.ts | 31 +++++++++++++++++-- 6 files changed, 46 insertions(+), 29 deletions(-) diff --git a/backend/src/lib/knex/dynamic.ts b/backend/src/lib/knex/dynamic.ts index 90530b979..62f35e79a 100644 --- a/backend/src/lib/knex/dynamic.ts +++ b/backend/src/lib/knex/dynamic.ts @@ -28,8 +28,8 @@ type TKnexGroupOperator = { export type TKnexDynamicOperator = TKnexGroupOperator | TKnexNonGroupOperator; export const buildDynamicKnexQuery = ( - dynamicQuery: TKnexDynamicOperator, - rootQueryBuild: Knex.QueryBuilder + rootQueryBuild: Knex.QueryBuilder, + dynamicQuery: TKnexDynamicOperator ) => { const stack = [{ filterAst: dynamicQuery, queryBuilder: rootQueryBuild }]; @@ -53,34 +53,25 @@ export const buildDynamicKnexQuery = ( break; } case "and": { - void queryBuilder.andWhere((subQueryBuilder) => { - filterAst.value.forEach((el) => { - stack.push({ - queryBuilder: subQueryBuilder, - filterAst: el - }); + filterAst.value.forEach((el) => { + void queryBuilder.andWhere((subQueryBuilder) => { + buildDynamicKnexQuery(subQueryBuilder, el); }); }); break; } case "or": { - void queryBuilder.orWhere((subQueryBuilder) => { - filterAst.value.forEach((el) => { - stack.push({ - queryBuilder: subQueryBuilder, - filterAst: el - }); + filterAst.value.forEach((el) => { + void queryBuilder.orWhere((subQueryBuilder) => { + buildDynamicKnexQuery(subQueryBuilder, el); }); }); break; } case "not": { - void queryBuilder.whereNot((subQueryBuilder) => { - filterAst.value.forEach((el) => { - stack.push({ - queryBuilder: subQueryBuilder, - filterAst: el - }); + filterAst.value.forEach((el) => { + void queryBuilder.whereNot((subQueryBuilder) => { + buildDynamicKnexQuery(subQueryBuilder, el); }); }); break; diff --git a/backend/src/lib/knex/index.ts b/backend/src/lib/knex/index.ts index 226b0b802..f55d8e6e6 100644 --- a/backend/src/lib/knex/index.ts +++ b/backend/src/lib/knex/index.ts @@ -42,7 +42,7 @@ export const buildFindFilter = }); } if ($complex) { - buildDynamicKnexQuery($complex, bd); + return buildDynamicKnexQuery(bd, $complex); } return bd; }; diff --git a/backend/src/server/plugins/error-handler.ts b/backend/src/server/plugins/error-handler.ts index 76bfa9023..4113c0f2d 100644 --- a/backend/src/server/plugins/error-handler.ts +++ b/backend/src/server/plugins/error-handler.ts @@ -61,7 +61,7 @@ export const fastifyErrHandler = fastifyPlugin(async (server: FastifyZodProvider void res.status(HttpStatusCodes.Forbidden).send({ statusCode: HttpStatusCodes.Forbidden, error: "PermissionDenied", - message: `You are not allowed to ${error.action} on ${error.subjectType}` + message: `You are not allowed to ${error.action} on ${error.subjectType} - ${JSON.stringify(error.subject)}` }); } else if (error instanceof ForbiddenRequestError) { void res.status(HttpStatusCodes.Forbidden).send({ diff --git a/backend/src/services/secret-import/secret-import-fns.ts b/backend/src/services/secret-import/secret-import-fns.ts index 8f83d2843..d75a25514 100644 --- a/backend/src/services/secret-import/secret-import-fns.ts +++ b/backend/src/services/secret-import/secret-import-fns.ts @@ -180,7 +180,7 @@ export const fnSecretsV2FromImports = async ({ ({ importPath, importEnv }) => !cyclicDetector.has(getImportUniqKey(importEnv.slug, importPath)) ); - if (sanitizedImports.length) continue; + if (!sanitizedImports.length) continue; const importedFolders = await folderDAL.findByManySecretPath( sanitizedImports.map(({ importEnv, importPath }) => ({ @@ -212,7 +212,7 @@ export const fnSecretsV2FromImports = async ({ const deeperImports = await secretImportDAL.findByFolderIds(importedFolderIds); const deeperImportsGroupByFolderId = groupBy(deeperImports, (i) => i.folderId); - const isFirstIteration = processedImports.length; + const isFirstIteration = !processedImports.length; sanitizedImports.forEach(({ importPath, importEnv, id, folderId }, i) => { const sourceImportFolder = importedFolderGroupBySourceImport[`${importEnv.id}-${importPath}`]?.[0]; const secretsWithDuplicate = (importedSecretsGroupByFolderId?.[importedFolders?.[i]?.id as string] || []) @@ -250,7 +250,7 @@ export const fnSecretsV2FromImports = async ({ folderId: importedFolders?.[i]?.id, id, importFolderId: folderId, - secrets: unique(secretsWithDuplicate, (el) => el.secretKey) + secrets: secretsWithDuplicate }); } else { parentImportedSecrets.push(...secretsWithDuplicate); @@ -258,7 +258,6 @@ export const fnSecretsV2FromImports = async ({ }); } /* eslint-enable */ - if (expandSecretReferences) { await Promise.allSettled( processedImports.map((processedImport) => { diff --git a/backend/src/services/secret-import/secret-import-service.ts b/backend/src/services/secret-import/secret-import-service.ts index f80f5b236..ecbb7f16a 100644 --- a/backend/src/services/secret-import/secret-import-service.ts +++ b/backend/src/services/secret-import/secret-import-service.ts @@ -493,7 +493,7 @@ export const secretImportServiceFactory = ({ ForbiddenError.from(permission).throwUnlessCan( ProjectPermissionActions.Read, - subject(ProjectPermissionSub.Secrets, { + subject(ProjectPermissionSub.SecretImports, { environment: folder.environment.envSlug, secretPath: folderWithPath.path }) 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 31eff6cfd..c871794a3 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 @@ -114,12 +114,14 @@ export const secretV2BridgeServiceFactory = ({ envId: referencesEnvironmentGroupBySlug[el.environment][0].id })) ); - const referencesFolderGroupByPath = groupBy(referredFolders.filter(Boolean), (i) => i?.path as string); + const referencesFolderGroupByPath = groupBy(referredFolders.filter(Boolean), (i) => `${i?.envId}-${i?.path}`); const referredSecrets = await secretDAL.find({ $complex: { operator: "or", value: references.map((el) => { - const folderId = referencesFolderGroupByPath[el.secretPath][0]?.id; + const folderId = + referencesFolderGroupByPath[`${referencesEnvironmentGroupBySlug[el.environment][0].id}-${el.secretPath}`][0] + ?.id; if (!folderId) throw new BadRequestError({ message: `Reference path ${el.secretPath} doesn't exist` }); return { @@ -140,6 +142,7 @@ export const secretV2BridgeServiceFactory = ({ }) } }); + if (referredSecrets.length !== references.length) throw new BadRequestError({ message: "Reference secret not found" }); @@ -356,6 +359,17 @@ export const secretV2BridgeServiceFactory = ({ const tags = inputSecret.tagIds ? await secretTagDAL.find({ projectId, $in: { id: inputSecret.tagIds } }) : []; if ((inputSecret.tagIds || []).length !== tags.length) throw new NotFoundError({ message: "Tag not found" }); + // now check with new ids + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Edit, + subject(ProjectPermissionSub.Secrets, { + environment, + secretPath, + secretName: inputSecret.secretName, + secretTags: tags?.map((el) => el.slug) + }) + ); + if (inputSecret.newSecretName) { const doesNewNameSecretExist = await secretDAL.findOne({ key: inputSecret.newSecretName, @@ -1229,6 +1243,19 @@ export const secretV2BridgeServiceFactory = ({ if (tags.length !== sanitizedTagIds.length) throw new NotFoundError({ message: "Tag not found" }); const tagsGroupByID = groupBy(tags, (i) => i.id); + // check again to avoid non authorized tags are removed + inputSecrets.forEach((el) => { + ForbiddenError.from(permission).throwUnlessCan( + ProjectPermissionActions.Edit, + subject(ProjectPermissionSub.Secrets, { + environment, + secretPath, + secretName: el.secretKey, + secretTags: (el.tagIds || []).map((i) => tagsGroupByID[i][0].slug) + }) + ); + }); + // now find any secret that needs to update its name // same process as above const secretsWithNewName = inputSecrets.filter(({ newSecretName }) => Boolean(newSecretName));