Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Approval Workflows): Workflows keep approval history after deletion #2834

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { Knex } from "knex";

import { TableName } from "../schemas";

export async function up(knex: Knex): Promise<void> {
const hasAccessApprovalPolicyDisabledColumn = await knex.schema.hasColumn(TableName.AccessApprovalPolicy, "disabled");
const hasSecretApprovalPolicyDisabledColumn = await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "disabled");

if (!hasAccessApprovalPolicyDisabledColumn) {
await knex.schema.alterTable(TableName.AccessApprovalPolicy, (t) => {
t.boolean("disabled").defaultTo(false).notNullable();
});
}
if (!hasSecretApprovalPolicyDisabledColumn) {
await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
t.boolean("disabled").defaultTo(false).notNullable();
});
}
}

export async function down(knex: Knex): Promise<void> {
const hasAccessApprovalPolicyDisabledColumn = await knex.schema.hasColumn(TableName.AccessApprovalPolicy, "disabled");
const hasSecretApprovalPolicyDisabledColumn = await knex.schema.hasColumn(TableName.SecretApprovalPolicy, "disabled");

if (hasAccessApprovalPolicyDisabledColumn) {
await knex.schema.alterTable(TableName.AccessApprovalPolicy, (t) => {
t.dropColumn("disabled");
});
}
if (hasSecretApprovalPolicyDisabledColumn) {
await knex.schema.alterTable(TableName.SecretApprovalPolicy, (t) => {
t.dropColumn("disabled");
});
}
}
3 changes: 2 additions & 1 deletion backend/src/db/schemas/access-approval-policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export const AccessApprovalPoliciesSchema = z.object({
envId: z.string().uuid(),
createdAt: z.date(),
updatedAt: z.date(),
enforcementLevel: z.string().default("hard")
enforcementLevel: z.string().default("hard"),
disabled: z.boolean().default(false)
});

export type TAccessApprovalPolicies = z.infer<typeof AccessApprovalPoliciesSchema>;
Expand Down
3 changes: 2 additions & 1 deletion backend/src/db/schemas/secret-approval-policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export const SecretApprovalPoliciesSchema = z.object({
envId: z.string().uuid(),
createdAt: z.date(),
updatedAt: z.date(),
enforcementLevel: z.string().default("hard")
enforcementLevel: z.string().default("hard"),
disabled: z.boolean().default(false)
});

