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

@@ -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';

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';
 

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<UserProfile | undefined>;
-}
-
-/**
- * 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

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<UserProfile> {
     return new Promise<UserProfile>((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);
     });
   }
 }

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<UserProfile | undefined>;
+}
+
+/**
+ * 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;
+}

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;

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<AuthenticationMetadata | undefined>;

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()', () => {