Open Bug 1298630 Opened 6 years ago Updated 3 years ago

New exception setting is gone for install add-ons after restarting

Categories

(Core :: Permission Manager, defect, P3)

defect

Tracking

()

Tracking Status
firefox48 --- affected
firefox49 --- affected
firefox-esr45 --- affected
firefox50 --- affected
firefox51 --- affected
firefox52 --- wontfix

People

(Reporter: magicp.jp, Unassigned, NeedInfo)

References

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160827030436

Steps to reproduce:

a-1. Start Nightly with new profile
a-2. Go to "https://addons.mozilla.org" and check the site info icon
a-3. Open menu > Options > Security
a-4. Click [Exceptions...] button in General
a-5. Confirm allowed sites for install add-ons
     "https://addons.mozilla.org" "Allow"
     "https://testpilot.firefox.org" "Allow"
a-6. Remove All Sites
a-7. Save Changes
a-8. Close Options

b-1. Restart Nightly
b-2. Go to "https://addons.mozilla.org" and check the site info icon
b-3. Open menu > Options > Security
b-4. Click [Exceptions...] button in General
b-5. Confirm allowed sites for install add-ons
      (Nothing)
b-6. Add allowed site "https://addons.mozilla.org"
b-7. Save Changes
b-8. Close Options

c-1. Restart Nightly
c-2. Go to "https://addons.mozilla.org" and check the site info icon
c-3. Open menu > Options > Security
c-4. Click [Exceptions...] button in General
c-5. Confirm allowed sites for install add-ons


Actual results:

In step c-5,  "https://addons.mozilla.org" is gone, and also "https://testpilot.firefox.org" is coming on its own.


Expected results:

The exception settings works correctly.
Has STR: --- → yes
Component: Untriaged → Security
OS: Unspecified → All
Hardware: Unspecified → All
Too late for firefox 52, mass-wontfix.
In that regression range probably Bug 1050080. Any ideas, Mark?
Blocks: 1050080
Component: Security → Add-ons Manager
Flags: needinfo?(markh)
Product: Firefox → Toolkit
Vlad, can you check if this issue still happens on release?
Flags: needinfo?(vlad.jiman)
Hello, I've verified using the above steps on: FF 51, Nightly 64.0a1 and the release version 62.0.3. On all of these the issue appears to reproduce, given that we remove the exception and upon restarting a link "https://testpilot.firefox.org" is added, also in some cases if we save the exception, upon restart it will be gone, I've attached videos of each of these problems. Tests were performed using Windows 10 x64 and new FF profiles on each version.
Flags: needinfo?(vlad.jiman)
Attached video Saving a new exception
I'm able to reproduce this but only inconsistently.  In any case, I think its an issue with site permissions, I think this is the right component for those.
Component: Add-ons Manager → Site Identity and Permission Panels
Product: Toolkit → Firefox
Yeah, this could be a bug in the import code, probably needs a lot deeper investigation.

