Closed Bug 241705 Opened 20 years ago Closed 19 years ago

XPInstall Permission Manager UI

Categories

(Firefox :: General, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox1.0beta

People

(Reporter: bugs, Assigned: bugs)

References

Details

(Keywords: fixed-aviary1.0)

Attachments

(2 files, 2 obsolete files)

This is the front end component of bug 240552. Provide a UI for manipulating the
list of whitelisted install sites.
Status: NEW → ASSIGNED
Depends on: 240552
Flags: blocking1.0+
Priority: -- → P3
Target Milestone: --- → Firefox0.9
*** Bug 246131 has been marked as a duplicate of this bug. ***
To be explicit (bug 246131) part of this UI will of course include a "blocked
install" indicator something like the blocked popup one.
Depends on: 246131
Suggest the UI be an addtion to the Software Installation pane at
Edit>Preferences>Advanced>Software Installation.  Alternatively, the UI should
be launched from a button on that pane.  
Priority: P3 → P2
Target Milestone: Firefox0.9 → Firefox1.0beta
http://www.pryan.org/mozilla/site/TheOneKEA/extensions/xpiwhitelist_02.xpi

A credible attempt at UI. I added an overlay to the Advanced preference panel's
Software tree which adds a button entitled "Modify Whitelist..." - this, along
with the EM entry, provides access to the UI.

Basically all I have is a checkbox for xpinstall.whitelist.required and a
dynamic listbox for xpinstall.whitelist.add. Simple, but it works.

I can try patching it into the chrome, if anyone is interested.
Flags: blocking-aviary1.0RC1+
Attached patch part 1 (obsolete) — — Splinter Review
... notification area.
Attached patch patch (obsolete) — — Splinter Review
more progress... remaining work: clean up and the exceptions dialog for
options.
Attachment #152236 - Attachment is obsolete: true
Oh cool! You're going to overlay it right on top of the page!

BTW, v0.3.1 of my extension is available now on its own homepage, for anyone
interested in using it.

http://www.pryan.org/mozilla/site/TheOneKEA/xpiwhitelist/
Attached patch full diffs — — Splinter Review
Attachment #152486 - Attachment is obsolete: true
Comment on attachment 152588 [details] [diff] [review]
xpinstall patch

this basically adds observer notifications in three places:

1) the content handler for when an XPI link is clicked
2) InstallTrigger.install
3) InstallTrigger.installChrome

it passes the associated docshell as aSubject, so the FE can locate the
window/tab to show the notification above.
Attachment #152588 - Flags: review?(dveditz)
Judging from how many clueless IE users handle ActiveX installation dialogs, it
might be a good idea to not even display this dialog/toolbar if a site has bad
track record. This can be done using a predefined list of blacklisted domains
(not that there are any at the moment). 

For reference, you can find a very extensive list of IE-hostile sites here:
https://netfiles.uiuc.edu/ehowes/www/res/ie-spyad.zip

Prog.
See Bug 250445 for a bug with this feature, and Bug 250430, Bug 250431 & Bug
250423 for possible regressions (not so sure about the last one).
Whiteboard: fixed-aviary1.0
(In reply to comment #14)
> See Bug 250445 for a bug with this feature

This bug talks about Firefox using wrong terminology ("theme" instead of
"extension"). It does not suggest blacklisting domains (predefined or not). It
does mention *whitelisting*, but this is a completely different concept.

Are there any other bugs that might fit my suggestion?

Prog.
See also Bug 250488 for another regression.

p.s. Prog, my comment 14 was not in reply to your comment 13, it was just a list
of some problems caused by the check-in of this feature :-)
I've separated the request for blacklist functionality to another bug:
http://bugzilla.mozilla.org/show_bug.cgi?id=250854

Prog.
Comment on attachment 152588 [details] [diff] [review]
xpinstall patch

