Need preference for whitelisting submission, instance loading, etc.

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: aaronr, Assigned: doronr)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

14 years ago
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"
(Reporter)

Comment 1

14 years ago
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.
(Reporter)

Comment 2

13 years ago
We need to make sure that there is a way to allow for submitting to a webservice
from a form loaded locally, too.
Assignee: aaronr → doronr
(Assignee)

Comment 3

13 years ago
Created attachment 184703 [details] [diff] [review]
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.
(Assignee)

Comment 4

13 years ago
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

13 years ago
Bug 295613 is checked in now, but the preferences does not seem to appear in the
Prefereces dialog?
(Assignee)

Comment 6

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

Comment 7

13 years ago
(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?
(Assignee)

Comment 8

13 years ago
Created attachment 186616 [details] [diff] [review]
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.
Attachment #184703 - Attachment is obsolete: true
Attachment #186616 - Flags: review?(allan)

Comment 9

13 years ago
(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...
(Assignee)

Updated

13 years ago
Attachment #186616 - Attachment is obsolete: true
Attachment #186616 - Flags: review?(allan)
(Assignee)

Comment 10

13 years ago
Created attachment 186951 [details] [diff] [review]
all files this time I hope
Attachment #186951 - Flags: review?(allan)

Comment 11

13 years ago
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.
Attachment #186951 - Flags: review?(allan) → review+
(Assignee)

Comment 12

13 years ago
> 
> >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.

Status: NEW → ASSIGNED
(Assignee)

Comment 13

13 years ago
Created attachment 187033 [details] [diff] [review]
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)
Attachment #187033 - Flags: review?(smaug)
(Assignee)

Comment 14

13 years ago
Created attachment 187417 [details] [diff] [review]
trunk patch
Attachment #187033 - Attachment is obsolete: true
Attachment #187417 - Flags: review?(smaug)
(Assignee)

Updated

13 years ago
Attachment #187033 - Flags: review?(smaug)

Updated

13 years ago
Attachment #187417 - Flags: review?(smaug) → review+
(Assignee)

Comment 15

13 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.