284519 - Need preference for whitelisting submission, instance loading, etc.

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

It would be nice to have a place in the preferences to allow the whitelisting
(or even blacklisting, I guess) of domains.  So then you could submit instance
data to other domains as long as the form came from a "trusted" domain (like
mozilla.org).

Right now you have to physically alter the permission manager file (hostperm.1)
using the following format:

"host\txforms-load\t1\tmozilla.org"

Comment 1

[aaronr]

jst said that we should have no problem modifing prefs using the pref service
(no security problems to worry about because we are an extension).  Now I'm
checking with Ben to see about the UI.

Comment 2

[aaronr]

We need to make sure that there is a way to allow for submitting to a webservice
from a form loaded locally, too.

Comment 3

[Doron Rosenberg (IBM)]

Attachment: patch

Requires the patch in bug 295613 to apply correctly to the firefox pref window.


UI is only added for Firefox right now, Suite's permission UI is harder to bind
to, and Firefox's perm manager is in browser/, not toolkit/, so can't reuse it.


I moved some chrome files from extensions/xforms to
extensions/xforms/resources/content (like xforms.css) to make it cleaner.

Comment 4

[Doron Rosenberg (IBM)]

Pref is called xforms.crossdomain.enabled and has to be set to true.

To manually modify the host perm, you can now add:

host    xforms-xd       1       goat.austin.ibm.com

Comment 5

[Allan Beaufour]

Bug 295613 is checked in now, but the preferences does not seem to appear in the
Prefereces dialog?

Comment 6

[Doron Rosenberg (IBM)]

Hmm, I applied the patch to a firefox tree and it worked fine for me...

Comment 7

[Allan Beaufour]

