1170851 - Warn about add-ons detected as no longer signed during the periodic check

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Add-on Manager, defect)

Description

[:Margaret Leibovic]

Android version of bug 1151509.

Comment 1

[:Margaret Leibovic]

Mossop, what's a good way to test this? Could I write a test that installs an unsigned add-on, then does something to change the fact that we require signed add-ons and force a check?

One tricky thing with our robocop test harness is that we can't restart the browser, so this would all need to happen during a single running instance.

I tried testing this manually by installing an unsigned add-on, applying a patch that requires add-on signing, and then rebuilding/reinstalling. The first time I installed my new build, nothing looked different, but then after killing it and restarting, I found the add-on was disabled.

I also tried adding an "xpi-signature-changed" observer, but that never fired, so I think I just ran into the startup case that I'll need to address in bug 1170846.

Comment 2

[Dave Townsend [:mossop]]

(In reply to :Margaret Leibovic from comment #1)
> Mossop, what's a good way to test this? Could I write a test that installs
> an unsigned add-on, then does something to change the fact that we require
> signed add-ons and force a check?
> 
> One tricky thing with our robocop test harness is that we can't restart the
> browser, so this would all need to happen during a single running instance.

What you want to do is turn on signing requirements then install a signed add-on. Then break the add-on in some way, add a file to its XPI or something. Then trigger the background check and you should get the notification.

> I tried testing this manually by installing an unsigned add-on, applying a
> patch that requires add-on signing, and then rebuilding/reinstalling. The
> first time I installed my new build, nothing looked different, but then
> after killing it and restarting, I found the add-on was disabled.
> 
> I also tried adding an "xpi-signature-changed" observer, but that never
> fired, so I think I just ran into the startup case that I'll need to address
> in bug 1170846.

Yeah that sounds likely.

Comment 3

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1170851 - Warn about add-ons detected as no longer signed. r=mfinkle,Mossop

Bug 1170851 - Warn about add-ons detected as no longer signed. r=mfinkle,Mossop

Comment 4

[:Margaret Leibovic]

I decided to just add the startup check needed for bug 1170851 while I'm here, although I had trouble figuring out how to test that code path...

To test the xpi-signature changed path, I did the following:

1) Set add-on signing required
2) Installed a signed add-on (ABP)
3) Ran this script to copy a modified version of the XPI from the sdcard to the extensions directory in the profile (and also force the signature check): https://gist.github.com/leibovic/4f30e00461304886d41b

Comment 5

[:Margaret Leibovic]

Attachment: screenshot

Comment 6

[:Margaret Leibovic]

(antlam and I talked about this on IRC, and I got the thumbs up there)

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8621322 [details]
MozReview Request: Bug 1170851 - Warn about add-ons detected as no longer signed. r=mfinkle,Mossop

https://reviewboard.mozilla.org/r/10947/#review9579

::: mobile/android/chrome/content/browser.js:6302
(Diff revision 1)
> +        // TODO: Open about:addons to show only unsigned add-ons?

Do you want to file a bug on adding this functionality to about:addons ?

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8621322 [details]
MozReview Request: Bug 1170851 - Warn about add-ons detected as no longer signed. r=mfinkle,Mossop

https://reviewboard.mozilla.org/r/10947/#review9581

Ship It!

Comment 9

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> Comment on attachment 8621322 [details]
> MozReview Request: Bug 1170851 - Warn about add-ons detected as no longer
> signed. r=mfinkle,Mossop
> 
> https://reviewboard.mozilla.org/r/10947/#review9579
> 
> ::: mobile/android/chrome/content/browser.js:6302
> (Diff revision 1)
> > +        // TODO: Open about:addons to show only unsigned add-ons?
> 
> Do you want to file a bug on adding this functionality to about:addons ?

Yeah, I can file a bug. Right now about:addons 1) doesn't know about signed/unsigned add-ons (bug 1170841) and 2) doesn't have a way to handle URL parameters (bug 1173893).

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8621322 [details]
MozReview Request: Bug 1170851 - Warn about add-ons detected as no longer signed. r=mfinkle,Mossop

https://reviewboard.mozilla.org/r/10947/#review9621

Ship It!

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e2566fef5f24

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e2566fef5f24