Allow to disable session background refresh (#3473)

* Allow to disable session background refresh In a server-side scenario, one may not want sessions to proactively refresh to keep their access token active. Instead, the session is logged in using the refresh token when loaded from storage, but it will only be active during the lifetime of its Access Token. When building a Session, a new option "keepAlive" is supported. It defaults to 'true' for backwards compatibility. If set to 'false', the refresh token of the session will not be used to proactively refresh the access token. `keepAlive` is persisted in storage with the session state so that it is used after the authorization request during the token request in the authorization code flow. --------- Co-authored-by: Pete Edwards <[email protected]> Co-authored-by: Jarlath Holleran <[email protected]>
inrupt · May 3, 2024 · a158ff4 · a158ff4
1 parent 8b7283c
commit a158ff4
Show file tree

Hide file tree

Showing 29 changed files with 299 additions and 106 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,16 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 
 The following changes have been implemented but not released yet:
 
+### New Feature
+
+#### node
+
+- It is now possible to prevent a `Session` self-refreshing in NodeJS. To do so, a new
+  parameter is added to the constructor: `Session({ keepAlive: false })`. This prevents
+  the `Session` setting a callback to refresh the Access Token before it expires, which
+  could cause a memory leak in the case of a server-side application with many users.
+  It also avoids unnecessary requests being sent to the OpenID Provider.
+
 ## [2.1.0](https://github.com/inrupt/solid-client-authn-js/releases/tag/v2.1.0) - 2024-03-13
 
 ### New Feature

diff --git a/e2e/node/server/e2e-app-test.spec.ts b/e2e/node/server/e2e-app-test.spec.ts
@@ -56,20 +56,88 @@ if (process.env.CI === "true") {
 const ENV = getNodeTestingEnvironment();
 const BROWSER_ENV = getBrowserTestingEnvironment();
 
-describe("Testing against express app", () => {
+async function performTest(seedInfo: ISeedPodResponse) {
+  const browser = await firefox.launch();
+  const page = await browser.newPage();
+  const url = new URL(
+    `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/login`,
+  );
+  url.searchParams.append("oidcIssuer", ENV.idp);
+  url.searchParams.append("clientId", seedInfo.clientId);
+
+  await page.goto(url.toString());
+
+  // Wait for navigation outside the localhost session
+  await page.waitForURL(/^https/);
+  const cognitoPageUrl = page.url();
+
+  const cognitoPage = new CognitoPage(page);
+  await cognitoPage.login(
+    BROWSER_ENV.clientCredentials.owner.login,
+    BROWSER_ENV.clientCredentials.owner.password,
+  );
+  const openidPage = new OpenIdPage(page);
+  try {
+    await openidPage.allow();
+  } catch (e) {
+    // Ignore allow error for now
+  }
+
+  await page.waitForURL(
+    `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/`,
+  );
+
+  // Fetching a protected resource once logged in
+  const resourceUrl = new URL(
+    `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/fetch`,
+  );
+  resourceUrl.searchParams.append("resource", seedInfo.clientResourceUrl);
+  await page.goto(resourceUrl.toString());
+  await expect(page.content()).resolves.toBe(
+    `<html><head></head><body>${seedInfo.clientResourceContent}</body></html>`,
+  );
+
+  // Performing idp logout and being redirected to the postLogoutUrl after doing so
+  await page.goto(
+    `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/idplogout`,
+  );
+  await page.waitForURL(
+    `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/postLogoutUrl`,
+  );
+  await expect(page.content()).resolves.toBe(
+    `<html><head></head><body>successfully at post logout</body></html>`,
+  );
+
+  // Should not be able to retrieve the protected resource after logout
+  await page.goto(resourceUrl.toString());
+  await expect(page.content()).resolves.toMatch("Unauthorized");
+
+  // Testing what happens if we try to log back in again after logging out
+  await page.goto(url.toString());
+
+  // It should go back to the cognito page when we try to log back in
+  // rather than skipping straight to the consent page
+  await page.waitForURL((navigationUrl) => {
+    const u1 = new URL(navigationUrl);
+    u1.searchParams.delete("state");
+
+    const u2 = new URL(cognitoPageUrl);
+    u2.searchParams.delete("state");
+
+    return u1.toString() === u2.toString();
+  });
+
+  await browser.close();
+}
+
+describe("Testing against express app with default session", () => {
   let app: Server;
   let seedInfo: ISeedPodResponse;
-  let clientId: string;
-  let clientResourceUrl: string;
-  let clientResourceContent: string;
 
   beforeEach(async () => {
     seedInfo = await seedPod(ENV);
-    clientId = seedInfo.clientId;
-    clientResourceUrl = seedInfo.clientResourceUrl;
-    clientResourceContent = seedInfo.clientResourceContent;
     await new Promise<void>((res) => {
-      app = createApp(res);
+      app = createApp(res, { keepAlive: true });
     });
   }, 30_000);
 
@@ -81,76 +149,29 @@ describe("Testing against express app", () => {
   }, 30_000);
 
   it("Should be able to properly login and out with idp logout", async () => {
-    const browser = await firefox.launch();
-    const page = await browser.newPage();
-    const url = new URL(
-      `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/login`,
-    );
-    url.searchParams.append("oidcIssuer", ENV.idp);
-    url.searchParams.append("clientId", clientId);
-
-    await page.goto(url.toString());
-
-    // Wait for navigation outside the localhost session
-    await page.waitForURL(/^https/);
-    const cognitoPageUrl = page.url();
-
-    const cognitoPage = new CognitoPage(page);
-    await cognitoPage.login(
-      BROWSER_ENV.clientCredentials.owner.login,
-      BROWSER_ENV.clientCredentials.owner.password,
-    );
-    const openidPage = new OpenIdPage(page);
-    try {
-      await openidPage.allow();
-    } catch (e) {
-      // Ignore allow error for now
-    }
-
-    await page.waitForURL(
-      `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/`,
-    );
-
-    // Fetching a protected resource once logged in
-    const resourceUrl = new URL(
-      `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/fetch`,
-    );
-    resourceUrl.searchParams.append("resource", clientResourceUrl);
-    await page.goto(resourceUrl.toString());
-    await expect(page.content()).resolves.toBe(
-      `<html><head></head><body>${clientResourceContent}</body></html>`,
-    );
-
-    // Performing idp logout and being redirected to the postLogoutUrl after doing so
-    await page.goto(
-      `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/idplogout`,
-    );
-    await page.waitForURL(
-      `http://localhost:${CONSTANTS.CLIENT_AUTHN_TEST_PORT}/postLogoutUrl`,
-    );
-    await expect(page.content()).resolves.toBe(
-      `<html><head></head><body>successfully at post logout</body></html>`,
-    );
-
-    // Should not be able to retrieve the protected resource after logout
-    await page.goto(resourceUrl.toString());
-    await expect(page.content()).resolves.toMatch("Unauthorized");
-
-    // Testing what happens if we try to log back in again after logging out
-    await page.goto(url.toString());
-
-    // It should go back to the cognito page when we try to log back in
-    // rather than skipping straight to the consent page
-    await page.waitForURL((navigationUrl) => {
-      const u1 = new URL(navigationUrl);
-      u1.searchParams.delete("state");
-
-      const u2 = new URL(cognitoPageUrl);
-      u2.searchParams.delete("state");
-
-      return u1.toString() === u2.toString();
+    await performTest(seedInfo);
+  }, 120_000);
+});
+
+describe("Testing against express app with session keep alive off", () => {
+  let app: Server;
+  let seedInfo: ISeedPodResponse;
+
+  beforeEach(async () => {
+    seedInfo = await seedPod(ENV);
+    await new Promise<void>((res) => {
+      app = createApp(res, { keepAlive: false });
+    });
+  }, 30_000);
+
+  afterEach(async () => {
+    await tearDownPod(seedInfo);
+    await new Promise<void>((res) => {
+      app.close(() => res());
     });
+  }, 30_000);
 
-    await browser.close();
+  it("Should be able to properly login and out with idp logout", async () => {
+    await performTest(seedInfo);
   }, 120_000);
 });
diff --git a/e2e/node/server/express.ts b/e2e/node/server/express.ts
@@ -22,14 +22,19 @@ import log from "loglevel";
 import express from "express";
 // Here we want to test how the local code behaves, not the already published one.
 // eslint-disable-next-line import/no-relative-packages
+import type { ISessionOptions } from "../../../packages/node/src/index";
+// eslint-disable-next-line import/no-relative-packages
 import { Session } from "../../../packages/node/src/index";
 // Extensions are required for JSON-LD imports.
 // eslint-disable-next-line import/extensions
 import CONSTANTS from "../../../playwright.client-authn.constants.json";
 
 log.setLevel("TRACE");
 
-export function createApp(onStart: () => void) {
+export function createApp(
+  onStart: (value: PromiseLike<void> | void) => void,
+  sessionOptions: Partial<ISessionOptions> = {},
+) {
   const app = express();
 
   // Initialised when the server comes up and is running...
@@ -111,7 +116,7 @@ export function createApp(onStart: () => void) {
   });
 
   return app.listen(CONSTANTS.CLIENT_AUTHN_TEST_PORT, async () => {
-    session = new Session();
+    session = new Session(sessionOptions);
 
     onStart();
   });

diff --git a/package.json b/package.json
@@ -28,7 +28,7 @@
     "test:unit:node": "jest --coverage --verbose --selectProjects node",
     "test:unit:browser": "jest --coverage --verbose --selectProjects browser",
     "test:unit:core": "jest --coverage --verbose --selectProjects core",
-    "test:unit:oidc-node": "jest --coverage --verbose --selectProjects oidc-node",
+    "test:unit:oidc-browser": "jest --coverage --verbose --selectProjects oidc-browser",
     "test:e2e:node:all": "jest --selectProjects e2e-node-script e2e-node-server --collectCoverage false",
     "test:e2e:node": "jest --selectProjects e2e-node-script --collectCoverage false",
     "test:e2e:node:script": "jest --selectProjects e2e-node-script --collectCoverage false",

diff --git a/packages/browser/src/ClientAuthentication.spec.ts b/packages/browser/src/ClientAuthentication.spec.ts
@@ -424,6 +424,7 @@ describe("ClientAuthentication", () => {
       expect(defaultMocks.redirectHandler.handle).toHaveBeenCalledWith(
         url,
         mockEmitter,
+        undefined,
       );
 
       // Calling the redirect handler should have updated the fetch.

diff --git a/packages/browser/src/ClientAuthentication.ts b/packages/browser/src/ClientAuthentication.ts
@@ -101,7 +101,11 @@ export default class ClientAuthentication extends ClientAuthenticationBase {
     eventEmitter: EventEmitter,
   ): Promise<ISessionInfo | undefined> => {
     try {
-      const redirectInfo = await this.redirectHandler.handle(url, eventEmitter);
+      const redirectInfo = await this.redirectHandler.handle(
+        url,
+        eventEmitter,
+        undefined,
+      );
       // The `FallbackRedirectHandler` directly returns the global `fetch` for
       // his value, so we should ensure it's bound to `window` rather than to
       // ClientAuthentication, to avoid the following error:

diff --git a/packages/browser/src/Session.ts b/packages/browser/src/Session.ts
@@ -115,7 +115,7 @@ function isLoggedIn(
 }
 
 /**
- * A {@link Session} object represents a user's session on an application. The session holds state, as it stores information enabling acces to private resources after login for instance.
+ * A {@link Session} object represents a user's session on an application. The session holds state, as it stores information enabling access to private resources after login for instance.
  */
 export class Session implements IHasSessionEventListener {
   /**

diff --git a/packages/core/src/Session.ts b/packages/core/src/Session.ts
@@ -0,0 +1,24 @@
+//
+// Copyright Inrupt Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal in
+// the Software without restriction, including without limitation the rights to use,
+// copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+// Software, and to permit persons to whom the Software is furnished to do so,
+// subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
+// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+// SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
+
+export type SessionConfig = {
+  keepAlive: boolean;
+};
diff --git a/packages/core/src/SessionEventListener.ts b/packages/core/src/SessionEventListener.ts
@@ -54,7 +54,7 @@ type NEW_REFRESH_TOKEN_ARGS = {
 };
 type FALLBACK_ARGS = {
   eventName: Parameters<InstanceType<typeof EventEmitter>["on"]>[0];
-  // Prevents from using a SessionEventEmitter as an aritrary EventEmitter.
+  // Prevents from using a SessionEventEmitter as an arbitrary EventEmitter.
   listener: never;
 };
 

diff --git a/packages/core/src/authenticatedFetch/fetchFactory.ts b/packages/core/src/authenticatedFetch/fetchFactory.ts
@@ -191,7 +191,7 @@ export async function buildAuthenticatedFetch(
         if (refreshToken !== undefined) {
           currentRefreshOptions.refreshToken = refreshToken;
         }
-        // Each time the access token is refreshed, we must plan fo the next
+        // Each time the access token is refreshed, we must plan for the next
         // refresh iteration.
         clearTimeout(latestTimeout);
         latestTimeout = setTimeout(

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -131,6 +131,8 @@ export {
   TokenEndpointResponse,
 } from "./login/oidc/refresh/ITokenRefresher";
 
+export type { SessionConfig } from "./Session";
+
 // Mocks.
 /**
  * @deprecated

diff --git a/packages/core/src/login/ILoginOptions.ts b/packages/core/src/login/ILoginOptions.ts
@@ -53,4 +53,6 @@ export default interface ILoginOptions extends ILoginInputOptions {
    * Event emitter enabling calling user-specified callbacks.
    */
   eventEmitter?: EventEmitter;
+
+  keepAlive?: boolean;
 }
diff --git a/packages/core/src/login/oidc/IIncomingRedirectHandler.ts b/packages/core/src/login/oidc/IIncomingRedirectHandler.ts
@@ -29,11 +29,16 @@ import type { EventEmitter } from "events";
 import type IHandleable from "../../util/handlerPattern/IHandleable";
 import type { ISessionInfo } from "../../sessionInfo/ISessionInfo";
 import type { IRpLogoutOptions } from "../../logout/ILogoutHandler";
+import type { SessionConfig } from "../../Session";
 
 export type IncomingRedirectResult = ISessionInfo & { fetch: typeof fetch } & {
   getLogoutUrl?: (options: IRpLogoutOptions) => string;
 };
-export type IncomingRedirectInput = [string, EventEmitter | undefined];
+export type IncomingRedirectInput = [
+  string,
+  EventEmitter | undefined,
+  SessionConfig | undefined,
+];
 
 /**
  * @hidden

diff --git a/packages/core/src/login/oidc/IOidcOptions.ts b/packages/core/src/login/oidc/IOidcOptions.ts
@@ -65,6 +65,11 @@ export interface IOidcOptions {
   redirectUrl?: string;
   handleRedirect?: (url: string) => unknown;
   eventEmitter?: EventEmitter;
+  /**
+   * Should the resulting session be refreshed in the background? This is persisted prior to redirection.
+   * Defaults to true.
+   */
+  keepAlive?: boolean;
 }
 
 export default IOidcOptions;
-Original file line number
+Diff line change
@@ Expand Up / @@ -115,7 +115,7 @@ function isLoggedIn( @@
     }
     /**
-     * A {@link Session} object represents a user's session on an application. The session holds state, as it stores information enabling acces to private resources after login for instance.
+     * A {@link Session} object represents a user's session on an application. The session holds state, as it stores information enabling access to private resources after login for instance.
      */
     export class Session implements IHasSessionEventListener {
       /**
@@ Expand Down @@