[Engine-devel] UI Plugins configuration

Itamar Heim iheim at redhat.com
Thu Aug 23 15:26:52 UTC 2012


On 08/23/2012 06:12 PM, Frantz, Chris wrote:
> Vojtech,
>
> Your assumption about the structure of the pluginDefinitions object is
> correct.  It’s no longer a String->String mapping , but a String to
> Object mapping.
>
> I liked the original IIFE approach, except that it seemed that having
> additional static resources (jquery, images, html templates, etc) was
> going to be more cumbersome.  I don’t think having the plugin author
> write a basic start.html is that big of a burden :).
>
> I agree that the plugin configuration was always going to be a resource
> (probably a local file) that the end user could customize.  I’m not sure
> it I really needs to be separate from the plugin definition file
> (/usr/share/ovirt-engine/ui-plugins/test.json).  I suppose it depends on
> how complex the configuration is going to be and on some of the
> implementation details surrounding the plugin definition file.

if we expect a user to "touch" it (change url of external service, etc.) 
it must be under /etc as a config file.
so we should separate what we exepct admin to change (config) and what 
is part of the plugin itself ("definition"?) which would reside where 
the code lives.


>
> In my patch, I simply used Jackson to parse the file into a tree of
> JsonNodes.  Should the plugin definition be a java object of some sort?
> (please please please don’t make me learn about java beans…).  I stuck
> with the JsonNodes because Jackson makes them easy to work with and
> they’re really easy to re-serialize back to json to give to the webadmin.
>
> We should probably turn on JsonParser.Feature.ALLOW_COMMENTS.  The
> definition and config files will difficult for end-users (or even
> developers) to understand without comments.
>
> We need to formalize the structure of the plugin definition and decide
> which fields are mandatory and which are optional:
>
> {
>
>    # Mandatory fields: name, enabled, version, url, apiversion, author,
> license
>    # Name of the plugin
>
>    "name": "test",
>
>    # Whether or not plugin is enabed
>
>    "enabled": true,
>
>    # version of the plugin
>
>    "version": "1.0",
>
>    # How to load the plugin
>
>    "url": "/webadmin/webadmin/plugin/test/start.html",
>
>    # Which version of engine plugin is meant to work with
>
>    "apiversion": "3.1.0",
>
>    # Who wrote the plugin and how is it licensed?
>
>    "author": "SuperBig Corporation",
>    "license": "Proprietary",
>
>    # Optional fields path, config
>
>    # Where to locate plugin (if loaded by webadmin/plugin)
>
>    "path": "/tmp",
>
>    # Plugin configuration information (if any)
>
>    "config": "test-config.json",
> }
>
> I can work on the plugin Definition loader some more and make it enforce
> mandatory/optional fields.  I’ll also investigate the directory climbing
> issue I mentioned in my previous mail.
>
> Also, I’m curious how things are going to work when the “url” points to
> a foreign resource as the plugin start page.  I don’t think the plugin’s
> iframe is going to be able to access parent.pluginApi.  Perhaps there is
> some aspect of CORS that I don’t understand?
>
> Thanks,
>
> --Chris
>
> *From:*Vojtech Szocs [mailto:vszocs at redhat.com]
> *Sent:* Thursday, August 23, 2012 7:14 AM
> *To:* Frantz, Chris
> *Cc:* engine-devel
> *Subject:* Re: UI Plugins configuration
>
> Hi Chris,
>
> thanks for taking the time to make this patch, these are some excellent
> ideas! (CC'ing engine-devel so that we can discuss this with other guys
> as well)
>
> First of all, I really like the way you designed plugin source page URLs
> (going through /PluginSourcePageServlet/), e.g.
> "/webadmin/webadmin/plugin/<pluginName>/<pluginSourcePage>.html", plus
> the concept of "path" JSON attribute.
>
> /WebadminDynamicHostingServlet/ loads and caches all plugin definitions
> (/*.json/ files), and directly embeds them into WebAdmin host page as
> /pluginDefinitions/ JavaScript object. I'm assuming that
> /pluginDefinitions/ object will now look like this:
>
> var pluginDefinitions = {
>    "test": {
>      "name": "test",
>      "version": "1.0",
>      "url": "/webadmin/webadmin/plugin/test/foo.html",
>      "path": "/tmp",
>      "config": {"a":1, "b":2, "c":3}
>    }
> }
>
> Originally, the /pluginDefinitions/ object looked like this:
>
> var pluginDefinitions = {
>    "test": "/webadmin/webadmin/plugin/test/foo.html" // Simple
> pluginName -> pluginSourcePageUrl mappings
> }
>
> This is because PluginManager (WebAdmin) only needs /pluginName/
> ("name") and /pluginSourcePageUrl/ ("url") during startup, when creating
> plugin iframe. But this can be changed :)
>
> Plugin "version" makes sense, plus the plugin configuration object
> ("config") can be useful directly on the client. Let me explain:
>
> Originally, plugin configuration was supposed to be passed to actual
> plugin code (through immediately-invoked-function-expression, or IIFE),
> just like this:
>
> (function (pluginApi, pluginConfig) { // JavaScript IIFE
>    // ... actual plugin code ...
> })(
>    parent.pluginApi, /* reference to global pluginApi object */
>    {"a":1, "b":2, "c":3} /* embedded plugin configuration as JavaScript
> object */
> );
>
> The whole purpose of /PluginSourcePageServlet/ was to "wrap" actual
> plugin code into HTML, so that users don't need to write HTML pages for
> their plugins manually. /PluginSourcePageServlet/ would handle any
> plugin dependencies (placed into HTML head), with actual plugin code
> being wrapped into IIFE, as shown above. Plugin configuration was meant
> to be stored in a separate file, e.g. /<pluginName>-config.json/, so
> that users could change the default plugin configuration to suit their
> needs.
>
> Inspired by your patch, rather than reading/embedding plugin
> configuration when serving plugin HTML page (/PluginSourcePageServlet/),
> it's even better to have the plugin configuration embedded directly into
> WebAdmin host page, along with introducing new /pluginApi/ function to
> retrieve the plugin configuration object.
>
> Based on this, I suggest following modifications to the original concept:
>
> - modify original /pluginDefinitions/ structure, from /pluginName ->
> pluginSourcePageUrl/, to /pluginName -> pluginDefObject/
> - /pluginDefObject/ is basically a subset of physical plugin definition
> (/test.json/, see below), suitable for use on the client
> - add following attributes to /pluginDefObject/: /version/, /url/, /config/
>    * note #1: /name/ is not needed, since it's already the key of
> /pluginName -> pluginDefObject/ mapping
>    * note #2: /path/ is not needed on the client (more on this below)
> - introduce /pluginApi.config(pluginName)/ function for plugins to
> retrieve their configuration object, and remove /pluginConfig/ parameter
> from main IIFE (as shown above)
>
> [a] Physical plugin definition file (JSON) might be located at oVirt
> "DataDir", e.g. //usr/share/ovirt-engine/ui-plugins/test.json/, for example:
>
> {
>    "name": "test",
>    "version": "1.0",
>    "url": "/webadmin/webadmin/plugin/test/start.html",
>    "path": "/tmp",
>    "config": "test-config.json"
> }
>
> [b] Plugin configuration file (JSON) might be located at oVirt
> "ConfigDir", e.g. //etc/ovirt-engine/ui-plugins/test-config.json/, for
> example:
>
> {
>    "a":1, "b":2, "c":3
> }
>
> [c] Finally, plugin static resources (plugin source page, actual plugin
> code, plugin dependencies, CSS/images, etc.) would be located at //tmp/
> (as shown in [a]), for example:
>
> /tmp/start.html -> plugin source page, used to load actual plugin code
> /tmp/test.js -> actual plugin code
> /tmp/deps/jquery-min.js -> simulate 3rd party plugin dependency
>
> For example:
> "/webadmin/webadmin/plugin/test/start.html" will be mapped to
> //tmp/start.html/
> "/webadmin/webadmin/plugin/test/deps/jquery-min.js" will be mapped to
> //tmp/deps/jquery-min.js/
>
> This approach has some pros and cons:
> (+) plugin static resources can be served through
> /PluginSourcePageServlet/ (pretty much like oVirt documentation
> resources, served through oVirt Engine root war's /FileServlet/)
> (+) plugin author has complete control over plugin source page
> (-) plugin author actually needs to write plugin source page
>
> Overall, I think this approach is better than the previous one (where
> /PluginSourcePageServlet/ took care of rendering plugin source page, but
> sacrificed some flexibility).
>
> By the way, here's what would happen behind the scenes:
>
>  1. user requests WebAdmin host page, /WebadminDynamicHostingServlet/
>     loads and caches all plugin definitions [a] + plugin configurations
>     [b] and constructs/embeds appropriate /pluginDefinitions/ JavaScript
>     object
>  2. during WebAdmin startup, /PluginManager/ registers the plugin
>     (name/version/url/config), and creates/attaches the iframe to fetch
>     plugin source page ansynchronously
>  3. /PluginSourcePageServlet/ handles plugin source page request,
>     resolves the correct path [c] and just streams the file content back
>     to client
>
>> 1.  The plugin configuration files should probably have an "enabled" field and an "apiVersion" field that should be examined to determine whether or not to use the plugin.
>
> Sounds good, we can implement these later on :)
>
>> 2.  I suspect the way I've modified PluginSourcePage makes it vulnerable to directory climbing attacks.
>
> Yes, but we can defend against these, restricting access only to
> plugin's "path" and its sub-directories.
>
>> 3.  Is /usr/share/ovirt-engine the right place for the plugin config files?
>
> I suppose you mean plugin definition files [a], cannot tell for sure,
> but we can change this anytime :)
>
>
> Chris, please let me know what you think, and again - many thanks for
> sending the patch!
>
>
> Regards,
> Vojtech
>
> ------------------------------------------------------------------------
>
>
> From: "Chris Frantz" <Chris.Frantz at hp.com <mailto:Chris.Frantz at hp.com>>
> To: vszocs at redhat.com <mailto:vszocs at redhat.com>
> Sent: Wednesday, August 22, 2012 7:56:45 PM
> Subject: UI Plugins configuration
>
> Vojtech,
>
> I decided to work on making the plugin patch a bit more configurable,
> following some of the ideas expressed by Itamar and others in the
> meeting yesterday.  The attached patch is a simple first-attempt.
>
> Plugin configurations are stored in
> /usr/share/ovirt-engine/ui-plugins/*.json.
>
> Example:
> {
>          "name": "test",
>          "version": "1.0",
>          "url": "/webadmin/webadmin/plugin/test/foo.html",
>          "path": "/tmp",
>          "config": {"a":1, "b":2, "c": 3}
> }
>
> The engine reads all of the *.json files in that directory to build the
> list of known plugins and gives that list to the webadmin.
>
> When webadmin loads a plugin, it requests the URL given in the plugin
> config file.  The "plugin" URL is mapped to PluginSourcePage, which will
> translate the first part of the path ("test") into whatever path is
> stored in pluginConfig ("/tmp") in this case, and then serve the static
> file (e.g. "/tmp/foo.html").
>
> I didn't use the renderPluginSourcePage() method in favor of just
> serving a static file, but I have no strong opinion on the matter.
>   However, a plugin may want to store static resources at "path" and
> have the engine serve those resources.  By just serving files through
> PluginSourcePage, we don't need any other servlets to provide those
> resources.
>
> There is still a bit of work to do:
>
> 1.  The plugin configuration files should probably have an "enabled"
> field and an "apiVersion" field that should be examined to determine
> whether or not to use the plugin.
>
> 2.  I suspect the way I've modified PluginSourcePage makes it vulnerable
> to directory climbing attacks.
>
> 3.  Is /usr/share/ovirt-engine the right place for the plugin config files?
>
> Let me know what you think,
> --Chris
>
>
>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>





More information about the Devel mailing list