mini-browser, webview: warn if unsecure

[paul-marechal]

Add security warnings to the mini-browser and webviews when modifying
the host patterns. You can disable those warnings by setting
warnOnPotentiallyInsecureHostPattern: false in your application's
package.json file, as frontend/backend configurations.

How to test

• Run the example applications with a clean env:
  • No webview warning.
  • No mini-browser warning.
• Set THEIA_WEBVIEW_EXTERNAL_ENDPOINT={{hostname}} and run the example applications:
  • It should warn on both the backend and frontend about this.
• Set THEIA_MINI_BROWSER_HOST_PATTERN={{hostname}} and run the example applications:
  • It should warn on both the backend and frontend about this.
• Edit the example package.json file to set the frontend/backend config's warnOnPotentiallyInsecureHostPattern: false:
  • None of the above should be happening again: no more checks/warnings even if you change your env.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[vince-fugnitto]

I confirmed that the changes work as intended 👍

• confirmed that the preference webview.warnIfUnsecure works correctly when the webview environment is unsecure - selecting 'do not show again' updates the preference.
• confirmed that the preference mini-browser.warnIfUnsecure works correctly when the environment for mini-browser is unsecure - selecting 'do not show again' updates the preference.
• confirmed that mini-browser.previewFile.preventUnsecure works correctly:
  • ask: prompts to choose which action to perform (confirmed they all work as intended).
  • alwaysOpen: opens the resource always.
  • alwaysPrevent: prevents opening the resource - notification is displayed when preventing.
• confirmed that setting the application property securityWarnings to false removes all functionality checks.

At the moment electron also complains like browser is it the intention to also check the environment for electron?

[marcdumais-work]

At the moment electron also complains like browser is it the intention to also check the environment for electron?

Do you mean if one sets the environment variable to insecure mode, the Electron version of Theia apps will respect it? In that case, it might be good to have the warning for Electron too, when it applies.

[vince-fugnitto]

At the moment electron also complains like browser is it the intention to also check the environment for electron?

Do you mean if one sets the environment variable to insecure mode, the Electron version of Theia apps will respect it? In that case, it might be good to have the warning for Electron too, when it applies.

@vince-fugnitto I mean even in the electron target if the environment is unescure for either webviews and mini-browser it will warn users which I think might be good, just wanted to confirm 👍 The functionality is on by default for both electron and browser applications.

[marcdumais-work]

I mean even in the electron target if the environment is unescure for either webviews and mini-browser

So long as the warning is valid, I see no problem. For example, if the environment variable were not used for Electron and a secure setup was done no matter what, there would be no need to warn if the environment variables were set to an insecure value.

[paul-marechal]

For example, if the environment variable were not used for Electron and a secure setup was done no matter what

Let's do that actually, will update this PR with a commit for this.

[vince-fugnitto]

I confirmed that the changes work as intended:

• confirmed that the preference webview.warnIfUnsecure works correctly when the webview environment is unsecure - selecting 'do not show again' updates the preference.
• confirmed that the preference mini-browser.warnIfUnsecure works correctly when the environment for mini-browser is unsecure - selecting 'do not show again' updates the preference.
• confirmed that mini-browser.previewFile.preventUnsecure works correctly:
  • ask: prompts to choose which action to perform:
    • Open: opens the resource
    • Always Open: sets the preference to alwaysOpen and opens the resource
    • Prevents: prevents opening the resource
    • Always Prevent: sets the preference alwaysPrevent and prevents opening the resource
  • alwaysOpen: opens the resource always.
  • alwaysPrevent: prevents opening the resource - notification is displayed when preventing.
• confirmed that setting the application property securityWarnings to false removes all functionality checks.
• confirmed that no notifications are present for electron as it is secure.

Open: successfully opens the preview

[paul-marechal]

@marcdumais-work @vince-fugnitto I updated this PR to be simpler.

It will just prompt a warning whenever someone changes the host patterns to something different from the default, like suggested by @marcdumais-work . The warnings point to READMEs with instructions on how to turn it off.

@eclipse-theia/core is the change in its current form acceptable for everyone?

[vince-fugnitto]

I confirmed that the latest updates work correctly 👍

• there are no errors when the environment is secure.
• a notification appears when the mini-browser environment is unsecure, and a message is logged on the backend (THEIA_MINI_BROWSER_EXTERNAL_ENDPOINT={{hostname}})
  • ok dismisses the notification.
  • go to readme opens the documentation for mini-browser.
• a notification appears when the webview environment is unsecure, and a message is logged on the backend (THEIA_WEBVIEW_EXTERNAL_ENDPOINT={{hostname}})
  • ok dismisses the notification.
  • go to readme opens the documentation for mini-browser.
• the warnings are suppressed if warnOnPotentiallyInsecureHostPattern is set to false in the application's config (package.json).

[marcdumais-work]

@vince-fugnitto @paul-marechal Are we good to merge this, before the release?