Merge pull request #69 from kinde-oss/fix/refresh_insecure_token_storage

fix: insecure token storage when using no non custom domain

Author: DanielRivers

CHANGELOG.md

@@ -1,6 +1,60 @@
 # Changelog
 
 
+## 0.7.1...fix/refresh_insecure_token_storage
+
+[compare changes](https://github.com/kinde-oss/js-utils/compare/0.7.1...fix/refresh_insecure_token_storage)
+
+### 🩹 Fixes
+
+- Insecure token storage when using no non custom domain ([7338321](https://github.com/kinde-oss/js-utils/commit/7338321))
+- Only show production warning when explicity setting the 'useInsecureForRefreshToken' ([958f4ed](https://github.com/kinde-oss/js-utils/commit/958f4ed))
+- Non prod kinde domains ([24be712](https://github.com/kinde-oss/js-utils/commit/24be712))
+
+### ✅ Tests
+
+- Add tests ([979974d](https://github.com/kinde-oss/js-utils/commit/979974d))
+
+### ❤️ Contributors
+
+- Daniel Rivers ([@DanielRivers](http://github.com/DanielRivers))
+
+## 0.7.1...fix/refresh_insecure_token_storage
+
+[compare changes](https://github.com/kinde-oss/js-utils/compare/0.7.1...fix/refresh_insecure_token_storage)
+
+### 🩹 Fixes
+
+- Insecure token storage when using no non custom domain ([7338321](https://github.com/kinde-oss/js-utils/commit/7338321))
+- Only show production warning when explicity setting the 'useInsecureForRefreshToken' ([958f4ed](https://github.com/kinde-oss/js-utils/commit/958f4ed))
+- Non prod kinde domains ([24be712](https://github.com/kinde-oss/js-utils/commit/24be712))
+
+### ✅ Tests
+
+- Add tests ([979974d](https://github.com/kinde-oss/js-utils/commit/979974d))
+
+### ❤️ Contributors
+
+- Daniel Rivers ([@DanielRivers](http://github.com/DanielRivers))
+
+## 0.7.1...fix/refresh_insecure_token_storage
+
+[compare changes](https://github.com/kinde-oss/js-utils/compare/0.7.1...fix/refresh_insecure_token_storage)
+
+### 🩹 Fixes
+
+- Insecure token storage when using no non custom domain ([7338321](https://github.com/kinde-oss/js-utils/commit/7338321))
+- Only show production warning when explicity setting the 'useInsecureForRefreshToken' ([958f4ed](https://github.com/kinde-oss/js-utils/commit/958f4ed))
+- Non prod kinde domains ([24be712](https://github.com/kinde-oss/js-utils/commit/24be712))
+
+### ✅ Tests
+
+- Add tests ([979974d](https://github.com/kinde-oss/js-utils/commit/979974d))
+
+### ❤️ Contributors
+
+- Daniel Rivers ([@DanielRivers](http://github.com/DanielRivers))
+
 ## 0.7.0...main
 
 [compare changes](https://github.com/kinde-oss/js-utils/compare/0.7.0...main)

lib/sessionManager/stores/localStorage.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, beforeEach, vi } from "vitest";
 import { LocalStorage } from "./localStorage";
 import { StorageKeys } from "../types";
+import { storageSettings } from "..";
 
 enum ExtraKeys {
   testKey = "testKey2",
@@ -33,6 +34,22 @@ describe("LocalStorage standard keys", () => {
     sessionManager = new LocalStorage();
   });
 
+  it("should show warning when using local storage access token explicity", async () => {
+    storageSettings.useInsecureForRefreshToken = true;
+    const consoleSpy = vi.spyOn(console, "warn");
+    new LocalStorage();
+    expect(consoleSpy).toHaveBeenCalledWith(
+      "LocalStorage store should not be used in production",
+    );
+    storageSettings.useInsecureForRefreshToken = false;
+  });
+
+  it("should not show warning when using secure refresh tokens", async () => {
+    const consoleSpy = vi.spyOn(console, "warn");
+    new LocalStorage();
+    expect(consoleSpy).not.toHaveBeenCalled();
+  });
+
   it("should set and get an item in session storage", async () => {
     await sessionManager.setSessionItem(StorageKeys.accessToken, "testValue");
     expect(await sessionManager.getSessionItem(StorageKeys.accessToken)).toBe(

lib/sessionManager/stores/localStorage.ts

@@ -12,7 +12,9 @@ export class LocalStorage<V extends string = StorageKeys>
 {
   constructor() {
     super();
-    console.warn("LocalStorage store should not be used in production");
+    if (storageSettings.useInsecureForRefreshToken) {
+      console.warn("LocalStorage store should not be used in production");
+    }
   }
 
   private internalItems: Set<V | StorageKeys> = new Set<V>();

lib/utils/exchangeAuthCode.ts

@@ -26,13 +26,25 @@ interface ExchangeAuthCodeParams {
   autoRefresh?: boolean;
 }
 
-interface ExchangeAuthCodeResult {
-  success: boolean;
-  error?: string;
+type ExchangeAuthCodeResultSuccess = {
+  success: true;
+  error?: never;
   [StorageKeys.accessToken]?: string;
   [StorageKeys.idToken]?: string;
   [StorageKeys.refreshToken]?: string;
-}
+};
+
+type ExchangeAuthCodeResultError = {
+  success: false;
+  error: string;
+  [StorageKeys.accessToken]?: never;
+  [StorageKeys.idToken]?: never;
+  [StorageKeys.refreshToken]?: never;
+};
+
+type ExchangeAuthCodeResult =
+  | ExchangeAuthCodeResultSuccess
+  | ExchangeAuthCodeResultError;
 
 export const exchangeAuthCode = async ({
   urlParams,
@@ -99,7 +111,7 @@ export const exchangeAuthCode = async ({
     headers["Kinde-SDK"] =
       `${frameworkSettings.framework}/${frameworkSettings.sdkVersion}/${frameworkSettings.frameworkVersion}/Javascript`;
   }
-  const response = await fetch(`${domain}/oauth2/token`, {
+  const fetchOptions: RequestInit = {
     method: "POST",
     ...(isCustomDomain(domain) && { credentials: "include" }),
     headers: new Headers(headers),
@@ -110,7 +122,9 @@ export const exchangeAuthCode = async ({
       grant_type: "authorization_code",
       redirect_uri: redirectURL,
     }),
-  });
+  };
+
+  const response = await fetch(`${domain}/oauth2/token`, fetchOptions);
   if (!response?.ok) {
     const errorText = await response.text();
     console.error("Token exchange failed:", response.status, errorText);
@@ -137,7 +151,7 @@ export const exchangeAuthCode = async ({
     });
   }
 
-  if (storageSettings.useInsecureForRefreshToken) {
+  if (storageSettings.useInsecureForRefreshToken || !isCustomDomain(domain)) {
     activeStorage.setSessionItem(StorageKeys.refreshToken, data.refresh_token);
   }
 

lib/utils/generateAuthUrl.test.ts

@@ -177,4 +177,24 @@ describe("generateAuthUrl", () => {
     expect(nonce).toBeDefined();
     expect(codeVerifier).toBeDefined();
   });
+
+  it("if state is defined, ensure its stored in correctly", async () => {
+    const store = new MemoryStorage();
+    setActiveStorage(store);
+
+    const testState = "testState:123";
+    const domain = "https://auth.example.com";
+    const options: LoginOptions = {
+      clientId: "client123",
+      redirectURL: "https://example.com",
+      prompt: PromptTypes.login,
+      state: testState,
+    };
+
+    await generateAuthUrl(domain, IssuerRouteTypes.login, options);
+
+    const state = await store.getSessionItem(StorageKeys.state);
+
+    expect(state).toEqual(testState);
+  });
 });

lib/utils/generateAuthUrl.ts

@@ -30,9 +30,9 @@ export const generateAuthUrl = async (
 
   if (!options.state) {
     options.state = generateRandomString(32);
-    if (activeStorage) {
-      activeStorage.setSessionItem(StorageKeys.state, options.state);
-    }
+  }
+  if (activeStorage) {
+    activeStorage.setSessionItem(StorageKeys.state, options.state);
   }
   searchParams["state"] = options.state;
 

lib/utils/isCustomDomain.test.ts

@@ -14,4 +14,8 @@ describe("isCustomDomain", () => {
     const result = isCustomDomain("test.kinde.com");
     expect(result).toEqual(false);
   });
+  it("works on no prod kinde domains", () => {
+    const result = isCustomDomain("https://test-test.au.kinde.com");
+    expect(result).toEqual(false);
+  });
 });

lib/utils/isCustomDomain.ts

@@ -1,5 +1,5 @@
 export const isCustomDomain = (domain: string): boolean => {
   return !domain.match(
-    /^(?:https?:\/\/)?[a-zA-Z0-9][-a-zA-Z0-9]*\.kinde\.com$/i,
+    /^(?:https?:\/\/)?[a-zA-Z0-9][.-a-zA-Z0-9]*\.kinde\.com$/i,
   );
 };

lib/utils/token/refreshToken.test.ts

@@ -5,6 +5,7 @@ import {
   storageSettings,
 } from "../../sessionManager";
 import * as tokenUtils from ".";
+import * as refreshTimer from "../refreshTimer";
 
 describe("refreshToken", () => {
   const mockDomain = "https://example.com";
@@ -17,6 +18,8 @@ describe("refreshToken", () => {
     vi.resetAllMocks();
     vi.spyOn(tokenUtils, "getDecodedToken").mockResolvedValue(null);
     vi.spyOn(memoryStorage, "setSessionItem");
+    vi.spyOn(refreshTimer, "setRefreshTimer");
+    vi.spyOn(tokenUtils, "refreshToken");
     tokenUtils.setActiveStorage(memoryStorage);
     global.fetch = vi.fn();
     vi.spyOn(console, "error").mockImplementation(() => {});
@@ -192,10 +195,19 @@ describe("refreshToken", () => {
       refresh_token: "new-refresh-token",
     };
     storageSettings.useInsecureForRefreshToken = true;
+
+    const insecureStorage = new MemoryStorage();
+
+    tokenUtils.setInsecureStorage(insecureStorage);
+    await insecureStorage.setSessionItem(
+      StorageKeys.refreshToken,
+      mockRefreshTokenValue,
+    );
+    vi.spyOn(insecureStorage, "setSessionItem");
     memoryStorage.getSessionItem = vi
       .fn()
       .mockResolvedValue(mockRefreshTokenValue);
-    vi.mocked(global.fetch).mockResolvedValue({
+    vi.mocked(global.fetch).mockResolvedValueOnce({
       ok: true,
       json: () => Promise.resolve(mockResponse),
     } as Response);
@@ -219,9 +231,94 @@ describe("refreshToken", () => {
       StorageKeys.idToken,
       "new-id-token",
     );
-    expect(memoryStorage.setSessionItem).toHaveBeenCalledWith(
+    expect(insecureStorage.setSessionItem).toHaveBeenCalledWith(
+      StorageKeys.refreshToken,
+      "new-refresh-token",
+    );
+    expect(memoryStorage.setSessionItem).not.toHaveBeenCalledWith(
       StorageKeys.refreshToken,
       "new-refresh-token",
     );
+
+    // reset storageSettings to default
+    storageSettings.useInsecureForRefreshToken = false;
+  });
+
+  it("raise error when no session storage is found when useInsecureForRefreshToken", async () => {
+    const mockResponse = {
+      access_token: "new-access-token",
+      id_token: "new-id-token",
+      refresh_token: "new-refresh-token",
+    };
+    storageSettings.useInsecureForRefreshToken = true;
+
+    tokenUtils.clearActiveStorage();
+
+    const insecureStorage = new MemoryStorage();
+    tokenUtils.setInsecureStorage(insecureStorage);
+    await insecureStorage.setSessionItem(
+      StorageKeys.refreshToken,
+      mockRefreshTokenValue,
+    );
+
+    vi.mocked(global.fetch).mockResolvedValueOnce({
+      ok: true,
+      json: () => Promise.resolve(mockResponse),
+    } as Response);
+
+    const result = await tokenUtils.refreshToken({
+      domain: mockDomain,
+      clientId: mockClientId,
+    });
+
+    expect(result).toStrictEqual({
+      error: "No active storage found",
+      success: false,
+    });
+
+    storageSettings.useInsecureForRefreshToken = false;
+  });
+
+  it("triggers new timer when expires_in supplied and calls refreshToken", async () => {
+    vi.useFakeTimers();
+    const mockResponse = {
+      access_token: "new-access-token",
+      id_token: "new-id-token",
+      refresh_token: "new-refresh-token",
+      expires_in: 1000,
+    };
+
+    const insecureStorage = new MemoryStorage();
+    tokenUtils.setInsecureStorage(insecureStorage);
+    await insecureStorage.setSessionItem(
+      StorageKeys.refreshToken,
+      mockRefreshTokenValue,
+    );
+
+    vi.mocked(global.fetch).mockResolvedValueOnce({
+      ok: true,
+      json: () => Promise.resolve(mockResponse),
+    } as Response);
+
+    const result = await tokenUtils.refreshToken({
+      domain: mockKindeDomain,
+      clientId: mockClientId,
+    });
+
+    expect(refreshTimer.setRefreshTimer).toHaveBeenCalledWith(
+      1000,
+      expect.any(Function),
+    );
+    vi.runAllTimers();
+    expect(tokenUtils.refreshToken).toHaveBeenCalledWith({
+      domain: mockKindeDomain,
+      clientId: mockClientId,
+    });
+    expect(result).toStrictEqual({
+      accessToken: "new-access-token",
+      idToken: "new-id-token",
+      refreshToken: "new-refresh-token",
+      success: true,
+    });
   });
 });