diff --git a/backend/src/controllers/v2/secretsController.ts b/backend/src/controllers/v2/secretsController.ts index ff9574711..f92fc4cee 100644 --- a/backend/src/controllers/v2/secretsController.ts +++ b/backend/src/controllers/v2/secretsController.ts @@ -2,8 +2,8 @@ import to from 'await-to-js'; import { Types } from 'mongoose'; import { Request, Response } from 'express'; import { ISecret, Secret } from '../../models'; -import { - SECRET_PERSONAL, +import { + SECRET_PERSONAL, SECRET_SHARED, ACTION_ADD_SECRETS, ACTION_READ_SECRETS, @@ -23,9 +23,9 @@ import { BadRequestError } from '../../utils/errors'; * @param res */ export const createSecrets = async (req: Request, res: Response) => { - const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; + const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; const { workspaceId, environment } = req.body; - + let toAdd; if (Array.isArray(req.body.secrets)) { // case: create multiple secrets @@ -34,7 +34,7 @@ export const createSecrets = async (req: Request, res: Response) => { // case: create 1 secret toAdd = [req.body.secrets]; } - + const newSecrets = await Secret.insertMany( toAdd.map(({ type, @@ -66,7 +66,7 @@ export const createSecrets = async (req: Request, res: Response) => { secretValueTag })) ); - + // (EE) add secret versions for new secrets EESecretService.addSecretVersions({ secretVersions: newSecrets.map(({ @@ -160,7 +160,7 @@ export const createSecrets = async (req: Request, res: Response) => { */ export const getSecrets = async (req: Request, res: Response) => { const { workspaceId, environment } = req.query; - + let userId: Types.ObjectId | undefined = undefined // used for getting personal secrets for user if (req.user) { userId = req.user._id; @@ -169,13 +169,13 @@ export const getSecrets = async (req: Request, res: Response) => { if (req.serviceTokenData) { userId = req.serviceTokenData.user._id } - + const [err, secrets] = await to(Secret.find( { workspace: workspaceId, environment, $or: [ - { user: userId }, + { user: userId }, { user: { $exists: false } } ], type: { $in: [SECRET_SHARED, SECRET_PERSONAL] } @@ -183,9 +183,9 @@ export const getSecrets = async (req: Request, res: Response) => { ).then()) if (err) throw ValidationError({ message: 'Failed to get secrets', stack: err.stack }); - + const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; - + const readAction = await EELogService.createActionSecret({ name: ACTION_READ_SECRETS, userId: req.user._id.toString(), @@ -214,7 +214,7 @@ export const getSecrets = async (req: Request, res: Response) => { } }); } - + return res.status(200).send({ secrets }); @@ -226,8 +226,8 @@ export const getSecrets = async (req: Request, res: Response) => { * @param res */ export const updateSecrets = async (req: Request, res: Response) => { - const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; - + const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; + // TODO: move type interface PatchSecret { id: string; @@ -242,7 +242,7 @@ export const updateSecrets = async (req: Request, res: Response) => { secretCommentTag: string; } - const ops = req.body.secrets.map((secret: PatchSecret) => { + const updateOperationsToPerform = req.body.secrets.map((secret: PatchSecret) => { const { secretKeyCiphertext, secretKeyIV, @@ -254,6 +254,7 @@ export const updateSecrets = async (req: Request, res: Response) => { secretCommentIV, secretCommentTag } = secret; + return ({ updateOne: { filter: { _id: new Types.ObjectId(secret.id) }, @@ -268,8 +269,8 @@ export const updateSecrets = async (req: Request, res: Response) => { secretValueIV, secretValueTag, ...(( - secretCommentCiphertext && - secretCommentIV && + secretCommentCiphertext && + secretCommentIV && secretCommentTag ) ? { secretCommentCiphertext, @@ -280,15 +281,17 @@ export const updateSecrets = async (req: Request, res: Response) => { } }); }); - await Secret.bulkWrite(ops); - - const newSecretsObj: { [key: string]: PatchSecret } = {}; + + await Secret.bulkWrite(updateOperationsToPerform); + + const secretModificationsBySecretId: { [key: string]: PatchSecret } = {}; req.body.secrets.forEach((secret: PatchSecret) => { - newSecretsObj[secret.id] = secret; + secretModificationsBySecretId[secret.id] = secret; }); - await EESecretService.addSecretVersions({ - secretVersions: req.secrets.map((secret: ISecret) => { + const ListOfSecretsBeforeModifications = req.secrets + const secretVersions = { + secretVersions: ListOfSecretsBeforeModifications.map((secret: ISecret) => { const { secretKeyCiphertext, secretKeyIV, @@ -298,37 +301,29 @@ export const updateSecrets = async (req: Request, res: Response) => { secretValueTag, secretCommentCiphertext, secretCommentIV, - secretCommentTag - } = newSecretsObj[secret._id.toString()] + secretCommentTag, + } = secretModificationsBySecretId[secret._id.toString()] + return ({ secret: secret._id, version: secret.version + 1, workspace: secret.workspace, type: secret.type, environment: secret.environment, - isDeleted: false, - secretKeyCiphertext, - secretKeyIV, - secretKeyTag, - secretValueCiphertext, - secretValueIV, - secretValueTag, - ...(( - secretCommentCiphertext && - secretCommentIV && - secretCommentTag - ) ? { - secretCommentCiphertext, - secretCommentIV, - secretCommentTag - } : { - secretCommentCiphertext: '', - secretCommentIV: '', - secretCommentTag: '' - }) + secretKeyCiphertext: secretKeyCiphertext ? secretKeyCiphertext : secret.secretKeyCiphertext, + secretKeyIV: secretKeyIV ? secretKeyIV : secret.secretKeyIV, + secretKeyTag: secretKeyTag ? secretKeyTag : secret.secretKeyTag, + secretValueCiphertext: secretValueCiphertext ? secretValueCiphertext : secret.secretValueCiphertext, + secretValueIV: secretValueIV ? secretValueIV : secret.secretValueIV, + secretValueTag: secretValueTag ? secretValueTag : secret.secretValueTag, + secretCommentCiphertext: secretCommentCiphertext ? secretCommentCiphertext : secret.secretCommentCiphertext, + secretCommentIV: secretCommentIV ? secretCommentIV : secret.secretCommentIV, + secretCommentTag: secretCommentTag ? secretCommentTag : secret.secretCommentTag, }); }) - }); + } + + await EESecretService.addSecretVersions(secretVersions); // group secrets into workspaces so updated secrets can @@ -355,7 +350,7 @@ export const updateSecrets = async (req: Request, res: Response) => { userId: req.user._id.toString(), workspaceId: key, secretIds: workspaceSecretObj[key].map((secret: ISecret) => secret._id) - }); + }); // (EE) create (audit) log updateAction && await EELogService.createLog({ @@ -367,9 +362,9 @@ export const updateSecrets = async (req: Request, res: Response) => { }); // (EE) take a secret snapshot - await EESecretService.takeSecretSnapshot({ - workspaceId: key - }) + await EESecretService.takeSecretSnapshot({ + workspaceId: key + }) if (postHogClient) { postHogClient.capture({ @@ -385,7 +380,7 @@ export const updateSecrets = async (req: Request, res: Response) => { }); } }); - + return res.status(200).send({ secrets: await Secret.find({ _id: { @@ -401,15 +396,15 @@ export const updateSecrets = async (req: Request, res: Response) => { * @param res */ export const deleteSecrets = async (req: Request, res: Response) => { - const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; + const channel = req.headers?.['user-agent']?.toLowerCase().includes('mozilla') ? 'web' : 'cli'; const toDelete = req.secrets.map((s: any) => s._id); - + await Secret.deleteMany({ _id: { $in: toDelete } }); - + await EESecretService.markDeletedSecretVersions({ secretIds: toDelete }); @@ -437,7 +432,7 @@ export const deleteSecrets = async (req: Request, res: Response) => { userId: req.user._id.toString(), workspaceId: key, secretIds: workspaceSecretObj[key].map((secret: ISecret) => secret._id) - }); + }); // (EE) create (audit) log deleteAction && await EELogService.createLog({ @@ -449,9 +444,9 @@ export const deleteSecrets = async (req: Request, res: Response) => { }); // (EE) take a secret snapshot - await EESecretService.takeSecretSnapshot({ - workspaceId: key - }) + await EESecretService.takeSecretSnapshot({ + workspaceId: key + }) if (postHogClient) { postHogClient.capture({ @@ -467,7 +462,7 @@ export const deleteSecrets = async (req: Request, res: Response) => { }); } }); - + return res.status(200).send({ secrets: req.secrets }); diff --git a/backend/src/ee/helpers/secret.ts b/backend/src/ee/helpers/secret.ts index 529c9a980..7edee915f 100644 --- a/backend/src/ee/helpers/secret.ts +++ b/backend/src/ee/helpers/secret.ts @@ -1,11 +1,11 @@ import { Types } from 'mongoose'; import * as Sentry from '@sentry/node'; import { - Secret, + Secret, ISecret } from '../../models'; import { - SecretSnapshot, + SecretSnapshot, SecretVersion, ISecretVersion } from '../models'; @@ -18,24 +18,24 @@ import { * @param {String} obj.workspaceId * @returns {SecretSnapshot} secretSnapshot - new secret snapshot */ - const takeSecretSnapshotHelper = async ({ +const takeSecretSnapshotHelper = async ({ workspaceId }: { workspaceId: string; }) => { - + let secretSnapshot; try { const secretIds = (await Secret.find({ workspace: workspaceId }, '_id')).map((s) => s._id); - + const latestSecretVersions = (await SecretVersion.aggregate([ { - $match: { - secret: { - $in: secretIds - } + $match: { + secret: { + $in: secretIds + } } }, { @@ -48,14 +48,14 @@ import { { $sort: { version: -1 } } - ]) + ]) .exec()) .map((s) => s.versionId); - + const latestSecretSnapshot = await SecretSnapshot.findOne({ workspace: workspaceId }).sort({ version: -1 }); - + secretSnapshot = await new SecretSnapshot({ workspace: workspaceId, version: latestSecretSnapshot ? latestSecretSnapshot.version + 1 : 1, @@ -66,7 +66,7 @@ import { Sentry.captureException(err); throw new Error('Failed to take a secret snapshot'); } - + return secretSnapshot; } @@ -87,9 +87,9 @@ const addSecretVersionsHelper = async ({ } catch (err) { Sentry.setUser(null); Sentry.captureException(err); - throw new Error('Failed to add secret versions'); + throw new Error(`Failed to add secret versions [err=${err}]`); } - + return newSecretVersions; } @@ -120,39 +120,39 @@ const markDeletedSecretVersionsHelper = async ({ const initSecretVersioningHelper = async () => { try { - await Secret.updateMany( + await Secret.updateMany( { version: { $exists: false } }, { $set: { version: 1 } } ); - - const unversionedSecrets: ISecret[] = await Secret.aggregate([ - { - $lookup: { - from: 'secretversions', - localField: '_id', - foreignField: 'secret', - as: 'versions', - }, - }, - { - $match: { - versions: { $size: 0 }, - }, - }, - ]); - - if (unversionedSecrets.length > 0) { - await addSecretVersionsHelper({ - secretVersions: unversionedSecrets.map((s, idx) => ({ - ...s, - secret: s._id, - version: s.version ? s.version : 1, - isDeleted: false, - workspace: s.workspace, - environment: s.environment - })) - }); - } + + const unversionedSecrets: ISecret[] = await Secret.aggregate([ + { + $lookup: { + from: 'secretversions', + localField: '_id', + foreignField: 'secret', + as: 'versions', + }, + }, + { + $match: { + versions: { $size: 0 }, + }, + }, + ]); + + if (unversionedSecrets.length > 0) { + await addSecretVersionsHelper({ + secretVersions: unversionedSecrets.map((s, idx) => ({ + ...s, + secret: s._id, + version: s.version ? s.version : 1, + isDeleted: false, + workspace: s.workspace, + environment: s.environment + })) + }); + } } catch (err) { Sentry.setUser(null); @@ -162,7 +162,7 @@ const initSecretVersioningHelper = async () => { } export { - takeSecretSnapshotHelper, + takeSecretSnapshotHelper, addSecretVersionsHelper, markDeletedSecretVersionsHelper, initSecretVersioningHelper diff --git a/backend/src/ee/models/secretVersion.ts b/backend/src/ee/models/secretVersion.ts index 616d44fbd..391c0faec 100644 --- a/backend/src/ee/models/secretVersion.ts +++ b/backend/src/ee/models/secretVersion.ts @@ -10,14 +10,14 @@ import { export interface ISecretVersion { _id: Types.ObjectId; - secret: Types.ObjectId; - version: number; + secret: Types.ObjectId; + version: number; workspace: Types.ObjectId; // new type: string; // new user: Types.ObjectId; // new environment: string; // new - isDeleted: boolean; - secretKeyCiphertext: string; + isDeleted: boolean; + secretKeyCiphertext: string; secretKeyIV: string; secretKeyTag: string; secretKeyHash: string; @@ -28,17 +28,17 @@ export interface ISecretVersion { } const secretVersionSchema = new Schema( - { - secret: { // could be deleted - type: Schema.Types.ObjectId, - ref: 'Secret', - required: true - }, - version: { - type: Number, - default: 1, - required: true - }, + { + secret: { // could be deleted + type: Schema.Types.ObjectId, + ref: 'Secret', + required: true + }, + version: { + type: Number, + default: 1, + required: true + }, workspace: { type: Schema.Types.ObjectId, ref: 'Workspace', @@ -59,12 +59,12 @@ const secretVersionSchema = new Schema( enum: [ENV_DEV, ENV_TESTING, ENV_STAGING, ENV_PROD], required: true }, - isDeleted: { // consider removing field - type: Boolean, - default: false, - required: true - }, - secretKeyCiphertext: { + isDeleted: { // consider removing field + type: Boolean, + default: false, + required: true + }, + secretKeyCiphertext: { type: String, required: true }, @@ -94,10 +94,10 @@ const secretVersionSchema = new Schema( secretValueHash: { type: String } - }, - { - timestamps: true - } + }, + { + timestamps: true + } ); const SecretVersion = model('SecretVersion', secretVersionSchema); diff --git a/backend/src/routes/v2/secrets.ts b/backend/src/routes/v2/secrets.ts index 29eac042f..085d487cd 100644 --- a/backend/src/routes/v2/secrets.ts +++ b/backend/src/routes/v2/secrets.ts @@ -8,8 +8,8 @@ import { } from '../../middleware'; import { query, check, body } from 'express-validator'; import { secretsController } from '../../controllers/v2'; -import { - ADMIN, +import { + ADMIN, MEMBER, SECRET_PERSONAL, SECRET_SHARED @@ -27,7 +27,7 @@ router.post( if (value.length === 0) throw new Error('secrets cannot be an empty array') for (const secret of value) { if ( - !secret.type || + !secret.type || !(secret.type === SECRET_PERSONAL || secret.type === SECRET_SHARED) || !secret.secretKeyCiphertext || !secret.secretKeyIV || @@ -42,7 +42,7 @@ router.post( } else if (typeof value === 'object') { // case: update 1 secret if ( - !value.type || + !value.type || !(value.type === SECRET_PERSONAL || value.type === SECRET_SHARED) || !value.secretKeyCiphertext || !value.secretKeyIV || @@ -52,13 +52,13 @@ router.post( !value.secretValueTag ) { throw new Error('secrets object is missing required secret properties'); - } + } } else { throw new Error('secrets must be an object or an array of objects') } - + return true; - }), + }), validateRequest, requireAuth({ acceptedAuthModes: ['jwt'] @@ -95,36 +95,24 @@ router.patch( if (value.length === 0) throw new Error('secrets cannot be an empty array') for (const secret of value) { if ( - !secret.id || - !secret.secretKeyCiphertext || - !secret.secretKeyIV || - !secret.secretKeyTag || - !secret.secretValueCiphertext || - !secret.secretValueIV || - !secret.secretValueTag + !secret.id ) { - throw new Error('secrets array must contain objects that have required secret properties'); + throw new Error('Each secret must contain a ID property'); } } } else if (typeof value === 'object') { // case: update 1 secret if ( - !value.id || - !value.secretKeyCiphertext || - !value.secretKeyIV || - !value.secretKeyTag || - !value.secretValueCiphertext || - !value.secretValueIV || - !value.secretValueTag + !value.id ) { - throw new Error('secrets object is missing required secret properties'); - } + throw new Error('secret must contain a ID property'); + } } else { throw new Error('secrets must be an object or an array of objects') } - + return true; - }), + }), validateRequest, requireAuth({ acceptedAuthModes: ['jwt'] @@ -142,13 +130,13 @@ router.delete( .custom((value) => { // case: delete 1 secret if (typeof value === 'string') return true; - + if (Array.isArray(value)) { // case: delete multiple secrets if (value.length === 0) throw new Error('secrets cannot be an empty array'); return value.every((id: string) => typeof id === 'string') } - + throw new Error('secretIds must be a string or an array of strings'); }) .not()