From afde68ae5bae6aac64ceaf4b21d9dff0f1568cf8 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 25 Jan 2019 20:29:32 +0300 Subject: [PATCH 1/4] Little refactoring of the accounts API for the frontend application --- src/components/user/actions.js | 12 ++- src/services/api/accounts.js | 145 +++++++++++++++-------------- src/services/api/authentication.js | 50 +++++----- 3 files changed, 110 insertions(+), 97 deletions(-) diff --git a/src/components/user/actions.js b/src/components/user/actions.js index 2342dd5..af26030 100644 --- a/src/components/user/actions.js +++ b/src/components/user/actions.js @@ -1,4 +1,8 @@ -import accounts from 'services/api/accounts'; +import { + getInfo as getInfoEndpoint, + changeLang as changeLangEndpoint, + acceptRules as acceptRulesEndpoint, +} from 'services/api/accounts'; import { setLocale } from 'components/i18n/actions'; export const UPDATE = 'USER_UPDATE'; @@ -36,7 +40,7 @@ export function changeLang(lang) { const {user: {isGuest, lang: oldLang}} = getState(); if (oldLang !== lang) { - !isGuest && accounts.changeLang(lang); + !isGuest && changeLangEndpoint(lang); dispatch({ type: CHANGE_LANG, @@ -59,7 +63,7 @@ export function setGuest() { export function fetchUserData() { return (dispatch) => - accounts.current() + getInfoEndpoint(0) .then((resp) => { dispatch(updateUser({ isGuest: false, @@ -72,7 +76,7 @@ export function fetchUserData() { export function acceptRules() { return (dispatch) => - accounts.acceptRules().then((resp) => { + acceptRulesEndpoint().then((resp) => { dispatch(updateUser({ shouldAcceptRules: false })); diff --git a/src/services/api/accounts.js b/src/services/api/accounts.js index 145e51a..c840fc3 100644 --- a/src/services/api/accounts.js +++ b/src/services/api/accounts.js @@ -1,7 +1,7 @@ // @flow import request from 'services/request'; -type UserResponse = { +export type UserResponse = { elyProfileLink: string, email: string, hasMojangUsernameCollision: bool, @@ -16,6 +16,79 @@ type UserResponse = { uuid: string, }; +export function getInfo(id: number, options: { token?: ?string } = {}): Promise { + return request.get(`/api/v1/accounts/${id}`, {}, { + token: options.token, + }); +} + +export function changePassword({ + password = '', + newPassword = '', + newRePassword = '', + logoutAll = true, +}: { + password?: string, + newPassword?: string, + newRePassword?: string, + logoutAll?: bool, +}) { + return request.post('/api/accounts/change-password', { + password, + newPassword, + newRePassword, + logoutAll, + }); +} + +export function acceptRules() { + return request.post('/api/accounts/accept-rules'); +} + +export function changeUsername({ + username = '', + password = '', +}: { + username?: string, + password?: string, +}) { + return request.post('/api/accounts/change-username', { + username, + password, + }); +} + +export function changeLang(lang: string) { + return request.post('/api/accounts/change-lang', { + lang, + }); +} + +export function requestEmailChange({ password = '' }: { password?: string }) { + return request.post('/api/accounts/change-email/initialize', { + password, + }); +} + +export function setNewEmail({ + email = '', + key = '', +}: { + email?: string, + key?: string, +}) { + return request.post('/api/accounts/change-email/submit-new-email', { + email, + key, + }); +} + +export function confirmNewEmail({ key }: { key: string }) { + return request.post('/api/accounts/change-email/confirm-new-email', { + key, + }); +} + export default { /** * @param {object} options @@ -25,76 +98,8 @@ export default { * @return {Promise} */ current(options: { token?: ?string } = {}): Promise { - return request.get('/api/accounts/current', {}, { - token: options.token - }); + return getInfo(0, options); }, - changePassword({ - password = '', - newPassword = '', - newRePassword = '', - logoutAll = true - }: { - password?: string, - newPassword?: string, - newRePassword?: string, - logoutAll?: bool, - }) { - return request.post( - '/api/accounts/change-password', - {password, newPassword, newRePassword, logoutAll} - ); - }, - acceptRules() { - return request.post('/api/accounts/accept-rules'); - }, - - changeUsername({ - username = '', - password = '' - }: { - username?: string, - password?: string, - }) { - return request.post( - '/api/accounts/change-username', - {username, password} - ); - }, - - changeLang(lang: string) { - return request.post( - '/api/accounts/change-lang', - {lang} - ); - }, - - requestEmailChange({password = ''}: { password?: string }) { - return request.post( - '/api/accounts/change-email/initialize', - {password} - ); - }, - - setNewEmail({ - email = '', - key = '' - }: { - email?: string, - key?: string, - }) { - return request.post( - '/api/accounts/change-email/submit-new-email', - {email, key} - ); - }, - - confirmNewEmail({key}: { key: string }) { - return request.post( - '/api/accounts/change-email/confirm-new-email', - {key} - ); - } }; diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index 86d13b9..f3bdde1 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -1,7 +1,8 @@ // @flow +import type { UserResponse } from 'services/api/accounts'; import logger from 'services/logger'; import request, { InternalServerError } from 'services/request'; -import accounts from 'services/api/accounts'; +import { getInfo as getInfoEndpoint } from 'services/api/accounts'; const authentication = { login({ @@ -70,35 +71,38 @@ const authentication = { /** * Resolves if token is valid * - * @param {object} options - * @param {string} options.token - * @param {string} options.refreshToken + * @param {string} token + * @param {string} refreshToken * * @return {Promise} - resolves with options.token or with a new token * if it was refreshed. As a side effect the response * will have a `user` field with current user data + * */ - validateToken({token, refreshToken}: { + async validateToken(token: string, refreshToken: ?string): Promise<{ token: string, - refreshToken: ?string - }) { - return new Promise((resolve) => { - if (typeof token !== 'string') { - throw new Error('token must be a string'); - } + refreshToken: ?string, + user: UserResponse, + }> { + if (typeof token !== 'string') { + throw new Error('token must be a string'); + } - resolve(); - }) - .then(() => accounts.current({token})) - .then((user) => ({token, refreshToken, user})) - .catch((resp) => - this.handleTokenError(resp, refreshToken) - // TODO: use recursion here - .then(({token}) => - accounts.current({token}) - .then((user) => ({token, refreshToken, user})) - ) - ); + // TODO: decode token to extract information about user id + + let user: UserResponse; + try { + user = await getInfoEndpoint(0, { token }); + } catch (resp) { + const { token } = await this.handleTokenError(resp, refreshToken); + user = await getInfoEndpoint(0, { token }); // TODO: replace with recursive call + } + + return { + token, + refreshToken, + user, + }; }, handleTokenError(resp: Error | { message: string }, refreshToken: ?string): Promise<{ From d64c2a728d806a746f51c12900cd7cf5edfaee70 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Sun, 27 Jan 2019 22:12:58 +0300 Subject: [PATCH 2/4] Completely migrate to the new backend API --- src/components/accounts/actions.js | 136 +++++---- src/components/accounts/actions.test.js | 47 ++- src/components/auth/actions.js | 21 +- .../profile/multiFactorAuth/MfaDisable.js | 9 +- .../profile/multiFactorAuth/MfaEnable.js | 31 +- src/components/ui/stepper/Stepper.js | 2 +- src/components/user/actions.js | 74 +++-- .../refreshTokenMiddleware.test.js | 8 +- src/components/user/reducer.js | 4 + src/pages/profile/ChangeEmailPage.js | 51 ++-- src/pages/profile/ChangePasswordPage.js | 21 +- src/pages/profile/ChangeUsernamePage.js | 31 +- src/pages/profile/MultiFactorAuthPage.js | 24 +- src/pages/profile/ProfilePage.js | 21 +- src/services/api/accounts.js | 61 ++-- src/services/api/authentication.js | 282 +++++++++--------- src/services/api/authentication.test.js | 71 ++--- src/services/api/mfa.js | 38 +-- 18 files changed, 500 insertions(+), 432 deletions(-) diff --git a/src/components/accounts/actions.js b/src/components/accounts/actions.js index 37d5cc7..e163fef 100644 --- a/src/components/accounts/actions.js +++ b/src/components/accounts/actions.js @@ -1,7 +1,8 @@ // @flow +import type { Account, State as AccountsState } from './reducer'; import { getJwtPayload } from 'functions'; import { sessionStorage } from 'services/localStorage'; -import authentication from 'services/api/authentication'; +import { validateToken, requestToken, logout } from 'services/api/authentication'; import { relogin as navigateToLogin } from 'components/auth/actions'; import { updateUser, setGuest } from 'components/user/actions'; import { setLocale } from 'components/i18n/actions'; @@ -9,7 +10,6 @@ import { setAccountSwitcher } from 'components/auth/actions'; import { getActiveAccount } from 'components/accounts/reducer'; import logger from 'services/logger'; -import type { Account, State as AccountsState } from './reducer'; import { add, remove, @@ -42,75 +42,95 @@ export function authenticate(account: Account | { token: string, refreshToken: ?string, }) { - const {token, refreshToken} = account; + const { token, refreshToken } = account; const email = account.email || null; - return (dispatch: Dispatch, getState: () => State): Promise => { - const accountId: number | null = typeof account.id === 'number' ? account.id : null; - const knownAccount: ?Account = accountId - ? getState().accounts.available.find((item) => item.id === accountId) - : null; + return async (dispatch: Dispatch, getState: () => State): Promise => { + let accountId: number; + if (typeof account.id === 'number') { + accountId = account.id; + } else { + accountId = findAccountIdFromToken(token); + } + const knownAccount = getState().accounts.available.find((item) => item.id === accountId); if (knownAccount) { // this account is already available // activate it before validation dispatch(activate(knownAccount)); } - return authentication.validateToken({token, refreshToken}) - .catch((resp = {}) => { - // all the logic to get the valid token was failed, - // looks like we have some problems with token - // lets redirect to login page - if (typeof email === 'string') { - // TODO: we should somehow try to find email by token - dispatch(relogin(email)); - } + try { + const { + token: newToken, + refreshToken: newRefreshToken, + user, + // $FlowFixMe have no idea why it's causes error about missing properties + } = await validateToken(accountId, token, refreshToken); + const { auth } = getState(); + const account: Account = { + id: user.id, + username: user.username, + email: user.email, + token: newToken, + refreshToken: newRefreshToken, + }; + dispatch(add(account)); + dispatch(activate(account)); + dispatch(updateUser({ + isGuest: false, + ...user, + })); - return Promise.reject(resp); - }) - .then(({token, refreshToken, user}) => ({ - user: { - isGuest: false, - ...user - }, - account: { - id: user.id, - username: user.username, - email: user.email, - token, - refreshToken - } - })) - .then(({user, account}) => { - const {auth} = getState(); + // TODO: probably should be moved from here, because it is a side effect + logger.setUser(user); - dispatch(add(account)); - dispatch(activate(account)); - dispatch(updateUser(user)); + if (!newRefreshToken) { + // mark user as stranger (user does not want us to remember his account) + sessionStorage.setItem(`stranger${account.id}`, 1); + } - // TODO: probably should be moved from here, because it is a side effect - logger.setUser(user); + if (auth && auth.oauth && auth.oauth.clientId) { + // if we authenticating during oauth, we disable account chooser + // because user probably has made his choise now + // this may happen, when user registers, logs in or uses account + // chooser panel during oauth + dispatch(setAccountSwitcher(false)); + } - if (!account.refreshToken) { - // mark user as stranger (user does not want us to remember his account) - sessionStorage.setItem(`stranger${account.id}`, 1); - } + await dispatch(setLocale(user.lang)); - if (auth && auth.oauth && auth.oauth.clientId) { - // if we authenticating during oauth, we disable account chooser - // because user probably has made his choise now - // this may happen, when user registers, logs in or uses account - // chooser panel during oauth - dispatch(setAccountSwitcher(false)); - } + return account; + } catch (resp) { + // all the logic to get the valid token was failed, + // looks like we have some problems with token + // lets redirect to login page + if (typeof email === 'string') { + // TODO: we should somehow try to find email by token + dispatch(relogin(email)); + } - return dispatch(setLocale(user.lang)) - .then(() => account); - }); + throw resp; + } }; } +function findAccountIdFromToken(token: string): number { + const encodedPayloads = token.split('.')[1]; + const { sub, jti }: { sub: string, jti: number } = JSON.parse(atob(encodedPayloads)); + // sub has the format "ely|{accountId}", so we must trim "ely|" part + if (sub) { + return parseInt(sub.substr(4), 10); + } + + // In older backend versions identity was stored in jti claim. Some users still have such tokens + if (jti) { + return jti; + } + + throw new Error('payloads is not contains any identity claim'); +} + /** * Checks the current user's token exp time. Supposed to be used before performing * any api request @@ -203,8 +223,8 @@ export function requestNewToken() { return Promise.resolve(); } - return authentication.requestToken(refreshToken) - .then(({ token }) => { + return requestToken(refreshToken) + .then((token) => { dispatch(updateToken(token)); }) .catch((resp) => { @@ -234,7 +254,7 @@ export function revoke(account: Account) { .finally(() => { // we need to logout user, even in case, when we can // not authenticate him with new account - authentication.logout(account) + logout(account.token) .catch(() => { // we don't care }); @@ -268,7 +288,7 @@ export function logoutAll() { const {accounts: {available}} = getState(); available.forEach((account) => - authentication.logout(account) + logout(account.token) .catch(() => { // we don't care }) @@ -303,7 +323,7 @@ export function logoutStrangers() { available.filter(isStranger) .forEach((account) => { dispatch(remove(account)); - authentication.logout(account); + logout(account.token); }); if (activeAccount && isStranger(activeAccount)) { diff --git a/src/components/accounts/actions.test.js b/src/components/accounts/actions.test.js index 68ec1b2..222f274 100644 --- a/src/components/accounts/actions.test.js +++ b/src/components/accounts/actions.test.js @@ -5,30 +5,33 @@ import { browserHistory } from 'services/history'; import { InternalServerError } from 'services/request'; import { sessionStorage } from 'services/localStorage'; -import authentication from 'services/api/authentication'; +import * as authentication from 'services/api/authentication'; import { authenticate, revoke, logoutAll, - logoutStrangers + logoutStrangers, } from 'components/accounts/actions'; import { add, ADD, activate, ACTIVATE, remove, - reset + reset, } from 'components/accounts/actions/pure-actions'; import { SET_LOCALE } from 'components/i18n/actions'; import { updateUser, setUser } from 'components/user/actions'; import { setLogin, setAccountSwitcher } from 'components/auth/actions'; +const token = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJlbHl8MSJ9.pRJ7vakt2eIscjqwG__KhSxKb3qwGsdBBeDbBffJs_I'; +const legacyToken = 'eyJhbGciOiJIUzI1NiJ9.eyJqdGkiOjF9.cRF-sQNrwWQ94xCb3vWioVdjxAZeefEE7GMGwh7708o'; + const account = { id: 1, username: 'username', email: 'email@test.com', - token: 'foo', - refreshToken: 'bar' + token, + refreshToken: 'bar', }; const user = { @@ -67,7 +70,7 @@ describe('components/accounts/actions', () => { authentication.validateToken.returns(Promise.resolve({ token: account.token, refreshToken: account.refreshToken, - user + user, })); }); @@ -81,7 +84,29 @@ describe('components/accounts/actions', () => { it('should request user state using token', () => authenticate(account)(dispatch, getState).then(() => expect(authentication.validateToken, 'to have a call satisfying', [ - {token: account.token, refreshToken: account.refreshToken} + account.id, + account.token, + account.refreshToken, + ]) + ) + ); + + it('should request user by extracting id from token', () => + authenticate({ token })(dispatch, getState).then(() => + expect(authentication.validateToken, 'to have a call satisfying', [ + 1, + token, + undefined, + ]) + ) + ); + + it('should request user by extracting id from legacy token', () => + authenticate({ token: legacyToken })(dispatch, getState).then(() => + expect(authentication.validateToken, 'to have a call satisfying', [ + 1, + legacyToken, + undefined, ]) ) ); @@ -142,7 +167,7 @@ describe('components/accounts/actions', () => { it('rejects when 5xx without logouting', () => { const resp = new InternalServerError(null, {status: 500}); - authentication.validateToken.returns(Promise.reject(resp)); + authentication.validateToken.rejects(resp); return expect(authenticate(account)(dispatch, getState), 'to be rejected with', resp) .then(() => expect(dispatch, 'to have no calls satisfying', [ @@ -152,10 +177,10 @@ describe('components/accounts/actions', () => { it('marks user as stranger, if there is no refreshToken', () => { const expectedKey = `stranger${account.id}`; - authentication.validateToken.returns(Promise.resolve({ + authentication.validateToken.resolves({ token: account.token, - user - })); + user, + }); sessionStorage.removeItem(expectedKey); diff --git a/src/components/auth/actions.js b/src/components/auth/actions.js index 06e4369..089c933 100644 --- a/src/components/auth/actions.js +++ b/src/components/auth/actions.js @@ -1,5 +1,6 @@ // @flow import type { OauthData } from 'services/api/oauth'; +import type { OAuthResponse } from 'services/api/authentication'; import { browserHistory } from 'services/history'; import logger from 'services/logger'; import localStorage from 'services/localStorage'; @@ -8,7 +9,11 @@ import history from 'services/history'; import { updateUser, acceptRules as userAcceptRules } from 'components/user/actions'; import { authenticate, logoutAll } from 'components/accounts/actions'; import { getActiveAccount } from 'components/accounts/reducer'; -import authentication from 'services/api/authentication'; +import { + login as loginEndpoint, + forgotPassword as forgotPasswordEndpoint, + recoverPassword as recoverPasswordEndpoint, +} from 'services/api/authentication'; import oauth from 'services/api/oauth'; import signup from 'services/api/signup'; import dispatchBsod from 'components/ui/bsod/dispatchBsod'; @@ -76,9 +81,7 @@ export function login({ rememberMe?: bool }) { return wrapInLoader((dispatch) => - authentication.login( - {login, password, totp, rememberMe} - ) + loginEndpoint({login, password, totp, rememberMe}) .then(authHandler(dispatch)) .catch((resp) => { if (resp.errors) { @@ -120,9 +123,9 @@ export function forgotPassword({ captcha: string }) { return wrapInLoader((dispatch, getState) => - authentication.forgotPassword({login, captcha}) + forgotPasswordEndpoint(login, captcha) .then(({data = {}}) => dispatch(updateUser({ - maskedEmail: data.emailMask || getState().user.email + maskedEmail: data.emailMask || getState().user.email, }))) .catch(validationErrorsHandler(dispatch)) ); @@ -138,7 +141,7 @@ export function recoverPassword({ newRePassword: string }) { return wrapInLoader((dispatch) => - authentication.recoverPassword({key, newPassword, newRePassword}) + recoverPasswordEndpoint(key, newPassword, newRePassword) .then(authHandler(dispatch)) .catch(validationErrorsHandler(dispatch, '/forgot-password')) ); @@ -544,9 +547,9 @@ function needActivation() { } function authHandler(dispatch) { - return (resp) => dispatch(authenticate({ + return (resp: OAuthResponse) => dispatch(authenticate({ token: resp.access_token, - refreshToken: resp.refresh_token + refreshToken: resp.refresh_token, })).then((resp) => { dispatch(setLogin(null)); diff --git a/src/components/profile/multiFactorAuth/MfaDisable.js b/src/components/profile/multiFactorAuth/MfaDisable.js index e3a731d..c48eb20 100644 --- a/src/components/profile/multiFactorAuth/MfaDisable.js +++ b/src/components/profile/multiFactorAuth/MfaDisable.js @@ -1,8 +1,9 @@ // @flow import React, { Component } from 'react'; +import PropTypes from 'prop-types'; import logger from 'services/logger'; -import mfa from 'services/api/mfa'; +import { disable as disableMFA } from 'services/api/mfa'; import MfaDisableForm from './disableForm/MfaDisableForm'; import MfaStatus from './status/MfaStatus'; @@ -15,6 +16,10 @@ export default class MfaDisable extends Component<{ }, { showForm?: bool }> { + static contextTypes = { + userId: PropTypes.number.isRequired, + }; + state = {}; render() { @@ -35,7 +40,7 @@ export default class MfaDisable extends Component<{ () => { const data = form.serialize(); - return mfa.disable(data); + return disableMFA(this.context.userId, data); } ) .then(() => this.props.onComplete()) diff --git a/src/components/profile/multiFactorAuth/MfaEnable.js b/src/components/profile/multiFactorAuth/MfaEnable.js index 41cfa61..6932acf 100644 --- a/src/components/profile/multiFactorAuth/MfaEnable.js +++ b/src/components/profile/multiFactorAuth/MfaEnable.js @@ -1,5 +1,6 @@ // @flow import React, { Component } from 'react'; +import PropTypes from 'prop-types'; import { Button, FormModel } from 'components/ui/form'; import styles from 'components/profile/profileForm.scss'; @@ -7,7 +8,7 @@ import Stepper from 'components/ui/stepper'; import { SlideMotion } from 'components/ui/motion'; import { ScrollIntoView } from 'components/ui/scroll'; import logger from 'services/logger'; -import mfa from 'services/api/mfa'; +import { getSecret, enable as enableMFA } from 'services/api/mfa'; import Instructions from './instructions'; import KeyForm from './keyForm'; @@ -24,25 +25,31 @@ type Props = { confirmationForm: FormModel, onSubmit: (form: FormModel, sendData: () => Promise<*>) => Promise<*>, onComplete: Function, - step: MfaStep + step: MfaStep, }; -export default class MfaEnable extends Component { +interface State { + isLoading: bool; + activeStep: MfaStep; + secret: string; + qrCodeSrc: string; +} + +export default class MfaEnable extends Component { static defaultProps = { confirmationForm: new FormModel(), - step: 0 + step: 0, + }; + + static contextTypes = { + userId: PropTypes.number.isRequired, }; state = { isLoading: false, activeStep: this.props.step, qrCodeSrc: '', - secret: '' + secret: '', }; confirmationFormEl: ?Form; @@ -124,7 +131,7 @@ export default class MfaEnable extends Component { + getSecret(this.context.userId).then((resp) => { this.setState({ isLoading: false, secret: resp.secret, @@ -154,7 +161,7 @@ export default class MfaEnable extends Component { const data = form.serialize(); - return mfa.enable(data); + return enableMFA(this.context.userId, data.totp, data.password); } ) .then(() => this.props.onComplete()) diff --git a/src/components/ui/stepper/Stepper.js b/src/components/ui/stepper/Stepper.js index d4e4b0d..f0c62e7 100644 --- a/src/components/ui/stepper/Stepper.js +++ b/src/components/ui/stepper/Stepper.js @@ -16,7 +16,7 @@ export default function Stepper({ }: { totalSteps: number, activeStep: number, - color: Color + color?: Color, }) { return (
diff --git a/src/components/user/actions.js b/src/components/user/actions.js index af26030..2513bc3 100644 --- a/src/components/user/actions.js +++ b/src/components/user/actions.js @@ -1,3 +1,5 @@ +// @flow +import type { User, State } from './reducer'; import { getInfo as getInfoEndpoint, changeLang as changeLangEndpoint, @@ -5,6 +7,8 @@ import { } from 'services/api/accounts'; import { setLocale } from 'components/i18n/actions'; +type Dispatch = (action: Object) => Promise<*>; + export const UPDATE = 'USER_UPDATE'; /** * Merge data into user's state @@ -12,10 +16,10 @@ export const UPDATE = 'USER_UPDATE'; * @param {object} payload * @return {object} - action definition */ -export function updateUser(payload) { +export function updateUser(payload: $Shape) { // Temp workaround return { type: UPDATE, - payload + payload, }; } @@ -26,61 +30,69 @@ export const SET = 'USER_SET'; * @param {User} payload * @return {object} - action definition */ -export function setUser(payload) { +export function setUser(payload: $Shape) { return { type: SET, - payload + payload, }; } export const CHANGE_LANG = 'USER_CHANGE_LANG'; -export function changeLang(lang) { - return (dispatch, getState) => dispatch(setLocale(lang)) +export function changeLang(lang: string) { + return (dispatch: Dispatch, getState: () => State) => dispatch(setLocale(lang)) .then((lang) => { - const {user: {isGuest, lang: oldLang}} = getState(); - - if (oldLang !== lang) { - !isGuest && changeLangEndpoint(lang); - - dispatch({ - type: CHANGE_LANG, - payload: { - lang - } - }); + const { id, isGuest, lang: oldLang } = getState().user; + if (oldLang === lang) { + return; } + + !isGuest && changeLangEndpoint(((id: any): number), lang); // hack to tell Flow that it's defined + + dispatch({ + type: CHANGE_LANG, + payload: { + lang, + }, + }); }); } export function setGuest() { - return (dispatch, getState) => { + return (dispatch: Dispatch, getState: () => Object) => { dispatch(setUser({ lang: getState().user.lang, - isGuest: true + isGuest: true, })); }; } export function fetchUserData() { - return (dispatch) => - getInfoEndpoint(0) - .then((resp) => { - dispatch(updateUser({ - isGuest: false, - ...resp - })); + return async (dispatch: Dispatch, getState: () => State) => { + // $FlowFixMe + const resp = await getInfoEndpoint(getState().user.id); + dispatch(updateUser({ + isGuest: false, + ...resp, + })); + dispatch(changeLang(resp.lang)); - return dispatch(changeLang(resp.lang)); - }); + return resp; + }; } export function acceptRules() { - return (dispatch) => - acceptRulesEndpoint().then((resp) => { + return (dispatch: Dispatch, getState: () => State) => { + const { id } = getState().user; + if (!id) { + throw new Error('user id is should be set at the moment when this action is called'); + } + + return acceptRulesEndpoint(id).then((resp) => { dispatch(updateUser({ - shouldAcceptRules: false + shouldAcceptRules: false, })); return resp; }); + }; } diff --git a/src/components/user/middlewares/refreshTokenMiddleware.test.js b/src/components/user/middlewares/refreshTokenMiddleware.test.js index 043f8d6..bf6f54d 100644 --- a/src/components/user/middlewares/refreshTokenMiddleware.test.js +++ b/src/components/user/middlewares/refreshTokenMiddleware.test.js @@ -3,7 +3,7 @@ import sinon from 'sinon'; import refreshTokenMiddleware from 'components/user/middlewares/refreshTokenMiddleware'; import { browserHistory } from 'services/history'; -import authentication from 'services/api/authentication'; +import * as authentication from 'services/api/authentication'; import { InternalServerError } from 'services/request'; import { updateToken } from 'components/accounts/actions'; @@ -86,7 +86,7 @@ describe('refreshTokenMiddleware', () => { } }; - authentication.requestToken.returns(Promise.resolve({token: validToken})); + authentication.requestToken.returns(Promise.resolve(validToken)); return middleware.before(data).then((resp) => { expect(resp, 'to satisfy', data); @@ -126,7 +126,7 @@ describe('refreshTokenMiddleware', () => { } }; - authentication.requestToken.returns(Promise.resolve({token: validToken})); + authentication.requestToken.returns(Promise.resolve(validToken)); return middleware.before(data).then(() => expect(dispatch, 'to have a call satisfying', [ @@ -251,7 +251,7 @@ describe('refreshTokenMiddleware', () => { restart = sinon.stub().named('restart'); - authentication.requestToken.returns(Promise.resolve({token: validToken})); + authentication.requestToken.returns(Promise.resolve(validToken)); }); function assertNewTokenRequest() { diff --git a/src/components/user/reducer.js b/src/components/user/reducer.js index 361061a..0a0f0b7 100644 --- a/src/components/user/reducer.js +++ b/src/components/user/reducer.js @@ -18,6 +18,10 @@ export type User = {| shouldAcceptRules?: bool, |}; +export type State = { + user: User, // TODO: replace with centralized global state +}; + const defaults: User = { id: null, uuid: null, diff --git a/src/pages/profile/ChangeEmailPage.js b/src/pages/profile/ChangeEmailPage.js index 1cbc992..67bc927 100644 --- a/src/pages/profile/ChangeEmailPage.js +++ b/src/pages/profile/ChangeEmailPage.js @@ -1,34 +1,36 @@ +// @flow +import type { RouterHistory, Match } from 'react-router'; import PropTypes from 'prop-types'; import React, { Component } from 'react'; import ChangeEmail from 'components/profile/changeEmail/ChangeEmail'; -import accounts from 'services/api/accounts'; +import { requestEmailChange, setNewEmail, confirmNewEmail } from 'services/api/accounts'; -class ChangeEmailPage extends Component { +interface Props { + lang: string; + email: string; + history: RouterHistory; + match: { + ...Match; + params: { + step: 'step1' | 'step2' | 'step3'; + code: string; + }; + }; +} + +class ChangeEmailPage extends Component { static displayName = 'ChangeEmailPage'; - static propTypes = { - email: PropTypes.string.isRequired, - lang: PropTypes.string.isRequired, - history: PropTypes.shape({ - push: PropTypes.func - }).isRequired, - match: PropTypes.shape({ - params: PropTypes.shape({ - step: PropTypes.oneOf(['step1', 'step2', 'step3']), - code: PropTypes.string - }) - }) - }; - static contextTypes = { + userId: PropTypes.number.isRequired, onSubmit: PropTypes.func.isRequired, - goToProfile: PropTypes.func.isRequired + goToProfile: PropTypes.func.isRequired, }; componentWillMount() { - const step = this.props.match.params.step; + const { step } = this.props.match.params; if (step && !/^step[123]$/.test(step)) { // wrong param value @@ -37,14 +39,14 @@ class ChangeEmailPage extends Component { } render() { - const {step = 'step1', code} = this.props.match.params; + const { step = 'step1', code } = this.props.match.params; return ( @@ -55,19 +57,20 @@ class ChangeEmailPage extends Component { this.props.history.push(`/profile/change-email/step${++step}`); }; - onSubmit = (step, form) => { + onSubmit = (step: number, form) => { return this.context.onSubmit({ form, sendData: () => { + const { userId } = this.context; const data = form.serialize(); switch (step) { case 0: - return accounts.requestEmailChange(data).catch(handleErrors()); + return requestEmailChange(userId, data.password).catch(handleErrors()); case 1: - return accounts.setNewEmail(data).catch(handleErrors('/profile/change-email')); + return setNewEmail(userId, data.email, data.key).catch(handleErrors('/profile/change-email')); case 2: - return accounts.confirmNewEmail(data).catch(handleErrors('/profile/change-email')); + return confirmNewEmail(userId, data.key).catch(handleErrors('/profile/change-email')); default: throw new Error(`Unsupported step ${step}`); } diff --git a/src/pages/profile/ChangePasswordPage.js b/src/pages/profile/ChangePasswordPage.js index 8c02c02..d11e45f 100644 --- a/src/pages/profile/ChangePasswordPage.js +++ b/src/pages/profile/ChangePasswordPage.js @@ -1,20 +1,23 @@ +// @flow +import type { User } from 'components/user'; import PropTypes from 'prop-types'; import React, { Component } from 'react'; -import accounts from 'services/api/accounts'; +import { changePassword } from 'services/api/accounts'; import { FormModel } from 'components/ui/form'; import ChangePassword from 'components/profile/changePassword/ChangePassword'; -class ChangePasswordPage extends Component { +interface Props { + updateUser: (fields: $Shape) => void; +} + +class ChangePasswordPage extends Component { static displayName = 'ChangePasswordPage'; - static propTypes = { - updateUser: PropTypes.func.isRequired - }; - static contextTypes = { + userId: PropTypes.number.isRequired, onSubmit: PropTypes.func.isRequired, - goToProfile: PropTypes.func.isRequired + goToProfile: PropTypes.func.isRequired, }; form = new FormModel(); @@ -29,7 +32,7 @@ class ChangePasswordPage extends Component { const {form} = this; return this.context.onSubmit({ form, - sendData: () => accounts.changePassword(form.serialize()) + sendData: () => changePassword(this.context.userId, form.serialize()), }).then(() => { this.props.updateUser({ passwordChangedAt: Date.now() / 1000 @@ -43,5 +46,5 @@ import { connect } from 'react-redux'; import { updateUser } from 'components/user/actions'; export default connect(null, { - updateUser + updateUser, })(ChangePasswordPage); diff --git a/src/pages/profile/ChangeUsernamePage.js b/src/pages/profile/ChangeUsernamePage.js index 6d2aeee..cc77b81 100644 --- a/src/pages/profile/ChangeUsernamePage.js +++ b/src/pages/profile/ChangeUsernamePage.js @@ -1,25 +1,29 @@ +// @flow import PropTypes from 'prop-types'; import React, { Component } from 'react'; -import accounts from 'services/api/accounts'; +import { changeUsername } from 'services/api/accounts'; import { FormModel } from 'components/ui/form'; import ChangeUsername from 'components/profile/changeUsername/ChangeUsername'; -class ChangeUsernamePage extends Component { +interface Props { + username: string; + updateUsername: (username: string) => void; +} + +class ChangeUsernamePage extends Component { static displayName = 'ChangeUsernamePage'; - static propTypes = { - username: PropTypes.string.isRequired, - updateUsername: PropTypes.func.isRequired - }; - static contextTypes = { + userId: PropTypes.number.isRequired, onSubmit: PropTypes.func.isRequired, - goToProfile: PropTypes.func.isRequired + goToProfile: PropTypes.func.isRequired, }; form = new FormModel(); + actualUsername: string; + componentWillMount() { this.actualUsername = this.props.username; } @@ -43,7 +47,7 @@ class ChangeUsernamePage extends Component { }; onSubmit = () => { - const {form} = this; + const { form } = this; if (this.actualUsername === this.props.username) { this.context.goToProfile(); return Promise.resolve(); @@ -51,7 +55,10 @@ class ChangeUsernamePage extends Component { return this.context.onSubmit({ form, - sendData: () => accounts.changeUsername(form.serialize()) + sendData: () => { + const { username, password } = form.serialize(); + return changeUsername(this.context.userId, username, password); + }, }).then(() => { this.actualUsername = form.value('username'); @@ -64,7 +71,7 @@ import { connect } from 'react-redux'; import { updateUser } from 'components/user/actions'; export default connect((state) => ({ - username: state.user.username + username: state.user.username, }), { - updateUsername: (username) => updateUser({username}) + updateUsername: (username) => updateUser({username}), })(ChangeUsernamePage); diff --git a/src/pages/profile/MultiFactorAuthPage.js b/src/pages/profile/MultiFactorAuthPage.js index b107ae7..50c4fc0 100644 --- a/src/pages/profile/MultiFactorAuthPage.js +++ b/src/pages/profile/MultiFactorAuthPage.js @@ -1,4 +1,5 @@ // @flow +import type { RouterHistory, Match } from 'react-router'; import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; @@ -8,17 +9,18 @@ import type { MfaStep } from 'components/profile/multiFactorAuth'; import type { FormModel } from 'components/ui/form'; import type { User } from 'components/user'; -class MultiFactorAuthPage extends Component<{ - user: User, - history: { - push: (string) => void - }, +interface Props { + user: User; + history: RouterHistory; match: { + ...Match; params: { - step?: '1' | '2' | '3' - } - } -}> { + step?: '1' | '2' | '3'; + }; + }; +} + +class MultiFactorAuthPage extends Component { static contextTypes = { onSubmit: PropTypes.func.isRequired, goToProfile: PropTypes.func.isRequired @@ -26,7 +28,7 @@ class MultiFactorAuthPage extends Component<{ componentWillMount() { const step = this.props.match.params.step; - const {user} = this.props; + const { user } = this.props; if (step) { if (!/^[1-3]$/.test(step)) { @@ -42,7 +44,7 @@ class MultiFactorAuthPage extends Component<{ } render() { - const {user} = this.props; + const { user } = this.props; return ( Promise<*>}) => void, - fetchUserData: () => Promise<*> -}> { +interface Props { + userId: number; + onSubmit: ({form: FormModel, sendData: () => Promise<*>}) => void; + fetchUserData: () => Promise<*>; +} + +class ProfilePage extends Component { static childContextTypes = { + userId: PropTypes.number, onSubmit: PropTypes.func, - goToProfile: PropTypes.func + goToProfile: PropTypes.func, }; getChildContext() { return { + userId: this.props.userId, onSubmit: this.props.onSubmit, - goToProfile: () => this.props.fetchUserData().then(this.goToProfile) + goToProfile: () => this.props.fetchUserData().then(this.goToProfile), }; } @@ -60,7 +65,9 @@ class ProfilePage extends Component<{ goToProfile = () => browserHistory.push('/'); } -export default connect(null, { +export default connect((state) => ({ + userId: state.user.id, +}), { fetchUserData, onSubmit: ({form, sendData}: { form: FormModel, diff --git a/src/services/api/accounts.js b/src/services/api/accounts.js index c840fc3..78ce184 100644 --- a/src/services/api/accounts.js +++ b/src/services/api/accounts.js @@ -16,13 +16,13 @@ export type UserResponse = { uuid: string, }; -export function getInfo(id: number, options: { token?: ?string } = {}): Promise { +export function getInfo(id: number, token?: string): Promise { return request.get(`/api/v1/accounts/${id}`, {}, { - token: options.token, + token, }); } -export function changePassword({ +export function changePassword(id: number, { password = '', newPassword = '', newRePassword = '', @@ -32,8 +32,8 @@ export function changePassword({ newPassword?: string, newRePassword?: string, logoutAll?: bool, -}) { - return request.post('/api/accounts/change-password', { +}): Promise<{ success: bool }> { + return request.post(`/api/v1/accounts/${id}/password`, { password, newPassword, newRePassword, @@ -41,65 +41,38 @@ export function changePassword({ }); } -export function acceptRules() { - return request.post('/api/accounts/accept-rules'); +export function acceptRules(id: number) { + return request.post(`/api/v1/accounts/${id}/rules`); } -export function changeUsername({ - username = '', - password = '', -}: { - username?: string, - password?: string, -}) { - return request.post('/api/accounts/change-username', { +export function changeUsername(id: number, username: ?string, password: ?string) { + return request.post(`/api/v1/accounts/${id}/username`, { username, password, }); } -export function changeLang(lang: string) { - return request.post('/api/accounts/change-lang', { +export function changeLang(id: number, lang: string) { + return request.post(`/api/v1/accounts/${id}/language`, { lang, }); } -export function requestEmailChange({ password = '' }: { password?: string }) { - return request.post('/api/accounts/change-email/initialize', { +export function requestEmailChange(id: number, password: string) { + return request.post(`/api/v1/accounts/${id}/email-verification`, { password, }); } -export function setNewEmail({ - email = '', - key = '', -}: { - email?: string, - key?: string, -}) { - return request.post('/api/accounts/change-email/submit-new-email', { +export function setNewEmail(id: number, email: string, key: string) { + return request.post(`/api/v1/accounts/${id}/new-email-verification`, { email, key, }); } -export function confirmNewEmail({ key }: { key: string }) { - return request.post('/api/accounts/change-email/confirm-new-email', { +export function confirmNewEmail(id: number, key: string) { + return request.post(`/api/v1/accounts/${id}/email`, { key, }); } - -export default { - /** - * @param {object} options - * @param {object} [options.token] - an optional token to overwrite headers - * in middleware and disable token auto-refresh - * - * @return {Promise} - */ - current(options: { token?: ?string } = {}): Promise { - return getInfo(0, options); - }, - - -}; diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index f3bdde1..203cbab 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -4,160 +4,152 @@ import logger from 'services/logger'; import request, { InternalServerError } from 'services/request'; import { getInfo as getInfoEndpoint } from 'services/api/accounts'; -const authentication = { - login({ +export interface OAuthResponse { + access_token: string; + refresh_token?: string; + expires_in: number; // count seconds before expire + success: true; +} + +export function login({ + login, + password, + totp, + rememberMe = false, +}: { + login: string, + password?: string, + totp?: string, + rememberMe: bool, +}): Promise { + return request.post('/api/authentication/login', { login, password, totp, - rememberMe = false - }: { - login: string, - password?: string, - totp?: string, - rememberMe: bool - }) { - return request.post( - '/api/authentication/login', - {login, password, totp, rememberMe}, - {token: null} - ); - }, + rememberMe, + }, { token: null }); +} - /** - * @param {object} options - * @param {string} [options.token] - an optional token to overwrite headers - * in middleware and disable token auto-refresh - * - * @return {Promise} - */ - logout(options: ?{ - token: string - }) { - return request.post('/api/authentication/logout', {}, { - token: options && options.token - }); - }, +/** + * @param {string} token - an optional token to overwrite headers + * in middleware and disable token auto-refresh + * + * @return {Promise} + */ +export function logout(token?: string): Promise<{success: bool}> { + return request.post('/api/authentication/logout', {}, { + token, + }); +} - forgotPassword({ +export function forgotPassword(login: string, captcha: string): Promise<{ + success: bool, + data: { + canRepeatIn: number, + emailMask: ?string, + repeatFrequency: number, + }, + errors: { + [key: string]: string, + }, +}> { + return request.post('/api/authentication/forgot-password', { login, - captcha - }: { - login: string, - captcha: string - }) { - return request.post( - '/api/authentication/forgot-password', - {login, captcha}, - {token: null} - ); - }, + captcha, + }, { token: null }); +} - recoverPassword({ +export function recoverPassword(key: string, newPassword: string, newRePassword: string): Promise { + return request.post('/api/authentication/recover-password', { key, newPassword, - newRePassword - }: { - key: string, - newPassword: string, - newRePassword: string - }) { - return request.post( - '/api/authentication/recover-password', - {key, newPassword, newRePassword}, - {token: null} - ); - }, + newRePassword, + }, { token: null }); +} - /** - * Resolves if token is valid - * - * @param {string} token - * @param {string} refreshToken - * - * @return {Promise} - resolves with options.token or with a new token - * if it was refreshed. As a side effect the response - * will have a `user` field with current user data - * - */ - async validateToken(token: string, refreshToken: ?string): Promise<{ - token: string, - refreshToken: ?string, - user: UserResponse, - }> { - if (typeof token !== 'string') { - throw new Error('token must be a string'); - } - - // TODO: decode token to extract information about user id - - let user: UserResponse; - try { - user = await getInfoEndpoint(0, { token }); - } catch (resp) { - const { token } = await this.handleTokenError(resp, refreshToken); - user = await getInfoEndpoint(0, { token }); // TODO: replace with recursive call - } - - return { - token, - refreshToken, - user, - }; - }, - - handleTokenError(resp: Error | { message: string }, refreshToken: ?string): Promise<{ - token: string, - }> { - if (resp instanceof InternalServerError) { - // delegate error recovering to the bsod middleware - return new Promise(() => {}); - } - - if (refreshToken) { - if ([ - 'Token expired', - 'Incorrect token', - 'You are requesting with an invalid credential.' - ].includes(resp.message)) { - return authentication.requestToken(refreshToken); - } - - logger.error('Unexpected error during token validation', { - resp - }); - } - - return Promise.reject(resp); - }, - - /** - * Request new access token using a refreshToken - * - * @param {string} refreshToken - * - * @return {Promise} - resolves to {token} - */ - requestToken(refreshToken: string): Promise<{token: string}> { - return request.post( - '/api/authentication/refresh-token', - {refresh_token: refreshToken}, // eslint-disable-line - {token: null} - ) - .then((resp: {access_token: string}) => ({ - token: resp.access_token - })) - .catch((resp) => { - const errors = resp.errors || {}; - - if (errors.refresh_token !== 'error.refresh_token_not_exist') { - logger.error('Failed refreshing token: unknown error', { - resp - }); - } - - return Promise.reject(resp); - }); +/** + * Resolves if token is valid + * + * @param {number} id + * @param {string} token + * @param {string} refreshToken + * + * @return {Promise} - resolves with options.token or with a new token + * if it was refreshed. As a side effect the response + * will have a `user` field with current user data + * + */ +export async function validateToken(id: number, token: string, refreshToken: ?string): Promise<{ + token: string, + refreshToken: ?string, + user: UserResponse, +}> { + if (typeof token !== 'string') { + throw new Error('token must be a string'); } -}; -export default authentication; + let user: UserResponse; + try { + user = await getInfoEndpoint(id, token); + } catch (resp) { + const token = await handleTokenError(resp, refreshToken); + user = await getInfoEndpoint(id, token); // TODO: replace with recursive call + } + + return { + token, + refreshToken, + user, + }; +} + +const recoverableErrors = [ + 'Token expired', + 'Incorrect token', + 'You are requesting with an invalid credential.', +]; + +function handleTokenError(resp: Error | { message: string }, refreshToken: ?string): Promise { + if (resp instanceof InternalServerError) { + // delegate error recovering to the bsod middleware + return new Promise(() => {}); + } + + if (refreshToken) { + if (recoverableErrors.includes(resp.message)) { + return requestToken(refreshToken); + } + + logger.error('Unexpected error during token validation', { resp }); + } + + return Promise.reject(resp); +} + +/** + * Request new access token using a refreshToken + * + * @param {string} refreshToken + * + * @return {Promise} - resolves to token + */ +export async function requestToken(refreshToken: string): Promise { + try { + const response: OAuthResponse = await request.post('/api/authentication/refresh-token', { + refresh_token: refreshToken, // eslint-disable-line camelcase + }, { + token: null, + }); + + return response.access_token; + } catch (resp) { + const errors = resp.errors || {}; + if (errors.refresh_token !== 'error.refresh_token_not_exist') { + logger.error('Failed refreshing token: unknown error', { + resp, + }); + } + + throw resp; + } +} diff --git a/src/services/api/authentication.test.js b/src/services/api/authentication.test.js index e0cfbd7..d522fd7 100644 --- a/src/services/api/authentication.test.js +++ b/src/services/api/authentication.test.js @@ -2,8 +2,8 @@ import expect from 'unexpected'; import sinon from 'sinon'; import request from 'services/request'; -import authentication from 'services/api/authentication'; -import accounts from 'services/api/accounts'; +import * as authentication from 'services/api/authentication'; +import * as accounts from 'services/api/accounts'; describe('authentication api', () => { describe('#login', () => { @@ -41,50 +41,53 @@ describe('authentication api', () => { }); describe('#validateToken()', () => { - const validTokens = {token: 'foo', refreshToken: 'bar'}; - const user = {id: 1}; + const validToken = 'foo'; + const validRefreshToken = 'bar'; + const user = { id: 1 }; + const validateTokenArgs = [user.id, validToken, validRefreshToken]; beforeEach(() => { - sinon.stub(accounts, 'current'); - - accounts.current.returns(Promise.resolve(user)); + sinon.stub(accounts, 'getInfo'); + accounts.getInfo.returns(Promise.resolve(user)); }); afterEach(() => { - accounts.current.restore(); + accounts.getInfo.restore(); }); - it('should request accounts.current', () => - expect(authentication.validateToken(validTokens), 'to be fulfilled') + it('should request accounts.getInfo', () => + expect(authentication.validateToken(...validateTokenArgs), 'to be fulfilled') .then(() => { - expect(accounts.current, 'to have a call satisfying', [ - {token: 'foo'} + expect(accounts.getInfo, 'to have a call satisfying', [ + user.id, + validToken, ]); }) ); it('should resolve with both tokens and user object', () => - expect(authentication.validateToken(validTokens), 'to be fulfilled with', { - ...validTokens, - user + expect(authentication.validateToken(...validateTokenArgs), 'to be fulfilled with', { + token: validToken, + refreshToken: validRefreshToken, + user, }) ); it('rejects if token has a bad type', () => - expect(authentication.validateToken({token: {}}), + expect(authentication.validateToken(user.id, {}), 'to be rejected with', 'token must be a string' ) ); it('should allow empty refreshToken', () => - expect(authentication.validateToken({token: 'foo', refreshToken: null}), 'to be fulfilled') + expect(authentication.validateToken(user.id, 'foo', null), 'to be fulfilled') ); - it('rejects if accounts.current request is unexpectedly failed', () => { + it('rejects if accounts.getInfo request is unexpectedly failed', () => { const error = 'Something wrong'; - accounts.current.returns(Promise.reject(error)); + accounts.getInfo.returns(Promise.reject(error)); - return expect(authentication.validateToken(validTokens), + return expect(authentication.validateToken(...validateTokenArgs), 'to be rejected with', error ); }); @@ -95,15 +98,15 @@ describe('authentication api', () => { message: 'Token expired', code: 0, status: 401, - type: 'yii\\web\\UnauthorizedHttpException' + 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})); + accounts.getInfo.onCall(0).returns(Promise.reject(expiredResponse)); + authentication.requestToken.returns(Promise.resolve(newToken)); }); afterEach(() => { @@ -111,8 +114,8 @@ describe('authentication api', () => { }); it('resolves with new token and user object', () => - expect(authentication.validateToken(validTokens), - 'to be fulfilled with', {...validTokens, token: newToken, user} + expect(authentication.validateToken(...validateTokenArgs), + 'to be fulfilled with', {token: newToken, refreshToken: validRefreshToken, user} ) ); @@ -120,7 +123,7 @@ describe('authentication api', () => { const error = 'Something wrong'; authentication.requestToken.returns(Promise.reject(error)); - return expect(authentication.validateToken(validTokens), + return expect(authentication.validateToken(...validateTokenArgs), 'to be rejected with', error ); }); @@ -132,15 +135,15 @@ describe('authentication api', () => { message: 'Incorrect token', code: 0, status: 401, - type: 'yii\\web\\UnauthorizedHttpException' + 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})); + accounts.getInfo.onCall(0).returns(Promise.reject(expiredResponse)); + authentication.requestToken.returns(Promise.resolve(newToken)); }); afterEach(() => { @@ -148,8 +151,8 @@ describe('authentication api', () => { }); it('resolves with new token and user object', () => - expect(authentication.validateToken(validTokens), - 'to be fulfilled with', {...validTokens, token: newToken, user} + expect(authentication.validateToken(...validateTokenArgs), + 'to be fulfilled with', {token: newToken, refreshToken: validRefreshToken, user} ) ); @@ -157,7 +160,7 @@ describe('authentication api', () => { const error = 'Something wrong'; authentication.requestToken.returns(Promise.reject(error)); - return expect(authentication.validateToken(validTokens), + return expect(authentication.validateToken(...validateTokenArgs), 'to be rejected with', error ); }); @@ -190,7 +193,7 @@ describe('authentication api', () => { it('overrides token if provided', () => { const token = 'foo'; - authentication.logout({token}); + authentication.logout(token); expect(request.post, 'to have a call satisfying', [ '/api/authentication/logout', {}, {token} @@ -241,7 +244,7 @@ describe('authentication api', () => { })); return expect(authentication.requestToken(refreshToken), - 'to be fulfilled with', {token} + 'to be fulfilled with', token, ); }); }); diff --git a/src/services/api/mfa.js b/src/services/api/mfa.js index b891963..0dbea0f 100644 --- a/src/services/api/mfa.js +++ b/src/services/api/mfa.js @@ -1,23 +1,25 @@ // @flow -import request from 'services/request'; import type { Resp } from 'services/request'; +import request from 'services/request'; -export default { - getSecret(): Promise> { - return request.get('/api/two-factor-auth'); - }, +export function getSecret(id: number): Promise> { + return request.get(`/api/v1/accounts/${id}/two-factor-auth`); +} - enable(data: {totp: string, password?: string}): Promise> { - return request.post('/api/two-factor-auth', { - totp: data.totp, - password: data.password || '' - }); - }, +export function enable(id: number, totp: string, password?: string): Promise> { + return request.post(`/api/v1/accounts/${id}/two-factor-auth`, { + totp, + password, + }); +} - disable(data: {totp: string, password?: string}): Promise> { - return request.delete('/api/two-factor-auth', { - totp: data.totp, - password: data.password || '' - }); - } -}; +export function disable(id: number, totp: string, password?: string): Promise> { + return request.delete(`/api/v1/accounts/${id}/two-factor-auth`, { + totp, + password, + }); +} From 22ae85c9c67a9f49eca6447bb90bce5cd8c414dd Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Mon, 4 Feb 2019 23:14:58 +0200 Subject: [PATCH 3/4] #355: fix authentication.test.js --- src/services/api/authentication.js | 2 +- src/services/api/authentication.test.js | 72 ++++++++++++++++++------- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/services/api/authentication.js b/src/services/api/authentication.js index 203cbab..c9eadea 100644 --- a/src/services/api/authentication.js +++ b/src/services/api/authentication.js @@ -92,7 +92,7 @@ export async function validateToken(id: number, token: string, refreshToken: ?st try { user = await getInfoEndpoint(id, token); } catch (resp) { - const token = await handleTokenError(resp, refreshToken); + token = await handleTokenError(resp, refreshToken); user = await getInfoEndpoint(id, token); // TODO: replace with recursive call } diff --git a/src/services/api/authentication.test.js b/src/services/api/authentication.test.js index d522fd7..6931561 100644 --- a/src/services/api/authentication.test.js +++ b/src/services/api/authentication.test.js @@ -1,3 +1,4 @@ +/* eslint-disable camelcase */ import expect from 'unexpected'; import sinon from 'sinon'; @@ -6,6 +7,28 @@ import * as authentication from 'services/api/authentication'; import * as accounts from 'services/api/accounts'; describe('authentication api', () => { + let server; + + beforeEach(() => { + server = sinon.createFakeServer({ + autoRespond: true + }); + + ['get', 'post'].forEach((method) => { + server[method] = (url, resp = {}, status = 200, headers = {}) => { + server.respondWith(method, url, [ + status, + { 'Content-Type': 'application/json', ...headers }, + JSON.stringify(resp) + ]); + }; + }); + }); + + afterEach(() => { + server.restore(); + }); + describe('#login', () => { const params = { login: 'foo', @@ -113,15 +136,25 @@ describe('authentication api', () => { authentication.requestToken.restore(); }); - it('resolves with new token and user object', () => - expect(authentication.validateToken(...validateTokenArgs), + it('resolves with new token and user object', async () => { + server.post('/api/authentication/refresh-token', { + access_token: newToken, + refresh_token: validRefreshToken, + success: true, + expires_in: 50000 + }); + + + await expect(authentication.validateToken(...validateTokenArgs), 'to be fulfilled with', {token: newToken, refreshToken: validRefreshToken, user} - ) - ); + ); + + expect(server.requests[0].requestBody, 'to equal', `refresh_token=${validRefreshToken}`); + }); it('rejects if token request failed', () => { - const error = 'Something wrong'; - authentication.requestToken.returns(Promise.reject(error)); + const error = {error: 'Unexpected error example'}; + server.post('/api/authentication/refresh-token', error, 500); return expect(authentication.validateToken(...validateTokenArgs), 'to be rejected with', error @@ -140,25 +173,28 @@ describe('authentication api', () => { const newToken = 'baz'; beforeEach(() => { - sinon.stub(authentication, 'requestToken'); - accounts.getInfo.onCall(0).returns(Promise.reject(expiredResponse)); - authentication.requestToken.returns(Promise.resolve(newToken)); }); - afterEach(() => { - authentication.requestToken.restore(); - }); + it('resolves with new token and user object', async () => { + server.post('/api/authentication/refresh-token', { + access_token: newToken, + refresh_token: validRefreshToken, + success: true, + expires_in: 50000 + }); - it('resolves with new token and user object', () => - expect(authentication.validateToken(...validateTokenArgs), + + await expect(authentication.validateToken(...validateTokenArgs), 'to be fulfilled with', {token: newToken, refreshToken: validRefreshToken, user} - ) - ); + ); + + expect(server.requests[0].requestBody, 'to equal', `refresh_token=${validRefreshToken}`); + }); it('rejects if token request failed', () => { - const error = 'Something wrong'; - authentication.requestToken.returns(Promise.reject(error)); + const error = {error: 'Unexpected error example'}; + server.post('/api/authentication/refresh-token', error, 500); return expect(authentication.validateToken(...validateTokenArgs), 'to be rejected with', error From 11f2673ca4a4aa453ebbbd7d0d05b6f4df925b35 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Mon, 4 Feb 2019 23:16:59 +0200 Subject: [PATCH 4/4] #355: fix accounts/actions.test.js --- src/components/accounts/actions.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/accounts/actions.test.js b/src/components/accounts/actions.test.js index 222f274..7255945 100644 --- a/src/components/accounts/actions.test.js +++ b/src/components/accounts/actions.test.js @@ -272,7 +272,7 @@ describe('components/accounts/actions', () => { it('should call logout api method in background', () => revoke(account)(dispatch, getState).then(() => expect(authentication.logout, 'to have a call satisfying', [ - account + account.token ]) ) ); @@ -323,7 +323,7 @@ describe('components/accounts/actions', () => { it('should call logout api method in background', () => revoke(account2)(dispatch, getState).then(() => expect(authentication.logout, 'to have a call satisfying', [ - account2 + account2.token ]) ) ); @@ -350,8 +350,8 @@ describe('components/accounts/actions', () => { logoutAll()(dispatch, getState); expect(authentication.logout, 'to have calls satisfying', [ - [account], - [account2] + [account.token], + [account2.token] ]); }); @@ -420,8 +420,8 @@ describe('components/accounts/actions', () => { logoutStrangers()(dispatch, getState); expect(authentication.logout, 'to have calls satisfying', [ - [foreignAccount], - [foreignAccount2] + [foreignAccount.token], + [foreignAccount2.token] ]); }); @@ -483,8 +483,8 @@ describe('components/accounts/actions', () => { it('logouts all accounts', () => { expect(authentication.logout, 'to have calls satisfying', [ - [foreignAccount], - [foreignAccount2], + [foreignAccount.token], + [foreignAccount2.token], ]); expect(dispatch, 'to have a call satisfying', [