feat(access-request): fix issue for deleted custom privileges reopening old closed access requests

This commit is contained in:
carlosmonastyrski
2025-05-30 14:17:52 -03:00
parent 4dc587576b
commit 858ec2095e
6 changed files with 97 additions and 10 deletions

View File

@@ -0,0 +1,61 @@
import { Knex } from "knex";
import { TableName } from "../schemas";
export async function up(knex: Knex): Promise<void> {
const hasPrivilegeDeletedAtColumn = await knex.schema.hasColumn(
TableName.AccessApprovalRequest,
"privilegeDeletedAt"
);
const hasStatusColumn = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "status");
if (!hasPrivilegeDeletedAtColumn) {
await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
t.timestamp("privilegeDeletedAt").nullable();
});
}
if (!hasStatusColumn) {
await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
t.string("status").defaultTo("pending").notNullable();
});
// Update existing rows based on business logic
// If privilegeId is not null, set status to "approved"
await knex(TableName.AccessApprovalRequest).whereNotNull("privilegeId").update({ status: "approved" });
// If privilegeId is null and there's a rejected reviewer, set to "rejected"
const rejectedRequestIds = await knex(TableName.AccessApprovalRequestReviewer)
.select("requestId")
.where("status", "rejected")
.distinct()
.pluck("requestId");
if (rejectedRequestIds.length > 0) {
await knex(TableName.AccessApprovalRequest)
.whereNull("privilegeId")
.whereIn("id", rejectedRequestIds)
.update({ status: "rejected" });
}
}
}
export async function down(knex: Knex): Promise<void> {
const hasPrivilegeDeletedAtColumn = await knex.schema.hasColumn(
TableName.AccessApprovalRequest,
"privilegeDeletedAt"
);
const hasStatusColumn = await knex.schema.hasColumn(TableName.AccessApprovalRequest, "status");
if (hasPrivilegeDeletedAtColumn) {
await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
t.dropColumn("privilegeDeletedAt");
});
}
if (hasStatusColumn) {
await knex.schema.alterTable(TableName.AccessApprovalRequest, (t) => {
t.dropColumn("status");
});
}
}

View File

@@ -18,7 +18,9 @@ export const AccessApprovalRequestsSchema = z.object({
createdAt: z.date(),
updatedAt: z.date(),
requestedByUserId: z.string().uuid(),
note: z.string().nullable().optional()
note: z.string().nullable().optional(),
privilegeDeletedAt: z.date().nullable().optional(),
status: z.string().default("pending")
});
export type TAccessApprovalRequests = z.infer<typeof AccessApprovalRequestsSchema>;

View File

@@ -145,7 +145,7 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => {
}
: null,
isApproved: !!doc.policyDeletedAt || !!doc.privilegeId
isApproved: !!doc.policyDeletedAt || !!doc.privilegeId || doc.status !== ApprovalStatus.PENDING
}),
childrenMapper: [
{
@@ -392,14 +392,20 @@ export const accessApprovalRequestDALFactory = (db: TDbClient) => {
]
});
// an approval is pending if there is no reviewer rejections and no privilege ID is set
// an approval is pending if there is no reviewer rejections, no privilege ID is set and the number of approvals is less than the number of approvals required
const pendingApprovals = formattedRequests.filter(
(req) => !req.privilegeId && !req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED)
(req) =>
!req.privilegeId &&
!req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) &&
req.status === ApprovalStatus.PENDING
);
// an approval is finalized if there are any rejections or a privilege ID is set
// an approval is finalized if there are any rejections, a privilege ID is set or the number of approvals is equal to the number of approvals required
const finalizedApprovals = formattedRequests.filter(
(req) => req.privilegeId || req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED)
(req) =>
req.privilegeId ||
req.reviewers.some((r) => r.status === ApprovalStatus.REJECTED) ||
req.status !== ApprovalStatus.PENDING
);
return { pendingCount: pendingApprovals.length, finalizedCount: finalizedApprovals.length };

