vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
590 stars 163 forks source link

Let the application handle JS expression evaluation #10810

Open Legioth opened 3 years ago

Legioth commented 3 years ago

https://github.com/vaadin/flow/issues/10759 describes a mechanism that would make it possible to write new code in a way that can be compatible with strict CSP settings for JS by using APIs that require all JS expressions to be known at compile time.

That new API would be effective only once all code in Vaadin components, add-ons and the application itself would be updated. That transition period could be handled by introducing a hook that the client-side engine of Flow could use for "evaluating" an JS expression that has been been received from the server. The application could then maintain a dictionary of the server-provided expressions that it uses and provide hardcoded JS versions for those expression. In that way, all the JS needed to run the application would be included in the frontend bundle itself at compile time which in turn makes CSP support relatively easy.

The suggested API for this would be that if window.Vaadin.Flow.functionFactory is defined, then Flow will use it instead of calling new Function to evaluate JS expressions. Flow would also fall back to using new Function in case the return value from the callback wouldn't be a JS function.

For a simple application, where the only JS invocation from the server is page.executeJs("window.alert('Hello ' + $0)", "world"), then it would be enough to include this code in a JS function that could be included in the main bundle using @JsModule.

window.Vaadin.Flow.functionFactory = function(code, ...argumentNames) {
  if (code == "window.alert('Hello ' + $0)") {
     return ($0) => window.alert('Hello ' + $0);
  }
}

This mechanism would thus require a bunch of manually maintained boilerplate in application code, but it would on the other hand make it possible for applications to start using strict CSP settings without needing any other code changes.

Any cases that are not covered by the application-provided callback would fall through and thus trigger a warning in the browser if CSP restrictions are enabled. Alternatively, the application-provided callback could manually handle that case by e.g. logging or throwing an exception rather than directly falling through to the default implementation.

Legioth commented 4 months ago

In addition to executeJs, there seems to be at least two other parts in the framework that could cause challenges with typical CSP settings for JavaScript:

mshabarov commented 1 month ago

Acceptance Criteria:

  1. Vaadin provides a way to define a JavaScript code in Java, that runs in browser, but added into the frontend bundle, i.e. that is known in advance and compiled, so unsafe-eval option can be removed, making an application more resistant to XSS.
  2. To make this possible, Vaadin exposes a declarative way to add a JavaScript expression, e.g. annotations, that are discoverable during project build, so these expressions can be placed into frontend bundle.
  3. Vaadin allows to add a JavaScript literal or load JavaScript from a referenced file. API prototype is given in this article.
  4. ~All the JavaScript that is provided by Vaadin is hard-coded in the Vaadin Flow, i.e. finding and parsing all the JavaScript added in Java by Vaadin products is outside of the scope of this enhancement.~ All the JavaScript that are added by Vaadin through .executeJs() (e.g. Flow core, Flow Components or add-ons) are not automatically added into the bundle, i.e. these JS literals aren't hard-coded in Vaadin. Application developer by himself checks which JavaScript is blocked by browser/missing and adds the needed JavaScript codes into his project, Vaadin provides an API to add them, but doesn't include them automatically when CSP is enabled. Finding and parsing all the JavaScript added in Java by Vaadin products is outside of the scope of this enhancement.
  5. Vaadin does not deprecate the executeJs method and this old approach will be still by default. This means that this new feature has to be enabled by developer, either by config parameter or by another appropriate way, e.g. by adding JsModule annotation that calls some Vaadin provided JS file and activates this feature. Note that developer should also add an extra header.
  6. This feature does not bring any breaking changes to existing applications.
knoobie commented 1 month ago

Is it feasible in development mode to detect somehow that all JS expression are correctly supplied by the developer? Like Vaadin Copilot keeping track of unsafe calls and informs the developer about the call + from where it originated?

Legioth commented 1 month ago

You can implement you custom callback to throw an exception instead of falling through to the default implementation that uses eval. I'm not aware of any current approach that would use static code analysis to find cases just based on code that isn't run.

mshabarov commented 1 month ago

Updated point 4 in the AC to highlight the fact that Vaadin is not going to have a hard-coded predefined list of Vaadin-provided JavaScript. This would be a huge maintenance burden otherwise.

Legioth commented 1 month ago

I would suggest further simplifying the AC by skipping the declarative API. Instead, there would only be an API for injecting a custom interceptor (e.g. something like window.Vaadin.Flow.functionFactory = ...) that could be run from a @JsModule script. I suspect that option will be needed regardless to make it easier for application developers to collect a list of all the things that need to be covered in their application.

We can always add a declarative option for additional convenience on top of that later. By skipping the declarative option for now, we could avoid making things complicated with collecting annotations from the classpath and generating JavaScript based on that.