fix: Handle error oAuth redirect in authentication client (#1307)

packages/authentication-client/src/core.ts

@@ -3,6 +3,19 @@ import { Application } from '@feathersjs/feathers';
 import { AuthenticationRequest, AuthenticationResult } from '@feathersjs/authentication';
 import { Storage, StorageWrapper } from './storage';
 
+const getMatch = (location: Location, key: string): [ string, RegExp ] => {
+  const regex = new RegExp(`(?:\&?)${key}=([^&]*)`);
+  const match = location.hash ? location.hash.match(regex) : null;
+
+  if (match !== null) {
+    const [ , value ] = match;
+
+    return [ value, regex ];
+  }
+
+  return [ null, regex ];
+};
+
 export type ClientConstructor = new (app: Application, options: AuthenticationClientOptions)
   => AuthenticationClient;
 
@@ -12,6 +25,7 @@ export interface AuthenticationClientOptions {
   scheme: string;
   storageKey: string;
   locationKey: string;
+  locationErrorKey: string;
   jwtStrategy: string;
   path: string;
   Authentication: ClientConstructor;
@@ -58,27 +72,29 @@ export class AuthenticationClient {
     });
   }
 
-  setJwt (accessToken: string) {
+  setAccessToken (accessToken: string) {
     return this.storage.setItem(this.options.storageKey, accessToken);
   }
 
-  getFromLocation (location: Location): Promise<string|null> {
-    const regex = new RegExp(`(?:\&?)${this.options.locationKey}=([^&]*)`);
-    const type = location.hash ? 'hash' : 'search';
-    const match = location[type] ? location[type].match(regex) : null;
+  async getFromLocation (location: Location) {
+    const [ accessToken, tokenRegex ] = getMatch(location, this.options.locationKey);
+
+    if (accessToken !== null) {
+      location.hash = location.hash.replace(tokenRegex, '');
 
-    if (match !== null) {
-      const [ , value ] = match;
+      return accessToken;
+    }
 
-      location[type] = location[type].replace(regex, '');
+    const [ errorMessage ] = getMatch(location, this.options.locationErrorKey);
 
-      return Promise.resolve(value);
+    if (errorMessage !== null) {
+      throw new NotAuthenticated(errorMessage);
     }
 
-    return Promise.resolve(null);
+    return null;
   }
 
-  getJwt (): Promise<string|null> {
+  getAccessToken (): Promise<string|null> {
     return this.storage.getItem(this.options.storageKey)
       .then((accessToken: string) => {
         if (!accessToken && typeof window !== 'undefined' && window.location) {
@@ -89,7 +105,7 @@ export class AuthenticationClient {
       });
   }
 
-  removeJwt () {
+  removeAccessToken () {
     return this.storage.removeItem(this.options.storageKey);
   }
 
@@ -106,7 +122,7 @@ export class AuthenticationClient {
     const authPromise = this.app.get('authentication');
 
     if (!authPromise || force === true) {
-      return this.getJwt().then(accessToken => {
+      return this.getAccessToken().then(accessToken => {
         if (!accessToken) {
           throw new NotAuthenticated('No accessToken found in storage');
         }
@@ -116,7 +132,7 @@ export class AuthenticationClient {
           accessToken
         });
       }).catch((error: Error) =>
-        this.removeJwt().then(() => Promise.reject(error))
+        this.removeAccessToken().then(() => Promise.reject(error))
       );
     }
 
@@ -136,7 +152,7 @@ export class AuthenticationClient {
         this.app.emit('login', authResult);
         this.app.emit('authenticated', authResult);
 
-        return this.setJwt(accessToken).then(() => authResult);
+        return this.setAccessToken(accessToken).then(() => authResult);
       }).catch((error: Error) =>
         this.reset().then(() => Promise.reject(error))
       );
@@ -149,7 +165,7 @@ export class AuthenticationClient {
   logout () {
     return this.app.get('authentication')
       .then(() => this.service.remove(null))
-      .then((authResult: AuthenticationResult) => this.removeJwt()
+      .then((authResult: AuthenticationResult) => this.removeAccessToken()
         .then(() => this.reset())
         .then(() => {
           this.app.emit('logout', authResult);

packages/authentication-client/src/index.ts

@@ -28,6 +28,7 @@ export const defaults: AuthenticationClientOptions = {
   scheme: 'Bearer',
   storageKey: 'feathers-jwt',
   locationKey: 'access_token',
+  locationErrorKey: 'error',
   jwtStrategy: 'jwt',
   path: '/authentication',
   Authentication: AuthenticationClient,

packages/authentication-client/test/index.test.ts

@@ -41,18 +41,18 @@ describe('@feathersjs/authentication-client', () => {
     assert.strictEqual(typeof app.logout, 'function');
   });
 
-  it('setJwt, getJwt, removeJwt', async () => {
+  it('setAccessToken, getAccessToken, removeJwt', async () => {
     const auth = app.authentication;
     const token = 'hi';
 
-    await auth.setJwt(token);
+    await auth.setAccessToken(token);
 
-    const res = await auth.getJwt();
+    const res = await auth.getAccessToken();
 
     assert.strictEqual(res, token);
 
-    await auth.removeJwt();
-    assert.strictEqual(await auth.getJwt(), null);
+    await auth.removeAccessToken();
+    assert.strictEqual(await auth.getAccessToken(), null);
   });
 
   it('getFromLocation', async () => {
@@ -73,9 +73,17 @@ describe('@feathersjs/authentication-client', () => {
     dummyLocation = { search: 'access_token=testing' } as Location;
     token = await auth.getFromLocation(dummyLocation);
 
-    assert.strictEqual(token, 'testing');
-    assert.strictEqual(dummyLocation.search, '');
     assert.strictEqual(await auth.getFromLocation({} as Location), null);
+
+    try {
+      await auth.getFromLocation({
+        hash: 'error=Error Happened&x=y'
+      } as Location);
+      assert.fail('Should never get here');
+    } catch (error) {
+      assert.strictEqual(error.name, 'NotAuthenticated');
+      assert.strictEqual(error.message, 'Error Happened');
+    }
   });
 
   it('authenticate, authentication hook, login event', () => {
@@ -96,7 +104,7 @@ describe('@feathersjs/authentication-client', () => {
         user
       });
 
-      return app.authentication.getJwt();
+      return app.authentication.getAccessToken();
     }).then(at => {
       assert.strictEqual(at, accessToken, 'Set accessToken in storage');