chore: refactoring (#14)

.gitignore

@@ -6,4 +6,4 @@ coverage
 .vscode
 .idea/
 *.pem
-
+.npmrc

src/example.ts

@@ -17,7 +17,6 @@ import {
   LogoutT,
   withSpid
 } from ".";
-import { overrideCacheProvider } from "./strategy/redis_cache_provider";
 import { logger } from "./utils/logger";
 import { IServiceProviderConfig } from "./utils/middleware";
 
@@ -85,7 +84,6 @@ const samlConfig: SamlConfig = {
   acceptedClockSkewMs: 0,
   attributeConsumingServiceIndex: "0",
   authnContext: "https://www.spid.gov.it/SpidL1",
-  cacheProvider: overrideCacheProvider(),
   callbackUrl: "http://localhost:3000" + appConfig.assertionConsumerServicePath,
   // decryptionPvk: fs.readFileSync("./certs/key.pem", "utf-8"),
   identifierFormat: "urn:oasis:names:tc:SAML:2.0:nameid-format:transient",
@@ -112,24 +110,37 @@ app.use(passport.initialize());
 withSpid(
   appConfig,
   samlConfig,
-  redisClient,
   serviceProviderConfig,
+  redisClient,
   app,
   acs,
   logout
 )
-  .map(withSpidApp => {
-    withSpidApp.get("/success", (_, res) => res.json({ success: "success" }));
+  .map(({ app: withSpidApp, startIdpMetadataRefreshTimer }) => {
+    const idpMetadataRefreshTimer = startIdpMetadataRefreshTimer();
+    withSpidApp.on("server:stop", () => clearInterval(idpMetadataRefreshTimer));
+    withSpidApp.get("/success", (_, res) =>
+      res.json({
+        success: "success"
+      })
+    );
     withSpidApp.get("/error", (_, res) =>
-      res.json({ error: "error" }).status(400)
+      res
+        .json({
+          error: "error"
+        })
+        .status(400)
     );
     withSpidApp.use(
       (
         error: Error,
         _: express.Request,
         res: express.Response,
         ___: express.NextFunction
-      ) => res.status(505).send({ error: error.message })
+      ) =>
+        res.status(505).send({
+          error: error.message
+        })
     );
     withSpidApp.listen(3000);
   })

src/index.ts

@@ -20,7 +20,7 @@ import * as passport from "passport";
 import { SamlConfig } from "passport-saml";
 import { RedisClient } from "redis";
 import { Builder } from "xml2js";
-import { overrideCacheProvider } from "./strategy/redis_cache_provider";
+import { noopCacheProvider } from "./strategy/redis_cache_provider";
 import { logger } from "./utils/logger";
 import {
   getSpidStrategyOptionsUpdater,
@@ -59,9 +59,7 @@ export interface IApplicationConfig {
 }
 
 // re-export
-export { overrideCacheProvider };
-export { IServiceProviderConfig };
-export { SamlConfig };
+export { noopCacheProvider, IServiceProviderConfig, SamlConfig };
 
 /**
  * Wraps assertion consumer service handler
@@ -118,12 +116,15 @@ const withSpidAuthMiddleware = (
 export function withSpid(
   appConfig: IApplicationConfig,
   samlConfig: SamlConfig,
-  redisClient: RedisClient,
   serviceProviderConfig: IServiceProviderConfig,
+  redisClient: RedisClient,
   app: express.Express,
   acs: AssertionConsumerServiceT,
   logout: LogoutT
-): TaskEither<Error, express.Express> {
+): TaskEither<
+  Error,
+  { app: express.Express; startIdpMetadataRefreshTimer: () => NodeJS.Timeout }
+> {
   const loadSpidStrategyOptions = getSpidStrategyOptionsUpdater(
     samlConfig,
     serviceProviderConfig
@@ -156,19 +157,20 @@ export function withSpid(
     .map(spidStrategy => {
       // Schedule get and refresh
       // SPID passport strategy options
-      const idpMetadataRefreshTimer = setInterval(
-        () =>
-          loadSpidStrategyOptions()
-            .map(opts => setSpidStrategyOption(app, opts))
-            .run()
-            .catch(e => {
-              logger.error("loadSpidStrategyOptions|error:%s", e);
-            }),
-        serviceProviderConfig.idpMetadataRefreshIntervalMillis
-      );
-
-      // Avoid hanging when express server exits
-      app.on("server:stop", () => clearInterval(idpMetadataRefreshTimer));
+      const startIdpMetadataRefreshTimer = () =>
+        // Remember to call
+        // app.on("server:stop", () => clearInterval(idpMetadataRefreshTimer));
+        // to avoid hanging when express server exits
+        setInterval(
+          () =>
+            loadSpidStrategyOptions()
+              .map(opts => setSpidStrategyOption(app, opts))
+              .run()
+              .catch(e => {
+                logger.error("loadSpidStrategyOptions|error:%s", e);
+              }),
+          serviceProviderConfig.idpMetadataRefreshIntervalMillis
+        );
 
       // Initializes SpidStrategy for passport
       passport.use("spid", spidStrategy);
@@ -225,6 +227,6 @@ export function withSpid(
       // Setup logout handler
       app.post(appConfig.sloPath, toExpressHandler(logout));
 
-      return app;
+      return { app, startIdpMetadataRefreshTimer };
     });
 }

src/strategy/redis_cache_provider.ts

@@ -27,7 +27,7 @@ export interface IExtendedCacheProvider {
 // those methods must never fail since there's
 // practically no error handling in passport-saml
 // (a very bad lot of spaghetti code)
-export const overrideCacheProvider = (): CacheProvider => {
+export const noopCacheProvider = (): CacheProvider => {
   return {
     // saves the key with the optional value
     // invokes the callback with the value saved

src/strategy/spid.ts

@@ -14,7 +14,8 @@ import { RedisClient } from "redis";
 import { MultiSamlConfig } from "passport-saml/multiSamlStrategy";
 import {
   getExtendedRedisCacheProvider,
-  IExtendedCacheProvider
+  IExtendedCacheProvider,
+  noopCacheProvider
 } from "./redis_cache_provider";
 import { CustomSamlClient } from "./saml_client";
 
@@ -56,20 +57,15 @@ export class SpidStrategy extends SamlStrategy {
       // 8 hours
       options.requestIdExpirationPeriodMs = 28800000;
     }
+
+    // use our custom cache provider
     this.extendedRedisCacheProvider = getExtendedRedisCacheProvider(
       this.redisClient,
       options.requestIdExpirationPeriodMs
     );
-    if (!options.cacheProvider) {
-      // WARNING: you cannot use this one if you have
-      // multiple instances of the express app running
-      // (ie. multiple pods on Kubernetes).
-      // Use a RedisCacheProvider instead which can be
-      // safely shared between instances.
-      options.cacheProvider = new InMemoryCacheProvider({
-        keyExpirationPeriodMs: options.requestIdExpirationPeriodMs
-      });
-    }
+
+    // bypass passport-saml cache provider
+    options.cacheProvider = noopCacheProvider();
   }
 
   public authenticate(