fix: AuthStateManager emitting inconsistence isAuthenticated state …

…during active token auto renew by only checking existence of both tokens from storage OKTA-410934 <<<Jenkins Check-In of Tested SHA: b1a08a1 for [email protected]>>> Artifact: okta-auth-js Files changed count: 9 PR Link: "#873"
okta · Jul 22, 2021 · 05cc1df · 05cc1df
1 parent e5087b8
commit 05cc1df
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## 5.2.3
+
+### Bug Fixes
+
+- [#873](https://github.com/okta/okta-auth-js/pull/873) Fixes AuthStateManager emitting inconsistence `isAuthenticated` state during active token auto renew by only checking existence of both tokens from storage
+
 ## 5.2.2
 
 - [#862](https://github.com/okta/okta-auth-js/pull/862) Fixes issue with untranspiled `class` keyword

diff --git a/jest.browser.js b/jest.browser.js
@@ -6,6 +6,7 @@ const config = Object.assign({}, baseConfig, {
     USER_AGENT
   }),
   testPathIgnorePatterns: baseConfig.testPathIgnorePatterns.concat([
+    '<rootDir>/test/spec/serverStorage.js',
     '<rootDir>/test/spec/features/server'
   ])
 });

diff --git a/jest.server.js b/jest.server.js
@@ -22,6 +22,8 @@ const config = Object.assign({}, baseConfig, {
     'oidc/getWithPopup',
     'oidc/getWithRedirect',
     'oidc/getWithoutPrompt',
+    'oidc/renewToken.ts',
+    'oidc/renewTokens.ts',
     'TokenManager/browser',
     'TokenManager/crossTabs'
   ])

diff --git a/lib/AuthStateManager.ts b/lib/AuthStateManager.ts
@@ -149,7 +149,7 @@ export class AuthStateManager {
       };
 
       this._sdk.isAuthenticated()
-        .then(isAuthenticated => {
+        .then(() => {
           if (cancelablePromise.isCanceled) {
             resolve();
             return;
@@ -160,7 +160,7 @@ export class AuthStateManager {
             accessToken,
             idToken,
             refreshToken,
-            isAuthenticated
+            isAuthenticated: !!(accessToken && idToken)
           };
           const promise: Promise<AuthState> = transformAuthState
             ? transformAuthState(this._sdk, authState)

diff --git a/lib/TokenManager.ts b/lib/TokenManager.ts
@@ -268,7 +268,7 @@ export class TokenManager implements TokenManagerInterface {
   async getTokens(): Promise<Tokens> {
     return this.getTokensSync();
   }
-  
+
   getStorageKeyByType(type: TokenType): string {
     const tokenStorage = this.storage.getStorage();
     const key = Object.keys(tokenStorage).filter(key => {

diff --git a/package.json b/package.json
@@ -2,7 +2,7 @@
   "private": true,
   "name": "@okta/okta-auth-js",
   "description": "The Okta Auth SDK",
-  "version": "5.2.2",
+  "version": "5.2.3",
   "homepage": "https://github.com/okta/okta-auth-js",
   "license": "Apache-2.0",
   "type": "commonjs",

diff --git a/test/spec/AuthStateManager.js b/test/spec/AuthStateManager.js
@@ -137,21 +137,6 @@ describe('AuthStateManager', () => {
       });
     });
 
-    it('should emit an authState with isAuthenticated', async () => {
-      jest.spyOn(sdkMock, 'isAuthenticated').mockResolvedValue('fake');
-      expect.assertions(2);
-      const instance = new AuthStateManager(sdkMock);
-      const handler = jest.fn();
-      instance.subscribe(handler);
-      await instance.updateAuthState();
-      expect(handler).toHaveBeenCalledTimes(1);
-      expect(handler).toHaveBeenCalledWith({
-        isAuthenticated: 'fake',
-        idToken: 'fakeIdToken0',
-        accessToken: 'fakeAccessToken0'
-      });
-    });
-
     it('should emit only latest authState', async () => {
       jest.spyOn(sdkMock, 'isAuthenticated');
       expect.assertions(3);

diff --git a/test/spec/TokenManager/renew.ts b/test/spec/TokenManager/renew.ts
@@ -1,9 +1,34 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
+// import { OktaAuth } from '@okta/okta-auth-js';
+import tokens from '@okta/test.support/tokens';
+import { 
+  OktaAuth, 
+  TOKEN_STORAGE_NAME, 
+  Token, 
+  TokenType, 
+  isAccessToken, 
+  isIDToken, 
+  isRefreshToken 
+} from '../../../lib';
 import { AuthApiError, AuthSdkError, OAuthError } from '../../../lib/errors';
 import { TokenManager } from '../../../lib/TokenManager';
 
 const Emitter = require('tiny-emitter');
 
+// TODO: move to TokenManager in the next minor version - OKTA-414643
+function getTokenType(token: Token): TokenType {
+  if (isAccessToken(token)) {
+    return 'accessToken';
+  }
+  if (isIDToken(token)) {
+    return 'idToken';
+  }
+  if(isRefreshToken(token)) {
+    return 'refreshToken';
+  }
+  throw new AuthSdkError('Unknown token type');
+}
+
 describe('TokenManager renew', () => {
   let testContext;
 
@@ -70,6 +95,83 @@ describe('TokenManager renew', () => {
     expect(testContext.instance.emitRenewed).toHaveBeenCalledWith('foo', testContext.freshToken, testContext.oldToken);
   });
 
+  describe('multiple token renew operations', () => {
+    const expiredTokens = {
+      idToken: { ...tokens.standardIdTokenParsed, expired: true },
+      accessToken: { ...tokens.standardAccessTokenParsed, expired: true }
+    };
+    const freshTokens = {
+      idToken: { ...tokens.standardIdTokenParsed },
+      accessToken: { ...tokens.standardAccessTokenParsed }
+    };
+
+    let authClient: OktaAuth;
+    beforeEach(() => {
+      const mockTokenStore = {};
+      const storageProvider = {
+        getItem: (key) => mockTokenStore[key],
+        setItem: (key, val) => mockTokenStore[key] = val
+      };
+      authClient = new OktaAuth({
+        pkce: false,
+        issuer: 'https://auth-js-test.okta.com',
+        clientId: 'NPSfOkH5eZrTy8PMDlvx',
+        redirectUri: 'https://example.com/redirect',
+        tokenManager: {
+          autoRenew: false,
+          storage: storageProvider
+        },
+      });
+      // preset expired token in storage
+      storageProvider.setItem(TOKEN_STORAGE_NAME, JSON.stringify(expiredTokens));
+      // mock functions
+      jest.spyOn(authClient.token, 'renew').mockImplementation((token) => {
+        const type = getTokenType(token);
+        return Promise.resolve(freshTokens[type]);
+      });
+      jest.spyOn(authClient.tokenManager, 'hasExpired').mockImplementation(token => !!token.expired);
+    });
+
+    it('produce consistent isAuthenticated state when renew happens in sequence', async () => {
+      const handler = jest.fn();
+      authClient.authStateManager.subscribe(handler);
+
+      await authClient.tokenManager.renew('idToken');
+      await authClient.tokenManager.renew('accessToken');
+
+      expect(handler).toHaveBeenCalledTimes(2);
+      expect(handler).toHaveBeenNthCalledWith(1, {
+        accessToken: expiredTokens.accessToken,
+        idToken: freshTokens.idToken,
+        isAuthenticated: true
+      });
+      expect(handler).toHaveBeenNthCalledWith(2, {
+        accessToken: freshTokens.accessToken,
+        idToken: freshTokens.idToken,
+        isAuthenticated: true
+      });
+    });
+
+    it('produces consistent isAuthenticated state when renew happens in parallel', async () => {
+      const handler = jest.fn();
+      authClient.authStateManager.subscribe(handler);
+
+      await Promise.all([
+        authClient.tokenManager.renew('idToken'),
+        authClient.tokenManager.renew('accessToken')
+      ]);
+
+      // AuthStateManager cancelled one event
+      expect(handler).toHaveBeenCalledTimes(1);
+      expect(handler).toHaveBeenCalledWith({
+        accessToken: freshTokens.accessToken,
+        idToken: freshTokens.idToken,
+        isAuthenticated: true
+      });
+    });
+
+  });
+
   describe('error handling', () => {
     beforeEach(() => {
       jest.spyOn(testContext.sdkMock.token, 'renew').mockImplementation(() => Promise.reject(testContext.error));
@@ -127,4 +229,4 @@ describe('TokenManager renew', () => {
 
   });
 
-});
+});
diff --git a/test/support/jest/jest.config.js b/test/support/jest/jest.config.js
@@ -44,9 +44,7 @@ module.exports = {
   'roots': [
     'test/spec'
   ],
-  'testPathIgnorePatterns': [
-    './test/spec/serverStorage.js'
-  ],
+  'testPathIgnorePatterns': [],
   'reporters': [
     'default',
     'jest-junit'