From b0e2a35fb06c2874b583c0cbcbe843d0e3948ad2 Mon Sep 17 00:00:00 2001 From: Jared Perreault Date: Fri, 21 Jul 2023 14:17:14 -0400 Subject: [PATCH 1/3] first pass --- lib/oidc/mixin/index.ts | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/oidc/mixin/index.ts b/lib/oidc/mixin/index.ts index d2460d2fd..9aeedb8a7 100644 --- a/lib/oidc/mixin/index.ts +++ b/lib/oidc/mixin/index.ts @@ -285,16 +285,18 @@ export function mixinOAuth options = Object.assign({}, options); // postLogoutRedirectUri must be whitelisted in Okta Admin UI - var defaultUri = window.location.origin; - var currentUri = window.location.href; - var postLogoutRedirectUri = options.postLogoutRedirectUri + const defaultUri = window.location.origin; + const currentUri = window.location.href; + const postLogoutRedirectUri = options.postLogoutRedirectUri || this.options.postLogoutRedirectUri || defaultUri; + const state = options?.state; + - var accessToken = options.accessToken; - var refreshToken = options.refreshToken; - var revokeAccessToken = options.revokeAccessToken !== false; - var revokeRefreshToken = options.revokeRefreshToken !== false; + let accessToken = options.accessToken; + let refreshToken = options.refreshToken; + const revokeAccessToken = options.revokeAccessToken !== false; + const revokeRefreshToken = options.revokeRefreshToken !== false; if (revokeRefreshToken && typeof refreshToken === 'undefined') { refreshToken = this.tokenManager.getTokensSync().refreshToken as RefreshToken; @@ -321,15 +323,18 @@ export function mixinOAuth // Fallback to XHR signOut, then simulate a redirect to the post logout uri if (!logoutUri) { // local tokens are cleared once session is closed - return this.closeSession() // can throw if the user cannot be signed out - .then(function(sessionClosed) { - if (postLogoutRedirectUri === currentUri) { - window.location.reload(); // force a hard reload if URI is not changing - } else { - window.location.assign(postLogoutRedirectUri); - } - return sessionClosed; - }); + const sessionClosed = await this.closeSession(); // can throw if the user cannot be signed out + const redirectUri = new URL(postLogoutRedirectUri); + if (state) { + redirectUri.searchParams.append('state', state); + } + if (postLogoutRedirectUri === currentUri) { + // window.location.reload(); // force a hard reload if URI is not changing + window.location.href = redirectUri.href; + } else { + window.location.assign(redirectUri); + } + return sessionClosed; } else { if (options.clearTokensBeforeRedirect) { // Clear all local tokens From 5696ccf61d809411c80a48e64d1e8d3ac3e89a27 Mon Sep 17 00:00:00 2001 From: Jared Perreault Date: Thu, 27 Jul 2023 10:39:16 -0400 Subject: [PATCH 2/3] allows postLogoutRedirectUri to be null for default behavior --- lib/oidc/mixin/index.ts | 15 +++++--- lib/oidc/types/api.ts | 2 +- test/spec/OktaAuth/browser.ts | 67 ++++++++++++++++++++++++++--------- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/lib/oidc/mixin/index.ts b/lib/oidc/mixin/index.ts index 9aeedb8a7..d38d38846 100644 --- a/lib/oidc/mixin/index.ts +++ b/lib/oidc/mixin/index.ts @@ -261,7 +261,7 @@ export function mixinOAuth if (!idToken) { return ''; } - if (!postLogoutRedirectUri) { + if (postLogoutRedirectUri === undefined) { postLogoutRedirectUri = this.options.postLogoutRedirectUri; } @@ -287,9 +287,14 @@ export function mixinOAuth // postLogoutRedirectUri must be whitelisted in Okta Admin UI const defaultUri = window.location.origin; const currentUri = window.location.href; - const postLogoutRedirectUri = options.postLogoutRedirectUri + // Fix for issue/1410 - allow for no postLogoutRedirectUri to be passed, resulting in /logout default behavior + // "If no Okta session exists, this endpoint has no effect and the browser is redirected immediately to the + // Okta sign-in page or the post_logout_redirect_uri (if specified)." + // - https://developer.okta.com/docs/reference/api/oidc/#logout + const postLogoutRedirectUri = options.postLogoutRedirectUri === null ? null : + (options.postLogoutRedirectUri || this.options.postLogoutRedirectUri - || defaultUri; + || defaultUri); const state = options?.state; @@ -324,7 +329,7 @@ export function mixinOAuth if (!logoutUri) { // local tokens are cleared once session is closed const sessionClosed = await this.closeSession(); // can throw if the user cannot be signed out - const redirectUri = new URL(postLogoutRedirectUri); + const redirectUri = new URL(postLogoutRedirectUri || defaultUri); // during fallback, redirectUri cannot be null if (state) { redirectUri.searchParams.append('state', state); } @@ -332,7 +337,7 @@ export function mixinOAuth // window.location.reload(); // force a hard reload if URI is not changing window.location.href = redirectUri.href; } else { - window.location.assign(redirectUri); + window.location.assign(redirectUri.href); } return sessionClosed; } else { diff --git a/lib/oidc/types/api.ts b/lib/oidc/types/api.ts index 5b2037471..150e13f6f 100644 --- a/lib/oidc/types/api.ts +++ b/lib/oidc/types/api.ts @@ -101,7 +101,7 @@ export interface IsAuthenticatedOptions { } export interface SignoutRedirectUrlOptions { - postLogoutRedirectUri?: string; + postLogoutRedirectUri?: string | null; idToken?: IDToken; state?: string; } diff --git a/test/spec/OktaAuth/browser.ts b/test/spec/OktaAuth/browser.ts index 739b2e3c1..1560b2bbb 100644 --- a/test/spec/OktaAuth/browser.ts +++ b/test/spec/OktaAuth/browser.ts @@ -46,8 +46,8 @@ describe('OktaAuth (browser)', function() { delete (global.window as any).location; global.window.location = { protocol: 'https:', - hostname: 'somesite.local', - href: 'https://somesite.local', + hostname: 'somesite.local/', + href: 'https://somesite.local/', replace: jest.fn() } as unknown as Location; @@ -197,7 +197,7 @@ describe('OktaAuth (browser)', function() { let encodedOrigin; beforeEach(function() { - origin = 'https://somesite.local'; + origin = 'https://somesite.local/'; href = `${origin}/some-route`; encodedOrigin = encodeURIComponent(origin); Object.assign(global.window.location, { @@ -233,7 +233,7 @@ describe('OktaAuth (browser)', function() { expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); - expect(signOutSuccess).toBeTruthy(); + expect(signOutSuccess).toBe(true); }); }); @@ -247,7 +247,7 @@ describe('OktaAuth (browser)', function() { expect(auth.revokeRefreshToken).toHaveBeenCalledWith(refreshToken); expect(auth.closeSession).not.toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`); - expect(signOutSuccess).toBeTruthy(); + expect(signOutSuccess).toBe(true); }); }); @@ -274,7 +274,7 @@ describe('OktaAuth (browser)', function() { describe('postLogoutRedirectUri', function() { it('can be set by config', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local/'; const encodedUri = encodeURIComponent(postLogoutRedirectUri); auth = new OktaAuth({ pkce: false, @@ -288,13 +288,20 @@ describe('OktaAuth (browser)', function() { }); }); it('can be passed as an option', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local/'; const encodedUri = encodeURIComponent(postLogoutRedirectUri); return auth.signOut({ postLogoutRedirectUri }) .then(function() { expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedUri}`); }); }); + + it('can be set to null', function() { + return auth.signOut({ postLogoutRedirectUri: null }) + .then(function() { + expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}`); + }); + }); }); it('Can pass a "state" option', function() { @@ -377,7 +384,7 @@ describe('OktaAuth (browser)', function() { expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); expect(auth.closeSession).toHaveBeenCalled(); expect(window.location.assign).toHaveBeenCalledWith(window.location.origin); - expect(signOutSuccess).toBeTruthy(); + expect(signOutSuccess).toBe(true); }); }); @@ -389,8 +396,8 @@ describe('OktaAuth (browser)', function() { expect(auth.tokenManager.getTokensSync).toHaveBeenCalledTimes(4); expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken); expect(auth.closeSession).toHaveBeenCalled(); - expect(window.location.reload).toHaveBeenCalled(); - expect(signOutSuccess).toBeTruthy(); + // expect(window.location.reload).toHaveBeenCalled(); // TODO: confirm href? + expect(signOutSuccess).toBe(true); }); }); @@ -411,17 +418,17 @@ describe('OktaAuth (browser)', function() { }); it('with postLogoutRedirectUri: will call window.location.assign', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local//'; spyOn(auth, 'closeSession').and.returnValue(Promise.resolve(true)); return auth.signOut({ postLogoutRedirectUri }) .then(function(signOutSuccess) { expect(window.location.assign).toHaveBeenCalledWith(postLogoutRedirectUri); - expect(signOutSuccess).toBeTruthy(); + expect(signOutSuccess).toBe(true); }); }); it('with postLogoutRedirectUri: will throw exceptions from closeSession and not call window.location.assign', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local/'; const testError = new Error('test error'); spyOn(auth, 'closeSession').and.callFake(function() { return Promise.reject(testError); @@ -438,22 +445,48 @@ describe('OktaAuth (browser)', function() { }); it('if closeSession resolves with true, signOut resolves with true', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local/'; spyOn(auth, 'closeSession').and.returnValue(Promise.resolve(true)); return auth.signOut({ postLogoutRedirectUri }) .then(function(signOutSuccess) { expect(window.location.assign).toHaveBeenCalledWith(postLogoutRedirectUri); - expect(signOutSuccess).toBeTruthy(); + expect(signOutSuccess).toBe(true); }); }); it('if closeSession resolves with false (session does not exist), signOut resolves with false', function() { - const postLogoutRedirectUri = 'http://someother'; + const postLogoutRedirectUri = 'http://someother.local/'; spyOn(auth, 'closeSession').and.returnValue(Promise.resolve(false)); return auth.signOut({ postLogoutRedirectUri }) .then(function(signOutSuccess) { expect(window.location.assign).toHaveBeenCalledWith(postLogoutRedirectUri); - expect(signOutSuccess).toBeFalsy(); + expect(signOutSuccess).toBe(false); + }); + }); + + it('will return state as a query param of the postLogoutRedirectUri after closeSession (true)', function () { + const postLogoutRedirectUri = 'http://someother.local/'; + const state = 'somestatevalue'; + spyOn(auth, 'closeSession').and.returnValue(Promise.resolve(true)); + return auth.signOut({ postLogoutRedirectUri, state }) + .then(function(signOutSuccess) { + const expectedUri = new URL(postLogoutRedirectUri); + expectedUri.searchParams.append('state', state); + expect(window.location.assign).toHaveBeenCalledWith(expectedUri.href); + expect(signOutSuccess).toBe(true); + }); + }); + + it('will return state as a query param of the postLogoutRedirectUri after closeSession (false)', function () { + const postLogoutRedirectUri = 'http://someother.local/'; + const state = 'somestatevalue'; + spyOn(auth, 'closeSession').and.returnValue(Promise.resolve(false)); + return auth.signOut({ postLogoutRedirectUri, state }) + .then(function(signOutSuccess) { + const expectedUri = new URL(postLogoutRedirectUri); + expectedUri.searchParams.append('state', state); + expect(window.location.assign).toHaveBeenCalledWith(expectedUri.href); + expect(signOutSuccess).toBe(false); }); }); }); From 5c531cbeeb52ef21a2fb2368688eeafe407de267 Mon Sep 17 00:00:00 2001 From: Jared Perreault Date: Thu, 27 Jul 2023 11:53:24 -0400 Subject: [PATCH 3/3] README and CHANGELOG --- CHANGELOG.md | 6 ++++++ README.md | 2 +- lib/oidc/mixin/index.ts | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea082ac28..6a9b1d8cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ### Features +- [#1440](https://github.com/okta/okta-auth-js/pull/1440) Fixes type of `tokenManager.getSync` + +- [#1439](https://github.com/okta/okta-auth-js/pull/1439) `.signOut` improvements + * Passing `postLogoutRedirectUri: null` to `.signOut` now omits the param from `/logout` call and will observe the behavior of `/logout` + * `state` is now returned as a query param to the `postLogoutRedirectUri` when `.signOut` falls back to `.closeSession` + - [#1412](https://github.com/okta/okta-auth-js/pull/1412) * Adds oauth2 introspect method, exposed as `authClient.token.introspect` * Adds optional `tokens` param to `renewTokens` diff --git a/README.md b/README.md index cc095999d..647be2f7d 100644 --- a/README.md +++ b/README.md @@ -996,7 +996,7 @@ Signs the user out of their current [Okta session](https://developer.okta.com/do `signOut` takes the following options: -* `postLogoutRedirectUri` - Setting a value will override the `postLogoutRedirectUri` configured on the SDK. +* `postLogoutRedirectUri` - Setting a value will override the `postLogoutRedirectUri` configured on the SDK. This will default to `window.location.origin` if no value is provided. To prevent this explicitly pass `null` to leverage the default behavior of `/logout`. If `signOut` falls back to `closeSession` `window.location.origin` will still be used as the default value, even if `null` is passed. * `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. * `clearTokensBeforeRedirect` - If `true` (default: `false`) local tokens will be removed before the logout redirect happens. 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 terminating the Okta SSO session can result in logging back in again when using [`@okta/okta-react`](https://www.npmjs.com/package/@okta/okta-react)'s [`SecureRoute`](https://github.com/okta/okta-react#secureroute) component. diff --git a/lib/oidc/mixin/index.ts b/lib/oidc/mixin/index.ts index d38d38846..ac1845972 100644 --- a/lib/oidc/mixin/index.ts +++ b/lib/oidc/mixin/index.ts @@ -329,7 +329,7 @@ export function mixinOAuth if (!logoutUri) { // local tokens are cleared once session is closed const sessionClosed = await this.closeSession(); // can throw if the user cannot be signed out - const redirectUri = new URL(postLogoutRedirectUri || defaultUri); // during fallback, redirectUri cannot be null + const redirectUri = new URL(postLogoutRedirectUri || defaultUri); // during fallback, redirectUri cannot be null if (state) { redirectUri.searchParams.append('state', state); }