mlflow / mlflow

Open source platform for the machine learning lifecycle
https://mlflow.org
Apache License 2.0
18.01k stars 4.07k forks source link

[BUG] UI - Dashboards as HTML Artifact - `allow-same-origin` Iframe attrib #2139

Open patryk-oleniuk opened 4 years ago

patryk-oleniuk commented 4 years ago

The current HTML artifact is great for static plots, however I'd like to embed some DASH dashboards, like this one https://dash-demo.plotly.host/ddk-clinical-trial-dashboard

The way I'm imagining this, is I create an HTML file and log it through mlflow.log_artifact(..):

<!DOCTYPE html>
<html>
<iframe src="https://dash-demo.plotly.host/ddk-clinical-trial-dashboard" style='width: 1200px; height: 1200px' sandbox='allow-same-origin allow-scripts'>
</iframe>
</html>

Currently, it does not render, and I get an error (in my browser using inspect) :

Uncaught DOMException: Failed to read the 'cookie' property from 'Document': The document is sandboxed and lacks the 'allow-same-origin' flag.

After debugging MLFlow code, I discovered that this is because the iframe where the HTML artifact is rendered only has sandbox="allow-scripts" and what I need for DASH dashboard is
sandbox="allow-same-origin allow-scripts ". After modifying this, my HTML is rendering nicely.

Therefore, I would like to request to modify that line, if possible. in the file https://github.com/mlflow/mlflow/blob/master/mlflow/server/js/src/components/artifact-view-components/ShowArtifactHtmlView.js#L60

replace this :

<Iframe url=""
                src={this.getBlobURL(this.state.html, 'text/html')}
                width="100%"
                height="500px"
                id="html"
                className="html-iframe"
                display="block"
                position="relative"
                sandbox="allow-scripts"/> 
               //  sandbox="allow-same-origin allow-scripts"/> 
      );

System information

smurching commented 4 years ago

@patryk-oleniuk thanks for raising this - adding the allow-same-origin attribute does seem to introduce a security vulnerability, in that I'm able to log an artifact that makes GET & POST requests to my local MLflow server once loaded into the UI.

In particular, I can log an artifact like the following (where my local UI is running on http://localhost:3000):

<html>
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.1/jquery.min.js" type="text/javascript"></script>

<script>
console.log("hi from sid's script " + document.cookie);
var jqxhr0 = $.post( "http://localhost:3000/ajax-api/2.0/mlflow/runs/search", function() {
})
  .done(function(data) {
    alert( "search runs success" + JSON.stringify(data) );
  })
  .fail(function(err) {
    console.log( "error " + JSON.stringify(err) );
  })
  .always(function() {
    console.log( "finished" );
  });

var jqxhr = $.get( "http://localhost:3000/ajax-api/2.0/mlflow/experiments/list", function() {
})
  .done(function(data) {
    console.log( "list experiments success" + JSON.stringify(data) );
  })
  .fail(function(err) {
    console.log( "error " + JSON.stringify(err) );
  })
  .always(function() {
    console.log( "finished" );
  });
</script>
</html>

and observe the following in the browser console:

hi from sid's script 
49a89bee-0fd5-447b-a718-de08d98aa22a:9 search runs success{}
49a89bee-0fd5-447b-a718-de08d98aa22a:15 finished
49a89bee-0fd5-447b-a718-de08d98aa22a:24 list experiments success{"experiments":[{"experiment_id":"0","name":"Default","artifact_location":"./mlruns/0","lifecycle_stage":"active"},{"experiment_id":"6","name":"HTML test","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/6","lifecycle_stage":"active"},{"experiment_id":"1","name":"/Users/test/blah","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/1","lifecycle_stage":"active"},{"experiment_id":"4","name":"runs-but-no-metrics-params","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/4","lifecycle_stage":"active"},{"experiment_id":"3","name":"my-empty-experiment","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/3","lifecycle_stage":"active"},{"experiment_id":"2","name":"/Users/test/blahh","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/2","lifecycle_stage":"active"},{"experiment_id":"5","name":"simple-parent-child","artifact_location":"file:///Users/sid.murching/code/mlflow/mlruns/5","lifecycle_stage":"active"}]}
49a89bee-0fd5-447b-a718-de08d98aa22a:30 finished

More detail

Because we directly embed the user-supplied artifact into our iframe via a blob:/ URL, the iframed HTML artifact has the same origin as the window serving it (as per MDN docs). Adding the allow-same-origin attribute allows the iframed content to make requests without automatically failing the same-origin policy, which in turn allows GET & POST requests to be made from the iframed HTML to my local MLflow server (these requests would otherwise fail as the local server doesn't allow CORS)

smurching commented 4 years ago

As a possible workaround, you might try downloading the artifact & viewing it locally, although admittedly this is harder than viewing it directly in the UI

patryk-oleniuk commented 4 years ago

@smurching thanks for fast reply and accurate explanation of the possible vulnerability. I understand this cannot be done that way.

Apart from downloading the artifact, can you think about any other possible solutions? Downloading each artifact is not acceptable for my team, since we have lots of runs and each of them would have at least a few dashboards. We'd ideally like to have all in 1 place (MLFlow window).

Thanks!

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Zangr commented 4 years ago

@smurching what do you think about making certain security-related features opt-in in some configurable way to unlock more OSS usages?

mrDzurb commented 1 year ago

I'm wondering if any changes were made since the Jul 17, 2020? It would be nice to have such config option that would allow users to make a decision what they do want to allow to open in iframe. Or maybe add some option to open HTML artifacts directly in browser but not in the iFrame.

chenmoneygithub commented 9 months ago

@harupy @BenWilson2 This is overlapping with rendering notebook in artifact section as we discussed yesterday, do we currently support rendering an html doc?

erenz14 commented 2 months ago

We're able to utilize html files as artifacts and doing some really useful things with ag-grid that lets us interact with the data.

There are however some things we can't do due to the above described iframe sandboxing issue such as:

I know the above could be seen as vulnerabilities if used as the default, but can we allow for customizing the iframe sandbox attribute as an opt in feature?

The alternative for us is modifying the source code of the lib directly.