Improve error experience when visiting outside of Cypress-launched br…

…owser (#6435) * force websockets transport * wip: ensure ws connections take place thru a known socket * clean up yesterday's work * remove dead code (request is undefined) * update websocket tests * add websocket tests * update socket + server specs * add token auth for file_server * Fix cy.visit file_server * restrict non-proxied URLs, serve error on runner URL non-proxied * add e2e test for server splash page * fix types * use clientRoute, fix tests * only run 6_non_proxied in electron * use browser.path * improve empty options type * add ws assertions in e2e tests * fix server_spec * refactor socket whitelisting logic * update server_spec * respond to PR feedback - added tests for non-clientRoute redirecting to clientRoute when not behind proxy - cleaned up comments - cleaned up logic in server.coffee - moved error html to own file - added unit test for socket whitelist + fixed removal bug
cypress-io · Feb 14, 2020 · 15c3e95 · 15c3e95 · cypress-bot · Feb 14, 2020
1 parent b8d6a6d
commit 15c3e95
Show file tree

Hide file tree

Showing 23 changed files with 466 additions and 91 deletions.
diff --git a/packages/extension/app/background.js b/packages/extension/app/background.js
@@ -17,7 +17,7 @@ const firstOrNull = (cookies) => {
   return cookies[0] != null ? cookies[0] : null
 }
 
-const connect = function (host, path) {
+const connect = function (host, path, extraOpts) {
   const listenToCookieChanges = once(() => {
     return browser.cookies.onChanged.addListener((info) => {
       if (info.cause !== 'overwrite') {
@@ -46,7 +46,7 @@ const connect = function (host, path) {
     })
   }
 
-  const ws = client.connect(host, path)
+  const ws = client.connect(host, path, extraOpts)
 
   ws.on('automation:request', (id, msg, data) => {
     switch (msg) {

diff --git a/packages/extension/app/client.js b/packages/extension/app/client.js
@@ -1,11 +1,12 @@
 const { client, circularParser } = require('@packages/socket/lib/browser')
 
-const connect = (host, path) => {
+const connect = (host, path, extraOpts = {}) => {
   return client.connect(host, {
     path,
     transports: ['websocket'],
     // @ts-ignore
     parser: circularParser,
+    ...extraOpts,
   })
 }
 

diff --git a/packages/https-proxy/lib/server.coffee b/packages/https-proxy/lib/server.coffee
@@ -97,11 +97,6 @@ class Server
     if fn
       return fn.call(@, req, res)
 
-    req.pipe(request(req.url))
-    .on "error", ->
-      res.statusCode = 500
-      res.end()
-    .pipe(res)
 
   _getProxyForUrl: (urlStr) ->
     port = Number(_.get(url.parse(urlStr), 'port'))
@@ -138,6 +133,8 @@ class Server
       upstreamSocket.setNoDelay(true)
       upstreamSocket.on "error", onError
 
+      browserSocket.emit 'upstream-connected', upstreamSocket
+
       browserSocket.pipe(upstreamSocket)
       upstreamSocket.pipe(browserSocket)
       upstreamSocket.write(head)

diff --git a/packages/proxy/lib/http/index.ts b/packages/proxy/lib/http/index.ts
@@ -47,6 +47,7 @@ type HttpMiddlewareCtx<T> = {
 const READONLY_MIDDLEWARE_KEYS: (keyof HttpMiddlewareThis<{}>)[] = [
   'buffers',
   'config',
+  'getFileServerToken',
   'getRemoteState',
   'request',
   'next',
@@ -59,6 +60,7 @@ const READONLY_MIDDLEWARE_KEYS: (keyof HttpMiddlewareThis<{}>)[] = [
 type HttpMiddlewareThis<T> = HttpMiddlewareCtx<T> & Readonly<{
   buffers: HttpBuffers
   config: any
+  getFileServerToken: () => string
   getRemoteState: () => any
   request: any
 
@@ -172,19 +174,22 @@ export function _runStage (type: HttpStages, ctx: any) {
 export class Http {
   buffers: HttpBuffers
   config: any
+  getFileServerToken: () => string
   getRemoteState: () => any
   middleware: MiddlewareStacks
   request: any
 
   constructor (opts: {
     config: any
+    getFileServerToken: () => string
     getRemoteState: () => any
     middleware?: MiddlewareStacks
     request: any
   }) {
     this.buffers = new HttpBuffers()
 
     this.config = opts.config
+    this.getFileServerToken = opts.getFileServerToken
     this.getRemoteState = opts.getRemoteState
     this.request = opts.request
 
@@ -206,6 +211,7 @@ export class Http {
 
       buffers: this.buffers,
       config: this.config,
+      getFileServerToken: this.getFileServerToken,
       getRemoteState: this.getRemoteState,
       request: this.request,
       middleware: _.cloneDeep(this.middleware),

diff --git a/packages/proxy/lib/http/request-middleware.ts b/packages/proxy/lib/http/request-middleware.ts
@@ -123,10 +123,12 @@ const SendRequestOutgoing: RequestMiddleware = function () {
     url: this.req.proxiedUrl,
   }
 
-  const remoteState = this.getRemoteState()
+  const { strategy, origin, fileServer } = this.getRemoteState()
+
+  if (strategy === 'file' && requestOptions.url.startsWith(origin)) {
+    this.req.headers['x-cypress-authorization'] = this.getFileServerToken()
 
-  if (remoteState.strategy === 'file' && requestOptions.url.startsWith(remoteState.origin)) {
-    requestOptions.url = requestOptions.url.replace(remoteState.origin, remoteState.fileServer)
+    requestOptions.url = requestOptions.url.replace(origin, fileServer)
   }
 
   const req = this.request.create(requestOptions)

diff --git a/packages/proxy/lib/network-proxy.ts b/packages/proxy/lib/network-proxy.ts
@@ -6,6 +6,7 @@ export class NetworkProxy {
   constructor (opts: {
     config: any
     getRemoteState: () => any
+    getFileServerToken: () => string
     middleware?: any
     request: any
   }) {

diff --git a/packages/server/__snapshots__/6_non_proxied_spec.ts.js b/packages/server/__snapshots__/6_non_proxied_spec.ts.js
@@ -0,0 +1,56 @@
+exports['e2e non-proxied spec / passes'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (spec.js)                                                                  │
+  │ Searched:   cypress/integration/spec.js                                                        │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+                                                                                                    
+  Running:  spec.js                                                                         (1 of 1)
+
+
+  non proxied e2e
+    ✓ has the expected look
+    ✓ cannot connect to ws
+    ✓ can connect to proxied ws
+
+
+  3 passing
+
+
+  (Results)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Tests:        3                                                                                │
+  │ Passing:      3                                                                                │
+  │ Failing:      0                                                                                │
+  │ Pending:      0                                                                                │
+  │ Skipped:      0                                                                                │
+  │ Screenshots:  0                                                                                │
+  │ Video:        false                                                                            │
+  │ Duration:     X seconds                                                                        │
+  │ Spec Ran:     spec.js                                                                          │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+       Spec                                              Tests  Passing  Failing  Pending  Skipped  
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔  spec.js                                  XX:XX        3        3        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    ✔  All specs passed!                        XX:XX        3        3        -        -        -  
+
+
+`
diff --git a/packages/server/lib/controllers/runner.coffee b/packages/server/lib/controllers/runner.coffee
@@ -2,12 +2,25 @@ _      = require("lodash")
 cache  = require("../cache")
 send   = require("send")
 os     = require("os")
+fs     = require("../util/fs")
+path   = require("path")
 debug  = require("debug")("cypress:server:runner")
 pkg    = require("@packages/root")
 runner = require("@packages/runner/lib/resolve-dist")
 
+PATH_TO_NON_PROXIED_ERROR = path.join(__dirname, "..", "html", "non_proxied_error.html")
+
+_serveNonProxiedError = (res) ->
+  fs.readFile(PATH_TO_NON_PROXIED_ERROR)
+  .then (html) =>
+    res.type('html').end(html)
+
 module.exports = {
   serve: (req, res, options = {}) ->
+    if req.proxiedUrl.startsWith('/')
+      debug('request was not proxied via Cypress, erroring %o', _.pick(req, 'proxiedUrl'))
+      return _serveNonProxiedError(res)
+
     { config, getRemoteState, project } = options
 
     { spec, browser } = project.getCurrentSpecAndBrowser()

diff --git a/packages/server/lib/file_server.coffee b/packages/server/lib/file_server.coffee
@@ -1,15 +1,25 @@
 ## TODO: move this to packages/core-file-server
 
 _            = require("lodash")
+debug        = require("debug")("cypress:server:file_server")
 url          = require("url")
 http         = require("http")
 path         = require("path")
 send         = require("send")
 errors       = require("./errors")
 allowDestroy = require("./util/server_destroy")
+random       = require("./util/random")
 networkFailures = require("./util/network_failures")
 
-onRequest = (req, res, fileServerFolder) ->
+onRequest = (req, res, expectedToken, fileServerFolder) ->
+  token = req.headers['x-cypress-authorization']
+
+  if token != expectedToken
+    debug('authorization failed on file_server request %o', { reqUrl: req.url, expectedToken, token })
+    res.statusCode = 401
+    res.end()
+    return
+
   args = _.compact([
     fileServerFolder,
     req.url
@@ -37,13 +47,17 @@ onRequest = (req, res, fileServerFolder) ->
 module.exports = {
   create: (fileServerFolder) ->
     new Promise (resolve) ->
+      token = random.id(64)
+
       srv = http.createServer (req, res) ->
-        onRequest(req, res, fileServerFolder)
+        onRequest(req, res, token, fileServerFolder)
 
       allowDestroy(srv)
 
       srv.listen 0, '127.0.0.1', ->
         resolve({
+          token
+
           port: ->
             srv.address().port
 

diff --git a/packages/server/lib/html/non_proxied_error.html b/packages/server/lib/html/non_proxied_error.html
@@ -0,0 +1,23 @@
+<html>
+  <head>
+    <meta http-equiv="content-type" content="text/html;charset=utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+    <title>Cypress</title>
+
+    <link href="/__cypress/static/favicon.ico" rel="icon">
+
+    <link rel="stylesheet" href="/__cypress/runner/cypress_runner.css">
+  </head>
+  <body>
+    <div id="app">
+      <div class="runner automation-failure">
+        <div class="automation-message">
+          <p>Whoops, we can't run your tests.</p>
+          <div>
+            <p class="muted">This browser was not launched through Cypress. Tests cannot run.</p>
+          </div>
+        </div>
+      </div>
+    </div>
+  </body>
+</html>