feat: extend CSRF token protection to all api routes (#9422)

* feat(security): extend CSRF token protection to all api routes

* update lock files

* revert axios usage

* fix unit tests

* fix more tests

* fix nits

* fix failing test

* rework axios mock

* cleanup

Author: OEvgeny

Composer/packages/client/__mocks__/@bfc/shared/lib/axios.ts

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+const axios = {
+  get: jest.fn().mockReturnValue(Promise.resolve(new Proxy({}, {
+    get() { return {} }
+  })))
+}
+
+module.exports = axios;
+module.exports.createAxios = jest.fn().mockReturnValue(axios)
+module.exports.axios = axios

Composer/packages/client/src/components/CreationFlow/CreateOptions.tsx

@@ -8,7 +8,7 @@ import formatMessage from 'format-message';
 import { BotTemplate } from '@bfc/shared';
 import { navigate, RouteComponentProps } from '@reach/router';
 import querystring from 'query-string';
-import axios from 'axios';
+import { axios } from '@bfc/shared/lib/axios';
 import { useRecoilValue } from 'recoil';
 
 import { getAliasFromPayload, isElectron } from '../../utils/electronUtil';

Composer/packages/client/src/components/ImportModal/ImportModal.tsx

@@ -8,7 +8,7 @@ import { navigate, RouteComponentProps } from '@reach/router';
 import { Dialog, DialogType } from '@fluentui/react/lib/Dialog';
 import { ExternalContentProviderType } from '@botframework-composer/types';
 import { useRecoilValue } from 'recoil';
-import axios from 'axios';
+import { axios } from '@bfc/shared/lib/axios';
 
 import { dispatcherState } from '../../recoilModel';
 import { createNotification } from '../../recoilModel/dispatchers/notification';

Composer/packages/client/src/components/WebChat/WebChatPanel.tsx

@@ -8,7 +8,7 @@ import {
   ConversationNetworkTrafficItem,
   ConversationNetworkErrorItem,
 } from '@botframework-composer/types';
-import { AxiosResponse } from 'axios';
+import type { AxiosResponse } from 'axios';
 import formatMessage from 'format-message';
 import { v4 as uuid } from 'uuid';
 import throttle from 'lodash/throttle';

Composer/packages/client/src/components/WebChat/utils/conversationService.ts

@@ -2,7 +2,8 @@
 // Licensed under the MIT License.
 
 import { v4 as uuidv4 } from 'uuid';
-import axios, { AxiosInstance, AxiosResponse } from 'axios';
+import type { AxiosInstance, AxiosResponse } from 'axios';
+import { axios, createAxios } from '@bfc/shared/lib/axios';
 import { createStore as createWebChatStore } from 'botframework-webchat-core';
 import { createDirectLine } from 'botframework-webchat';
 import moment from 'moment';
@@ -21,7 +22,7 @@ export class ConversationService {
 
   constructor(directlineHostUrl: string) {
     this.directlineHostUrl = directlineHostUrl.endsWith('/') ? directlineHostUrl.slice(0, -1) : directlineHostUrl;
-    this.composerApiClient = axios.create({
+    this.composerApiClient = createAxios({
       baseURL: directlineHostUrl,
     });
     this.setUpConversationServer();

Composer/packages/client/src/pages/publish/pullDialog.tsx

@@ -7,7 +7,7 @@ import { PublishTarget } from '@botframework-composer/types';
 import formatMessage from 'format-message';
 import React, { useCallback, useEffect, useState } from 'react';
 import { useRecoilValue } from 'recoil';
-import axios from 'axios';
+import { axios } from '@bfc/shared/lib/axios';
 
 import { createNotification } from '../../recoilModel/dispatchers/notification';
 import { ImportSuccessNotificationWrapper } from '../../components/ImportModal/ImportSuccessNotification';

Composer/packages/client/src/types/window.d.ts

@@ -40,6 +40,6 @@ declare global {
     /**
      * Token generated by the server, and sent with certain auth-related requests to the server to be verified and prevent CSRF attacks.
      */
-    __csrf__?: string;
+    __csrf__: string;
   }
 }

Composer/packages/client/src/utils/authClient.ts

@@ -22,20 +22,20 @@ import { isElectron } from './electronUtil';
 import storage from './storage';
 
 let idToken = getTokenFromCache('idToken');
+// eslint-disable-next-line no-underscore-dangle
+const fetchHeaders = { 'X-CSRF-Token': window.__csrf__ };
 
 async function getAccessToken(options: AuthParameters): Promise<string> {
   const { targetResource = '', scopes = [] } = options;
   try {
     if (isElectron()) {
-      const { __csrf__ = '' } = window;
-
       let url = '/api/auth/getAccessToken?';
       const params = new URLSearchParams();
       if (targetResource) {
         params.append('targetResource', targetResource);
       }
       url += params.toString();
-      const result = await fetch(url, { method: 'GET', headers: { 'X-CSRF-Token': __csrf__ } });
+      const result = await fetch(url, { method: 'GET', headers: fetchHeaders });
       const { accessToken = '' } = await result.json();
       return accessToken;
     } else if (authConfig.clientId && authConfig.redirectUrl && authConfig.tenantId) {
@@ -98,7 +98,7 @@ async function logOut() {
   cleanTokenFromCache('graphToken');
   if (isElectron()) {
     const url = '/api/auth/logOut';
-    const result = await fetch(url, { method: 'GET' });
+    const result = await fetch(url, { method: 'GET', headers: fetchHeaders });
     return result.ok;
   } else if (authConfig.clientId) {
     // clean token cache in storage
@@ -118,12 +118,8 @@ async function logOut() {
 async function getARMTokenForTenant(tenantId: string): Promise<string> {
   const options = {
     method: 'GET',
-    headers: {},
+    headers: fetchHeaders,
   };
-  if (isElectron()) {
-    const { __csrf__ = '' } = window;
-    options.headers['X-CSRF-Token'] = __csrf__;
-  }
   // do we have a valid token in the cache for this tenant?
   if (getTokenFromCache(`token-${tenantId}`)) {
     return getTokenFromCache(`token-${tenantId}`);
@@ -150,12 +146,8 @@ async function getARMTokenForTenant(tenantId: string): Promise<string> {
 async function getTenants(): Promise<AzureTenant[]> {
   const options = {
     method: 'GET',
-    headers: {},
+    headers: fetchHeaders,
   };
-  if (isElectron()) {
-    const { __csrf__ = '' } = window;
-    options.headers['X-CSRF-Token'] = __csrf__;
-  }
 
   const result = await fetch('/api/auth/getTenants', options);
   const { tenants = [] } = await result.json();
@@ -164,8 +156,7 @@ async function getTenants(): Promise<AzureTenant[]> {
 
 async function getAccount() {
   if (isElectron()) {
-    const { __csrf__ = '' } = window;
-    const result = await fetch('/api/auth/getAccount', { method: 'GET', headers: { 'X-CSRF-Token': __csrf__ } });
+    const result = await fetch('/api/auth/getAccount', { method: 'GET', headers: fetchHeaders });
     const { account = {} } = await result.json();
     return account;
   }

Composer/packages/client/src/utils/httpUtil.ts

@@ -1,11 +1,11 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import axios from 'axios';
+import { createAxios } from '@bfc/shared/lib/axios';
 
 import { BASEURL } from '../constants';
 
-const httpClient = axios.create({
+const httpClient = createAxios({
   baseURL: BASEURL,
 });
 

Composer/packages/extension-client/src/common/index.ts

@@ -8,12 +8,15 @@ export function render(component: React.ReactElement) {
   window[ComposerGlobalName].render(component);
 }
 
+const fetchHeaders = { 'X-CSRF-Token': window['__csrf__'] }
+
 /** Allows extension client bundles to make AJAX calls from the server -- avoiding the issue of CORS */
 function fetchProxy(url: string, options: RequestInit) {
   return fetch(`/api/extensions/proxy/${encodeURIComponent(url)}`, {
     method: 'POST',
     body: JSON.stringify(options),
     headers: {
+      ...fetchHeaders,
       'Content-Type': 'application/json',
     },
   });

Composer/packages/lib/shared/package.json

@@ -20,8 +20,10 @@
     "watch": "yarn build:ts --watch"
   },
   "peerDependencies": {
+    "axios": "^0.21.1",
     "react": "16.13.1",
-    "react-dom": "16.13.1"
+    "react-dom": "16.13.1",
+    "tslib": "2.4.0"
   },
   "devDependencies": {
     "@botframework-composer/test-utils": "*",
@@ -42,7 +44,6 @@
     "json-schema": "0.4.0",
     "multimatch": "^5.0.0",
     "nanoid": "^3.1.3",
-    "nanoid-dictionary": "^3.0.0",
-    "tslib": "2.4.0"
+    "nanoid-dictionary": "^3.0.0"
   }
 }

Composer/packages/lib/shared/src/axios.ts

@@ -0,0 +1,24 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import Axios, { AxiosInstance, AxiosRequestConfig } from 'axios';
+
+export * from 'axios';
+
+const csrfInterceptor = (config: AxiosRequestConfig) => {
+  if (config.baseURL?.startsWith('/api') || config.url?.startsWith('/api')) {
+    // eslint-disable-next-line no-underscore-dangle
+    config.headers['X-CSRF-Token'] = ((window as unknown) as { __csrf__: string }).__csrf__;
+  }
+  return config;
+};
+
+export const addCSRFInterceptor = (instance: AxiosInstance) => instance?.interceptors.request.use(csrfInterceptor);
+
+export const createAxios: typeof Axios.create = (...args) => {
+  const axiosInstance = Axios.create(...args);
+  addCSRFInterceptor(axiosInstance);
+  return axiosInstance;
+};
+
+export const axios = createAxios();

Composer/packages/server/src/middleware/csrfProtection.ts

@@ -9,7 +9,6 @@ import { authService } from '../services/auth/auth';
  * Middleware that verifies if the server-generated CSRF token is sent with the incoming request via header.
  */
 export const csrfProtection = (req: Request, res: Response, next?: NextFunction) => {
-  // the CSRF token will only be generated in the production environment
   if (authService.csrfToken) {
     const csrfToken = req.get('X-CSRF-Token');
     if (!csrfToken) {

Composer/packages/server/src/router/api.ts

@@ -25,6 +25,14 @@ import { UtilitiesController } from './../controllers/utilities';
 
 const router: Router = express.Router({});
 
+// Routes bellow are NOT CSRF protected
+// Place only routes loaded by browser script tags and navigation
+router.get('/extensions/settings/schema.json', ExtensionsController.getSettingsSchema);
+router.get('/extensions/:id/:bundleId', ExtensionsController.getBundleForView);
+
+router.use(csrfProtection);
+
+// Routes bellow are CSRF protected
 router.post('/projects', ProjectController.createProject);
 router.post('/projects/migrate', ProjectController.migrateProject);
 router.get('/projects', ProjectController.getAllProjects);
@@ -111,19 +119,17 @@ router.post('/extensions', ExtensionsController.addExtension);
 router.delete('/extensions', ExtensionsController.removeExtension);
 router.patch('/extensions/toggle', ExtensionsController.toggleExtension);
 router.get('/extensions/search', ExtensionsController.searchExtensions);
-router.get('/extensions/settings/schema.json', ExtensionsController.getSettingsSchema);
 router.get('/extensions/settings', ExtensionsController.getSettings);
 router.patch('/extensions/settings', ExtensionsController.updateSettings);
-router.get('/extensions/:id/:bundleId', ExtensionsController.getBundleForView);
 // proxy route for extensions (allows extension client code to make fetch calls using the Composer server as a proxy -- avoids browser blocking request due to CORS)
 router.post('/extensions/proxy/:url', ExtensionsController.performExtensionFetch);
 
 // authentication from client
-router.get('/auth/getAccessToken', csrfProtection, AuthController.getAccessToken);
+router.get('/auth/getAccessToken', AuthController.getAccessToken);
 router.get('/auth/logOut', AuthController.logOut);
-router.get('/auth/getTenants', csrfProtection, AuthController.getTenants);
-router.get('/auth/getAccount', csrfProtection, AuthController.getAccount);
-router.get('/auth/getARMTokenForTenant', csrfProtection, AuthController.getARMTokenForTenant);
+router.get('/auth/getTenants', AuthController.getTenants);
+router.get('/auth/getAccount', AuthController.getAccount);
+router.get('/auth/getARMTokenForTenant', AuthController.getARMTokenForTenant);
 
 // FeatureFlags
 router.get('/featureFlags', FeatureFlagController.getFeatureFlags);

Composer/packages/server/src/services/__tests__/auth.test.ts

@@ -21,13 +21,13 @@ describe('auth service', () => {
     expect((authService as any)._csrfToken).toBeTruthy();
   });
 
-  it('should not generate a CSRF token in the development environment', () => {
+  it('should use template as a CSRF token in the development environment', () => {
     Object.assign(process.env, { ...process.env, NODE_ENV: 'development' });
     // eslint-disable-next-line @typescript-eslint/no-var-requires
     const { authService } = require('../auth/auth');
 
     // eslint-disable-next-line no-underscore-dangle
-    expect((authService as any)._csrfToken).not.toBeTruthy();
+    expect((authService as any)._csrfToken).toEqual('<?= __csrf__ ?>');
   });
 
   it('should get an access token', async () => {

Composer/packages/server/src/services/auth/auth.ts

@@ -32,7 +32,8 @@ class AuthService {
       log('Production environment detected. Generating CSRF token.');
       this._csrfToken = uuid();
     } else {
-      this._csrfToken = '';
+      // use a placeholder as a token in development environment
+      this._csrfToken = '<?= __csrf__ ?>';
     }
   }
 

Composer/packages/types/src/shell.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
-import { AxiosInstance } from 'axios';
+import type { AxiosInstance } from 'axios';
 
 import { IDiagnostic } from './diagnostic';
 import type {

Composer/yarn-berry.lock

@@ -4253,10 +4253,11 @@ __metadata:
     react: 16.13.1
     react-dom: 16.13.1
     rimraf: 3.0.2
-    tslib: 2.4.0
   peerDependencies:
+    axios: ^0.21.1
     react: 16.13.1
     react-dom: 16.13.1
+    tslib: 2.4.0
   languageName: unknown
   linkType: soft
 

extensions/azurePublish/src/components/api.tsx

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 /* eslint-disable no-underscore-dangle */
-import axios from 'axios';
+import { axios } from '@bfc/shared/src/axios';
 import formatMessage from 'format-message';
 import { SubscriptionClient } from '@azure/arm-subscriptions';
 import { Subscription } from '@azure/arm-subscriptions/esm/models';

extensions/azurePublish/src/node/luisAndQnA.ts

@@ -4,7 +4,6 @@
 import * as path from 'path';
 
 import * as fs from 'fs-extra';
-import { isUsingAdaptiveRuntime } from '@bfc/shared';
 import { ILuisConfig, FileInfo, IBotProject, RuntimeTemplate, DialogSetting } from '@botframework-composer/types';
 import axios, { AxiosRequestConfig } from 'axios';
 

extensions/azurePublish/yarn-berry.lock

@@ -1908,7 +1908,7 @@ __metadata:
 
 "@bfc/extension-client@file:../../Composer/packages/extension-client::locator=azurePublish%40workspace%3A.":
   version: 1.0.0
-  resolution: "@bfc/extension-client@file:../../Composer/packages/extension-client#../../Composer/packages/extension-client::hash=1ccdd5&locator=azurePublish%40workspace%3A."
+  resolution: "@bfc/extension-client@file:../../Composer/packages/extension-client#../../Composer/packages/extension-client::hash=8dd52b&locator=azurePublish%40workspace%3A."
   dependencies:
     debug: ^4.1.1
     lodash: ^4.17.19
@@ -1917,7 +1917,7 @@ __metadata:
     "@botframework-composer/types": "*"
     react: 16.13.1
     react-dom: 16.13.1
-  checksum: e871e220343b04a641b46e94b9419281156c2f2920558dc345232eae130c0757bc386f4a567db50b9e77c4b1dc8ef393572277952859ca599319a697c77abff4
+  checksum: e915e895f8bddee9e70e18f12ba5ab4668bc6883ab3c9dd00833f3d7fb9be0e8cb0dbc35f686f4baf5baf31390f8e6bd037274219c68cea718b0d3f72d9ecc8a
   languageName: node
   linkType: hard
 
@@ -1938,7 +1938,7 @@ __metadata:
 
 "@bfc/shared@file:../../Composer/packages/lib/shared::locator=azurePublish%40workspace%3A.":
   version: 0.0.0
-  resolution: "@bfc/shared@file:../../Composer/packages/lib/shared#../../Composer/packages/lib/shared::hash=b1189d&locator=azurePublish%40workspace%3A."
+  resolution: "@bfc/shared@file:../../Composer/packages/lib/shared#../../Composer/packages/lib/shared::hash=8bb646&locator=azurePublish%40workspace%3A."
   dependencies:
     "@botframework-composer/types": "*"
     format-message: 6.2.4
@@ -1947,11 +1947,12 @@ __metadata:
     multimatch: ^5.0.0
     nanoid: ^3.1.3
     nanoid-dictionary: ^3.0.0
-    tslib: 2.4.0
   peerDependencies:
+    axios: ^0.21.1
     react: 16.13.1
     react-dom: 16.13.1
-  checksum: d6d584b1231582c660585be243a577ecc112e3834ce711241ec981d354bbd3efc4084d1a556c3bf6aa155f2506101e1f33f7fb3eddb154657eac688ba6c167f8
+    tslib: 2.4.0
+  checksum: ed6af3f1b57b302c951d934f2686f139541e99eb23765906e96f3d4b024b059f0e17f1af869939a6bf8170b366b3d0034a8fa4855395ad545f430a76e377b545
   languageName: node
   linkType: hard
 
@@ -1996,7 +1997,7 @@ __metadata:
 
 "@botframework-composer/types@file:../../Composer/packages/types::locator=azurePublish%40workspace%3A.":
   version: 0.0.2
-  resolution: "@botframework-composer/types@file:../../Composer/packages/types#../../Composer/packages/types::hash=33ce41&locator=azurePublish%40workspace%3A."
+  resolution: "@botframework-composer/types@file:../../Composer/packages/types#../../Composer/packages/types::hash=ff5977&locator=azurePublish%40workspace%3A."
   dependencies:
     "@types/express": 4.16.1
     "@types/passport": ^1.0.4
@@ -2005,7 +2006,7 @@ __metadata:
     express-serve-static-core: 0.1.1
     json-schema: 0.4.0
     tslib: 2.4.0
-  checksum: ca240110a807681973dcb661819b8979d4b05ad08763a171c17a10ba25aad32e3f0ec5125cb6b2462d0a6e1afa5b13b2c4dc336c1529a15ea4ac1c47654e0e31
+  checksum: 377541c2d09305b36e9e12a1b200725bf7c37580a959f2d24007f51052b5f9536b7e0dea48f1f7b574fcca0b8858c97658f413c2df50238f9e70cdcbeaf678e5
   languageName: node
   linkType: hard