Merge branch '246-auth-refactoring' into develop

This commit is contained in:
SleepWalker 2017-01-12 08:14:43 +02:00
commit befc058a3c
12 changed files with 259 additions and 390 deletions

View File

@ -1,16 +1,17 @@
import { routeActions } from 'react-router-redux';
import authentication from 'services/api/authentication';
import accounts from 'services/api/accounts';
import { updateUser, logout } from 'components/user/actions';
import { updateUser, setGuest } from 'components/user/actions';
import { setLocale } from 'components/i18n/actions';
import logger from 'services/logger';
/**
* @typedef {object} Account
* @property {string} account.id
* @property {string} account.username
* @property {string} account.email
* @property {string} account.token
* @property {string} account.refreshToken
* @property {string} id
* @property {string} username
* @property {string} email
* @property {string} token
* @property {string} refreshToken
*/
/**
@ -23,15 +24,15 @@ import logger from 'services/logger';
export function authenticate({token, refreshToken}) {
return (dispatch) =>
authentication.validateToken({token, refreshToken})
.catch(() => {
// TODO: log this case
dispatch(logout());
.catch((resp) => {
logger.warn('Error validating token during auth', {
resp
});
return Promise.reject();
return dispatch(logoutAll())
.then(() => Promise.reject());
})
.then(({token, refreshToken}) =>
accounts.current({token})
.then((user) => ({
.then(({token, refreshToken, user}) => ({
user: {
isGuest: false,
...user
@ -44,7 +45,6 @@ export function authenticate({token, refreshToken}) {
refreshToken
}
}))
)
.then(({user, account}) => {
dispatch(add(account));
dispatch(activate(account));
@ -80,17 +80,23 @@ export function revoke(account) {
});
}
return dispatch(logout());
return dispatch(logoutAll());
};
}
export function logoutAll() {
return (dispatch, getState) => {
dispatch(setGuest());
const {accounts: {available}} = getState();
available.forEach((account) => authentication.logout(account));
dispatch(reset());
dispatch(routeActions.push('/login'));
return Promise.resolve();
};
}
@ -104,10 +110,11 @@ export function logoutAll() {
*/
export function logoutStrangers() {
return (dispatch, getState) => {
const {accounts: {available}} = getState();
const {accounts: {available, active}} = getState();
const isStranger = ({refreshToken, id}) => !refreshToken && !sessionStorage.getItem(`stranger${id}`);
if (available.some(isStranger)) {
const accountToReplace = available.filter((account) => !isStranger(account))[0];
if (accountToReplace) {
@ -117,10 +124,13 @@ export function logoutStrangers() {
authentication.logout(account);
});
if (isStranger(active)) {
return dispatch(authenticate(accountToReplace));
}
dispatch(logout());
} else {
return dispatch(logoutAll());
}
}
return Promise.resolve();
};

View File

