Browse Source

Merge pull request #9397 from electron/enable-webview

Allow enabling <webview> tag with node integration disabled
Kevin Sawicki 8 years ago
parent
commit
8404bdd568

+ 7 - 0
atom/browser/web_contents_preferences.cc

@@ -101,6 +101,13 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches(
   if (web_preferences.GetBoolean(options::kNodeIntegrationInWorker, &b) && b)
     command_line->AppendSwitch(switches::kNodeIntegrationInWorker);
 
+  // Check if webview tag creation is enabled, default to nodeIntegration value.
+  // TODO(kevinsawicki): Default to false in 2.0
+  bool webview_tag = node_integration;
+  web_preferences.GetBoolean(options::kWebviewTag, &webview_tag);
+  command_line->AppendSwitchASCII(switches::kWebviewTag,
+                                  webview_tag ? "true" : "false");
+
   // If the `sandbox` option was passed to the BrowserWindow's webPreferences,
   // pass `--enable-sandbox` to the renderer so it won't have any node.js
   // integration.

+ 4 - 0
atom/common/options_switches.cc

@@ -128,6 +128,9 @@ const char kDisableBlinkFeatures[] = "disableBlinkFeatures";
 // Enable the node integration in WebWorker.
 const char kNodeIntegrationInWorker[] = "nodeIntegrationInWorker";
 
+// Enable the web view tag.
+const char kWebviewTag[] = "webviewTag";
+
 }  // namespace options
 
 namespace switches {
@@ -173,6 +176,7 @@ const char kOpenerID[]         = "opener-id";
 const char kScrollBounce[]     = "scroll-bounce";
 const char kHiddenPage[]       = "hidden-page";
 const char kNativeWindowOpen[] = "native-window-open";
+const char kWebviewTag[]       = "webview-tag";
 
 // Command switch passed to renderer process to control nodeIntegration.
 const char kNodeIntegrationInWorker[]  = "node-integration-in-worker";

+ 2 - 0
atom/common/options_switches.h

@@ -64,6 +64,7 @@ extern const char kScrollBounce[];
 extern const char kBlinkFeatures[];
 extern const char kDisableBlinkFeatures[];
 extern const char kNodeIntegrationInWorker[];
+extern const char kWebviewTag[];
 
 }   // namespace options
 
@@ -94,6 +95,7 @@ extern const char kScrollBounce[];
 extern const char kHiddenPage[];
 extern const char kNativeWindowOpen[];
 extern const char kNodeIntegrationInWorker[];
+extern const char kWebviewTag[];
 
 extern const char kWidevineCdmPath[];
 extern const char kWidevineCdmVersion[];

+ 9 - 1
docs/api/browser-window.md

@@ -308,6 +308,14 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
       Console tab. **Note:** This option is currently experimental and may
       change or be removed in future Electron releases.
     * `nativeWindowOpen` Boolean (optional) - Whether to use native `window.open()`. Defaults to `false`.
+    * `webviewTag` Boolean (optional) - Whether to enable the [`<webview>` tag](webview-tag.md).
+      Defaults to the value of the `nodeIntegration` option. **Note:** The
+      `preload` script configured for the `<webview>` will have node integration
+      enabled when it is executed so you should ensure remote/untrusted content
+      is not able to create a `<webview>` tag with a possibly malicious `preload`
+      script. You can use the `will-attach-webview` event on [webContents](web-contents.md)
+      to strip away the `preload` script and to validate or alter the
+      `<webview>`'s initial settings.
 
 When setting minimum or maximum window size with `minWidth`/`maxWidth`/
 `minHeight`/`maxHeight`, it only constrains the users. It won't prevent you from
@@ -379,7 +387,7 @@ remove the reference to the window and avoid using it any more.
 
 #### Event: 'session-end' _Windows_
 
-Emitted when window session is going to end due to force shutdown or machine restart 
+Emitted when window session is going to end due to force shutdown or machine restart
 or session log off.
 
 #### Event: 'unresponsive'

+ 3 - 0
docs/api/web-contents.md

@@ -558,6 +558,9 @@ This event can be used to configure `webPreferences` for the `webContents`
 of a `<webview>` before it's loaded, and provides the ability to set settings
 that can't be set via `<webview>` attributes.
 
+**Note:** The specified `preload` script option will be appear as `preloadURL`
+(not `preload`) in the `webPreferences` object emitted with this event.
+
 ### Instance Methods
 
 #### `contents.loadURL(url[, options])`

+ 3 - 3
docs/api/webview-tag.md

@@ -15,9 +15,6 @@ between your app and embedded content will be asynchronous. This keeps your app
 safe from the embedded content. **Note:** Most methods called on the
 webview from the host page require a syncronous call to the main process.
 
