Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for echarts visualization #3

Closed
wants to merge 1 commit into from

Conversation

bahaaldine
Copy link

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

@PhaedrusTheGreek
Copy link
Owner

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
Copy link
Owner

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:

  • angular directive modules for custom charts
  • custom scripting

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

@bahaaldine
Copy link
Author

@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
Copy link
Author

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants