fix(authentication-oauth): Fix oAuth origin and error handling (#2752)

feathersjs · Sep 15, 2022 · f7e1c33 · f7e1c33
1 parent 1d45427
commit f7e1c33
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 20 deletions.
diff --git a/packages/authentication-oauth/src/service.ts b/packages/authentication-oauth/src/service.ts
@@ -1,5 +1,6 @@
 import { createDebug } from '@feathersjs/commons'
 import { HookContext, NextFunction, Params } from '@feathersjs/feathers'
+import { FeathersError } from '@feathersjs/errors'
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 //@ts-ignore
 import Grant from 'grant/lib/grant'
@@ -23,17 +24,35 @@ export type OAuthParams = Omit<Params, 'route'> & {
   }
 }
 
+export class OAuthError extends FeathersError {
+  constructor(message: string, data: any, public location: string) {
+    super(message, 'NotAuthenticated', 401, 'not-authenticated', data)
+  }
+}
+
 export const redirectHook = () => async (context: HookContext, next: NextFunction) => {
-  await next()
+  try {
+    await next()
 
-  const { location } = context.result
+    const { location } = context.result
 
-  debug(`oAuth redirect to ${location}`)
+    debug(`oAuth redirect to ${location}`)
 
-  if (location) {
-    context.http = {
-      ...context.http,
-      location
+    if (location) {
+      context.http = {
+        ...context.http,
+        location
+      }
+    }
+  } catch (error: any) {
+    if (error.location) {
+      context.http = {
+        ...context.http,
+        location: error.location
+      }
+      context.result = typeof error.toJSON === 'function' ? error.toJSON() : error
+    } else {
+      throw error
     }
   }
 }
@@ -93,21 +112,33 @@ export class OAuthService {
       ...payload
     }
 
-    debug(`Calling ${authService}.create authentication with strategy ${name}`)
+    try {
+      debug(`Calling ${authService}.create authentication with strategy ${name}`)
 
-    const authResult = await this.service.create(authentication, authParams)
+      const authResult = await this.service.create(authentication, authParams)
 
-    debug('Successful oAuth authentication, sending response')
+      debug('Successful oAuth authentication, sending response')
 
-    const location = await strategy.getRedirect(authResult, authParams)
+      const location = await strategy.getRedirect(authResult, authParams)
 
-    if (typeof params.session.destroy === 'function') {
-      await params.session.destroy()
-    }
+      if (typeof params.session.destroy === 'function') {
+        await params.session.destroy()
+      }
+
+      return {
+        ...authResult,
+        location
+      }
+    } catch (error: any) {
+      const location = await strategy.getRedirect(error, authParams)
+      const e = new OAuthError(error.message, error.data, location)
+
+      if (typeof params.session.destroy === 'function') {
+        await params.session.destroy()
+      }
 
-    return {
-      ...authResult,
-      location
+      e.stack = error.stack
+      throw e
     }
   }
 

diff --git a/packages/authentication-oauth/src/strategy.ts b/packages/authentication-oauth/src/strategy.ts
@@ -71,11 +71,11 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
     const { redirect, origins = this.app.get('origins') } = this.authentication.configuration.oauth
 
     if (Array.isArray(origins)) {
-      const referer = params?.headers?.referer || ''
+      const referer = params?.headers?.referer || origins[0]
       const allowedOrigin = origins.find((current) => referer.toLowerCase().startsWith(current.toLowerCase()))
 
       if (!allowedOrigin) {
-        throw new NotAuthenticated(`Referer "${referer || '[header not available]'}" not allowed.`)
+        throw new NotAuthenticated(`Referer "${referer}" is not allowed.`)
       }
 
       return allowedOrigin

diff --git a/packages/authentication-oauth/test/strategy.test.ts b/packages/authentication-oauth/test/strategy.test.ts
@@ -78,6 +78,9 @@ describe('@feathersjs/authentication-oauth/strategy', () => {
     )
     assert.equal(redirect, 'https://feathersjs.com#access_token=testing')
 
+    redirect = await strategy.getRedirect({ accessToken: 'testing' }, {})
+    assert.equal(redirect, 'https://feathersjs.com#access_token=testing')
+
     redirect = await strategy.getRedirect(
       { accessToken: 'testing' },
       {
@@ -110,7 +113,7 @@ describe('@feathersjs/authentication-oauth/strategy', () => {
           }
         ),
       {
-        message: 'Referer "https://example.com" not allowed.'
+        message: 'Referer "https://example.com" is not allowed.'
       }
     )
   })