Merge branch 'main' into IMN-736-explicit-error-handling-api-gw

pagopa · Oct 1, 2024 · 1a9c059 · 1a9c059
2 parents eadfe95 + 6c85746
commit 1a9c059
Show file tree

Hide file tree

Showing 22 changed files with 350 additions and 100 deletions.
diff --git a/packages/agreement-lifecycle/src/validator/attributesValidator.ts b/packages/agreement-lifecycle/src/validator/attributesValidator.ts
@@ -20,12 +20,14 @@ const attributesSatisfied = (
   descriptorAttributes: EServiceAttribute[][],
   consumerAttributeIds: Array<TenantAttribute["id"]>
 ): boolean =>
-  descriptorAttributes.every((attributeList) => {
-    const attributes = attributeList.map((a) => a.id);
-    return (
-      attributes.filter((a) => consumerAttributeIds.includes(a)).length > 0
-    );
-  });
+  descriptorAttributes
+    .filter((attGroup) => attGroup.length > 0)
+    .every((attributeList) => {
+      const attributes = attributeList.map((a) => a.id);
+      return (
+        attributes.filter((a) => consumerAttributeIds.includes(a)).length > 0
+      );
+    });
 
 export const certifiedAttributesSatisfied = (
   descriptor: DescriptorWithOnlyAttributes,

diff --git a/packages/api-clients/template-bff.hbs b/packages/api-clients/template-bff.hbs
@@ -40,8 +40,8 @@ export const {{@key}}Endpoints = makeApi([
 				{{/if}}
 				{{#if (and (eq type "Query") (eq schema "z.array(z.string()).optional().default([])"))}}
 						schema: z.union([z.string().optional().transform(v => v ? v.split(",") : undefined).pipe(z.array(z.string()).optional().default([])), z.array(z.string()).optional().default([])]),
-				{{else if (and (eq type "Query") (eq schema "z.array(AttributeKind)"))}}
-						schema: z.union([z.string().transform(v => v.split(",")).pipe(z.array(AttributeKind)), z.array(AttributeKind)])
+				{{else if (and (eq type "Query") (eq schema "z.array(AttributeKind).default([])"))}}
+						schema: z.union([z.string().transform(v => v.split(",")).pipe(z.array(AttributeKind).default([])), z.array(AttributeKind).default([])])
 				{{else if (and (eq type "Query") (eq schema "z.array(AgreementState).optional().default([])"))}}
 						schema: z.union([z.string().optional().transform(v => v ? v.split(",") : undefined ).pipe(z.array(AgreementState).optional().default([])),z.array(AgreementState).optional().default([])])
 				{{else if (and (eq type "Query") (eq schema "z.array(EServiceDescriptorState).optional().default([])"))}}

diff --git a/packages/authorization-process/src/model/domain/errors.ts b/packages/authorization-process/src/model/domain/errors.ts
@@ -37,6 +37,8 @@ export const errorCodes = {
   producerKeyNotFound: "0024",
   organizationNotAllowedOnEService: "0025",
   eserviceAlreadyLinkedToProducerKeychain: "0026",
+  userNotAllowedToDeleteClientKey: "0027",
+  userNotAllowedToDeleteProducerKeychainKey: "0028",
 };
 
 export type ErrorCodes = keyof typeof errorCodes;
@@ -260,6 +262,30 @@ export function userNotAllowedOnProducerKeychain(
   });
 }
 
+export function userNotAllowedToDeleteClientKey(
+  userId: UserId,
+  client: ClientId,
+  kid: string
+): ApiError<ErrorCodes> {
+  return new ApiError({
+    detail: `User ${userId} is not allowed to delete client (${client}) key ${kid}`,
+    code: "userNotAllowedToDeleteClientKey",
+    title: "User not allowed to delete producer keychain key",
+  });
+}
+
+export function userNotAllowedToDeleteProducerKeychainKey(
+  userId: UserId,
+  producerKeychain: ProducerKeychainId,
+  kid: string
+): ApiError<ErrorCodes> {
+  return new ApiError({
+    detail: `User ${userId} is not allowed to delete producer keychain (${producerKeychain}) key ${kid}`,
+    code: "userNotAllowedToDeleteProducerKeychainKey",
+    title: "User not allowed to delete producer keychain key",
+  });
+}
+
 export function producerKeyNotFound(
   keyId: string,
   producerKeychainId: ProducerKeychainId

diff --git a/packages/authorization-process/src/services/authorizationService.ts b/packages/authorization-process/src/services/authorizationService.ts
@@ -58,6 +58,8 @@ import {
   producerKeychainUserAlreadyAssigned,
   producerKeychainUserIdNotFound,
   eserviceAlreadyLinkedToProducerKeychain,
+  userNotAllowedToDeleteProducerKeychainKey,
+  userNotAllowedToDeleteClientKey,
 } from "../model/domain/errors.js";
 import {
   toCreateEventClientAdded,
@@ -362,17 +364,28 @@ export function authorizationServiceBuilder(
       const client = await retrieveClient(clientId, readModelService);
       assertOrganizationIsClientConsumer(authData.organizationId, client.data);
 
+      const hasSecurityRole = authData.userRoles.includes(
+        userRoles.SECURITY_ROLE
+      );
+
+      if (hasSecurityRole && !client.data.users.includes(authData.userId)) {
+        throw userNotAllowedOnClient(authData.userId, client.data.id);
+      }
+
       const keyToRemove = client.data.keys.find(
         (key) => key.kid === keyIdToRemove
       );
+
       if (!keyToRemove) {
         throw clientKeyNotFound(keyIdToRemove, client.data.id);
       }
-      if (
-        authData.userRoles.includes(userRoles.SECURITY_ROLE) &&
-        !client.data.users.includes(authData.userId)
-      ) {
-        throw userNotAllowedOnClient(authData.userId, client.data.id);
+
+      if (hasSecurityRole && keyToRemove.userId !== authData.userId) {
+        throw userNotAllowedToDeleteClientKey(
+          authData.userId,
+          client.data.id,
+          keyToRemove.kid
+        );
       }
 
       const updatedClient: Client = {
@@ -1080,8 +1093,12 @@ export function authorizationServiceBuilder(
         producerKeychain.data
       );
 
+      const hasSecurityRole = authData.userRoles.includes(
+        userRoles.SECURITY_ROLE
+      );
+
       if (
-        authData.userRoles.includes(userRoles.SECURITY_ROLE) &&
+        hasSecurityRole &&
         !producerKeychain.data.users.includes(authData.userId)
       ) {
         throw userNotAllowedOnProducerKeychain(
@@ -1098,6 +1115,14 @@ export function authorizationServiceBuilder(
         throw producerKeyNotFound(keyIdToRemove, producerKeychain.data.id);
       }
 
+      if (hasSecurityRole && keyToRemove.userId !== authData.userId) {
+        throw userNotAllowedToDeleteProducerKeychainKey(
+          authData.userId,
+          producerKeychain.data.id,
+          keyToRemove.kid
+        );
+      }
+
       const updatedProducerKeychain: ProducerKeychain = {
         ...producerKeychain.data,
         keys: producerKeychain.data.keys.filter(

diff --git a/packages/authorization-process/test/removeClientKeyById.test.ts b/packages/authorization-process/test/removeClientKeyById.test.ts
@@ -21,6 +21,7 @@ import {
   clientKeyNotFound,
   organizationNotAllowedOnClient,
   userNotAllowedOnClient,
+  userNotAllowedToDeleteClientKey,
 } from "../src/model/domain/errors.js";
 import {
   addOneClient,
@@ -33,10 +34,15 @@ describe("remove client key", () => {
     const mockConsumer = getMockTenant();
     const keyToRemove = getMockKey();
     const keyToNotRemove = getMockKey();
+    const mockAuthData = {
+      ...getMockAuthData(mockConsumer.id),
+      userRoles: [userRoles.ADMIN_ROLE],
+    };
 
     const mockClient: Client = {
       ...getMockClient(),
       consumerId: mockConsumer.id,
+      users: [mockAuthData.userId],
       keys: [keyToRemove, keyToNotRemove],
     };
 
@@ -45,7 +51,7 @@ describe("remove client key", () => {
     await authorizationService.deleteClientKeyById({
       clientId: mockClient.id,
       keyIdToRemove: keyToRemove.kid,
-      authData: getMockAuthData(mockConsumer.id),
+      authData: mockAuthData,
       correlationId: generateId(),
       logger: genericLogger,
     });
@@ -209,4 +215,37 @@ describe("remove client key", () => {
       })
     ).rejects.toThrowError(userNotAllowedOnClient(mockUserId, mockClient.id));
   });
+  it("should throw userNotAllowedToDeleteClientKey if a security user tries to delete a key not uploaded by himself", async () => {
+    const mockConsumer = getMockTenant();
+    const mockUserId: UserId = generateId();
+    const keyToRemove: Key = { ...getMockKey(), userId: generateId() };
+    const mockClient: Client = {
+      ...getMockClient(),
+      consumerId: mockConsumer.id,
+      keys: [keyToRemove],
+      users: [mockUserId],
+    };
+
+    await addOneClient(mockClient);
+
+    expect(
+      authorizationService.deleteClientKeyById({
+        clientId: mockClient.id,
+        keyIdToRemove: keyToRemove.kid,
+        authData: {
+          ...getMockAuthData(mockConsumer.id),
+          userRoles: [userRoles.SECURITY_ROLE],
+          userId: mockUserId,
+        },
+        correlationId: generateId(),
+        logger: genericLogger,
+      })
+    ).rejects.toThrowError(
+      userNotAllowedToDeleteClientKey(
+        mockUserId,
+        mockClient.id,
+        keyToRemove.kid
+      )
+    );
+  });
 });
diff --git a/packages/authorization-process/test/removeProducerKeychainKeyById.test.ts b/packages/authorization-process/test/removeProducerKeychainKeyById.test.ts
@@ -21,6 +21,7 @@ import {
   producerKeyNotFound,
   organizationNotAllowedOnProducerKeychain,
   userNotAllowedOnProducerKeychain,
+  userNotAllowedToDeleteProducerKeychainKey,
 } from "../src/model/domain/errors.js";
 import {
   addOneProducerKeychain,
@@ -34,7 +35,10 @@ describe("remove producer keychain key", () => {
     const keyToRemove = getMockKey();
     const keyToNotRemove = getMockKey();
 
-    const authData = getMockAuthData(mockProducer.id);
+    const authData = {
+      ...getMockAuthData(mockProducer.id),
+      userRoles: [userRoles.ADMIN_ROLE],
+    };
 
     const mockProducerKeychain: ProducerKeychain = {
       ...getMockProducerKeychain(),
@@ -249,4 +253,37 @@ describe("remove producer keychain key", () => {
       userNotAllowedOnProducerKeychain(mockUserId, mockProducerKeychain.id)
     );
   });
+  it("should throw userNotAllowedToDeleteClientKey if a security user tries to delete a key not uploaded by himself", async () => {
+    const mockProducer = getMockTenant();
+    const mockUserId: UserId = generateId();
+    const keyToRemove: Key = { ...getMockKey(), userId: generateId() };
+    const mockProducerKeychain: ProducerKeychain = {
+      ...getMockProducerKeychain(),
+      producerId: mockProducer.id,
+      keys: [keyToRemove],
+      users: [mockUserId],
+    };
+
+    await addOneProducerKeychain(mockProducerKeychain);
+
+    expect(
+      authorizationService.removeProducerKeychainKeyById({
+        producerKeychainId: mockProducerKeychain.id,
+        keyIdToRemove: keyToRemove.kid,
+        authData: {
+          ...getMockAuthData(mockProducer.id),
+          userRoles: [userRoles.SECURITY_ROLE],
+          userId: mockUserId,
+        },
+        correlationId: generateId(),
+        logger: genericLogger,
+      })
+    ).rejects.toThrowError(
+      userNotAllowedToDeleteProducerKeychainKey(
+        mockUserId,
+        mockProducerKeychain.id,
+        keyToRemove.kid
+      )
+    );
+  });
 });
diff --git a/packages/backend-for-frontend/src/api/agreementApiConverter.ts b/packages/backend-for-frontend/src/api/agreementApiConverter.ts
@@ -29,3 +29,15 @@ export function toBffCompactAgreement(
     canBeUpgraded: isAgreementUpgradable(eservice, agreement),
   };
 }
+
+export function toBffAgreementConsumerDocument(
+  doc: agreementApi.Document
+): bffApi.Document {
+  return {
+    id: doc.id,
+    name: doc.name,
+    prettyName: doc.prettyName,
+    contentType: doc.contentType,
+    createdAt: doc.createdAt,
+  };
+}
diff --git a/packages/backend-for-frontend/src/api/attributeApiConverter.ts b/packages/backend-for-frontend/src/api/attributeApiConverter.ts
@@ -1,7 +1,7 @@
 import { createHash } from "crypto";
 import { bffApi, attributeRegistryApi } from "pagopa-interop-api-clients";
 
-export const toApiAttributeProcessSeed = (
+export const toApiCertifiedAttributeProcessSeed = (
   seed: bffApi.AttributeSeed
 ): attributeRegistryApi.CertifiedAttributeSeed => ({
   ...seed,

diff --git a/packages/backend-for-frontend/src/api/purposeApiConverter.ts b/packages/backend-for-frontend/src/api/purposeApiConverter.ts
@@ -0,0 +1,29 @@
+import { bffApi, purposeApi } from "pagopa-interop-api-clients";
+
+function toBffApiPurposeVersionDocument(
+  riskAnalysis: purposeApi.PurposeVersionDocument
+): bffApi.PurposeVersionDocument {
+  return {
+    id: riskAnalysis.id,
+    contentType: riskAnalysis.contentType,
+    createdAt: riskAnalysis.createdAt,
+  };
+}
+
+export function toBffApiPurposeVersion(
+  purposeVersion: purposeApi.PurposeVersion
+): bffApi.PurposeVersion {
+  return {
+    id: purposeVersion.id,
+    state: purposeVersion.state,
+    createdAt: purposeVersion.createdAt,
+    suspendedAt: purposeVersion.suspendedAt,
+    updatedAt: purposeVersion.updatedAt,
+    firstActivationAt: purposeVersion.firstActivationAt,
+    dailyCalls: purposeVersion.dailyCalls,
+    riskAnalysisDocument:
+      purposeVersion.riskAnalysis &&
+      toBffApiPurposeVersionDocument(purposeVersion.riskAnalysis),
+    rejectionReason: purposeVersion.rejectionReason,
+  };
+}
diff --git a/packages/backend-for-frontend/src/app.ts b/packages/backend-for-frontend/src/app.ts
@@ -50,6 +50,8 @@ const redisRateLimiter = await initRedisRateLimiter({
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
+app.disable("etag");
+
 app.use(loggerMiddleware(serviceName));
 
 app.use(multerMiddleware);

diff --git a/packages/backend-for-frontend/src/routers/catalogRouter.ts b/packages/backend-for-frontend/src/routers/catalogRouter.ts
@@ -21,7 +21,10 @@ import { makeApiProblem } from "../model/errors.js";
 import { fromBffAppContext } from "../utilities/context.js";
 import {
   bffGetCatalogErrorMapper,
+  createEServiceDocumentErrorMapper,
   emptyErrorMapper,
+  exportEServiceDescriptorErrorMapper,
+  importEServiceErrorMapper,
 } from "../utilities/errorMappers.js";
 import { config } from "../config/config.js";
 import { toEserviceCatalogProcessQueryParams } from "../api/catalogApiConverter.js";
@@ -361,7 +364,7 @@ const catalogRouter = (
         } catch (error) {
           const errorRes = makeApiProblem(
             error,
-            emptyErrorMapper,
+            createEServiceDocumentErrorMapper,
             ctx.logger,
             `Error creating eService document of kind ${req.body.kind} and name ${req.body.prettyName} for eService ${req.params.eServiceId} and descriptor ${req.params.descriptorId}`
           );
@@ -663,7 +666,7 @@ const catalogRouter = (
         } catch (error) {
           const errorRes = makeApiProblem(
             error,
-            bffGetCatalogErrorMapper,
+            exportEServiceDescriptorErrorMapper,
             ctx.logger,
             `Error exporting eservice ${req.params.eserviceId} with descriptor ${req.params.descriptorId}`
           );
@@ -705,7 +708,7 @@ const catalogRouter = (
       } catch (error) {
         const errorRes = makeApiProblem(
           error,
-          emptyErrorMapper,
+          importEServiceErrorMapper,
           ctx.logger,
           "Error importing eService"
         );

diff --git a/packages/backend-for-frontend/src/routers/producerKeychainRouter.ts b/packages/backend-for-frontend/src/routers/producerKeychainRouter.ts
@@ -82,7 +82,9 @@ const producerKeychainRouter = (
           error,
           emptyErrorMapper,
           ctx.logger,
-          `Error creating producer keychain with seed: ${JSON.stringify(req)}`
+          `Error creating producer keychain with seed: ${JSON.stringify(
+            req.body
+          )}`
         );
         return res.status(errorRes.status).send(errorRes);
       }