From 332cc696a575119a0e5796e63bd2751e32152cd4 Mon Sep 17 00:00:00 2001 From: Roberto Gregnanin Date: Wed, 18 Sep 2024 11:31:47 +0200 Subject: [PATCH 1/2] added email sanitization --- .../tenant-process/src/model/domain/errors.ts | 9 +++ .../src/services/tenantService.ts | 30 +++++++- .../tenant-process/test/addTenantMail.test.ts | 70 +++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/tenant-process/src/model/domain/errors.ts b/packages/tenant-process/src/model/domain/errors.ts index 4fc4a3c12e..9867c7cf36 100644 --- a/packages/tenant-process/src/model/domain/errors.ts +++ b/packages/tenant-process/src/model/domain/errors.ts @@ -33,6 +33,7 @@ export const errorCodes = { certifierWithExistingAttributes: "0024", attributeNotFoundInTenant: "0025", tenantNotFoundByExternalId: "0026", + notValidMailAddress: "0027", }; export type ErrorCodes = keyof typeof errorCodes; @@ -292,3 +293,11 @@ export function attributeNotFoundInTenant( title: "Attribute not found in tenant", }); } + +export function notValidMailAddress(address: string): ApiError { + return new ApiError({ + detail: `mail address ${address} not valid`, + code: "notValidMailAddress", + title: "Not valid mail address", + }); +} diff --git a/packages/tenant-process/src/services/tenantService.ts b/packages/tenant-process/src/services/tenantService.ts index efe26dccb6..cb5c7729e9 100644 --- a/packages/tenant-process/src/services/tenantService.ts +++ b/packages/tenant-process/src/services/tenantService.ts @@ -64,6 +64,7 @@ import { tenantNotFound, tenantIsAlreadyACertifier, verifiedAttributeSelfRevocationNotAllowed, + notValidMailAddress, } from "../model/domain/errors.js"; import { assertOrganizationIsInAttributeVerifiers, @@ -1080,9 +1081,11 @@ export function tenantServiceBuilder( throw mailAlreadyExists(); } + const validatedAddress = validateAddress(mailSeed.address); + const newMail: TenantMail = { kind: mailSeed.kind, - address: mailSeed.address, + address: validatedAddress, description: mailSeed.description, id: crypto.createHash("sha256").update(mailSeed.address).digest("hex"), createdAt: new Date(), @@ -1734,4 +1737,29 @@ async function revokeCertifiedAttribute( } satisfies Tenant; } +function validateAddress(address: string): string { + // Here I am removing the non-printing control characters + const removeNonPrintingcontrolCharacters = address.replace( + // eslint-disable-next-line no-control-regex + /[\x00-\x1F\x7F]/g, + "" + ); + + // Here I am removing the extra spaces and tabs + const removeExtraSpace = removeNonPrintingcontrolCharacters + .replace(/\s+/g, "") + .trim(); + + // Here I am removing strange characters or special symbols + const sanitizedMail = removeExtraSpace + .replace(/[^\w.@-_]/g, "") + .replace(/\^/g, ""); + + const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailPattern.test(sanitizedMail)) { + throw notValidMailAddress(address); + } + return sanitizedMail; +} + export type TenantService = ReturnType; diff --git a/packages/tenant-process/test/addTenantMail.test.ts b/packages/tenant-process/test/addTenantMail.test.ts index 63b19af91a..027f502a92 100644 --- a/packages/tenant-process/test/addTenantMail.test.ts +++ b/packages/tenant-process/test/addTenantMail.test.ts @@ -15,6 +15,7 @@ import { tenantApi } from "pagopa-interop-api-clients"; import { getMockTenant } from "pagopa-interop-commons-test"; import { mailAlreadyExists, + notValidMailAddress, tenantNotFound, } from "../src/model/domain/errors.js"; import { addOneTenant, postgresDB, tenantService } from "./utils.js"; @@ -77,6 +78,53 @@ describe("addTenantMail", async () => { }; expect(writtenPayload.tenant).toEqual(toTenantV2(updatedTenant)); }); + it("Should correctly add email by cleaning the address from unwanted characters", async () => { + const mailSeedWithStrangeCharacters: tenantApi.MailSeed = { + kind: "CONTACT_EMAIL", + address: " test#°¶^ Mail@test.$%*it", + description: "mail description", + }; + await addOneTenant(mockTenant); + await tenantService.addTenantMail( + { + tenantId: mockTenant.id, + mailSeed: mailSeedWithStrangeCharacters, + organizationId: mockTenant.id, + correlationId: generateId(), + }, + genericLogger + ); + const writtenEvent = await readLastEventByStreamId( + mockTenant.id, + "tenant", + postgresDB + ); + + expect(writtenEvent).toMatchObject({ + stream_id: mockTenant.id, + version: "1", + type: "TenantMailAdded", + event_version: 2, + }); + + const writtenPayload: TenantMailAddedV2 | undefined = protobufDecoder( + TenantMailAddedV2 + ).parse(writtenEvent.data); + + const updatedTenant: Tenant = { + ...mockTenant, + mails: [ + { + ...mailSeed, + id: writtenPayload.mailId, + createdAt: new Date(), + }, + ], + updatedAt: new Date(), + }; + expect(writtenPayload.tenant).toEqual(toTenantV2(updatedTenant)); + }); + it("Should throw tenantNotFound if the tenant doesn't exists", async () => { expect( tenantService.addTenantMail( @@ -132,4 +180,26 @@ describe("addTenantMail", async () => { ) ).rejects.toThrowError(mailAlreadyExists()); }); + + it("Should throw notValidMailAddress if the address doesn't respect the valid pattern", async () => { + const mailSeedWithStrangeCharacters: tenantApi.MailSeed = { + kind: "CONTACT_EMAIL", + address: " test#°¶^ Mail@test.$%*@@it", + description: "mail description", + }; + await addOneTenant(mockTenant); + expect( + tenantService.addTenantMail( + { + tenantId: mockTenant.id, + mailSeed: mailSeedWithStrangeCharacters, + organizationId: mockTenant.id, + correlationId: generateId(), + }, + genericLogger + ) + ).rejects.toThrowError( + notValidMailAddress(mailSeedWithStrangeCharacters.address) + ); + }); }); From 0ae095d802f6c2d76c752cd8fb5a4250d32a967c Mon Sep 17 00:00:00 2001 From: Roberto Gregnanin Date: Wed, 18 Sep 2024 12:53:51 +0200 Subject: [PATCH 2/2] Added validation to selfcareUpsertTenant --- .../src/services/tenantService.ts | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/tenant-process/src/services/tenantService.ts b/packages/tenant-process/src/services/tenantService.ts index 42caaafce0..d3a0a0dc60 100644 --- a/packages/tenant-process/src/services/tenantService.ts +++ b/packages/tenant-process/src/services/tenantService.ts @@ -336,20 +336,6 @@ export function tenantServiceBuilder( logger.info( `Creating tenant with external id ${tenantSeed.externalId} via SelfCare request"` ); - const mails = tenantSeed.digitalAddress - ? [ - { - id: crypto - .createHash("sha256") - .update(tenantSeed.digitalAddress.address) - .digest("hex"), - kind: tenantMailKind.DigitalAddress, - address: tenantSeed.digitalAddress.address, - description: tenantSeed.digitalAddress.description, - createdAt: new Date(), - }, - ] - : []; const newTenant: Tenant = { id: generateId(), @@ -357,7 +343,7 @@ export function tenantServiceBuilder( attributes: [], externalId: tenantSeed.externalId, features: [], - mails, + mails: formatTenantMail(tenantSeed.digitalAddress), selfcareId: tenantSeed.selfcareId, onboardedAt: new Date(tenantSeed.onboardedAt), subUnitType: tenantSeed.subUnitType, @@ -1094,17 +1080,17 @@ export function tenantServiceBuilder( const tenant = await retrieveTenant(tenantId, readModelService); - if (tenant.data.mails.find((m) => m.address === mailSeed.address)) { + const validatedAddress = validateAddress(mailSeed.address); + + if (tenant.data.mails.find((m) => m.address === validatedAddress)) { throw mailAlreadyExists(); } - const validatedAddress = validateAddress(mailSeed.address); - const newMail: TenantMail = { kind: mailSeed.kind, address: validatedAddress, description: mailSeed.description, - id: crypto.createHash("sha256").update(mailSeed.address).digest("hex"), + id: crypto.createHash("sha256").update(validatedAddress).digest("hex"), createdAt: new Date(), }; @@ -1772,11 +1758,33 @@ function validateAddress(address: string): string { .replace(/[^\w.@-_]/g, "") .replace(/\^/g, ""); - const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + // same path used by the frontend + // Taken from HTML spec: https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address + const emailPattern = + // eslint-disable-next-line no-useless-escape + /^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/; if (!emailPattern.test(sanitizedMail)) { throw notValidMailAddress(address); } return sanitizedMail; } +function formatTenantMail( + digitalAddress: tenantApi.MailSeed | undefined +): TenantMail[] { + if (!digitalAddress) { + return []; + } + const validatedAddress = validateAddress(digitalAddress.address); + return [ + { + id: crypto.createHash("sha256").update(validatedAddress).digest("hex"), + kind: tenantMailKind.DigitalAddress, + address: validatedAddress, + description: digitalAddress.description, + createdAt: new Date(), + }, + ]; +} + export type TenantService = ReturnType;