Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: migrate firewall to allowedHosts option #3345

Merged
merged 8 commits into from
May 28, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions bin/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ module.exports = {
negative: true,
},
{
name: 'firewall',
name: 'allowed-hosts',
anshumanv marked this conversation as resolved.
Show resolved Hide resolved
type: [Boolean, String],
configs: [
{
Expand All @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion examples/cli/web-socket-url/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ module.exports = setup({
client: {
webSocketURL: 'ws://localhost:8080',
},
firewall: false,
allowedHosts: true,
},
});
6 changes: 3 additions & 3 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions lib/options.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
anshumanv marked this conversation as resolved.
Show resolved Hide resolved
"anyOf": [
{
"type": "boolean"
},
{
"type": "string"
},
{
"type": "array",
"items": {
Expand All @@ -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": [
Expand Down
9 changes: 6 additions & 3 deletions lib/utils/normalizeOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
33 changes: 17 additions & 16 deletions test/__snapshots__/validate-options.test.js.snap.webpack4
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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:
Expand Down
33 changes: 17 additions & 16 deletions test/__snapshots__/validate-options.test.js.snap.webpack5
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions test/cli/__snapshots__/cli.test.js.snap.webpack4
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions test/cli/__snapshots__/cli.test.js.snap.webpack5
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
43 changes: 43 additions & 0 deletions test/cli/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

anshumanv marked this conversation as resolved.
Show resolved Hide resolved
it('--no-static-serve-index', (done) => {
testBin('--no-static-serve-index')
.then((output) => {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/web-socket-server-and-url.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ for (const webSocketServerType of webSocketServerTypes) {
webSocketServer: webSocketServerType,
port: devServerPort,
host: devServerHost,
firewall: false,
allowedHosts: true,
hot: true,
};

Expand Down Expand Up @@ -132,7 +132,7 @@ for (const webSocketServerType of webSocketServerTypes) {
webSocketServer: webSocketServerType,
port: devServerPort,
host: devServerHost,
firewall: false,
allowedHosts: true,
hot: true,
};

Expand Down Expand Up @@ -204,7 +204,7 @@ for (const webSocketServerType of webSocketServerTypes) {
port: devServerPort,
host: devServerHost,
webSocketServer: webSocketServerType,
firewall: false,
allowedHosts: true,
hot: true,
static: true,
};
Expand Down
14 changes: 7 additions & 7 deletions test/server/Server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
anshumanv marked this conversation as resolved.
Show resolved Hide resolved
const options = {
client: {
webSocketURL: 'ws://test.host:80',
},
firewall: false,
allowedHosts: true,
};

const headers = {
Expand Down Expand Up @@ -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 };
Expand All @@ -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',
Expand Down
Loading