Andrew, there's something else I noticed. addons.mozilla.org and testpilot.firefox.com seem to be able to install add-ons just fine without the "install" permission for me. Is that permission still needed?
Component: Site Identity and Permission Panels → Permission Manager
Flags: needinfo?(aswan)
Priority: -- → P3
Product: Firefox → Core
(In reply to Johann Hofmann [:johannh] from comment #9)
> Andrew, there's something else I noticed. addons.mozilla.org and
> testpilot.firefox.com seem to be able to install add-ons just fine without
> the "install" permission for me. Is that permission still needed?

Both those sites have access to two different APIs: InstallTrigger which is controlled by site permissions, and mozAddonManager which is not.  It sounds like both sites have moved over to using mozAddonManager instead of InstallTrigger.  Perhaps we should remove the default xpinstall site permissions if those sites aren't using InstallTrigger any more.  Jared, Stuart, any objection?

One note, there are some (undocumented) preferences that users can flip that disable mozAddonManager and these sites might use InstallTrigger as a fallback in that case, but I don't think we need to worry about it -- InstallTrigger will still work but with one extra propmt.
Flags: needinfo?(scolville)
Flags: needinfo?(jhirsch)
Flags: needinfo?(aswan)
Once the shield study on the disco pane concludes (Oct 29th) we'll be switching to the unified mozAddonManager powered button that's already in use on AMO. When that happens all install buttons across AMO and disco will have a direct install of the XPI as a fallback (no InstallTrigger).
Flags: needinfo?(scolville)
Test Pilot relies on mozAddonManager only. This is the first I've even heard of InstallTrigger.
Flags: needinfo?(jhirsch)
Thank you!

(In reply to Stuart Colville [:scolville] [:muffinresearch] from comment #11)
> Once the shield study on the disco pane concludes (Oct 29th) we'll be
> switching to the unified mozAddonManager powered button that's already in
> use on AMO. When that happens all install buttons across AMO and disco will
> have a direct install of the XPI as a fallback (no InstallTrigger).

So that means we can remove this permission after October 29th? Is there a bug for the shield study that we can link this bug to?
(In reply to Johann Hofmann [:johannh] from comment #13)
> Thank you!
> 
> (In reply to Stuart Colville [:scolville] [:muffinresearch] from comment #11)
> > Once the shield study on the disco pane concludes (Oct 29th) we'll be
> > switching to the unified mozAddonManager powered button that's already in
> > use on AMO. When that happens all install buttons across AMO and disco will
> > have a direct install of the XPI as a fallback (no InstallTrigger).
> 
> So that means we can remove this permission after October 29th? Is there a
> bug for the shield study that we can link this bug to?

Not directly, but if you follow this issue that's should be close enough https://github.com/mozilla/addons-frontend/issues/6539
Stuart, sorry to needinfo you again, but do we have confirmation now that the InstallTrigger permissions are no longer needed?
Flags: needinfo?(scolville)
(In reply to Johann Hofmann [:johannh] from comment #15)
> Stuart, sorry to needinfo you again, but do we have confirmation now that
> the InstallTrigger permissions are no longer needed?

The buttons on disco pane are now live, so the only place that InstallTrigger continues to exist is the old AMO frontend. Here: https://github.com/mozilla/addons-server/blob/dce35f1d21380c806855ae9496dc828b190cc3a3/static/js/zamboni/buttons.js#L309

Unfortunately we're not yet ready to remove the old frontend (It's likely to happen in Q1 next year though - so would you be able to wait until then?) If not we'd need to remove installTrigger from the old frontend and start using mozAddonManager there too to avoid breaking people. Or alternatively switch to a link which would mostly work except cases where MITM proxies are used by end users since that breaks installs as the MITM cert isn't trusted for add-on installs. However, getting people to use the new frontend as a workaround might be ok, though I'd have to check with product for that.
Flags: needinfo?(scolville)
(In reply to Stuart Colville [:scolville] [:muffinresearch] from comment #16)
> (In reply to Johann Hofmann [:johannh] from comment #15)
> > Stuart, sorry to needinfo you again, but do we have confirmation now that
> > the InstallTrigger permissions are no longer needed?
> 
> The buttons on disco pane are now live, so the only place that
> InstallTrigger continues to exist is the old AMO frontend. Here:
> https://github.com/mozilla/addons-server/blob/
> dce35f1d21380c806855ae9496dc828b190cc3a3/static/js/zamboni/buttons.js#L309
> 
> Unfortunately we're not yet ready to remove the old frontend (It's likely to
> happen in Q1 next year though - so would you be able to wait until then?) If
> not we'd need to remove installTrigger from the old frontend and start using
> mozAddonManager there too to avoid breaking people. Or alternatively switch
> to a link which would mostly work except cases where MITM proxies are used
> by end users since that breaks installs as the MITM cert isn't trusted for
> add-on installs. However, getting people to use the new frontend as a
> workaround might be ok, though I'd have to check with product for that.

Is the old frontend still used, then? If so then we can just block this bug on that. Otherwise (if there are no users, you just haven't removed it yet) it's fine to remove the permission since that shouldn't actually break the API, it just adds an additional permission prompt in front of it, AFAIK.
Flags: needinfo?(scolville)
(In reply to Johann Hofmann [:johannh] from comment #17)
> (In reply to Stuart Colville [:scolville] [:muffinresearch] from comment #16)
> > (In reply to Johann Hofmann [:johannh] from comment #15)
> > > Stuart, sorry to needinfo you again, but do we have confirmation now that
> > > the InstallTrigger permissions are no longer needed?
> > 
> > The buttons on disco pane are now live, so the only place that
> > InstallTrigger continues to exist is the old AMO frontend. Here:
> > https://github.com/mozilla/addons-server/blob/
> > dce35f1d21380c806855ae9496dc828b190cc3a3/static/js/zamboni/buttons.js#L309
> > 
> > Unfortunately we're not yet ready to remove the old frontend (It's likely to
> > happen in Q1 next year though - so would you be able to wait until then?) If
> > not we'd need to remove installTrigger from the old frontend and start using
> > mozAddonManager there too to avoid breaking people. Or alternatively switch
> > to a link which would mostly work except cases where MITM proxies are used
> > by end users since that breaks installs as the MITM cert isn't trusted for
> > add-on installs. However, getting people to use the new frontend as a
> > workaround might be ok, though I'd have to check with product for that.
> 
> Is the old frontend still used, then? If so then we can just block this bug
> on that. Otherwise (if there are no users, you just haven't removed it yet)
> it's fine to remove the permission since that shouldn't actually break the
> API, it just adds an additional permission prompt in front of it, AFAIK.

We default to the new site but the old site is available optionally. I don't think having an additional dialogue should be a problem but Product (:jorgev) should have input on that.
Flags: needinfo?(scolville) → needinfo?(jorge)
That should be fine for users of the old front-end.
Flags: needinfo?(jorge)
Depends on: 1530371
You need to log in before you can comment on or make changes to this bug.