------=_Part_14115416_27862123.1346078924448 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Chris,=20
Your assumption about the structure of the pluginDefinitions object is co= rrect. It=E2=80=99s no longer a String->String mapping , but a String to Ob= ject mapping.=20
Yes :) but maybe we could also formalize some terms, for example:=20 Plugin descriptor is the JSON file that contains important plugin meta-data= (including the plugin source page URL), e.g. /usr/share/ovirt-engine/ui-pl= ugins/test.json=20 Plugin definition is the JavaScript object representing plugin descriptor m= eta-data suitable for use on client (GWT WebAdmin). Plugin definition is em= bedded into WebAdmin host page within pluginDefinitions object, and read by= PluginManager during WebAdmin startup.=20 Plugin configuration is the JSON file that contains optional plugin configu= ration, e.g. /etc/ovirt-engine/ui-plugins/test-config.json=20 I think we can combine two things here:=20 1) allow plugin authors to define standard (fallback) configuration directl= y inside plugin descriptor=20 2) allow plugin users to override standard configuration by modifying dedic= ated plugin configuration file=20 Finally, plugin source page is the HTML page used to invoke actual plugin c= ode (this page is referenced by plugin descriptor's "url" attribute). Plugi= n source page can also load external resources required by the plugin, e.g.= 3rd party JavaScript libraries, CSS, images, etc.=20
I liked the original IIFE approach, except that it seemed that having add= itional static resources (jquery, images, html templates, etc) was going to= be more cumbersome. I don=E2=80=99t think having the plugin author write a= basic start.html is that big of a burden :).=20
I agree that the plugin configuration was always going to be a resource (=
You're right, for such additional plugin resources, even more configuration= /parsing/logic would be required. Even though plugin authors need to write = the plugin source page themselves, they have full control over it, which is= a good thing in general.=20 probably a local file) that the end user could customize. I=E2=80=99m not s= ure 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 compl= ex the configuration is going to be and on some of the implementation detai= ls surrounding the plugin definition file.=20 Yeah, let's make the concept of the plugin configuration file optional for = now (standard plugin configuration can be part of plugin descriptor).=20
In my patch, I simply used Jackson to parse the file into a tree of JsonN= odes. Should the plugin definition be a java object of some sort? (please p= lease please don=E2=80=99t make me learn about java beans=E2=80=A6). I stuc= k with the JsonNodes because Jackson makes them easy to work with and they= =E2=80=99re really easy to re-serialize back to json to give to the webadmi= n.=20
I think using Jackson's JSON representation in Java (JsonNode) is perfectly= suitable in this situation. No need to have separate Java bean for that :)= =20
We should probably turn on JsonParser.Feature.ALLOW_COMMENTS. The definit= ion and config files will difficult for end-users (or even developers) to u= nderstand without comments.=20
Agreed.=20
We need to formalize the structure of the plugin definition and decide wh= ich fields are mandatory and which are optional=20
Sounds good, but I'd skip some attributes for now (enabled, apiVersion, aut= hor, license) for the sake of simplicity.=20 As you wrote, when loading plugin descriptor, we should enforce mandatory a= ttributes (name, version, url) .=20 As for plugin configuration, there could be two different attributes:=20 - "config" for standard (fallback) plugin configuration (JSON object)=20 - "configFile" for external plugin configuration file (path to file, relati= ve to /etc/ovirt-engine/ui-plugins/ ) , that overrides the standard configu= ration=20 Note that when loading plugin descriptor, the loader should also "merge" th= e configuration together (custom config on top of standard config).=20
I can work on the plugin Definition loader some more and make it enforce = mandatory/optional fields. I=E2=80=99ll also investigate the directory clim= bing issue I mentioned in my previous mail.=20
Also, I=E2=80=99m curious how things are going to work when the =E2=80=9C= url=E2=80=9D points to a foreign resource as the plugin start page. I don= =E2=80=99t think the plugin=E2=80=99s iframe is going to be able to access =
Sounds good! I was planning to incorporate your original patch in next PoC = revision, but of course, you can work on the loader some more and send anot= her patch :)=20 For the directory climbing issue, see <OVIRT_ROOT>/backend/manager/modules/= root/src/main/java/org/ovirt/engine/core/FileServlet.java (there's a method= called isSane for dealing with such issue).=20 parent.pluginApi. Perhaps there is some aspect of CORS that I don=E2=80=99t= understand?=20 When the plugin iframe references a resource on different origin (protocol,= domain, port) than WebAdmin main page origin, JavaScript code running insi= de that iframe will not be able to access parent (top-level) pluginApi obje= ct. You're right, the statement "parent.pluginApi" will not work, because o= f Same-Origin Policy enforced by the browser.=20 CORS is just one alternative, see http://stackoverflow.com/questions/307641= 4/ways-to-circumvent-the-same-origin-policy for more. However, CORS needs t= o be supported by the browser (a special HTTP response header is used to te= ll that the iframe is allowed to access resources from another - WebAdmin m= ain page - origin). We need to investigate this a bit more I guess.=20 Regards,=20 Vojtech=20 ----- Original Message ----- From: "Chris Frantz" <Chris.Frantz@hp.com>=20 To: "Vojtech Szocs" <vszocs@redhat.com>=20 Cc: "engine-devel" <engine-devel@ovirt.org>=20 Sent: Thursday, August 23, 2012 5:12:02 PM=20 Subject: RE: UI Plugins configuration=20 Vojtech,=20 Your assumption about the structure of the pluginDefinitions object is corr= ect. It=E2=80=99s no longer a String->String mapping , but a String to Obje= ct mapping.=20 I liked the original IIFE approach, except that it seemed that having addit= ional static resources (jquery, images, html templates, etc) was going to b= e more cumbersome. I don=E2=80=99t think having the plugin author write a b= asic start.html is that big of a burden :).=20 I agree that the plugin configuration was always going to be a resource (pr= obably a local file) that the end user could customize. I=E2=80=99m not sur= e it I really needs to be separate from the plugin definition file (/usr/sh= are/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.=20 In my patch, I simply used Jackson to parse the file into a tree of JsonNod= es. Should the plugin definition be a java object of some sort? (please ple= ase please don=E2=80=99t make me learn about java beans=E2=80=A6). I stuck = with the JsonNodes because Jackson makes them easy to work with and they=E2= =80=99re really easy to re-serialize back to json to give to the webadmin.= =20 We should probably turn on JsonParser.Feature.ALLOW_COMMENTS. The definitio= n and config files will difficult for end-users (or even developers) to und= erstand without comments.=20 We need to formalize the structure of the plugin definition and decide whic= h fields are mandatory and which are optional:=20 {=20 # Mandatory fields: name, enabled, version, url, apiversion, author, licens= e=20 # Name of the plugin=20 "name": "test",=20 # Whether or not plugin is enabed=20 "enabled": true,=20 # version of the plugin=20 "version": "1.0",=20 # How to load the plugin=20 "url": "/webadmin/webadmin/plugin/test/start.html",=20 # Which version of engine plugin is meant to work with=20 "apiversion": "3.1.0",=20 # Who wrote the plugin and how is it licensed?=20 "author": "SuperBig Corporation",=20 "license": "Proprietary",=20 # Optional fields path, config=20 # Where to locate plugin (if loaded by webadmin/plugin)=20 "path": "/tmp",=20 # Plugin configuration information (if any)=20 "config": "test-config.json",=20 }=20 I can work on the plugin Definition loader some more and make it enforce ma= ndatory/optional fields. I=E2=80=99ll also investigate the directory climbi= ng issue I mentioned in my previous mail.=20 Also, I=E2=80=99m curious how things are going to work when the =E2=80=9Cur= l=E2=80=9D points to a foreign resource as the plugin start page. I don=E2= =80=99t think the plugin=E2=80=99s iframe is going to be able to access par= ent.pluginApi. Perhaps there is some aspect of CORS that I don=E2=80=99t un= derstand?=20 Thanks,=20 --Chris=20 From: Vojtech Szocs [mailto:vszocs@redhat.com]=20 Sent: Thursday, August 23, 2012 7:14 AM=20 To: Frantz, Chris=20 Cc: engine-devel=20 Subject: Re: UI Plugins configuration=20 Hi Chris,=20 thanks for taking the time to make this patch, these are some excellent ide= as! (CC'ing engine-devel so that we can discuss this with other guys as wel= l)=20 First of all, I really like the way you designed plugin source page URLs (g= oing through PluginSourcePageServlet ), e.g. "/webadmin/webadmin/plugin/<pl= uginName>/<pluginSourcePage>.html", plus the concept of "path" JSON attribu= te.=20 WebadminDynamicHostingServlet loads and caches all plugin definitions ( *.j= son files), and directly embeds them into WebAdmin host page as pluginDefin= itions JavaScript object. I'm assuming that pluginDefinitions object will n= ow look like this:=20 var pluginDefinitions =3D {=20 "test": {=20 "name": "test",=20 "version": "1.0",=20 "url": "/webadmin/webadmin/plugin/test/foo.html",=20 "path": "/tmp",=20 "config": {"a":1, "b":2, "c":3}=20 }=20 }=20 Originally, the pluginDefinitions object looked like this:=20 var pluginDefinitions =3D {=20 "test": "/webadmin/webadmin/plugin/test/foo.html" // Simple pluginName -> p= luginSourcePageUrl mappings=20 }=20 This is because PluginManager (WebAdmin) only needs pluginName ("name") and= pluginSourcePageUrl ("url") during startup, when creating plugin iframe. B= ut this can be changed :)=20 Plugin "version" makes sense, plus the plugin configuration object ("config= ") can be useful directly on the client. Let me explain:=20 Originally, plugin configuration was supposed to be passed to actual plugin= code (through immediately-invoked-function-expression, or IIFE), just like= this:=20 (function (pluginApi, pluginConfig) { // JavaScript IIFE=20 // ... actual plugin code ...=20 })(=20 parent.pluginApi, /* reference to global pluginApi object */=20 {"a":1, "b":2, "c":3} /* embedded plugin configuration as JavaScript object= */=20 );=20 The whole purpose of PluginSourcePageServlet was to "wrap" actual plugin co= de into HTML, so that users don't need to write HTML pages for their plugin= s manually. PluginSourcePageServlet would handle any plugin dependencies (p= laced 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 pl= ugin configuration to suit their needs.=20 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 conf= iguration object.=20 Based on this, I suggest following modifications to the original concept:= =20 - modify original pluginDefinitions structure, from pluginName -> pluginSou= rcePageUrl , to pluginName -> pluginDefObject=20 - pluginDefObject is basically a subset of physical plugin definition ( tes= t.json , see below), suitable for use on the client=20 - add following attributes to pluginDefObject : version , url , config=20 * note #1: name is not needed, since it's already the key of pluginName -> = pluginDefObject mapping=20 * note #2: path is not needed on the client (more on this below)=20 - introduce pluginApi.config(pluginName) function for plugins to retrieve t= heir configuration object, and remove pluginConfig parameter from main IIFE= (as shown above)=20 [a] Physical plugin definition file (JSON) might be located at oVirt "DataD= ir", e.g. /usr/share/ovirt-engine/ui-plugins/test.json , for example:=20 {=20 "name": "test",=20 "version": "1.0",=20 "url": "/webadmin/webadmin/plugin/test/start.html",=20 "path": "/tmp",=20 "config": "test-config.json"=20 }=20 [b] Plugin configuration file (JSON) might be located at oVirt "ConfigDir",= e.g. /etc/ovirt-engine/ui-plugins/test-config.json , for example:=20 {=20 "a":1, "b":2, "c":3=20 }=20 [c] Finally, plugin static resources (plugin source page, actual plugin cod= e, plugin dependencies, CSS/images, etc.) would be located at /tmp (as show= n in [a]), for example:=20 /tmp/start.html -> plugin source page, used to load actual plugin code=20 /tmp/test.js -> actual plugin code=20 /tmp/deps/jquery-min.js -> simulate 3rd party plugin dependency=20 For example:=20 "/webadmin/webadmin/plugin/test/start.html" will be mapped to /tmp/start.ht= ml=20 "/webadmin/webadmin/plugin/test/deps/jquery-min.js" will be mapped to /tmp/= deps/jquery-min.js=20 This approach has some pros and cons:=20 (+) plugin static resources can be served through PluginSourcePageServlet (= pretty much like oVirt documentation resources, served through oVirt Engine= root war's FileServlet )=20 (+) plugin author has complete control over plugin source page=20 (-) plugin author actually needs to write plugin source page=20 Overall, I think this approach is better than the previous one (where Plugi= nSourcePageServlet took care of rendering plugin source page, but sacrifice= d some flexibility).=20 By the way, here's what would happen behind the scenes:=20 1. user requests WebAdmin host page, WebadminDynamicHostingServlet load= s and caches all plugin definitions [a] + plugin configurations [b] and con= structs/embeds appropriate pluginDefinitions JavaScript object=20 2. during WebAdmin startup, PluginManager registers the plugin (name/ve= rsion/url/config), and creates/attaches the iframe to fetch plugin source p= age ansynchronously=20 3. PluginSourcePageServlet handles plugin source page request, resolves= the correct path [c] and just streams the file content back to client=20
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.=20
Sounds good, we can implement these later on :)=20
2. I suspect the way I've modified PluginSourcePage makes it vulnerable t= o directory climbing attacks.=20
Yes, but we can defend against these, restricting access only to plugin's "= path" and its sub-directories.=20
3. Is /usr/share/ovirt-engine the right place for the plugin config files= ?=20
- "config" for standard (fallback) plugin configuration (JSON object)<br>-= "configFile" for external plugin configuration file (path to file, relativ= e to <span style=3D"color:black">/etc/ovirt-engine/ui-plugins/)</span>, tha= t overrides the standard configuration<br><br>Note that when loading plugin= descriptor, the loader should also "merge" the configuration together (cus= tom config on top of standard config).<br><br><span style=3D"font-size:11.0=
I suppose you mean plugin definition files [a], cannot tell for sure, but w= e can change this anytime :)=20 Chris, please let me know what you think, and again - many thanks for sendi= ng the patch!=20 Regards,=20 Vojtech=20 ----- Original Message ----- From: "Chris Frantz" < Chris.Frantz@hp.com >=20 To: vszocs@redhat.com=20 Sent: Wednesday, August 22, 2012 7:56:45 PM=20 Subject: UI Plugins configuration=20 Vojtech,=20 I decided to work on making the plugin patch a bit more configurable, follo= wing some of the ideas expressed by Itamar and others in the meeting yester= day. The attached patch is a simple first-attempt.=20 Plugin configurations are stored in /usr/share/ovirt-engine/ui-plugins/*.js= on.=20 Example:=20 {=20 "name": "test",=20 "version": "1.0",=20 "url": "/webadmin/webadmin/plugin/test/foo.html",=20 "path": "/tmp",=20 "config": {"a":1, "b":2, "c": 3}=20 }=20 The engine reads all of the *.json files in that directory to build the lis= t of known plugins and gives that list to the webadmin.=20 When webadmin loads a plugin, it requests the URL given in the plugin confi= g file. The "plugin" URL is mapped to PluginSourcePage, which will translat= e the first part of the path ("test") into whatever path is stored in plugi= nConfig ("/tmp") in this case, and then serve the static file (e.g. "/tmp/f= oo.html").=20 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 tho= se resources. By just serving files through PluginSourcePage, we don't need= any other servlets to provide those resources.=20 There is still a bit of work to do:=20 1. The plugin configuration files should probably have an "enabled" field a= nd an "apiVersion" field that should be examined to determine whether or no= t to use the plugin.=20 2. I suspect the way I've modified PluginSourcePage makes it vulnerable to = directory climbing attacks.=20 3. Is /usr/share/ovirt-engine the right place for the plugin config files?= =20 Let me know what you think,=20 --Chris=20 ------=_Part_14115416_27862123.1346078924448 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable <html><head><style type=3D'text/css'>p { margin: 0; }</style></head><body><= div style=3D'font-family: times new roman,new york,times,serif; font-size: = 12pt; color: #000000'>Hi Chris,<br><br><span style=3D"font-size:11.0pt;font= -family:"Calibri","sans-serif";color:#1F497D">> Your assumption about the structure of the pluginDefinitions object is=20 correct. It=E2=80=99s no longer a String->String mapping , but a S= tring to=20 Object mapping.</span><br><br>Yes :) but maybe we could also formalize some= terms, for example:<br><br><em>Plugin descriptor</em> is the JSON file tha= t contains important plugin meta-data (including the plugin source page URL= ), e.g. <span style=3D"color: black;"><i>/usr/share/ovirt-engine/ui-plugins= /test.json<br><span style=3D"font-style: italic;"><span style=3D"font-style= : italic;"></span></span></i><br><span style=3D"font-style: italic;">Plugin= definition</span> is the JavaScript object representing plugin descriptor = meta-data suitable for use on client (GWT WebAdmin). Plugin definition is e= mbedded into WebAdmin host page within <span style=3D"font-style: italic;">= pluginDefinitions</span> object, and read by <span style=3D"font-style: ita= lic;">PluginManager</span> during WebAdmin startup.<br><i><span style=3D"fo= nt-style: italic;"><span style=3D"font-style: italic;"></span><br>Plugin co= nfiguration</span></i> is the JSON file that contains <strong>optional</str= ong> plugin configuration, e.g. </span><span style=3D"color:black"><i>/etc/= ovirt-engine/ui-plugins/test-config.json</i></span><br><span style=3D"color= : black;"><i><span style=3D"font-style: italic;"><br></span></i>I think we = can combine two things here:<br>1) allow plugin authors to define standard = (fallback) configuration directly inside plugin descriptor<br>2) allow plug= in users to override standard configuration by modifying dedicated plugin c= onfiguration file<br><br>Finally, <span style=3D"font-style: italic;">plugi= n source page</span> is the HTML page used to invoke actual plugin code (th= is page is referenced by plugin descriptor's "url" attribute). Plugin sourc= e page can also load external resources required by the plugin, e.g. 3rd pa= rty JavaScript libraries, CSS, images, etc.<br><br></span><span style=3D"fo= nt-size:11.0pt;font-family:"Calibri","sans-serif";color= :#1F497D">> I liked the original IIFE approach, except that it seemed that having=20 additional static resources (jquery, images, html templates, etc) was=20 going to be more cumbersome. I don=E2=80=99t think having the plugin author write a b= asic=20 start.html is that big of a burden :).</span><br><br>You're right, for such= additional plugin resources, even more configuration/parsing/logic would b= e required. Even though plugin authors need to write the plugin source page= themselves, they have full control over it, which is a good thing in gener= al.<br><br><span style=3D"font-size:11.0pt;font-family:"Calibri",= "sans-serif";color:#1F497D">> I agree that the plugin configuration was always going to be a resource=20 (probably a local file) that the end user could customize. I=E2=80=99= m not sure it I really needs to be separate from the plugin definition file=20 (/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=20 implementation details surrounding the plugin definition file.</span><br><b= r>Yeah, let's make the concept of the plugin configuration file optional fo= r now (standard plugin configuration can be part of plugin descriptor).<br>= <br><span style=3D"font-size:11.0pt;font-family:"Calibri","s= ans-serif";color:#1F497D">> In my patch, I simply used Jackson to parse the file into a tree of=20 JsonNodes. Should the plugin definition be a java object of some sort= ?=20 (please please please don=E2=80=99t make me learn about java beans=E2=80=A6). I stuck with= the JsonNodes=20 because Jackson makes them easy to work with and they=E2=80=99re really eas= y to=20 re-serialize back to json to give to the webadmin.</span><br><br>I think us= ing Jackson's JSON representation in Java (JsonNode) is perfectly suitable = in this situation. No need to have separate Java bean for that :)<br><br><s= pan style=3D"font-size:11.0pt;font-family:"Calibri","sans-se= rif";color:#1F497D">> We should probably turn on JsonParser.Feature.ALLOW_COMMENTS. The=20 definition and config files will difficult for end-users (or even=20 developers) to understand without comments.</span><br><br>Agreed.<br><br><span style=3D"font-size:11= .0pt;font-family:"Calibri","sans-serif";color:#1F497D">= > We need to formalize the structure of the plugin definition and decide= which fields are mandatory and which are optional</span><br><br>Sounds goo= d, but I'd skip some attributes for now (enabled, apiVersion, author, licen= se) for the sake of simplicity.<br><br>As you wrote, when loading plugin de= scriptor, we should enforce mandatory attributes (name, version, url).<br><= br>As for plugin configuration, there could be two different attributes:<br= pt;font-family:"Calibri","sans-serif";color:#1F497D">&g= t; I can work on the plugin Definition loader some more and make it enforce=20 mandatory/optional fields. I=E2=80=99ll also investigate the director= y climbing issue I mentioned in my previous mail.</span><br><br>Sounds good! I was planning to incorpor= ate your original patch in next PoC revision, but of course, you can work o= n the loader some more and send another patch :)<br><br>For the directory c= limbing issue, see <span style=3D"font-style: italic;"><OVIRT_ROOT>/b= ackend/manager/modules/root/src/main/java/org/ovirt/engine/core/FileServlet= .java</span> (there's a method called isSane for dealing with such issue).<= br><br><span style=3D"font-size:11.0pt;font-family:"Calibri",&quo= t;sans-serif";color:#1F497D">> Also, I=E2=80=99m curious how things are going to work when the =E2=80=9Curl=E2= =80=9D points to a=20 foreign resource as the plugin start page. I don=E2=80=99t think the = plugin=E2=80=99s=20 iframe is going to be able to access parent.pluginApi. Perhaps there is some aspect = of CORS that I don=E2=80=99t understand?</span><br><br>When the plugin iframe= references a resource on different origin (protocol, domain, port) than We= bAdmin main page origin, JavaScript code running inside that iframe will no= t be able to access parent (top-level) <span style=3D"font-style: italic;">= pluginApi</span> object. You're right, the statement "parent.pluginApi" wil= l not work, because of Same-Origin Policy enforced by the browser.<br><br>C= ORS is just one alternative, see http://stackoverflow.com/questions/3076414= /ways-to-circumvent-the-same-origin-policy for more. However, CORS needs to= be supported by the browser (a special HTTP response header is used to tel= l that the iframe is allowed to access resources from another - WebAdmin ma= in page - origin). We need to investigate this a bit more I guess.<br><br>R= egards,<br>Vojtech<br><br><br><hr id=3D"zwchr"><div style=3D"color:#000;fon= t-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetic= a,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Chris Frantz" <Chris.F= rantz@hp.com><br><b>To: </b>"Vojtech Szocs" <vszocs@redhat.com><br=
<b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b>T= hursday, August 23, 2012 5:12:02 PM<br><b>Subject: </b>RE: UI Plugins confi= guration<br><br>
<style><!-- @font-face =09{font-family:"Cambria Math"; =09panose-1:2 4 5 3 5 4 6 3 2 4;} @font-face =09{font-family:Calibri; =09panose-1:2 15 5 2 2 2 4 3 2 4;} @font-face =09{font-family:Tahoma; =09panose-1:2 11 6 4 3 5 4 4 2 4;} p.MsoNormal, li.MsoNormal, div.MsoNormal =09{margin:0in; =09margin-bottom:.0001pt; =09font-size:12.0pt; =09font-family:"Times New Roman","serif";} a:link, span.MsoHyperlink =09{mso-style-priority:99; =09color:blue; =09text-decoration:underline;} a:visited, span.MsoHyperlinkFollowed =09{mso-style-priority:99; =09color:purple; =09text-decoration:underline;} p =09{mso-style-priority:99; =09margin:0in; =09margin-bottom:.0001pt; =09font-size:12.0pt; =09font-family:"Times New Roman","serif";} span.EmailStyle19 =09{mso-style-type:personal-reply; =09font-family:"Calibri","sans-serif"; =09color:#1F497D;} .MsoChpDefault =09{mso-style-type:export-only; =09font-size:10.0pt;} @page WordSection1 =09{size:8.5in 11.0in; =09margin:1.0in 1.0in 1.0in 1.0in;} div.WordSection1 =09{page:WordSection1;} @list l0 =09{mso-list-id:135345730; =09mso-list-template-ids:-1544269566;} ol =09{margin-bottom:0in;} ul =09{margin-bottom:0in;} --></style> <div class=3D"WordSection1"> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">Vojtech,</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">Your assumption about the= structure of the pluginDefinitions object is correct. It=E2=80=99s n= o longer a String->String mapping , but a String to Object mapping.</spa= n></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">I liked the original IIFE= approach, except that it seemed that having additional static resources (j= query, images, html templates, etc) was going to be more cumbersome. I don=E2=80=99t think having the plugin author write a b= asic start.html is that big of a burden :).</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">I agree that the plugin c= onfiguration was always going to be a resource (probably a local file) that= the end user could customize. I=E2=80=99m not sure it I really needs to be separate from the plugin definition file (/usr/share/ovirt-eng= ine/ui-plugins/test.json). I suppose it depends on how complex the co= nfiguration is going to be and on some of the implementation details surrou= nding the plugin definition file.</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">In my patch, I simply use= d Jackson to parse the file into a tree of JsonNodes. Should the plug= in definition be a java object of some sort? (please please please don=E2=80=99t make me learn about java beans=E2=80=A6). I stuck with= the JsonNodes because Jackson makes them easy to work with and they=E2=80= =99re really easy to re-serialize back to json to give to the webadmin.</sp= an></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">We should probably turn o= n JsonParser.Feature.ALLOW_COMMENTS. The definition and config files = will difficult for end-users (or even developers) to understand without comments.</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">We need to formalize the = structure of the plugin definition and decide which fields are mandatory an= d which are optional:</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black">{</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Mandatory fields: name, enabled, version, url, apivers= ion, author, license<br> # Name of the plugin </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "name": "test",<br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Whether or not plugin is enabed</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "enabled": true,<br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # version of the plugin</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "version": "1.0",<br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # How to load the plugin</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "url": "/webadmin/webadmin/plugin/test/start.html",= <br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Which version of engine plugin is meant to work with</= span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "apiversion": "3.1.0",<br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Who wrote the plugin and how is it licensed?</span></p=
<p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "author": "SuperBig Corporation",<br> "license": "Proprietary",<br> <br> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Optional fields path, config</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Where to locate plugin (if loaded by webadmin/plugin)<= /span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "path": "/tmp",</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> # Plugin configuration information (if any)</span></p> <p class=3D"MsoNormal"><span style=3D"font-family:"Courier New";c= olor:black"> "config": "test-config.json",<br> }</span><span style=3D"color:black"><br> </span><span style=3D"font-size:11.0pt;font-family:"Courier New";= color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">I can work on the plugin = Definition loader some more and make it enforce mandatory/optional fields. = I=E2=80=99ll also investigate the directory climbing issue I mentione= d in my previous mail.</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">Also, I=E2=80=99m curious= how things are going to work when the =E2=80=9Curl=E2=80=9D points to a fo= reign resource as the plugin start page. I don=E2=80=99t think the pl= ugin=E2=80=99s iframe is going to be able to access parent.pluginApi. Perhaps there is some aspect = of CORS that I don=E2=80=99t understand?</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">Thanks,</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D">--Chris</span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <p class=3D"MsoNormal"><span style=3D"font-size:11.0pt;font-family:"Ca= libri","sans-serif";color:#1F497D"> </span></p> <div> <div style=3D"border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in = 0in 0in"> <p class=3D"MsoNormal"><b><span style=3D"font-size:10.0pt;font-family:"= ;Tahoma","sans-serif"">From:</span></b><span style=3D"font-s= ize:10.0pt;font-family:"Tahoma","sans-serif""> Vojtech = Szocs [mailto:vszocs@redhat.com] <br> <b>Sent:</b> Thursday, August 23, 2012 7:14 AM<br> <b>To:</b> Frantz, Chris<br> <b>Cc:</b> engine-devel<br> <b>Subject:</b> Re: UI Plugins configuration</span></p> </div> </div> <p class=3D"MsoNormal"> </p> <div> <p class=3D"MsoNormal"><span style=3D"color:black">Hi Chris,<br> <br> thanks for taking the time to make this patch, these are some excellent ide= as! (CC'ing engine-devel so that we can discuss this with other guys as wel= l)<br> <br> First of all, I really like the way you designed plugin source page URLs (g= oing through <em>PluginSourcePageServlet</em>), e.g. "/webadmin/webadmin/plugin/<plug= inName>/<pluginSourcePage>.html", plus the concept of "path" JSON = attribute.<br> <br> <i>WebadminDynamicHostingServlet</i> loads and caches all plugin definition= s (<i>*.json</i> files), and directly embeds them into WebAdmin host page a= s <i>pluginDefinitions</i> JavaScript object. I'm assuming that <i>pluginDefi= nitions</i> object will now look like this:<br> <br> </span><span style=3D"font-family:"Courier New";color:black">var = pluginDefinitions =3D {<br> "test": {<br> "name": "test",<br> "version": "1.0",<br> "url": "/webadmin/webadmin/plugin/test/foo.html",<br> "path": "/tmp",<br> "config": {"a":1, "b":2, "c":3}<br> }<br> }</span><span style=3D"color:black"><br> <br> Originally, the <i>pluginDefinitions</i> object looked like this:<br> </span><span style=3D"font-family:"Courier New";color:black"><br> var pluginDefinitions =3D {<br> "test": "/webadmin/webadmin/plugin/test/foo.html" // Simple plu= ginName -> pluginSourcePageUrl mappings<br> }</span><span style=3D"color:black"><br> <br> This is because PluginManager (WebAdmin) only needs <i>pluginName</i> ("nam= e") and <i>pluginSourcePageUrl</i> ("url") during startup, when creating plugin ifr= ame. But this can be changed :)<br> <br> Plugin "version" makes sense, plus the plugin configuration object ("config= ") can be useful directly on the client. Let me explain:<br> <br> Originally, plugin configuration was supposed to be passed to actual plugin= code (through immediately-invoked-function-expression, or IIFE), just like= this:<br> </span><span style=3D"font-family:"Courier New";color:black"><br> (function (pluginApi, pluginConfig) { // JavaScript IIFE<br> // ... actual plugin code ...<br> })(<br> parent.pluginApi, /* reference to global pluginApi object */<br=
{"a":1, "b":2, "c":3} /* embedded plugin configuration as JavaS= cript object */<br> );</span><span style=3D"color:black"><br> <br> The whole purpose of <i>PluginSourcePageServlet</i> was to "wrap" actual pl= ugin code into HTML, so that users don't need to write HTML pages for their= plugins manually. <i>PluginSourcePageServlet</i> 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= . <i><pluginName>-config.json</i>, so that users could change the defau= lt plugin configuration to suit their needs.<br> <br> Inspired by your patch, rather than reading/embedding plugin configuration = when serving plugin HTML page (<i>PluginSourcePageServlet</i>), it's even b= etter to have the plugin configuration embedded directly into WebAdmin host= page, along with introducing new <i>pluginApi</i> function to retrieve the plugin configuration object.<br> <br> Based on this, I suggest following modifications to the original concept:<b= r> <br> - modify original <i>pluginDefinitions</i> structure, from <i>pluginName -&= gt; pluginSourcePageUrl</i>, to <i>pluginName -> pluginDefObject</i><br> - <i>pluginDefObject</i> is basically a subset of physical plugin definitio= n (<i>test.json</i>, see below), suitable for use on the client<br> - add following attributes to <i>pluginDefObject</i>: <i>version</i>, <i>ur= l</i>, <i>config</i><br> * note #1: <i>name</i> is not needed, since it's already the ke= y of <i>pluginName -> pluginDefObject</i> mapping<br> * note #2: <i>path</i> is not needed on the client (more on thi= s below)<br> - introduce <i>pluginApi.config(pluginName)</i> function for plugins to ret= rieve their configuration object, and remove <i>pluginConfig</i> parameter from main IIFE (as shown above)<br> <br> [a] Physical plugin definition file (JSON) might be located at oVirt "DataD= ir", e.g. <i>/usr/share/ovirt-engine/ui-plugins/test.json</i>, for example:<br> </span><span style=3D"font-family:"Courier New";color:black"><br> {<br> "name": "test",<br> "version": "1.0",<br> "url": "/webadmin/webadmin/plugin/test/start.html",<br> "path": "/tmp",<br> "config": "test-config.json"<br> }</span><span style=3D"color:black"><br> <br> [b] Plugin configuration file (JSON) might be located at oVirt "ConfigDir",= e.g. <i> /etc/ovirt-engine/ui-plugins/test-config.json</i>, for example:<br> </span><span style=3D"font-family:"Courier New";color:black"><br> {<br> "a":1, "b":2, "c":3<br> }</span><span style=3D"color:black"><br> <br> [c] Finally, plugin static resources (plugin source page, actual plugin cod= e, plugin dependencies, CSS/images, etc.) would be located at <i>/tmp</i> (as shown in [a]), for example:<br> </span><span style=3D"font-family:"Courier New";color:black"><br> /tmp/start.html -> plugin source page, used to load actual plugin code<b= r> /tmp/test.js -> actual plugin code<br> /tmp/deps/jquery-min.js -> simulate 3rd party plugin dependency</span><s= pan style=3D"color:black"><br> <br> For example:<br> "/webadmin/webadmin/plugin/test/start.html" will be mapped to <i>/tmp/start= .html</i><br> "/webadmin/webadmin/plugin/test/deps/jquery-min.js" will be mapped to <i>/t= mp/deps/jquery-min.js</i><br> <br> This approach has some pros and cons:<br> (+) plugin static resources can be served through <i>PluginSourcePageServle= t</i> (pretty much like oVirt documentation resources, served through oVirt= Engine root war's <i>FileServlet</i>)<br> (+) plugin author has complete control over plugin source page<br> (-) plugin author actually needs to write plugin source page<br> <br> Overall, I think this approach is better than the previous one (where <i>Pl= uginSourcePageServlet</i> took care of rendering plugin source page, but sa= crificed some flexibility).<br> <br> By the way, here's what would happen behind the scenes:</span></p> <ol start=3D"1" type=3D"1"> <li class=3D"MsoNormal" style=3D"color:black;mso-margin-top-alt:auto;margin= -bottom:12.0pt;mso-list:l0 level1 lfo1"> user requests WebAdmin host page, <i>WebadminDynamicHostingServlet</i> load= s and caches all plugin definitions [a] + plugin configurations [b] and con= structs/embeds appropriate <i>pluginDefinitions</i> JavaScript object</li><li class=3D"MsoNormal" styl= e=3D"color:black;mso-margin-top-alt:auto;margin-bottom:12.0pt;mso-list:l0 l= evel1 lfo1"> during WebAdmin startup, <i>PluginManager</i> registers the plugin (name/ve= rsion/url/config), and creates/attaches the iframe to fetch plugin source p= age ansynchronously</li><li class=3D"MsoNormal" style=3D"color:black;mso-ma= rgin-top-alt:auto;margin-bottom:12.0pt;mso-list:l0 level1 lfo1"> <i>PluginSourcePageServlet</i> handles plugin source page request, resolves= the correct path [c] and just streams the file content back to client</li>= </ol> <p class=3D"MsoNormal" style=3D"margin-bottom:12.0pt"><span style=3D"color:= black">> 1. The plugin configuration files should probably have an= "enabled" field and an "apiVersion" field that should be examined to deter= mine whether or not to use the plugin.<br> <br> Sounds good, we can implement these later on :)<br> <br> > 2. I suspect the way I've modified PluginSourcePage makes it vul= nerable to directory climbing attacks.<br> <br> Yes, but we can defend against these, restricting access only to plugin's "= path" and its sub-directories.<br> <br> > 3. Is /usr/share/ovirt-engine the right place for the plugin con= fig files?<br> <br> I suppose you mean plugin definition files [a], cannot tell for sure, but w= e can change this anytime :)<br> <br> <br> Chris, please let me know what you think, and again - many thanks for sendi= ng the patch!<br> <br> <br> Regards,<br> Vojtech<br> <br> </span></p> <div class=3D"MsoNormal" style=3D"text-align:center" align=3D"center"><span= style=3D"color:black"> <hr id=3D"zwchr" size=3D"2" width=3D"100%" align=3D"center"> </span></div> <p class=3D"MsoNormal" style=3D"margin-bottom:12.0pt"><span style=3D"color:= black"><br> From: "Chris Frantz" <<a href=3D"mailto:Chris.Frantz@hp.com" target=3D"_= blank">Chris.Frantz@hp.com</a>><br> To: <a href=3D"mailto:vszocs@redhat.com" target=3D"_blank">vszocs@redhat.co= m</a><br> Sent: Wednesday, August 22, 2012 7:56:45 PM<br> Subject: UI Plugins configuration<br> <br> Vojtech,<br> <br> I decided to work on making the plugin patch a bit more configurable, follo= wing some of the ideas expressed by Itamar and others in the meeting yester= day. The attached patch is a simple first-attempt.<br> <br> Plugin configurations are stored in /usr/share/ovirt-engine/ui-plugins/*.js= on.<br> <br> Example:<br> {<br> "name": "test",<br> "version": "1.0",<br> "url": "/webadmin/webadmin/= plugin/test/foo.html",<br> "path": "/tmp",<br> "config": {"a":1, "b":2, "c= ": 3}<br> }<br> <br> The engine reads all of the *.json files in that directory to build the lis= t of known plugins and gives that list to the webadmin.<br> <br> When webadmin loads a plugin, it requests the URL given in the plugin confi= g file. The "plugin" URL is mapped to PluginSourcePage, which will tr= anslate 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").<br> <br> 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 ser= ve those resources. By just serving files through PluginSourcePage, we don't need any other servlets to provid= e those resources.<br> <br> There is still a bit of work to do:<br> <br> 1. The plugin configuration files should probably have an "enabled" f= ield and an "apiVersion" field that should be examined to determine whether= or not to use the plugin.<br> <br> 2. I suspect the way I've modified PluginSourcePage makes it vulnerab= le to directory climbing attacks.<br> <br> 3. Is /usr/share/ovirt-engine the right place for the plugin config f= iles?<br> <br> Let me know what you think,<br> --Chris</span></p> </div> </div> </div><br></div></body></html> ------=_Part_14115416_27862123.1346078924448--