From f0dbdc860aa6de8f8a9c722627871054b7c4b052 Mon Sep 17 00:00:00 2001
From: David Luecke <daff@neyeon.com>
Date: Sat, 24 Aug 2019 08:39:44 -0700
Subject: [PATCH 1/2] fix: Only remove token on NotAuthenticated error in
 authentication client and handle error better

---
 packages/authentication-client/src/core.ts    | 47 ++++++++++---------
 .../authentication-client/test/index.test.ts  |  3 +-
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/packages/authentication-client/src/core.ts b/packages/authentication-client/src/core.ts
index d5385878e5..b4b08e6ac0 100644
--- a/packages/authentication-client/src/core.ts
+++ b/packages/authentication-client/src/core.ts
@@ -1,4 +1,4 @@
-import { NotAuthenticated } from '@feathersjs/errors';
+import { NotAuthenticated, FeathersError } from '@feathersjs/errors';
 import { Application } from '@feathersjs/feathers';
 import { AuthenticationRequest, AuthenticationResult } from '@feathersjs/authentication';
 import { Storage, StorageWrapper } from './storage';
@@ -118,6 +118,16 @@ export class AuthenticationClient {
     return Promise.resolve(null);
   }
 
+  handleError (error: FeathersError, type: 'authenticate'|'logout') {
+    if (error.code === 401) {
+      const promise = this.removeAccessToken().then(() => this.reset());
+
+      return type === 'logout' ? promise : promise.then(() => Promise.reject(error));
+    }
+    
+    return Promise.reject(error);
+  }
+
   reAuthenticate (force: boolean = false): Promise<AuthenticationResult> {
     // Either returns the authentication state or
     // tries to re-authenticate with the stored JWT and strategy
@@ -133,9 +143,7 @@ export class AuthenticationClient {
           strategy: this.options.jwtStrategy,
           accessToken
         });
-      }).catch((error: Error) =>
-        this.removeAccessToken().then(() => Promise.reject(error))
-      );
+      });
     }
 
     return authPromise;
@@ -155,8 +163,8 @@ export class AuthenticationClient {
         this.app.emit('authenticated', authResult);
 
         return this.setAccessToken(accessToken).then(() => authResult);
-      }).catch((error: Error) =>
-        this.reset().then(() => Promise.reject(error))
+      }).catch((error: FeathersError) =>
+        this.handleError(error, 'authenticate')
       );
 
     this.app.set('authentication', promise);
@@ -166,20 +174,17 @@ export class AuthenticationClient {
 
   logout () {
     return Promise.resolve(this.app.get('authentication'))
-      .then(auth => {
-        if (!auth) {
-          return null;
-        }
-
-        return this.service.remove(null)
-          .then((authResult: AuthenticationResult) => this.removeAccessToken()
-            .then(() => this.reset())
-            .then(() => {
-              this.app.emit('logout', authResult);
-
-              return authResult;
-            })
-          );
-      });
+      .then(() => this.service.remove(null)
+      .then((authResult: AuthenticationResult) => this.removeAccessToken()
+        .then(() => this.reset())
+        .then(() => {
+          this.app.emit('logout', authResult);
+
+          return authResult;
+        })
+      ))
+      .catch((error: FeathersError) =>
+        this.handleError(error, 'logout')
+      );
   }
 }
diff --git a/packages/authentication-client/test/index.test.ts b/packages/authentication-client/test/index.test.ts
index 125bfc0a96..01b247abbe 100644
--- a/packages/authentication-client/test/index.test.ts
+++ b/packages/authentication-client/test/index.test.ts
@@ -2,6 +2,7 @@ import assert from 'assert';
 import feathers, { Application } from '@feathersjs/feathers';
 
 import client, { AuthenticationClient } from '../src';
+import { NotAuthenticated } from '@feathersjs/errors';
 
 describe('@feathersjs/authentication-client', () => {
   const accessToken = 'testing';
@@ -25,7 +26,7 @@ describe('@feathersjs/authentication-client', () => {
 
       remove (id) {
         if (!app.get('authentication')) {
-          throw new Error('Not logged in');
+          throw new NotAuthenticated('Not logged in');
         }
 
         return Promise.resolve({ id });

From 26b639492aee120b0bc023423f5cfba1f0c2b5ce Mon Sep 17 00:00:00 2001
From: David Luecke <daff@neyeon.com>
Date: Sat, 24 Aug 2019 09:06:15 -0700
Subject: [PATCH 2/2] Improve test coverage, also handle 403

---
 packages/authentication-client/src/core.ts        |  4 ++--
 packages/authentication-client/test/index.test.ts | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/packages/authentication-client/src/core.ts b/packages/authentication-client/src/core.ts
index b4b08e6ac0..47b4040ed1 100644
--- a/packages/authentication-client/src/core.ts
+++ b/packages/authentication-client/src/core.ts
@@ -119,12 +119,12 @@ export class AuthenticationClient {
   }
 
   handleError (error: FeathersError, type: 'authenticate'|'logout') {
-    if (error.code === 401) {
+    if (error.code === 401 || error.code === 403) {
       const promise = this.removeAccessToken().then(() => this.reset());
 
       return type === 'logout' ? promise : promise.then(() => Promise.reject(error));
     }
-    
+
     return Promise.reject(error);
   }
 
diff --git a/packages/authentication-client/test/index.test.ts b/packages/authentication-client/test/index.test.ts
index 01b247abbe..06a7d737ef 100644
--- a/packages/authentication-client/test/index.test.ts
+++ b/packages/authentication-client/test/index.test.ts
@@ -16,7 +16,11 @@ describe('@feathersjs/authentication-client', () => {
 
     app.configure(client());
     app.use('/authentication', {
-      create (data) {
+      create (data: any) {
+        if (data.error) {
+          return Promise.reject(new Error('Did not work'));
+        }
+
         return Promise.resolve({
           accessToken,
           data,
@@ -136,6 +140,15 @@ describe('@feathersjs/authentication-client', () => {
     });
   });
 
+  it('does not remove AccessToken on other errors', () => {
+    return app.authenticate({
+      strategy: 'testing'
+    }).then(() => app.authenticate({
+      strategy: 'testing'
+    })).then(() => app.authentication.getAccessToken())
+    .then(at => assert.strictEqual(at, accessToken));
+  });
+
   it('logout when not logged in without error', async () => {
     const result = await app.logout();