Address PR comments

This commit is contained in:
Carlos Monastyrski
2025-12-18 00:44:52 -03:00
parent d17e36aa9b
commit 7e2ef2efd2
10 changed files with 124 additions and 150 deletions

View File

@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import { Knex } from "knex";
import { TableName } from "../schemas";

View File

@@ -33,3 +33,7 @@ export const sanitizeString = (dto: { unsanitizedString: string; tokens: string[
});
return sanitizedWords.join("");
};
export const sanitizeSqlLikeString = (value: string): string => {
return String(value).replace(new RE2("[%_\\\\]", "g"), "\\$&");
};

View File

@@ -372,10 +372,11 @@ export const registerCertificateRouter = async (server: FastifyZodProvider) => {
fromDate: z.coerce.date().optional(),
toDate: z.coerce.date().optional(),
profileIds: z
.union([z.string().uuid(), z.array(z.string().uuid())])
.transform((val) => (Array.isArray(val) ? val : [val]))
.string()
.transform((val) => val.split(",").map((id) => id.trim()))
.pipe(z.array(z.string().uuid()))
.optional()
.describe("Filter by profile IDs"),
.describe("Comma-separated list of profile IDs"),
sortBy: z.string().trim().optional(),
sortOrder: z.enum(["asc", "desc"]).optional()
}),

View File