View File

@@ -204,7 +204,7 @@ export const accessApprovalRequestServiceFactory = ({
const isRejected = reviewers.some((reviewer) => reviewer.status === ApprovalStatus.REJECTED);
if (!isRejected) {
if (!isRejected && duplicateRequest.status === ApprovalStatus.PENDING) {
throw new BadRequestError({ message: "You already have a pending access request with the same criteria" });
}
}
@@ -478,7 +478,11 @@ export const accessApprovalRequestServiceFactory = ({
);
privilegeIdToSet = privilege.id;
}
await accessApprovalRequestDAL.updateById(accessApprovalRequest.id, { privilegeId: privilegeIdToSet }, tx);
await accessApprovalRequestDAL.updateById(
accessApprovalRequest.id,
{ privilegeId: privilegeIdToSet, status: ApprovalStatus.APPROVED },
tx
);
}
}

View File

@@ -9,6 +9,7 @@ import { UnpackedPermissionSchema } from "@app/server/routes/sanitizedSchema/per
import { ActorType } from "@app/services/auth/auth-type";
import { TProjectMembershipDALFactory } from "@app/services/project-membership/project-membership-dal";
import { TAccessApprovalRequestDALFactory } from "../access-approval-request/access-approval-request-dal";
import { constructPermissionErrorMessage, validatePrivilegeChangeOperation } from "../permission/permission-fns";
import { TPermissionServiceFactory } from "../permission/permission-service";
import {
@@ -16,6 +17,7 @@ import {
ProjectPermissionSet,
ProjectPermissionSub
} from "../permission/project-permission";
import { ApprovalStatus } from "../secret-approval-request/secret-approval-request-types";
import { TProjectUserAdditionalPrivilegeDALFactory } from "./project-user-additional-privilege-dal";
import {
ProjectUserAdditionalPrivilegeTemporaryMode,
@@ -30,6 +32,7 @@ type TProjectUserAdditionalPrivilegeServiceFactoryDep = {
projectUserAdditionalPrivilegeDAL: TProjectUserAdditionalPrivilegeDALFactory;
projectMembershipDAL: Pick<TProjectMembershipDALFactory, "findById" | "findOne">;
permissionService: Pick<TPermissionServiceFactory, "getProjectPermission">;
accessApprovalRequestDAL: Pick<TAccessApprovalRequestDALFactory, "update">;
};
export type TProjectUserAdditionalPrivilegeServiceFactory = ReturnType<
@@ -44,7 +47,8 @@ const unpackPermissions = (permissions: unknown) =>
export const projectUserAdditionalPrivilegeServiceFactory = ({
projectUserAdditionalPrivilegeDAL,
projectMembershipDAL,
permissionService
permissionService,
accessApprovalRequestDAL
}: TProjectUserAdditionalPrivilegeServiceFactoryDep) => {
const create = async ({
slug,
@@ -279,6 +283,15 @@ export const projectUserAdditionalPrivilegeServiceFactory = ({
});
ForbiddenError.from(permission).throwUnlessCan(ProjectPermissionMemberActions.Edit, ProjectPermissionSub.Member);
await accessApprovalRequestDAL.update(
{
privilegeId: userPrivilege.id
},
{
privilegeDeletedAt: new Date(),
status: ApprovalStatus.REJECTED
}
);
const deletedPrivilege = await projectUserAdditionalPrivilegeDAL.deleteById(userPrivilege.id);
return {
...deletedPrivilege,

View File

@@ -794,7 +794,8 @@ export const registerRoutes = async (
const projectUserAdditionalPrivilegeService = projectUserAdditionalPrivilegeServiceFactory({
permissionService,
projectMembershipDAL,
projectUserAdditionalPrivilegeDAL
projectUserAdditionalPrivilegeDAL,
accessApprovalRequestDAL
});
const projectKeyService = projectKeyServiceFactory({
permissionService,