feathersjs · daffl · May 8, 2019 · May 8, 2019 · May 8, 2019 · May 8, 2019
diff --git a/packages/authentication-oauth/src/express.ts b/packages/authentication-oauth/src/express.ts
@@ -1,7 +1,6 @@
 // @ts-ignore
 import { express as grantExpress } from 'grant';
 import Debug from 'debug';
-import session from 'express-session';
 import querystring from 'querystring';
 import { Application } from '@feathersjs/feathers';
 import { AuthenticationService, AuthenticationResult } from '@feathersjs/authentication';
@@ -10,30 +9,27 @@ import {
  original as express
 } from '@feathersjs/express';
 import { OauthSetupSettings } from './utils';
+import { OAuthStrategy } from '.';

 const grant = grantExpress();
 const debug = Debug('@feathersjs/authentication-oauth/express');

 export default (options: OauthSetupSettings) => {
  return (feathersApp: Application) => {
-    const { path, authService, linkStrategy } = options;
+    const { authService, linkStrategy } = options;
    const app = feathersApp as ExpressApplication;
    const config = app.get('grant');
-    const secret = Math.random().toString(36).substring(7);
-
+
    if (!config) {
      debug('No grant configuration found, skipping Express oAuth setup');
      return;
    }
-
+
+    const { path } = config.defaults;
    const grantApp = grant(config);
    const authApp = express();

-    authApp.use(session({
-      secret,
-      resave: true,
-      saveUninitialized: true
-    }));
+    authApp.use(options.expressSession);

    authApp.get('/:name', (req, res) => {
      const { name } = req.params;
@@ -56,15 +52,21 @@ export default (options: OauthSetupSettings) => {
      const { name } = req.params;
      const { accessToken, grant } = req.session;
      const service: AuthenticationService = app.service(authService);
+      const [ strategy ] = service.getStrategies(name) as OAuthStrategy[];
      const sendResponse = async (data: AuthenticationResult|Error) => {
-        const redirect = await options.getRedirect(service, data);
-
-        if (redirect !== null) {
-          res.redirect(redirect);
-        } else if (data instanceof Error) {
-          next(data);
-        } else {
-          res.json(data);
+        try {
+          const redirect = await strategy.getRedirect(data);
+
+          if (redirect !== null) {
+            res.redirect(redirect);
+          } else if (data instanceof Error) {
+            throw data;
+          } else {
+            res.json(data);
+          }
+        } catch (error) {
+          debug('oAuth error', error);
+          next(error);
        }
      };

@@ -73,7 +75,6 @@ export default (options: OauthSetupSettings) => {
          grant.response : req.query;

        const params = {
-          provider: 'rest',
          authStrategies: [ name ],
          authentication: accessToken ? {
            strategy: linkStrategy,
@@ -95,7 +96,7 @@ export default (options: OauthSetupSettings) => {
        await sendResponse(authResult);
      } catch (error) {
        debug('Received oAuth authentication error', error);
-        sendResponse(error);
+        await sendResponse(error);
      }
    });


diff --git a/packages/authentication-oauth/src/index.ts b/packages/authentication-oauth/src/index.ts
@@ -11,33 +11,34 @@ const debug = Debug('@feathersjs/authentication-oauth');
 export { OauthSetupSettings, OAuthStrategy, OAuthProfile };

 export const setup = (options: OauthSetupSettings) => (app: Application) => {
-  const path = options.authService;
-  const service: AuthenticationService = app.service(path);
+  const authPath = options.authService;
+  const service: AuthenticationService = app.service(authPath);

  if (!service) {
-    throw new Error(`'${path}' authentication service must exist before registering @feathersjs/authentication-oauth`);
+    throw new Error(`'${authPath}' authentication service must exist before registering @feathersjs/authentication-oauth`);
  }

  const { oauth } = service.configuration;

  if (!oauth) {
-    debug(`No oauth configuration found at '${path}'. Skipping oAuth setup.`);
+    debug(`No oauth configuration found at '${authPath}'. Skipping oAuth setup.`);
    return;
  }

  const { strategyNames } = service;
+  const { path = '/auth' } = oauth.defaults;
  const grant = merge({
    defaults: {
+      path,
      host: `${app.get('host')}:${app.get('port')}`,
-      path: '/auth',
      protocol: app.get('env') === 'production' ? 'https' : 'http',
      transport: 'session'
    }
  }, omit(oauth, 'redirect'));

  each(grant, (value, key) => {
    if (key !== 'defaults') {
-      value.callback = value.callback || `/auth/${key}/authenticate`;
+      value.callback = value.callback || `${path}/${key}/authenticate`;

      if (!strategyNames.includes(key)) {
        debug(`Registering oAuth default strategy for '${key}'`);
@@ -49,7 +50,7 @@ export const setup = (options: OauthSetupSettings) => (app: Application) => {
  app.set('grant', grant);
 };

-export const express = (settings: OauthSetupSettings = {}) => (app: Application) => {
+export const express = (settings: Partial<OauthSetupSettings> = {}) => (app: Application) => {
  const options = getDefaultSettings(app, settings);

  app.configure(setup(options));

diff --git a/packages/authentication-oauth/src/strategy.ts b/packages/authentication-oauth/src/strategy.ts
@@ -1,8 +1,9 @@
 // @ts-ignore
 import getProfile from 'grant-profile/lib/client';
+import querystring from 'querystring';
 import Debug from 'debug';
 import {
-  AuthenticationRequest, AuthenticationBaseStrategy
+  AuthenticationRequest, AuthenticationBaseStrategy, AuthenticationResult
 } from '@feathersjs/authentication';
 import { Params } from '@feathersjs/feathers';

@@ -29,6 +30,18 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
    return this.configuration.entityId || this.entityService.id;
  }

+  async getEntityQuery (profile: OAuthProfile, _params: Params) {
+    return {
+      [`${this.name}Id`]: profile.sub || profile.id
+    };
+  }
+
+  async getEntityData (profile: OAuthProfile, _existingEntity: any, _params: Params) {
+    return {
+      [`${this.name}Id`]: profile.sub || profile.id
+    };
+  }
+
  /* istanbul ignore next */
  async getProfile (data: AuthenticationRequest, _params: Params) {
    const config = this.app.get('grant');
@@ -50,17 +63,33 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
      const authResult = await this.authentication
        .authenticate(authentication, params, strategy);

-      return authResult[entity] || null;
+      return authResult[entity];
    }

    return null;
  }

-  async findEntity (profile: OAuthProfile, params: Params) {
-    const query = {
-      [`${this.name}Id`]: profile.id
+  async getRedirect (data: AuthenticationResult|Error) {
+    const { redirect } = this.authentication.configuration.oauth;
+
+    if (!redirect) {
+      return null;
+    }
+
+    const separator = redirect.endsWith('?') ? '' : '#';
+    const authResult: AuthenticationResult = data;
+    const query = authResult.accessToken ? {
+      access_token: authResult.accessToken
+    } : {
+      error: data.message || 'OAuth Authentication not successful'
    };

+    return redirect + separator + querystring.stringify(query);
+  }
+
+  async findEntity (profile: OAuthProfile, params: Params) {
+    const query = await this.getEntityQuery(profile, params);
+
    debug('findEntity with query', query);

    const result = await this.entityService.find({
@@ -75,9 +104,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
  }

  async createEntity (profile: OAuthProfile, params: Params) {
-    const data = {
-      [`${this.name}Id`]: profile.id
-    };
+    const data = await this.getEntityData(profile, null, params);

    debug('createEntity with data', data);

@@ -86,9 +113,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {

  async updateEntity (entity: any, profile: OAuthProfile, params: Params) {
    const id = entity[this.entityId];
-    const data = {
-      [`${this.name}Id`]: profile.id
-    };
+    const data = await this.getEntityData(profile, entity, params);

    debug(`updateEntity with id ${id} and data`, data);

@@ -103,8 +128,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {

    debug(`authenticate with (existing) entity`, existingEntity);

-    const authEntity = existingEntity === null
-      ? await this.createEntity(profile, params)
+    const authEntity = !existingEntity ? await this.createEntity(profile, params)
      : await this.updateEntity(existingEntity, profile, params);

    return {

diff --git a/packages/authentication-oauth/src/utils.ts b/packages/authentication-oauth/src/utils.ts
@@ -1,38 +1,22 @@
-import querystring from 'querystring';
+import { RequestHandler } from 'express';
+import session from 'express-session';
 import { Application } from '@feathersjs/feathers';
-import { AuthenticationService, AuthenticationResult } from '@feathersjs/authentication';

 export interface OauthSetupSettings {
-  path?: string;
-  authService?: string;
-  linkStrategy?: string;
-  getRedirect? (service: AuthenticationService, data: AuthenticationResult|Error): Promise<string>;
+  authService: string;
+  linkStrategy: string;
+  expressSession: RequestHandler;
 }

-export const getRedirect = async (service: AuthenticationService, data: AuthenticationResult|Error) => {
-  const { redirect } = service.configuration.oauth;
-
-  if (!redirect) {
-    return null;
-  }
-
-  const separator = redirect.endsWith('?') ? '' : '#';
-  const authResult: AuthenticationResult = data;
-  const query = authResult.accessToken ? {
-    access_token: authResult.accessToken
-  } : {
-    error: data.message || 'OAuth Authentication not successful'
-  };
-
-  return redirect + separator + querystring.stringify(query);
-};
-
-export const getDefaultSettings = (app: Application, other?: OauthSetupSettings) => {
+export const getDefaultSettings = (app: Application, other?: Partial<OauthSetupSettings>) => {
  const defaults: OauthSetupSettings = {
-    path: '/auth',
    authService: app.get('defaultAuthentication'),
    linkStrategy: 'jwt',
-    getRedirect,
+    expressSession: session({
+      secret: Math.random().toString(36).substring(7),
+      saveUninitialized: true,
+      resave: true
+    }),
    ...other
  };


diff --git a/packages/authentication-oauth/test/index.test.ts b/packages/authentication-oauth/test/index.test.ts
@@ -1,6 +1,6 @@
 import { strict as assert } from 'assert';
 import feathers from '@feathersjs/feathers';
-import { setup, express } from '../src';
+import { setup, express, OauthSetupSettings } from '../src';
 import { AuthenticationService } from '@feathersjs/authentication/lib';

 describe('@feathersjs/authentication-oauth', () => {
@@ -9,7 +9,7 @@ describe('@feathersjs/authentication-oauth', () => {
      const app = feathers();

      try {
-        app.configure(setup({ authService: 'something' }));
+        app.configure(setup({ authService: 'something' } as OauthSetupSettings));
        assert.fail('Should never get here');
      } catch (error) {
        assert.equal(error.message,

diff --git a/packages/authentication-oauth/test/strategy.test.ts b/packages/authentication-oauth/test/strategy.test.ts
@@ -4,14 +4,37 @@ import { AuthenticationService } from '@feathersjs/authentication/lib';

 describe('@feathersjs/authentication-oauth/strategy', () => {
  const authService: AuthenticationService = app.service('authentication');
-  const [strategy] = authService.getStrategies('test') as TestOAuthStrategy[];
+  const [ strategy ] = authService.getStrategies('test') as TestOAuthStrategy[];

  it('initializes, has .entityId and configuration', () => {
    assert.ok(strategy);
    assert.strictEqual(strategy.entityId, 'id');
    assert.ok(strategy.configuration.entity);
  });

+  it('getRedirect', async () => {
+    app.get('authentication').oauth.redirect = '/home';
+
+    let redirect = await strategy.getRedirect({ accessToken: 'testing' });
+    assert.equal(redirect, '/home#access_token=testing');
+
+    redirect = await strategy.getRedirect(new Error('something went wrong'));
+    assert.equal(redirect, '/home#error=something%20went%20wrong');
+
+    redirect = await strategy.getRedirect(new Error());
+    assert.equal(redirect, '/home#error=OAuth%20Authentication%20not%20successful');
+
+    app.get('authentication').oauth.redirect = '/home?';
+
+    redirect = await strategy.getRedirect({ accessToken: 'testing' });
+    assert.equal(redirect, '/home?access_token=testing');
+
+    delete app.get('authentication').oauth.redirect;
+
+    redirect = await strategy.getRedirect({ accessToken: 'testing' });
+    assert.equal(redirect, null);
+  });
+
  it('getProfile', async () => {
    const data = { id: 'getProfileTest' };
    const profile = await strategy.getProfile(data, {});

diff --git a/packages/authentication-oauth/test/utils.test.ts b/packages/authentication-oauth/test/utils.test.ts
@@ -1,38 +1,11 @@
 import { strict as assert } from 'assert';
-import { getRedirect, getDefaultSettings } from '../src/utils';
+import { getDefaultSettings } from '../src/utils';
 import { app } from './fixture';
-import { AuthenticationService } from '@feathersjs/authentication/lib';

 describe('@feathersjs/authentication-oauth/utils', () => {
-  it('getRedirect', async () => {
-    const service: AuthenticationService = app.service('authentication');
-
-    app.get('authentication').oauth.redirect = '/home';
-
-    let redirect = await getRedirect(service, { accessToken: 'testing' });
-    assert.equal(redirect, '/home#access_token=testing');
-
-    redirect = await getRedirect(service, { message: 'something went wrong' });
-    assert.equal(redirect, '/home#error=something%20went%20wrong');
-
-    redirect = await getRedirect(service, {});
-    assert.equal(redirect, '/home#error=OAuth%20Authentication%20not%20successful');
-
-    app.get('authentication').oauth.redirect = '/home?';
-
-    redirect = await getRedirect(service, { accessToken: 'testing' });
-    assert.equal(redirect, '/home?access_token=testing');
-
-    delete app.get('authentication').oauth.redirect;
-
-    redirect = await getRedirect(service, { accessToken: 'testing' });
-    assert.equal(redirect, null);
-  });
-
  it('getDefaultSettings', () => {
    const settings = getDefaultSettings(app);

-    assert.equal(settings.path, '/auth');
    assert.equal(settings.authService, 'authentication');
    assert.equal(settings.linkStrategy, 'jwt');
  });