#365: improve token api errors handling

This commit is contained in:
SleepWalker 2017-12-25 22:03:21 +02:00
parent d70ac10721
commit 3f869f92e2
6 changed files with 98 additions and 30 deletions

View File

@ -8,7 +8,7 @@
"license": "UNLICENSED", "license": "UNLICENSED",
"repository": "git@gitlab.ely.by:elyby/accounts.git", "repository": "git@gitlab.ely.by:elyby/accounts.git",
"engines": { "engines": {
"node": "9" "node": ">=8.0.0"
}, },
"scripts": { "scripts": {
"start": "yarn run clean && yarn run build:dll && webpack-dev-server --progress --colors", "start": "yarn run clean && yarn run build:dll && webpack-dev-server --progress --colors",

View File

@ -6,7 +6,6 @@ import { updateUser, setGuest } from 'components/user/actions';
import { setLocale } from 'components/i18n/actions'; import { setLocale } from 'components/i18n/actions';
import { setAccountSwitcher } from 'components/auth/actions'; import { setAccountSwitcher } from 'components/auth/actions';
import logger from 'services/logger'; import logger from 'services/logger';
import { InternalServerError } from 'services/request';
import { import {
add, add,
@ -38,15 +37,9 @@ export function authenticate({token, refreshToken}) {
return (dispatch, getState) => return (dispatch, getState) =>
authentication.validateToken({token, refreshToken}) authentication.validateToken({token, refreshToken})
.catch((resp = {}) => { .catch((resp = {}) => {
if (resp instanceof InternalServerError) {
// delegate error recovering to the later logic
return Promise.reject(resp);
}
logger.warn('Error validating token during auth', {
resp
});
// all the logic to get the valid token was failed,
// we must forget current token, but leave other user's accounts
return dispatch(logoutAll()) return dispatch(logoutAll())
.then(() => Promise.reject(resp)); .then(() => Promise.reject(resp));
}) })

View File

@ -3,7 +3,6 @@ import sinon from 'sinon';
import { browserHistory } from 'services/history'; import { browserHistory } from 'services/history';
import logger from 'services/logger';
import { InternalServerError } from 'services/request'; import { InternalServerError } from 'services/request';
import { sessionStorage } from 'services/localStorage'; import { sessionStorage } from 'services/localStorage';
import authentication from 'services/api/authentication'; import authentication from 'services/api/authentication';
@ -58,7 +57,6 @@ describe('components/accounts/actions', () => {
}); });
sinon.stub(authentication, 'validateToken').named('authentication.validateToken'); sinon.stub(authentication, 'validateToken').named('authentication.validateToken');
sinon.stub(logger, 'warn').named('logger.warn');
authentication.validateToken.returns(Promise.resolve({ authentication.validateToken.returns(Promise.resolve({
token: account.token, token: account.token,
refreshToken: account.refreshToken, refreshToken: account.refreshToken,
@ -68,7 +66,6 @@ describe('components/accounts/actions', () => {
afterEach(() => { afterEach(() => {
authentication.validateToken.restore(); authentication.validateToken.restore();
logger.warn.restore();
}); });
describe('#authenticate()', () => { describe('#authenticate()', () => {
@ -122,9 +119,6 @@ describe('components/accounts/actions', () => {
authentication.validateToken.returns(Promise.reject({})); authentication.validateToken.returns(Promise.reject({}));
return expect(authenticate(account)(dispatch, getState), 'to be rejected').then(() => { return expect(authenticate(account)(dispatch, getState), 'to be rejected').then(() => {
expect(logger.warn, 'to have a call satisfying', [
'Error validating token during auth', {}
]);
expect(dispatch, 'to have a call satisfying', [ expect(dispatch, 'to have a call satisfying', [
{payload: {isGuest: true}}, {payload: {isGuest: true}},
]); ]);
@ -139,13 +133,10 @@ describe('components/accounts/actions', () => {
authentication.validateToken.returns(Promise.reject(resp)); authentication.validateToken.returns(Promise.reject(resp));
return expect(authenticate(account)(dispatch, getState), 'to be rejected with', resp).then(() => { return expect(authenticate(account)(dispatch, getState), 'to be rejected with', resp)
expect(dispatch, 'to have no calls satisfying', [ .then(() => expect(dispatch, 'to have no calls satisfying', [
{payload: {isGuest: true}}, {payload: {isGuest: true}},
]); ]));
expect(logger.warn, 'was not called');
});
}); });
it('marks user as stranger, if there is no refreshToken', () => { it('marks user as stranger, if there is no refreshToken', () => {

View File

@ -1,14 +1,30 @@
// @flow
import request from 'services/request'; import request from 'services/request';
type UserResponse = {
elyProfileLink: string,
email: string,
hasMojangUsernameCollision: bool,
id: number,
isActive: bool,
isOtpEnabled: bool,
lang: string,
passwordChangedAt: number, // timestamp
registeredAt: number, // timestamp
shouldAcceptRules: bool,
username: string,
uuid: string,
};
export default { export default {
/** /**
* @param {object} options * @param {object} options
* @param {object} [options.token] - an optional token to overwrite headers * @param {object} [options.token] - an optional token to overwrite headers
* in middleware and disable token auto-refresh * in middleware and disable token auto-refresh
* *
* @return {Promise<User>} * @return {Promise<UserResponse>}
*/ */
current(options = {}) { current(options: { token?: ?string } = {}): Promise<UserResponse> {
return request.get('/api/accounts/current', {}, { return request.get('/api/accounts/current', {}, {
token: options.token token: options.token
}); });
@ -19,6 +35,11 @@ export default {
newPassword = '', newPassword = '',
newRePassword = '', newRePassword = '',
logoutAll = true logoutAll = true
}: {
password?: string,
newPassword?: string,
newRePassword?: string,
logoutAll?: bool,
}) { }) {
return request.post( return request.post(
'/api/accounts/change-password', '/api/accounts/change-password',
@ -33,6 +54,9 @@ export default {
changeUsername({ changeUsername({
username = '', username = '',
password = '' password = ''
}: {
username?: string,
password?: string,
}) { }) {
return request.post( return request.post(
'/api/accounts/change-username', '/api/accounts/change-username',
@ -40,14 +64,14 @@ export default {
); );
}, },
changeLang(lang) { changeLang(lang: string) {
return request.post( return request.post(
'/api/accounts/change-lang', '/api/accounts/change-lang',
{lang} {lang}
); );
}, },
requestEmailChange({password = ''}) { requestEmailChange({password = ''}: { password?: string }) {
return request.post( return request.post(
'/api/accounts/change-email/initialize', '/api/accounts/change-email/initialize',
{password} {password}
@ -57,6 +81,9 @@ export default {
setNewEmail({ setNewEmail({
email = '', email = '',
key = '' key = ''
}: {
email?: string,
key?: string,
}) { }) {
return request.post( return request.post(
'/api/accounts/change-email/submit-new-email', '/api/accounts/change-email/submit-new-email',
@ -64,7 +91,7 @@ export default {
); );
}, },
confirmNewEmail({key}) { confirmNewEmail({key}: { key: string }) {
return request.post( return request.post(
'/api/accounts/change-email/confirm-new-email', '/api/accounts/change-email/confirm-new-email',
{key} {key}

View File

@ -1,5 +1,6 @@
// @flow // @flow
import request from 'services/request'; import logger from 'services/logger';
import request, { InternalServerError } from 'services/request';
import accounts from 'services/api/accounts'; import accounts from 'services/api/accounts';
const authentication = { const authentication = {
@ -91,12 +92,31 @@ const authentication = {
.then(() => accounts.current({token})) .then(() => accounts.current({token}))
.then((user) => ({token, refreshToken, user})) .then((user) => ({token, refreshToken, user}))
.catch((resp) => { .catch((resp) => {
if (resp.message === 'Token expired') { if (resp instanceof InternalServerError) {
// delegate error recovering to the bsod middleware
return new Promise(() => {});
}
if (['Token expired', 'Incorrect token'].includes(resp.message)) {
return authentication.requestToken(refreshToken) return authentication.requestToken(refreshToken)
.then(({token}) => .then(({token}) =>
accounts.current({token}) accounts.current({token})
.then((user) => ({token, refreshToken, user})) .then((user) => ({token, refreshToken, user}))
); )
.catch((error) => {
logger.error('Failed refreshing token during token validation', {
error
});
return Promise.reject(error);
});
}
const errors = resp.errors || {};
if (errors.refresh_token !== 'error.refresh_token_not_exist') {
logger.error('Unexpected error during token validation', {
resp
});
} }
return Promise.reject(resp); return Promise.reject(resp);

View File

@ -125,6 +125,43 @@ describe('authentication api', () => {
); );
}); });
}); });
describe('when token is incorrect', () => {
const expiredResponse = {
name: 'Unauthorized',
message: 'Incorrect token',
code: 0,
status: 401,
type: 'yii\\web\\UnauthorizedHttpException'
};
const newToken = 'baz';
beforeEach(() => {
sinon.stub(authentication, 'requestToken');
accounts.current.onCall(0).returns(Promise.reject(expiredResponse));
authentication.requestToken.returns(Promise.resolve({token: newToken}));
});
afterEach(() => {
authentication.requestToken.restore();
});
it('resolves with new token and user object', () =>
expect(authentication.validateToken(validTokens),
'to be fulfilled with', {...validTokens, token: newToken, user}
)
);
it('rejects if token request failed', () => {
const error = 'Something wrong';
authentication.requestToken.returns(Promise.reject(error));
return expect(authentication.validateToken(validTokens),
'to be rejected with', error
);
});
});
}); });
describe('#logout', () => { describe('#logout', () => {