Skip to content

Commit

Permalink
fix: bad accessToken key in signOut (#436)
Browse files Browse the repository at this point in the history
  • Loading branch information
@shuowu
shuowu authored Jul 29, 2020
1 parent cecae41 commit e020f5b
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 21 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## PENDING

### Bug Fixes

- [#422](https://github.com/okta/okta-auth-js/pull/422) Fix bad accessToken storage key in signOut method

## 3,2.1

### Bug Fixes
Expand Down
6 changes: 3 additions & 3 deletions packages/okta-auth-js/lib/browser/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ proto.closeSession = function closeSession() {
proto.revokeAccessToken = async function revokeAccessToken(accessToken) {
var sdk = this;
if (!accessToken) {
accessToken = await sdk.tokenManager.get('accessToken');
accessToken = await sdk.tokenManager.get(constants.ACCESS_TOKEN_STORAGE_KEY);
}
// Access token may have been removed. In this case, we will silently succeed.
if (!accessToken) {
Expand All @@ -289,11 +289,11 @@ proto.signOut = async function (options) {
var logoutUrl = oauthUtil.getOAuthUrls(sdk).logoutUrl;

if (typeof idToken === 'undefined') {
idToken = await sdk.tokenManager.get('idToken');
idToken = await sdk.tokenManager.get(constants.ID_TOKEN_STORAGE_KEY);
}

if (revokeAccessToken && typeof accessToken === 'undefined') {
accessToken = await sdk.tokenManager.get('token');
accessToken = await sdk.tokenManager.get(constants.ACCESS_TOKEN_STORAGE_KEY);
}

// Clear all local tokens
Expand Down
4 changes: 3 additions & 1 deletion packages/okta-auth-js/lib/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ module.exports = {
'REDIRECT_NONCE_COOKIE_NAME': 'okta-oauth-nonce',
'TOKEN_STORAGE_NAME': 'okta-token-storage',
'CACHE_STORAGE_NAME': 'okta-cache-storage',
'PKCE_STORAGE_NAME': 'okta-pkce-storage'
'PKCE_STORAGE_NAME': 'okta-pkce-storage',
'ACCESS_TOKEN_STORAGE_KEY': 'accessToken',
'ID_TOKEN_STORAGE_KEY': 'idToken'
};
4 changes: 2 additions & 2 deletions packages/okta-auth-js/lib/token.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,10 @@ function parseFromUrl(sdk, options) {
async function getUserInfo(sdk, accessTokenObject, idTokenObject) {
// If token objects were not passed, attempt to read from the TokenManager
if (!accessTokenObject) {
accessTokenObject = await sdk.tokenManager.get('accessToken');
accessTokenObject = await sdk.tokenManager.get(constants.ACCESS_TOKEN_STORAGE_KEY);
}
if (!idTokenObject) {
idTokenObject = await sdk.tokenManager.get('idToken');
idTokenObject = await sdk.tokenManager.get(constants.ID_TOKEN_STORAGE_KEY);
}

if (!accessTokenObject ||
Expand Down
31 changes: 16 additions & 15 deletions packages/okta-auth-js/test/spec/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ jest.mock('cross-fetch');
var Emitter = require('tiny-emitter');
var OktaAuth = require('../../lib/browser/browserIndex');
var AuthApiError = require('../../lib/errors/AuthApiError');
var constants = require('../../lib/constants');

describe('Browser', function() {
let auth;
Expand Down Expand Up @@ -147,7 +148,7 @@ describe('Browser', function() {
spyOn(auth.token, 'revoke').and.returnValue(Promise.resolve());
return auth.revokeAccessToken()
.then(function() {
expect(auth.tokenManager.get).toHaveBeenCalledWith('accessToken');
expect(auth.tokenManager.get).toHaveBeenCalledWith(constants.ACCESS_TOKEN_STORAGE_KEY);
expect(auth.token.revoke).toHaveBeenCalledWith(accessToken);
});
});
Expand Down Expand Up @@ -236,9 +237,9 @@ describe('Browser', function() {

function initSpies() {
spyOn(auth.tokenManager, 'get').and.callFake(key => {
if (key === 'idToken') {
if (key === constants.ID_TOKEN_STORAGE_KEY) {
return idToken;
} else if (key === 'token') {
} else if (key === constants.ACCESS_TOKEN_STORAGE_KEY) {
return accessToken;
} else {
throw new Error(`Unknown token key: ${key}`);
Expand All @@ -258,8 +259,8 @@ describe('Browser', function() {
it('Default options: will revokeAccessToken and use window.location.href for postLogoutRedirectUri', function() {
return auth.signOut()
.then(function() {
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'idToken');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(2, 'token');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ID_TOKEN_STORAGE_KEY);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(2, constants.ACCESS_TOKEN_STORAGE_KEY);
expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken);
expect(auth.tokenManager.clear).toHaveBeenCalled();
expect(auth.closeSession).not.toHaveBeenCalled();
Expand All @@ -285,7 +286,7 @@ describe('Browser', function() {
return auth.signOut({ idToken: customToken })
.then(function() {
expect(auth.tokenManager.get).toHaveBeenCalledTimes(1);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'token');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ACCESS_TOKEN_STORAGE_KEY);
expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${customToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`);
});
});
Expand All @@ -294,7 +295,7 @@ describe('Browser', function() {
return auth.signOut({ idToken: false })
.then(function() {
expect(auth.tokenManager.get).toHaveBeenCalledTimes(1);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'token');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ACCESS_TOKEN_STORAGE_KEY);
expect(auth.closeSession).toHaveBeenCalled();
expect(window.location.reload).toHaveBeenCalled();
});
Expand Down Expand Up @@ -338,7 +339,7 @@ describe('Browser', function() {
return auth.signOut({ revokeAccessToken: false })
.then(function() {
expect(auth.tokenManager.get).toHaveBeenCalledTimes(1);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'idToken');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ID_TOKEN_STORAGE_KEY);
expect(auth.revokeAccessToken).not.toHaveBeenCalled();
expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`);
});
Expand All @@ -348,7 +349,7 @@ describe('Browser', function() {
return auth.signOut({ accessToken: false })
.then(function() {
expect(auth.tokenManager.get).toHaveBeenCalledTimes(1);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'idToken');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ID_TOKEN_STORAGE_KEY);
expect(auth.revokeAccessToken).not.toHaveBeenCalled();
expect(window.location.assign).toHaveBeenCalledWith(`${issuer}/oauth2/v1/logout?id_token_hint=${idToken.idToken}&post_logout_redirect_uri=${encodedOrigin}`);
});
Expand All @@ -361,9 +362,9 @@ describe('Browser', function() {
beforeEach(() => {
accessToken = { accessToken: 'fake' };
spyOn(auth.tokenManager, 'get').and.callFake(key => {
if (key === 'idToken') {
if (key === constants.ID_TOKEN_STORAGE_KEY) {
return;
} else if (key === 'token') {
} else if (key === constants.ACCESS_TOKEN_STORAGE_KEY) {
return accessToken;
} else {
throw new Error(`Unknown token key: ${key}`);
Expand All @@ -377,8 +378,8 @@ describe('Browser', function() {
spyOn(auth, 'closeSession').and.returnValue(Promise.resolve());
return auth.signOut()
.then(function() {
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, 'idToken');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(2, 'token');
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(1, constants.ID_TOKEN_STORAGE_KEY);
expect(auth.tokenManager.get).toHaveBeenNthCalledWith(2, constants.ACCESS_TOKEN_STORAGE_KEY);
expect(auth.revokeAccessToken).toHaveBeenCalledWith(accessToken);
expect(auth.tokenManager.clear).toHaveBeenCalled();
expect(auth.closeSession).toHaveBeenCalled();
Expand Down Expand Up @@ -434,9 +435,9 @@ describe('Browser', function() {
beforeEach(() => {
idToken = { idToken: 'fake' };
spyOn(auth.tokenManager, 'get').and.callFake(key => {
if (key === 'idToken') {
if (key === constants.ID_TOKEN_STORAGE_KEY) {
return idToken;
} else if (key === 'token') {
} else if (key === constants.ACCESS_TOKEN_STORAGE_KEY) {
return;
} else {
throw new Error(`Unknown token key: ${key}`);
Expand Down

0 comments on commit e020f5b

Please sign in to comment.