Addressed PR comments

This commit is contained in:
Carlos Monastyrski
2025-10-14 03:18:35 -03:00
parent 073be13906
commit 42800fdfe5
82 changed files with 4223 additions and 1956 deletions
@@ -31,42 +31,72 @@ export const createCertificateProfileSchema = z
})
.refine(
(data) => {
if (data.enrollmentType === EnrollmentType.EST && !data.estConfig) {
return false;
if (data.enrollmentType === EnrollmentType.EST) {
if (!data.estConfig) {
return false;
}
if (data.apiConfig) {
return false;
}
}
if (data.enrollmentType === EnrollmentType.API && !data.apiConfig) {
return false;
if (data.enrollmentType === EnrollmentType.API) {
if (!data.apiConfig) {
return false;
}
if (data.estConfig) {
return false;
}
}
return true;
},
{
message: "Config must be provided based on enrollment type"
message:
"EST enrollment type requires EST configuration and cannot have API configuration. API enrollment type requires API configuration and cannot have EST configuration."
}
);
export const updateCertificateProfileSchema = z.object({
slug: z
.string()
.min(1)
.max(255)
.regex(new RE2("^[a-z0-9-]+$"), "Slug must contain only lowercase letters, numbers, and hyphens")
.optional(),
description: z.string().max(1000).optional(),
enrollmentType: z.nativeEnum(EnrollmentType).optional(),
estConfig: z
.object({
disableBootstrapCaValidation: z.boolean().default(false),
passphrase: z.string().min(1),
encryptedCaChain: z.string()
})
.optional(),
apiConfig: z
.object({
autoRenew: z.boolean().default(false),
autoRenewDays: z.number().min(1).max(365).optional()
})
.optional()
});
export const updateCertificateProfileSchema = z
.object({
slug: z
.string()
.min(1)
.max(255)
.regex(new RE2("^[a-z0-9-]+$"), "Slug must contain only lowercase letters, numbers, and hyphens")
.optional(),
description: z.string().max(1000).optional(),
enrollmentType: z.nativeEnum(EnrollmentType).optional(),
estConfig: z
.object({
disableBootstrapCaValidation: z.boolean().default(false),
passphrase: z.string().min(1),
encryptedCaChain: z.string()
})
.optional(),
apiConfig: z
.object({
autoRenew: z.boolean().default(false),
autoRenewDays: z.number().min(1).max(365).optional()
})
.optional()
})
.refine(
(data) => {
if (data.enrollmentType === EnrollmentType.EST) {
if (data.apiConfig) {
return false;
}
}
if (data.enrollmentType === EnrollmentType.API) {
if (data.estConfig) {
return false;
}
}
return true;
},
{
message: "Cannot have EST config with API enrollment type or API config with EST enrollment type."
}
);
export const getCertificateProfileByIdSchema = z.object({
id: z.string().uuid()
@@ -122,9 +122,7 @@ describe("CertificateProfileService", () => {
create: vi.fn().mockResolvedValue({ id: "api-config-123" }),
findById: vi.fn(),
updateById: vi.fn(),
deleteById: vi.fn(),
findProfilesForAutoRenewal: vi.fn(),
isConfigInUse: vi.fn(),
transaction: vi.fn(),
find: vi.fn(),
findOne: vi.fn(),
@@ -136,8 +134,6 @@ describe("CertificateProfileService", () => {
create: vi.fn().mockResolvedValue({ id: "est-config-123" }),
findById: vi.fn(),
updateById: vi.fn(),
deleteById: vi.fn(),
isConfigInUse: vi.fn(),
transaction: vi.fn(),
find: vi.fn(),
findOne: vi.fn(),
@@ -158,6 +154,12 @@ describe("CertificateProfileService", () => {
throwUnlessCan: vi.fn()
} as any);
// Mock the transaction method to execute the callback and return the result
(mockCertificateProfileDAL.transaction as any).mockImplementation(async (fn: any) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/return-await
return await fn();
});
service = certificateProfileServiceFactory({
certificateProfileDAL: mockCertificateProfileDAL,
certificateTemplateV2DAL: mockCertificateTemplateV2DAL,
@@ -188,7 +190,10 @@ describe("CertificateProfileService", () => {
(mockCertificateTemplateV2DAL.findById as any).mockResolvedValue(sampleTemplate);
(mockCertificateProfileDAL.findByNameAndProjectId as any).mockResolvedValue(null);
(mockCertificateProfileDAL.findBySlugAndProjectId as any).mockResolvedValue(null);
(mockCertificateProfileDAL.create as any).mockResolvedValue(sampleProfile);
(mockCertificateProfileDAL.create as any).mockResolvedValue({
...sampleProfile,
enrollmentType: EnrollmentType.API // Ensure enrollmentType is explicitly included
});
});
it("should create profile successfully", async () => {
@@ -201,16 +206,19 @@ describe("CertificateProfileService", () => {
expect(result).toEqual(sampleProfile);
expect(mockCertificateTemplateV2DAL.findById).toHaveBeenCalledWith("template-123");
expect(mockCertificateProfileDAL.findBySlugAndProjectId).toHaveBeenCalledWith("new-profile", "project-123");
expect(mockCertificateProfileDAL.create).toHaveBeenCalledWith({
slug: "new-profile",
description: "New test profile",
enrollmentType: EnrollmentType.API,
caId: "ca-123",
certificateTemplateId: "template-123",
apiConfigId: "api-config-123",
estConfigId: null,
projectId: "project-123"
});
expect(mockCertificateProfileDAL.create).toHaveBeenCalledWith(
{
slug: "new-profile",
description: "New test profile",
enrollmentType: EnrollmentType.API,
caId: "ca-123",
certificateTemplateId: "template-123",
apiConfigId: "api-config-123",
estConfigId: null,
projectId: "project-123"
},
undefined
);
});
it("should throw NotFoundError when certificate template not found", async () => {
@@ -318,7 +326,11 @@ describe("CertificateProfileService", () => {
beforeEach(() => {
(mockCertificateProfileDAL.findById as any).mockResolvedValue(sampleProfile);
(mockCertificateProfileDAL.updateById as any).mockResolvedValue({ ...sampleProfile, ...updateData });
(mockCertificateProfileDAL.updateById as any).mockResolvedValue({
...sampleProfile,
...updateData,
enrollmentType: EnrollmentType.API // Ensure enrollmentType is explicitly included
});
});
it("should update profile successfully", async () => {
@@ -330,7 +342,7 @@ describe("CertificateProfileService", () => {
expect(result.slug).toBe("updated-profile");
expect(mockCertificateProfileDAL.findById).toHaveBeenCalledWith("profile-123");
expect(mockCertificateProfileDAL.updateById).toHaveBeenCalledWith("profile-123", updateData);
expect(mockCertificateProfileDAL.updateById).toHaveBeenCalledWith("profile-123", updateData, undefined);
});
it("should throw NotFoundError when profile not found", async () => {
@@ -720,11 +732,14 @@ describe("CertificateProfileService", () => {
});
expect(result.enrollmentType).toBe(EnrollmentType.EST);
expect(mockEstEnrollmentConfigDAL.create).toHaveBeenCalledWith({
disableBootstrapCaValidation: estProfileData.estConfig.disableBootstrapCaValidation,
hashedPassphrase: "mocked-hash",
encryptedCaChain: Buffer.from(estProfileData.estConfig.encryptedCaChain, "base64")
});
expect(mockEstEnrollmentConfigDAL.create).toHaveBeenCalledWith(
{
disableBootstrapCaValidation: estProfileData.estConfig.disableBootstrapCaValidation,
hashedPassphrase: "mocked-hash",
encryptedCaChain: Buffer.from(estProfileData.estConfig.encryptedCaChain, "base64")
},
undefined
);
});
it("should handle profile slug uniqueness validation", async () => {
@@ -772,7 +787,8 @@ describe("CertificateProfileService", () => {
(mockCertificateProfileDAL.findBySlugAndProjectId as any).mockResolvedValue(null);
(mockCertificateProfileDAL.create as any).mockResolvedValue({
...sampleProfile,
apiConfigId: "api-config-123"
apiConfigId: "api-config-123",
enrollmentType: EnrollmentType.API
});
const result = await service.createProfile({
@@ -781,10 +797,13 @@ describe("CertificateProfileService", () => {
data: autoRenewData
});
expect(mockApiEnrollmentConfigDAL.create).toHaveBeenCalledWith({
autoRenew: true,
autoRenewDays: 7
});
expect(mockApiEnrollmentConfigDAL.create).toHaveBeenCalledWith(
{
autoRenew: true,
autoRenewDays: 7
},
undefined
);
expect(result).toBeDefined();
});
});
@@ -1076,7 +1095,8 @@ describe("CertificateProfileService", () => {
(mockCertificateProfileDAL.findBySlugAndProjectId as any).mockResolvedValue(null);
(mockCertificateProfileDAL.create as any).mockResolvedValue({
...sampleProfile,
slug: invalidSlugData.slug
slug: invalidSlugData.slug,
enrollmentType: EnrollmentType.API
});
const result = await service.createProfile({
@@ -49,70 +49,6 @@ const convertDalToService = (dalResult: Record<string, unknown>): TCertificatePr
} as TCertificateProfile;
};
const validateEnrollmentConfig = async (data: {
enrollmentType: EnrollmentType;
estConfig?: TEstConfigData | null;
apiConfig?: TApiConfigData | null;
}): Promise<void> => {
if (data.enrollmentType === EnrollmentType.EST) {
if (!data.estConfig) {
throw new ForbiddenRequestError({
message: "EST enrollment type requires EST configuration"
});
}
if (data.apiConfig) {
throw new ForbiddenRequestError({
message: "EST enrollment type cannot have API configuration"
});
}
} else if (data.enrollmentType === EnrollmentType.API) {
if (!data.apiConfig) {
throw new ForbiddenRequestError({
message: "API enrollment type requires API configuration"
});
}
if (data.estConfig) {
throw new ForbiddenRequestError({
message: "API enrollment type cannot have EST configuration"
});
}
}
};
const validateEnrollmentConfigForUpdate = async (data: {
enrollmentType: EnrollmentType;
estConfigId?: string | null;
apiConfigId?: string | null;
}): Promise<void> => {
if (data.enrollmentType === EnrollmentType.EST) {
if (!data.estConfigId) {
throw new ForbiddenRequestError({
message: "EST enrollment type requires EST configuration ID"
});
}
if (data.apiConfigId) {
throw new ForbiddenRequestError({
message: "EST enrollment type cannot have API configuration ID"
});
}
} else if (data.enrollmentType === EnrollmentType.API) {
if (!data.apiConfigId) {
throw new ForbiddenRequestError({
message: "API enrollment type requires API configuration ID"
});
}
if (data.estConfigId) {
throw new ForbiddenRequestError({
message: "API enrollment type cannot have EST configuration ID"
});
}
}
};
const hasEnrollmentConfigChanges = (data: TCertificateProfileUpdate): boolean => {
return !!(data.enrollmentType || data.estConfigId || data.apiConfigId);
};
export const certificateProfileServiceFactory = ({
certificateProfileDAL,
certificateTemplateV2DAL,
@@ -169,43 +105,61 @@ export const certificateProfileServiceFactory = ({
});
}
// Validate enrollment type configuration
await validateEnrollmentConfig({
enrollmentType: data.enrollmentType,
estConfig: data.estConfig,
apiConfig: data.apiConfig
});
// Create enrollment configs based on type
let estConfigId: string | null = null;
let apiConfigId: string | null = null;
if (data.enrollmentType === EnrollmentType.EST && data.estConfig) {
const appCfg = getConfig();
// Hash the passphrase
const hashedPassphrase = await crypto.hashing().createHash(data.estConfig.passphrase, appCfg.SALT_ROUNDS);
const estConfig = await estEnrollmentConfigDAL.create({
disableBootstrapCaValidation: data.estConfig.disableBootstrapCaValidation,
hashedPassphrase,
encryptedCaChain: Buffer.from(data.estConfig.encryptedCaChain, "base64")
// Validate enrollment configuration requirements
if (data.enrollmentType === EnrollmentType.EST && !data.estConfig) {
throw new ForbiddenRequestError({
message: "EST enrollment requires EST configuration"
});
estConfigId = estConfig.id;
} else if (data.enrollmentType === EnrollmentType.API && data.apiConfig) {
const apiConfig = await apiEnrollmentConfigDAL.create({
autoRenew: data.apiConfig.autoRenew,
autoRenewDays: data.apiConfig.autoRenewDays
}
if (data.enrollmentType === EnrollmentType.API && !data.apiConfig) {
throw new ForbiddenRequestError({
message: "API enrollment requires API configuration"
});
apiConfigId = apiConfig.id;
}
// Create the profile with the created config IDs
const { estConfig, apiConfig, ...profileData } = data;
const profile = await certificateProfileDAL.create({
...profileData,
projectId,
estConfigId,
apiConfigId
// Create enrollment configs and profile
const profile = await certificateProfileDAL.transaction(async (tx) => {
let estConfigId: string | null = null;
let apiConfigId: string | null = null;
if (data.enrollmentType === EnrollmentType.EST && data.estConfig) {
const appCfg = getConfig();
// Hash the passphrase
const hashedPassphrase = await crypto.hashing().createHash(data.estConfig.passphrase, appCfg.SALT_ROUNDS);
const estConfig = await estEnrollmentConfigDAL.create(
{
disableBootstrapCaValidation: data.estConfig.disableBootstrapCaValidation,
hashedPassphrase,
encryptedCaChain: Buffer.from(data.estConfig.encryptedCaChain, "base64")
},
tx
);
estConfigId = estConfig.id;
} else if (data.enrollmentType === EnrollmentType.API && data.apiConfig) {
const apiConfig = await apiEnrollmentConfigDAL.create(
{
autoRenew: data.apiConfig.autoRenew,
autoRenewDays: data.apiConfig.autoRenewDays
},
tx
);
apiConfigId = apiConfig.id;
}
// Create the profile with the created config IDs
const { estConfig, apiConfig, ...profileData } = data;
const profileResult = await certificateProfileDAL.create(
{
...profileData,
projectId,
estConfigId,
apiConfigId
},
tx
);
return profileResult;
});
return convertDalToService(profile);
@@ -268,37 +222,40 @@ export const certificateProfileServiceFactory = ({
}
}
if (hasEnrollmentConfigChanges(data)) {
const mergedData = { ...existingProfile, ...data };
await validateEnrollmentConfigForUpdate({
enrollmentType: mergedData.enrollmentType as EnrollmentType,
estConfigId: mergedData.estConfigId,
apiConfigId: mergedData.apiConfigId
});
}
const { estConfig, apiConfig, ...profileUpdateData } = data;
if (estConfig && existingProfile.estConfigId) {
await estEnrollmentConfigDAL.updateById(existingProfile.estConfigId, {
disableBootstrapCaValidation: estConfig.disableBootstrapCaValidation,
...(estConfig.passphrase && {
hashedPassphrase: await crypto.hashing().createHash(estConfig.passphrase, getConfig().SALT_ROUNDS)
}),
...(estConfig.caChain && {
encryptedCaChain: Buffer.from(estConfig.caChain, "base64")
})
});
}
const updatedProfile = await certificateProfileDAL.transaction(async (tx) => {
if (estConfig && existingProfile.estConfigId) {
await estEnrollmentConfigDAL.updateById(
existingProfile.estConfigId,
{
disableBootstrapCaValidation: estConfig.disableBootstrapCaValidation,
...(estConfig.passphrase && {
hashedPassphrase: await crypto.hashing().createHash(estConfig.passphrase, getConfig().SALT_ROUNDS)
}),
...(estConfig.caChain && {
encryptedCaChain: Buffer.from(estConfig.caChain, "base64")
})
},
tx
);
}
if (apiConfig && existingProfile.apiConfigId) {
await apiEnrollmentConfigDAL.updateById(existingProfile.apiConfigId, {
autoRenew: apiConfig.autoRenew,
autoRenewDays: apiConfig.autoRenewDays
});
}
if (apiConfig && existingProfile.apiConfigId) {
await apiEnrollmentConfigDAL.updateById(
existingProfile.apiConfigId,
{
autoRenew: apiConfig.autoRenew,
autoRenewDays: apiConfig.autoRenewDays
},
tx
);
}
const profileResult = await certificateProfileDAL.updateById(profileId, profileUpdateData, tx);
return profileResult;
});
const updatedProfile = await certificateProfileDAL.updateById(profileId, profileUpdateData);
return convertDalToService(updatedProfile);
};