Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compat fix: PKCE loadMeta will fallback to local storage #399

Merged
merged 2 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/okta-auth-js/lib/browser/browserStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ storageUtil.browserHasSessionStorage = function() {
};

storageUtil.getPKCEStorage = function(options) {
if (storageUtil.browserHasSessionStorage()) {
options = options || {};
if (!options.preferLocalStorage && storageUtil.browserHasSessionStorage()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

return storageBuilder(storageUtil.getSessionStorage(), constants.PKCE_STORAGE_NAME);
} else if (storageUtil.browserHasLocalStorage()) {
return storageBuilder(storageUtil.getLocalStorage(), constants.PKCE_STORAGE_NAME);
Expand Down
17 changes: 15 additions & 2 deletions packages/okta-auth-js/lib/pkce.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ function generateVerifier(prefix) {
return encodeURIComponent(verifier).slice(0, MAX_VERIFIER_LENGTH);
}

function getStorage(sdk) {
return sdk.options.storageUtil.getPKCEStorage(sdk.options.cookies);
function getStorage(sdk, options) {
options = util.extend({}, sdk.options.cookies, options);
return sdk.options.storageUtil.getPKCEStorage(options);
}

function saveMeta(sdk, meta) {
Expand All @@ -53,6 +54,18 @@ function saveMeta(sdk, meta) {
function loadMeta(sdk) {
var storage = getStorage(sdk);
var obj = storage.getStorage();
// Verify the Meta
if (!obj.codeVerifier) {
// If meta is not valid, try reading from localStorage.
// This is for compatibility with older versions of the signin widget.
Copy link
Contributor

@swiftone swiftone Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love a Jira ticket reference here so someone could look up the specifics and know if the comment still applies.

storage = getStorage(sdk, { preferLocalStorage: true });
obj = storage.getStorage();
if (!obj.codeVerifier) {
// If meta is not valid, throw an exception to avoid misleading server-side error
// The most likely cause of this error is trying to handle a callback twice
throw new AuthSdkError('Could not load PKCE codeVerifier from storage');
}
}
return obj;
}

Expand Down
7 changes: 7 additions & 0 deletions packages/okta-auth-js/test/spec/browserStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ describe('browserStorage', () => {
expect(browserStorage.getCookieStorage).toHaveBeenCalledWith(opts);
});

it('Uses localStorage instead of sessionStorage if options.preferLocalStorage is set', () => {
browserStorage.getPKCEStorage({ preferLocalStorage: true });
expect(storageBuilder).toHaveBeenCalledTimes(1);
// .toHaveBeenCalledWith doesn't do a strict comparison, so an empty localStorage reads the same as an empty sessionStorage
expect(storageBuilder.mock.calls[0][0]).toBe(global.window.localStorage);
expect(storageBuilder.mock.calls[0][1]).toBe('okta-pkce-storage');
});
});

describe('getHttpCache', () => {
Expand Down
46 changes: 46 additions & 0 deletions packages/okta-auth-js/test/spec/pkce.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,52 @@ var token = require('../../lib/token');
var oauthUtil = require('../../lib/oauthUtil');

describe('pkce', function() {
afterEach(() => {
window.sessionStorage.clear();
window.localStorage.clear();
});
describe('clearMeta', () => {
it('clears meta from sessionStorage', () => {
const meta = { codeVerifier: 'fake', redirectUri: 'http://localhost/fake' };
window.sessionStorage.setItem('okta-pkce-storage', JSON.stringify(meta));
const sdk = new OktaAuth({ issuer: 'https://foo.com' });
pkce.clearMeta(sdk);
const res = JSON.parse(window.sessionStorage.getItem('okta-pkce-storage'));
expect(res).toEqual({});
});
});
describe('saveMeta', () => {
it('saves meta in sessionStorage', () => {
const meta = { codeVerifier: 'fake', redirectUri: 'http://localhost/fake' };
const sdk = new OktaAuth({ issuer: 'https://foo.com' });
pkce.saveMeta(sdk, meta);
const res = JSON.parse(window.sessionStorage.getItem('okta-pkce-storage'));
expect(res).toEqual(meta);
});
});
describe('loadMeta', () => {
it('can return the meta from sessionStorage', () => {
const meta = { codeVerifier: 'fake' };
window.sessionStorage.setItem('okta-pkce-storage', JSON.stringify(meta));
const sdk = new OktaAuth({ issuer: 'https://foo.com' });
const res = pkce.loadMeta(sdk);
expect(res.codeVerifier).toBe(meta.codeVerifier);
});
it('can return the meta from localStorage', () => {
const meta = { codeVerifier: 'fake' };
window.localStorage.setItem('okta-pkce-storage', JSON.stringify(meta));
const sdk = new OktaAuth({ issuer: 'https://foo.com' });
const res = pkce.loadMeta(sdk);
expect(res.codeVerifier).toBe(meta.codeVerifier);
});
it('throws an error if meta cannot be found', () => {
const sdk = new OktaAuth({ issuer: 'https://foo.com' });
const fn = () => {
pkce.loadMeta(sdk);
};
expect(fn).toThrowError('Could not load PKCE codeVerifier from storage');
});
});

describe('prepare oauth params', function() {

Expand Down