@@ -1,9 +1,9 @@
import { Knex } from "knex";
import RE2 from "re2";
import { TDbClient } from "@app/db";
import { TableName, TCertificateRequests, TCertificates } from "@app/db/schemas";
import { DatabaseError } from "@app/lib/errors";
import { sanitizeSqlLikeString } from "@app/lib/fn/string";
import { ormify, selectAllTableCols } from "@app/lib/knex";
import {
applyProcessedPermissionRulesToQuery,
@@ -15,10 +15,12 @@ type TCertificateRequestWithCertificate = TCertificateRequests & {
profileName: string | null;
};
type TCertificateRequestQueryResult = {
certificate: string | null;
type TCertificateRequestQueryResult = TCertificateRequests & {
certId: string | null;
certSerialNumber: string | null;
certStatus: string | null;
profileName: string | null;
} & Omit<TCertificateRequests, "certificate">;
};
export type TCertificateRequestDALFactory = ReturnType<typeof certificateRequestDALFactory>;
@@ -41,31 +43,27 @@ export const certificateRequestDALFactory = (db: TDbClient) => {
.where(`${TableName.CertificateRequests}.id`, id)
.select(selectAllTableCols(TableName.CertificateRequests))
.select(db.ref("slug").withSchema(TableName.PkiCertificateProfile).as("profileName"))
.select(db.raw(`row_to_json(${TableName.Certificate}.*) as certificate`))
.select(db.ref("id").withSchema(TableName.Certificate).as("certId"))
.select(db.ref("serialNumber").withSchema(TableName.Certificate).as("certSerialNumber"))
.select(db.ref("status").withSchema(TableName.Certificate).as("certStatus"))
.first()) as TCertificateRequestQueryResult | undefined;
if (!result) return null;
const { certificate: certificateJson, profileName, ...certificateRequestData } = result;
const { certId, certSerialNumber, certStatus, profileName, ...certificateRequestData } = result;
let parsedCertificate: TCertificates | null = null;
if (certificateJson && typeof certificateJson === "string") {
try {
const parsed = JSON.parse(certificateJson) as Record<string, unknown>;
if (parsed && typeof parsed === "object" && "id" in parsed) {
parsedCertificate = parsed as TCertificates;
}
} catch {
// Ignore parsing errors
}
} else if (certificateJson && typeof certificateJson === "object" && "id" in certificateJson) {
parsedCertificate = certificateJson as TCertificates;
}
const certificate: TCertificates | null = certId
? ({
id: certId,
serialNumber: certSerialNumber,
status: certStatus
} as TCertificates)
: null;
return {
...certificateRequestData,
profileName: profileName || null,
certificate: parsedCertificate
certificate
};
} catch (error) {
throw new DatabaseError({ error, name: "Find certificate request by ID with certificate" });
@@ -144,12 +142,12 @@ export const certificateRequestDALFactory = (db: TDbClient) => {
}
if (search) {
const sanitizedSearch = String(search).replace(new RE2("[%_\\\\]", "g"), "\\$&");
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
const sanitizedSearch = sanitizeSqlLikeString(search);
query = query.where((builder) => {
void builder
.whereILike(`${TableName.CertificateRequests}.commonName`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.status`, `%${sanitizedSearch}%`);
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`);
});
}
@@ -212,12 +210,12 @@ export const certificateRequestDALFactory = (db: TDbClient) => {
}
if (search) {
const sanitizedSearch = String(search).replace(new RE2("[%_\\\\]", "g"), "\\$&");
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
const sanitizedSearch = sanitizeSqlLikeString(search);
query = query.where((builder) => {
void builder
.whereILike(`${TableName.CertificateRequests}.commonName`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.status`, `%${sanitizedSearch}%`);
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`);
});
}
@@ -297,16 +295,18 @@ export const certificateRequestDALFactory = (db: TDbClient) => {
query = query
.select(selectAllTableCols(TableName.CertificateRequests))
.select(db.ref("slug").withSchema(TableName.PkiCertificateProfile).as("profileName"))
.select(db.raw(`row_to_json(${TableName.Certificate}.*) as certificate`))
.select(db.ref("id").withSchema(TableName.Certificate).as("certId"))
.select(db.ref("serialNumber").withSchema(TableName.Certificate).as("certSerialNumber"))
.select(db.ref("status").withSchema(TableName.Certificate).as("certStatus"))
.where(`${TableName.CertificateRequests}.projectId`, projectId);
if (search) {
const sanitizedSearch = String(search).replace(new RE2("[%_\\\\]", "g"), "\\$&");
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
const sanitizedSearch = sanitizeSqlLikeString(search);
query = query.where((builder) => {
void builder
.whereILike(`${TableName.CertificateRequests}.commonName`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`)
.orWhereILike(`${TableName.CertificateRequests}.status`, `%${sanitizedSearch}%`);
.orWhereILike(`${TableName.CertificateRequests}.altNames`, `%${sanitizedSearch}%`);
});
}
@@ -336,26 +336,20 @@ export const certificateRequestDALFactory = (db: TDbClient) => {
.limit(limit)) as TCertificateRequestQueryResult[];
return results.map((row): TCertificateRequestWithCertificate => {
const { certificate: certificateJson, profileName: rowProfileName, ...certificateRequestData } = row;
const { certId, certSerialNumber, certStatus, profileName: rowProfileName, ...certificateRequestData } = row;
let parsedCertificate: TCertificates | null = null;
if (certificateJson && typeof certificateJson === "string") {
try {
const parsed = JSON.parse(certificateJson) as Record<string, unknown>;
if (parsed && typeof parsed === "object" && "id" in parsed) {
parsedCertificate = parsed as TCertificates;
}
} catch {
// Ignore parsing errors
}
} else if (certificateJson && typeof certificateJson === "object" && "id" in certificateJson) {
parsedCertificate = certificateJson as TCertificates;
}
const certificate: TCertificates | null = certId
? ({
id: certId,
serialNumber: certSerialNumber,
status: certStatus
} as TCertificates)
: null;
return {
...certificateRequestData,
profileName: rowProfileName || null,
certificate: parsedCertificate
certificate
};
});
} catch (error) {

View File

@@ -1,23 +1,18 @@
import RE2 from "re2";
import { TDbClient } from "@app/db";
import { TableName, TCertificates } from "@app/db/schemas";
import { DatabaseError } from "@app/lib/errors";
import { sanitizeSqlLikeString } from "@app/lib/fn/string";
import { ormify, selectAllTableCols } from "@app/lib/knex";
import {
applyProcessedPermissionRulesToQuery,
type ProcessedPermissionRules
} from "@app/lib/knex/permission-filter-utils";
import { isUuidV4 } from "@app/lib/validator";
import { CertStatus } from "./certificate-types";
export type TCertificateDALFactory = ReturnType<typeof certificateDALFactory>;
const isValidUUID = (str: string): boolean => {
const uuidRegex = new RE2("^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$", "i");
return uuidRegex.test(str);
};
export const certificateDALFactory = (db: TDbClient) => {
const certificateOrm = ormify(db, TableName.Certificate);
@@ -81,25 +76,26 @@ export const certificateDALFactory = (db: TDbClient) => {
.where(`${TableName.Project}.id`, projectId);
if (friendlyName) {
const sanitizedValue = String(friendlyName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(friendlyName);
query = query.andWhere(`${TableName.Certificate}.friendlyName`, "like", `%${sanitizedValue}%`);
}
if (commonName) {
const sanitizedValue = String(commonName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(commonName);
query = query.andWhere(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`);
}
if (search) {
const sanitizedValue = String(search).replace(new RE2("[%_\\\\]", "g"), "\\$&");
query = query.andWhere(function () {
void this.where(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`)
const sanitizedValue = sanitizeSqlLikeString(search);
query = query.andWhere((qb) => {
void qb
.where(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.altNames`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.serialNumber`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.friendlyName`, "like", `%${sanitizedValue}%`);
if (isValidUUID(sanitizedValue)) {
void this.orWhere(`${TableName.Certificate}.id`, sanitizedValue);
if (isUuidV4(sanitizedValue)) {
void qb.orWhere(`${TableName.Certificate}.id`, sanitizedValue);
}
});
}
@@ -108,28 +104,24 @@ export const certificateDALFactory = (db: TDbClient) => {
const now = new Date();
const statuses = Array.isArray(status) ? status : [status];
query = query.andWhere(function () {
query = query.andWhere((qb) => {
statuses.forEach((statusValue, index) => {
const whereMethod = index === 0 ? "where" : "orWhere";
if (statusValue === "active") {
void this[whereMethod](function () {
void this.where(`${TableName.Certificate}.notAfter`, ">", now).andWhere(
`${TableName.Certificate}.status`,
"!=",
"revoked"
);
if (statusValue === CertStatus.ACTIVE) {
void qb[whereMethod]((innerQb) => {
void innerQb
.where(`${TableName.Certificate}.notAfter`, ">", now)
.andWhere(`${TableName.Certificate}.status`, "!=", CertStatus.REVOKED);
});
} else if (statusValue === "expired") {
void this[whereMethod](function () {
void this.where(`${TableName.Certificate}.notAfter`, "<=", now).andWhere(
`${TableName.Certificate}.status`,
"!=",
"revoked"
);
} else if (statusValue === CertStatus.EXPIRED) {
void qb[whereMethod]((innerQb) => {
void innerQb
.where(`${TableName.Certificate}.notAfter`, "<=", now)
.andWhere(`${TableName.Certificate}.status`, "!=", CertStatus.REVOKED);
});
} else {
void this[whereMethod](`${TableName.Certificate}.status`, statusValue);
void qb[whereMethod](`${TableName.Certificate}.status`, statusValue);
}
});
});
@@ -232,7 +224,7 @@ export const certificateDALFactory = (db: TDbClient) => {
Object.entries(filter).forEach(([key, value]) => {
if (value !== undefined && value !== null) {
if (key === "friendlyName" || key === "commonName") {
const sanitizedValue = String(value).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(String(value));
query = query.andWhere(`${TableName.Certificate}.${key}`, "like", `%${sanitizedValue}%`);
} else {
query = query.andWhere(`${TableName.Certificate}.${key}`, value);
@@ -285,12 +277,12 @@ export const certificateDALFactory = (db: TDbClient) => {
.whereNull(`${TableName.Certificate}.renewedByCertificateId`);
if (friendlyName) {
const sanitizedValue = String(friendlyName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(friendlyName);
query = query.andWhere(`${TableName.Certificate}.friendlyName`, "like", `%${sanitizedValue}%`);
}
if (commonName) {
const sanitizedValue = String(commonName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(commonName);
query = query.andWhere(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`);
}
@@ -377,25 +369,26 @@ export const certificateDALFactory = (db: TDbClient) => {
});
if (friendlyName) {
const sanitizedValue = String(friendlyName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(friendlyName);
query = query.andWhere(`${TableName.Certificate}.friendlyName`, "like", `%${sanitizedValue}%`);
}
if (commonName) {
const sanitizedValue = String(commonName).replace(new RE2("[%_\\\\]", "g"), "\\$&");
const sanitizedValue = sanitizeSqlLikeString(commonName);
query = query.andWhere(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`);
}
if (search) {
const sanitizedValue = String(search).replace(new RE2("[%_\\\\]", "g"), "\\$&");
query = query.andWhere(function () {
void this.where(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`)
const sanitizedValue = sanitizeSqlLikeString(search);
query = query.andWhere((qb) => {
void qb
.where(`${TableName.Certificate}.commonName`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.altNames`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.serialNumber`, "like", `%${sanitizedValue}%`)
.orWhere(`${TableName.Certificate}.friendlyName`, "like", `%${sanitizedValue}%`);
if (isValidUUID(sanitizedValue)) {
void this.orWhere(`${TableName.Certificate}.id`, sanitizedValue);
if (isUuidV4(sanitizedValue)) {
void qb.orWhere(`${TableName.Certificate}.id`, sanitizedValue);
}
});
}
@@ -404,28 +397,24 @@ export const certificateDALFactory = (db: TDbClient) => {
const now = new Date();
const statuses = Array.isArray(status) ? status : [status];
query = query.andWhere(function () {
query = query.andWhere((qb) => {
statuses.forEach((statusValue, index) => {
const whereMethod = index === 0 ? "where" : "orWhere";
if (statusValue === "active") {
void this[whereMethod](function () {
void this.where(`${TableName.Certificate}.notAfter`, ">", now).andWhere(
`${TableName.Certificate}.status`,
"!=",
"revoked"
);
if (statusValue === CertStatus.ACTIVE) {
void qb[whereMethod]((innerQb) => {
void innerQb
.where(`${TableName.Certificate}.notAfter`, ">", now)
.andWhere(`${TableName.Certificate}.status`, "!=", CertStatus.REVOKED);
});
} else if (statusValue === "expired") {
void this[whereMethod](function () {
void this.where(`${TableName.Certificate}.notAfter`, "<=", now).andWhere(
`${TableName.Certificate}.status`,
"!=",
"revoked"
);
} else if (statusValue === CertStatus.EXPIRED) {
void qb[whereMethod]((innerQb) => {
void innerQb
.where(`${TableName.Certificate}.notAfter`, "<=", now)
.andWhere(`${TableName.Certificate}.status`, "!=", CertStatus.REVOKED);
});
} else {
void this[whereMethod](`${TableName.Certificate}.status`, statusValue);
void qb[whereMethod](`${TableName.Certificate}.status`, statusValue);
}
});
});

View File

@@ -212,6 +212,9 @@ export const useUnifiedCertificateIssuance = () => {
queryClient.invalidateQueries({
queryKey: projectKeys.forProjectCertificates(projectSlug)
});
queryClient.invalidateQueries({
queryKey: ["certificateRequests", "list", projectSlug]
});
}
});
};

View File

@@ -79,37 +79,39 @@ export const useGetCertBundle = (serialNumber: string) => {
});
};
const DATE_RANGE_DAYS = 90;
export const useListCertificateRequests = (params: TListCertificateRequestsParams) => {
return useQuery({
queryKey: certKeys.listCertificateRequests(params),
queryFn: async () => {
const searchParams = new URLSearchParams();
searchParams.append("projectSlug", params.projectSlug);
if (params.offset !== undefined) searchParams.append("offset", params.offset.toString());
if (params.limit !== undefined) searchParams.append("limit", params.limit.toString());
if (params.search) searchParams.append("search", params.search);
if (params.status) searchParams.append("status", params.status);
if (params.fromDate) searchParams.append("fromDate", params.fromDate.toISOString());
if (params.toDate) searchParams.append("toDate", params.toDate.toISOString());
if (params.profileIds && params.profileIds.length > 0) {
params.profileIds.forEach((id) => {
searchParams.append("profileIds", id);
});
}
if (params.sortBy) searchParams.append("sortBy", params.sortBy);
if (params.sortOrder) searchParams.append("sortOrder", params.sortOrder);
const now = Date.now();
const daysInMs = DATE_RANGE_DAYS * 24 * 60 * 60 * 1000;
const fromDate = params.fromDate || new Date(now - daysInMs);
const toDate = params.toDate || new Date(now);
const { data } = await apiRequest.get<TListCertificateRequestsResponse>(
`/api/v1/cert-manager/certificates/certificate-requests?${searchParams.toString()}`
"/api/v1/cert-manager/certificates/certificate-requests",
{
params: {
projectSlug: params.projectSlug,
...(params.offset !== undefined && { offset: params.offset }),
...(params.limit !== undefined && { limit: params.limit }),
...(params.search && { search: params.search }),
...(params.status && { status: params.status }),
fromDate: fromDate.toISOString(),
toDate: toDate.toISOString(),
...(params.profileIds &&
params.profileIds.length > 0 && { profileIds: params.profileIds.join(",") }),
...(params.sortBy && { sortBy: params.sortBy }),
...(params.sortOrder && { sortOrder: params.sortOrder })
}
}
);
return data;
},
enabled: Boolean(params.projectSlug),
staleTime: 5 * 60 * 1000,
refetchOnWindowFocus: false,
refetchInterval: false
placeholderData: (previousData) => previousData
});
};

View File

@@ -89,7 +89,7 @@ export const CertificateRequestRow = ({ request, onViewCertificates }: Props) =>
disabled={!request.certificateId}
className="flex items-center gap-2"
>
View in Certificates
View Certificate
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>

View File

@@ -40,7 +40,6 @@ import { CertificateIssuanceModal } from "../../CertificatesPage/components/Cert
import { CertificateRequestRow } from "./CertificateRequestRow";
const PAGE_SIZE = 20;
const DATE_RANGE_DAYS = 90;
const SEARCH_DEBOUNCE_DELAY = 500;
type CertificateRequestStatus = "pending" | "issued" | "failed";
@@ -78,15 +77,6 @@ export const CertificateRequestsSection = ({ onViewCertificateFromRequest }: Pro
limit: 100
});
const { fromDate, toDate } = useMemo(() => {
const now = Date.now();
const daysInMs = DATE_RANGE_DAYS * 24 * 60 * 60 * 1000;
return {
fromDate: new Date(now - daysInMs),
toDate: new Date(now)
};
}, []);
const profileIds = useMemo(() => {
return appliedProfileIds.length > 0 ? appliedProfileIds : undefined;
}, [appliedProfileIds]);
@@ -100,19 +90,9 @@ export const CertificateRequestsSection = ({ onViewCertificateFromRequest }: Pro
sortOrder: "desc",
...(debouncedSearch && { search: debouncedSearch }),
...(appliedFilters.status && { status: appliedFilters.status }),
...(profileIds && { profileIds }),
fromDate,
toDate
...(profileIds && { profileIds })
}),
[
currentProject?.slug,
currentPage,
debouncedSearch,
appliedFilters.status,
profileIds,
fromDate,
toDate
]
[currentProject?.slug, currentPage, debouncedSearch, appliedFilters.status, profileIds]
);
const {
@@ -193,7 +173,7 @@ export const CertificateRequestsSection = ({ onViewCertificateFromRequest }: Pro
value={pendingSearch}
onChange={(e) => setPendingSearch(e.target.value)}
leftIcon={<FontAwesomeIcon icon={faMagnifyingGlass} />}
placeholder="Search by SAN, CN or Profile Name..."
placeholder="Search by SAN or CN"
className="flex-1"
/>
<DropdownMenu>

View File

@@ -231,7 +231,7 @@ export const CertificatesTable = ({ handlePopUpOpen, externalFilter }: Props) =>
value={pendingSearch}
onChange={(e) => setPendingSearch(e.target.value)}
leftIcon={<FontAwesomeIcon icon={faMagnifyingGlass} />}
placeholder="Search by SAN, CN, ID or Serial Number..."
placeholder="Search by SAN, CN, ID or Serial Number"
className="flex-1"
/>
<DropdownMenu>