From 5cc52897490c21df83512c231c55f453326a2ad5 Mon Sep 17 00:00:00 2001 From: silvicir <127971996+silvicir@users.noreply.github.com> Date: Fri, 4 Oct 2024 11:59:57 +0200 Subject: [PATCH] chore: removed PDV user id Refs: #1156 SIW-1706 --- package.json | 2 +- src/controllers/ioWalletController.ts | 95 ++++---------- .../__tests__/ioWalletService.test.ts | 124 ++---------------- src/services/ioWalletService.ts | 50 +------ 4 files changed, 41 insertions(+), 230 deletions(-) diff --git a/package.json b/package.json index 36ecd9934..964277c5b 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "generate:proxy:api-session": "rimraf generated/session && gen-api-models --api-spec api_session.yaml --out-dir generated/session", "generate:lollipop-definitions": "rimraf generated/lollipop && gen-api-models --api-spec openapi/lollipop_definitions.yaml --out-dir generated/lollipop", "generate:lollipop-first-sign": "rimraf generated/lollipop-first-consumer && gen-api-models --api-spec openapi/consumed/lollipop_first_consumer.yaml --out-dir generated/lollipop-first-consumer --request-types --response-decoders --client", - "generate:api:io-wallet": "rimraf generated/io-wallet-api && gen-api-models --api-spec https://raw.githubusercontent.com/pagopa/io-wallet/io-wallet-user-func@1.2.0/apps/io-wallet-user-func/openapi.yaml --no-strict --out-dir generated/io-wallet-api --request-types --response-decoders --client", + "generate:api:io-wallet": "rimraf generated/io-wallet-api && gen-api-models --api-spec https://raw.githubusercontent.com/pagopa/io-wallet/io-wallet-user-func@1.2.3/apps/io-wallet-user-func/openapi.yaml --no-strict --out-dir generated/io-wallet-api --request-types --response-decoders --client", "generate:proxy:io-wallet-models": "rimraf generated/io-wallet && gen-api-models --api-spec api_io_wallet.yaml --out-dir generated/io-wallet", "postversion": "git push && git push --tags", "dist:modules": "modclean -r -n default:safe && yarn install --production", diff --git a/src/controllers/ioWalletController.ts b/src/controllers/ioWalletController.ts index 0545a9249..3c212d1ed 100644 --- a/src/controllers/ioWalletController.ts +++ b/src/controllers/ioWalletController.ts @@ -1,5 +1,3 @@ -/* eslint-disable sonarjs/no-identical-functions */ -// TODO SIW-1706 /** * This controller handles the IO_WALLET requests from the * app by forwarding the call to the API system. @@ -8,7 +6,6 @@ import * as express from "express"; import * as TE from "fp-ts/TaskEither"; import * as E from "fp-ts/Either"; -import { sequenceS } from "fp-ts/lib/Apply"; import { getResponseErrorForbiddenNotAuthorized, @@ -20,14 +17,13 @@ import { IResponseErrorValidation, IResponseSuccessJson, IResponseSuccessNoContent, - ResponseErrorInternal, + ResponseErrorValidation, } from "@pagopa/ts-commons/lib/responses"; import { pipe } from "fp-ts/lib/function"; import { Errors } from "io-ts"; import { FiscalCode, NonEmptyString } from "@pagopa/ts-commons/lib/strings"; import { readableReport } from "@pagopa/ts-commons/lib/reporters"; -import { UserDetailView } from "../../generated/io-wallet-api/UserDetailView"; import IoWalletService from "../services/ioWalletService"; import { NonceDetailView } from "../../generated/io-wallet-api/NonceDetailView"; @@ -38,13 +34,9 @@ import { WalletAttestationView } from "../../generated/io-wallet-api/WalletAttes import { FF_IO_WALLET_TRIAL_ENABLED } from "../config"; import { SetCurrentWalletInstanceStatusBody } from "../../generated/io-wallet/SetCurrentWalletInstanceStatusBody"; -const toErrorRetrievingTheUserId = ResponseErrorInternal( - "Error retrieving the user id" -); - const toValidationError = (errors: Errors) => - ResponseErrorInternal( - // TODO SIW-1706 replace with ResponseErrorValidation + ResponseErrorValidation( + "Bad request", `Error validating the request body: ${readableReport(errors)}` ); @@ -75,26 +67,22 @@ export default class IoWalletController { > => withUserFromRequest(req, async (user) => pipe( - sequenceS(TE.ApplyPar)({ - body: pipe( + this.ensureFiscalCodeIsAllowed(user.fiscal_code), + TE.chainW(() => + pipe( req.body, CreateWalletInstanceBody.decode, E.mapLeft(toValidationError), TE.fromEither - ), - userId: this.getAllowedUserId(user.fiscal_code), - }), - TE.map( - ({ - body: { challenge, key_attestation, hardware_key_tag }, - userId, - }) => - this.ioWalletService.createWalletInstance( - challenge, - hardware_key_tag, - key_attestation, - userId - ) + ) + ), + TE.map(({ challenge, key_attestation, hardware_key_tag }) => + this.ioWalletService.createWalletInstance( + challenge, + hardware_key_tag, + key_attestation, + user.fiscal_code + ) ), TE.toUnion )() @@ -116,20 +104,20 @@ export default class IoWalletController { > => withUserFromRequest(req, async (user) => pipe( - sequenceS(TE.ApplyPar)({ - body: pipe( + this.ensureFiscalCodeIsAllowed(user.fiscal_code), + TE.chainW(() => + pipe( req.body, CreateWalletAttestationBody.decode, E.mapLeft(toValidationError), TE.fromEither - ), - userId: this.getAllowedUserId(user.fiscal_code), - }), - TE.map(({ body: { grant_type, assertion }, userId }) => + ) + ), + TE.map(({ grant_type, assertion }) => this.ioWalletService.createWalletAttestation( assertion, grant_type, - userId + user.fiscal_code ) ), TE.toUnion @@ -170,24 +158,6 @@ export default class IoWalletController { )() ); - private readonly retrieveUserId = (fiscalCode: FiscalCode) => - pipe( - TE.tryCatch( - () => this.ioWalletService.getUserByFiscalCode(fiscalCode), - E.toError - ), - TE.chain( - TE.fromPredicate( - (r): r is IResponseSuccessJson => - r.kind === "IResponseSuccessJson", - (e) => - new Error( - `An error occurred while retrieving the User id. | ${e.detail}` - ) - ) - ) - ); - private readonly ensureUserIsAllowed = ( userId: NonEmptyString ): TE.TaskEither => @@ -207,7 +177,6 @@ export default class IoWalletController { ) : TE.right(undefined); - // TODO SIW-1706 private readonly ensureFiscalCodeIsAllowed = (fiscalCode: FiscalCode) => pipe( fiscalCode, @@ -220,24 +189,4 @@ export default class IoWalletController { ) ) ); - - private readonly getAllowedUserId = (fiscalCode: FiscalCode) => - pipe( - fiscalCode, - NonEmptyString.decode, - TE.fromEither, - TE.chainW(this.ensureUserIsAllowed), - TE.mapLeft(() => - getResponseErrorForbiddenNotAuthorized( - "Not authorized to perform this action" - ) - ), - TE.chainW(() => - pipe( - this.retrieveUserId(fiscalCode), - TE.mapLeft(() => toErrorRetrievingTheUserId) - ) - ), - TE.map((response) => response.value.id) - ); } diff --git a/src/services/__tests__/ioWalletService.test.ts b/src/services/__tests__/ioWalletService.test.ts index 78e0bc21d..181eca9b6 100644 --- a/src/services/__tests__/ioWalletService.test.ts +++ b/src/services/__tests__/ioWalletService.test.ts @@ -9,7 +9,6 @@ import { StatusEnum } from "../../../generated/io-wallet-api/SetWalletInstanceSt const mockGetEntityConfiguration = jest.fn(); const mockGetNonce = jest.fn(); -const mockGetUserByFiscalCode = jest.fn(); const mockCreateWalletInstance = jest.fn(); const mockCreateWalletAttestation = jest.fn(); const mockHealthCheck = jest.fn(); @@ -17,15 +16,6 @@ const mockGetCurrentWalletInstanceStatus = jest.fn(); const mockSetWalletInstanceStatus = jest.fn(); const mockSetCurrentWalletInstanceStatus = jest.fn(); -mockGetUserByFiscalCode.mockImplementation(() => - t.success({ - status: 200, - value: { - id: "000000000000", - }, - }) -); - mockGetNonce.mockImplementation(() => t.success({ status: 200, @@ -71,7 +61,6 @@ mockSetCurrentWalletInstanceStatus.mockImplementation(() => const api = { getEntityConfiguration: mockGetEntityConfiguration, getNonce: mockGetNonce, - getUserByFiscalCode: mockGetUserByFiscalCode, createWalletInstance: mockCreateWalletInstance, createWalletAttestation: mockCreateWalletAttestation, healthCheck: mockHealthCheck, @@ -101,91 +90,6 @@ const trialSystemApi = { const aFiscalCode = "GRBGPP87L04L741X" as FiscalCode; -describe("IoWalletService#getUserByFiscalCode", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should make the correct api call", async () => { - const service = new IoWalletService(api, trialSystemApi); - - await service.getUserByFiscalCode(aFiscalCode); - - expect(mockGetUserByFiscalCode).toHaveBeenCalledWith({ - body: { - fiscal_code: aFiscalCode, - }, - }); - }); - - it("should handle a success response", async () => { - const service = new IoWalletService(api, trialSystemApi); - - const res = await service.getUserByFiscalCode(aFiscalCode); - - expect(res).toMatchObject({ - kind: "IResponseSuccessJson", - }); - }); - - it("should handle an internal error when the API client returns 422", async () => { - mockGetUserByFiscalCode.mockImplementationOnce(() => - t.success({ status: 422 }) - ); - - const service = new IoWalletService(api, trialSystemApi); - - const res = await service.getUserByFiscalCode(aFiscalCode); - - expect(res).toMatchObject({ - kind: "IResponseErrorGeneric", - }); - }); - - it("should handle an internal error when the API client returns 500", async () => { - const aGenericProblem = {}; - mockGetUserByFiscalCode.mockImplementationOnce(() => - t.success({ status: 500, value: aGenericProblem }) - ); - - const service = new IoWalletService(api, trialSystemApi); - - const res = await service.getUserByFiscalCode(aFiscalCode); - - expect(res).toMatchObject({ - kind: "IResponseErrorInternal", - }); - }); - - it("should handle an internal error when the API client returns a code not specified in spec", async () => { - const aGenericProblem = {}; - mockGetUserByFiscalCode.mockImplementationOnce(() => - t.success({ status: 599, value: aGenericProblem }) - ); - - const service = new IoWalletService(api, trialSystemApi); - - const res = await service.getUserByFiscalCode(aFiscalCode); - - expect(res).toMatchObject({ - kind: "IResponseErrorInternal", - }); - }); - - it("should return an error if the api call throws an error", async () => { - mockGetUserByFiscalCode.mockImplementationOnce(() => { - throw new Error(); - }); - const service = new IoWalletService(api, trialSystemApi); - - const res = await service.getUserByFiscalCode(aFiscalCode); - - expect(res).toMatchObject({ - kind: "IResponseErrorInternal", - }); - }); -}); - describe("IoWalletService#getNonce", () => { beforeEach(() => { jest.clearAllMocks(); @@ -265,7 +169,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(mockCreateWalletInstance).toHaveBeenCalledWith({ @@ -273,8 +177,8 @@ describe("IoWalletService#createWalletInstance", () => { challenge: "challenge", key_attestation: "key_attestation", hardware_key_tag: "hardware_key_tag", + fiscal_code: aFiscalCode, }, - "x-iowallet-user-id": "userId", }); }); @@ -285,7 +189,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -304,7 +208,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -324,7 +228,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -344,7 +248,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -362,7 +266,7 @@ describe("IoWalletService#createWalletInstance", () => { "challenge" as NonEmptyString, "hardware_key_tag" as NonEmptyString, "key_attestation" as NonEmptyString, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -385,15 +289,15 @@ describe("IoWalletService#createWalletAttestation", () => { await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(mockCreateWalletAttestation).toHaveBeenCalledWith({ body: { grant_type, assertion: "assertion", + fiscal_code: aFiscalCode, }, - "x-iowallet-user-id": "userId", }); }); @@ -403,7 +307,7 @@ describe("IoWalletService#createWalletAttestation", () => { const res = await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -421,7 +325,7 @@ describe("IoWalletService#createWalletAttestation", () => { const res = await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -440,7 +344,7 @@ describe("IoWalletService#createWalletAttestation", () => { const res = await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -459,7 +363,7 @@ describe("IoWalletService#createWalletAttestation", () => { const res = await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(res).toMatchObject({ @@ -476,7 +380,7 @@ describe("IoWalletService#createWalletAttestation", () => { const res = await service.createWalletAttestation( "assertion" as NonEmptyString, grant_type, - "userId" + aFiscalCode ); expect(res).toMatchObject({ diff --git a/src/services/ioWalletService.ts b/src/services/ioWalletService.ts index c52e13434..f54ba55b6 100644 --- a/src/services/ioWalletService.ts +++ b/src/services/ioWalletService.ts @@ -21,9 +21,7 @@ import { } from "@pagopa/ts-commons/lib/responses"; import { FiscalCode, NonEmptyString } from "@pagopa/ts-commons/lib/strings"; -import { UserDetailView } from "generated/io-wallet-api/UserDetailView"; import { NonceDetailView } from "generated/io-wallet-api/NonceDetailView"; -import { Id } from "generated/io-wallet-api/Id"; import { Grant_typeEnum } from "generated/io-wallet-api/CreateWalletAttestationBody"; import { pipe } from "fp-ts/lib/function"; import * as O from "fp-ts/Option"; @@ -56,46 +54,6 @@ export default class IoWalletService { > ) {} - /** - * Get the Wallet User id. - */ - public readonly getUserByFiscalCode = ( - fiscalCode: FiscalCode - ): Promise< - | IResponseErrorInternal - | IResponseErrorGeneric - | IResponseSuccessJson - | IResponseErrorServiceUnavailable - > => - withCatchAsInternalError(async () => { - const validated = await this.ioWalletApiClient.getUserByFiscalCode({ - body: { fiscal_code: fiscalCode }, - }); - return withValidatedOrInternalError(validated, (response) => { - switch (response.status) { - case 200: - return ResponseSuccessJson(response.value); - case 422: - return ResponseErrorGeneric( - response.status, - unprocessableContentError, - invalidRequest - ); - case 500: - return ResponseErrorInternal( - `Internal server error | ${response.value}` - ); - case 503: - return ResponseErrorServiceTemporarilyUnavailable( - serviceUnavailableDetail, - "10" - ); - default: - return ResponseErrorStatusNotDefinedInSpec(response); - } - }); - }); - /** * Get a nonce. */ @@ -132,7 +90,7 @@ export default class IoWalletService { challenge: NonEmptyString, hardware_key_tag: NonEmptyString, key_attestation: NonEmptyString, - userId: Id + fiscal_code: FiscalCode ): Promise< | IResponseErrorInternal | IResponseErrorGeneric @@ -143,10 +101,10 @@ export default class IoWalletService { const validated = await this.ioWalletApiClient.createWalletInstance({ body: { challenge, + fiscal_code, hardware_key_tag, key_attestation, }, - "x-iowallet-user-id": userId, }); return withValidatedOrInternalError(validated, (response) => { switch (response.status) { @@ -185,7 +143,7 @@ export default class IoWalletService { public readonly createWalletAttestation = ( assertion: NonEmptyString, grant_type: Grant_typeEnum, - userId: Id + fiscal_code: FiscalCode ): Promise< | IResponseErrorInternal | IResponseErrorGeneric @@ -198,9 +156,9 @@ export default class IoWalletService { const validated = await this.ioWalletApiClient.createWalletAttestation({ body: { assertion, + fiscal_code, grant_type, }, - "x-iowallet-user-id": userId, }); return withValidatedOrInternalError(validated, (response) => { switch (response.status) {