[WIP] fix: correctly respect client expiry

packages/browser/examples/demoClientApp/package.json

@@ -29,6 +29,7 @@
     "webpack-dev-server": "^4.7.3"
   },
   "dependencies": {
+    "@inrupt/solid-client-authn-browser": "file:../../",
     "react": "^17.0.2",
     "react-dom": "^17.0.2",
     "regenerator-runtime": "^0.13.9"

packages/browser/examples/demoClientApp/src/DemoClientApp.js

@@ -1,14 +1,10 @@
 import React, { Component } from "react";
 import "regenerator-runtime/runtime";
 
-// import {
-//   Session,
-//   getClientAuthenticationWithDependencies,
-// } from "@inrupt/solid-client-authn-browser";
 import {
   Session,
   getClientAuthenticationWithDependencies,
-} from "../../../dist/index";
+} from "@inrupt/solid-client-authn-browser";
 
 const clientApplicationName = "S-C-A Browser Demo Client App";
 let snackBarTimeout = undefined;
@@ -17,11 +13,11 @@ let identityProviderLogoutEndpointTimeout = null;
 const NSS_SERVER_URL = "https://inrupt.net/";
 
 const preconfiguedIdpList = [
-  "https://openid.dev-next.inrupt.com",
-  "https://broker.pod.inrupt.com",
-  "https://broker.dev-ess.inrupt.com",
-  "https://broker.demo-ess.inrupt.com",
-  "https://inrupt.net",
+  "https://openid.dev-next.inrupt.com/",
+  "https://broker.pod.inrupt.com/",
+  "https://broker.dev-ess.inrupt.com/",
+  "https://broker.demo-ess.inrupt.com/",
+  "https://inrupt.net/",
 ];
 
 const defaultIssuer = preconfiguedIdpList[1];

packages/browser/package.json

@@ -23,6 +23,7 @@
     "lint:prettier": "prettier \"src/*.{ts,tsx,js,jsx,css}\" \"**/*.{md,mdx,yml}\"",
     "licenses:check": "license-checker --production --out license.csv --failOn \"AGPL-1.0-only; AGPL-1.0-or-later; AGPL-3.0-only; AGPL-3.0-or-later; Beerware; CC-BY-NC-1.0; CC-BY-NC-2.0; CC-BY-NC-2.5; CC-BY-NC-3.0; CC-BY-NC-4.0; CC-BY-NC-ND-1.0; CC-BY-NC-ND-2.0; CC-BY-NC-ND-2.5; CC-BY-NC-ND-3.0; CC-BY-NC-ND-4.0; CC-BY-NC-SA-1.0; CC-BY-NC-SA-2.0; CC-BY-NC-SA-2.5; CC-BY-NC-SA-3.0; CC-BY-NC-SA-4.0; CPAL-1.0; EUPL-1.0; EUPL-1.1; EUPL-1.1;  GPL-1.0-only; GPL-1.0-or-later; GPL-2.0-only;  GPL-2.0-or-later; GPL-3.0; GPL-3.0-only; GPL-3.0-or-later; SISSL;  SISSL-1.2; WTFPL\"",
     "test": "jest --coverage --verbose",
+    "test:debug": "node --inspect-brk ../../node_modules/.bin/jest -i --coverage=false",
     "build-api-docs": "npx typedoc --out docs/api/source/api --readme none",
     "build-docs-preview-site": "npm run build-api-docs; cd docs/api; make html"
   },

packages/browser/src/ClientAuthentication.spec.ts