>@@ -265,17 +266,22 @@ nsInstallTrigger::HandleContent(const ch
>-        // TODO: fire event signaling blocked Install attempt
>+        nsCOMPtr<nsIObserverService> os(do_GetService("@mozilla.org/observer-service;1"));
>+        if (os) {
>+            os->NotifyObservers(globalObject->GetDocShell(), 
>+                                "xpinstall-install-blocked", 
>+                                NS_LITERAL_STRING("install-chrome").get());
>+        }

This should be "install" rather than install-chrome, HandleContent will only
get full .xpi's, never a theme install. Test by clicking on a .xpi link.

Could we instead pass the urispec in this argument instead? This would give the
information bar the information it would need to be able to install that one
item without making the user whitelist the site for the future.

>@@ -170,18 +172,25 @@ InstallTriggerGlobalInstall(JSContext *c
>@@ -326,18 +335,25 @@ InstallTriggerGlobalInstallChrome(JSCont

You also need this in InstallTriggerGlobalStartSoftwareUpdate

>-  if (!enabled || !globalObject)
>+  if (!enabled || !globalObject) {

nit: predominant style here is brace below if

>@@ -326,18 +335,25 @@ InstallTriggerGlobalInstallChrome(JSCont
>-  if (!enabled || !globalObject)
>+  if (!enabled || !globalObject) {

nit: ditto

>+      nsCOMPtr<nsIObserverService> os(do_GetService("@mozilla.org/observer-service;1"));
>+      if (os) {
>+          os->NotifyObservers(globalObject->GetDocShell(), 
>+                              "xpinstall-install-blocked", 
>+                              NS_LITERAL_STRING("install-chrome").get());
>+      }

You need to move the UpdateEnabled check after argument parsing -- if the type
is anything other than SKIN this should be exactly the same as the other
installs. Unless you change the 3rd parameter to be the urispec, in which case
you can lump chrome installs together since they can't be restarted from just
the urispec.

One thing that troubles me is that if the user has turned off software installs
entirely they'll still get these notifications. Could be left for later I
suppose, most people won't turn off installs. One solution might be to return
more than just a boolean from UpdateEnabled(), using different codes to
indicate whether the feature is off or the site was blocked.

Rather than sending a notification in 4 different places, could this be moved
inside AllowInstall() itself? Then we'd have to pass a flag into
UpdateEnabled() about whether we're trying an install, or just the web site
asking about it.

r- predominantly because of the missing startSoftwareUpdate spot and the
"install-chrome" in HandleContent. You probably don't have the bandwidth to
re-write this following the other suggestions and that's OK, I'll look into
that on the trunk.
Attachment #152588 - Flags: review?(dveditz) → review-
Some of the "clean up" that happened in pref-features.js actually changed the
image checkboxes worked and caused bug 252953.
sounds like this is not on the branch.  should fixed-aviary1.0 be removed from
status whiteboard?  if fixed it should be in the keyword field
I'm seeing it on the branch (20040725):

Tools->Options->Web Features->Allow web sites to install software->Exceptions
If I go Options->Web Features I see a box marked 'Allow Web sites to install
software'. Then I see a button marked exceptions. This implies that all websites
can install software (without permission?) EXCEPT those listed seperately  
which are blocked (although the manager is worded correctly). Same goes for load
images (although not under the scope of this bug).
Agreed, the UI is not intuitive. The wording does not even mention extensions,
so doesn't tie in very well with Tools->Extensions. "Allow web sites to install
software" could be any ol' software in the users mind (i.e. nasty executables).
Keywords: fixed-aviary1.0
Whiteboard: fixed-aviary1.0
Blocks: 250543
bug 250543 deals with the poor wording
The Risks Digest 23.49 contains an item at
<http://catless.ncl.ac.uk/Risks/23.49.html#subj9> cites a hostile exploit of the
problem this UI is intended to fix.  The exploit is detailed at
<http://www.squarefree.com/archives/000487.html>.  

I notice this bug is an Aviary blocker.  Does that include blocking Mozilla 1.8?
 If not, it should.  
I am not sure if this idea has been proposed, or if this is the best bug to
mention it in but... someone looking over my shoulder just mentioned that as you
exit after installing one or more extensions, Firefox should present a list of
the extensions that will be installed upon restart (and allow you to change your
mind by removing one or all from the list).  Of course this would be another
annoyance to people who install a lot of extensions but it might help people
notice and "undo" an undesired pending install.
Need to change the target milestone since the one there has now passed.
Branch checkin was 2004-07-08 00:54.

Trunk checkin was mostly with the aviary landing, with the exception of:
- mozilla/xpcom/windbgdlg/windbgdlg.cpp
- mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp
Keywords: aviary-landing
Blocks: 274271
Flags: blocking-aviary1.1?
(In reply to comment #28)
> Branch checkin was 2004-07-08 00:54.
> 
> Trunk checkin was mostly with the aviary landing, with the exception of:
> - mozilla/xpcom/windbgdlg/windbgdlg.cpp
> - mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp

Any idea on the ETA for those?
Flags: blocking-aviary1.1? → blocking-aviary1.1+
I guess this is fixed on trunk finally by bug 274271.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Removing aviary-landing keyword.
Keywords: aviary-landing
QA Contact: general
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: