diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f3717eeb..a03be9af8 100644 --- 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 index 73b3ece9e..72452d9e2 100644 --- a/jest.browser.js +++ b/jest.browser.js @@ -6,6 +6,7 @@ const config = Object.assign({}, baseConfig, { USER_AGENT }), testPathIgnorePatterns: baseConfig.testPathIgnorePatterns.concat([ + '/test/spec/serverStorage.js', '/test/spec/features/server' ]) }); diff --git a/jest.server.js b/jest.server.js index de67cff4c..6c749e47b 100644 --- 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 index 6f13b51b4..6f63a8715 100644 --- 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 = transformAuthState ? transformAuthState(this._sdk, authState) diff --git a/lib/TokenManager.ts b/lib/TokenManager.ts index 42063e185..f237c82c1 100644 --- a/lib/TokenManager.ts +++ b/lib/TokenManager.ts @@ -268,7 +268,7 @@ export class TokenManager implements TokenManagerInterface { async getTokens(): Promise { 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 index ffaf3a879..00ffc0b79 100644 --- 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 index 6c86d5b1e..95f24783f 100644 --- 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 index 838b0249e..26bc8e8bd 100644 --- 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', () => { }); -}); \ No newline at end of file +}); diff --git a/test/support/jest/jest.config.js b/test/support/jest/jest.config.js index 8d71e46a6..5cb1aed47 100644 --- 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'