@@ -59,13 +59,15 @@ jest.mock("@inrupt/solid-client-authn-core", () => {
 
 type SessionStorageOptions = {
   clientId: string;
+  clientExpiresAt: number;
   issuer: string;
 };
 
 const mockSessionStorage = async (
   sessionId: string,
   options: SessionStorageOptions = {
     clientId: "https://some.app/registration",
+    clientExpiresAt: 0,
     issuer: "https://some.issuer",
   }
 ): Promise<StorageUtility> => {
@@ -79,6 +81,7 @@ const mockSessionStorage = async (
     mockStorage({
       [`${USER_SESSION_PREFIX}:${sessionId}`]: {
         clientId: options.clientId,
+        clientExpiresAt: options.clientExpiresAt,
         issuer: options.issuer,
       },
     })
@@ -291,6 +294,7 @@ describe("ClientAuthentication", () => {
         isLoggedIn: "true",
         sessionId: "mySession",
         webId: "https://pod.com/profile/card#me",
+        issuer: "https://some.issuer",
       };
       const clientAuthn = getClientAuthentication({
         sessionInfoManager: mockSessionInfoManager(
@@ -451,7 +455,8 @@ describe("ClientAuthentication", () => {
       ).resolves.toBeNull();
     });
 
-    it("returns null if the current session has no stored client ID", async () => {
+    // This is no longer handled in validateCurrentSession, and instead the client is dynamically registered:
+    it.skip("returns null if the current session has no stored client ID", async () => {
       const sessionId = "mySession";
       const mockedStorage = new StorageUtility(
         mockStorage({
@@ -477,6 +482,7 @@ describe("ClientAuthentication", () => {
       const sessionId = "mySession";
       const mockedStorage = await mockSessionStorage(sessionId, {
         clientId: "https://some.app/registration",
+        clientExpiresAt: 0,
         issuer: "https://some.issuer",
       });
 
@@ -489,7 +495,7 @@ describe("ClientAuthentication", () => {
       ).resolves.toStrictEqual(
         expect.objectContaining({
           issuer: "https://some.issuer",
-          clientAppId: "https://some.app/registration",
+          // clientAppId: "https://some.app/registration",
           sessionId,
           webId: "https://my.pod/profile#me",
         })

packages/browser/src/ClientAuthentication.ts

@@ -30,20 +30,38 @@ import {
   IIncomingRedirectHandler,
   ISessionInfo,
   ISessionInfoManager,
-  IIssuerConfigFetcher,
   ISessionInternalInfo,
-  ILoginOptions,
+  IPublicIdentifierClientOptions,
+  IDynamicClientOptions,
   EVENTS,
+  ClientManager,
+  isValidUrl,
 } from "@inrupt/solid-client-authn-core";
 import { removeOidcQueryParam } from "@inrupt/oidc-client-ext";
 import { EventEmitter } from "events";
 
+import { KEY_CURRENT_URL } from "./constant";
+
 // By only referring to `window` at runtime, apps that do server-side rendering
 // won't run into errors when rendering code that instantiates a
 // ClientAuthentication:
 const globalFetch: typeof window.fetch = (request, init) =>
   window.fetch(request, init);
 
+export type IClientOptions =
+  | IPublicIdentifierClientOptions
+  | IDynamicClientOptions;
+export type ILoginOptions = IClientOptions & {
+  sessionId: string;
+  oidcIssuer: string;
+  redirectUrl: string;
+  tokenType?: "DPoP" | "Bearer" = "DPoP";
+};
+
+export interface ISilentLoginOptions {
+  sessionId: string;
+}
+
 /**
  * @hidden
  */
@@ -53,39 +71,86 @@ export default class ClientAuthentication {
     private redirectHandler: IIncomingRedirectHandler,
     private logoutHandler: ILogoutHandler,
     private sessionInfoManager: ISessionInfoManager,
-    private issuerConfigFetcher: IIssuerConfigFetcher
+    private clientManager: ClientManager
   ) {}
 
   // Define these functions as properties so that they don't get accidentally re-bound.
   // Isn't Javascript fun?
+  // login = async (
+  //   options: ILoginOptions,
+  //   eventEmitter: EventEmitter
+  // ): Promise<void> => {
+  //   // In order to get a clean start, make sure that the session is logged out
+  //   // on login.
+  //   // But we may want to preserve our client application info, particularly if
+  //   // we used Dynamic Client Registration to register (since we don't
+  //   // necessarily want the user to have to register this app each time they
+  //   // login).
+  //   // FIXME: now preserves the client, but deletes the session info:
+  //   await this.sessionInfoManager.clear(options.sessionId);
+
+  //   // In the case of the user hitting the 'back' button in their browser, they
+  //   // could return to a previous redirect URL that contains OIDC params that
+  //   // are now longer valid - so just to be safe, strip relevant params now.
+  //   const redirectUrl = removeOidcQueryParam(
+  //     options.redirectUrl ?? window.location.href
+  //   );
+
+  //   await this.loginHandler.handle({
+  //     ...options,
+  //     redirectUrl,
+  //     // If no clientName is provided, the clientId may be used instead.
+  //     clientName: options.clientName ?? options.clientId,
+  //     eventEmitter,
+  //   });
+  // };
+
   login = async (
+    sessionId: string,
     options: ILoginOptions,
     eventEmitter: EventEmitter
   ): Promise<void> => {
-    // In order to get a clean start, make sure that the session is logged out
-    // on login.
-    // But we may want to preserve our client application info, particularly if
-    // we used Dynamic Client Registration to register (since we don't
-    // necessarily want the user to have to register this app each time they
-    // login).
-    await this.sessionInfoManager.clear(options.sessionId);
-
-    // In the case of the user hitting the 'back' button in their browser, they
-    // could return to a previous redirect URL that contains OIDC params that
-    // are now longer valid - so just to be safe, strip relevant params now.
-    const redirectUrl = removeOidcQueryParam(
-      options.redirectUrl ?? window.location.href
-    );
-
-    await this.loginHandler.handle({
-      ...options,
+    const { oidcIssuer, ...clientOptions } = options;
+
+    // TODO: Should we validate presence of oidcIssuer and redirectUrl here?
+
+    const redirectUrl =
+      options.redirectUrl && isValidUrl(options.redirectUrl)
+        ? options.redirectUrl
+        : window.location.href;
+
+    this.loginHandler.handle({
+      sessionId,
+      oidcIssuer,
       redirectUrl,
-      // If no clientName is provided, the clientId may be used instead.
-      clientName: options.clientName ?? options.clientId,
-      eventEmitter,
     });
   };
 
+  silentlyLogin = async (options: ISilentLoginOptions): Promise<boolean> => {
+    // FIXME: Implement
+    const sessionInfo = await this.sessionInfoManager.get(options.sessionId);
+
+    if (sessionInfo === undefined || sessionInfo.issuer === undefined) {
+      return false;
+    }
+
+    const client = await this.clientManager.get(sessionInfo.issuer);
+
+    if (!client) {
+      return false;
+    }
+
+    // FIXME: use storage adapter:
+    window.localStorage.setItem(KEY_CURRENT_URL, window.location.href);
+
+    // await this.loginHandler.handle({
+    //   oidcIssuer: sessionInfo.issuer,
+    //   redirectUrl: sessionInfo.redirectUrl,
+    // });
+
+    return false;
+  };
+
   // By default, our fetch() resolves to the environment fetch() function.
   fetch = globalFetch;
 
@@ -112,17 +177,16 @@ export default class ClientAuthentication {
   // if the expected information cannot be found.
   // Note that the ID token is not stored, which means the session information
   // cannot be validated at this point.
+  // FIXME: not actually part of the public interface:
   validateCurrentSession = async (
     currentSessionId: string
   ): Promise<(ISessionInfo & ISessionInternalInfo) | null> => {
     const sessionInfo = await this.sessionInfoManager.get(currentSessionId);
-    if (
-      sessionInfo === undefined ||
-      sessionInfo.clientAppId === undefined ||
-      sessionInfo.issuer === undefined
-    ) {
+
+    if (sessionInfo === undefined) {
       return null;
     }
+
     return sessionInfo;
   };
 
@@ -162,6 +226,7 @@ export default class ClientAuthentication {
     }
   };
 
+  // FIXME: technically should be the same as removeOidcQueryParam method, though isn't:
   private cleanUrlAfterRedirect(url: string): void {
     const cleanedUpUrl = new URL(url);
     cleanedUpUrl.searchParams.delete("state");

packages/browser/src/Session.ts

@@ -118,8 +118,6 @@ export async function silentlyAuthenticate(
         prompt: "none",
         oidcIssuer: storedSessionInfo.issuer,
         redirectUrl: storedSessionInfo.redirectUrl,
-        clientId: storedSessionInfo.clientAppId,
-        clientSecret: storedSessionInfo.clientAppSecret,
         tokenType: storedSessionInfo.tokenType ?? "DPoP",
         inIframe: options.inIframe,
       },
@@ -251,15 +249,16 @@ export class Session extends EventEmitter {
   // Define these functions as properties so that they don't get accidentally re-bound.
   // Isn't Javascript fun?
   login = async (options: ILoginInputOptions): Promise<void> => {
-    await this.clientAuthentication.login(
-      {
-        sessionId: this.info.sessionId,
-        ...options,
-        // Defaults the token type to DPoP
-        tokenType: options.tokenType ?? "DPoP",
-      },
-      this
-    );
+    // FIXME: implement
+    // await this.clientAuthentication.login(
+    //   {
+    //     sessionId: this.info.sessionId,
+    //     ...options,
+    //     // Defaults the token type to DPoP
+    //     tokenType: options.tokenType ?? "DPoP",
+    //   },
+    //   this
+    // );
     // `login` redirects the user away from the app,
     // so unless it throws an error, there is no code that should run afterwards
     // (since there is no "after" in the lifetime of the script).