refactor(rest-express): use middleware to setup request infos (#1028)

pradel · web-flow · commit 99d988f235ce · 2020-07-27T10:15:42.000+02:00
diff --git a/packages/rest-express/__tests__/endpoints/impersonate.ts b/packages/rest-express/__tests__/endpoints/impersonate.ts
@@ -25,16 +25,17 @@ describe('impersonate', () => {
         accessToken: 'token',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.impersonate).toHaveBeenCalledWith('token', 'toto', {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.impersonate).toHaveBeenCalledWith('token', 'toto', req.infos);
     expect(res.json).toHaveBeenCalledWith(impersonateReturnType);
     expect(res.status).not.toHaveBeenCalled();
   });
@@ -53,16 +54,17 @@ describe('impersonate', () => {
         accessToken: 'token',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.impersonate).toHaveBeenCalledWith('token', 'toto', {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.impersonate).toHaveBeenCalledWith('token', 'toto', req.infos);
     expect(res.status).toHaveBeenCalledWith(400);
     expect(res.json).toHaveBeenCalledWith(error);
   });
diff --git a/packages/rest-express/__tests__/endpoints/oauth/provider-callback.ts b/packages/rest-express/__tests__/endpoints/oauth/provider-callback.ts
@@ -27,6 +27,10 @@ describe('providerCallback', () => {
         accessTokenSecret: 'secret',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
@@ -36,7 +40,7 @@ describe('providerCallback', () => {
     expect(accountsServer.loginWithService).toHaveBeenCalledWith(
       'oauth',
       { accessToken: 'token', accessTokenSecret: 'secret' },
-      { ip: null, userAgent: '' }
+      req.infos
     );
     expect(res.json).toHaveBeenCalledWith(user);
     expect(res.status).not.toHaveBeenCalled();
@@ -93,6 +97,10 @@ describe('providerCallback', () => {
         accessToken: 'token',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
@@ -102,7 +110,7 @@ describe('providerCallback', () => {
     expect(accountsServer.loginWithService).toHaveBeenCalledWith(
       'oauth',
       { accessToken: 'token' },
-      { ip: null, userAgent: '' }
+      req.infos
     );
     expect(res.status).toHaveBeenCalledWith(400);
     expect(res.json).toHaveBeenCalledWith(error);
diff --git a/packages/rest-express/__tests__/endpoints/password/reset.ts b/packages/rest-express/__tests__/endpoints/password/reset.ts
@@ -28,6 +28,10 @@ describe('resetPassword', () => {
           newPassword: 'new-password',
         },
         headers: {},
+        infos: {
+          ip: 'ipTest',
+          userAgent: 'userAgentTest',
+        },
       };
       const reqCopy = { ...req };
 
@@ -37,7 +41,7 @@ describe('resetPassword', () => {
       expect(accountsServer.getServices().password.resetPassword).toHaveBeenCalledWith(
         'token',
         'new-password',
-        { ip: null, userAgent: '' }
+        req.infos
       );
       expect(res.json).toHaveBeenCalledWith(null);
       expect(res.status).not.toHaveBeenCalled();
@@ -62,6 +66,10 @@ describe('resetPassword', () => {
           newPassword: 'new-password',
         },
         headers: {},
+        infos: {
+          ip: 'ipTest',
+          userAgent: 'userAgentTest',
+        },
       };
       const reqCopy = { ...req };
 
@@ -71,7 +79,7 @@ describe('resetPassword', () => {
       expect(accountsServer.getServices().password.resetPassword).toHaveBeenCalledWith(
         'token',
         'new-password',
-        { ip: null, userAgent: '' }
+        req.infos
       );
       expect(res.status).toHaveBeenCalledWith(400);
       expect(res.json).toHaveBeenCalledWith(error);
diff --git a/packages/rest-express/__tests__/endpoints/refresh-access-token.ts b/packages/rest-express/__tests__/endpoints/refresh-access-token.ts
@@ -27,15 +27,16 @@ describe('refreshAccessToken', () => {
         accessToken: 'token',
         refreshToken: 'refresh',
       },
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
-    expect(accountsServer.refreshTokens).toHaveBeenCalledWith('token', 'refresh', {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.refreshTokens).toHaveBeenCalledWith('token', 'refresh', req.infos);
     expect(req).toEqual(reqCopy);
     expect(res.json).toHaveBeenCalledWith(session);
     expect(res.status).not.toHaveBeenCalled();
@@ -55,16 +56,17 @@ describe('refreshAccessToken', () => {
         accessToken: 'token',
         refreshToken: 'refresh',
       },
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.refreshTokens).toHaveBeenCalledWith('token', 'refresh', {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.refreshTokens).toHaveBeenCalledWith('token', 'refresh', req.infos);
     expect(res.status).toHaveBeenCalledWith(400);
     expect(res.json).toHaveBeenCalledWith(error);
   });
diff --git a/packages/rest-express/__tests__/endpoints/service-authenticate.ts b/packages/rest-express/__tests__/endpoints/service-authenticate.ts
@@ -24,16 +24,17 @@ describe('serviceAuthenticate', () => {
         service: 'sms',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.loginWithService).toHaveBeenCalledWith('sms', undefined, {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.loginWithService).toHaveBeenCalledWith('sms', undefined, req.infos);
     expect(res.json).toHaveBeenCalledWith(user);
     expect(res.status).not.toHaveBeenCalled();
   });
@@ -51,16 +52,17 @@ describe('serviceAuthenticate', () => {
         service: 'sms',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.loginWithService).toHaveBeenCalledWith('sms', undefined, {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.loginWithService).toHaveBeenCalledWith('sms', undefined, req.infos);
     expect(res.status).toHaveBeenCalledWith(400);
     expect(res.json).toHaveBeenCalledWith(error);
   });
diff --git a/packages/rest-express/__tests__/endpoints/verify-authentication.ts b/packages/rest-express/__tests__/endpoints/verify-authentication.ts
@@ -21,16 +21,21 @@ describe('serviceVerifyAuthentication', () => {
         service: 'sms',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.authenticateWithService).toHaveBeenCalledWith('sms', undefined, {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.authenticateWithService).toHaveBeenCalledWith(
+      'sms',
+      undefined,
+      req.infos
+    );
     expect(res.json).toHaveBeenCalledWith(true);
     expect(res.status).not.toHaveBeenCalled();
   });
@@ -48,16 +53,21 @@ describe('serviceVerifyAuthentication', () => {
         service: 'sms',
       },
       headers: {},
+      infos: {
+        ip: 'ipTest',
+        userAgent: 'userAgentTest',
+      },
     };
     const reqCopy = { ...req };
 
     await middleware(req as any, res);
 
     expect(req).toEqual(reqCopy);
-    expect(accountsServer.authenticateWithService).toHaveBeenCalledWith('sms', undefined, {
-      ip: null,
-      userAgent: '',
-    });
+    expect(accountsServer.authenticateWithService).toHaveBeenCalledWith(
+      'sms',
+      undefined,
+      req.infos
+    );
     expect(res.status).toHaveBeenCalledWith(400);
     expect(res.json).toHaveBeenCalledWith(error);
   });
diff --git a/packages/rest-express/__tests__/express-middleware.ts b/packages/rest-express/__tests__/express-middleware.ts
@@ -3,6 +3,7 @@ import * as express from 'express';
 
 jest.mock('express', () => {
   const mockRouter = {
+    use: jest.fn(),
     post: jest.fn(),
     get: jest.fn(),
   };
diff --git a/packages/rest-express/src/custom.d.ts b/packages/rest-express/src/custom.d.ts
@@ -0,0 +1,10 @@
+declare namespace Express {
+  export interface Request {
+    userAgent: string;
+    ip: string;
+    infos: {
+      userAgent: string;
+      ip: string;
+    };
+  }
+}
diff --git a/packages/rest-express/src/endpoints/impersonate.ts b/packages/rest-express/src/endpoints/impersonate.ts
@@ -1,8 +1,6 @@
 import * as express from 'express';
-import * as requestIp from 'request-ip';
 import { AccountsServer } from '@accounts/server';
 import { ImpersonationUserIdentity } from '@accounts/types';
-import { getUserAgent } from '../utils/get-user-agent';
 import { sendError } from '../utils/send-error';
 
 export const impersonate = (accountsServer: AccountsServer) => async (
@@ -17,12 +15,7 @@ export const impersonate = (accountsServer: AccountsServer) => async (
       accessToken: string;
       impersonated: ImpersonationUserIdentity;
     } = req.body;
-    const userAgent = getUserAgent(req);
-    const ip = requestIp.getClientIp(req);
-    const impersonateRes = await accountsServer.impersonate(accessToken, impersonated, {
-      ip,
-      userAgent,
-    });
+    const impersonateRes = await accountsServer.impersonate(accessToken, impersonated, req.infos);
     res.json(impersonateRes);
   } catch (err) {
     sendError(res, err);
diff --git a/packages/rest-express/src/endpoints/oauth/provider-callback.ts b/packages/rest-express/src/endpoints/oauth/provider-callback.ts
@@ -1,7 +1,5 @@
 import * as express from 'express';
-import * as requestIp from 'request-ip';
 import { AccountsServer } from '@accounts/server';
-import { getUserAgent } from '../../utils/get-user-agent';
 import { sendError } from '../../utils/send-error';
 import { AccountsExpressOptions } from '../../types';
 
@@ -14,8 +12,6 @@ export const providerCallback = (
   options?: AccountsExpressOptions
 ) => async (req: express.Request, res: express.Response) => {
   try {
-    const userAgent = getUserAgent(req);
-    const ip = requestIp.getClientIp(req);
     const loggedInUser = await accountsServer.loginWithService(
       'oauth',
       {
@@ -24,7 +20,7 @@ export const providerCallback = (
         ...(req.body || {}),
         ...((req as RequestWithSession).session || {}),
       },
-      { ip, userAgent }
+      req.infos
     );
 
     if (options && options.onOAuthSuccess) {
diff --git a/packages/rest-express/src/endpoints/password/register.ts b/packages/rest-express/src/endpoints/password/register.ts
@@ -1,10 +1,8 @@
 import * as express from 'express';
-import * as requestIp from 'request-ip';
 import { AccountsServer, AccountsJsError } from '@accounts/server';
 import { AccountsPassword, CreateUserErrors } from '@accounts/password';
 import { CreateUserResult } from '@accounts/types';
 import { sendError } from '../../utils/send-error';
-import { getUserAgent } from '../../utils/get-user-agent';
 
 export const registerPassword = (accountsServer: AccountsServer) => async (
   req: express.Request,
@@ -44,17 +42,11 @@ export const registerPassword = (accountsServer: AccountsServer) => async (
     // When initializing AccountsServer we check that enableAutologin and ambiguousErrorMessages options
     // are not enabled at the same time
 
-    const userAgent = getUserAgent(req);
-    const ip = requestIp.getClientIp(req);
-
     const createdUser = await accountsServer.findUserById(userId);
 
     // If we are here - user must be created successfully
     // Explicitly saying this to Typescript compiler
-    const loginResult = await accountsServer.loginWithUser(createdUser!, {
-      ip,
-      userAgent,
-    });
+    const loginResult = await accountsServer.loginWithUser(createdUser!, req.infos);
 
     return res.json({
       userId,
diff --git a/packages/rest-express/src/endpoints/password/reset.ts b/packages/rest-express/src/endpoints/password/reset.ts
@@ -2,19 +2,15 @@ import * as express from 'express';
 import { AccountsServer } from '@accounts/server';
 import { AccountsPassword } from '@accounts/password';
 import { sendError } from '../../utils/send-error';
-import { getUserAgent } from '../../utils/get-user-agent';
-import { getClientIp } from 'request-ip';
 
 export const resetPassword = (accountsServer: AccountsServer) => async (
   req: express.Request,
   res: express.Response
 ) => {
   try {
     const { token, newPassword } = req.body;
-    const userAgent = getUserAgent(req);
-    const ip = getClientIp(req);
     const accountsPassword = accountsServer.getServices().password as AccountsPassword;
-    const loginResult = await accountsPassword.resetPassword(token, newPassword, { userAgent, ip });
+    const loginResult = await accountsPassword.resetPassword(token, newPassword, req.infos);
     res.json(loginResult);
   } catch (err) {
     sendError(res, err);
diff --git a/packages/rest-express/src/endpoints/refresh-access-token.ts b/packages/rest-express/src/endpoints/refresh-access-token.ts
diff --git a/packages/rest-express/src/endpoints/service-authenticate.ts b/packages/rest-express/src/endpoints/service-authenticate.ts
diff --git a/packages/rest-express/src/endpoints/verify-authentication.ts b/packages/rest-express/src/endpoints/verify-authentication.ts
diff --git a/packages/rest-express/src/express-middleware.ts b/packages/rest-express/src/express-middleware.ts
