Add non try catch error handle and fix bulk patch

Infisical · Jan 1, 2023 · 3c6b1e5 · 3c6b1e5
1 parent a5e8741
commit 3c6b1e5
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 95 deletions.
diff --git a/backend/package-lock.json b/backend/package-lock.json
diff --git 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
@@ -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.UserIDJwtPayload>(
 		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
@@ -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 secretsUserCanModify: ISecret[] = await Secret.find({ workspace: workspaceId, environment: environmentName })
-
-      const secretsUserCanModifyMapBySecretId: Map<string, ISecret> = new Map<string, ISecret>();
-      secretsUserCanModify.forEach(secret => secretsUserCanModifyMapBySecretId.set(secret._id.toString(), secret))
+    const { workspaceId, environmentName } = req.params
+    const secretsModificationsRequested: ModifySecretPayload[] = req.body.secrets;
 
-      // Check if the entity has access to the secret ids it wants to modify
-      const updateOperationsToPerform: AnyBulkWriteOperation<ISecret>[] = []
-      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 [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 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 secretsUserCanModifySet: Set<string> = 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 bulkModificationInfo = await Secret.bulkWrite(updateOperationsToPerform);
+        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" })
+      }
+    })
 
-      return res.status(200).json({
-        bulkModificationInfo
-      })
-
-    } catch (e) {
-      throw BadRequestError()
+    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 → backend/src/types/secret/types.ts b/backend/src/types/secret/index.ts → backend/src/types/secret/types.ts
@@ -1,4 +1,6 @@
 import { Omit } from 'utility-types';
 import { ISecret } from '../../models';
 
-export type ModifySecretPayload = Omit<ISecret, "user" | "version" | "environment" | "workspace">;
+export type ModifySecretPayload = Omit<ISecret, "user" | "version" | "environment" | "workspace">;
+
+export type SafeUpdateSecret = Partial<Omit<ISecret, "user" | "version" | "environment" | "workspace">>;
diff --git 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"
+  ]
+}
diff --git a/package-lock.json b/package-lock.json
diff --git 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"
   }
 }