From fdd56bc8865320afc2883b0b1f6df927cf98c8bb Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Sat, 27 Aug 2016 13:19:02 +0300 Subject: [PATCH] Improve oauth state restore logic --- src/components/auth/actions.js | 11 ++++- src/services/authFlow/AuthFlow.js | 21 +++++++-- tests/services/authFlow/AuthFlow.test.js | 58 ++++++++++++++++-------- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/components/auth/actions.js b/src/components/auth/actions.js index b8b5f0d..7e5faa1 100644 --- a/src/components/auth/actions.js +++ b/src/components/auth/actions.js @@ -162,12 +162,18 @@ export function oAuthValidate(oauthData) { ); } +/** + * @param {object} params + * @param {bool} params.accept=false + * + * @return {Promise} + */ export function oAuthComplete(params = {}) { - localStorage.removeItem('oauthData'); - return wrapInLoader((dispatch, getState) => oauth.complete(getState().auth.oauth, params) .then((resp) => { + localStorage.removeItem('oauthData'); + if (resp.redirectUri.startsWith('static_page')) { resp.code = resp.redirectUri.match(/code=(.+)&/)[1]; resp.redirectUri = resp.redirectUri.match(/^(.+)\?/)[1]; @@ -195,6 +201,7 @@ export function oAuthComplete(params = {}) { function handleOauthParamsValidation(resp = {}) { dispatchBsod(); + localStorage.removeItem('oauthData'); // eslint-disable-next-line no-alert resp.userMessage && setTimeout(() => alert(resp.userMessage), 500); // 500 ms to allow re-render diff --git a/src/services/authFlow/AuthFlow.js b/src/services/authFlow/AuthFlow.js index d33d925..d79e4b0 100644 --- a/src/services/authFlow/AuthFlow.js +++ b/src/services/authFlow/AuthFlow.js @@ -6,6 +6,7 @@ import OAuthState from './OAuthState'; import ForgotPasswordState from './ForgotPasswordState'; import RecoverPasswordState from './RecoverPasswordState'; import ActivationState from './ActivationState'; +import CompleteState from './CompleteState'; import ResendActivationState from './ResendActivationState'; export default class AuthFlow { @@ -39,8 +40,6 @@ export default class AuthFlow { this.getState = store.getState.bind(store); this.dispatch = store.dispatch.bind(store); - - this.restoreOAuthState(); } resolve(payload = {}) { @@ -131,6 +130,10 @@ export default class AuthFlow { this.currentRequest = request; + if (this.restoreOAuthState()) { + return; + } + switch (path) { case '/register': this.setState(new RegisterState()); @@ -176,15 +179,25 @@ export default class AuthFlow { } /** + * Tries to restore last oauth request, if it was stored in localStorage + * in last 2 hours * @api private + * + * @return {bool} - whether oauth state is being restored */ restoreOAuthState() { try { const data = JSON.parse(localStorage.getItem('oauthData')); + const expirationTime = 2 * 60 * 60 * 1000; // 2h - if (Date.now() - data.timestamp < 60 * 60 * 1000) { - this.run('oAuthValidate', data.payload); + if (Date.now() - data.timestamp < expirationTime) { + this.run('oAuthValidate', data.payload) + .then(() => this.setState(new CompleteState())); + + return true; } } catch (err) {/* bad luck :( */} + + return false; } } diff --git a/tests/services/authFlow/AuthFlow.test.js b/tests/services/authFlow/AuthFlow.test.js index 5ea9661..6372f02 100644 --- a/tests/services/authFlow/AuthFlow.test.js +++ b/tests/services/authFlow/AuthFlow.test.js @@ -10,6 +10,7 @@ import ForgotPasswordState from 'services/authFlow/ForgotPasswordState'; import ActivationState from 'services/authFlow/ActivationState'; import ResendActivationState from 'services/authFlow/ResendActivationState'; import LoginState from 'services/authFlow/LoginState'; +import CompleteState from 'services/authFlow/CompleteState'; describe('AuthFlow', () => { let flow; @@ -34,10 +35,6 @@ describe('AuthFlow', () => { }); describe('#setStore', () => { - afterEach(() => { - localStorage.removeItem('oauthData'); - }); - it('should create #navigate, #getState, #dispatch', () => { flow.setStore({ getState() {}, @@ -48,39 +45,64 @@ describe('AuthFlow', () => { expect(flow.dispatch, 'to be defined'); expect(flow.navigate, 'to be defined'); }); + }); - it('should restore oauth state from localStorage', () => { - const oauthData = {}; + describe('#restoreOAuthState', () => { + let oauthData; + + beforeEach(() => { + oauthData = {foo: 'bar'}; localStorage.setItem('oauthData', JSON.stringify({ timestamp: Date.now() - 10, payload: oauthData })); sinon.stub(flow, 'run').named('flow.run'); + flow.run.returns({then: (fn) => fn()}); + sinon.stub(flow, 'setState').named('flow.setState'); + }); - flow.setStore({ - getState() {}, - dispatch() {} - }); + afterEach(() => { + localStorage.removeItem('oauthData'); + }); + + it('should call to restoreOAuthState', () => { + sinon.stub(flow, 'restoreOAuthState').named('flow.restoreOAuthState'); + + flow.handleRequest({path: '/'}); + + expect(flow.restoreOAuthState, 'was called'); + }); + + it('should restore oauth state from localStorage', () => { + flow.handleRequest({path: '/'}); expect(flow.run, 'to have a call satisfying', [ 'oAuthValidate', oauthData ]); }); + it('should transition to CompleteState', () => { + flow.handleRequest({path: '/'}); + + expect(flow.setState, 'to have a call satisfying', [ + expect.it('to be a', CompleteState) + ]); + }); + + it('should not handle current request', () => { + flow.handleRequest({path: '/'}); + + expect(flow.setState, 'was called once'); + }); + it('should not restore outdated (>1h) oauth state', () => { - const oauthData = {}; localStorage.setItem('oauthData', JSON.stringify({ - timestamp: Date.now() - 60 * 60 * 1000, + timestamp: Date.now() - 2 * 60 * 60 * 1000, payload: oauthData })); - sinon.stub(flow, 'run').named('flow.run'); - - flow.setStore({ - getState() {}, - dispatch() {} - }); + flow.handleRequest({path: '/'}); expect(flow.run, 'was not called'); });