1132521 - Mozmill add-on tests are broken because installing via an unsecure connection is not allowed anymore

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Bug 1128126 changed the way when extensions can be installed. So by default only the HTTPS schema is allowed now in latest Nightly builds since Feb 11th.

To allow our tests to install those add-ons for testing, we have to set "extensions.install.requireSecureOrigin" to false. This should be done for all the tests which make use of localhost. For tests against mozqa.com we should change them to HTTPS.

Comment 1

[Teodor Druta]

Attachment: fix_addons_modal.patch

This patch should fix all the add-on tests.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It looks like the feature as implemented to secure the addon installation is not complete. There is one use case which let the user still install extensions via an unsafe connection. That is the reason why not all of our addon tests are affected!

So we see the notification when clicking on an XPI link or using the install trigger method. But not when directly loading http://mozqa.com/data/firefox/addons/extensions/icons.xpi.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8563504 [details] [diff] [review]
fix_addons_modal.patch

Review of attachment 8563504 [details] [diff] [review]:
-----------------------------------------------------------------

We have to fix all the tests. I have a patch nearly ready.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: fix permissions for non HTTPS sources

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8563527 [details] [diff] [review]
fix permissions for non HTTPS sources

Review of attachment 8563527 [details] [diff] [review]:
-----------------------------------------------------------------

So it looks like that we do not need the changes for tests which do directly load the URL. To be sure that this is not getting regressed I will revert my changes, or better we use the trigger all the time. I will come with an update later.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: fix permissions for non HTTPS sources v2

So this fixes the problem by changing all install sources to use the XPI trigger and white-listing the URL beside allowing an installation via HTTP from localhost. A direct install is being tested in the remote addon install via FTP.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8563663 [details] [diff] [review]
fix permissions for non HTTPS sources v2

Review of attachment 8563663 [details] [diff] [review]:
-----------------------------------------------------------------

::: firefox/tests/remote/testAddons/testInstallAddonFromFTP.js
@@ +27,1 @@
>    prefs.setPref(PREF_INSTALL_DIALOG, INSTALL_DIALOG_DELAY);

Is this block necessary or incomplete? Changes to this test seem inconsistent with the others.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8563663 [details] [diff] [review]
fix permissions for non HTTPS sources v2

Review of attachment 8563663 [details] [diff] [review]:
-----------------------------------------------------------------

::: firefox/tests/remote/testAddons/testInstallAddonFromFTP.js
@@ +27,1 @@
>    prefs.setPref(PREF_INSTALL_DIALOG, INSTALL_DIALOG_DELAY);

This is a remote test and triggers the add-on installation by directly loading the XPI URL. In such a case there is no security check performed, and we also don't need a white-listing of the host.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://hg.mozilla.org/qa/mozmill-tests/rev/cc4868faaff9

Comment 10

[Teodor Druta]

(In reply to Henrik Skupin (:whimboo) from comment #9)
> https://hg.mozilla.org/qa/mozmill-tests/rev/cc4868faaff9

I think this patch misses the fix for "remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/" test.

Comment 11

[Daniela Domnici]

This test(/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js) is still failing.Here are the reports:

 http://mozmill-daily.blargon7.com/#/remote/failure?app=All&branch=All&platform=All&from=2015-02-13&to=2015-02-13&test=%2FrestartTests%2FtestAddons_RestartlessExtensionWorksAfterRestart%2Ftest1.js&func=testInstallRestartlessExtension

Comment 12

[Daniela Domnici]

Also failed in the following tests: /testAddons/testInstallUninstallSoftBlocklistedExtension.js

Reports: http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2015-02-13&to=&test=%2FtestAddons%2FtestInstallUninstallSoftBlocklistedExtension.js&func=testInstallBlocklistedExtension 

And : /testAddons/testUninstallExtension.js | testInstallExtensions

Reports: http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2015-02-13&to=2015-02-13&test=%2FtestAddons%2FtestUninstallExtension.js&func=testInstallExtensions

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Daniela Domnici from comment #12)
> Also failed in the following tests:
> /testAddons/testInstallUninstallSoftBlocklistedExtension.js
> 
> And : /testAddons/testUninstallExtension.js | testInstallExtensions

Those seem to be unrelated. They have my fix included and I do not see anything wrong locally. They work like they should. Maybe that is due to bug 1132916.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: follow-up v1

Comment 15

[Mihaela Velimiroviciu (:mihaelav)]

Comment on attachment 8564153 [details] [diff] [review]
follow-up v1

Review of attachment 8564153 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed follow-up patch: https://hg.mozilla.org/qa/mozmill-tests/rev/e6e9d6e6d546 (default)

We need a backport here for aurora because the change will be backported soon.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: backport (aurora) v1

This is a combined patch for the backport of the patch to the Aurora branch. It has to land once the patch on bug 1128126 has been backported, which will happen soon.

Comment 18

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8564184 [details] [diff] [review]
backport (aurora) v1

Review of attachment 8564184 [details] [diff] [review]:
-----------------------------------------------------------------

::: firefox/tests/remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js
@@ +49,5 @@
>  /**
>   * Test installing a restartless addon
>   */
>  function testInstallRestartlessExtension() {
>    persisted.addon = ADDON;

This file is the only difference between this patch and the one in comment 6. Is it that this test was removed for more recent versions of Firefox?

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8564184 [details] [diff] [review]
backport (aurora) v1

Review of attachment 8564184 [details] [diff] [review]:
-----------------------------------------------------------------

::: firefox/tests/remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js
@@ +49,5 @@
>  /**
>   * Test installing a restartless addon
>   */
>  function testInstallRestartlessExtension() {
>    persisted.addon = ADDON;

Please see the follow-up patch I had to do today. There was one test hiding under remote/restartTests which I wasn't aware of that it is using the local httpd server.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I decided to already push this fix to aurora, given that it doesn't make any harm to us. The preference is simply not in use yet. Otherwise we could have expected lots of test failures the next days, until I'm back on Wednesday.

https://hg.mozilla.org/qa/mozmill-tests/rev/9e3ce6ec1f1e (aurora)

I will keep the bug open until it's clear if this patch will land on beta or not.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The underlying patch for Firefox will not land for the 36.0 release. So all done here.