PIN-5682 - Reverting #1131 to fix JWKS timeout (#1206)

pagopa · Nov 28, 2024 · f830541 · f830541
1 parent 2dd5194
commit f830541
Show file tree

Hide file tree

Showing 15 changed files with 29 additions and 78 deletions.
diff --git a/packages/agreement-process/src/app.ts b/packages/agreement-process/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   loggerMiddleware,
   zodiosCtx,
@@ -13,15 +12,13 @@ const serviceName = "agreement-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(loggerMiddleware(serviceName));
 app.use(agreementRouter(zodiosCtx));
 

diff --git a/packages/api-gateway/src/app.ts b/packages/api-gateway/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   initRedisRateLimiter,
   loggerMiddleware,
@@ -18,8 +17,6 @@ const clients = getInteropBeClients();
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 const redisRateLimiter = await initRedisRateLimiter({
   limiterGroup: "API_GW",
   maxRequests: config.rateLimiterMaxRequests,
@@ -40,7 +37,7 @@ app.use(
   `/api-gateway/${config.apiGatewayInterfaceVersion}`,
   healthRouter,
   contextMiddleware(serviceName, false),
-  authenticationMiddleware(config, jwksClients),
+  authenticationMiddleware(config),
   // Authenticated routes - rate limiter relies on auth data to work
   rateLimiterMiddleware(redisRateLimiter),
   apiGatewayRouter(zodiosCtx, clients)

diff --git a/packages/attribute-registry-process/src/app.ts b/packages/attribute-registry-process/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   loggerMiddleware,
   zodiosCtx,
@@ -13,15 +12,13 @@ const serviceName = "attribute-registry-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(loggerMiddleware(serviceName));
 app.use(attributeRouter(zodiosCtx));
 

diff --git a/packages/authorization-process/src/app.ts b/packages/authorization-process/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   zodiosCtx,
 } from "pagopa-interop-commons";
@@ -12,15 +11,13 @@ const serviceName = "authorization-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(authorizationRouter(zodiosCtx));
 
 export default app;
diff --git a/packages/backend-for-frontend/src/app.ts b/packages/backend-for-frontend/src/app.ts
@@ -6,7 +6,6 @@ import {
   zodiosCtx,
   initRedisRateLimiter,
   rateLimiterMiddleware,
-  buildJwksClients,
 } from "pagopa-interop-commons";
 import express from "express";
 import { config } from "./config/config.js";
@@ -40,8 +39,6 @@ const clients = getInteropBeClients();
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 const redisRateLimiter = await initRedisRateLimiter({
   limiterGroup: "BFF",
   maxRequests: config.rateLimiterMaxRequests,
@@ -71,22 +68,16 @@ app.use(
   `/backend-for-frontend/${config.backendForFrontendInterfaceVersion}`,
   healthRouter,
   contextMiddleware(serviceName, false),
-  authorizationRouter(
-    zodiosCtx,
-    clients,
-    allowList,
-    redisRateLimiter,
-    jwksClients
-  ),
-  authenticationMiddleware(config, jwksClients),
+  authorizationRouter(zodiosCtx, clients, allowList, redisRateLimiter),
+  authenticationMiddleware(config),
   // Authenticated routes - rate limiter relies on auth data to work
   rateLimiterMiddleware(redisRateLimiter),
   catalogRouter(zodiosCtx, clients, fileManager),
   attributeRouter(zodiosCtx, clients),
   purposeRouter(zodiosCtx, clients),
   agreementRouter(zodiosCtx, clients, fileManager),
   selfcareRouter(clients, zodiosCtx),
-  supportRouter(zodiosCtx, clients, redisRateLimiter, jwksClients),
+  supportRouter(zodiosCtx, clients, redisRateLimiter),
   toolRouter(zodiosCtx, clients),
   tenantRouter(zodiosCtx, clients),
   clientRouter(zodiosCtx, clients),

diff --git a/packages/backend-for-frontend/src/routers/authorizationRouter.ts b/packages/backend-for-frontend/src/routers/authorizationRouter.ts
@@ -8,7 +8,6 @@ import {
   zodiosValidationErrorToApiProblem,
   RateLimiter,
   rateLimiterHeadersFromStatus,
-  buildJwksClients,
 } from "pagopa-interop-commons";
 import { tooManyRequestsError } from "pagopa-interop-models";
 import { makeApiProblem } from "../model/errors.js";
@@ -22,8 +21,7 @@ const authorizationRouter = (
   ctx: ZodiosContext,
   { tenantProcessClient }: PagoPAInteropBeClients,
   allowList: string[],
-  rateLimiter: RateLimiter,
-  jwksClients: ReturnType<typeof buildJwksClients>
+  rateLimiter: RateLimiter
 ): ZodiosRouter<ZodiosEndpointDefinitions, ExpressContext> => {
   const authorizationRouter = ctx.router(bffApi.authorizationApi.api, {
     validationErrorHandler: zodiosValidationErrorToApiProblem,
@@ -34,8 +32,7 @@ const authorizationRouter = (
     interopTokenGenerator,
     tenantProcessClient,
     allowList,
-    rateLimiter,
-    jwksClients
+    rateLimiter
   );
 
   authorizationRouter

diff --git a/packages/backend-for-frontend/src/routers/supportRouter.ts b/packages/backend-for-frontend/src/routers/supportRouter.ts
@@ -1,7 +1,6 @@
 import { ZodiosEndpointDefinitions } from "@zodios/core";
 import { ZodiosRouter } from "@zodios/express";
 import {
-  buildJwksClients,
   ExpressContext,
   InteropTokenGenerator,
   RateLimiter,
@@ -20,8 +19,7 @@ import { fromBffAppContext } from "../utilities/context.js";
 const supportRouter = (
   ctx: ZodiosContext,
   { tenantProcessClient }: PagoPAInteropBeClients,
-  rateLimiter: RateLimiter,
-  jwksClients: ReturnType<typeof buildJwksClients>
+  rateLimiter: RateLimiter
 ): ZodiosRouter<ZodiosEndpointDefinitions, ExpressContext> => {
   const supportRouter = ctx.router(bffApi.supportApi.api, {
     validationErrorHandler: zodiosValidationErrorToApiProblem,
@@ -32,8 +30,7 @@ const supportRouter = (
     interopTokenGenerator,
     tenantProcessClient,
     config.tenantAllowedOrigins,
-    rateLimiter,
-    jwksClients
+    rateLimiter
   );
 
   supportRouter.post("/session/saml2/tokens", async (req, res) => {

diff --git a/packages/backend-for-frontend/src/services/authorizationService.ts b/packages/backend-for-frontend/src/services/authorizationService.ts
@@ -16,7 +16,6 @@ import {
   USER_ROLES,
   WithLogger,
   decodeJwtToken,
-  buildJwksClients,
   userRoles,
   verifyJwtToken,
 } from "pagopa-interop-commons";
@@ -52,8 +51,7 @@ export function authorizationServiceBuilder(
   interopTokenGenerator: InteropTokenGenerator,
   tenantProcessClient: PagoPAInteropBeClients["tenantProcessClient"],
   allowList: string[],
-  rateLimiter: RateLimiter,
-  jwksClients: ReturnType<typeof buildJwksClients>
+  rateLimiter: RateLimiter
 ) {
   const readJwt = async (
     identityToken: string,
@@ -63,12 +61,7 @@ export function authorizationServiceBuilder(
     sessionClaims: SessionClaims;
     selfcareId: string;
   }> => {
-    const verified = await verifyJwtToken(
-      identityToken,
-      jwksClients,
-      config,
-      logger
-    );
+    const verified = await verifyJwtToken(identityToken, config, logger);
     if (!verified) {
       throw tokenVerificationFailed();
     }

diff --git a/packages/catalog-process/src/app.ts b/packages/catalog-process/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   loggerMiddleware,
   zodiosCtx,
@@ -13,15 +12,13 @@ const serviceName = "catalog-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(loggerMiddleware(serviceName));
 app.use(eservicesRouter(zodiosCtx));
 

diff --git a/packages/commons/src/auth/authenticationMiddleware.ts b/packages/commons/src/auth/authenticationMiddleware.ts
@@ -5,7 +5,6 @@ import {
   unauthorizedError,
 } from "pagopa-interop-models";
 import { match } from "ts-pattern";
-import { JwksClient } from "jwks-rsa";
 import {
   ExpressContext,
   fromAppContext,
@@ -18,23 +17,17 @@ import { readAuthDataFromJwtToken, verifyJwtToken } from "./jwt.js";
 const makeApiProblem = makeApiProblemBuilder({});
 
 export const authenticationMiddleware: (
-  config: JWTConfig,
-  jwksClients: JwksClient[]
+  config: JWTConfig
 ) => ZodiosRouterContextRequestHandler<ExpressContext> =
-  (config: JWTConfig, jwksClients: JwksClient[]) =>
+  (config: JWTConfig) =>
   async (req, res, next): Promise<unknown> => {
     // We assume that:
     // - contextMiddleware already set ctx.serviceName and ctx.correlationId
     const ctx = fromAppContext(req.ctx);
 
     try {
       const jwtToken = jwtFromAuthHeader(req, ctx.logger);
-      const valid = await verifyJwtToken(
-        jwtToken,
-        jwksClients,
-        config,
-        ctx.logger
-      );
+      const valid = await verifyJwtToken(jwtToken, config, ctx.logger);
       if (!valid) {
         throw unauthorizedError("Invalid token");
       }

diff --git a/packages/commons/src/auth/jwk.ts b/packages/commons/src/auth/jwk.ts
@@ -70,13 +70,17 @@ export function sortJWK(jwk: JsonWebKey): JsonWebKey {
 export function buildJwksClients(config: JWTConfig): JwksClient[] {
   return config.wellKnownUrls.map((url) =>
     jwksClient({
-      cache: true,
-      rateLimit: true,
       jwksUri: url,
       /* If JWKS_CACHE_MAX_AGE_MILLIS not provided using 10 minutes as default value:
       https://github.com/auth0/node-jwks-rsa/blob/master/EXAMPLES.md#configuration
       */
-      cacheMaxAge: config.jwksCacheMaxAge ?? 600000,
+
+      // Caching is not being leveraged at the moment since we are building
+      // a new client for each request.
+      // Building clients only once at startup caused https://pagopa.atlassian.net/browse/PIN-5682
+      // cache: true,
+      // rateLimit: true,
+      // cacheMaxAge: config.jwksCacheMaxAge ?? 600000,
     })
   );
 }
diff --git a/packages/commons/src/auth/jwt.ts b/packages/commons/src/auth/jwt.ts
@@ -5,7 +5,7 @@ import {
   jwtDecodingError,
   jwksSigningKeyError,
 } from "pagopa-interop-models";
-import { JWTConfig, Logger } from "../index.js";
+import { buildJwksClients, JWTConfig, Logger } from "../index.js";
 import { AuthData, AuthToken, getAuthDataFromToken } from "./authData.js";
 
 export const decodeJwtToken = (
@@ -68,7 +68,6 @@ const getKey = async (
 
 export const verifyJwtToken = async (
   jwtToken: string,
-  jwksClients: jwksClient.JwksClient[],
   config: JWTConfig,
   logger: Logger
 ): Promise<boolean> => {
@@ -81,6 +80,7 @@ export const verifyJwtToken = async (
       return Promise.resolve(false);
     }
 
+    const jwksClients = buildJwksClients(config);
     const secret: Secret = await getKey(jwksClients, jwtHeader.kid, logger);
 
     return new Promise((resolve) => {

diff --git a/packages/delegation-process/src/app.ts b/packages/delegation-process/src/app.ts
@@ -3,7 +3,6 @@ import {
   contextMiddleware,
   loggerMiddleware,
   zodiosCtx,
-  buildJwksClients,
 } from "pagopa-interop-commons";
 
 import healthRouter from "./routers/HealthRouter.js";
@@ -15,15 +14,13 @@ const serviceName = "delegation-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(loggerMiddleware(serviceName));
 app.use(delegationRouter(zodiosCtx));
 app.use(delegationProducerRouter(zodiosCtx));

diff --git a/packages/purpose-process/src/app.ts b/packages/purpose-process/src/app.ts
@@ -1,6 +1,5 @@
 import {
   authenticationMiddleware,
-  buildJwksClients,
   contextMiddleware,
   loggerMiddleware,
   zodiosCtx,
@@ -13,15 +12,13 @@ const serviceName = "purpose-process";
 
 const app = zodiosCtx.app();
 
-const jwksClients = buildJwksClients(config);
-
 // Disable the "X-Powered-By: Express" HTTP header for security reasons.
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
 app.use(healthRouter);
 app.use(contextMiddleware(serviceName));
-app.use(authenticationMiddleware(config, jwksClients));
+app.use(authenticationMiddleware(config));
 app.use(loggerMiddleware(serviceName));
 app.use(purposeRouter(zodiosCtx));