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

Issue 442 - blacklisting hosts with config.json #1062

Merged
merged 6 commits into from
Dec 15, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions packages/server/lib/config.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ defaults = {
isTextTerminal: false
reporter: "spec"
reporterOptions: null
blacklistHosts: null
clientRoute: "/__/"
xhrRoute: "/xhrs/"
socketIoRoute: "/__socket.io"
Expand Down Expand Up @@ -103,6 +104,7 @@ defaults = {
validationRules = {
animationDistanceThreshold: v.isNumber
baseUrl: v.isFullyQualifiedUrl
blacklistHosts: v.isStringOrArrayOfStrings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string or array of strings here

chromeWebSecurity: v.isBoolean
defaultCommandTimeout: v.isNumber
env: v.isPlainObject
Expand Down
11 changes: 11 additions & 0 deletions packages/server/lib/controllers/proxy.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ logger = require("../logger")
cors = require("../util/cors")
buffers = require("../util/buffers")
rewriter = require("../util/rewriter")
blacklist = require("../util/blacklist")
networkFailures = require("../util/network_failures")

redirectRe = /^30(1|2|3|7|8)$/
Expand Down Expand Up @@ -55,6 +56,16 @@ module.exports = {
## root path that serves the app
return res.redirect(config.clientRoute)

## if we have black listed hosts
if blh = config.blacklistHosts
## and url matches any of our blacklisted hosts
if matched = blacklist.matches(req.proxiedUrl, blh)
## then bail and return with 503
## and set a custom header
res.set("x-cypress-matched-blacklisted-host", matched)

return res.status(503).end()

thr = through (d) -> @queue(d)

@getHttpContent(thr, req, res, remoteState, config, request)
Expand Down
12 changes: 11 additions & 1 deletion packages/server/lib/server.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ origin = require("./util/origin")
connect = require("./util/connect")
appData = require("./util/app_data")
buffers = require("./util/buffers")
blacklist = require("./util/blacklist")
statusCode = require("./util/status_code")
headersUtil = require("./util/headers")
allowDestroy = require("./util/server_destroy")
Expand Down Expand Up @@ -133,7 +134,7 @@ class Server

createServer: (app, config, request) ->
new Promise (resolve, reject) =>
{port, fileServerFolder, socketIoRoute, baseUrl} = config
{port, fileServerFolder, socketIoRoute, baseUrl, blacklistHosts} = config

@_server = http.createServer(app)
@_wsProxy = httpProxy.createProxyServer()
Expand Down Expand Up @@ -175,6 +176,15 @@ class Server

log("HTTPS request #{word} match URL: #{urlToCheck} with props: %o", @_remoteProps)

## if we have blacklisted hosts lets
## see if this matches - if so then
## we cannot allow it to make a direct
## connection
if blacklistHosts
isMatching = blacklist.matches(urlToCheck, blacklistHosts)

log("HTTPS request #{urlToCheck} matches blacklist?", isMatching)

## make a direct connection only if
## our req url does not match the origin policy
## which is the superDomain + port
Expand Down
37 changes: 37 additions & 0 deletions packages/server/lib/util/blacklist.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
_ = require("lodash")
url = require("url")
minimatch = require("minimatch")

DEFAULT_PORTS = ["443", "80"]

stripProtocolAndDefaultPorts = (urlToCheck) ->
## grab host which is 'hostname:port' only
{ host, hostname, port } = url.parse(urlToCheck)

## if we have a default port for 80 or 443
## then just return the hostname
if port in DEFAULT_PORTS
return hostname

## else return the host
return host

matches = (urlToCheck, blacklistHosts) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would check blackListHosts type here - if type is not string or array of strings, something went wrong. Throw

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, what does this return? needs a jsdoc block I feel, because this is so reusable

## normalize into flat array
blacklistHosts = [].concat(blacklistHosts)

urlToCheck = stripProtocolAndDefaultPorts(urlToCheck)

matchUrl = (hostMatcher) ->
## use minimatch against the url
## to see if any match
minimatch(urlToCheck, hostMatcher)

_.find(blacklistHosts, matchUrl)


module.exports = {
matches

stripProtocolAndDefaultPorts
}
84 changes: 84 additions & 0 deletions packages/server/test/integration/http_requests_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2610,6 +2610,90 @@ describe "Routes", ->

expect(res.body).to.include("Cypress errored attempting to make an http request to this url")

context "blacklisted hosts", ->
beforeEach ->
@setup({
config: {
blacklistHosts: [
"*.google.com"
"shop.apple.com"
"cypress.io"
"localhost:6666"
"*adwords.com"
]
}
})

it "returns 503 and custom headers for all hosts", ->
expectedHeader = (res, val) ->
expect(res.headers["x-cypress-matched-blacklisted-host"]).to.eq(val)

Promise.all([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can also use https://github.com/bahmutov/snap-shot-it#data-driven-testing to make the test simpler here

@rp("https://mail.google.com/f")
@rp("http://shop.apple.com/o")
@rp("https://cypress.io")
@rp("https://localhost:6666/o")
@rp("https://some.adwords.com")
])
.spread (responses...) ->
_.every responses, (res) ->
expect(res.statusCode).to.eq(503)
expect(res.body).to.be.empty

expectedHeader(responses[0], "*.google.com")
expectedHeader(responses[1], "shop.apple.com")
expectedHeader(responses[2], "cypress.io")
expectedHeader(responses[3], "localhost:6666")
expectedHeader(responses[4], "*adwords.com")

context "extra http request headers", ->
beforeEach ->
@setup({
initialUrl: "http://localhost:3333/"
config: {
extraRequestHeaders: {
"x-foo-bar": "baz"
"some": "override"
}
}
})

it "sets and overrides extra request headers", ->
nock("http://localhost:3333")
.matchHeader("x-foo-bar", "baz")
.matchHeader("some", "override")
.matchHeader("another", "one")
.get("/headers")
.reply(200)

@rp({
url: "http://localhost:3333/headers"
headers: {
some: "thing"
another: "one"
}
})
.then (res) ->
expect(res.statusCode).to.eq(200)

it "does not set extra request headers for domains not matching", ->
# nock("http://localhost:3322")
# .matchHeader("some", "thing")
# .matchHeader("another", "one")
# .get("/headers")
# .reply(200)

@rp({
# url: "https://mail.google.com/"
url: "https://localhost:3322/headers"
headers: {
some: "thing"
another: "one"
}
})
.then (res) ->
expect(res.statusCode).to.eq(200)

context "POST *", ->
beforeEach ->
@setup("http://localhost:8000")
Expand Down
1 change: 1 addition & 0 deletions packages/server/test/scripts/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ if (env.VERBOSE === '1') {
'nock.*',
'-nock.common',
'-nock.scope',
'-nock.interceptor',
'socket.io:*',
'xvfb-maybe',
])
Expand Down
44 changes: 44 additions & 0 deletions packages/server/test/unit/blacklist_spec.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
require("../spec_helper")

blacklist = require("#{root}lib/util/blacklist")

hosts = [
"*.google.com"
"shop.apple.com"
"localhost:6666"
"adwords.com"
"*yahoo.com"
]

matchesStr = (url, host, val) ->
m = blacklist.matches(url, host)
expect(!!m).to.eq(val, "url: '#{url}' did not pass")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh, the !!m notation


matchesArray = (url, val) ->
m = blacklist.matches(url, hosts)
expect(!!m).to.eq(val, "url: '#{url}' did not pass")

matchesHost = (url, host) ->
expect(blacklist.matches(url, hosts)).to.eq(host)

describe "lib/util/blacklist", ->
it "handles hosts, ports, wildcards", ->
matchesArray("https://mail.google.com/foo", true)
matchesArray("https://shop.apple.com/bar", true)
matchesArray("http://localhost:6666/", true)
matchesArray("https://localhost:6666/", true)
matchesArray("https://adwords.com:443/", true)
matchesArray("http://adwords.com:80/quux", true)
matchesArray("https://yahoo.com:443/asdf", true)
matchesArray("http://mail.yahoo.com:443/asdf", true)

matchesArray("https://buy.adwords.com:443/", false)
matchesArray("http://localhost:66667", false)
matchesArray("http://mac.apple.com/", false)

matchesStr("https://localhost:6666/foo", "localhost:6666", true)

it "returns the matched host", ->
matchesHost("https://shop.apple.com:443/foo", "shop.apple.com")
matchesHost("http://mail.yahoo.com:80/bar", "*yahoo.com")
matchesHost("https://localhost:6666/bar", "localhost:6666")
21 changes: 21 additions & 0 deletions packages/server/test/unit/config_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,24 @@ describe "lib/config", ->
@setup({watchForFileChanges: 42})
@expectValidationFails("be a boolean")

context "blacklistHosts", ->
it "passes if a string", ->
@setup({blacklistHosts: "google.com"})
@expectValidationPasses()

it "passes if an array of strings", ->
@setup({blacklistHosts: ["google.com"]})
@expectValidationPasses()

it "fails if not a string or array", ->
@setup({blacklistHosts: 5})
@expectValidationFails("be a string or an array of string")

it "fails if not an array of strings", ->
@setup({blacklistHosts: [5]})
@expectValidationFails("be a string or an array of string")
@expectValidationFails("the value was: [5]")

context ".resolveConfigValues", ->
beforeEach ->
@expected = (obj) ->
Expand Down Expand Up @@ -541,6 +559,9 @@ describe "lib/config", ->
it "supportFile=false", ->
@defaults "supportFile", false, {supportFile: false}

it "blacklistHosts=null", ->
@defaults("blacklistHosts", null)

it "resets numTestsKeptInMemory to 0 when headless", ->
config.mergeDefaults({projectRoot: "/foo/bar/"}, {isTextTerminal: true})
.then (cfg) ->
Expand Down