From d44dc408f51660a35c352838ac159d7328f20643 Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Tue, 11 Jan 2022 22:39:37 -0500 Subject: [PATCH 1/6] BREAKING CHANGE: set signOut option clearTokensAfterRedirect default to true --- README.md | 2 +- lib/OktaAuth.ts | 3 ++- test/spec/OktaAuth/browser.ts | 30 +++++++++++++++++------------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 871e8dad8..a5498251c 100644 --- a/README.md +++ b/README.md @@ -928,7 +928,7 @@ Signs the user out of their current [Okta session](https://developer.okta.com/do * `postLogoutRedirectUri` - Setting a value will override the `postLogoutRedirectUri` configured on the SDK. * `state` - An optional value, used along with `postLogoutRedirectUri`. If set, this value will be returned as a query parameter during the redirect to the `postLogoutRedirectUri` * `idToken` - Specifies the ID token object. By default, `signOut` will look for a token object named `idToken` within the `TokenManager`. If you have stored the id token object in a different location, you should retrieve it first and then pass it here. -* `clearTokensAfterRedirect` - If `true` (default: `false`) a flag (`pendingRemove`) will be added to local tokens instead of clearing them immediately. Calling `oktaAuth.start()` after logout redirect will clear local tokens if flags are found. This option can be used when work with `SecureRoute` component from Okta's downstream client SDKs to guarantee the local tokens can only be cleared after the Okta SSO session is fully killed. +* `clearTokensAfterRedirect` - By default (`true`) a flag (`pendingRemove`) will be added to local tokens instead of clearing them immediately. Calling `oktaAuth.start()` after logout redirect will clear local tokens if flags are found. Set this option to `false` will remove local tokens before the logout redirect. Use this option with care: removing local tokens before fully kill the Okta SSO session can result in logging back in again when `SecureRoute` component from Okta's downstream client SDKs is used in the application. * `revokeAccessToken` - If `false` (default: `true`) the access token will not be revoked. Use this option with care: not revoking tokens may pose a security risk if tokens have been leaked outside the application. * `revokeRefreshToken` - If `false` (default: `true`) the refresh token will not be revoked. Use this option with care: not revoking tokens may pose a security risk if tokens have been leaked outside the application. Revoking a refresh token will revoke any access tokens minted by it, even if `revokeAccessToken` is `false`. * `accessToken` - Specifies the access token object. By default, `signOut` will look for a token object named `accessToken` within the `TokenManager`. If you have stored the access token object in a different location, you should retrieve it first and then pass it here. This options is ignored if the `revokeAccessToken` option is `false`. diff --git a/lib/OktaAuth.ts b/lib/OktaAuth.ts index d301f4013..497f4a04f 100644 --- a/lib/OktaAuth.ts +++ b/lib/OktaAuth.ts @@ -475,6 +475,7 @@ class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI { var refreshToken = options.refreshToken; var revokeAccessToken = options.revokeAccessToken !== false; var revokeRefreshToken = options.revokeRefreshToken !== false; + var clearTokensAfterRedirect = options.clearTokensAfterRedirect !== false; if (revokeRefreshToken && typeof refreshToken === 'undefined') { refreshToken = this.tokenManager.getTokensSync().refreshToken as RefreshToken; @@ -510,7 +511,7 @@ class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI { } }); } else { - if (options.clearTokensAfterRedirect) { + if (clearTokensAfterRedirect) { this.tokenManager.addPendingRemoveFlags(); } else { // Clear all local tokens diff --git a/test/spec/OktaAuth/browser.ts b/test/spec/OktaAuth/browser.ts index c0dbfb560..9d2d02ac2 100644 --- a/test/spec/OktaAuth/browser.ts +++ b/test/spec/OktaAuth/browser.ts @@ -216,10 +216,9 @@ describe('OktaAuth (browser)', function() { it('Default options when no refreshToken: will revokeAccessToken and use window.location.origin for postLogoutRedirectUri', function() { return auth.signOut() .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(3); expect(auth.revokeRefreshToken).not.toHaveBeenCalled(); expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); - expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); @@ -231,10 +230,9 @@ describe('OktaAuth (browser)', function() { return auth.signOut() .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(3); expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); expect(auth.revokeRefreshToken).toHaveBeenCalledWith(refreshToken); - expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); @@ -257,7 +255,6 @@ describe('OktaAuth (browser)', function() { var customToken = { idToken: 'fake-custom' }; return auth.signOut({ idToken: customToken }) .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(2); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${customToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); @@ -302,10 +299,9 @@ describe('OktaAuth (browser)', function() { return auth.signOut({ revokeAccessToken: false }) .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(2); expect(auth.revokeAccessToken).not.toHaveBeenCalled(); expect(auth.revokeRefreshToken).toHaveBeenCalled(); - expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); @@ -316,10 +312,9 @@ describe('OktaAuth (browser)', function() { return auth.signOut({ revokeRefreshToken: false }) .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(2); expect(auth.revokeAccessToken).toHaveBeenCalled(); expect(auth.revokeRefreshToken).not.toHaveBeenCalled(); - expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); @@ -327,22 +322,31 @@ describe('OktaAuth (browser)', function() { it('Can pass a "accessToken=false" to skip accessToken logic', function() { return auth.signOut({ accessToken: false }) .then(function() { - expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(2); expect(auth.revokeAccessToken).not.toHaveBeenCalled(); - expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); - it('Can pass a "clearTokensAfterRedirect=true" to skip clear tokens logic', function() { + it('skips token clear logic by default', () => { auth.tokenManager.addPendingRemoveFlags = jest.fn(); - return auth.signOut({ clearTokensAfterRedirect: true }) + return auth.signOut() .then(function() { expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(auth.tokenManager.addPendingRemoveFlags).toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); + + it('Can pass a "clearTokensAfterRedirect=false" to force clear tokens logic', function() { + auth.tokenManager.addPendingRemoveFlags = jest.fn(); + return auth.signOut({ clearTokensAfterRedirect: false }) + .then(function() { + expect(auth.tokenManager.clear).toHaveBeenCalled(); + expect(auth.tokenManager.addPendingRemoveFlags).not.toHaveBeenCalled(); + expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); + }); + }); }); describe('without idToken', () => { From e8501b2646d4e9d2b269ff50a0fa1c0017c2e79b Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Wed, 12 Jan 2022 10:48:19 -0500 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6037e8a9..193865c81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - [#1050](https://github.com/okta/okta-auth-js/pull/1050) Removes `userAgent` field from oktaAuth instance - [#1014](https://github.com/okta/okta-auth-js/pull/1014) Shared transaction storage is automatically cleared on success and error states. Storage is not cleared for "terminal" state which is neither success nor error. - [#1051](https://github.com/okta/okta-auth-js/pull/1051) Removes `useMultipleCookies` from CookieStorage options +- [#1059](https://github.com/okta/okta-auth-js/pull/1059) Sets signOut option `clearTokensAfterRedirect` default value to true ### Features From 4702b2a60bfb1d0ed7b003778eb881c469dc09f2 Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Wed, 12 Jan 2022 14:14:42 -0500 Subject: [PATCH 3/6] Update e2e test app --- test/apps/app/public/static/index.html | 2 +- test/apps/app/src/testApp.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/apps/app/public/static/index.html b/test/apps/app/public/static/index.html index 045874c91..cf0748dff 100644 --- a/test/apps/app/public/static/index.html +++ b/test/apps/app/public/static/index.html @@ -109,7 +109,7 @@ function logout(e) { e.preventDefault(); - authClient.signOut() + authClient.signOut({ clearTokensAfterRedirect: false }) } main(); diff --git a/test/apps/app/src/testApp.ts b/test/apps/app/src/testApp.ts index 55a21a8e6..ce98b6940 100644 --- a/test/apps/app/src/testApp.ts +++ b/test/apps/app/src/testApp.ts @@ -97,10 +97,10 @@ function logoutLink(app: TestApp): string {
  • - Logout (and redirect here) + Logout (and redirect here)
  • - Logout (and redirect here, tokens will be cleared afterward) + Logout (and redirect here, tokens will be cleared afterward)
  • Logout (XHR + reload) From 876b2691c127d49cd202784346da97fd512b4f7d Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Wed, 12 Jan 2022 17:38:06 -0500 Subject: [PATCH 4/6] use clearTokensBeforeRedirect instead --- CHANGELOG.md | 4 +++- README.md | 2 +- lib/OktaAuth.ts | 7 +++---- lib/types/api.ts | 2 +- test/apps/app/public/static/index.html | 2 +- test/apps/app/src/testApp.ts | 2 +- test/spec/OktaAuth/browser.ts | 2 +- 7 files changed, 11 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 193865c81..d057a237a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ - [#1050](https://github.com/okta/okta-auth-js/pull/1050) Removes `userAgent` field from oktaAuth instance - [#1014](https://github.com/okta/okta-auth-js/pull/1014) Shared transaction storage is automatically cleared on success and error states. Storage is not cleared for "terminal" state which is neither success nor error. - [#1051](https://github.com/okta/okta-auth-js/pull/1051) Removes `useMultipleCookies` from CookieStorage options -- [#1059](https://github.com/okta/okta-auth-js/pull/1059) Sets signOut option `clearTokensAfterRedirect` default value to true +- [#1059](https://github.com/okta/okta-auth-js/pull/1059) + - Removes signOut option `clearTokensAfterRedirect` + - Adds signOut option `clearTokensAfterRedirect` (default: `false`) to remove local tokens before logout redirect happen ### Features diff --git a/README.md b/README.md index a5498251c..f4232b5ba 100644 --- a/README.md +++ b/README.md @@ -928,7 +928,7 @@ Signs the user out of their current [Okta session](https://developer.okta.com/do * `postLogoutRedirectUri` - Setting a value will override the `postLogoutRedirectUri` configured on the SDK. * `state` - An optional value, used along with `postLogoutRedirectUri`. If set, this value will be returned as a query parameter during the redirect to the `postLogoutRedirectUri` * `idToken` - Specifies the ID token object. By default, `signOut` will look for a token object named `idToken` within the `TokenManager`. If you have stored the id token object in a different location, you should retrieve it first and then pass it here. -* `clearTokensAfterRedirect` - By default (`true`) a flag (`pendingRemove`) will be added to local tokens instead of clearing them immediately. Calling `oktaAuth.start()` after logout redirect will clear local tokens if flags are found. Set this option to `false` will remove local tokens before the logout redirect. Use this option with care: removing local tokens before fully kill the Okta SSO session can result in logging back in again when `SecureRoute` component from Okta's downstream client SDKs is used in the application. +* `clearTokensBeforeRedirect` - If `true` (default: `false`) local tokens will be removed before the logout redirect happen. Otherwise a flag (`pendingRemove`) will be added to each local token instead of clearing them immediately. Calling `oktaAuth.start()` after logout redirect will clear local tokens if flags are found. Use this option with care: removing local tokens before fully kill the Okta SSO session can result in logging back in again when `SecureRoute` component from Okta's downstream client SDKs is used in the application. * `revokeAccessToken` - If `false` (default: `true`) the access token will not be revoked. Use this option with care: not revoking tokens may pose a security risk if tokens have been leaked outside the application. * `revokeRefreshToken` - If `false` (default: `true`) the refresh token will not be revoked. Use this option with care: not revoking tokens may pose a security risk if tokens have been leaked outside the application. Revoking a refresh token will revoke any access tokens minted by it, even if `revokeAccessToken` is `false`. * `accessToken` - Specifies the access token object. By default, `signOut` will look for a token object named `accessToken` within the `TokenManager`. If you have stored the access token object in a different location, you should retrieve it first and then pass it here. This options is ignored if the `revokeAccessToken` option is `false`. diff --git a/lib/OktaAuth.ts b/lib/OktaAuth.ts index 497f4a04f..8d86314d7 100644 --- a/lib/OktaAuth.ts +++ b/lib/OktaAuth.ts @@ -475,7 +475,6 @@ class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI { var refreshToken = options.refreshToken; var revokeAccessToken = options.revokeAccessToken !== false; var revokeRefreshToken = options.revokeRefreshToken !== false; - var clearTokensAfterRedirect = options.clearTokensAfterRedirect !== false; if (revokeRefreshToken && typeof refreshToken === 'undefined') { refreshToken = this.tokenManager.getTokensSync().refreshToken as RefreshToken; @@ -511,11 +510,11 @@ class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI { } }); } else { - if (clearTokensAfterRedirect) { - this.tokenManager.addPendingRemoveFlags(); - } else { + if (options.clearTokensBeforeRedirect) { // Clear all local tokens this.tokenManager.clear(); + } else { + this.tokenManager.addPendingRemoveFlags(); } // Flow ends with logout redirect window.location.assign(logoutUri); diff --git a/lib/types/api.ts b/lib/types/api.ts index e5b4900a0..f7560f615 100644 --- a/lib/types/api.ts +++ b/lib/types/api.ts @@ -243,7 +243,7 @@ export interface SignoutOptions extends SignoutRedirectUrlOptions { revokeRefreshToken?: boolean; accessToken?: AccessToken; refreshToken?: RefreshToken; - clearTokensAfterRedirect?: boolean; + clearTokensBeforeRedirect?: boolean; } export interface SignoutAPI { diff --git a/test/apps/app/public/static/index.html b/test/apps/app/public/static/index.html index cf0748dff..4b7e2fca1 100644 --- a/test/apps/app/public/static/index.html +++ b/test/apps/app/public/static/index.html @@ -109,7 +109,7 @@ function logout(e) { e.preventDefault(); - authClient.signOut({ clearTokensAfterRedirect: false }) + authClient.signOut({ clearTokensBeforeRedirect: true }) } main(); diff --git a/test/apps/app/src/testApp.ts b/test/apps/app/src/testApp.ts index ce98b6940..709005bd3 100644 --- a/test/apps/app/src/testApp.ts +++ b/test/apps/app/src/testApp.ts @@ -97,7 +97,7 @@ function logoutLink(app: TestApp): string {
    • - Logout (and redirect here) + Logout (and redirect here)
    • Logout (and redirect here, tokens will be cleared afterward) diff --git a/test/spec/OktaAuth/browser.ts b/test/spec/OktaAuth/browser.ts index 9d2d02ac2..3e0dc587e 100644 --- a/test/spec/OktaAuth/browser.ts +++ b/test/spec/OktaAuth/browser.ts @@ -340,7 +340,7 @@ describe('OktaAuth (browser)', function() { it('Can pass a "clearTokensAfterRedirect=false" to force clear tokens logic', function() { auth.tokenManager.addPendingRemoveFlags = jest.fn(); - return auth.signOut({ clearTokensAfterRedirect: false }) + return auth.signOut({ clearTokensBeforeRedirect: true }) .then(function() { expect(auth.tokenManager.clear).toHaveBeenCalled(); expect(auth.tokenManager.addPendingRemoveFlags).not.toHaveBeenCalled(); From cf550bb41882be5b2e2305729ea286b9814c1dda Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Thu, 13 Jan 2022 11:12:32 -0500 Subject: [PATCH 5/6] unit --- test/apps/app/public/static/index.html | 2 ++ test/spec/OktaAuth/browser.ts | 5 ----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/apps/app/public/static/index.html b/test/apps/app/public/static/index.html index 4b7e2fca1..68f6204dc 100644 --- a/test/apps/app/public/static/index.html +++ b/test/apps/app/public/static/index.html @@ -109,6 +109,8 @@ function logout(e) { e.preventDefault(); + // authClient.start() is not triggered by default + // clear local tokens before redirect to end up with correct authState authClient.signOut({ clearTokensBeforeRedirect: true }) } diff --git a/test/spec/OktaAuth/browser.ts b/test/spec/OktaAuth/browser.ts index 3e0dc587e..a50867818 100644 --- a/test/spec/OktaAuth/browser.ts +++ b/test/spec/OktaAuth/browser.ts @@ -218,7 +218,6 @@ describe('OktaAuth (browser)', function() { .then(function() { expect(auth.revokeRefreshToken).not.toHaveBeenCalled(); expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); - expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); @@ -232,7 +231,6 @@ describe('OktaAuth (browser)', function() { .then(function() { expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); expect(auth.revokeRefreshToken).toHaveBeenCalledWith(refreshToken); - expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); @@ -301,7 +299,6 @@ describe('OktaAuth (browser)', function() { .then(function() { expect(auth.revokeAccessToken).not.toHaveBeenCalled(); expect(auth.revokeRefreshToken).toHaveBeenCalled(); - expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); @@ -314,7 +311,6 @@ describe('OktaAuth (browser)', function() { .then(function() { expect(auth.revokeAccessToken).toHaveBeenCalled(); expect(auth.revokeRefreshToken).not.toHaveBeenCalled(); - expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); @@ -323,7 +319,6 @@ describe('OktaAuth (browser)', function() { return auth.signOut({ accessToken: false }) .then(function() { expect(auth.revokeAccessToken).not.toHaveBeenCalled(); - expect(auth.tokenManager.clear).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); }); }); From 3bb06dc8a5959ed8f066a7c1ec7b98df2f1cc019 Mon Sep 17 00:00:00 2001 From: Shuo Wu Date: Thu, 13 Jan 2022 13:57:51 -0500 Subject: [PATCH 6/6] resolve feedback --- CHANGELOG.md | 2 +- test/spec/OktaAuth/browser.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d057a237a..96665fd1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ - [#1051](https://github.com/okta/okta-auth-js/pull/1051) Removes `useMultipleCookies` from CookieStorage options - [#1059](https://github.com/okta/okta-auth-js/pull/1059) - Removes signOut option `clearTokensAfterRedirect` - - Adds signOut option `clearTokensAfterRedirect` (default: `false`) to remove local tokens before logout redirect happen + - Adds signOut option `clearTokensBeforeRedirect` (default: `false`) to remove local tokens before logout redirect happen ### Features diff --git a/test/spec/OktaAuth/browser.ts b/test/spec/OktaAuth/browser.ts index a50867818..a6893e395 100644 --- a/test/spec/OktaAuth/browser.ts +++ b/test/spec/OktaAuth/browser.ts @@ -333,7 +333,7 @@ describe('OktaAuth (browser)', function() { }); }); - it('Can pass a "clearTokensAfterRedirect=false" to force clear tokens logic', function() { + it('if "clearTokensBeforeRedirect" is true, then tokens will be cleared and pending remove flag will not be set', function() { auth.tokenManager.addPendingRemoveFlags = jest.fn(); return auth.signOut({ clearTokensBeforeRedirect: true }) .then(function() {