From 32c9ef874d03d4cbdef71a83534c772ba7b12195 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 11:53:19 +0100 Subject: [PATCH 01/12] Imoproving Bruno collection and API spec for delegation --- collections/collection.bru | 1 + ...r delegations.bru => List delegations.bru} | 12 ++-- .../producer => }/Retrieves a delegation.bru | 2 +- .../health/Health status endpoint.bru | 0 .../producer/Approves a delegation.bru | 6 +- .../producer/Delegation Creation.bru | 10 ++- .../producer/Rejects a delegation.bru | 8 +-- .../producer/Revoke a delegation.bru | 6 +- collections/environments/PagoPA local.bru | 1 + .../tenant/Add DelegatedProducer feature.bru | 16 +++++ .../api-clients/open-api/delegationApi.yml | 68 ++++++++++--------- 11 files changed, 81 insertions(+), 49 deletions(-) rename collections/delegation/{Delegation Process Micro Service/producer/List producer delegations.bru => List delegations.bru} (68%) rename collections/delegation/{Delegation Process Micro Service/producer => }/Retrieves a delegation.bru (82%) rename collections/delegation/{Delegation Process Micro Service => }/health/Health status endpoint.bru (100%) rename collections/delegation/{Delegation Process Micro Service => }/producer/Approves a delegation.bru (76%) rename collections/delegation/{Delegation Process Micro Service => }/producer/Delegation Creation.bru (64%) rename collections/delegation/{Delegation Process Micro Service => }/producer/Rejects a delegation.bru (70%) rename collections/delegation/{Delegation Process Micro Service => }/producer/Revoke a delegation.bru (79%) create mode 100644 collections/tenant/Add DelegatedProducer feature.bru diff --git a/collections/collection.bru b/collections/collection.bru index 2597da5bd6..5c993b3ebd 100644 --- a/collections/collection.bru +++ b/collections/collection.bru @@ -1,5 +1,6 @@ vars:pre-request { tenantId: 69e2865e-65ab-4e48-a638-2037a9ee2ee7 + tenantId2: 0cf1db41-3085-43a6-9e4c-57e0fb81a916 userId1: f07ddb8f-17f9-47d4-b31e-35d1ac10e521 userId2: 2a1614d7-c1aa-4148-895f-dcadb75b6660 keyEncodedPem: LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlHZk1BMEdDU3FHU0liM0RRRUJBUVVBQTRHTkFEQ0JpUUtCZ1FETUVrdEZVVyszY2VWN01FT2RBVG5ZYWc3eQpJc3JtRDZ6eVBTZHhJTDlJczNmdnlGREMvbnVCWVFpa3Izampjc21aREdTN0RGKzNWRUJ3UXFYUldGM3NObElRCnFIc21Td2x2NjZ2ZDQ0OHEzSXpSb1JBWktGMGc3c3BGcUJ5bi9DTXZaM0RET2xVK2V0c2xDYWRNa084UktyM1YKd2xqQjFJdk90TWtCd2lLTU53SURBUUFCCi0tLS0tRU5EIFBVQkxJQyBLRVktLS0tLQo= diff --git a/collections/delegation/Delegation Process Micro Service/producer/List producer delegations.bru b/collections/delegation/List delegations.bru similarity index 68% rename from collections/delegation/Delegation Process Micro Service/producer/List producer delegations.bru rename to collections/delegation/List delegations.bru index 733ec7c274..69ee28f0e9 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/List producer delegations.bru +++ b/collections/delegation/List delegations.bru @@ -1,5 +1,5 @@ meta { - name: List producer delegations + name: List delegations type: http seq: 1 } @@ -15,12 +15,16 @@ params:query { limit: 50 ~kind: DELEGATED_PRODUCER ~delegationStates: WAITING_FOR_APPROVAL - ~delegatorIds: - ~delegateIds: - ~eserviceIds: + ~delegatorIds: + ~delegateIds: + ~eserviceIds: } headers { Authorization: {{JWT}} X-Correlation-Id: {{correlation-id}} } + +vars:post-response { + delegationId: res.body.results.at(-1).id +} diff --git a/collections/delegation/Delegation Process Micro Service/producer/Retrieves a delegation.bru b/collections/delegation/Retrieves a delegation.bru similarity index 82% rename from collections/delegation/Delegation Process Micro Service/producer/Retrieves a delegation.bru rename to collections/delegation/Retrieves a delegation.bru index 4e9425bd6d..398d20875c 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/Retrieves a delegation.bru +++ b/collections/delegation/Retrieves a delegation.bru @@ -11,7 +11,7 @@ get { } params:path { - delegationId: 123e4567-e89b-12d3-a456-426614174000 + delegationId: {{delegationId}} } headers { diff --git a/collections/delegation/Delegation Process Micro Service/health/Health status endpoint.bru b/collections/delegation/health/Health status endpoint.bru similarity index 100% rename from collections/delegation/Delegation Process Micro Service/health/Health status endpoint.bru rename to collections/delegation/health/Health status endpoint.bru diff --git a/collections/delegation/Delegation Process Micro Service/producer/Approves a delegation.bru b/collections/delegation/producer/Approves a delegation.bru similarity index 76% rename from collections/delegation/Delegation Process Micro Service/producer/Approves a delegation.bru rename to collections/delegation/producer/Approves a delegation.bru index 23bd4e2e56..3f94f781fd 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/Approves a delegation.bru +++ b/collections/delegation/producer/Approves a delegation.bru @@ -1,7 +1,7 @@ meta { name: Approves a delegation type: http - seq: 4 + seq: 2 } post { @@ -11,10 +11,10 @@ post { } params:path { - delegationId: + delegationId: {{delegationId}} } headers { - Authorization: {{JWT}} + Authorization: {{JWT2}} X-Correlation-Id: {{correlation-id}} } diff --git a/collections/delegation/Delegation Process Micro Service/producer/Delegation Creation.bru b/collections/delegation/producer/Delegation Creation.bru similarity index 64% rename from collections/delegation/Delegation Process Micro Service/producer/Delegation Creation.bru rename to collections/delegation/producer/Delegation Creation.bru index fa36a2e7e5..4e1656fadf 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/Delegation Creation.bru +++ b/collections/delegation/producer/Delegation Creation.bru @@ -1,7 +1,7 @@ meta { name: Delegation Creation type: http - seq: 3 + seq: 1 } post { @@ -17,7 +17,11 @@ headers { body:json { { - "eserviceId": "", - "delegateId": "" + "eserviceId": "{{eserviceId}}", + "delegateId": "{{tenantId2}}" } } + +vars:post-response { + delegationId: res.body.id +} diff --git a/collections/delegation/Delegation Process Micro Service/producer/Rejects a delegation.bru b/collections/delegation/producer/Rejects a delegation.bru similarity index 70% rename from collections/delegation/Delegation Process Micro Service/producer/Rejects a delegation.bru rename to collections/delegation/producer/Rejects a delegation.bru index b2b1477d62..b19d86bff6 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/Rejects a delegation.bru +++ b/collections/delegation/producer/Rejects a delegation.bru @@ -1,7 +1,7 @@ meta { name: Rejects a delegation type: http - seq: 5 + seq: 3 } post { @@ -11,16 +11,16 @@ post { } params:path { - delegationId: + delegationId: {{delegationId}} } headers { - Authorization: {{JWT}} + Authorization: {{JWT2}} X-Correlation-Id: {{correlation-id}} } body:json { { - "rejectionReason": "" + "rejectionReason": "testtets" } } diff --git a/collections/delegation/Delegation Process Micro Service/producer/Revoke a delegation.bru b/collections/delegation/producer/Revoke a delegation.bru similarity index 79% rename from collections/delegation/Delegation Process Micro Service/producer/Revoke a delegation.bru rename to collections/delegation/producer/Revoke a delegation.bru index 69741abb1d..392a524b0d 100644 --- a/collections/delegation/Delegation Process Micro Service/producer/Revoke a delegation.bru +++ b/collections/delegation/producer/Revoke a delegation.bru @@ -1,7 +1,7 @@ meta { name: Revoke a delegation type: http - seq: 6 + seq: 4 } delete { @@ -10,6 +10,10 @@ delete { auth: none } +params:path { + delegationId: {{delegationId}} +} + headers { Authorization: {{JWT}} X-Correlation-Id: {{correlation-id}} diff --git a/collections/environments/PagoPA local.bru b/collections/environments/PagoPA local.bru index f0051f88bb..30c756afeb 100644 --- a/collections/environments/PagoPA local.bru +++ b/collections/environments/PagoPA local.bru @@ -12,4 +12,5 @@ vars { host-delegation: http://localhost:3800 host-api-gw: http://localhost:3700/api-gateway/0.0 JWT-M2M: Bearer {{process.env.JWT-M2M}} + JWT2: Bearer {{process.env.JWT2}} } diff --git a/collections/tenant/Add DelegatedProducer feature.bru b/collections/tenant/Add DelegatedProducer feature.bru new file mode 100644 index 0000000000..00659420c8 --- /dev/null +++ b/collections/tenant/Add DelegatedProducer feature.bru @@ -0,0 +1,16 @@ +meta { + name: Add DelegatedProducer feature + type: http + seq: 26 +} + +post { + url: {{host-tenant}}/tenants/delegatedProducer + body: none + auth: none +} + +headers { + Authorization: {{JWT}} + X-Correlation-Id: {{correlation-id}} +} diff --git a/packages/api-clients/open-api/delegationApi.yml b/packages/api-clients/open-api/delegationApi.yml index c87eeff3b2..acf7810c2b 100644 --- a/packages/api-clients/open-api/delegationApi.yml +++ b/packages/api-clients/open-api/delegationApi.yml @@ -118,19 +118,20 @@ paths: schema: $ref: "#/components/schemas/Problem" /delegations/{delegationId}: + parameters: + - name: delegationId + in: path + description: The delegation id + required: true + schema: + type: string + format: uuid get: description: Retrieves a delegation summary: Retrieves a delegation tags: - delegation operationId: getDelegation - parameters: - - name: delegationId - in: path - description: The delegation id - required: true - schema: - type: string responses: "200": description: Delegation retrieved @@ -158,8 +159,8 @@ paths: $ref: "#/components/schemas/Problem" /producer/delegations: post: - description: creates the delegation - summary: Delegation Creation + description: Creates a producer delegation + summary: Producer delegation creation tags: - producer operationId: createProducerDelegation @@ -168,7 +169,7 @@ paths: application/json: schema: $ref: "#/components/schemas/DelegationSeed" - description: payload for delegation creation + description: Payload for delegation creation required: true responses: "200": @@ -196,20 +197,20 @@ paths: schema: $ref: "#/components/schemas/Problem" /producer/delegations/{delegationId}/approve: + parameters: + - name: delegationId + in: path + description: The delegation id + required: true + schema: + type: string + format: uuid post: - description: Approves a delegation - summary: Approves a delegation + description: Approves a producer delegation + summary: Producer delegation approval tags: - producer operationId: approveProducerDelegation - parameters: - - name: delegationId - in: path - description: The delegation id - required: true - schema: - type: string - format: uuid responses: "204": description: Delegation approved @@ -238,9 +239,17 @@ paths: schema: $ref: "#/components/schemas/Problem" /producer/delegations/{delegationId}/reject: + parameters: + - name: delegationId + in: path + description: The delegation id + required: true + schema: + type: string + format: uuid post: - description: Rejects a delegation - summary: Rejects a delegation + description: Rejects a producer delegation + summary: Producer delegation rejection tags: - producer operationId: rejectProducerDelegation @@ -250,15 +259,7 @@ paths: schema: $ref: "#/components/schemas/RejectDelegationPayload" required: true - description: payload for delegation rejection - parameters: - - name: delegationId - in: path - description: The delegation id - required: true - schema: - type: string - format: uuid + description: Payload for delegation rejection responses: "204": description: Delegation rejected @@ -294,9 +295,10 @@ paths: required: true schema: type: string + format: uuid delete: - description: Revokes a delegation - summary: Revokes a delegation + description: Revokes a producer delegation + summary: Producer delegation revocation tags: - producer operationId: revokeProducerDelegation From 4d420a29879d3829cea9f213f5fb4dd04c4f45d5 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:10:28 +0100 Subject: [PATCH 02/12] Improving errors --- .../src/model/domain/errors.ts | 6 ++- .../src/services/delegationProducerService.ts | 13 ++++-- .../src/services/validators.ts | 43 ++++++++----------- .../test/createProducerDelegation.test.ts | 7 ++- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/packages/delegation-process/src/model/domain/errors.ts b/packages/delegation-process/src/model/domain/errors.ts index b5b0064bb6..44ae0a8a41 100644 --- a/packages/delegation-process/src/model/domain/errors.ts +++ b/packages/delegation-process/src/model/domain/errors.ts @@ -5,6 +5,7 @@ import { makeApiProblemBuilder, TenantId, DelegationState, + DelegationKind, } from "pagopa-interop-models"; export const errorCodes = { @@ -81,10 +82,11 @@ export function invalidExternalOriginError( } export function tenantNotAllowedToDelegation( - tenantId: string + tenantId: string, + kind: DelegationKind ): ApiError { return new ApiError({ - detail: `Tenant ${tenantId} not allowed to delegation`, + detail: `Tenant ${tenantId} not allowed to receive delegations of kind: ${kind}`, code: "tenantNotAllowedToDelegation", title: "Tenant not allowed to delegation", }); diff --git a/packages/delegation-process/src/services/delegationProducerService.ts b/packages/delegation-process/src/services/delegationProducerService.ts index f323a8de45..611ae9f45b 100644 --- a/packages/delegation-process/src/services/delegationProducerService.ts +++ b/packages/delegation-process/src/services/delegationProducerService.ts @@ -34,8 +34,8 @@ import { assertDelegationNotExists, assertDelegatorIsIPA, assertDelegatorIsNotDelegate, - assertEserviceExists, - assertTenantAllowedToReceiveProducerDelegation, + assertDelegatorIsProducer, + assertTenantAllowedToReceiveDelegation, assertIsDelegate, assertIsState, } from "./validators.js"; @@ -84,9 +84,14 @@ export function delegationProducerServiceBuilder( const delegator = await retrieveTenantById(delegatorId); const delegate = await retrieveTenantById(delegateId); - assertTenantAllowedToReceiveProducerDelegation(delegate); + assertTenantAllowedToReceiveDelegation( + delegate, + delegationKind.delegatedProducer + ); await assertDelegatorIsIPA(delegator); - await assertEserviceExists(delegatorId, eserviceId, readModelService); + + const eservice = await retrieveEserviceById(eserviceId); + assertDelegatorIsProducer(delegatorId, eservice); await assertDelegationNotExists( delegator, eserviceId, diff --git a/packages/delegation-process/src/services/validators.ts b/packages/delegation-process/src/services/validators.ts index c5a6437d4a..fb376ec7a2 100644 --- a/packages/delegation-process/src/services/validators.ts +++ b/packages/delegation-process/src/services/validators.ts @@ -1,25 +1,26 @@ import { Delegation, + delegationKind, DelegationKind, DelegationState, delegationState, + EService, EServiceId, PUBLIC_ADMINISTRATIONS_IDENTIFIER, Tenant, TenantId, } from "pagopa-interop-models"; +import { match } from "ts-pattern"; import { delegationAlreadyExists, delegationNotRevokable, delegatorAndDelegateSameIdError, delegatorNotAllowToRevoke, differentEServiceProducer, - eserviceNotFound, incorrectState, invalidExternalOriginError, operationRestrictedToDelegate, tenantNotAllowedToDelegation, - tenantNotFound, } from "../model/domain/errors.js"; import { ReadModelService } from "./readModelService.js"; @@ -34,17 +35,11 @@ export const activeDelegationStates: DelegationState[] = [ delegationState.active, ]; -export const assertEserviceExists = async ( +export const assertDelegatorIsProducer = ( delegatorId: TenantId, - eserviceId: EServiceId, - readModelService: ReadModelService -): Promise => { - const eservice = await readModelService.getEServiceById(eserviceId); - if (!eservice) { - throw eserviceNotFound(eserviceId); - } - - if (eservice.data.producerId !== delegatorId) { + eservice: EService +): void => { + if (eservice.producerId !== delegatorId) { throw differentEServiceProducer(delegatorId); } }; @@ -66,25 +61,21 @@ export const assertDelegatorIsIPA = async ( } }; -export const assertTenantAllowedToReceiveProducerDelegation = ( - tenant: Tenant +export const assertTenantAllowedToReceiveDelegation = ( + tenant: Tenant, + kind: DelegationKind ): void => { const delegationFeature = tenant.features.find( - (f) => f.type === "DelegatedProducer" + (f) => + f.type === + match(kind) + .with(delegationKind.delegatedProducer, () => "DelegatedProducer") + .with(delegationKind.delegatedConsumer, () => "DelegatedConsumer") + .exhaustive() ); if (!delegationFeature) { - throw tenantNotAllowedToDelegation(tenant.id); - } -}; - -export const assertTenantExists = async ( - tenantId: TenantId, - readModelService: ReadModelService -): Promise => { - const tenant = await readModelService.getTenantById(tenantId); - if (!tenant) { - throw tenantNotFound(tenantId); + throw tenantNotAllowedToDelegation(tenant.id, kind); } }; diff --git a/packages/delegation-process/test/createProducerDelegation.test.ts b/packages/delegation-process/test/createProducerDelegation.test.ts index 97b0cec3c2..e3d64ae75d 100644 --- a/packages/delegation-process/test/createProducerDelegation.test.ts +++ b/packages/delegation-process/test/createProducerDelegation.test.ts @@ -559,6 +559,11 @@ describe("create delegation", () => { serviceName: "DelegationServiceTest", } ) - ).rejects.toThrowError(tenantNotAllowedToDelegation(delegate.id)); + ).rejects.toThrowError( + tenantNotAllowedToDelegation( + delegate.id, + delegationKind.delegatedProducer + ) + ); }); }); From 42f62c76380d62205dc94a4948f2bb9a85e46878 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:13:38 +0100 Subject: [PATCH 03/12] Adding correlationId param where missing in API spec --- packages/api-clients/open-api/delegationApi.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/api-clients/open-api/delegationApi.yml b/packages/api-clients/open-api/delegationApi.yml index acf7810c2b..5e92cade91 100644 --- a/packages/api-clients/open-api/delegationApi.yml +++ b/packages/api-clients/open-api/delegationApi.yml @@ -33,6 +33,8 @@ tags: url: http://swagger.io paths: /delegations: + parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" get: description: List delegations summary: List delegations @@ -119,6 +121,7 @@ paths: $ref: "#/components/schemas/Problem" /delegations/{delegationId}: parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" - name: delegationId in: path description: The delegation id @@ -158,6 +161,8 @@ paths: schema: $ref: "#/components/schemas/Problem" /producer/delegations: + parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" post: description: Creates a producer delegation summary: Producer delegation creation @@ -198,6 +203,7 @@ paths: $ref: "#/components/schemas/Problem" /producer/delegations/{delegationId}/approve: parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" - name: delegationId in: path description: The delegation id @@ -240,6 +246,7 @@ paths: $ref: "#/components/schemas/Problem" /producer/delegations/{delegationId}/reject: parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" - name: delegationId in: path description: The delegation id @@ -289,6 +296,7 @@ paths: $ref: "#/components/schemas/Problem" /producer/delegations/{delegationId}: parameters: + - $ref: "#/components/parameters/CorrelationIdHeader" - name: delegationId in: path description: The delegation id From f6a31abf43c5e2ce55a1fb4fcc107e2307edcdcc Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:14:45 +0100 Subject: [PATCH 04/12] Removing whitespaces --- collections/delegation/List delegations.bru | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/collections/delegation/List delegations.bru b/collections/delegation/List delegations.bru index 69ee28f0e9..a7503108d1 100644 --- a/collections/delegation/List delegations.bru +++ b/collections/delegation/List delegations.bru @@ -15,9 +15,9 @@ params:query { limit: 50 ~kind: DELEGATED_PRODUCER ~delegationStates: WAITING_FOR_APPROVAL - ~delegatorIds: - ~delegateIds: - ~eserviceIds: + ~delegatorIds: + ~delegateIds: + ~eserviceIds: } headers { From 7f928be00ca4732e1564c7c4b1ce0e7bb764f4c6 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:15:43 +0100 Subject: [PATCH 05/12] Improving string in bruno call --- collections/delegation/producer/Rejects a delegation.bru | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collections/delegation/producer/Rejects a delegation.bru b/collections/delegation/producer/Rejects a delegation.bru index b19d86bff6..57f0709361 100644 --- a/collections/delegation/producer/Rejects a delegation.bru +++ b/collections/delegation/producer/Rejects a delegation.bru @@ -21,6 +21,6 @@ headers { body:json { { - "rejectionReason": "testtets" + "rejectionReason": "test rejection reason" } } From 5d6409464e8ff3e39ebd48d5af342f875e3ec1c2 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:16:45 +0100 Subject: [PATCH 06/12] Removing duplicated call --- .../tenant/Add DelegatedProducer feature.bru | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 collections/tenant/Add DelegatedProducer feature.bru diff --git a/collections/tenant/Add DelegatedProducer feature.bru b/collections/tenant/Add DelegatedProducer feature.bru deleted file mode 100644 index 00659420c8..0000000000 --- a/collections/tenant/Add DelegatedProducer feature.bru +++ /dev/null @@ -1,16 +0,0 @@ -meta { - name: Add DelegatedProducer feature - type: http - seq: 26 -} - -post { - url: {{host-tenant}}/tenants/delegatedProducer - body: none - auth: none -} - -headers { - Authorization: {{JWT}} - X-Correlation-Id: {{correlation-id}} -} From 1bdff3d861347f0d9d04454fce486835975c7d2d Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:18:47 +0100 Subject: [PATCH 07/12] Adding missing component in delegationApi --- packages/api-clients/open-api/delegationApi.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/api-clients/open-api/delegationApi.yml b/packages/api-clients/open-api/delegationApi.yml index 5e92cade91..d912ab74c0 100644 --- a/packages/api-clients/open-api/delegationApi.yml +++ b/packages/api-clients/open-api/delegationApi.yml @@ -347,6 +347,13 @@ paths: schema: $ref: "#/components/schemas/Problem" components: + parameters: + CorrelationIdHeader: + in: header + name: X-Correlation-Id + required: true + schema: + type: string schemas: Delegations: type: object From 3dd8a0ba5f19f26cfe088ea65b3ac37baea1fd7e Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:25:50 +0100 Subject: [PATCH 08/12] Improving delegation tests --- packages/commons-test/src/testUtils.ts | 10 +- .../src/services/validators.ts | 2 +- .../test/approveProducerDelegation.test.ts | 17 +- .../test/createProducerDelegation.test.ts | 201 +++++++++--------- .../test/getDelegationById.test.ts | 16 +- .../test/getDelegations.test.ts | 12 +- .../test/rejectProducerDelegation.test.ts | 14 +- .../test/revokeProducerDelegation.test.ts | 20 +- 8 files changed, 166 insertions(+), 126 deletions(-) diff --git a/packages/commons-test/src/testUtils.ts b/packages/commons-test/src/testUtils.ts index dbb0143273..9191803405 100644 --- a/packages/commons-test/src/testUtils.ts +++ b/packages/commons-test/src/testUtils.ts @@ -49,7 +49,6 @@ import { PurposeVersionId, ProducerKeychain, Delegation, - delegationKind, DelegationId, DelegationContractDocument, DelegationContractId, @@ -67,6 +66,7 @@ import { PlatformStatesClientPK, PlatformStatesClientEntry, makePlatformStatesClientPK, + DelegationKind, } from "pagopa-interop-models"; import { AuthData } from "pagopa-interop-commons"; import { z } from "zod"; @@ -366,7 +366,8 @@ export const getMockAuthData = (organizationId?: TenantId): AuthData => ({ selfcareId: generateId(), }); -export const getMockDelegationProducer = ({ +export const getMockDelegation = ({ + kind, id = generateId(), delegatorId = generateId(), delegateId = generateId(), @@ -375,6 +376,7 @@ export const getMockDelegationProducer = ({ activationContract = undefined, revocationContract = undefined, }: { + kind: DelegationKind; id?: DelegationId; delegatorId?: TenantId; delegateId?: TenantId; @@ -382,7 +384,7 @@ export const getMockDelegationProducer = ({ state?: DelegationState; activationContract?: DelegationContractDocument; revocationContract?: DelegationContractDocument; -} = {}): Delegation => { +}): Delegation => { const creationTime = new Date(); return { @@ -395,7 +397,7 @@ export const getMockDelegationProducer = ({ state, activationContract, revocationContract, - kind: delegationKind.delegatedProducer, + kind, stamps: { submission: { who: delegatorId, diff --git a/packages/delegation-process/src/services/validators.ts b/packages/delegation-process/src/services/validators.ts index fb376ec7a2..4b3acc17f2 100644 --- a/packages/delegation-process/src/services/validators.ts +++ b/packages/delegation-process/src/services/validators.ts @@ -25,7 +25,7 @@ import { import { ReadModelService } from "./readModelService.js"; /* ========= STATES ========= */ -export const delegationNotActivableStates: DelegationState[] = [ +export const inactiveDelegationStates: DelegationState[] = [ delegationState.rejected, delegationState.revoked, ]; diff --git a/packages/delegation-process/test/approveProducerDelegation.test.ts b/packages/delegation-process/test/approveProducerDelegation.test.ts index dc1841a5ca..69199af69a 100644 --- a/packages/delegation-process/test/approveProducerDelegation.test.ts +++ b/packages/delegation-process/test/approveProducerDelegation.test.ts @@ -1,7 +1,7 @@ /* eslint-disable functional/no-let */ import { decodeProtobufPayload, - getMockDelegationProducer, + getMockDelegation, getMockTenant, getMockEService, getMockAuthData, @@ -16,6 +16,7 @@ import { Tenant, toDelegationV2, unsafeBrandId, + delegationKind, } from "pagopa-interop-models"; import { delegationState } from "pagopa-interop-models"; import { @@ -40,7 +41,7 @@ import { flushPDFMetadata, } from "./utils.js"; -describe("approve delegation", () => { +describe("approve producer delegation", () => { const currentExecutionTime = new Date(); beforeAll(async () => { vi.useFakeTimers(); @@ -63,7 +64,8 @@ describe("approve delegation", () => { it("should approve delegation if validations succeed", async () => { const delegationId = generateId(); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, id: delegationId, state: "WaitingForApproval", delegateId: delegate.id, @@ -173,7 +175,8 @@ describe("approve delegation", () => { it("should throw operationRestrictedToDelegate when approver is not the delegate", async () => { const wrongDelegate = getMockTenant(); await addOneTenant(wrongDelegate); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "WaitingForApproval", delegateId: delegate.id, delegatorId: delegator.id, @@ -194,7 +197,8 @@ describe("approve delegation", () => { }); it("should throw incorrectState when delegation is not in WaitingForApproval state", async () => { - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "Active", delegateId: delegate.id, delegatorId: delegator.id, @@ -219,7 +223,8 @@ describe("approve delegation", () => { }); it("should generete a pdf document for a delegation", async () => { - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "WaitingForApproval", delegateId: delegate.id, delegatorId: delegator.id, diff --git a/packages/delegation-process/test/createProducerDelegation.test.ts b/packages/delegation-process/test/createProducerDelegation.test.ts index e3d64ae75d..0a2bde3089 100644 --- a/packages/delegation-process/test/createProducerDelegation.test.ts +++ b/packages/delegation-process/test/createProducerDelegation.test.ts @@ -2,7 +2,7 @@ import { fail } from "assert"; import { genericLogger } from "pagopa-interop-commons"; import { decodeProtobufPayload, - getMockDelegationProducer, + getMockDelegation, getMockEService, getMockTenant, getRandomAuthData, @@ -32,7 +32,7 @@ import { import { activeDelegationStates, - delegationNotActivableStates, + inactiveDelegationStates, } from "../src/services/validators.js"; import { addOneDelegation, @@ -81,8 +81,8 @@ const expectedDelegationCreation = async ( ); }; -describe("create delegation", () => { - it("should create a delegation if not exists", async () => { +describe("create producer delegation", () => { + it("should create a delegation if it does not exist", async () => { const currentExecutionTime = new Date(); vi.useFakeTimers(); vi.setSystemTime(currentExecutionTime); @@ -93,7 +93,7 @@ describe("create delegation", () => { ...getMockTenant(delegatorId), externalId: { origin: "IPA", - value: "anythings", + value: "test", }, }; @@ -147,83 +147,87 @@ describe("create delegation", () => { vi.useRealTimers(); }); - it("should create a delegation if already exists the same delegation in status Rejected or Revoked", async () => { - const currentExecutionTime = new Date(); - vi.useFakeTimers(); - vi.setSystemTime(currentExecutionTime); - - const delegatorId = generateId(); - const authData = getRandomAuthData(delegatorId); - const delegator = { - ...getMockTenant(delegatorId), - externalId: { - origin: "IPA", - value: "anythings", - }, - }; + it.each(inactiveDelegationStates)( + "should create a delegation the same delegation exists and is in state %s", + async (inactiveDelegationState) => { + const currentExecutionTime = new Date(); + vi.useFakeTimers(); + vi.setSystemTime(currentExecutionTime); - const delegate = { - ...getMockTenant(), - features: [ - { - type: "DelegatedProducer" as const, - availabilityTimestamp: currentExecutionTime, + const delegatorId = generateId(); + const authData = getRandomAuthData(delegatorId); + const delegator = { + ...getMockTenant(delegatorId), + externalId: { + origin: "IPA", + value: "test", }, - ], - }; - const eservice = getMockEService(generateId(), delegatorId); - - const existentDelegation = { - ...getMockDelegationProducer({ - id: generateId(), - delegatorId, - delegateId: delegate.id, - eserviceId: eservice.id, - }), - state: randomArrayItem(delegationNotActivableStates), - }; + }; - await addOneTenant(delegator); - await addOneTenant(delegate); - await addOneEservice(eservice); - await addOneDelegation(existentDelegation); + const delegate = { + ...getMockTenant(), + features: [ + { + type: "DelegatedProducer" as const, + availabilityTimestamp: currentExecutionTime, + }, + ], + }; + const eservice = getMockEService(generateId(), delegatorId); - const actualDelegation = - await delegationProducerService.createProducerDelegation( - { + const existentDelegation = { + ...getMockDelegation({ + kind: delegationKind.delegatedProducer, + id: generateId(), + delegatorId, delegateId: delegate.id, eserviceId: eservice.id, - }, - { - authData, - logger: genericLogger, - correlationId: generateId(), - serviceName: "DelegationServiceTest", - } - ); + }), + state: inactiveDelegationState, + }; - const expectedDelegation: Delegation = { - id: actualDelegation.id, - delegatorId, - delegateId: delegate.id, - eserviceId: eservice.id, - kind: delegationKind.delegatedProducer, - state: delegationState.waitingForApproval, - createdAt: currentExecutionTime, - submittedAt: currentExecutionTime, - stamps: { - submission: { - who: delegatorId, - when: currentExecutionTime, + await addOneTenant(delegator); + await addOneTenant(delegate); + await addOneEservice(eservice); + await addOneDelegation(existentDelegation); + + const actualDelegation = + await delegationProducerService.createProducerDelegation( + { + delegateId: delegate.id, + eserviceId: eservice.id, + }, + { + authData, + logger: genericLogger, + correlationId: generateId(), + serviceName: "DelegationServiceTest", + } + ); + + const expectedDelegation: Delegation = { + id: actualDelegation.id, + delegatorId, + delegateId: delegate.id, + eserviceId: eservice.id, + kind: delegationKind.delegatedProducer, + state: delegationState.waitingForApproval, + createdAt: currentExecutionTime, + submittedAt: currentExecutionTime, + stamps: { + submission: { + who: delegatorId, + when: currentExecutionTime, + }, }, - }, - }; + }; - await expectedDelegationCreation(actualDelegation, expectedDelegation); - vi.useRealTimers(); - }); + await expectedDelegationCreation(actualDelegation, expectedDelegation); + vi.useRealTimers(); + } + ); - it("should throw an differentEServiceProducer error if requester is not Eservice producer", async () => { + it("should throw a differentEServiceProducer error if requester is not Eservice producer", async () => { const currentExecutionTime = new Date(); vi.useFakeTimers(); vi.setSystemTime(currentExecutionTime); @@ -234,7 +238,7 @@ describe("create delegation", () => { ...getMockTenant(delegatorId), externalId: { origin: "IPA", - value: "anythings", + value: "test", }, }; @@ -272,7 +276,7 @@ describe("create delegation", () => { }); it.each(activeDelegationStates)( - "should throw an delegationAlreadyExists error when Delegation for eservice producer already exists with for same delegator, delegate and eserivce ", + "should throw a delegationAlreadyExists error when a producer Delegation in state %s already exists with for same delegator, delegate and eservice", async (validDelegationState) => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); @@ -280,7 +284,7 @@ describe("create delegation", () => { ...getMockTenant(delegatorId), externalId: { origin: "IPA", - value: "anythings", + value: "test", }, }; @@ -294,8 +298,9 @@ describe("create delegation", () => { ], }; const eservice = getMockEService(generateId(), delegatorId); - const existentValidDelegation = { - ...getMockDelegationProducer({ + const existentActiveDelegation = { + ...getMockDelegation({ + kind: delegationKind.delegatedProducer, id: generateId(), delegatorId, delegateId: delegate.id, @@ -307,35 +312,40 @@ describe("create delegation", () => { await addOneTenant(delegate); await addOneTenant(delegator); await addOneEservice(eservice); - // Add existent valid delegation for the same delegator, delegate and eservice - await addOneDelegation(existentValidDelegation); - // Add existent invalid delegation for the same delegator, delegate and eservice + // Add existent active delegation for the same delegator, delegate and eservice + await addOneDelegation(existentActiveDelegation); + // Add existent inactive delegation for the same delegator, delegate and eservice await addOneDelegation({ - ...existentValidDelegation, + ...existentActiveDelegation, id: generateId(), - state: validDelegationState, + state: randomArrayItem(inactiveDelegationStates), }); // Add another generic delegation - await addOneDelegation(getMockDelegationProducer()); + await addOneDelegation( + getMockDelegation({ kind: delegationKind.delegatedProducer }) + ); // Add another delegation with same delegator await addOneDelegation( - getMockDelegationProducer({ + getMockDelegation({ + kind: delegationKind.delegatedProducer, delegatorId, }) ); // Add another delegation with same delegate await addOneDelegation( - getMockDelegationProducer({ + getMockDelegation({ + kind: delegationKind.delegatedProducer, delegateId: delegate.id, }) ); // Add another delegation for the same eservice await addOneDelegation( - getMockDelegationProducer({ + getMockDelegation({ + kind: delegationKind.delegatedProducer, eserviceId: eservice.id, }) ); @@ -356,14 +366,14 @@ describe("create delegation", () => { ).rejects.toThrowError( delegationAlreadyExists( delegatorId, - existentValidDelegation.eserviceId, + existentActiveDelegation.eserviceId, delegationKind.delegatedProducer ) ); } ); - it("should throw an tenantNotFound error if delegated tenant not exists", async () => { + it("should throw a tenantNotFound error if delegated tenant does not exist", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); const delegator = getMockTenant(delegatorId); @@ -388,7 +398,7 @@ describe("create delegation", () => { ).rejects.toThrowError(tenantNotFound(delegateId)); }); - it("should throw an tenantNotFound error if delegator tenant not exists", async () => { + it("should throw a tenantNotFound error if delegator tenant does not exist", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); @@ -448,7 +458,7 @@ describe("create delegation", () => { ...getMockTenant(delegatorId), externalId: { origin: "NOT_IPA", - value: "anythings", + value: "test", }, }; @@ -483,14 +493,14 @@ describe("create delegation", () => { ); }); - it("should throw an eserviceNotFound error if Eservice not exists", async () => { + it("should throw an eserviceNotFound error if Eservice does not exist", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); const delegator = { ...getMockTenant(delegatorId), externalId: { origin: "IPA", - value: "anythings", + value: "test", }, }; @@ -504,7 +514,8 @@ describe("create delegation", () => { ], }; const eserviceId = generateId(); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, id: generateId(), delegatorId, delegateId: delegate.id, @@ -530,14 +541,14 @@ describe("create delegation", () => { ).rejects.toThrowError(eserviceNotFound(eserviceId)); }); - it("should throw an invalidExternalOriginError error if delegator has externalId origin different from IPA", async () => { + it("should throw a tenantNotAllowedToDelegation error if delegate tenant has no DelegatedProducer feature", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); const delegator = { ...getMockTenant(delegatorId), externalId: { origin: "IPA", - value: "anythings", + value: "test", }, }; diff --git a/packages/delegation-process/test/getDelegationById.test.ts b/packages/delegation-process/test/getDelegationById.test.ts index ddaf026c79..9a323bbaec 100644 --- a/packages/delegation-process/test/getDelegationById.test.ts +++ b/packages/delegation-process/test/getDelegationById.test.ts @@ -1,6 +1,10 @@ /* eslint-disable functional/no-let */ -import { getMockDelegationProducer } from "pagopa-interop-commons-test/index.js"; -import { DelegationId, generateId } from "pagopa-interop-models"; +import { getMockDelegation } from "pagopa-interop-commons-test/index.js"; +import { + DelegationId, + delegationKind, + generateId, +} from "pagopa-interop-models"; import { describe, expect, it } from "vitest"; import { genericLogger } from "pagopa-interop-commons"; import { delegationNotFound } from "../src/model/domain/errors.js"; @@ -8,7 +12,9 @@ import { addOneDelegation, delegationService } from "./utils.js"; describe("get delegation by id", () => { it("should get the delegation if it exists", async () => { - const delegation = getMockDelegationProducer(); + const delegation = getMockDelegation({ + kind: delegationKind.delegatedConsumer, + }); await addOneDelegation(delegation); @@ -21,7 +27,9 @@ describe("get delegation by id", () => { }); it("should fail with delegationNotFound", async () => { - const delegation = getMockDelegationProducer(); + const delegation = getMockDelegation({ + kind: delegationKind.delegatedConsumer, + }); await addOneDelegation(delegation); diff --git a/packages/delegation-process/test/getDelegations.test.ts b/packages/delegation-process/test/getDelegations.test.ts index e7ba0c8381..2ad8673498 100644 --- a/packages/delegation-process/test/getDelegations.test.ts +++ b/packages/delegation-process/test/getDelegations.test.ts @@ -1,13 +1,19 @@ /* eslint-disable functional/no-let */ -import { getMockDelegationProducer } from "pagopa-interop-commons-test/index.js"; +import { getMockDelegation } from "pagopa-interop-commons-test/index.js"; import { describe, expect, it } from "vitest"; import { genericLogger } from "pagopa-interop-commons"; import { addOneDelegation, delegationService } from "./utils.js"; +import { delegationKind } from "pagopa-interop-models"; describe("get delegations", () => { it("should get delegations", async () => { - const delegation1 = getMockDelegationProducer({ state: "Active" }); - const delegation2 = getMockDelegationProducer(); + const delegation1 = getMockDelegation({ + kind: delegationKind.delegatedConsumer, + state: "Active", + }); + const delegation2 = getMockDelegation({ + kind: delegationKind.delegatedConsumer, + }); await addOneDelegation(delegation1); await addOneDelegation(delegation2); diff --git a/packages/delegation-process/test/rejectProducerDelegation.test.ts b/packages/delegation-process/test/rejectProducerDelegation.test.ts index f24401386d..cf5afe2063 100644 --- a/packages/delegation-process/test/rejectProducerDelegation.test.ts +++ b/packages/delegation-process/test/rejectProducerDelegation.test.ts @@ -2,13 +2,14 @@ import { decodeProtobufPayload, getMockAuthData, - getMockDelegationProducer, + getMockDelegation, getMockTenant, } from "pagopa-interop-commons-test/index.js"; import { describe, expect, it, vi } from "vitest"; import { DelegationId, ProducerDelegationRejectedV2, + delegationKind, generateId, toDelegationV2, unsafeBrandId, @@ -26,14 +27,15 @@ import { readLastDelegationEvent, } from "./utils.js"; -describe("reject delegation", () => { +describe("reject producer delegation", () => { it("should reject delegation if all validations succed", async () => { const currentExecutionTime = new Date(); vi.useFakeTimers(); vi.setSystemTime(currentExecutionTime); const delegate = getMockTenant(); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "WaitingForApproval", delegateId: delegate.id, }); @@ -93,7 +95,8 @@ describe("reject delegation", () => { it("should throw operationRestrictedToDelegate when rejecter is not the delegate", async () => { const delegate = getMockTenant(); const wrongDelegate = getMockTenant(); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "WaitingForApproval", delegateId: delegate.id, }); @@ -113,7 +116,8 @@ describe("reject delegation", () => { it("should throw incorrectState when delegation is not in WaitingForApproval state", async () => { const delegate = getMockTenant(); - const delegation = getMockDelegationProducer({ + const delegation = getMockDelegation({ + kind: delegationKind.delegatedProducer, state: "Active", delegateId: delegate.id, }); diff --git a/packages/delegation-process/test/revokeProducerDelegation.test.ts b/packages/delegation-process/test/revokeProducerDelegation.test.ts index ebff03f0cb..ffc5c3c287 100644 --- a/packages/delegation-process/test/revokeProducerDelegation.test.ts +++ b/packages/delegation-process/test/revokeProducerDelegation.test.ts @@ -1,7 +1,7 @@ import { fail } from "assert"; import { decodeProtobufPayload, - getMockDelegationProducer, + getMockDelegation, getMockEService, getMockTenant, getRandomAuthData, @@ -17,6 +17,7 @@ import { EServiceId, unsafeBrandId, DelegationContractId, + delegationKind, } from "pagopa-interop-models"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { @@ -102,7 +103,7 @@ const getNotRevocableStateSeeds = (): DelegationStateSeed[] => { ]; }; -describe("revoke delegation", () => { +describe("revoke producer delegation", () => { const TEST_EXECUTION_DATE = new Date(); beforeAll(() => { @@ -138,7 +139,8 @@ describe("revoke delegation", () => { await addOneEservice(eservice); const existentDelegation: Delegation = { - ...getMockDelegationProducer({ + ...getMockDelegation({ + kind: delegationKind.delegatedProducer, delegatorId, delegateId, }), @@ -257,7 +259,7 @@ describe("revoke delegation", () => { expect(delegationFromLastEvent).toMatchObject(expectedDelegation); }); - it("should throw an delegationNotFound if Delegation not exists", async () => { + it("should throw a delegationNotFound if Delegation does not exist", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); const delegationId = generateId(); @@ -271,7 +273,7 @@ describe("revoke delegation", () => { ).rejects.toThrow(delegationNotFound(delegationId)); }); - it("should throw an delegatorNotAllowToRevoke if Requester Id and DelegatorId are differents", async () => { + it("should throw a delegatorNotAllowToRevoke if Requester Id and DelegatorId are differents", async () => { const currentExecutionTime = new Date(); const delegatorId = generateId(); @@ -286,7 +288,8 @@ describe("revoke delegation", () => { delegationApprovalDate.setMonth(currentExecutionTime.getMonth() - 1); const existentDelegation = { - ...getMockDelegationProducer({ + ...getMockDelegation({ + kind: delegationKind.delegatedProducer, id: delegationId, delegateId, }), @@ -318,7 +321,7 @@ describe("revoke delegation", () => { }); it.each(notRevocableDelegationState)( - "should throw an delegatorNotAllowToRevoke if delegation doesn't have revocable one of revocable states [Rejected,Revoked]", + "should throw a delegatorNotAllowToRevoke if delegation doesn't have revocable one of revocable states [Rejected,Revoked]", async (notRevocableDelegationState: DelegationStateSeed) => { const currentExecutionTime = new Date(); @@ -333,7 +336,8 @@ describe("revoke delegation", () => { delegationActivationDate.setMonth(currentExecutionTime.getMonth() - 1); const existentDelegation: Delegation = { - ...getMockDelegationProducer({ + ...getMockDelegation({ + kind: delegationKind.delegatedProducer, delegatorId, delegateId, }), From 48f82db3544b98cf2771ad97c1c7df1e6e1a0247 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Fri, 15 Nov 2024 15:56:17 +0100 Subject: [PATCH 09/12] Fixing tests --- .../test/consumerServiceV2.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/delegation-readmodel-writer/test/consumerServiceV2.test.ts b/packages/delegation-readmodel-writer/test/consumerServiceV2.test.ts index 48348a453f..e9aa448762 100644 --- a/packages/delegation-readmodel-writer/test/consumerServiceV2.test.ts +++ b/packages/delegation-readmodel-writer/test/consumerServiceV2.test.ts @@ -1,4 +1,7 @@ -import { getMockDelegationProducer } from "pagopa-interop-commons-test/index.js"; +import { + getMockDelegation, + randomArrayItem, +} from "pagopa-interop-commons-test/index.js"; import { DelegationEventEnvelopeV2, toDelegationV2, @@ -6,13 +9,16 @@ import { ProducerDelegationRejectedV2, ProducerDelegationRevokedV2, ProducerDelegationSubmittedV2, + delegationKind, } from "pagopa-interop-models"; import { describe, expect, it } from "vitest"; import { handleMessageV2 } from "../src/delegationConsumerServiceV2.js"; import { delegations } from "./utils.js"; describe("Events V2", async () => { - const mockDelegation = getMockDelegationProducer(); + const mockDelegation = getMockDelegation({ + kind: randomArrayItem(Object.values(delegationKind)), + }); const mockMessage: DelegationEventEnvelopeV2 = { event_version: 2, stream_id: mockDelegation.id, From b78cd6e48bb6545a6ed67ea82a77b42dfc0b1f12 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:30:11 +0100 Subject: [PATCH 10/12] Improving error mapper --- packages/delegation-process/src/utilities/errorMappers.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/delegation-process/src/utilities/errorMappers.ts b/packages/delegation-process/src/utilities/errorMappers.ts index 98e2a40d3a..d6e689b18e 100644 --- a/packages/delegation-process/src/utilities/errorMappers.ts +++ b/packages/delegation-process/src/utilities/errorMappers.ts @@ -12,6 +12,7 @@ const { HTTP_STATUS_BAD_REQUEST, HTTP_STATUS_FORBIDDEN, HTTP_STATUS_UNAUTHORIZED, + HTTP_STATUS_CONFLICT, } = constants; export const getDelegationsErrorMapper = ( @@ -32,13 +33,16 @@ export const createProducerDelegationErrorMapper = ( match(error.code) .with( "eserviceNotFound", - "delegationAlreadyExists", "tenantNotFound", "invalidDelegatorAndDelegateIds", + () => HTTP_STATUS_BAD_REQUEST + ) + .with( "invalidExternalOriginId", "tenantNotAllowedToDelegation", - () => HTTP_STATUS_BAD_REQUEST + () => HTTP_STATUS_FORBIDDEN ) + .with("delegationAlreadyExists", () => HTTP_STATUS_CONFLICT) .otherwise(() => HTTP_STATUS_INTERNAL_SERVER_ERROR); export const revokeDelegationErrorMapper = ( From 2feec220c1fa370a3c9dfb3a7131b4d93f253459 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 12:31:56 +0100 Subject: [PATCH 11/12] Fixing tests --- packages/delegation-process/test/getDelegations.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/delegation-process/test/getDelegations.test.ts b/packages/delegation-process/test/getDelegations.test.ts index 2ad8673498..0a11027266 100644 --- a/packages/delegation-process/test/getDelegations.test.ts +++ b/packages/delegation-process/test/getDelegations.test.ts @@ -2,17 +2,17 @@ import { getMockDelegation } from "pagopa-interop-commons-test/index.js"; import { describe, expect, it } from "vitest"; import { genericLogger } from "pagopa-interop-commons"; -import { addOneDelegation, delegationService } from "./utils.js"; import { delegationKind } from "pagopa-interop-models"; +import { addOneDelegation, delegationService } from "./utils.js"; describe("get delegations", () => { it("should get delegations", async () => { const delegation1 = getMockDelegation({ - kind: delegationKind.delegatedConsumer, + kind: delegationKind.delegatedProducer, state: "Active", }); const delegation2 = getMockDelegation({ - kind: delegationKind.delegatedConsumer, + kind: delegationKind.delegatedProducer, }); await addOneDelegation(delegation1); await addOneDelegation(delegation2); From 5dd2321f6aaa3b5259f30bc8fe4c263a9c812074 Mon Sep 17 00:00:00 2001 From: Eric Camellini Date: Mon, 18 Nov 2024 17:49:08 +0100 Subject: [PATCH 12/12] Adding check IPA for delegator and delegate --- .../src/model/domain/errors.ts | 19 ++++--- .../src/services/delegationProducerService.ts | 4 +- .../src/services/validators.ts | 13 +++-- .../src/utilities/errorMappers.ts | 2 +- .../test/createProducerDelegation.test.ts | 52 +++++++++++++++++-- 5 files changed, 72 insertions(+), 18 deletions(-) diff --git a/packages/delegation-process/src/model/domain/errors.ts b/packages/delegation-process/src/model/domain/errors.ts index 44ae0a8a41..7822e1e03c 100644 --- a/packages/delegation-process/src/model/domain/errors.ts +++ b/packages/delegation-process/src/model/domain/errors.ts @@ -6,7 +6,9 @@ import { TenantId, DelegationState, DelegationKind, + Tenant, } from "pagopa-interop-models"; +import { match } from "ts-pattern"; export const errorCodes = { delegationNotFound: "0001", @@ -14,7 +16,7 @@ export const errorCodes = { delegationAlreadyExists: "0003", tenantNotFound: "0004", invalidDelegatorAndDelegateIds: "0005", - invalidExternalOriginId: "0006", + tenantIsNotIPAError: "0006", tenantNotAllowedToDelegation: "0007", delegationNotRevokable: "0008", operationNotAllowOnDelegation: "0009", @@ -71,13 +73,18 @@ export function delegatorAndDelegateSameIdError(): ApiError { }); } -export function invalidExternalOriginError( - externalOrigin?: string +export function tenantIsNotIPAError( + tenant: Tenant, + delegatorOrDelegate: "Delegator" | "Delegate" ): ApiError { + const delegatorOrDelegateString = match(delegatorOrDelegate) + .with("Delegator", () => "Delegator") + .with("Delegate", () => "Delegate") + .exhaustive(); return new ApiError({ - detail: `Delegator is not an IPA`, - code: "invalidExternalOriginId", - title: `Invalid External origin ${externalOrigin}`, + detail: `${delegatorOrDelegateString} ${tenant.id} with external origin ${tenant.externalId.origin} is not an IPA`, + code: "tenantIsNotIPAError", + title: `Invalid external origin`, }); } diff --git a/packages/delegation-process/src/services/delegationProducerService.ts b/packages/delegation-process/src/services/delegationProducerService.ts index 611ae9f45b..81c844f3f3 100644 --- a/packages/delegation-process/src/services/delegationProducerService.ts +++ b/packages/delegation-process/src/services/delegationProducerService.ts @@ -32,12 +32,12 @@ import { ReadModelService } from "./readModelService.js"; import { assertDelegationIsRevokable, assertDelegationNotExists, - assertDelegatorIsIPA, assertDelegatorIsNotDelegate, assertDelegatorIsProducer, assertTenantAllowedToReceiveDelegation, assertIsDelegate, assertIsState, + assertDelegatorAndDelegateIPA, } from "./validators.js"; import { contractBuilder } from "./delegationContractBuilder.js"; import { retrieveDelegationById } from "./delegationService.js"; @@ -88,7 +88,7 @@ export function delegationProducerServiceBuilder( delegate, delegationKind.delegatedProducer ); - await assertDelegatorIsIPA(delegator); + await assertDelegatorAndDelegateIPA(delegator, delegate); const eservice = await retrieveEserviceById(eserviceId); assertDelegatorIsProducer(delegatorId, eservice); diff --git a/packages/delegation-process/src/services/validators.ts b/packages/delegation-process/src/services/validators.ts index 4b3acc17f2..e4060bff3f 100644 --- a/packages/delegation-process/src/services/validators.ts +++ b/packages/delegation-process/src/services/validators.ts @@ -18,8 +18,8 @@ import { delegatorNotAllowToRevoke, differentEServiceProducer, incorrectState, - invalidExternalOriginError, operationRestrictedToDelegate, + tenantIsNotIPAError, tenantNotAllowedToDelegation, } from "../model/domain/errors.js"; import { ReadModelService } from "./readModelService.js"; @@ -53,11 +53,16 @@ export const assertDelegatorIsNotDelegate = ( } }; -export const assertDelegatorIsIPA = async ( - delegator?: Tenant +export const assertDelegatorAndDelegateIPA = async ( + delegator: Tenant, + delegate: Tenant ): Promise => { if (delegator?.externalId?.origin !== PUBLIC_ADMINISTRATIONS_IDENTIFIER) { - throw invalidExternalOriginError(delegator?.externalId?.origin); + throw tenantIsNotIPAError(delegator, "Delegator"); + } + + if (delegate?.externalId?.origin !== PUBLIC_ADMINISTRATIONS_IDENTIFIER) { + throw tenantIsNotIPAError(delegate, "Delegate"); } }; diff --git a/packages/delegation-process/src/utilities/errorMappers.ts b/packages/delegation-process/src/utilities/errorMappers.ts index d6e689b18e..00268107f2 100644 --- a/packages/delegation-process/src/utilities/errorMappers.ts +++ b/packages/delegation-process/src/utilities/errorMappers.ts @@ -38,7 +38,7 @@ export const createProducerDelegationErrorMapper = ( () => HTTP_STATUS_BAD_REQUEST ) .with( - "invalidExternalOriginId", + "tenantIsNotIPAError", "tenantNotAllowedToDelegation", () => HTTP_STATUS_FORBIDDEN ) diff --git a/packages/delegation-process/test/createProducerDelegation.test.ts b/packages/delegation-process/test/createProducerDelegation.test.ts index 0a2bde3089..1463fba985 100644 --- a/packages/delegation-process/test/createProducerDelegation.test.ts +++ b/packages/delegation-process/test/createProducerDelegation.test.ts @@ -25,7 +25,7 @@ import { delegatorAndDelegateSameIdError, differentEServiceProducer, eserviceNotFound, - invalidExternalOriginError, + tenantIsNotIPAError, tenantNotAllowedToDelegation, tenantNotFound, } from "../src/model/domain/errors.js"; @@ -451,7 +451,7 @@ describe("create producer delegation", () => { ).rejects.toThrowError(delegatorAndDelegateSameIdError()); }); - it("should throw an invalidExternalOriginError error if delegator has externalId origin different from IPA", async () => { + it("should throw an tenantIsNotIPAError error if delegator has externalId origin different from IPA", async () => { const delegatorId = generateId(); const authData = getRandomAuthData(delegatorId); const delegator = { @@ -488,9 +488,51 @@ describe("create producer delegation", () => { serviceName: "DelegationServiceTest", } ) - ).rejects.toThrowError( - invalidExternalOriginError(delegator.externalId.origin) - ); + ).rejects.toThrowError(tenantIsNotIPAError(delegator, "Delegator")); + }); + + it("should throw an tenantIsNotIPAError error if delegate has externalId origin different from IPA", async () => { + const delegatorId = generateId(); + const authData = getRandomAuthData(delegatorId); + const delegator = { + ...getMockTenant(delegatorId), + externalId: { + origin: "IPA", + value: "test", + }, + }; + + const delegate = { + ...getMockTenant(), + externalId: { + origin: "NOT_IPA", + value: "test", + }, + features: [ + { + type: "DelegatedProducer" as const, + availabilityTimestamp: new Date(), + }, + ], + }; + + await addOneTenant(delegate); + await addOneTenant(delegator); + + await expect( + delegationProducerService.createProducerDelegation( + { + delegateId: delegate.id, + eserviceId: generateId(), + }, + { + authData, + logger: genericLogger, + correlationId: generateId(), + serviceName: "DelegationServiceTest", + } + ) + ).rejects.toThrowError(tenantIsNotIPAError(delegate, "Delegate")); }); it("should throw an eserviceNotFound error if Eservice does not exist", async () => {