From f8646885a550a575d405229fc762dfb04be92a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 16 May 2018 11:06:45 +0200 Subject: [PATCH] refactor(authentication): leverage Express req/res types Now that the REST layer uses Express request/response types internally, we can simplify StrategyAdapter by removing ShimRequest. While making these changes, other places are cleaned up as well. --- packages/authentication/src/index.ts | 4 +- packages/authentication/src/keys.ts | 2 +- .../src/providers/authentication.provider.ts | 30 +-------- .../authentication/src/strategy-adapter.ts | 65 ++++--------------- packages/authentication/src/types.ts | 24 +++++++ .../test/unit/fixtures/mock-strategy.ts | 21 +++--- .../providers/auth-metadata.provider.unit.ts | 7 +- .../providers/authentication.provider.unit.ts | 8 +-- 8 files changed, 53 insertions(+), 108 deletions(-) create mode 100644 packages/authentication/src/types.ts diff --git a/packages/authentication/src/index.ts b/packages/authentication/src/index.ts index d1ce056046ff..9d67afd34c49 100644 --- a/packages/authentication/src/index.ts +++ b/packages/authentication/src/index.ts @@ -3,10 +3,8 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT +export * from './types'; export * from './authentication.component'; export * from './decorators'; export * from './keys'; export * from './strategy-adapter'; - -// internals for tests -export * from './providers'; diff --git a/packages/authentication/src/keys.ts b/packages/authentication/src/keys.ts index a6fb38be729c..7d0c5e885137 100644 --- a/packages/authentication/src/keys.ts +++ b/packages/authentication/src/keys.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Strategy} from 'passport'; -import {AuthenticateFn, UserProfile} from './providers/authentication.provider'; +import {AuthenticateFn, UserProfile} from './types'; import {AuthenticationMetadata} from './decorators/authenticate.decorator'; import {BindingKey, MetadataAccessor} from '@loopback/context'; diff --git a/packages/authentication/src/providers/authentication.provider.ts b/packages/authentication/src/providers/authentication.provider.ts index 5b1ee566f6c2..880f8a8f89dc 100644 --- a/packages/authentication/src/providers/authentication.provider.ts +++ b/packages/authentication/src/providers/authentication.provider.ts @@ -3,36 +3,12 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT +import {Getter, Provider, Setter, inject} from '@loopback/context'; import {Request} from '@loopback/rest'; -import {inject} from '@loopback/core'; -import {Provider, Getter, Setter} from '@loopback/context'; import {Strategy} from 'passport'; -import {StrategyAdapter} from '../strategy-adapter'; import {AuthenticationBindings} from '../keys'; - -/** - * Passport monkey-patches Node.js' IncomingMessage prototype - * and adds extra methods like "login" and "isAuthenticated" - */ -export type PassportRequest = Request & Express.Request; - -/** - * interface definition of a function which accepts a request - * and returns an authenticated user - */ -export interface AuthenticateFn { - (request: Request): Promise; -} - -/** - * interface definition of a user profile - * http://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - */ -export interface UserProfile { - id: string; - name?: string; - email?: string; -} +import {StrategyAdapter} from '../strategy-adapter'; +import {AuthenticateFn, UserProfile} from '../types'; /** * @description Provider of a function which authenticates diff --git a/packages/authentication/src/strategy-adapter.ts b/packages/authentication/src/strategy-adapter.ts index cd514168314e..1747db7cab24 100644 --- a/packages/authentication/src/strategy-adapter.ts +++ b/packages/authentication/src/strategy-adapter.ts @@ -5,57 +5,9 @@ import {HttpErrors, Request} from '@loopback/rest'; import {Strategy} from 'passport'; -import {UserProfile} from './providers/authentication.provider'; +import {UserProfile} from './types'; -const PassportRequestExtras: Express.Request = require('passport/lib/http/request'); - -/** - * Shimmed Request to satisfy express requirements of passport strategies. - */ -export class ShimRequest implements Express.Request { - headers: Object; - query: Object; - url: string; - path: string; - method: string; - - constructor(request: Request) { - this.headers = request.headers; - this.query = request.query; - this.url = request.url; - this.path = request.path; - this.method = request.method; - } - - // tslint:disable:no-any - login(user: any, done: (err: any) => void): void; - login(user: any, options: any, done: (err: any) => void): void; - login(user: any, options: any, done?: any) { - PassportRequestExtras.login.apply(this, arguments); - } - - logIn(user: any, done: (err: any) => void): void; - logIn(user: any, options: any, done: (err: any) => void): void; - logIn(user: any, options: any, done?: any) { - PassportRequestExtras.logIn.apply(this, arguments); - } - - logout(): void { - PassportRequestExtras.logout.apply(this, arguments); - } - - logOut(): void { - PassportRequestExtras.logOut.apply(this, arguments); - } - - isAuthenticated(): boolean { - return PassportRequestExtras.isAuthenticated.apply(this, arguments); - } - isUnauthenticated(): boolean { - return PassportRequestExtras.isUnauthenticated.apply(this, arguments); - } - // tslint:enable:no-any -} +const passportRequestMixin = require('passport/lib/http/request'); /** * Adapter class to invoke passport-strategy @@ -77,11 +29,16 @@ export class StrategyAdapter { * 1. Create an instance of the strategy * 2. add success and failure state handlers * 3. authenticate using the strategy - * @param req {http.ServerRequest} The incoming request. + * @param request The incoming request. */ - authenticate(req: Request) { - const shimReq = new ShimRequest(req); + authenticate(request: Request): Promise { return new Promise((resolve, reject) => { + // mix-in passport additions like req.logIn and req.logOut + for (const key in passportRequestMixin) { + // tslint:disable-next-line:no-any + (request as any)[key] = passportRequestMixin[key]; + } + // create a prototype chain of an instance of a passport strategy const strategy = Object.create(this.strategy); @@ -101,7 +58,7 @@ export class StrategyAdapter { }; // authenticate - strategy.authenticate(shimReq); + strategy.authenticate(request); }); } } diff --git a/packages/authentication/src/types.ts b/packages/authentication/src/types.ts new file mode 100644 index 000000000000..12a170ce3d29 --- /dev/null +++ b/packages/authentication/src/types.ts @@ -0,0 +1,24 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/authentication +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {Request} from '@loopback/rest'; + +/** + * interface definition of a function which accepts a request + * and returns an authenticated user + */ +export interface AuthenticateFn { + (request: Request): Promise; +} + +/** + * interface definition of a user profile + * http://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + */ +export interface UserProfile { + id: string; + name?: string; + email?: string; +} diff --git a/packages/authentication/test/unit/fixtures/mock-strategy.ts b/packages/authentication/test/unit/fixtures/mock-strategy.ts index 4726644f92b4..f315a81db13e 100644 --- a/packages/authentication/test/unit/fixtures/mock-strategy.ts +++ b/packages/authentication/test/unit/fixtures/mock-strategy.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Strategy, AuthenticateOptions} from 'passport'; -import {PassportRequest} from '../../..'; +import {Request} from 'express'; /** * Test fixture for a mock asynchronous passport-strategy @@ -21,7 +21,7 @@ export class MockStrategy extends Strategy { * authenticate() function similar to passport-strategy packages * @param req */ - async authenticate(req: Express.Request, options?: AuthenticateOptions) { + async authenticate(req: Request, options?: AuthenticateOptions) { await this.verify(req); } /** @@ -33,21 +33,18 @@ export class MockStrategy extends Strategy { * pass req.query.testState = 'fail' to mock failed authorization * pass req.query.testState = 'error' to mock unexpected error */ - async verify(request: Express.Request) { - // A workaround for buggy typings provided by @types/passport - const req = request as PassportRequest; - + async verify(request: Request) { if ( - req.headers && - req.headers.testState && - req.headers.testState === 'fail' + request.headers && + request.headers.testState && + request.headers.testState === 'fail' ) { this.returnUnauthorized('authorization failed'); return; } else if ( - req.headers && - req.headers.testState && - req.headers.testState === 'error' + request.headers && + request.headers.testState && + request.headers.testState === 'error' ) { this.returnError('unexpected error'); return; diff --git a/packages/authentication/test/unit/providers/auth-metadata.provider.unit.ts b/packages/authentication/test/unit/providers/auth-metadata.provider.unit.ts index 28adfc50ecda..5f82a0e59169 100644 --- a/packages/authentication/test/unit/providers/auth-metadata.provider.unit.ts +++ b/packages/authentication/test/unit/providers/auth-metadata.provider.unit.ts @@ -6,11 +6,8 @@ import {expect} from '@loopback/testlab'; import {CoreBindings} from '@loopback/core'; import {Context, Provider} from '@loopback/context'; -import { - AuthMetadataProvider, - AuthenticationMetadata, - authenticate, -} from '../../..'; +import {AuthenticationMetadata, authenticate} from '../../..'; +import {AuthMetadataProvider} from '../../../src/providers'; describe('AuthMetadataProvider', () => { let provider: Provider; diff --git a/packages/authentication/test/unit/providers/authentication.provider.unit.ts b/packages/authentication/test/unit/providers/authentication.provider.unit.ts index b8bf148c5b62..0c727be83e7b 100644 --- a/packages/authentication/test/unit/providers/authentication.provider.unit.ts +++ b/packages/authentication/test/unit/providers/authentication.provider.unit.ts @@ -6,14 +6,10 @@ import {expect} from '@loopback/testlab'; import {Context, instantiateClass} from '@loopback/context'; import {Request} from '@loopback/rest'; -import { - AuthenticateActionProvider, - AuthenticateFn, - UserProfile, - AuthenticationBindings, -} from '../../..'; +import {AuthenticateFn, UserProfile, AuthenticationBindings} from '../../..'; import {MockStrategy} from '../fixtures/mock-strategy'; import {Strategy} from 'passport'; +import {AuthenticateActionProvider} from '../../../src/providers'; describe('AuthenticateActionProvider', () => { describe('constructor()', () => {