Skip to content

Commit

Permalink
fix: Add method to reliably get default authentication service (#1470)
Browse files Browse the repository at this point in the history
  • Loading branch information
daffl committed Jul 20, 2019
1 parent 8211b98 commit e542cb3
Show file tree
Hide file tree
Showing 13 changed files with 55 additions and 40 deletions.
8 changes: 4 additions & 4 deletions packages/authentication-local/src/hooks/hash-password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { get, set, cloneDeep } from 'lodash';
import { BadRequest } from '@feathersjs/errors';
import Debug from 'debug';
import { HookContext } from '@feathersjs/feathers';
import { LocalStrategy } from '../strategy';

const debug = Debug('@feathersjs/authentication-local/hooks/hash-password');

Expand All @@ -28,15 +29,14 @@ export default function hashPassword (field: string, options: HashPasswordOption
return context;
}

const serviceName = options.authentication || app.get('defaultAuthentication');
const authService = app.service(serviceName);
const authService = app.defaultAuthentication(options.authentication);
const { strategy = 'local' } = options;

if (!authService || typeof authService.getStrategies !== 'function') {
throw new BadRequest(`Could not find '${serviceName}' service to hash password`);
throw new BadRequest(`Could not find an authentication service to hash password`);
}

const [ localStrategy ] = authService.getStrategies(strategy);
const [ localStrategy ] = authService.getStrategies(strategy) as LocalStrategy[];

if (!localStrategy || typeof localStrategy.hashPassword !== 'function') {
throw new BadRequest(`Could not find '${strategy}' strategy to hash password`);
Expand Down
2 changes: 1 addition & 1 deletion packages/authentication-local/src/strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class LocalStrategy extends AuthenticationBaseStrategy {
throw new NotAuthenticated(errorMessage);
}

async hashPassword (password: string) {
async hashPassword (password: string, _params: Params) {
return bcrypt.hash(password, this.configuration.hashSize);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('@feathersjs/authentication-local/hooks/hash-password', () => {
assert.fail('Should never get here');
} catch (error) {
assert.strictEqual(error.message,
`Could not find 'authentication' service to hash password`
'Could not find an authentication service to hash password'
);
}
});
Expand Down
4 changes: 2 additions & 2 deletions packages/authentication-oauth/src/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { express as grantExpress } from 'grant';
import Debug from 'debug';
import { Application } from '@feathersjs/feathers';
import { AuthenticationService, AuthenticationResult } from '@feathersjs/authentication';
import { AuthenticationResult } from '@feathersjs/authentication';
import qs from 'querystring';
import {
Application as ExpressApplication,
Expand Down Expand Up @@ -49,7 +49,7 @@ export default (options: OauthSetupSettings) => {
authApp.get('/:name/authenticate', async (req, res, next) => {
const { name } = req.params;
const { accessToken, grant } = req.session;
const service: AuthenticationService = app.service(authService);
const service = app.defaultAuthentication(authService);
const [ strategy ] = service.getStrategies(name) as OAuthStrategy[];
const sendResponse = async (data: AuthenticationResult|Error) => {
try {
Expand Down
8 changes: 3 additions & 5 deletions packages/authentication-oauth/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Debug from 'debug';
import { merge, each, omit } from 'lodash';
import { Application } from '@feathersjs/feathers';
import { AuthenticationService } from '@feathersjs/authentication';
import { OAuthStrategy, OAuthProfile } from './strategy';
import { default as setupExpress } from './express';
import { OauthSetupSettings, getDefaultSettings } from './utils';
Expand All @@ -11,17 +10,16 @@ const debug = Debug('@feathersjs/authentication-oauth');
export { OauthSetupSettings, OAuthStrategy, OAuthProfile };

export const setup = (options: OauthSetupSettings) => (app: Application) => {
const authPath = options.authService;
const service: AuthenticationService = app.service(authPath);
const service = app.defaultAuthentication ? app.defaultAuthentication(options.authService) : null;

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

const { oauth } = service.configuration;

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

Expand Down
5 changes: 2 additions & 3 deletions packages/authentication-oauth/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ import session from 'express-session';
import { Application } from '@feathersjs/feathers';

export interface OauthSetupSettings {
authService: string;
authService?: string;
linkStrategy: string;
expressSession: RequestHandler;
}

export const getDefaultSettings = (app: Application, other?: Partial<OauthSetupSettings>) => {
export const getDefaultSettings = (_app: Application, other?: Partial<OauthSetupSettings>) => {
const defaults: OauthSetupSettings = {
authService: app.get('defaultAuthentication'),
linkStrategy: 'jwt',
expressSession: session({
secret: Math.random().toString(36).substring(7),
Expand Down
2 changes: 1 addition & 1 deletion packages/authentication-oauth/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('@feathersjs/authentication-oauth', () => {
assert.fail('Should never get here');
} catch (error) {
assert.equal(error.message,
`'something' authentication service must exist before registering @feathersjs/authentication-oauth`
'An authentication service must exist before registering @feathersjs/authentication-oauth'
);
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/authentication-oauth/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('@feathersjs/authentication-oauth/utils', () => {
it('getDefaultSettings', () => {
const settings = getDefaultSettings(app);

assert.equal(settings.authService, 'authentication');
assert.equal(settings.authService, undefined);
assert.equal(settings.linkStrategy, 'jwt');
});
});
10 changes: 3 additions & 7 deletions packages/authentication/src/hooks/authenticate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { flatten, omit, merge } from 'lodash';
import { HookContext } from '@feathersjs/feathers';
import { NotAuthenticated } from '@feathersjs/errors';
import Debug from 'debug';
import { AuthenticationService } from '../service';

const debug = Debug('@feathersjs/authentication/hooks/authenticate');

Expand All @@ -22,12 +21,9 @@ export default (originalSettings: string|AuthenticateHookSettings, ...originalSt

return async (context: HookContext) => {
const { app, params, type, path, service } = context;
const {
service: authPath = app.get('defaultAuthentication'),
strategies
} = settings;
const { strategies } = settings;
const { provider, authentication } = params;
const authService = app.service(authPath) as unknown as AuthenticationService;
const authService = app.defaultAuthentication(settings.service);

debug(`Running authenticate hook on '${path}'`);

Expand All @@ -36,7 +32,7 @@ export default (originalSettings: string|AuthenticateHookSettings, ...originalSt
}

if (!authService || typeof authService.authenticate !== 'function') {
throw new NotAuthenticated(`Could not find authentication service at '${authPath}'`);
throw new NotAuthenticated('Could not find a valid authentication service');
}

// @ts-ignore
Expand Down
29 changes: 28 additions & 1 deletion packages/authentication/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,38 @@ import { merge, get } from 'lodash';
import { NotAuthenticated } from '@feathersjs/errors';
import { AuthenticationBase, AuthenticationResult, AuthenticationRequest } from './core';
import { connection, events } from './hooks';
import { Params, ServiceMethods } from '@feathersjs/feathers';
import { Application, Params, ServiceMethods } from '@feathersjs/feathers';

const debug = Debug('@feathersjs/authentication/service');

declare module '@feathersjs/feathers' {
interface Application<ServiceTypes = {}> {

/**
* Returns the default authentication service or the
* authentication service for a given path.
*
* @param location The service path to use (optional)
*/
defaultAuthentication (location?: string): AuthenticationService;
}
}

export class AuthenticationService extends AuthenticationBase implements Partial<ServiceMethods<AuthenticationResult>> {
constructor (app: Application, configKey: string = 'authentication', options = {}) {
super(app, configKey, options);

if (typeof app.defaultAuthentication !== 'function') {
app.defaultAuthentication = function (location?: string) {
const configKey = app.get('defaultAuthentication');
const path = location || Object.keys(this.services).find(current =>
this.service(current).configKey === configKey
);

return path ? this.service(path) : null;
};
}
}
/**
* Return the payload for a JWT based on the authentication result.
* Called internally by the `create` method.
Expand Down
2 changes: 1 addition & 1 deletion packages/authentication/test/hooks/authenticate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('authentication/hooks/authenticate', () => {
assert.fail('Should never get here');
} catch (error) {
assert.strictEqual(error.name, 'NotAuthenticated');
assert.strictEqual(error.message, `Could not find authentication service at 'authentication'`);
assert.strictEqual(error.message, `Could not find a valid authentication service`);
}
});

Expand Down
8 changes: 6 additions & 2 deletions packages/authentication/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import feathers, { Application, Service } from '@feathersjs/feathers';
import memory from 'feathers-memory';

import defaultOptions from '../src/options';
import { AuthenticationService } from '../src/service';
import { AuthenticationResult } from '../src/core';
import { AuthenticationService, AuthenticationResult } from '../src';

import { Strategy1 } from './fixtures';

Expand Down Expand Up @@ -38,6 +37,11 @@ describe('authentication/service', () => {
assert.deepStrictEqual(app.service('authentication').configuration, Object.assign({}, defaultOptions, app.get('authentication')));
});

it('app.defaultAuthentication()', () => {
assert.strictEqual(app.defaultAuthentication(), app.service('authentication'));
assert.strictEqual(app.defaultAuthentication('dummy'), undefined);
});

describe('create', () => {
it('creates a valid accessToken and includes strategy result', async () => {
const service = app.service('authentication');
Expand Down
13 changes: 2 additions & 11 deletions packages/express/lib/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,11 @@ const normalizeStrategy = (_settings = [], ..._strategies) =>
typeof _settings === 'string'
? { strategies: flatten([ _settings, ..._strategies ]) }
: _settings;
const getService = (settings, app) => {
const path = settings.service || app.get('defaultAuthentication');

if (typeof path !== 'string') {
return null;
}

return app.service(path) || null;
};

exports.parseAuthentication = (settings = {}) => {
return function (req, res, next) {
const { app } = req;
const service = getService(settings, app);
const service = app.defaultAuthentication ? app.defaultAuthentication(settings.service) : null;

if (service === null) {
return next();
Expand Down Expand Up @@ -53,7 +44,7 @@ exports.authenticate = (...strategies) => {

return function (req, res, next) {
const { app, authentication } = req;
const service = getService(settings, app);
const service = app.defaultAuthentication(settings.service);

debug('Authenticating with Express middleware and strategies', settings.strategies);

Expand Down

0 comments on commit e542cb3

Please sign in to comment.