(In reply to comment #6)
> Hmm, I applied the patch to a firefox tree and it worked fine for me...

I can see the pref. now too, dunno what happened. But it does not save the
state? If I open the preferences window and check "Allow XForms to ...", click
"Close" and then re-open the preferences window it's not checked anymore?

Comment 8

[Doron Rosenberg (IBM)]

Attachment: v2

Fixed the preference panel and included the submission code to call the
permission manager.

I also moved 2 files from extensions/xforms to
extensions/xforms/resources/content - xforms.css and contents.rdf.  I also
updated jar.mn to create chrome.manifest information for us for the XPI
automatically.

Comment 9

[Allan Beaufour]

(In reply to comment #8)
> Created an attachment (id=186616) [edit]
> v2

Sorry for my delay in this, but xforms.dtd is missing and
nsXFormsSubmissionElement.cpp is there twice...

Comment 10

[Doron Rosenberg (IBM)]

Attachment: all files this time I hope

Comment 11

[Allan Beaufour]

Comment on attachment 186951 [details] [diff] [review]
all files this time I hope

>Index: extensions/xforms/nsXFormsSubmissionElement.cpp
>===================================================================
>+    nsCOMPtr<nsIPrefBranch> prefBranch =
>+      do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
>+
>+    PRUint32 permission = 0;

Should use nsIPermissionManager::UNKNOWN_ACTION.

>+    if (permission != 1) {

Should use nsIPermissionManager::ALLOW_ACTION


>Index: extensions/xforms/package/Makefile.in
>===================================================================
>+	$(NSINSTALL) $(srcdir)/chrome.manifest stage/xforms

Is chrome.manifest created automatically?

>Index: extensions/xforms/resources/locale/en-US/xforms.properties
>===================================================================
> labelLinkLoadOrigin  = XForms Error (17): Security check failed! Trying to load label data from a different domain than document
> labelLink1Error      = XForms Error (18): External file (%S) for Label element not found
> labelLink2Error      = XForms Error (19): Failed to load Label element from external file: %S

Could you add some comments before the error messages... something like "#
Error Messages:", and before this "# Permission Preference:".

>+
>+xformsXDPermissionDialogTitle = Allowed Sites - XForms Cross Domain Access
>+xformsXDPermissionDialogIntro = You can specify which web sites containing XForms may access content on other domains.  Type the exact address of the site you want to allow and then press Allow.

* What does exact address mean? Will http://foo.com/ allow all content from
foo.com, and does http://foo.com/bar/ restrict to foo.com/bar?

* what does "access content" exactly mean. I read "get content", not submitting
data.

>+++ extensions/xforms/resources/content/contents.rdf	2005-01-27 18:48:32.000000000 -0600

Licence it.

>+++ extensions/xforms/resources/content/xforms-prefs.xul	2005-05-26 14:01:14.000000000 -0500

Licence it.

>+++ extensions/xforms/resources/content/xforms-prefs-ui.xul	2005-06-17 11:22:22.000000000 -0500

Licence it.

>+++ extensions/xforms/resources/content/xforms-prefs.js	2005-06-17 11:23:19.000000000 -0500

Licence it.

>+var xformsUI = new XFormsUIClass();

I guess this will not fail?

>+++ extensions/xforms/resources/locale/en-US/xforms.dtd	2005-05-26 12:18:24.000000000 -0500

License?

>+<!ENTITY xforms.crossdomain.ui.label "Allow XForms to access cross-domain content">

Again, "access cross-domain content" ... what does that mean?

And how about instance data loading across domains? And what about replacing?
What's the plan for that, and what's the implications?

I think we need a security review, and somebody who knows toolkit too.

r=me with fixes for the above and more (better) reviews, but create a bug for
instance loading etc.

Comment 12

[Doron Rosenberg (IBM)]

> 
> >Index: extensions/xforms/package/Makefile.in
> >===================================================================
> >+	$(NSINSTALL) $(srcdir)/chrome.manifest stage/xforms
> 
> Is chrome.manifest created automatically?
>
jar.mn will create it, right.
 
> >Index: extensions/xforms/resources/locale/en-US/xforms.properties
> >===================================================================
> > labelLinkLoadOrigin  = XForms Error (17): Security check failed! Trying to
load label data from a different domain than document
> > labelLink1Error      = XForms Error (18): External file (%S) for Label
element not found
> > labelLink2Error      = XForms Error (19): Failed to load Label element from
external file: %S
> 
> Could you add some comments before the error messages... something like "#
> Error Messages:", and before this "# Permission Preference:".
> 
> >+
> >+xformsXDPermissionDialogTitle = Allowed Sites - XForms Cross Domain Access
> >+xformsXDPermissionDialogIntro = You can specify which web sites containing
XForms may access content on other domains.  Type the exact address of the site
you want to allow and then press Allow.
> 
> * What does exact address mean? Will http://foo.com/ allow all content from
> foo.com, and does http://foo.com/bar/ restrict to foo.com/bar?
> 
> * what does "access content" exactly mean. I read "get content", not submitting
> data.

I was using the existing wording convention.  Permission Manager is domain based
- so foo.com/bar means foo.com

> >+<!ENTITY xforms.crossdomain.ui.label "Allow XForms to access cross-domain
content">
> 
> Again, "access cross-domain content" ... what does that mean?
> 
> And how about instance data loading across domains? And what about replacing?
> What's the plan for that, and what's the implications?
> 
> I think we need a security review, and somebody who knows toolkit too.
> 
> r=me with fixes for the above and more (better) reviews, but create a bug for
> instance loading etc.
> 

I am changing the code to only call the permission manager if
!mIsReplaceInstance, just like the file:// patch.

Will create one for instance loading as well.

I don't think we need a toolkit review, the code is pretty much basic stuff.

New patch coming in a sec with these and the permission manager change.

Comment 13

[Doron Rosenberg (IBM)]

Attachment: better patch

This patch:
- checks the permission manager if we are NOT replacing instance
- Renames the UI to make it clearer we are talking about submitting data to
other domains only (comments welcome)

Comment 14

[Doron Rosenberg (IBM)]

Attachment: trunk patch

Comment 15

[Doron Rosenberg (IBM)]

checked in