IMN-879 - Refactoring headers parsing and improving auth (#1079)

packages/api-gateway/src/app.ts

@@ -31,7 +31,7 @@ const redisRateLimiter = await initRedisRateLimiter({
 // See https://cheatsheetseries.owasp.org/cheatsheets/HTTP_Headers_Cheat_Sheet.html#recommendation_16
 app.disable("x-powered-by");
 
-app.use(contextMiddleware(serviceName, true));
+app.use(contextMiddleware(serviceName, false));
 
 app.use(
   `/api-gateway/${config.apiGatewayInterfaceVersion}`,

packages/backend-for-frontend/src/app.ts

@@ -53,16 +53,15 @@ app.disable("x-powered-by");
 
 app.disable("etag");
 
-app.use(loggerMiddleware(serviceName));
-
 // parse files from multipart/form-data and put them in req.body
 app.use(multerMiddleware);
 app.use(fromFilesToBodyMiddleware);
 
 // parse application/x-www-form-urlencoded and put it in req.body
 app.use(express.urlencoded({ extended: true }));
 
-app.use(contextMiddleware(serviceName, true));
+app.use(contextMiddleware(serviceName, false));
+app.use(loggerMiddleware(serviceName));
 
 app.use(
   `/backend-for-frontend/${config.backendForFrontendInterfaceVersion}`,

packages/commons/src/auth/authenticationMiddleware.ts

@@ -2,15 +2,17 @@
 import { ZodiosRouterContextRequestHandler } from "@zodios/express";
 import {
   makeApiProblemBuilder,
-  missingBearer,
-  missingHeader,
   unauthorizedError,
 } from "pagopa-interop-models";
-import { match, P } from "ts-pattern";
-import { ExpressContext, getJwksClients, JWTConfig } from "../index.js";
-import { Logger, logger } from "../logging/index.js";
+import { match } from "ts-pattern";
+import {
+  ExpressContext,
+  fromAppContext,
+  getJwksClients,
+  JWTConfig,
+  jwtFromAuthHeader,
+} from "../index.js";
 import { AuthData } from "./authData.js";
-import { Headers } from "./headers.js";
 import { readAuthDataFromJwtToken, verifyJwtToken } from "./jwt.js";
 
 const makeApiProblem = makeApiProblemBuilder({});
@@ -20,90 +22,38 @@ export const authenticationMiddleware: (
 ) => ZodiosRouterContextRequestHandler<ExpressContext> =
   (config: JWTConfig) =>
   async (req, res, next): Promise<unknown> => {
-    const jwksClients = getJwksClients(config);
-
-    const addCtxAuthData = async (
-      authHeader: string,
-      logger: Logger
-    ): Promise<void> => {
-      const authorizationHeader = authHeader.split(" ");
-      if (
-        authorizationHeader.length !== 2 ||
-        authorizationHeader[0] !== "Bearer"
-      ) {
-        logger.warn(
-          `No authentication has been provided for this call ${req.method} ${req.url}`
-        );
-        throw missingBearer;
-      }
+    // We assume that:
+    // - contextMiddleware already set ctx.serviceName and ctx.correlationId
+    const ctx = fromAppContext(req.ctx);
 
-      const jwtToken = authorizationHeader[1];
-      const valid = await verifyJwtToken(jwtToken, jwksClients, config, logger);
+    try {
+      const jwksClients = getJwksClients(config);
+
+      const jwtToken = jwtFromAuthHeader(req, ctx.logger);
+      const valid = await verifyJwtToken(
+        jwtToken,
+        jwksClients,
+        config,
+        ctx.logger
+      );
       if (!valid) {
         throw unauthorizedError("Invalid token");
       }
 
-      const authData: AuthData = readAuthDataFromJwtToken(jwtToken, logger);
+      const authData: AuthData = readAuthDataFromJwtToken(jwtToken, ctx.logger);
       // eslint-disable-next-line functional/immutable-data
       req.ctx.authData = authData;
-      next();
-    };
-
-    const loggerInstance = logger({
-      serviceName: req.ctx.serviceName,
-      correlationId: req.ctx.correlationId,
-    });
-
-    try {
-      const headers = Headers.safeParse(req.headers);
-      if (!headers.success) {
-        throw missingHeader();
-      }
-
-      return await match(headers.data)
-        .with(
-          {
-            authorization: P.string,
-            "x-correlation-id": P.string,
-          },
-          async (headers) =>
-            await addCtxAuthData(headers.authorization, loggerInstance)
-        )
-        .with(
-          {
-            authorization: P.nullish,
-            "x-correlation-id": P._,
-          },
-          () => {
-            loggerInstance.warn(
-              `No authentication has been provided for this call ${req.method} ${req.url}`
-            );
-
-            throw missingBearer;
-          }
-        )
-        .with(
-          {
-            authorization: P.string,
-            "x-correlation-id": P.nullish,
-          },
-          () => {
-            throw missingHeader("x-correlation-id");
-          }
-        )
-        .otherwise(() => {
-          throw missingHeader();
-        });
+      return next();
     } catch (error) {
       const problem = makeApiProblem(
         error,
         (err) =>
           match(err.code)
             .with("unauthorizedError", () => 401)
             .with("operationForbidden", () => 403)
-            .with("missingHeader", () => 400)
+            .with("missingHeader", "badBearerToken", () => 400)
             .otherwise(() => 500),
-        loggerInstance
+        ctx.logger
       );
       return res.status(problem.status).send(problem);
     }

packages/commons/src/auth/authorizationMiddleware.ts

@@ -3,22 +3,18 @@ import {
   ZodiosEndpointDefinition,
   Method,
 } from "@zodios/core";
-import { Request } from "express";
 import {
   Problem,
   makeApiProblemBuilder,
   genericError,
   ApiError,
   unauthorizedError,
   CommonErrorCodes,
-  missingBearer,
 } from "pagopa-interop-models";
 import { P, match } from "ts-pattern";
 import { z } from "zod";
 import { Middleware } from "../types/middleware.js";
-import { UserRole, readHeaders } from "../index.js";
-import { genericLogger, logger } from "../logging/index.js";
-import { readAuthDataFromJwtToken } from "./jwt.js";
+import { AuthData, fromAppContext, UserRole } from "../index.js";
 
 type RoleValidation =
   | {
@@ -28,15 +24,9 @@ type RoleValidation =
   | { isValid: true };
 
 const hasValidRoles = (
-  req: Request,
+  authData: AuthData,
   admittedRoles: UserRole[]
 ): RoleValidation => {
-  const jwtToken = req.headers.authorization?.split(" ")[1];
-  if (!jwtToken) {
-    throw missingBearer;
-  }
-
-  const authData = readAuthDataFromJwtToken(jwtToken, genericLogger);
   if (!authData.userRoles || authData.userRoles.length === 0) {
     return {
       isValid: false,
@@ -77,33 +67,30 @@ export const authorizationMiddleware =
     admittedRoles: UserRole[]
   ): Middleware<Api, M, Path, Context> =>
   (req, res, next) => {
+    // We assume that:
+    // - contextMiddleware already set ctx.serviceName and ctx.correlationId
+    // - authorizationMiddleware already validated the token and set ctx.authData
+    const ctx = fromAppContext(req.ctx);
+
     try {
-      const validationResult = hasValidRoles(req as Request, admittedRoles);
+      const validationResult = hasValidRoles(ctx.authData, admittedRoles);
       if (!validationResult.isValid) {
         throw validationResult.error;
       }
 
       return next();
     } catch (err) {
-      const headers = readHeaders(req as Request);
-
-      const loggerInstance = logger({
-        userId: headers?.userId,
-        organizationId: headers?.organizationId,
-        correlationId: headers?.correlationId,
-      });
-
       const problem = match<unknown, Problem>(err)
         .with(P.instanceOf(ApiError), (error) =>
           makeApiProblem(
             new ApiError({
               code: error.code,
               detail: error.detail,
               title: error.title,
-              correlationId: headers?.correlationId,
+              correlationId: ctx.correlationId,
             }),
             (error) => (error.code === "unauthorizedError" ? 403 : 500),
-            loggerInstance
+            ctx.logger
           )
         )
         .otherwise(() =>
@@ -112,7 +99,7 @@ export const authorizationMiddleware =
               "An unexpected error occurred during authorization checks"
             ),
             () => 500,
-            loggerInstance
+            ctx.logger
           )
         );
 

packages/commons/src/auth/headers.ts

@@ -1,61 +1,40 @@
 import { Request } from "express";
-import { P, match } from "ts-pattern";
+import { badBearerToken, missingHeader } from "pagopa-interop-models";
 import { z } from "zod";
-import { genericLogger } from "../logging/index.js";
-import { AuthData } from "./authData.js";
-import { readAuthDataFromJwtToken } from "./jwt.js";
+import { Logger } from "../logging/index.js";
 
-export const Headers = z.object({
-  authorization: z.string().nullish(),
-  "x-correlation-id": z.string().nullish(),
-});
+export function parseCorrelationIdHeader(req: Request): string | undefined {
+  const parsed = z
+    .object({ "x-correlation-id": z.string() })
+    .safeParse(req.headers);
 
-export type Headers = z.infer<typeof Headers>;
-
-export const ParsedHeaders = z
-  .object({
-    correlationId: z.string(),
-  })
-  .and(AuthData);
-export type ParsedHeaders = z.infer<typeof ParsedHeaders>;
-
-export const readCorrelationIdHeader = (req: Request): string | undefined =>
-  match(req.headers)
-    .with(
-      { "x-correlation-id": P.string },
-      (headers) => headers["x-correlation-id"]
-    )
-    .otherwise(() => undefined);
+  if (parsed.success) {
+    return parsed.data["x-correlation-id"];
+  }
+  return undefined;
+}
+export function parseAuthHeader(req: Request): string | undefined {
+  const parsed = z.object({ authorization: z.string() }).safeParse(req.headers);
 
-export const readHeaders = (req: Request): ParsedHeaders | undefined => {
-  try {
-    const headers = Headers.parse(req.headers);
-    return match(headers)
-      .with(
-        {
-          authorization: P.string,
-          "x-correlation-id": P.string,
-        },
-        (headers) => {
-          const authorizationHeader = headers.authorization.split(" ");
-          if (
-            authorizationHeader.length !== 2 ||
-            authorizationHeader[0] !== "Bearer"
-          ) {
-            return undefined;
-          }
+  if (parsed.success) {
+    return parsed.data.authorization;
+  }
+  return undefined;
+}
 
-          const jwtToken = authorizationHeader[1];
-          const authData = readAuthDataFromJwtToken(jwtToken, genericLogger);
+export function jwtFromAuthHeader(req: Request, logger: Logger): string {
+  const authHeader = parseAuthHeader(req);
+  if (!authHeader) {
+    throw missingHeader("Authorization");
+  }
 
-          return {
-            ...authData,
-            correlationId: headers["x-correlation-id"],
-          };
-        }
-      )
-      .otherwise(() => undefined);
-  } catch (error) {
-    return undefined;
+  const authHeaderParts = authHeader.split(" ");
+  if (authHeaderParts.length !== 2 || authHeaderParts[0] !== "Bearer") {
+    logger.warn(
+      `Invalid authentication provided for this call ${req.method} ${req.url}`
+    );
+    throw badBearerToken;
   }
-};
+
+  return authHeaderParts[1];
+}

packages/commons/src/auth/jwt.ts

@@ -78,12 +78,12 @@ export const verifyJwtToken = async (
     const jwtHeader = decodeJwtTokenHeaders(jwtToken, logger);
     if (!jwtHeader?.kid) {
       logger.warn("Token verification failed: missing kid");
-      return Promise.reject(false);
+      return Promise.resolve(false);
     }
 
     const secret: Secret = await getKey(jwksClients, jwtHeader.kid, logger);
 
-    return new Promise((resolve, _reject) => {
+    return new Promise((resolve) => {
       jwt.verify(
         jwtToken,
         secret,
@@ -101,7 +101,7 @@ export const verifyJwtToken = async (
     });
   } catch (error) {
     logger.error(`Error verifying JWT token: ${error}`);
-    return Promise.reject(false);
+    return Promise.resolve(false);
   }
 };