diff --git a/backend/src/db/migrations/20240702131735_secret-approval-groups.ts b/backend/src/db/migrations/20240702131735_secret-approval-groups.ts new file mode 100644 index 0000000000..537230b470 --- /dev/null +++ b/backend/src/db/migrations/20240702131735_secret-approval-groups.ts @@ -0,0 +1,188 @@ +import { Knex } from "knex"; + +import { TableName } from "../schemas"; + +export async function up(knex: Knex): Promise { + // migrate secret approval policy approvers to user id + const hasApproverUserId = await knex.schema.hasColumn(TableName.SecretApprovalPolicyApprover, "approverUserId"); + const hasApproverId = await knex.schema.hasColumn(TableName.SecretApprovalPolicyApprover, "approverId"); + if (!hasApproverUserId) { + // add the new fields + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (tb) => { + // if (hasApproverId) tb.setNullable("approverId"); + tb.uuid("approverUserId"); + tb.foreign("approverUserId").references("id").inTable(TableName.Users).onDelete("CASCADE"); + }); + + // convert project membership id => user id + await knex(TableName.SecretApprovalPolicyApprover).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + approverUserId: knex(TableName.ProjectMembership) + .select("userId") + .where("id", knex.raw("??", [`${TableName.SecretApprovalPolicyApprover}.approverId`])) + }); + // drop the old field + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (tb) => { + if (hasApproverId) tb.dropColumn("approverId"); + tb.uuid("approverUserId").notNullable().alter(); + }); + } + + // migrate secret approval request committer and statusChangeBy to user id + const hasSecretApprovalRequestTable = await knex.schema.hasTable(TableName.SecretApprovalRequest); + const hasCommitterUserId = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "committerUserId"); + const hasCommitterId = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "committerId"); + const hasStatusChangeBy = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "statusChangeBy"); + const hasStatusChangedByUserId = await knex.schema.hasColumn( + TableName.SecretApprovalRequest, + "statusChangedByUserId" + ); + if (hasSecretApprovalRequestTable) { + // new fields + await knex.schema.alterTable(TableName.SecretApprovalRequest, (tb) => { + // if (hasCommitterId) tb.setNullable("committerId"); + if (!hasCommitterUserId) { + tb.uuid("committerUserId"); + tb.foreign("committerUserId").references("id").inTable(TableName.Users).onDelete("SET NULL"); + } + if (!hasStatusChangedByUserId) { + tb.uuid("statusChangedByUserId"); + tb.foreign("statusChangedByUserId").references("id").inTable(TableName.Users).onDelete("SET NULL"); + } + }); + + // copy the assigned project membership => user id to new fields + await knex(TableName.SecretApprovalRequest).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + committerUserId: knex(TableName.ProjectMembership) + .select("userId") + .where("id", knex.raw("??", [`${TableName.SecretApprovalRequest}.committerId`])), + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + statusChangedByUserId: knex(TableName.ProjectMembership) + .select("userId") + .where("id", knex.raw("??", [`${TableName.SecretApprovalRequest}.statusChangeBy`])) + }); + // drop old fields + await knex.schema.alterTable(TableName.SecretApprovalRequest, (tb) => { + if (hasStatusChangeBy) tb.dropColumn("statusChangeBy"); + if (hasCommitterId) tb.dropColumn("committerId"); + tb.uuid("committerUserId").notNullable().alter(); + }); + } + + // migrate secret approval request reviewer to user id + const hasMemberId = await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "member"); + const hasReviewerUserId = await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "reviewerUserId"); + if (!hasReviewerUserId) { + // new fields + await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (tb) => { + // if (hasMemberId) tb.setNullable("member"); + tb.uuid("reviewerUserId"); + tb.foreign("reviewerUserId").references("id").inTable(TableName.Users).onDelete("SET NULL"); + }); + // copy project membership => user id to new fields + await knex(TableName.SecretApprovalRequestReviewer).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + reviewerUserId: knex(TableName.ProjectMembership) + .select("userId") + .where("id", knex.raw("??", [`${TableName.SecretApprovalRequestReviewer}.member`])) + }); + // drop table + await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (tb) => { + if (hasMemberId) tb.dropColumn("member"); + tb.uuid("reviewerUserId").notNullable().alter(); + }); + } +} + +export async function down(knex: Knex): Promise { + const hasApproverUserId = await knex.schema.hasColumn(TableName.SecretApprovalPolicyApprover, "approverUserId"); + const hasApproverId = await knex.schema.hasColumn(TableName.SecretApprovalPolicyApprover, "approverId"); + if (hasApproverUserId) { + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (tb) => { + if (!hasApproverId) { + tb.uuid("approverId"); + tb.foreign("approverId").references("id").inTable(TableName.ProjectMembership).onDelete("CASCADE"); + } + }); + + if (!hasApproverId) { + await knex(TableName.SecretApprovalPolicyApprover).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + approverId: knex(TableName.ProjectMembership) + .select("id") + .where("userId", knex.raw("??", [`${TableName.SecretApprovalPolicyApprover}.approverUserId`])) + }); + await knex.schema.alterTable(TableName.SecretApprovalPolicyApprover, (tb) => { + tb.dropColumn("approverUserId"); + tb.uuid("approverId").notNullable().alter(); + }); + } + } + + const hasSecretApprovalRequestTable = await knex.schema.hasTable(TableName.SecretApprovalRequest); + const hasCommitterUserId = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "committerUserId"); + const hasCommitterId = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "committerId"); + const hasStatusChangeBy = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "statusChangeBy"); + const hasStatusChangedByUser = await knex.schema.hasColumn(TableName.SecretApprovalRequest, "statusChangedByUserId"); + if (hasSecretApprovalRequestTable) { + await knex.schema.alterTable(TableName.SecretApprovalRequest, (tb) => { + // if (hasCommitterId) tb.uuid("committerId").notNullable().alter(); + if (!hasCommitterId) { + tb.uuid("committerId"); + tb.foreign("committerId").references("id").inTable(TableName.ProjectMembership).onDelete("CASCADE"); + } + if (!hasStatusChangeBy) { + tb.uuid("statusChangeBy"); + tb.foreign("statusChangeBy").references("id").inTable(TableName.ProjectMembership).onDelete("SET NULL"); + } + }); + + await knex(TableName.SecretApprovalRequest).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + committerId: knex(TableName.ProjectMembership) + .select("id") + .where("userId", knex.raw("??", [`${TableName.SecretApprovalRequest}.committerUserId`])), + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + statusChangeBy: knex(TableName.ProjectMembership) + .select("id") + .where("userId", knex.raw("??", [`${TableName.SecretApprovalRequest}.statusChangedByUserId`])) + }); + + await knex.schema.alterTable(TableName.SecretApprovalRequest, (tb) => { + if (hasCommitterUserId) tb.dropColumn("committerUserId"); + if (hasStatusChangedByUser) tb.dropColumn("statusChangedByUserId"); + if (hasCommitterId) tb.uuid("committerId").notNullable().alter(); + }); + } + + const hasMemberId = await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "member"); + const hasReviewerUserId = await knex.schema.hasColumn(TableName.SecretApprovalRequestReviewer, "reviewerUserId"); + if (hasReviewerUserId) { + if (!hasMemberId) { + await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (tb) => { + // if (hasMemberId) tb.uuid("member").notNullable().alter(); + tb.uuid("member"); + tb.foreign("member").references("id").inTable(TableName.ProjectMembership).onDelete("CASCADE"); + }); + } + await knex(TableName.SecretApprovalRequestReviewer).update({ + // eslint-disable-next-line + // @ts-ignore because generate schema happens after this + member: knex(TableName.ProjectMembership) + .select("id") + .where("userId", knex.raw("??", [`${TableName.SecretApprovalRequestReviewer}.reviewerUserId`])) + }); + await knex.schema.alterTable(TableName.SecretApprovalRequestReviewer, (tb) => { + tb.uuid("member").notNullable().alter(); + tb.dropColumn("reviewerUserId"); + }); + } +} diff --git a/backend/src/db/schemas/secret-approval-policies-approvers.ts b/backend/src/db/schemas/secret-approval-policies-approvers.ts index 12a3119e6d..3af5614f4d 100644 --- a/backend/src/db/schemas/secret-approval-policies-approvers.ts +++ b/backend/src/db/schemas/secret-approval-policies-approvers.ts @@ -9,10 +9,10 @@ import { TImmutableDBKeys } from "./models"; export const SecretApprovalPoliciesApproversSchema = z.object({ id: z.string().uuid(), - approverId: z.string().uuid(), policyId: z.string().uuid(), createdAt: z.date(), - updatedAt: z.date() + updatedAt: z.date(), + approverUserId: z.string().uuid() }); export type TSecretApprovalPoliciesApprovers = z.infer; diff --git a/backend/src/db/schemas/secret-approval-requests-reviewers.ts b/backend/src/db/schemas/secret-approval-requests-reviewers.ts index f3ff880473..a5c4455878 100644 --- a/backend/src/db/schemas/secret-approval-requests-reviewers.ts +++ b/backend/src/db/schemas/secret-approval-requests-reviewers.ts @@ -9,11 +9,11 @@ import { TImmutableDBKeys } from "./models"; export const SecretApprovalRequestsReviewersSchema = z.object({ id: z.string().uuid(), - member: z.string().uuid(), status: z.string(), requestId: z.string().uuid(), createdAt: z.date(), - updatedAt: z.date() + updatedAt: z.date(), + reviewerUserId: z.string().uuid() }); export type TSecretApprovalRequestsReviewers = z.infer; diff --git a/backend/src/db/schemas/secret-approval-requests.ts b/backend/src/db/schemas/secret-approval-requests.ts index 77ad370b7f..aa6896d106 100644 --- a/backend/src/db/schemas/secret-approval-requests.ts +++ b/backend/src/db/schemas/secret-approval-requests.ts @@ -15,11 +15,11 @@ export const SecretApprovalRequestsSchema = z.object({ conflicts: z.unknown().nullable().optional(), slug: z.string(), folderId: z.string().uuid(), - statusChangeBy: z.string().uuid().nullable().optional(), - committerId: z.string().uuid(), createdAt: z.date(), updatedAt: z.date(), - isReplicated: z.boolean().nullable().optional() + isReplicated: z.boolean().nullable().optional(), + committerUserId: z.string().uuid(), + statusChangedByUserId: z.string().uuid().nullable().optional() }); export type TSecretApprovalRequests = z.infer; diff --git a/backend/src/ee/routes/v1/secret-approval-policy-router.ts b/backend/src/ee/routes/v1/secret-approval-policy-router.ts index b09b58e261..ee10131dd0 100644 --- a/backend/src/ee/routes/v1/secret-approval-policy-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-policy-router.ts @@ -25,10 +25,10 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .optional() .nullable() .transform((val) => (val ? removeTrailingSlash(val) : val)), - approvers: z.string().array().min(1), + approverUserIds: z.string().array().min(1), approvals: z.number().min(1).default(1) }) - .refine((data) => data.approvals <= data.approvers.length, { + .refine((data) => data.approvals <= data.approverUserIds.length, { path: ["approvals"], message: "The number of approvals should be lower than the number of approvers." }), @@ -66,7 +66,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi body: z .object({ name: z.string().optional(), - approvers: z.string().array().min(1), + approverUserIds: z.string().array().min(1), approvals: z.number().min(1).default(1), secretPath: z .string() @@ -74,7 +74,7 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi .nullable() .transform((val) => (val ? removeTrailingSlash(val) : val)) }) - .refine((data) => data.approvals <= data.approvers.length, { + .refine((data) => data.approvals <= data.approverUserIds.length, { path: ["approvals"], message: "The number of approvals should be lower than the number of approvers." }), @@ -139,7 +139,15 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi }), response: { 200: z.object({ - approvals: sapPubSchema.merge(z.object({ approvers: z.string().array() })).array() + approvals: sapPubSchema + .extend({ + userApprovers: z + .object({ + userId: z.string() + }) + .array() + }) + .array() }) } }, @@ -170,7 +178,11 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi }), response: { 200: z.object({ - policy: sapPubSchema.merge(z.object({ approvers: z.string().array() })).optional() + policy: sapPubSchema + .extend({ + userApprovers: z.object({ userId: z.string() }).array() + }) + .optional() }) } }, diff --git a/backend/src/ee/routes/v1/secret-approval-request-router.ts b/backend/src/ee/routes/v1/secret-approval-request-router.ts index b7204f72e2..8e72597bde 100644 --- a/backend/src/ee/routes/v1/secret-approval-request-router.ts +++ b/backend/src/ee/routes/v1/secret-approval-request-router.ts @@ -6,7 +6,8 @@ import { SecretApprovalRequestsSecretsSchema, SecretsSchema, SecretTagsSchema, - SecretVersionsSchema + SecretVersionsSchema, + UsersSchema } from "@app/db/schemas"; import { EventType } from "@app/ee/services/audit-log/audit-log-types"; import { ApprovalStatus, RequestState } from "@app/ee/services/secret-approval-request/secret-approval-request-types"; @@ -14,6 +15,15 @@ import { readLimit, writeLimit } from "@app/server/config/rateLimiter"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; +const approvalRequestUser = z.object({ userId: z.string() }).merge( + UsersSchema.pick({ + email: true, + firstName: true, + lastName: true, + username: true + }) +); + export const registerSecretApprovalRequestRouter = async (server: FastifyZodProvider) => { server.route({ method: "GET", @@ -41,9 +51,10 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv approvers: z.string().array(), secretPath: z.string().optional().nullable() }), + committerUser: approvalRequestUser, commits: z.object({ op: z.string(), secretId: z.string().nullable().optional() }).array(), environment: z.string(), - reviewers: z.object({ member: z.string(), status: z.string() }).array(), + reviewers: z.object({ userId: z.string(), status: z.string() }).array(), approvers: z.string().array() }).array() }) @@ -195,7 +206,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv type: isClosing ? EventType.SECRET_APPROVAL_CLOSED : EventType.SECRET_APPROVAL_REOPENED, // eslint-disable-next-line metadata: { - [isClosing ? ("closedBy" as const) : ("reopenedBy" as const)]: approval.statusChangeBy as string, + [isClosing ? ("closedBy" as const) : ("reopenedBy" as const)]: approval.statusChangedByUserId as string, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug // eslint-disable-next-line @@ -216,6 +227,7 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv }) .array() .optional(); + server.route({ method: "GET", url: "/:id", @@ -235,12 +247,13 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv id: z.string(), name: z.string(), approvals: z.number(), - approvers: z.string().array(), + approvers: approvalRequestUser.array(), secretPath: z.string().optional().nullable() }), environment: z.string(), - reviewers: z.object({ member: z.string(), status: z.string() }).array(), - approvers: z.string().array(), + statusChangedByUser: approvalRequestUser.optional(), + committerUser: approvalRequestUser, + reviewers: approvalRequestUser.extend({ status: z.string() }).array(), secretPath: z.string(), commits: SecretApprovalRequestsSecretsSchema.omit({ secretBlindIndex: true }) .merge( diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts index 883e63747c..daf6d0bd8f 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-dal.ts @@ -1,49 +1,59 @@ import { Knex } from "knex"; import { TDbClient } from "@app/db"; -import { TableName, TSecretApprovalPolicies } from "@app/db/schemas"; +import { SecretApprovalPoliciesSchema, TableName, TSecretApprovalPolicies } from "@app/db/schemas"; import { DatabaseError } from "@app/lib/errors"; -import { buildFindFilter, mergeOneToManyRelation, ormify, selectAllTableCols, TFindFilter } from "@app/lib/knex"; +import { buildFindFilter, ormify, selectAllTableCols, sqlNestRelationships, TFindFilter } from "@app/lib/knex"; export type TSecretApprovalPolicyDALFactory = ReturnType; export const secretApprovalPolicyDALFactory = (db: TDbClient) => { const secretApprovalPolicyOrm = ormify(db, TableName.SecretApprovalPolicy); - const sapFindQuery = (tx: Knex, filter: TFindFilter) => + const secretApprovalPolicyFindQuery = (tx: Knex, filter: TFindFilter) => tx(TableName.SecretApprovalPolicy) // eslint-disable-next-line .where(buildFindFilter(filter)) .join(TableName.Environment, `${TableName.SecretApprovalPolicy}.envId`, `${TableName.Environment}.id`) - .join( + .leftJoin( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) - .select(tx.ref("approverId").withSchema(TableName.SecretApprovalPolicyApprover)) - .select(tx.ref("name").withSchema(TableName.Environment).as("envName")) - .select(tx.ref("slug").withSchema(TableName.Environment).as("envSlug")) - .select(tx.ref("id").withSchema(TableName.Environment).as("envId")) - .select(tx.ref("projectId").withSchema(TableName.Environment)) + .select(tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover)) + .select( + tx.ref("name").withSchema(TableName.Environment).as("envName"), + tx.ref("slug").withSchema(TableName.Environment).as("envSlug"), + tx.ref("id").withSchema(TableName.Environment).as("envId"), + tx.ref("projectId").withSchema(TableName.Environment) + ) .select(selectAllTableCols(TableName.SecretApprovalPolicy)) .orderBy("createdAt", "asc"); const findById = async (id: string, tx?: Knex) => { try { - const doc = await sapFindQuery(tx || db.replicaNode(), { + const doc = await secretApprovalPolicyFindQuery(tx || db.replicaNode(), { [`${TableName.SecretApprovalPolicy}.id` as "id"]: id }); - const formatedDoc = mergeOneToManyRelation( - doc, - "id", - ({ approverId, envId, envName: name, envSlug: slug, ...el }) => ({ - ...el, - envId, - environment: { id: envId, name, slug } + const formatedDoc = sqlNestRelationships({ + data: doc, + key: "id", + parentMapper: (data) => ({ + environment: { id: data.envId, name: data.envName, slug: data.envSlug }, + projectId: data.projectId, + ...SecretApprovalPoliciesSchema.parse(data) }), - ({ approverId }) => approverId, - "approvers" - ); + childrenMapper: [ + { + key: "approverUserId", + label: "userApprovers" as const, + mapper: ({ approverUserId }) => ({ + userId: approverUserId + }) + } + ] + }); + return formatedDoc?.[0]; } catch (error) { throw new DatabaseError({ error, name: "FindById" }); @@ -52,18 +62,25 @@ export const secretApprovalPolicyDALFactory = (db: TDbClient) => { const find = async (filter: TFindFilter, tx?: Knex) => { try { - const docs = await sapFindQuery(tx || db.replicaNode(), filter); - const formatedDoc = mergeOneToManyRelation( - docs, - "id", - ({ approverId, envId, envName: name, envSlug: slug, ...el }) => ({ - ...el, - envId, - environment: { id: envId, name, slug } + const docs = await secretApprovalPolicyFindQuery(tx || db.replicaNode(), filter); + const formatedDoc = sqlNestRelationships({ + data: docs, + key: "id", + parentMapper: (data) => ({ + environment: { id: data.envId, name: data.envName, slug: data.envSlug }, + projectId: data.projectId, + ...SecretApprovalPoliciesSchema.parse(data) }), - ({ approverId }) => approverId, - "approvers" - ); + childrenMapper: [ + { + key: "approverUserId", + label: "userApprovers" as const, + mapper: ({ approverUserId }) => ({ + userId: approverUserId + }) + } + ] + }); return formatedDoc; } catch (error) { throw new DatabaseError({ error, name: "Find" }); diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts index f99384de61..2db825c888 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-service.ts @@ -7,7 +7,6 @@ import { BadRequestError } from "@app/lib/errors"; import { removeTrailingSlash } from "@app/lib/fn"; import { containsGlobPatterns } from "@app/lib/picomatch"; import { TProjectEnvDALFactory } from "@app/services/project-env/project-env-dal"; -import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal"; import { TSecretApprovalPolicyApproverDALFactory } from "./secret-approval-policy-approver-dal"; import { TSecretApprovalPolicyDALFactory } from "./secret-approval-policy-dal"; @@ -29,7 +28,6 @@ type TSecretApprovalPolicyServiceFactoryDep = { secretApprovalPolicyDAL: TSecretApprovalPolicyDALFactory; projectEnvDAL: Pick; secretApprovalPolicyApproverDAL: TSecretApprovalPolicyApproverDALFactory; - projectMembershipDAL: Pick; }; export type TSecretApprovalPolicyServiceFactory = ReturnType; @@ -38,8 +36,7 @@ export const secretApprovalPolicyServiceFactory = ({ secretApprovalPolicyDAL, permissionService, secretApprovalPolicyApproverDAL, - projectEnvDAL, - projectMembershipDAL + projectEnvDAL }: TSecretApprovalPolicyServiceFactoryDep) => { const createSecretApprovalPolicy = async ({ name, @@ -48,12 +45,12 @@ export const secretApprovalPolicyServiceFactory = ({ actorOrgId, actorAuthMethod, approvals, - approvers, + approverUserIds, projectId, secretPath, environment }: TCreateSapDTO) => { - if (approvals > approvers.length) + if (approvals > approverUserIds.length) throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); const { permission } = await permissionService.getProjectPermission( @@ -70,13 +67,6 @@ export const secretApprovalPolicyServiceFactory = ({ const env = await projectEnvDAL.findOne({ slug: environment, projectId }); if (!env) throw new BadRequestError({ message: "Environment not found" }); - const secretApprovers = await projectMembershipDAL.find({ - projectId, - $in: { id: approvers } - }); - if (secretApprovers.length !== approvers.length) - throw new BadRequestError({ message: "Approver not found in project" }); - const secretApproval = await secretApprovalPolicyDAL.transaction(async (tx) => { const doc = await secretApprovalPolicyDAL.create( { @@ -88,8 +78,8 @@ export const secretApprovalPolicyServiceFactory = ({ tx ); await secretApprovalPolicyApproverDAL.insertMany( - secretApprovers.map(({ id }) => ({ - approverId: id, + approverUserIds.map((approverUserId) => ({ + approverUserId, policyId: doc.id })), tx @@ -100,7 +90,7 @@ export const secretApprovalPolicyServiceFactory = ({ }; const updateSecretApprovalPolicy = async ({ - approvers, + approverUserIds, secretPath, name, actorId, @@ -132,22 +122,11 @@ export const secretApprovalPolicyServiceFactory = ({ }, tx ); - if (approvers) { - const secretApprovers = await projectMembershipDAL.find( - { - projectId: secretApprovalPolicy.projectId, - $in: { id: approvers } - }, - { tx } - ); - if (secretApprovers.length !== approvers.length) - throw new BadRequestError({ message: "Approver not found in project" }); - if (doc.approvals > secretApprovers.length) - throw new BadRequestError({ message: "Approvals cannot be greater than approvers" }); + if (approverUserIds) { await secretApprovalPolicyApproverDAL.delete({ policyId: doc.id }, tx); await secretApprovalPolicyApproverDAL.insertMany( - secretApprovers.map(({ id }) => ({ - approverId: id, + approverUserIds.map((approverUserId) => ({ + approverUserId, policyId: doc.id })), tx diff --git a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts index 2ddd9b51be..1a527289c5 100644 --- a/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts +++ b/backend/src/ee/services/secret-approval-policy/secret-approval-policy-types.ts @@ -4,7 +4,7 @@ export type TCreateSapDTO = { approvals: number; secretPath?: string | null; environment: string; - approvers: string[]; + approverUserIds: string[]; projectId: string; name: string; } & Omit; @@ -13,7 +13,7 @@ export type TUpdateSapDTO = { secretPolicyId: string; approvals?: number; secretPath?: string | null; - approvers: string[]; + approverUserIds: string[]; name?: string; } & Omit; diff --git a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts index 4eda64f8f2..06c48ac8b3 100644 --- a/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts +++ b/backend/src/ee/services/secret-approval-request/secret-approval-request-dal.ts @@ -5,7 +5,8 @@ import { SecretApprovalRequestsSchema, TableName, TSecretApprovalRequests, - TSecretApprovalRequestsSecrets + TSecretApprovalRequestsSecrets, + TUsers } from "@app/db/schemas"; import { DatabaseError } from "@app/lib/errors"; import { ormify, selectAllTableCols, sqlNestRelationships, stripUndefinedInWhere, TFindFilter } from "@app/lib/knex"; @@ -16,7 +17,7 @@ export type TSecretApprovalRequestDALFactory = ReturnType { `${TableName.SecretApprovalRequest}.policyId`, `${TableName.SecretApprovalPolicy}.id` ) + .leftJoin( + db(TableName.Users).as("statusChangedByUser"), + `${TableName.SecretApprovalRequest}.statusChangedByUserId`, + `statusChangedByUser.id` + ) + .join( + db(TableName.Users).as("committerUser"), + `${TableName.SecretApprovalRequest}.committerUserId`, + `committerUser.id` + ) .join( TableName.SecretApprovalPolicyApprover, `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .join( + db(TableName.Users).as("secretApprovalPolicyApproverUser"), + `${TableName.SecretApprovalPolicyApprover}.approverUserId`, + "secretApprovalPolicyApproverUser.id" + ) .leftJoin( TableName.SecretApprovalRequestReviewer, `${TableName.SecretApprovalRequest}.id`, `${TableName.SecretApprovalRequestReviewer}.requestId` ) + .leftJoin( + db(TableName.Users).as("secretApprovalReviewerUser"), + `${TableName.SecretApprovalRequestReviewer}.reviewerUserId`, + `secretApprovalReviewerUser.id` + ) .select(selectAllTableCols(TableName.SecretApprovalRequest)) .select( - tx.ref("member").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerMemberId"), + tx.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), + tx.ref("email").withSchema("secretApprovalPolicyApproverUser").as("approverEmail"), + tx.ref("username").withSchema("secretApprovalPolicyApproverUser").as("approverUsername"), + tx.ref("firstName").withSchema("secretApprovalPolicyApproverUser").as("approverFirstName"), + tx.ref("lastName").withSchema("secretApprovalPolicyApproverUser").as("approverLastName"), + tx.ref("email").withSchema("statusChangedByUser").as("statusChangedByUserEmail"), + tx.ref("username").withSchema("statusChangedByUser").as("statusChangedByUserUsername"), + tx.ref("firstName").withSchema("statusChangedByUser").as("statusChangedByUserFirstName"), + tx.ref("lastName").withSchema("statusChangedByUser").as("statusChangedByUserLastName"), + tx.ref("email").withSchema("committerUser").as("committerUserEmail"), + tx.ref("username").withSchema("committerUser").as("committerUserUsername"), + tx.ref("firstName").withSchema("committerUser").as("committerUserFirstName"), + tx.ref("lastName").withSchema("committerUser").as("committerUserLastName"), + tx.ref("reviewerUserId").withSchema(TableName.SecretApprovalRequestReviewer), tx.ref("status").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerStatus"), + tx.ref("email").withSchema("secretApprovalReviewerUser").as("reviewerEmail"), + tx.ref("username").withSchema("secretApprovalReviewerUser").as("reviewerUsername"), + tx.ref("firstName").withSchema("secretApprovalReviewerUser").as("reviewerFirstName"), + tx.ref("lastName").withSchema("secretApprovalReviewerUser").as("reviewerLastName"), tx.ref("id").withSchema(TableName.SecretApprovalPolicy).as("policyId"), tx.ref("name").withSchema(TableName.SecretApprovalPolicy).as("policyName"), tx.ref("projectId").withSchema(TableName.Environment), tx.ref("slug").withSchema(TableName.Environment).as("environment"), tx.ref("secretPath").withSchema(TableName.SecretApprovalPolicy).as("policySecretPath"), - tx.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals"), - tx.ref("approverId").withSchema(TableName.SecretApprovalPolicyApprover) + tx.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals") ); const findById = async (id: string, tx?: Knex) => { @@ -71,6 +108,22 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { ...SecretApprovalRequestsSchema.parse(el), projectId: el.projectId, environment: el.environment, + statusChangedByUser: el.statusChangedByUserId + ? { + userId: el.statusChangedByUserId, + email: el.statusChangedByUserEmail, + firstName: el.statusChangedByUserFirstName, + lastName: el.statusChangedByUserLastName, + username: el.statusChangedByUserUsername + } + : undefined, + committerUser: { + userId: el.committerUserId, + email: el.committerUserEmail, + firstName: el.committerUserFirstName, + lastName: el.committerUserLastName, + username: el.committerUserUsername + }, policy: { id: el.policyId, name: el.policyName, @@ -80,11 +133,34 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { }), childrenMapper: [ { - key: "reviewerMemberId", + key: "reviewerUserId", label: "reviewers" as const, - mapper: ({ reviewerMemberId: member, reviewerStatus: status }) => (member ? { member, status } : undefined) + mapper: ({ + reviewerUserId: userId, + reviewerStatus: status, + reviewerEmail: email, + reviewerLastName: lastName, + reviewerUsername: username, + reviewerFirstName: firstName + }) => (userId ? { userId, status, email, firstName, lastName, username } : undefined) }, - { key: "approverId", label: "approvers" as const, mapper: ({ approverId }) => approverId } + { + key: "approverUserId", + label: "approvers" as const, + mapper: ({ + approverUserId, + approverEmail: email, + approverUsername: username, + approverLastName: lastName, + approverFirstName: firstName + }) => ({ + userId: approverUserId, + email, + firstName, + lastName, + username + }) + } ] }); if (!formatedDoc?.[0]) return; @@ -97,7 +173,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { } }; - const findProjectRequestCount = async (projectId: string, membershipId: string, tx?: Knex) => { + const findProjectRequestCount = async (projectId: string, userId: string, tx?: Knex) => { try { const docs = await (tx || db) .with( @@ -114,8 +190,8 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { .andWhere( (bd) => void bd - .where(`${TableName.SecretApprovalPolicyApprover}.approverId`, membershipId) - .orWhere(`${TableName.SecretApprovalRequest}.committerId`, membershipId) + .where(`${TableName.SecretApprovalPolicyApprover}.approverUserId`, userId) + .orWhere(`${TableName.SecretApprovalRequest}.committerUserId`, userId) ) .select("status", `${TableName.SecretApprovalRequest}.id`) .groupBy(`${TableName.SecretApprovalRequest}.id`, "status") @@ -142,7 +218,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { }; const findByProjectId = async ( - { status, limit = 20, offset = 0, projectId, committer, environment, membershipId }: TFindQueryFilter, + { status, limit = 20, offset = 0, projectId, committer, environment, userId }: TFindQueryFilter, tx?: Knex ) => { try { @@ -161,6 +237,11 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { `${TableName.SecretApprovalPolicy}.id`, `${TableName.SecretApprovalPolicyApprover}.policyId` ) + .join( + db(TableName.Users).as("committerUser"), + `${TableName.SecretApprovalRequest}.committerUserId`, + `committerUser.id` + ) .leftJoin( TableName.SecretApprovalRequestReviewer, `${TableName.SecretApprovalRequest}.id`, @@ -176,20 +257,21 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { projectId, [`${TableName.Environment}.slug` as "slug"]: environment, [`${TableName.SecretApprovalRequest}.status`]: status, - committerId: committer + committerUserId: committer }) ) .andWhere( (bd) => void bd - .where(`${TableName.SecretApprovalPolicyApprover}.approverId`, membershipId) - .orWhere(`${TableName.SecretApprovalRequest}.committerId`, membershipId) + .where(`${TableName.SecretApprovalPolicyApprover}.approverUserId`, userId) + .orWhere(`${TableName.SecretApprovalRequest}.committerUserId`, userId) ) .select(selectAllTableCols(TableName.SecretApprovalRequest)) .select( db.ref("projectId").withSchema(TableName.Environment), db.ref("slug").withSchema(TableName.Environment).as("environment"), - db.ref("id").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerMemberId"), + db.ref("id").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerId"), + db.ref("reviewerUserId").withSchema(TableName.SecretApprovalRequestReviewer), db.ref("status").withSchema(TableName.SecretApprovalRequestReviewer).as("reviewerStatus"), db.ref("id").withSchema(TableName.SecretApprovalPolicy).as("policyId"), db.ref("name").withSchema(TableName.SecretApprovalPolicy).as("policyName"), @@ -201,7 +283,11 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { ), db.ref("secretPath").withSchema(TableName.SecretApprovalPolicy).as("policySecretPath"), db.ref("approvals").withSchema(TableName.SecretApprovalPolicy).as("policyApprovals"), - db.ref("approverId").withSchema(TableName.SecretApprovalPolicyApprover) + db.ref("approverUserId").withSchema(TableName.SecretApprovalPolicyApprover), + db.ref("email").withSchema("committerUser").as("committerUserEmail"), + db.ref("username").withSchema("committerUser").as("committerUserUsername"), + db.ref("firstName").withSchema("committerUser").as("committerUserFirstName"), + db.ref("lastName").withSchema("committerUser").as("committerUserLastName") ) .orderBy("createdAt", "desc"); @@ -223,18 +309,26 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => { name: el.policyName, approvals: el.policyApprovals, secretPath: el.policySecretPath + }, + committerUser: { + userId: el.committerUserId, + email: el.committerUserEmail, + firstName: el.committerUserFirstName, + lastName: el.committerUserLastName, + username: el.committerUserUsername } }), childrenMapper: [ { - key: "reviewerMemberId", + key: "reviewerId", label: "reviewers" as const, - mapper: ({ reviewerMemberId: member, reviewerStatus: s }) => (member ? { member, status: s } : undefined) + mapper: ({ reviewerUserId, reviewerStatus: s }) => + reviewerUserId ? { userId: reviewerUserId, status: s } : undefined }, { - key: "approverId", + key: "approverUserId", label: "approvers" as const, - mapper: ({ approverId }) => approverId + mapper: ({ approverUserId }) => approverUserId }, { key: "commitId", 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 5d09771347..a519af4fd2 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 @@ -87,7 +87,7 @@ export const secretApprovalRequestServiceFactory = ({ const requestCount = async ({ projectId, actor, actorId, actorOrgId, actorAuthMethod }: TApprovalRequestCountDTO) => { if (actor === ActorType.SERVICE) throw new BadRequestError({ message: "Cannot use service token" }); - const { membership } = await permissionService.getProjectPermission( + await permissionService.getProjectPermission( actor as ActorType.USER, actorId, projectId, @@ -95,7 +95,7 @@ export const secretApprovalRequestServiceFactory = ({ actorOrgId ); - const count = await secretApprovalRequestDAL.findProjectRequestCount(projectId, membership.id); + const count = await secretApprovalRequestDAL.findProjectRequestCount(projectId, actorId); return count; }; @@ -113,19 +113,13 @@ export const secretApprovalRequestServiceFactory = ({ }: TListApprovalsDTO) => { if (actor === ActorType.SERVICE) throw new BadRequestError({ message: "Cannot use service token" }); - const { membership } = await permissionService.getProjectPermission( - actor, - actorId, - projectId, - actorAuthMethod, - actorOrgId - ); + await permissionService.getProjectPermission(actor, actorId, projectId, actorAuthMethod, actorOrgId); const approvals = await secretApprovalRequestDAL.findByProjectId({ projectId, committer, environment, status, - membershipId: membership.id, + userId: actorId, limit, offset }); @@ -145,7 +139,7 @@ export const secretApprovalRequestServiceFactory = ({ if (!secretApprovalRequest) throw new BadRequestError({ message: "Secret approval request not found" }); const { policy } = secretApprovalRequest; - const { membership, hasRole } = await permissionService.getProjectPermission( + const { hasRole } = await permissionService.getProjectPermission( actor, actorId, secretApprovalRequest.projectId, @@ -154,8 +148,8 @@ export const secretApprovalRequestServiceFactory = ({ ); if ( !hasRole(ProjectMembershipRole.Admin) && - secretApprovalRequest.committerId !== membership.id && - !policy.approvers.find((approverId) => approverId === membership.id) + secretApprovalRequest.committerUserId !== actorId && + !policy.approvers.find(({ userId }) => userId === actorId) ) { throw new UnauthorizedError({ message: "User has no access" }); } @@ -180,7 +174,7 @@ export const secretApprovalRequestServiceFactory = ({ if (actor !== ActorType.USER) throw new BadRequestError({ message: "Must be a user" }); const { policy } = secretApprovalRequest; - const { membership, hasRole } = await permissionService.getProjectPermission( + const { hasRole } = await permissionService.getProjectPermission( ActorType.USER, actorId, secretApprovalRequest.projectId, @@ -189,8 +183,8 @@ export const secretApprovalRequestServiceFactory = ({ ); if ( !hasRole(ProjectMembershipRole.Admin) && - secretApprovalRequest.committerId !== membership.id && - !policy.approvers.find((approverId) => approverId === membership.id) + secretApprovalRequest.committerUserId !== actorId && + !policy.approvers.find(({ userId }) => userId === actorId) ) { throw new UnauthorizedError({ message: "User has no access" }); } @@ -198,7 +192,7 @@ export const secretApprovalRequestServiceFactory = ({ const review = await secretApprovalRequestReviewerDAL.findOne( { requestId: secretApprovalRequest.id, - member: membership.id + reviewerUserId: actorId }, tx ); @@ -207,7 +201,7 @@ export const secretApprovalRequestServiceFactory = ({ { status, requestId: secretApprovalRequest.id, - member: membership.id + reviewerUserId: actorId }, tx ); @@ -230,7 +224,7 @@ export const secretApprovalRequestServiceFactory = ({ if (actor !== ActorType.USER) throw new BadRequestError({ message: "Must be a user" }); const { policy } = secretApprovalRequest; - const { membership, hasRole } = await permissionService.getProjectPermission( + const { hasRole } = await permissionService.getProjectPermission( ActorType.USER, actorId, secretApprovalRequest.projectId, @@ -239,8 +233,8 @@ export const secretApprovalRequestServiceFactory = ({ ); if ( !hasRole(ProjectMembershipRole.Admin) && - secretApprovalRequest.committerId !== membership.id && - !policy.approvers.find((approverId) => approverId === membership.id) + secretApprovalRequest.committerUserId !== actorId && + !policy.approvers.find(({ userId }) => userId === actorId) ) { throw new UnauthorizedError({ message: "User has no access" }); } @@ -253,7 +247,7 @@ export const secretApprovalRequestServiceFactory = ({ const updatedRequest = await secretApprovalRequestDAL.updateById(secretApprovalRequest.id, { status, - statusChangeBy: membership.id + statusChangedByUserId: actorId }); return { ...secretApprovalRequest, ...updatedRequest }; }; @@ -270,7 +264,7 @@ export const secretApprovalRequestServiceFactory = ({ if (actor !== ActorType.USER) throw new BadRequestError({ message: "Must be a user" }); const { policy, folderId, projectId } = secretApprovalRequest; - const { membership, hasRole } = await permissionService.getProjectPermission( + const { hasRole } = await permissionService.getProjectPermission( ActorType.USER, actorId, projectId, @@ -280,19 +274,19 @@ export const secretApprovalRequestServiceFactory = ({ if ( !hasRole(ProjectMembershipRole.Admin) && - secretApprovalRequest.committerId !== membership.id && - !policy.approvers.find((approverId) => approverId === membership.id) + secretApprovalRequest.committerUserId !== actorId && + !policy.approvers.find(({ userId }) => userId === actorId) ) { throw new UnauthorizedError({ message: "User has no access" }); } const reviewers = secretApprovalRequest.reviewers.reduce>( - (prev, curr) => ({ ...prev, [curr.member.toString()]: curr.status as ApprovalStatus }), + (prev, curr) => ({ ...prev, [curr.userId.toString()]: curr.status as ApprovalStatus }), {} ); const hasMinApproval = secretApprovalRequest.policy.approvals <= secretApprovalRequest.policy.approvers.filter( - (approverId) => reviewers[approverId.toString()] === ApprovalStatus.APPROVED + ({ userId: approverId }) => reviewers[approverId.toString()] === ApprovalStatus.APPROVED ).length; if (!hasMinApproval) throw new BadRequestError({ message: "Doesn't have minimum approvals needed" }); @@ -472,7 +466,7 @@ export const secretApprovalRequestServiceFactory = ({ conflicts: JSON.stringify(conflicts), hasMerged: true, status: RequestState.Closed, - statusChangeBy: membership.id + statusChangedByUserId: actorId }, tx ); @@ -509,7 +503,7 @@ export const secretApprovalRequestServiceFactory = ({ }: TGenerateSecretApprovalRequestDTO) => { if (actor === ActorType.SERVICE) throw new BadRequestError({ message: "Cannot use service token" }); - const { permission, membership } = await permissionService.getProjectPermission( + const { permission } = await permissionService.getProjectPermission( actor, actorId, projectId, @@ -663,7 +657,7 @@ export const secretApprovalRequestServiceFactory = ({ policyId: policy.id, status: "open", hasMerged: false, - committerId: membership.id + committerUserId: actorId }, tx ); diff --git a/backend/src/ee/services/secret-replication/secret-replication-service.ts b/backend/src/ee/services/secret-replication/secret-replication-service.ts index fd2f7cc1a4..01f7d066ce 100644 --- a/backend/src/ee/services/secret-replication/secret-replication-service.ts +++ b/backend/src/ee/services/secret-replication/secret-replication-service.ts @@ -11,7 +11,6 @@ import { alphaNumericNanoId } from "@app/lib/nanoid"; import { QueueName, TQueueServiceFactory } from "@app/queue"; import { ActorType } from "@app/services/auth/auth-type"; import { TProjectBotServiceFactory } from "@app/services/project-bot/project-bot-service"; -import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal"; import { TSecretDALFactory } from "@app/services/secret/secret-dal"; import { fnSecretBulkInsert, fnSecretBulkUpdate } from "@app/services/secret/secret-fns"; import { TSecretQueueFactory, uniqueSecretQueueKey } from "@app/services/secret/secret-queue"; @@ -46,7 +45,6 @@ type TSecretReplicationServiceFactoryDep = { secretBlindIndexDAL: Pick; secretTagDAL: Pick; secretApprovalRequestDAL: Pick; - projectMembershipDAL: Pick; secretApprovalRequestSecretDAL: Pick< TSecretApprovalRequestSecretDALFactory, "insertMany" | "insertApprovalSecretTags" @@ -92,7 +90,6 @@ export const secretReplicationServiceFactory = ({ secretApprovalRequestSecretDAL, secretApprovalRequestDAL, secretQueueService, - projectMembershipDAL, projectBotService }: TSecretReplicationServiceFactoryDep) => { const getReplicatedSecrets = ( @@ -297,12 +294,6 @@ export const secretReplicationServiceFactory = ({ ); // this means it should be a approval request rather than direct replication if (policy && actor === ActorType.USER) { - const membership = await projectMembershipDAL.findOne({ projectId, userId: actorId }); - if (!membership) { - logger.error("Project membership not found in %s for user %s", projectId, actorId); - return; - } - const localSecretsLatestVersions = destinationLocalSecrets.map(({ id }) => id); const latestSecretVersions = await secretVersionDAL.findLatestVersionMany( destinationReplicationFolderId, @@ -316,7 +307,7 @@ export const secretReplicationServiceFactory = ({ policyId: policy.id, status: "open", hasMerged: false, - committerId: membership.id, + committerUserId: actorId, isReplicated: true }, tx diff --git a/backend/src/server/routes/index.ts b/backend/src/server/routes/index.ts index 54c5e51d66..e27d76af0c 100644 --- a/backend/src/server/routes/index.ts +++ b/backend/src/server/routes/index.ts @@ -319,7 +319,6 @@ export const registerRoutes = async ( auditLogStreamDAL }); const secretApprovalPolicyService = secretApprovalPolicyServiceFactory({ - projectMembershipDAL, projectEnvDAL, secretApprovalPolicyApproverDAL: sapApproverDAL, permissionService, @@ -768,7 +767,6 @@ export const registerRoutes = async ( secretApprovalRequestDAL, secretApprovalRequestSecretDAL, secretQueueService, - projectMembershipDAL, projectBotService }); const secretRotationQueue = secretRotationQueueFactory({ diff --git a/backend/src/server/routes/v3/secret-router.ts b/backend/src/server/routes/v3/secret-router.ts index b92138ec8f..910cc3db97 100644 --- a/backend/src/server/routes/v3/secret-router.ts +++ b/backend/src/server/routes/v3/secret-router.ts @@ -949,7 +949,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } @@ -1133,7 +1133,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } @@ -1271,7 +1271,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } @@ -1397,7 +1397,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } @@ -1524,7 +1524,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } @@ -1638,7 +1638,7 @@ export const registerSecretRouter = async (server: FastifyZodProvider) => { event: { type: EventType.SECRET_APPROVAL_REQUEST, metadata: { - committedBy: approval.committerId, + committedBy: approval.committerUserId, secretApprovalRequestId: approval.id, secretApprovalRequestSlug: approval.slug } diff --git a/frontend/src/hooks/api/secretApproval/mutation.tsx b/frontend/src/hooks/api/secretApproval/mutation.tsx index e0a8df95da..991111ef99 100644 --- a/frontend/src/hooks/api/secretApproval/mutation.tsx +++ b/frontend/src/hooks/api/secretApproval/mutation.tsx @@ -9,12 +9,12 @@ export const useCreateSecretApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TCreateSecretPolicyDTO>({ - mutationFn: async ({ environment, workspaceId, approvals, approvers, secretPath, name }) => { + mutationFn: async ({ environment, workspaceId, approvals, approverUserIds, secretPath, name }) => { const { data } = await apiRequest.post("/api/v1/secret-approvals", { environment, workspaceId, approvals, - approvers, + approverUserIds, secretPath, name }); @@ -30,10 +30,10 @@ export const useUpdateSecretApprovalPolicy = () => { const queryClient = useQueryClient(); return useMutation<{}, {}, TUpdateSecretPolicyDTO>({ - mutationFn: async ({ id, approvers, approvals, secretPath, name }) => { + mutationFn: async ({ id, approverUserIds, approvals, secretPath, name }) => { const { data } = await apiRequest.patch(`/api/v1/secret-approvals/${id}`, { approvals, - approvers, + approverUserIds, secretPath, name }); diff --git a/frontend/src/hooks/api/secretApproval/types.ts b/frontend/src/hooks/api/secretApproval/types.ts index 38e8d7ee8e..f3b8639f70 100644 --- a/frontend/src/hooks/api/secretApproval/types.ts +++ b/frontend/src/hooks/api/secretApproval/types.ts @@ -7,8 +7,8 @@ export type TSecretApprovalPolicy = { envId: string; environment: WorkspaceEnv; secretPath?: string; - approvers: string[]; approvals: number; + userApprovers: { userId: string }[]; }; export type TGetSecretApprovalPoliciesDTO = { @@ -26,14 +26,14 @@ export type TCreateSecretPolicyDTO = { name?: string; environment: string; secretPath?: string | null; - approvers?: string[]; + approverUserIds?: string[]; approvals?: number; }; export type TUpdateSecretPolicyDTO = { id: string; name?: string; - approvers?: string[]; + approverUserIds?: string[]; secretPath?: string | null; approvals?: number; // for invalidating list diff --git a/frontend/src/hooks/api/secretApprovalRequest/types.ts b/frontend/src/hooks/api/secretApprovalRequest/types.ts index 8c2ba69636..f039cc815e 100644 --- a/frontend/src/hooks/api/secretApprovalRequest/types.ts +++ b/frontend/src/hooks/api/secretApprovalRequest/types.ts @@ -47,10 +47,14 @@ export type TSecretApprovalRequest = { isReplicated?: boolean; slug: string; createdAt: string; - committerId: string; + committerUserId: string; reviewers: { - member: string; + userId: string; status: ApprovalStatus; + email: string; + firstName: string; + lastName: string; + username: string; }[]; workspace: string; environment: string; @@ -58,8 +62,30 @@ export type TSecretApprovalRequest = { secretPath: string; hasMerged: boolean; status: "open" | "close"; - policy: TSecretApprovalPolicy; - statusChangeBy: string; + policy: Omit & { + approvers: { + userId: string; + email: string; + firstName: string; + lastName: string; + username: string; + }[]; + }; + statusChangedByUserId: string; + statusChangedByUser?: { + userId: string; + email: string; + firstName: string; + lastName: string; + username: string; + }; + committerUser: { + userId: string; + email: string; + firstName: string; + lastName: string; + username: string; + }; conflicts: Array<{ secretId: string; op: CommitType.UPDATE }>; commits: ({ // if there is no secret means it was creation diff --git a/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/SecretApprovalPolicyList.tsx b/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/SecretApprovalPolicyList.tsx index d3a36988c1..f377f44e96 100644 --- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/SecretApprovalPolicyList.tsx +++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/SecretApprovalPolicyList.tsx @@ -7,6 +7,8 @@ import { Button, DeleteActionModal, EmptyState, + Modal, + ModalContent, Table, TableContainer, TableSkeleton, @@ -145,13 +147,20 @@ export const SecretApprovalPolicyList = ({ workspaceId }: Props) => { - handlePopUpToggle("secretPolicyForm", isOpen)} - members={members} - editValues={popUp.secretPolicyForm.data as TSecretApprovalPolicy} - /> + onOpenChange={(isOpen) => handlePopUpToggle("secretPolicyForm", isOpen)} + > + + handlePopUpToggle("secretPolicyForm", isOpen)} + members={members} + editValues={popUp.secretPolicyForm.data as TSecretApprovalPolicy} + /> + + { @@ -60,7 +60,7 @@ export const SecretApprovalPolicyRow = ({ } ); } else { - setSelectedApprovers(policy.approvers); + setSelectedApprovers(policy.userApprovers.map(({ userId }) => userId)); } }} > @@ -73,7 +73,9 @@ export const SecretApprovalPolicyRow = ({ > @@ -84,17 +86,17 @@ export const SecretApprovalPolicyRow = ({ Select members that are allowed to approve changes - {members?.map(({ id, user }) => { - const isChecked = selectedApprovers.includes(id); + {members?.map(({ user }) => { + const isChecked = selectedApprovers.includes(user.id); return ( { evt.preventDefault(); setSelectedApprovers((state) => - isChecked ? state.filter((el) => el !== id) : [...state, id] + isChecked ? state.filter((el) => el !== user.id) : [...state, user.id] ); }} - key={`create-policy-members-${id}`} + key={`create-policy-members-${user.id}`} iconPos="right" icon={isChecked && } > diff --git a/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/components/SecretPolicyForm.tsx b/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/components/SecretPolicyForm.tsx index db59761c11..b0db2affd5 100644 --- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/components/SecretPolicyForm.tsx +++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalPolicyList/components/SecretPolicyForm.tsx @@ -1,4 +1,3 @@ -import { useEffect } from "react"; import { Controller, useForm } from "react-hook-form"; import { faCheckCircle } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -15,8 +14,6 @@ import { DropdownMenuTrigger, FormControl, Input, - Modal, - ModalContent, Select, SelectItem } from "@app/components/v2"; @@ -40,9 +37,9 @@ const formSchema = z name: z.string().optional(), secretPath: z.string().optional().nullable(), approvals: z.number().min(1), - approvers: z.string().array().min(1) + approverUserIds: z.string().array().min(1) }) - .refine((data) => data.approvals <= data.approvers.length, { + .refine((data) => data.approvals <= data.approverUserIds.length, { path: ["approvals"], message: "The number of approvals should be lower than the number of approvers." }); @@ -50,7 +47,6 @@ const formSchema = z type TFormSchema = z.infer; export const SecretPolicyForm = ({ - isOpen, onToggle, members = [], workspaceId, @@ -59,20 +55,22 @@ export const SecretPolicyForm = ({ const { control, handleSubmit, - reset, watch, formState: { isSubmitting } } = useForm({ resolver: zodResolver(formSchema), - values: editValues ? { ...editValues, environment: editValues.environment.slug } : undefined + values: editValues + ? { + ...editValues, + approverUserIds: editValues.userApprovers.map(({ userId }) => userId), + environment: editValues.environment.slug + } + : undefined }); const { currentWorkspace } = useWorkspace(); const selectedEnvironment = watch("environment"); const environments = currentWorkspace?.environments || []; - useEffect(() => { - if (!isOpen) reset({}); - }, [isOpen]); const isEditMode = Boolean(editValues); @@ -131,8 +129,6 @@ export const SecretPolicyForm = ({ }; return ( - -