From 3c6b1e51b59083fba9306993b66ad89fc05fa93a Mon Sep 17 00:00:00 2001 From: Maidul Islam Date: Sat, 31 Dec 2022 20:43:49 -0500 Subject: [PATCH] Add non try catch error handle and fix bulk patch --- backend/package-lock.json | 28 +++++ backend/package.json | 2 + backend/src/middleware/requireAuth.ts | 12 +-- backend/src/routes/v2/secret.ts | 100 ++++++++---------- .../src/types/secret/{index.ts => types.ts} | 4 +- backend/tsconfig.json | 19 +++- package-lock.json | 30 ------ package.json | 4 - 8 files changed, 100 insertions(+), 99 deletions(-) rename backend/src/types/secret/{index.ts => types.ts} (50%) diff --git a/backend/package-lock.json b/backend/package-lock.json index 0b557b81c..95f146812 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -15,6 +15,7 @@ "@sentry/tracing": "^7.19.0", "@types/crypto-js": "^4.1.1", "@types/libsodium-wrappers": "^0.7.10", + "await-to-js": "^3.0.0", "axios": "^1.1.3", "bigint-conversion": "^2.2.2", "cookie-parser": "^1.4.6", @@ -38,6 +39,7 @@ "tweetnacl": "^1.0.3", "tweetnacl-util": "^0.15.1", "typescript": "^4.9.3", + "utility-types": "^3.10.0", "winston": "^3.8.2", "winston-loki": "^6.0.6" }, @@ -3678,6 +3680,14 @@ "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==" }, + "node_modules/await-to-js": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/await-to-js/-/await-to-js-3.0.0.tgz", + "integrity": "sha512-zJAaP9zxTcvTHRlejau3ZOY4V7SRpiByf3/dxx2uyKxxor19tpmpV2QRsTKikckwhaPmr2dVpxxMr7jOCYVp5g==", + "engines": { + "node": ">=6.0.0" + } + }, "node_modules/axios": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/axios/-/axios-1.1.3.tgz", @@ -11276,6 +11286,14 @@ "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", "integrity": "sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==" }, + "node_modules/utility-types": { + "version": "3.10.0", + "resolved": "https://registry.npmjs.org/utility-types/-/utility-types-3.10.0.tgz", + "integrity": "sha512-O11mqxmi7wMKCo6HKFt5AhO4BwY3VV68YU07tgxfz8zJTIxr4BpsezN49Ffwy9j3ZpwwJp4fkRwjRzq3uWE6Rg==", + "engines": { + "node": ">= 4" + } + }, "node_modules/utils-merge": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", @@ -14656,6 +14674,11 @@ "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==" }, + "await-to-js": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/await-to-js/-/await-to-js-3.0.0.tgz", + "integrity": "sha512-zJAaP9zxTcvTHRlejau3ZOY4V7SRpiByf3/dxx2uyKxxor19tpmpV2QRsTKikckwhaPmr2dVpxxMr7jOCYVp5g==" + }, "axios": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/axios/-/axios-1.1.3.tgz", @@ -20178,6 +20201,11 @@ "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", "integrity": "sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==" }, + "utility-types": { + "version": "3.10.0", + "resolved": "https://registry.npmjs.org/utility-types/-/utility-types-3.10.0.tgz", + "integrity": "sha512-O11mqxmi7wMKCo6HKFt5AhO4BwY3VV68YU07tgxfz8zJTIxr4BpsezN49Ffwy9j3ZpwwJp4fkRwjRzq3uWE6Rg==" + }, "utils-merge": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", diff --git a/backend/package.json b/backend/package.json index d1a03a74c..1eb5e3b45 100644 --- a/backend/package.json +++ b/backend/package.json @@ -6,6 +6,7 @@ "@sentry/tracing": "^7.19.0", "@types/crypto-js": "^4.1.1", "@types/libsodium-wrappers": "^0.7.10", + "await-to-js": "^3.0.0", "axios": "^1.1.3", "bigint-conversion": "^2.2.2", "cookie-parser": "^1.4.6", @@ -29,6 +30,7 @@ "tweetnacl": "^1.0.3", "tweetnacl-util": "^0.15.1", "typescript": "^4.9.3", + "utility-types": "^3.10.0", "winston": "^3.8.2", "winston-loki": "^6.0.6" }, diff --git a/backend/src/middleware/requireAuth.ts b/backend/src/middleware/requireAuth.ts index d917d362a..172f7b68e 100644 --- a/backend/src/middleware/requireAuth.ts +++ b/backend/src/middleware/requireAuth.ts @@ -20,10 +20,10 @@ declare module 'jsonwebtoken' { */ const requireAuth = async (req: Request, res: Response, next: NextFunction) => { // JWT authentication middleware - const [ AUTH_TOKEN_TYPE, AUTH_TOKEN_VALUE ] = <[string, string]>req.headers['authorization']?.split(' ', 2) ?? [null, null] - if(AUTH_TOKEN_TYPE === null) return next(BadRequestError({message: `Missing Authorization Header in the request header.`})) - if(AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(BadRequestError({message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.`})) - if(AUTH_TOKEN_VALUE === null) return next(BadRequestError({message: 'Missing Authorization Body in the request header'})) + const [AUTH_TOKEN_TYPE, AUTH_TOKEN_VALUE] = <[string, string]>req.headers['authorization']?.split(' ', 2) ?? [null, null] + if (AUTH_TOKEN_TYPE === null) return next(BadRequestError({ message: `Missing Authorization Header in the request header.` })) + if (AUTH_TOKEN_TYPE.toLowerCase() !== 'bearer') return next(BadRequestError({ message: `The provided authentication type '${AUTH_TOKEN_TYPE}' is not supported.` })) + if (AUTH_TOKEN_VALUE === null) return next(BadRequestError({ message: 'Missing Authorization Body in the request header' })) const decodedToken = ( jwt.verify(AUTH_TOKEN_VALUE, JWT_AUTH_SECRET) @@ -33,9 +33,9 @@ const requireAuth = async (req: Request, res: Response, next: NextFunction) => { _id: decodedToken.userId }).select('+publicKey'); - if (!user) return next(AccountNotFoundError({message: 'Failed to locate User account'})) + if (!user) return next(AccountNotFoundError({ message: 'Failed to locate User account' })) if (!user?.publicKey) - return next(UnauthorizedRequestError({message: 'Unable to authenticate due to partially set up account'})) + return next(UnauthorizedRequestError({ message: 'Unable to authenticate due to partially set up account' })) req.user = user; return next(); diff --git a/backend/src/routes/v2/secret.ts b/backend/src/routes/v2/secret.ts index 42badbff8..078a04106 100644 --- a/backend/src/routes/v2/secret.ts +++ b/backend/src/routes/v2/secret.ts @@ -4,11 +4,12 @@ import { ISecret, Secret } from '../../models'; import { decryptSymmetric } from '../../utils/crypto'; import { getLogger } from '../../utils/logger'; import { body, param, query, check } from 'express-validator'; -import { BadRequestError, UnauthorizedRequestError } from '../../utils/errors'; +import { BadRequestError, InternalServerError, UnauthorizedRequestError } from '../../utils/errors'; import { ADMIN, MEMBER, COMPLETED, GRANTED } from '../../variables'; -import { ModifySecretPayload } from '../../types/secret'; +import { ModifySecretPayload, SafeUpdateSecret } from '../../types/secret/types'; import { AnyBulkWriteOperation } from 'mongodb'; import to from 'await-to-js'; +import { Types } from 'mongoose'; const router = express.Router(); @@ -141,66 +142,59 @@ router.delete( /** * Apply modifications to many existing secrets in a given workspace and environment - * Note: although we do not check access for environments, we will in the future */ router.patch( '/bulk-modify/:workspaceId/:environmentName', requireAuth, body('secrets').exists().isArray().custom((value) => value.every((item: ISecret) => typeof item === 'object')), - param('workspaceId').exists().trim(), + param('workspaceId').exists().isMongoId().trim(), param('environmentName').exists().trim(), - // requireWorkspaceAuth({ - // acceptedRoles: [ADMIN, MEMBER], - // acceptedStatuses: [COMPLETED, GRANTED] - // }), + requireWorkspaceAuth({ + acceptedRoles: [ADMIN, MEMBER], + acceptedStatuses: [COMPLETED, GRANTED] + }), validateRequest, async (req: Request, res: Response) => { - try { - const { workspaceId, environmentName } = req.params - const secretsModificationsRequested: ModifySecretPayload[] = req.body.secrets; + const { workspaceId, environmentName } = req.params + const secretsModificationsRequested: ModifySecretPayload[] = req.body.secrets; - const secretsUserCanModify: ISecret[] = await Secret.find({ workspace: workspaceId, environment: environmentName }) - - const secretsUserCanModifyMapBySecretId: Map = new Map(); - secretsUserCanModify.forEach(secret => secretsUserCanModifyMapBySecretId.set(secret._id.toString(), secret)) - - // Check if the entity has access to the secret ids it wants to modify - const updateOperationsToPerform: AnyBulkWriteOperation[] = [] - secretsModificationsRequested.forEach(userModifiedSecret => { - const canModifyRequestedSecret = secretsUserCanModifyMapBySecretId.has(userModifiedSecret._id.toString()) - if (canModifyRequestedSecret) { - const oldSecretInDB = secretsUserCanModifyMapBySecretId.get(userModifiedSecret._id.toString()) - - if (oldSecretInDB !== undefined) { - oldSecretInDB.secretKeyCiphertext = userModifiedSecret.secretKeyCiphertext - oldSecretInDB.secretKeyIV = userModifiedSecret.secretKeyIV - oldSecretInDB.secretKeyTag = userModifiedSecret.secretKeyTag - oldSecretInDB.secretKeyHash = userModifiedSecret.secretKeyHash - oldSecretInDB.secretValueCiphertext = userModifiedSecret.secretValueCiphertext - oldSecretInDB.secretValueIV = userModifiedSecret.secretValueIV - oldSecretInDB.secretValueTag = userModifiedSecret.secretValueTag - oldSecretInDB.secretValueHash = userModifiedSecret.secretValueHash - oldSecretInDB.secretCommentCiphertext = userModifiedSecret.secretCommentCiphertext - oldSecretInDB.secretCommentIV = userModifiedSecret.secretCommentIV - oldSecretInDB.secretCommentTag = userModifiedSecret.secretCommentTag - oldSecretInDB.secretCommentHash = userModifiedSecret.secretCommentHash - - const updateOperation = { updateOne: { filter: { _id: oldSecretInDB._id, workspace: oldSecretInDB.workspace }, update: { $inc: { version: 1 }, $set: oldSecretInDB } } } - updateOperationsToPerform.push(updateOperation) - } - } else { - throw UnauthorizedRequestError({ message: "You do not have permission to modify one or more of the requested secrets" }) - } - }) - - const bulkModificationInfo = await Secret.bulkWrite(updateOperationsToPerform); - - return res.status(200).json({ - bulkModificationInfo - }) - - } catch (e) { - throw BadRequestError() + const [secretIdsUserCanModifyError, secretIdsUserCanModify] = await to(Secret.find({ workspace: workspaceId, environment: environmentName }, { _id: 1 }).then()) + if (secretIdsUserCanModifyError) { + throw InternalServerError({ message: "Unable to fetch secrets you own" }) } + + const secretsUserCanModifySet: Set = new Set(secretIdsUserCanModify.map(objectId => objectId._id.toString())); + const updateOperationsToPerform: any = [] + + secretsModificationsRequested.forEach(userModifiedSecret => { + if (secretsUserCanModifySet.has(userModifiedSecret._id.toString())) { + const safeUpdateFields: SafeUpdateSecret = { + secretKeyCiphertext: userModifiedSecret.secretKeyCiphertext, + secretKeyIV: userModifiedSecret.secretKeyIV, + secretKeyTag: userModifiedSecret.secretKeyTag, + secretKeyHash: userModifiedSecret.secretKeyHash, + secretValueCiphertext: userModifiedSecret.secretValueCiphertext, + secretValueIV: userModifiedSecret.secretValueIV, + secretValueTag: userModifiedSecret.secretValueTag, + secretValueHash: userModifiedSecret.secretValueHash, + secretCommentCiphertext: userModifiedSecret.secretCommentCiphertext, + secretCommentIV: userModifiedSecret.secretCommentIV, + secretCommentTag: userModifiedSecret.secretCommentTag, + secretCommentHash: userModifiedSecret.secretCommentHash, + } + + const updateOperation = { updateOne: { filter: { _id: userModifiedSecret._id, workspace: workspaceId }, update: { $inc: { version: 1 }, $set: safeUpdateFields } } } + updateOperationsToPerform.push(updateOperation) + } else { + throw UnauthorizedRequestError({ message: "You do not have permission to modify one or more of the requested secrets" }) + } + }) + + const [bulkModificationInfoError, bulkModificationInfo] = await to(Secret.bulkWrite(updateOperationsToPerform).then()) + if (bulkModificationInfoError) { + throw InternalServerError({ message: "Unable to apply modifications, please try again" }) + } + + return res.status(200).send() } ); diff --git a/backend/src/types/secret/index.ts b/backend/src/types/secret/types.ts similarity index 50% rename from backend/src/types/secret/index.ts rename to backend/src/types/secret/types.ts index 11696484e..c4a8d0cc9 100644 --- a/backend/src/types/secret/index.ts +++ b/backend/src/types/secret/types.ts @@ -1,4 +1,6 @@ import { Omit } from 'utility-types'; import { ISecret } from '../../models'; -export type ModifySecretPayload = Omit; \ No newline at end of file +export type ModifySecretPayload = Omit; + +export type SafeUpdateSecret = Partial>; diff --git a/backend/tsconfig.json b/backend/tsconfig.json index 0bfe3c372..98ea0808a 100644 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -1,7 +1,9 @@ { "compilerOptions": { "target": "es2016", - "lib": ["es6"], + "lib": [ + "es6" + ], "module": "commonjs", "rootDir": "src", "resolveJsonModule": true, @@ -13,8 +15,15 @@ "strict": true, "noImplicitAny": true, "skipLibCheck": true, - "typeRoots": ["./src/types", "./node_modules/@types"] + "typeRoots": [ + "./src/types", + "./node_modules/@types" + ] }, - "include": ["src/**/*"], - "exclude": ["node_modules"] -} + "include": [ + "src/**/*" + ], + "exclude": [ + "node_modules" + ] +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 0ba1a0659..cf02a1760 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,10 +6,6 @@ "": { "name": "infisical", "license": "ISC", - "dependencies": { - "await-to-js": "^3.0.0", - "utility-types": "^3.10.0" - }, "devDependencies": { "eslint": "^8.29.0", "husky": "^8.0.2" @@ -173,14 +169,6 @@ "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", "dev": true }, - "node_modules/await-to-js": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/await-to-js/-/await-to-js-3.0.0.tgz", - "integrity": "sha512-zJAaP9zxTcvTHRlejau3ZOY4V7SRpiByf3/dxx2uyKxxor19tpmpV2QRsTKikckwhaPmr2dVpxxMr7jOCYVp5g==", - "engines": { - "node": ">=6.0.0" - } - }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -1104,14 +1092,6 @@ "punycode": "^2.1.0" } }, - "node_modules/utility-types": { - "version": "3.10.0", - "resolved": "https://registry.npmjs.org/utility-types/-/utility-types-3.10.0.tgz", - "integrity": "sha512-O11mqxmi7wMKCo6HKFt5AhO4BwY3VV68YU07tgxfz8zJTIxr4BpsezN49Ffwy9j3ZpwwJp4fkRwjRzq3uWE6Rg==", - "engines": { - "node": ">= 4" - } - }, "node_modules/which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", @@ -1268,11 +1248,6 @@ "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", "dev": true }, - "await-to-js": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/await-to-js/-/await-to-js-3.0.0.tgz", - "integrity": "sha512-zJAaP9zxTcvTHRlejau3ZOY4V7SRpiByf3/dxx2uyKxxor19tpmpV2QRsTKikckwhaPmr2dVpxxMr7jOCYVp5g==" - }, "balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -1935,11 +1910,6 @@ "punycode": "^2.1.0" } }, - "utility-types": { - "version": "3.10.0", - "resolved": "https://registry.npmjs.org/utility-types/-/utility-types-3.10.0.tgz", - "integrity": "sha512-O11mqxmi7wMKCo6HKFt5AhO4BwY3VV68YU07tgxfz8zJTIxr4BpsezN49Ffwy9j3ZpwwJp4fkRwjRzq3uWE6Rg==" - }, "which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", diff --git a/package.json b/package.json index 5b50c5859..4d042a13d 100644 --- a/package.json +++ b/package.json @@ -21,9 +21,5 @@ "devDependencies": { "eslint": "^8.29.0", "husky": "^8.0.2" - }, - "dependencies": { - "await-to-js": "^3.0.0", - "utility-types": "^3.10.0" } }