diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js new file mode 100644 index 0000000..7365783 --- /dev/null +++ b/src/components/accounts/actions.js @@ -0,0 +1,97 @@ +import authentication from 'services/api/authentication'; +import accounts from 'services/api/accounts'; +import { updateUser, logout } from 'components/user/actions'; + +/** + * @typedef {object} Account + * @property {string} account.id + * @property {string} account.username + * @property {string} account.email + * @property {string} account.token + * @property {string} account.refreshToken + */ + +/** + * @param {Account|object} account + * @param {string} account.token + * @param {string} account.refreshToken + */ +export function authenticate({token, refreshToken}) { + return (dispatch) => { + return authentication.validateToken({token, refreshToken}) + .then(({token, refreshToken}) => + accounts.current({token}) + .then((user) => ({ + user, + account: { + id: user.id, + username: user.username, + email: user.email, + token, + refreshToken + } + })) + ) + .then(({user, account}) => { + dispatch(add(account)); + dispatch(activate(account)); + dispatch(updateUser(user)); + + return account; + }); + }; +} + +/** + * @param {Account} account + */ +export function revoke(account) { + return (dispatch, getState) => { + dispatch(remove(account)); + + if (getState().accounts.length) { + return dispatch(authenticate(getState().accounts[0])); + } else { + return dispatch(logout()); + } + }; +} + +export const ADD = 'accounts:add'; +/** + * @api private + * + * @param {Account} account + */ +export function add(account) { + return { + type: ADD, + payload: account + }; +} + +export const REMOVE = 'accounts:remove'; +/** + * @api private + * + * @param {Account} account + */ +export function remove(account) { + return { + type: REMOVE, + payload: account + }; +} + +export const ACTIVATE = 'accounts:activate'; +/** + * @api private + * + * @param {Account} account + */ +export function activate(account) { + return { + type: ACTIVATE, + payload: account + }; +} diff --git a/src/components/accounts/reducer.js b/src/components/accounts/reducer.js new file mode 100644 index 0000000..eea892a --- /dev/null +++ b/src/components/accounts/reducer.js @@ -0,0 +1,58 @@ +import { ADD, REMOVE, ACTIVATE } from './actions'; + +/** + * @typedef {AccountsState} + * @property {Account} active + * @property {Account[]} available + */ + +/** + * @param {AccountsState} state + * @param {string} options.type + * @param {object} options.payload + * + * @return {AccountsState} + */ +export default function accounts( + state, + {type, payload = {}} +) { + switch (type) { + case ADD: + if (!payload || !payload.id || !payload.token || !payload.refreshToken) { + throw new Error('Invalid or empty payload passed for accounts.add'); + } + + if (!state.available.some((account) => account.id === payload.id)) { + state.available = state.available.concat(payload); + } + + return state; + + case ACTIVATE: + if (!payload || !payload.id || !payload.token || !payload.refreshToken) { + throw new Error('Invalid or empty payload passed for accounts.add'); + } + + return { + ...state, + active: payload + }; + + case REMOVE: + if (!payload || !payload.id) { + throw new Error('Invalid or empty payload passed for accounts.remove'); + } + + return { + ...state, + available: state.available.filter((account) => account.id !== payload.id) + }; + + default: + return { + active: null, + available: [] + }; + } +} diff --git a/src/components/auth/actions.js b/src/components/auth/actions.js index 442db44..cbe2fce 100644 --- a/src/components/auth/actions.js +++ b/src/components/auth/actions.js @@ -36,7 +36,7 @@ export function login({login = '', password = '', rememberMe = false}) { return dispatch(needActivation()); } else if (resp.errors.login === LOGIN_REQUIRED && password) { // return to the first step - dispatch(logout()); + return dispatch(logout()); } } diff --git a/src/components/user/User.js b/src/components/user/User.js index ff61eb2..568d8b2 100644 --- a/src/components/user/User.js +++ b/src/components/user/User.js @@ -4,7 +4,7 @@ const KEY_USER = 'user'; export default class User { /** - * @param {object|string|undefined} data plain object or jwt token or empty to load from storage + * @param {object} [data] - plain object or jwt token or empty to load from storage * * @return {User} */ @@ -18,8 +18,6 @@ export default class User { const defaults = { id: null, uuid: null, - token: '', - refreshToken: '', username: '', email: '', // will contain user's email or masked email @@ -27,12 +25,18 @@ export default class User { maskedEmail: '', avatar: '', lang: '', - goal: null, // the goal with wich user entered site - isGuest: true, isActive: false, shouldAcceptRules: false, // whether user need to review updated rules passwordChangedAt: null, hasMojangUsernameCollision: false, + + // frontend app specific attributes + isGuest: true, + goal: null, // the goal with wich user entered site + + // TODO: the following does not belongs here. Move it later + token: '', + refreshToken: '', }; const user = Object.keys(defaults).reduce((user, key) => { diff --git a/src/components/user/middlewares/bearerHeaderMiddleware.js b/src/components/user/middlewares/bearerHeaderMiddleware.js index caca14a..cb1c9db 100644 --- a/src/components/user/middlewares/bearerHeaderMiddleware.js +++ b/src/components/user/middlewares/bearerHeaderMiddleware.js @@ -8,14 +8,18 @@ */ export default function bearerHeaderMiddleware({getState}) { return { - before(data) { - const {token} = getState().user; + before(req) { + let {token} = getState().user; - if (token) { - data.options.headers.Authorization = `Bearer ${token}`; + if (req.options.token) { + token = req.options.token; } - return data; + if (token) { + req.options.headers.Authorization = `Bearer ${token}`; + } + + return req; } }; } diff --git a/src/components/user/middlewares/refreshTokenMiddleware.js b/src/components/user/middlewares/refreshTokenMiddleware.js index faeb7ca..9f6cc9c 100644 --- a/src/components/user/middlewares/refreshTokenMiddleware.js +++ b/src/components/user/middlewares/refreshTokenMiddleware.js @@ -12,12 +12,12 @@ import {updateUser, logout} from '../actions'; */ export default function refreshTokenMiddleware({dispatch, getState}) { return { - before(data) { + before(req) { const {refreshToken, token} = getState().user; - const isRefreshTokenRequest = data.url.includes('refresh-token'); + const isRefreshTokenRequest = req.url.includes('refresh-token'); - if (!token || isRefreshTokenRequest) { - return data; + if (!token || isRefreshTokenRequest || req.options.autoRefreshToken === false) { + return req; } try { @@ -25,33 +25,17 @@ export default function refreshTokenMiddleware({dispatch, getState}) { const jwt = getJWTPayload(token); if (jwt.exp - SAFETY_FACTOR < Date.now() / 1000) { - return requestAccessToken(refreshToken, dispatch).then(() => data); + return requestAccessToken(refreshToken, dispatch).then(() => req); } } catch (err) { dispatch(logout()); } - return data; + return req; }, - catch(resp, restart) { - /* - { - "name": "Unauthorized", - "message": "You are requesting with an invalid credential.", - "code": 0, - "status": 401, - "type": "yii\\web\\UnauthorizedHttpException" - } - { - "name": "Unauthorized", - "message": "Token expired", - "code": 0, - "status": 401, - "type": "yii\\web\\UnauthorizedHttpException" - } - */ - if (resp && resp.status === 401) { + catch(resp, req, restart) { + if (resp && resp.status === 401 && req.options.autoRefreshToken !== false) { const {refreshToken} = getState().user; if (resp.message === 'Token expired' && refreshToken) { // request token and retry diff --git a/src/services/api/accounts.js b/src/services/api/accounts.js index 34dac16..014ec76 100644 --- a/src/services/api/accounts.js +++ b/src/services/api/accounts.js @@ -1,8 +1,18 @@ import request from 'services/request'; export default { - current() { - return request.get('/api/accounts/current'); + /** + * @param {object} options + * @param {object} [options.token] - an optional token to overwrite headers in middleware + * @param {bool} [options.autoRefreshToken=true] - disable token auto refresh during request + * + * @return {Promise} + */ + current(options = {}) { + return request.get('/api/accounts/current', {}, { + token: options.token, + autoRefreshToken: options.autoRefreshToken + }); }, changePassword({ diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index 52e5fa7..7c005db 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -1,4 +1,5 @@ import request from 'services/request'; +import accounts from 'services/api/accounts'; export default { login({ @@ -36,6 +37,23 @@ export default { ); }, + /** + * Resolves if token is valid + * + * @param {object} options + * @param {string} options.token + * @param {string} options.refreshToken + * + * @return {Promise} - resolves with options.token or with a new token + * if it was refreshed + */ + validateToken({token, refreshToken}) { + // TODO: use refresh token to get fresh token. Dont forget, that it may be broken by refreshTokenMiddleware + // TODO: cover with tests + return accounts.current({token, autoRefreshToken: false}) + .then(() => ({token, refreshToken})); + }, + /** * Request new access token using a refreshToken * diff --git a/src/services/request/request.js b/src/services/request/request.js index a94a433..7ea6640 100644 --- a/src/services/request/request.js +++ b/src/services/request/request.js @@ -5,33 +5,36 @@ const middlewareLayer = new PromiseMiddlewareLayer(); export default { /** * @param {string} url - * @param {object} data + * @param {object} data - request data + * @param {object} options - additional options for fetch or middlewares * * @return {Promise} */ - post(url, data) { + post(url, data, options = {}) { return doFetch(url, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded; charset=UTF-8' }, - body: buildQuery(data) + body: buildQuery(data), + ...options }); }, /** * @param {string} url - * @param {object} data + * @param {object} data - request data + * @param {object} options - additional options for fetch or middlewares * * @return {Promise} */ - get(url, data) { - if (typeof data === 'object') { + get(url, data, options = {}) { + if (typeof data === 'object' && Object.keys(data).length) { const separator = url.indexOf('?') === -1 ? '?' : '&'; url += separator + buildQuery(data); } - return doFetch(url); + return doFetch(url, options); }, /** @@ -82,8 +85,8 @@ function doFetch(url, options = {}) { .then(checkStatus) .then(toJSON, rejectWithJSON) .then(handleResponseSuccess) - .then((resp) => middlewareLayer.run('then', resp)) - .catch((resp) => middlewareLayer.run('catch', resp, () => doFetch(url, options))) + .then((resp) => middlewareLayer.run('then', resp, {url, options})) + .catch((resp) => middlewareLayer.run('catch', resp, {url, options}, () => doFetch(url, options))) ; } diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js new file mode 100644 index 0000000..2cd69d7 --- /dev/null +++ b/tests/components/accounts/actions.test.js @@ -0,0 +1,134 @@ +import expect from 'unexpected'; + +import accounts from 'services/api/accounts'; +import { authenticate, revoke, add, activate, remove, ADD, REMOVE, ACTIVATE } from 'components/accounts/actions'; + +import { updateUser, logout } from 'components/user/actions'; + +const account = { + id: 1, + username: 'username', + email: 'email@test.com', + token: 'foo', + refreshToken: 'foo' +}; + +const user = { + id: 1, + username: 'username', + email: 'email@test.com', +}; + +describe('Accounts actions', () => { + let dispatch; + let getState; + + beforeEach(() => { + dispatch = sinon.spy(function dispatch(arg) { + return typeof arg === 'function' ? arg(dispatch, getState) : arg; + }).named('dispatch'); + getState = sinon.stub().named('getState'); + + getState.returns({ + accounts: [], + user: {} + }); + + sinon.stub(accounts, 'current').named('accounts.current'); + accounts.current.returns(Promise.resolve(user)); + }); + + afterEach(() => { + accounts.current.restore(); + }); + + describe('#authenticate()', () => { + it('should request user state using token', () => { + authenticate(account)(dispatch); + + expect(accounts.current, 'to have a call satisfying', [ + {token: account.token} + ]); + }); + + it(`dispatches ${ADD} action`, () => + authenticate(account)(dispatch).then(() => + expect(dispatch, 'to have a call satisfying', [ + add(account) + ]) + ) + ); + + it(`dispatches ${ACTIVATE} action`, () => + authenticate(account)(dispatch).then(() => + expect(dispatch, 'to have a call satisfying', [ + activate(account) + ]) + ) + ); + + it('should update user state', () => + authenticate(account)(dispatch).then(() => + expect(dispatch, 'to have a call satisfying', [ + updateUser(user) + ]) + ) + ); + + it('resolves with account', () => + authenticate(account)(dispatch).then((resp) => + expect(resp, 'to equal', account) + ) + ); + + it('rejects when bad auth data', () => { + accounts.current.returns(Promise.reject({})); + + const promise = authenticate(account)(dispatch); + + expect(promise, 'to be rejected'); + + return promise.catch(() => { + expect(dispatch, 'was not called'); + return Promise.resolve(); + }); + }); + }); + + describe('#revoke()', () => { + it(`should dispatch ${REMOVE} action`, () => { + revoke(account)(dispatch, getState); + + expect(dispatch, 'to have a call satisfying', [ + remove(account) + ]); + }); + + it('should switch next account if available', () => { + const account2 = {...account, id: 2}; + + getState.returns({ + accounts: [account2] + }); + + return revoke(account)(dispatch, getState).then(() => + expect(dispatch, 'to have calls satisfying', [ + [remove(account)], + [expect.it('to be a function')] + // [authenticate(account2)] // TODO: this is not a plain action. How should we simplify its testing? + ]) + ); + }); + + it('should logout if no other accounts available', () => { + revoke(account)(dispatch, getState) + .then(() => + expect(dispatch, 'to have calls satisfying', [ + [remove(account)], + [expect.it('to be a function')] + // [logout()] // TODO: this is not a plain action. How should we simplify its testing? + ]) + ); + }); + }); +}); diff --git a/tests/components/accounts/reducer.test.js b/tests/components/accounts/reducer.test.js new file mode 100644 index 0000000..d9d34ec --- /dev/null +++ b/tests/components/accounts/reducer.test.js @@ -0,0 +1,87 @@ +import expect from 'unexpected'; + +import accounts from 'components/accounts/reducer'; +import { ADD, REMOVE, ACTIVATE } from 'components/accounts/actions'; + +const account = { + id: 1, + username: 'username', + email: 'email@test.com', + token: 'foo', + refreshToken: 'foo' +}; + +describe('Accounts reducer', () => { + let initial; + + beforeEach(() => { + initial = accounts(null, {}); + }); + + it('should be empty', () => expect(accounts(null, {}), 'to equal', { + active: null, + available: [] + })); + + describe(ACTIVATE, () => { + it('sets active account', () => { + expect(accounts(initial, { + type: ACTIVATE, + payload: account + }), 'to satisfy', { + active: account + }); + }); + }); + + describe(ADD, () => { + it('adds an account', () => + expect(accounts(initial, { + type: ADD, + payload: account + }), 'to satisfy', { + available: [account] + }) + ); + + it('should not add the same account twice', () => + expect(accounts({...initial, available: [account]}, { + type: ADD, + payload: account + }), 'to satisfy', { + available: [account] + }) + ); + + it('throws, when account is invalid', () => { + expect(() => accounts(initial, { + type: ADD + }), 'to throw', 'Invalid or empty payload passed for accounts.add'); + + expect(() => accounts(initial, { + type: ADD, + payload: {} + }), 'to throw', 'Invalid or empty payload passed for accounts.add'); + }); + }); + + describe(REMOVE, () => { + it('should remove an account', () => + expect(accounts({...initial, available: [account]}, { + type: REMOVE, + payload: account + }), 'to equal', initial) + ); + + it('throws, when account is invalid', () => { + expect(() => accounts(initial, { + type: REMOVE + }), 'to throw', 'Invalid or empty payload passed for accounts.remove'); + + expect(() => accounts(initial, { + type: REMOVE, + payload: {} + }), 'to throw', 'Invalid or empty payload passed for accounts.remove'); + }); + }); +}); diff --git a/tests/components/user/middlewares/bearerHeaderMiddleware.test.js b/tests/components/user/middlewares/bearerHeaderMiddleware.test.js index 0d2ea5e..49ee57b 100644 --- a/tests/components/user/middlewares/bearerHeaderMiddleware.test.js +++ b/tests/components/user/middlewares/bearerHeaderMiddleware.test.js @@ -3,7 +3,7 @@ import expect from 'unexpected'; import bearerHeaderMiddleware from 'components/user/middlewares/bearerHeaderMiddleware'; describe('bearerHeaderMiddleware', () => { - it('should set Authorization header', () => { + describe('when token available', () => { const token = 'foo'; const middleware = bearerHeaderMiddleware({ getState: () => ({ @@ -11,16 +11,34 @@ describe('bearerHeaderMiddleware', () => { }) }); - const data = { - options: { - headers: {} - } - }; + it('should set Authorization header', () => { + const data = { + options: { + headers: {} + } + }; - middleware.before(data); + middleware.before(data); - expect(data.options.headers, 'to satisfy', { - Authorization: `Bearer ${token}` + expect(data.options.headers, 'to satisfy', { + Authorization: `Bearer ${token}` + }); + }); + + it('overrides user.token with options.token if available', () => { + const tokenOverride = 'tokenOverride'; + const data = { + options: { + headers: {}, + token: tokenOverride + } + }; + + middleware.before(data); + + expect(data.options.headers, 'to satisfy', { + Authorization: `Bearer ${tokenOverride}` + }); }); }); diff --git a/tests/components/user/middlewares/refreshTokenMiddleware.test.js b/tests/components/user/middlewares/refreshTokenMiddleware.test.js index bd8559c..8282b02 100644 --- a/tests/components/user/middlewares/refreshTokenMiddleware.test.js +++ b/tests/components/user/middlewares/refreshTokenMiddleware.test.js @@ -28,29 +28,64 @@ describe('refreshTokenMiddleware', () => { }); describe('#before', () => { - it('should request new token', () => { - getState.returns({ - user: { - token: expiredToken, - refreshToken - } + describe('when token expired', () => { + beforeEach(() => { + getState.returns({ + user: { + token: expiredToken, + refreshToken + } + }); }); - const data = { - url: 'foo', - options: { - headers: {} - } - }; + it('should request new token', () => { + const data = { + url: 'foo', + options: { + headers: {} + } + }; - authentication.requestToken.returns(Promise.resolve({token: validToken})); + authentication.requestToken.returns(Promise.resolve({token: validToken})); + + return middleware.before(data).then((resp) => { + expect(resp, 'to satisfy', data); + + expect(authentication.requestToken, 'to have a call satisfying', [ + refreshToken + ]); + }); + }); + + it('should not apply to refresh-token request', () => { + const data = {url: '/refresh-token'}; + const resp = middleware.before(data); - return middleware.before(data).then((resp) => { expect(resp, 'to satisfy', data); - expect(authentication.requestToken, 'to have a call satisfying', [ - refreshToken - ]); + expect(authentication.requestToken, 'was not called'); + }); + + it('should not apply if options.autoRefreshToken === false', () => { + const data = { + url: 'foo', + options: {autoRefreshToken: false} + }; + middleware.before(data); + + expect(authentication.requestToken, 'was not called'); + }); + + xit('should update user with new token'); // TODO: need a way to test, that action was called + xit('should logout if invalid token'); // TODO: need a way to test, that action was called + + xit('should logout if token request failed', () => { + authentication.requestToken.returns(Promise.reject()); + + return middleware.before({url: 'foo'}).then((resp) => { + // TODO: need a way to test, that action was called + expect(dispatch, 'to have a call satisfying', logout); + }); }); }); @@ -66,74 +101,78 @@ describe('refreshTokenMiddleware', () => { expect(authentication.requestToken, 'was not called'); }); - - it('should not apply to refresh-token request', () => { - getState.returns({ - user: {} - }); - - const data = {url: '/refresh-token'}; - const resp = middleware.before(data); - - expect(resp, 'to satisfy', data); - - expect(authentication.requestToken, 'was not called'); - }); - - xit('should update user with new token'); // TODO: need a way to test, that action was called - xit('should logout if invalid token'); // TODO: need a way to test, that action was called - - xit('should logout if token request failed', () => { - getState.returns({ - user: { - token: expiredToken, - refreshToken - } - }); - - authentication.requestToken.returns(Promise.reject()); - - return middleware.before({url: 'foo'}).then((resp) => { - // TODO: need a way to test, that action was called - expect(dispatch, 'to have a call satisfying', logout); - }); - }); }); describe('#catch', () => { - it('should request new token', () => { + const expiredResponse = { + name: 'Unauthorized', + message: 'Token expired', + code: 0, + status: 401, + type: 'yii\\web\\UnauthorizedHttpException' + }; + + const badTokenReponse = { + name: 'Unauthorized', + message: 'Token expired', + code: 0, + status: 401, + type: 'yii\\web\\UnauthorizedHttpException' + }; + + let restart; + + beforeEach(() => { getState.returns({ user: { refreshToken } }); - const restart = sinon.stub().named('restart'); + restart = sinon.stub().named('restart'); authentication.requestToken.returns(Promise.resolve({token: validToken})); + }); - return middleware.catch({ - status: 401, - message: 'Token expired' - }, restart).then(() => { + it('should request new token if expired', () => + middleware.catch(expiredResponse, {options: {}}, restart).then(() => { expect(authentication.requestToken, 'to have a call satisfying', [ refreshToken ]); expect(restart, 'was called'); + }) + ); + + xit('should logout user if token cannot be refreshed', () => { + // TODO: need a way to test, that action was called + return middleware.catch(badTokenReponse, {options: {}}, restart).then(() => { + // TODO }); }); - xit('should logout user if token cannot be refreshed'); // TODO: need a way to test, that action was called + it('should pass the request through if options.autoRefreshToken === false', () => { + const promise = middleware.catch(expiredResponse, { + options: { + autoRefreshToken: false + } + }, restart); + + return expect(promise, 'to be rejected with', expiredResponse).then(() => { + expect(restart, 'was not called'); + expect(authentication.requestToken, 'was not called'); + }); + }); it('should pass the rest of failed requests through', () => { const resp = {}; - const promise = middleware.catch(resp); + const promise = middleware.catch(resp, { + options: {} + }, restart); - expect(promise, 'to be rejected'); - - return promise.catch((actual) => { - expect(actual, 'to be', resp); + return expect(promise, 'to be rejected with', resp).then(() => { + expect(restart, 'was not called'); + expect(authentication.requestToken, 'was not called'); }); }); });