#246: remove redundant /current calls during account authentication

src/components/accounts/actions.js

@@ -25,28 +25,27 @@ import logger from 'services/logger';
 export function authenticate({token, refreshToken}) {
     return (dispatch) =>
         authentication.validateToken({token, refreshToken})
-            .catch(() => {
+            .catch((resp) => {
-                // TODO: log this case
+                logger.warn('Error validating token during auth', {
-                dispatch(logoutAll());
+                    resp
+                });
 
-                return Promise.reject();
+                return dispatch(logoutAll())
+                    .then(() => Promise.reject());
             })
-            .then(({token, refreshToken}) =>
+            .then(({token, refreshToken, user}) => ({
-                accounts.current({token})
+                user: {
-                    .then((user) => ({
+                    isGuest: false,
-                        user: {
+                    ...user
-                            isGuest: false,
+                },
-                            ...user
+                account: {
-                        },
+                    id: user.id,
-                        account: {
+                    username: user.username,
-                            id: user.id,
+                    email: user.email,
-                            username: user.username,
+                    token,
-                            email: user.email,
+                    refreshToken
-                            token,
+                }
-                            refreshToken
+            }))
-                        }
-                    }))
-            )
             .then(({user, account}) => {
                 dispatch(add(account));
                 dispatch(activate(account));

src/services/api/authentication.js

@@ -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);

tests/components/accounts/actions.test.js

@@ -45,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', [
+                expect(authentication.validateToken, 'to have a call satisfying', [
-                    {token: account.token}
+                    {token: account.token, refreshToken: account.refreshToken}
                 ])
             )
         );
@@ -112,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(() =>
+            return expect(authenticate(account)(dispatch), 'to be rejected').then(() => {
-                expect(dispatch, 'was not called')
+                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);

tests/services/api/authentication.test.js

@@ -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', () =>
+        it('should resolve with both tokens and user object', () =>
-            expect(authentication.validateToken(validTokens), 'to be fulfilled with', validTokens)
+            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}
                 )
             );