PhaedrusTheGreek / transform_vis

Transform Visualization for Kibana
Apache License 2.0
44 stars 32 forks source link

support for echarts visualization #3

Closed bahaaldine closed 6 years ago

bahaaldine commented 7 years ago

We need to refactor this code to allow more visualizations to be supported but this is a good start.

PhaedrusTheGreek commented 7 years ago

PR is failing tests. Seems that element.html(value) bypasses the sanitize. I tested this instead:

if ( option.allow_unsafe ) {
    element.html(value);
} else {
    element.html($sanitize(value));
}

But transform_vis.allow_unsafe will have to be enabled in order to permit the <chart> tag. There is an effort ongoing to support tag whitelist enhancement that could help us here eventually. Alternatively, some other folks have discussed manually sanitizing using 3rd party libraries

Also, I suggest we move the compile directive into it's own file: directives/compile.js

import chrome from 'ui/chrome';

const module = require('ui/modules').get('kibana/transform_vis', []);

module.directive('compile', ['$compile', '$sanitize', ($compile, $sanitize) => {
  const option = _.pick( chrome.getInjected('transformVisOptions'), 'allow_unsafe');
  return function(scope, element, attrs) {
    scope.$watch(
      (scope) => {
        return scope.$eval(attrs.compile);
      },
      (value) => {
        if ( option.allow_unsafe ) {
                element.html(value);
        } else {
                element.html($sanitize(value));
        }
        $compile(element.contents())(scope);
      }
    );
  };
}]);
PhaedrusTheGreek commented 7 years ago

I Just spoke with @spalger about this, and we are thinking that Javascript being exposed via .kibana is too dangerous for the community.

We're considering another route to be able to accomplish this exact thing, via including javascript directives from the disk of the Kibana instance. That way, you could supply these directives modularly which accomplishes both our goals:

Theoretically this will be more power-user friendly, and wouldn't require knowledge of how to build plugins - only some angular knowledge.

bahaaldine commented 7 years ago

@PhaedrusTheGreek agreed on the security concerns. On my end I don't need this anyway so not sure it's related to this PR. By the way the plugin system I've build (next PR) will allow users to install their directive and be loaded.

bahaaldine commented 7 years ago

For the unsafe parameter I can switch the directive to be use as an attribute instead of an element. It's will just add more verbosity when the user will need to use a plugin.

PhaedrusTheGreek commented 6 years ago

This is awesome, but I'm finding it too complicated to integrate at this time. I'll probably be investigating a way to load javascript extensions off the Kibana server.