------=_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
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
I agree that the plugin configuration was always going to be a
resource (=
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
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
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 =
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(a)hp.com>=20
To: "Vojtech Szocs" <vszocs(a)redhat.com>=20
Cc: "engine-devel" <engine-devel(a)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
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(a)hp.com >=20
To: vszocs(a)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=
- "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=
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(a)hp.com&gt;<br><b>To: </b>"Vojtech Szocs"
&lt;vszocs(a)redhat.com&gt;<br=
<b>Cc: </b>"engine-devel"
&lt;engine-devel(a)ovirt.org&gt;<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(a)hp.com</a>&gt;<br
To: <a
href=3D"mailto:vszocs@redhat.com"
target=3D"_blank">vszocs(a)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--