export type TSecretApprovalPolicies = z.infer<typeof SecretApprovalPoliciesSchema>;
Expand Down
3 changes: 2 additions & 1 deletion backend/src/ee/routes/v1/access-approval-policy-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ export const registerAccessApprovalPolicyRouter = async (server: FastifyZodProvi
.array()
.min(1, { message: "At least one approver should be provided" }),
approvals: z.number().min(1).optional(),
enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard)
enforcementLevel: z.nativeEnum(EnforcementLevel).default(EnforcementLevel.Hard),
disabled: z.boolean().optional()
}),
response: {
200: z.object({
Expand Down
3 changes: 2 additions & 1 deletion backend/src/ee/routes/v1/secret-approval-policy-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export const registerSecretApprovalPolicyRouter = async (server: FastifyZodProvi
.nullable()
.transform((val) => (val ? removeTrailingSlash(val) : val))
.transform((val) => (val === "" ? "/" : val)),
enforcementLevel: z.nativeEnum(EnforcementLevel).optional()
enforcementLevel: z.nativeEnum(EnforcementLevel).optional(),
disabled: z.boolean().optional()
}),
response: {
200: z.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export const accessApprovalPolicyServiceFactory = ({
);
// ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval);

const accessApprovalPolicies = await accessApprovalPolicyDAL.find({ projectId: project.id });
const accessApprovalPolicies = await accessApprovalPolicyDAL.find({ projectId: project.id, disabled: false });
return accessApprovalPolicies;
};

Expand All @@ -203,7 +203,8 @@ export const accessApprovalPolicyServiceFactory = ({
actorOrgId,
actorAuthMethod,
approvals,
enforcementLevel
enforcementLevel,
disabled
}: TUpdateAccessApprovalPolicy) => {
const groupApprovers = approvers
.filter((approver) => approver.type === ApproverType.Group)
Expand Down Expand Up @@ -248,7 +249,8 @@ export const accessApprovalPolicyServiceFactory = ({
approvals,
secretPath,
name,
enforcementLevel
enforcementLevel,
disabled
},
tx
);
Expand Down Expand Up @@ -356,7 +358,11 @@ export const accessApprovalPolicyServiceFactory = ({
const environment = await projectEnvDAL.findOne({ projectId: project.id, slug: envSlug });
if (!environment) throw new NotFoundError({ message: `Environment with slug '${envSlug}' not found` });

const policies = await accessApprovalPolicyDAL.find({ envId: environment.id, projectId: project.id });
const policies = await accessApprovalPolicyDAL.find({
envId: environment.id,
projectId: project.id,
disabled: false
});
if (!policies) throw new NotFoundError({ message: `No policies found in environment with slug '${envSlug}'` });

return { count: policies.length };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type TUpdateAccessApprovalPolicy = {
secretPath?: string;
name?: string;
enforcementLevel?: EnforcementLevel;
disabled?: boolean;
} & Omit<TProjectPermission, "projectId">;

export type TDeleteAccessApprovalPolicy = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ export const secretApprovalPolicyServiceFactory = ({
actorAuthMethod,
approvals,
secretPolicyId,
enforcementLevel
enforcementLevel,
disabled
}: TUpdateSapDTO) => {
const groupApprovers = approvers
?.filter((approver) => approver.type === ApproverType.Group)
Expand Down Expand Up @@ -211,7 +212,8 @@ export const secretApprovalPolicyServiceFactory = ({
approvals,
secretPath,
name,
enforcementLevel
enforcementLevel,
disabled
},
tx
);
Expand Down Expand Up @@ -321,7 +323,7 @@ export const secretApprovalPolicyServiceFactory = ({
);
ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionActions.Read, ProjectPermissionSub.SecretApproval);

const sapPolicies = await secretApprovalPolicyDAL.find({ projectId });
const sapPolicies = await secretApprovalPolicyDAL.find({ projectId, disabled: false });
return sapPolicies;
};

Expand All @@ -334,7 +336,7 @@ export const secretApprovalPolicyServiceFactory = ({
});
}

const policies = await secretApprovalPolicyDAL.find({ envId: env.id });
const policies = await secretApprovalPolicyDAL.find({ envId: env.id, disabled: false });
if (!policies.length) return;
// this will filter policies either without scoped to secret path or the one that matches with secret path
const policiesFilteredByPath = policies.filter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type TUpdateSapDTO = {
approvers: ({ type: ApproverType.Group; id: string } | { type: ApproverType.User; id?: string; name?: string })[];
name?: string;
enforcementLevel?: EnforcementLevel;
disabled?: boolean;
} & Omit<TProjectPermission, "projectId">;

export type TDeleteSapDTO = {
Expand Down
1 change: 0 additions & 1 deletion frontend/src/hooks/api/accessApproval/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export {
useCreateAccessApprovalPolicy,
useCreateAccessRequest,
useDeleteAccessApprovalPolicy,
useReviewAccessRequest,
useUpdateAccessApprovalPolicy
} from "./mutation";
Expand Down
28 changes: 11 additions & 17 deletions frontend/src/hooks/api/accessApproval/mutation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
TAccessApproval,
TCreateAccessPolicyDTO,
TCreateAccessRequestDTO,
TDeleteSecretPolicyDTO,
TUpdateAccessPolicyDTO
} from "./types";

Expand Down Expand Up @@ -46,13 +45,22 @@ export const useUpdateAccessApprovalPolicy = () => {
const queryClient = useQueryClient();

return useMutation<{}, {}, TUpdateAccessPolicyDTO>({
mutationFn: async ({ id, approvers, approvals, name, secretPath, enforcementLevel }) => {
mutationFn: async ({
id,
approvers,
approvals,
name,
secretPath,
enforcementLevel,
disabled
}) => {
const { data } = await apiRequest.patch(`/api/v1/access-approvals/policies/${id}`, {
approvals,
approvers,
secretPath,
name,
enforcementLevel
enforcementLevel,
disabled
});
return data;
},
Expand All @@ -62,20 +70,6 @@ export const useUpdateAccessApprovalPolicy = () => {
});
};

export const useDeleteAccessApprovalPolicy = () => {
DanielHougaard marked this conversation as resolved.
Show resolved Hide resolved
const queryClient = useQueryClient();

return useMutation<{}, {}, TDeleteSecretPolicyDTO>({
mutationFn: async ({ id }) => {
const { data } = await apiRequest.delete(`/api/v1/access-approvals/policies/${id}`);
return data;
},
onSuccess: (_, { projectSlug }) => {
queryClient.invalidateQueries(accessApprovalKeys.getAccessApprovalPolicies(projectSlug));
}
});
};

export const useCreateAccessRequest = () => {
const queryClient = useQueryClient();
return useMutation<{}, {}, TCreateAccessRequestDTO>({
Expand Down
13 changes: 4 additions & 9 deletions frontend/src/hooks/api/accessApproval/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ export type TAccessApprovalPolicy = {
approvers?: Approver[];
};

export enum ApproverType{
export enum ApproverType {
User = "user",
Group = "group"
}

export type Approver ={
export type Approver = {
id: string;
type: ApproverType;
}
};

export type TAccessApprovalRequest = {
id: string;
Expand Down Expand Up @@ -153,12 +153,7 @@ export type TUpdateAccessPolicyDTO = {
environment?: string;
approvals?: number;
enforcementLevel?: EnforcementLevel;
// for invalidating list
projectSlug: string;
};

export type TDeleteSecretPolicyDTO = {
id: string;
disabled?: boolean;
// for invalidating list
projectSlug: string;
};
6 changes: 1 addition & 5 deletions frontend/src/hooks/api/secretApproval/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,2 @@
export {
useCreateSecretApprovalPolicy,
useDeleteSecretApprovalPolicy,
useUpdateSecretApprovalPolicy
} from "./mutation";
export { useCreateSecretApprovalPolicy, useUpdateSecretApprovalPolicy } from "./mutation";
export { useGetSecretApprovalPolicies, useGetSecretApprovalPolicyOfABoard } from "./queries";
29 changes: 12 additions & 17 deletions frontend/src/hooks/api/secretApproval/mutation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query";
import { apiRequest } from "@app/config/request";

import { secretApprovalKeys } from "./queries";
import { TCreateSecretPolicyDTO, TDeleteSecretPolicyDTO, TUpdateSecretPolicyDTO } from "./types";
import { TCreateSecretPolicyDTO, TUpdateSecretPolicyDTO } from "./types";

export const useCreateSecretApprovalPolicy = () => {
const queryClient = useQueryClient();
Expand Down Expand Up @@ -39,13 +39,22 @@ export const useUpdateSecretApprovalPolicy = () => {
const queryClient = useQueryClient();

return useMutation<{}, {}, TUpdateSecretPolicyDTO>({
mutationFn: async ({ id, approvers, approvals, secretPath, name, enforcementLevel }) => {
mutationFn: async ({
id,
approvers,
approvals,
secretPath,
name,
enforcementLevel,
disabled
}) => {
const { data } = await apiRequest.patch(`/api/v1/secret-approvals/${id}`, {
approvals,
approvers,
secretPath,
name,
enforcementLevel
enforcementLevel,
disabled
});
return data;
},
Expand All @@ -54,17 +63,3 @@ export const useUpdateSecretApprovalPolicy = () => {
}
});
};

export const useDeleteSecretApprovalPolicy = () => {
DanielHougaard marked this conversation as resolved.
Show resolved Hide resolved
const queryClient = useQueryClient();

return useMutation<{}, {}, TDeleteSecretPolicyDTO>({
mutationFn: async ({ id }) => {
const { data } = await apiRequest.delete(`/api/v1/secret-approvals/${id}`);
return data;
},
onSuccess: (_, { workspaceId }) => {
queryClient.invalidateQueries(secretApprovalKeys.getApprovalPolicies(workspaceId));
}
});
};
13 changes: 4 additions & 9 deletions frontend/src/hooks/api/secretApproval/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ export type TSecretApprovalPolicy = {
enforcementLevel: EnforcementLevel;
};

export enum ApproverType{
export enum ApproverType {
User = "user",
Group = "group"
}

export type Approver ={
export type Approver = {
id: string;
type: ApproverType;
}
};

export type TGetSecretApprovalPoliciesDTO = {
workspaceId: string;
Expand Down Expand Up @@ -51,12 +51,7 @@ export type TUpdateSecretPolicyDTO = {
secretPath?: string | null;
approvals?: number;
enforcementLevel?: EnforcementLevel;
// for invalidating list
workspaceId: string;
};

export type TDeleteSecretPolicyDTO = {
id: string;
disabled?: boolean;
// for invalidating list
workspaceId: string;
};
19 changes: 13 additions & 6 deletions frontend/src/views/SecretApprovalPage/SecretApprovalPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@ export const SecretApprovalPage = () => {
const { currentWorkspace } = useWorkspace();
const projectId = currentWorkspace?.id || "";
const projectSlug = currentWorkspace?.slug || "";
const { data: secretApprovalReqCount } = useGetSecretApprovalRequestCount({ workspaceId: projectId });
const { data: secretApprovalReqCount } = useGetSecretApprovalRequestCount({
workspaceId: projectId
});
const { data: accessApprovalRequestCount } = useGetAccessRequestsCount({ projectSlug });
const defaultTab = (accessApprovalRequestCount?.pendingCount || 0) > (secretApprovalReqCount?.open || 0)
? TabSection.ResourceApprovalRequests
: TabSection.SecretApprovalRequests;
const defaultTab =
(accessApprovalRequestCount?.pendingCount || 0) > (secretApprovalReqCount?.open || 0)
? TabSection.ResourceApprovalRequests
: TabSection.SecretApprovalRequests;

return (
<div className="container mx-auto h-full w-full max-w-7xl bg-bunker-800 px-6 text-white">
Expand Down Expand Up @@ -55,11 +58,15 @@ export const SecretApprovalPage = () => {
<TabList>
<Tab value={TabSection.SecretApprovalRequests}>
Secret Requests
{Boolean(secretApprovalReqCount?.open) && (<Badge className="ml-2">{secretApprovalReqCount?.open}</Badge>)}
{Boolean(secretApprovalReqCount?.open) && (
<Badge className="ml-2">{secretApprovalReqCount?.open}</Badge>
)}
</Tab>
<Tab value={TabSection.ResourceApprovalRequests}>
Access Requests
{Boolean(accessApprovalRequestCount?.pendingCount) && <Badge className="ml-2">{accessApprovalRequestCount?.pendingCount}</Badge>}
{Boolean(accessApprovalRequestCount?.pendingCount) && (
<Badge className="ml-2">{accessApprovalRequestCount?.pendingCount}</Badge>
)}
</Tab>
<Tab value={TabSection.Policies}>Policies</Tab>
</TabList>
Expand Down
Loading
Loading