@ -1,12 +1,16 @@
import { routeActions } from 'react-router-redux';
import { updateUser, logout, acceptRules as userAcceptRules } from 'components/user/actions';
import { authenticate } from 'components/accounts/actions';
import logger from 'services/logger';
import { updateUser, acceptRules as userAcceptRules } from 'components/user/actions';
import { authenticate, logoutAll } from 'components/accounts/actions';
import authentication from 'services/api/authentication';
import oauth from 'services/api/oauth';
import signup from 'services/api/signup';
import dispatchBsod from 'components/ui/bsod/dispatchBsod';
export { updateUser } from 'components/user/actions';
export { authenticate, logoutAll as logout } from 'components/accounts/actions';
export function login({login = '', password = '', rememberMe = false}) {
const PASSWORD_REQUIRED = 'error.password_required';
const LOGIN_REQUIRED = 'error.login_required';
@ -24,9 +28,9 @@ export function login({login = '', password = '', rememberMe = false}) {
} else if (resp.errors.login === ACTIVATION_REQUIRED) {
return dispatch(needActivation());
} else if (resp.errors.login === LOGIN_REQUIRED && password) {
// TODO: log this case to backend
// return to the first step
return dispatch(logout());
logger.warn('No login on password panel');
return dispatch(logoutAll());
}
}
@ -144,9 +148,6 @@ export function clearErrors() {
return setErrors(null);
}
export { logout, updateUser } from 'components/user/actions';
export { authenticate } from 'components/accounts/actions';
/**
* @param {object} oauthData
* @param {string} oauthData.clientId

View File

@ -1,66 +1,18 @@
import { PropTypes } from 'react';
const KEY_USER = 'user';
export default class User {
/**
* @param {object} [data] - plain object or jwt token or empty to load from storage
*
* @return {User}
/**
* @typedef {object} User
* @property {number} id
* @property {string} uuid
* @property {string} token
* @property {string} username
* @property {string} email
* @property {string} avatar
* @property {bool} isGuest
* @property {bool} isActive
* @property {number} passwordChangedAt - timestamp
* @property {bool} hasMojangUsernameCollision
*/
constructor(data) {
if (!data) {
return this.load();
}
// TODO: strict value types validation
const defaults = {
id: null,
uuid: null,
username: '',
email: '',
// will contain user's email or masked email
// (e.g. ex**ple@em*il.c**) depending on what information user have already provided
maskedEmail: '',
avatar: '',
lang: '',
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: remove me after migration to multy accs
token: '',
refreshToken: ''
};
const user = Object.keys(defaults).reduce((user, key) => {
if (data.hasOwnProperty(key)) {
user[key] = data[key];
}
return user;
}, defaults);
localStorage.setItem(KEY_USER, JSON.stringify(user));
return user;
}
load() {
try {
return new User(JSON.parse(localStorage.getItem(KEY_USER)));
} catch (error) {
return new User({isGuest: true});
}
}
}
export const userShape = PropTypes.shape({
id: PropTypes.number,
uuid: PropTypes.string,

View File

@ -1,7 +1,4 @@
import { routeActions } from 'react-router-redux';
import accounts from 'services/api/accounts';
import { logoutAll } from 'components/accounts/actions';
import { setLocale } from 'components/i18n/actions';
export const UPDATE = 'USER_UPDATE';
@ -18,6 +15,20 @@ export function updateUser(payload) {
};
}
export const SET = 'USER_SET';
/**
* Replace current user's state with a new one
*
* @param {User} payload
* @return {object} - action definition
*/
export function setUser(payload) {
return {
type: SET,
payload
};
}
export const CHANGE_LANG = 'USER_CHANGE_LANG';
export function changeLang(lang) {
return (dispatch, getState) => dispatch(setLocale(lang))
@ -37,32 +48,12 @@ export function changeLang(lang) {
});
}
export const SET = 'USER_SET';
/**
* Replace current user's state with a new one
*
* @param {User} payload
* @return {object} - action definition
*/
export function setUser(payload) {
return {
type: SET,
payload
};
}
export function logout() {
export function setGuest() {
return (dispatch, getState) => {
dispatch(setUser({
lang: getState().user.lang,
isGuest: true
}));
dispatch(logoutAll());
dispatch(routeActions.push('/login'));
return Promise.resolve();
};
}

View File

@ -1,6 +1,7 @@
import { getJwtPayload } from 'functions';
import authentication from 'services/api/authentication';
import { updateToken } from 'components/accounts/actions';
import { logout } from '../actions';
import logger from 'services/logger';
import { updateToken, logoutAll } from 'components/accounts/actions';
/**
* Ensures, that all user's requests have fresh access token
@ -14,7 +15,7 @@ import { logout } from '../actions';
export default function refreshTokenMiddleware({dispatch, getState}) {
return {
before(req) {
const {user, accounts} = getState();
const {accounts} = getState();
let refreshToken;
let token;
@ -24,25 +25,25 @@ export default function refreshTokenMiddleware({dispatch, getState}) {
if (accounts.active) {
token = accounts.active.token;
refreshToken = accounts.active.refreshToken;
} else { // #legacy token
token = user.token;
refreshToken = user.refreshToken;
}
if (!token || req.options.token || isRefreshTokenRequest) {
return req;
return Promise.resolve(req);
}
try {
const SAFETY_FACTOR = 300; // ask new token earlier to overcome time dissynchronization problem
const jwt = getJWTPayload(token);
const jwt = getJwtPayload(token);
if (jwt.exp - SAFETY_FACTOR < Date.now() / 1000) {
return requestAccessToken(refreshToken, dispatch).then(() => req);
}
} catch (err) {
// console.error('Bad token', err); // TODO: it would be cool to log such things to backend
return dispatch(logout()).then(() => req);
logger.warn('Refresh token error: bad token', {
token
});
return dispatch(logoutAll()).then(() => req);
}
return Promise.resolve(req);
@ -50,15 +51,15 @@ export default function refreshTokenMiddleware({dispatch, getState}) {
catch(resp, req, restart) {
if (resp && resp.status === 401 && !req.options.token) {
const {user, accounts} = getState();
const {refreshToken} = accounts.active ? accounts.active : user;
const {accounts} = getState();
const {refreshToken} = accounts.active || {};
if (resp.message === 'Token expired' && refreshToken) {
// request token and retry
return requestAccessToken(refreshToken, dispatch).then(restart);
}
return dispatch(logout()).then(() => Promise.reject(resp));
return dispatch(logoutAll()).then(() => Promise.reject(resp));
}
return Promise.reject(resp);
@ -76,20 +77,7 @@ function requestAccessToken(refreshToken, dispatch) {
return promise
.then(({token}) => dispatch(updateToken(token)))
.catch(() => dispatch(logout()));
.catch(() => dispatch(logoutAll()));
}
function getJWTPayload(jwt) {
const parts = (jwt || '').split('.');
if (parts.length !== 3) {
throw new Error('Invalid jwt token');
}
try {
return JSON.parse(atob(parts[1]));
} catch (err) {
throw new Error('Can not decode jwt token');
}
}

View File

@ -1,10 +1,28 @@
import { UPDATE, SET, CHANGE_LANG } from './actions';
import User from './User';
// TODO: возможно есть смысл инитить обьект User снаружи, так как редусер не должен столько знать
const defaults = {
id: null,
uuid: null,
username: '',
email: '',
// will contain user's email or masked email
// (e.g. ex**ple@em*il.c**) depending on what information user have already provided
maskedEmail: '',
avatar: '',
lang: '',
isActive: false,
shouldAcceptRules: false, // whether user need to review updated rules
passwordChangedAt: null,
hasMojangUsernameCollision: false,
// frontend specific attributes
isGuest: true,
goal: null // the goal with wich user entered site
};
export default function user(
state = new User(),
state = null,
{type, payload = null}
) {
switch (type) {
@ -13,25 +31,32 @@ export default function user(
throw new Error('payload.lang is required for user reducer');
}
return new User({
return {
...state,
lang: payload.lang
});
};
case UPDATE:
if (!payload) {
throw new Error('payload is required for user reducer');
}
return new User({
return {
...state,
...payload
});
};
case SET:
return new User(payload || {});
payload = payload || {};
return {
...defaults,
...payload
};
default:
return state;
return state || {
...defaults
};
}
}

View File

@ -61,3 +61,24 @@ export const rAF = window.requestAnimationFrame
* @param {bool} [immediate=false] - whether to execute at the beginning
*/
export debounce from 'debounce';
/**
* @param {string} jwt
*
* @throws {Error} If can not decode token
*
* @return {object} - decoded jwt payload
*/
export function getJwtPayload(jwt) {
const parts = (jwt || '').split('.');
if (parts.length !== 3) {
throw new Error('Invalid jwt token');
}
try {
return JSON.parse(atob(parts[1]));
} catch (err) {
throw new Error('Can not decode jwt token');
}
}

View File

@ -55,7 +55,8 @@ const authentication = {
* @param {string} options.refreshToken
*
* @return {Promise} - resolves with options.token or with a new token
* if it was refreshed
* if it was refreshed. As a side effect the response
* will have a `user` field with current user data
*/
validateToken({token, refreshToken}) {
return new Promise((resolve) => {
@ -66,11 +67,14 @@ const authentication = {
resolve();
})
.then(() => accounts.current({token}))
.then(() => ({token, refreshToken}))
.then((user) => ({token, refreshToken, user}))
.catch((resp) => {
if (resp.message === 'Token expired') {
return authentication.requestToken(refreshToken)
.then(({token}) => ({token, refreshToken}));
.then(({token}) =>
accounts.current({token})
.then((user) => ({token, refreshToken, user}))
);
}
return Promise.reject(resp);

View File

@ -1,6 +1,8 @@
import expect from 'unexpected';
import sinon from 'sinon';
import { routeActions } from 'react-router-redux';
import accounts from 'services/api/accounts';
import authentication from 'services/api/authentication';
import {
@ -15,7 +17,7 @@ import {
} from 'components/accounts/actions';
import { SET_LOCALE } from 'components/i18n/actions';
import { updateUser } from 'components/user/actions';
import { updateUser, setUser } from 'components/user/actions';
const account = {
id: 1,
@ -43,30 +45,30 @@ describe('components/accounts/actions', () => {
getState = sinon.stub().named('store.getState');
getState.returns({
accounts: [],
accounts: {
available: [],
active: null
},
user: {}
});
sinon.stub(authentication, 'validateToken').named('authentication.validateToken');
authentication.validateToken.returns(Promise.resolve({
token: account.token,
refreshToken: account.refreshToken
refreshToken: account.refreshToken,
user
}));
sinon.stub(accounts, 'current').named('accounts.current');
accounts.current.returns(Promise.resolve(user));
});
afterEach(() => {
authentication.validateToken.restore();
accounts.current.restore();
});
describe('#authenticate()', () => {
it('should request user state using token', () =>
authenticate(account)(dispatch).then(() =>
expect(accounts.current, 'to have a call satisfying', [
{token: account.token}
expect(authentication.validateToken, 'to have a call satisfying', [
{token: account.token, refreshToken: account.refreshToken}
])
)
);
@ -110,17 +112,23 @@ describe('components/accounts/actions', () => {
);
it('rejects when bad auth data', () => {
accounts.current.returns(Promise.reject({}));
authentication.validateToken.returns(Promise.reject({}));
return expect(authenticate(account)(dispatch), 'to be rejected').then(() =>
expect(dispatch, 'was not called')
);
return expect(authenticate(account)(dispatch), 'to be rejected').then(() => {
expect(dispatch, 'to have a call satisfying', [
{payload: {isGuest: true}},
]);
expect(dispatch, 'to have a call satisfying', [
reset()
]);
});
});
it('marks user as stranger, if there is no refreshToken', () => {
const expectedKey = `stranger${account.id}`;
authentication.validateToken.returns(Promise.resolve({
token: account.token
token: account.token,
user
}));
sessionStorage.removeItem(expectedKey);
@ -257,6 +265,25 @@ describe('components/accounts/actions', () => {
reset()
]);
});
it('should redirect to /login', () =>
logoutAll()(dispatch, getState).then(() => {
expect(dispatch, 'to have a call satisfying', [
routeActions.push('/login')
]);
})
);
it('should change user to guest', () =>
logoutAll()(dispatch, getState).then(() => {
expect(dispatch, 'to have a call satisfying', [
setUser({
lang: user.lang,
isGuest: true
})
]);
})
);
});
describe('#logoutStrangers', () => {
@ -274,7 +301,7 @@ describe('components/accounts/actions', () => {
beforeEach(() => {
getState.returns({
accounts: {
active: account,
active: foreignAccount,
available: [account, foreignAccount, foreignAccount2]
},
user
@ -316,6 +343,37 @@ describe('components/accounts/actions', () => {
)
);
it('should not activate another account if active account is already not a stranger', () => {
getState.returns({
accounts: {
active: account,
available: [account, foreignAccount]
},
user
});
return logoutStrangers()(dispatch, getState)
.then(() =>
expect(dispatch, 'was always called with',
expect.it('not to satisfy', activate(account)))
);
});
it('should not dispatch if no strangers', () => {
getState.returns({
accounts: {
active: account,
available: [account]
},
user
});
return logoutStrangers()(dispatch, getState)
.then(() =>
expect(dispatch, 'was not called')
);
});
describe('when all accounts are strangers', () => {
beforeEach(() => {
getState.returns({
@ -341,7 +399,7 @@ describe('components/accounts/actions', () => {
});
});
describe('when an stranger has a mark in sessionStorage', () => {
describe('when a stranger has a mark in sessionStorage', () => {
const key = `stranger${foreignAccount.id}`;
beforeEach(() => {
@ -355,12 +413,9 @@ describe('components/accounts/actions', () => {
});
it('should not log out', () =>
expect(dispatch, 'to have calls satisfying', [
[expect.it('not to equal', {payload: foreignAccount})],
// for some reason it says, that dispatch(authenticate(...))
// must be removed if only one args assertion is listed :(
[expect.it('not to equal', {payload: foreignAccount})]
])
expect(dispatch, 'was always called with',
expect.it('not to equal', {payload: foreignAccount})
)
);
});
});

View File

@ -1,134 +1,2 @@
import expect from 'unexpected';
import sinon from 'sinon';
import { routeActions } from 'react-router-redux';
import request from 'services/request';
import { reset, RESET } from 'components/accounts/actions';
import {
logout,
setUser
} from 'components/user/actions';
describe('components/user/actions', () => {
const getState = sinon.stub().named('store.getState');
const dispatch = sinon.spy((arg) =>
typeof arg === 'function' ? arg(dispatch, getState) : arg
).named('store.dispatch');
const callThunk = function(fn, ...args) {
const thunk = fn(...args);
return thunk(dispatch, getState);
};
beforeEach(() => {
dispatch.reset();
getState.reset();
getState.returns({});
sinon.stub(request, 'get').named('request.get');
sinon.stub(request, 'post').named('request.post');
});
afterEach(() => {
request.get.restore();
request.post.restore();
});
describe('#logout()', () => {
beforeEach(() => {
request.post.returns(Promise.resolve());
});
describe('user with jwt', () => {
const token = 'iLoveRockNRoll';
beforeEach(() => {
getState.returns({
user: {
lang: 'foo'
},
accounts: {
active: {token},
available: [{token}]
}
});
});
it('should post to /api/authentication/logout with user jwt', () =>
callThunk(logout).then(() => {
expect(request.post, 'to have a call satisfying', [
'/api/authentication/logout', {}, {
token: expect.it('not to be empty')
}
]);
})
);
testChangedToGuest();
testAccountsReset();
testRedirectedToLogin();
});
describe('user without jwt', () => {
// (a guest with partially filled user's state)
// DEPRECATED
beforeEach(() => {
getState.returns({
user: {
lang: 'foo'
},
accounts: {
active: null,
available: []
}
});
});
it('should not post to /api/authentication/logout', () =>
callThunk(logout).then(() => {
expect(request.post, 'was not called');
})
);
testChangedToGuest();
testAccountsReset();
testRedirectedToLogin();
});
function testChangedToGuest() {
it('should change user to guest', () =>
callThunk(logout).then(() => {
expect(dispatch, 'to have a call satisfying', [
setUser({
lang: 'foo',
isGuest: true
})
]);
})
);
}
function testRedirectedToLogin() {
it('should redirect to /login', () =>
callThunk(logout).then(() => {
expect(dispatch, 'to have a call satisfying', [
routeActions.push('/login')
]);
})
);
}
function testAccountsReset() {
it(`should dispatch ${RESET}`, () =>
callThunk(logout).then(() => {
expect(dispatch, 'to have a call satisfying', [
reset()
]);
})
);
}
});
});

View File

@ -1,4 +1,5 @@
import expect from 'unexpected';
import sinon from 'sinon';
import refreshTokenMiddleware from 'components/user/middlewares/refreshTokenMiddleware';
@ -75,9 +76,11 @@ describe('refreshTokenMiddleware', () => {
const data = {url: '/refresh-token', options: {}};
const resp = middleware.before(data);
expect(resp, 'to satisfy', data);
expect(authentication.requestToken, 'was not called');
return expect(resp, 'to be fulfilled with', data)
.then(() =>
expect(authentication.requestToken, 'was not called')
);
});
it('should not auto refresh token if options.token specified', () => {
@ -142,40 +145,6 @@ describe('refreshTokenMiddleware', () => {
});
});
describe('when token expired legacy user', () => {
beforeEach(() => {
getState.returns({
accounts: {
active: null,
available: []
},
user: {
token: expiredToken,
refreshToken
}
});
});
it('should request new token', () => {
const data = {
url: 'foo',
options: {
headers: {}
}
};
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 be applied if no token', () => {
getState.returns({
accounts: {
@ -187,9 +156,10 @@ describe('refreshTokenMiddleware', () => {
const data = {url: 'foo'};
const resp = middleware.before(data);
expect(resp, 'to satisfy', data);
expect(authentication.requestToken, 'was not called');
return expect(resp, 'to be fulfilled with', data)
.then(() =>
expect(authentication.requestToken, 'was not called')
);
});
});
@ -290,25 +260,5 @@ describe('refreshTokenMiddleware', () => {
expect(authentication.requestToken, 'was not called');
});
});
describe('legacy user.refreshToken', () => {
beforeEach(() => {
getState.returns({
accounts: {
active: null
},
user: {refreshToken}
});
});
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');
})
);
});
});
});

View File

@ -40,11 +40,12 @@ describe('authentication api', () => {
describe('#validateToken()', () => {
const validTokens = {token: 'foo', refreshToken: 'bar'};
const user = {id: 1};
beforeEach(() => {
sinon.stub(accounts, 'current');
accounts.current.returns(Promise.resolve());
accounts.current.returns(Promise.resolve(user));
});
afterEach(() => {
@ -60,8 +61,11 @@ describe('authentication api', () => {
})
);
it('should resolve with both tokens', () =>
expect(authentication.validateToken(validTokens), 'to be fulfilled with', validTokens)
it('should resolve with both tokens and user object', () =>
expect(authentication.validateToken(validTokens), 'to be fulfilled with', {
...validTokens,
user
})
);
it('rejects if token has a bad type', () =>
@ -96,7 +100,7 @@ describe('authentication api', () => {
beforeEach(() => {
sinon.stub(authentication, 'requestToken');
accounts.current.returns(Promise.reject(expiredResponse));
accounts.current.onCall(0).returns(Promise.reject(expiredResponse));
authentication.requestToken.returns(Promise.resolve({token: newToken}));
});
@ -104,9 +108,9 @@ describe('authentication api', () => {
authentication.requestToken.restore();
});
it('resolves with new token', () =>
it('resolves with new token and user object', () =>
expect(authentication.validateToken(validTokens),
'to be fulfilled with', {...validTokens, token: newToken}
'to be fulfilled with', {...validTokens, token: newToken, user}
)
);