fix: migrate firewall to allowedHosts option

anshumanv · anshumanv · commit 1dde16ceadda · 2021-05-26T16:53:18.000+05:30
diff --git a/bin/cli-flags.js b/bin/cli-flags.js
@@ -395,7 +395,7 @@ module.exports = {
       negative: true,
     },
     {
-      name: 'firewall',
+      name: 'allowed-hosts',
       type: [Boolean, String],
       configs: [
         {
@@ -405,9 +405,8 @@ module.exports = {
           type: 'string',
         },
       ],
-      description:
-        'Enable firewall or set hosts that are allowed to access the dev server.',
-      negatedDescription: 'Disable firewall.',
+      description: 'Set hosts that are allowed to access the dev server.',
+      negatedDescription: 'Allow any host to access dev server.',
       multiple: true,
       negative: true,
     },
diff --git a/examples/cli/web-socket-url/webpack.config.js b/examples/cli/web-socket-url/webpack.config.js
@@ -12,6 +12,6 @@ module.exports = setup({
     client: {
       webSocketURL: 'ws://localhost:8080',
     },
-    firewall: false,
+    allowedHosts: true,
   },
 });
diff --git a/lib/Server.js b/lib/Server.js
@@ -899,8 +899,8 @@ class Server {
 
   checkHeaders(headers, headerToCheck) {
     // allow user to opt out of this security check, at their own risk
-    // by explicitly disabling firewall
-    if (!this.options.firewall) {
+    // by explicitly enabling allowedHosts
+    if (this.options.allowedHosts) {
       return true;
     }
 
@@ -942,7 +942,7 @@ class Server {
       return true;
     }
 
-    const allowedHosts = this.options.firewall;
+    const allowedHosts = this.options.allowedHosts;
 
     // always allow localhost host, for convenience
     // allow if hostname is in allowedHosts
diff --git a/lib/options.json b/lib/options.json
@@ -337,11 +337,14 @@
       "type": "object",
       "description": "Provide options to webpack-dev-middleware which handles webpack assets. https://webpack.js.org/configuration/dev-server/#devserverdevmiddleware"
     },
-    "firewall": {
+    "allowedHosts": {
       "anyOf": [
         {
           "type": "boolean"
         },
+        {
+          "type": "string"
+        },
         {
           "type": "array",
           "items": {
@@ -350,7 +353,7 @@
           "minItems": 1
         }
       ],
-      "description": "Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverfirewall"
+      "description": "Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverallowedhosts"
     },
     "headers": {
       "anyOf": [
diff --git a/lib/utils/normalizeOptions.js b/lib/utils/normalizeOptions.js
@@ -128,9 +128,12 @@ function normalizeOptions(compiler, options, logger) {
 
   options.devMiddleware = options.devMiddleware || {};
 
-  if (typeof options.firewall === 'undefined') {
-    // firewall is enabled by default
-    options.firewall = true;
+  if (typeof options.allowedHosts === 'undefined') {
+    // allowedHosts is disabled by default
+    options.allowedHosts = false;
+  } else if (typeof options.allowedHosts === 'string') {
+    // we store allowedHosts as array when supplied as string
+    options.allowedHosts = [options.allowedHosts];
   }
 
   if (typeof options.setupExitSignals === 'undefined') {
diff --git a/test/__snapshots__/validate-options.test.js.snap.webpack4 b/test/__snapshots__/validate-options.test.js.snap.webpack4
@@ -1,5 +1,22 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`options validate should throw an error on the "allowedHosts" option with '[]' value 1`] = `
+"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
+ - configuration.allowedHosts should be an non-empty array."
+`;
+
+exports[`options validate should throw an error on the "allowedHosts" option with '123' value 1`] = `
+"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
+ - configuration.allowedHosts should be one of these:
+   boolean | string | [string, ...] (should not have fewer than 1 item)
+   -> Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverallowedhosts
+   Details:
+    * configuration.allowedHosts should be a boolean.
+    * configuration.allowedHosts should be a string.
+    * configuration.allowedHosts should be an array:
+      [string, ...] (should not have fewer than 1 item)"
+`;
+
 exports[`options validate should throw an error on the "bonjour" option with '' value 1`] = `
 "ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
  - configuration.bonjour should be one of these:
@@ -141,22 +158,6 @@ exports[`options validate should throw an error on the "devMiddleware" option wi
    -> Provide options to webpack-dev-middleware which handles webpack assets. https://webpack.js.org/configuration/dev-server/#devserverdevmiddleware"
 `;
 
-exports[`options validate should throw an error on the "firewall" option with '' value 1`] = `
-"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
- - configuration.firewall should be one of these:
-   boolean | [string, ...] (should not have fewer than 1 item)
-   -> Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverfirewall
-   Details:
-    * configuration.firewall should be a boolean.
-    * configuration.firewall should be an array:
-      [string, ...] (should not have fewer than 1 item)"
-`;
-
-exports[`options validate should throw an error on the "firewall" option with '[]' value 1`] = `
-"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
- - configuration.firewall should be an non-empty array."
-`;
-
 exports[`options validate should throw an error on the "headers" option with '1' value 1`] = `
 "ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
  - configuration.headers should be one of these:
diff --git a/test/__snapshots__/validate-options.test.js.snap.webpack5 b/test/__snapshots__/validate-options.test.js.snap.webpack5
@@ -1,5 +1,22 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`options validate should throw an error on the "allowedHosts" option with '[]' value 1`] = `
+"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
+ - configuration.allowedHosts should be an non-empty array."
+`;
+
+exports[`options validate should throw an error on the "allowedHosts" option with '123' value 1`] = `
+"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
+ - configuration.allowedHosts should be one of these:
+   boolean | string | [string, ...] (should not have fewer than 1 item)
+   -> Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverallowedhosts
+   Details:
+    * configuration.allowedHosts should be a boolean.
+    * configuration.allowedHosts should be a string.
+    * configuration.allowedHosts should be an array:
+      [string, ...] (should not have fewer than 1 item)"
+`;
+
 exports[`options validate should throw an error on the "bonjour" option with '' value 1`] = `
 "ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
  - configuration.bonjour should be one of these:
@@ -141,22 +158,6 @@ exports[`options validate should throw an error on the "devMiddleware" option wi
    -> Provide options to webpack-dev-middleware which handles webpack assets. https://webpack.js.org/configuration/dev-server/#devserverdevmiddleware"
 `;
 
-exports[`options validate should throw an error on the "firewall" option with '' value 1`] = `
-"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
- - configuration.firewall should be one of these:
-   boolean | [string, ...] (should not have fewer than 1 item)
-   -> Defines routes which are enabled by default, on by default and allows localhost. https://webpack.js.org/configuration/dev-server/#devserverfirewall
-   Details:
-    * configuration.firewall should be a boolean.
-    * configuration.firewall should be an array:
-      [string, ...] (should not have fewer than 1 item)"
-`;
-
-exports[`options validate should throw an error on the "firewall" option with '[]' value 1`] = `
-"ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
- - configuration.firewall should be an non-empty array."
-`;
-
 exports[`options validate should throw an error on the "headers" option with '1' value 1`] = `
 "ValidationError: Invalid configuration object. Object has been initialized using a configuration object that does not match the API schema.
  - configuration.headers should be one of these:
diff --git a/test/cli/__snapshots__/cli.test.js.snap.webpack4 b/test/cli/__snapshots__/cli.test.js.snap.webpack4
@@ -271,9 +271,9 @@ Options:
                                    Page Applications.
   --compress                       Enable gzip compression.
   --no-compress                    Disable gzip compression.
-  --firewall [value...]            Enable firewall or set hosts that are
-                                   allowed to access the dev server.
-  --no-firewall                    Disable firewall.
+  --allowed-hosts [value...]       Set hosts that are allowed to access the dev
+                                   server.
+  --no-allowed-hosts               Allow any host to access dev server.
   --watch-files <value...>         Watch static files for file changes.
 
 Global options:
diff --git a/test/cli/__snapshots__/cli.test.js.snap.webpack5 b/test/cli/__snapshots__/cli.test.js.snap.webpack5
@@ -272,9 +272,9 @@ Options:
                                    Page Applications.
   --compress                       Enable gzip compression.
   --no-compress                    Disable gzip compression.
-  --firewall [value...]            Enable firewall or set hosts that are
-                                   allowed to access the dev server.
-  --no-firewall                    Disable firewall.
+  --allowed-hosts [value...]       Set hosts that are allowed to access the dev
+                                   server.
+  --no-allowed-hosts               Allow any host to access dev server.
   --watch-files <value...>         Watch static files for file changes.
 
 Global options:
diff --git a/test/cli/cli.test.js b/test/cli/cli.test.js
@@ -654,6 +654,49 @@ describe('CLI', () => {
       .catch(done);
   });
 
+  describe('allowed-hosts', () => {
+    it('--allowed-hosts', (done) => {
+      testBin('--allowed-hosts')
+        .then((output) => {
+          expect(output.exitCode).toEqual(0);
+          done();
+        })
+        .catch(done);
+    });
+
+    it('--no-allowed-hosts', (done) => {
+      testBin('--no-allowed-hosts')
+        .then((output) => {
+          expect(output.exitCode).toEqual(0);
+          done();
+        })
+        .catch(done);
+    });
+
+    it('--allowed-hosts string', (done) => {
+      testBin('--allowed-hosts', 'testhost.com')
+        .then((output) => {
+          expect(output.exitCode).toEqual(0);
+          done();
+        })
+        .catch(done);
+    });
+
+    it('--allowed-hosts multiple', (done) => {
+      testBin(
+        '--allowed-hosts',
+        'testhost.com',
+        '--allowed-hosts',
+        'testhost1.com'
+      )
+        .then((output) => {
+          expect(output.exitCode).toEqual(0);
+          done();
+        })
+        .catch(done);
+    });
+  });
+
   it('--no-static-serve-index', (done) => {
     testBin('--no-static-serve-index')
       .then((output) => {
diff --git a/test/e2e/web-socket-server-and-url.test.js b/test/e2e/web-socket-server-and-url.test.js
@@ -65,7 +65,7 @@ for (const webSocketServerType of webSocketServerTypes) {
         webSocketServer: webSocketServerType,
         port: devServerPort,
         host: devServerHost,
-        firewall: false,
+        allowedHosts: true,
         hot: true,
       };
 
@@ -132,7 +132,7 @@ for (const webSocketServerType of webSocketServerTypes) {
         webSocketServer: webSocketServerType,
         port: devServerPort,
         host: devServerHost,
-        firewall: false,
+        allowedHosts: true,
         hot: true,
       };
 
@@ -204,7 +204,7 @@ for (const webSocketServerType of webSocketServerTypes) {
         port: devServerPort,
         host: devServerHost,
         webSocketServer: webSocketServerType,
-        firewall: false,
+        allowedHosts: true,
         hot: true,
         static: true,
       };
diff --git a/test/server/Server.test.js b/test/server/Server.test.js
@@ -301,12 +301,12 @@ describe('Server', () => {
       });
     });
 
-    it('should always allow any host if options.firewall is disabled', () => {
+    it('should always allow any host if options.allowedHosts is enabled', () => {
       const options = {
         client: {
           webSocketURL: 'ws://test.host:80',
         },
-        firewall: false,
+        allowedHosts: true,
       };
 
       const headers = {
@@ -430,10 +430,10 @@ describe('Server', () => {
       }
     });
 
-    describe('firewall', () => {
-      it('should allow hosts in firewall', () => {
+    describe('allowedHosts', () => {
+      it('should allow hosts in allowedHosts', () => {
         const tests = ['test.host', 'test2.host', 'test3.host'];
-        const options = { firewall: tests };
+        const options = { allowedHosts: tests };
         server = createServer(compiler, options);
         tests.forEach((test) => {
           const headers = { host: test };
@@ -443,8 +443,8 @@ describe('Server', () => {
         });
       });
 
-      it('should allow hosts that pass a wildcard in firewall', () => {
-        const options = { firewall: ['.example.com'] };
+      it('should allow hosts that pass a wildcard in allowedHosts', () => {
+        const options = { allowedHosts: ['.example.com'] };
         server = createServer(compiler, options);
         const tests = [
           'www.example.com',
diff --git a/test/server/utils/__snapshots__/normalizeOptions.test.js.snap.webpack4 b/test/server/utils/__snapshots__/normalizeOptions.test.js.snap.webpack4
diff --git a/test/server/utils/__snapshots__/normalizeOptions.test.js.snap.webpack5 b/test/server/utils/__snapshots__/normalizeOptions.test.js.snap.webpack5
diff --git a/test/server/utils/normalizeOptions.test.js b/test/server/utils/normalizeOptions.test.js
diff --git a/test/validate-options.test.js b/test/validate-options.test.js