-For security purposes, `webview` can only be used in `BrowserWindow`s that have
-`nodeIntegration` enabled.
-
 ## Example
 
 To embed a web page in your app, add the `webview` tag to your app's embedder
@@ -150,6 +147,9 @@ When the guest page doesn't have node integration this script will still have
 access to all Node APIs, but global objects injected by Node will be deleted
 after this script has finished executing.
 
+**Note:** This option will be appear as `preloadURL` (not `preload`) in
+the `webPreferences` specified to the `will-attach-webview` event.
+
 ### `httpreferrer`
 
 ```html

+ 20 - 0
docs/tutorial/security.md

@@ -77,6 +77,26 @@ This is not bulletproof, but at the least, you should attempt the following:
 * WebViews: Do not use `disablewebsecurity`
 * WebViews: Do not use `allowpopups`
 * WebViews: Do not use `insertCSS` or `executeJavaScript` with remote CSS/JS.
+* WebViews: Verify the options and params of all `<webview>` tags before they
+  get attached using the `will-attach-webview` event:
+
+```js
+app.on('web-contents-created', (event, contents) => {
+  contents.on('will-attach-webview', (event, webPreferences, params) => {
+    // Strip away preload scripts if unused or verify their location is legitimate
+    delete webPreferences.preload
+    delete webPreferences.preloadURL
+
+    // Disable node integration
+    webPreferences.nodeIntegration = false
+
+    // Verify URL being loaded
+    if (!params.src.startsWith('https://yourapp.com/')) {
+      event.preventDefault()
+    }
+  })
+})
+```
 
 Again, this list merely minimizes the risk, it does not remove it. If your goal
 is to display a website, a browser will be a more secure option.

+ 14 - 14
lib/browser/guest-window-manager.js

@@ -7,6 +7,14 @@ const parseFeaturesString = require('../common/parse-features-string')
 const hasProp = {}.hasOwnProperty
 const frameToGuest = new Map()
 
+// Security options that child windows will always inherit from parent windows
+const inheritedWebPreferences = new Map([
+  ['contextIsolation', true],
+  ['javascript', false],
+  ['nodeIntegration', false],
+  ['webviewTag', false]
+])
+
 // Copy attribute of |parent| to |child| if it is not defined in |child|.
 const mergeOptions = function (child, parent, visited) {
   // Check for circular reference.
@@ -43,19 +51,11 @@ const mergeBrowserWindowOptions = function (embedder, options) {
     mergeOptions(options.webPreferences, embedder.getWebPreferences())
   }
 
-  // Disable node integration on child window if disabled on parent window
-  if (embedder.getWebPreferences().nodeIntegration === false) {
-    options.webPreferences.nodeIntegration = false
-  }
-
-  // Enable context isolation on child window if enabled on parent window
-  if (embedder.getWebPreferences().contextIsolation === true) {
-    options.webPreferences.contextIsolation = true
-  }
-
-  // Disable JavaScript on child window if disabled on parent window
-  if (embedder.getWebPreferences().javascript === false) {
-    options.webPreferences.javascript = false
+  // Inherit certain option values from parent window
+  for (const [name, value] of inheritedWebPreferences) {
+    if (embedder.getWebPreferences()[name] === value) {
+      options.webPreferences[name] = value
+    }
   }
 
   // Sets correct openerId here to give correct options to 'new-window' event handler
@@ -191,7 +191,7 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', (event, url, frameName,
   const options = {}
 
   const ints = ['x', 'y', 'width', 'height', 'minWidth', 'maxWidth', 'minHeight', 'maxHeight', 'zoomFactor']
-  const webPreferences = ['zoomFactor', 'nodeIntegration', 'preload', 'javascript', 'contextIsolation']
+  const webPreferences = ['zoomFactor', 'nodeIntegration', 'preload', 'javascript', 'contextIsolation', 'webviewTag']
   const disposition = 'new-window'
 
   // Used to store additional features

+ 4 - 1
lib/renderer/init.js

@@ -54,6 +54,7 @@ electron.ipcRenderer.on('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', (ev
 
 // Process command line arguments.
 let nodeIntegration = 'false'
+let webviewTag = 'false'
 let preloadScript = null
 let isBackgroundPage = false
 let appPath = null
@@ -72,6 +73,8 @@ for (let arg of process.argv) {
     isBackgroundPage = true
   } else if (arg.indexOf('--app-path=') === 0) {
     appPath = arg.substr(arg.indexOf('=') + 1)
+  } else if (arg.indexOf('--webview-tag=') === 0) {
+    webviewTag = arg.substr(arg.indexOf('=') + 1)
   }
 }
 
@@ -94,7 +97,7 @@ if (window.location.protocol === 'chrome-devtools:') {
   require('./content-scripts-injector')
 
   // Load webview tag implementation.
-  if (nodeIntegration === 'true' && process.guestInstanceId == null) {
+  if (webviewTag === 'true' && process.guestInstanceId == null) {
     require('./web-view/web-view')
     require('./web-view/web-view-attributes')
   }

+ 20 - 0
spec/chromium-spec.js

@@ -309,6 +309,26 @@ describe('chromium feature', function () {
       b = window.open(windowUrl, '', 'javascript=no,show=no')
     })
 
+    it('disables the <webview> tag when it is disabled on the parent window', function (done) {
+      let b
+      listener = function (event) {
+        assert.equal(event.data.isWebViewGlobalUndefined, true)
+        b.close()
+        done()
+      }
+      window.addEventListener('message', listener)
+
+      var windowUrl = require('url').format({
+        pathname: `${fixtures}/pages/window-opener-no-webview-tag.html`,
+        protocol: 'file',
+        query: {
+          p: `${fixtures}/pages/window-opener-webview.html`
+        },
+        slashes: true
+      })
+      b = window.open(windowUrl, '', 'webviewTag=no,nodeIntegration=yes,show=no')
+    })
+
     it('does not override child options', function (done) {
       var b, size
       size = {

+ 1 - 0
spec/fixtures/module/preload-set-global.js

@@ -0,0 +1 @@
+window.foo = 'bar'

+ 7 - 0
spec/fixtures/pages/webview-stripped-preload.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  console.log(typeof window.foo);
+</script>
+</body>
+</html>

+ 15 - 0
spec/fixtures/pages/window-opener-no-webview-tag.html

@@ -0,0 +1,15 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  var windowUrl = decodeURIComponent(window.location.search.substring(3))
+  var opened = window.open('file://' + windowUrl, '', 'webviewTag=yes,nodeIntegration=yes,show=no')
+  window.addEventListener('message', function (event) {
+    try {
+      opened.close()
+    } finally {
+      window.opener.postMessage(event.data, '*')
+    }
+  })
+</script>
+</body>
+</html>

+ 11 - 0
spec/fixtures/pages/window-opener-webview.html

@@ -0,0 +1,11 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  document.addEventListener('DOMContentLoaded', function (event) {
+    window.opener.postMessage({
+      isWebViewGlobalUndefined: typeof WebView === 'undefined'
+    }, '*')
+  })
+</script>
+</body>
+</html>

+ 8 - 0
spec/static/main.js

@@ -277,6 +277,14 @@ ipcMain.on('disable-node-on-next-will-attach-webview', (event, id) => {
   })
 })
 
+ipcMain.on('disable-preload-on-next-will-attach-webview', (event, id) => {
+  event.sender.once('will-attach-webview', (event, webPreferences, params) => {
+    params.src = `file://${path.join(__dirname, '..', 'fixtures', 'pages', 'webview-stripped-preload.html')}`
+    delete webPreferences.preload
+    delete webPreferences.preloadURL
+  })
+})
+
 ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => {
   const consoleWarn = console.warn
   let warningMessage = null

+ 31 - 0
spec/webview-spec.js

@@ -54,6 +54,25 @@ describe('<webview> tag', function () {
     w.loadURL('file://' + fixtures + '/pages/webview-no-script.html')
   })
 
+  it('is enabled when the webviewTag option is enabled and the nodeIntegration option is disabled', function (done) {
+    w = new BrowserWindow({
+      show: false,
+      webPreferences: {
+        nodeIntegration: false,
+        preload: path.join(fixtures, 'module', 'preload-webview.js'),
+        webviewTag: true
+      }
+    })
+    ipcMain.once('webview', function (event, type) {
+      if (type !== 'undefined') {
+        done()
+      } else {
+        done('WebView is not created')
+      }
+    })
+    w.loadURL('file://' + fixtures + '/pages/webview-no-script.html')
+  })
+
   describe('src attribute', function () {
     it('specifies the page to load', function (done) {
       webview.addEventListener('console-message', function (e) {
@@ -1121,6 +1140,18 @@ describe('<webview> tag', function () {
       webview.src = 'file://' + fixtures + '/pages/c.html'
       document.body.appendChild(webview)
     })
+
+    it('supports removing the preload script', (done) => {
+      ipcRenderer.send('disable-preload-on-next-will-attach-webview')
+      webview.addEventListener('console-message', (event) => {
+        assert.equal(event.message, 'undefined')
+        done()
+      })
+      webview.setAttribute('nodeintegration', 'yes')
+      webview.setAttribute('preload', path.join(fixtures, 'module', 'preload-set-global.js'))
+      webview.src = 'file://' + fixtures + '/pages/a.html'
+      document.body.appendChild(webview)
+    })
   })
 
   it('loads devtools extensions registered on the parent window', function (done) {