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

RESOLVED FIXED in Firefox 37

Status

Mozilla QA
Mozmill Tests
P1
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

({regression})

unspecified
mozilla38
regression
Dependency tree / graph

Firefox Tracking Flags

(firefox35 unaffected, firefox36 unaffected, firefox37 fixed, firefox38 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

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.
Blocks: 1087191

Comment 1

3 years ago
Created attachment 8563504 [details] [diff] [review]
fix_addons_modal.patch

This patch should fix all the add-on tests.
Attachment #8563504 - Flags: review?(mihaela.velimiroviciu)
Attachment #8563504 - Flags: review?(hskupin)
Attachment #8563504 - Flags: review?(andreea.matei)
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 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.
Attachment #8563504 - Flags: review?(mihaela.velimiroviciu)
Attachment #8563504 - Flags: review?(hskupin)
Attachment #8563504 - Flags: review?(andreea.matei)
Attachment #8563504 - Flags: review-
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Created attachment 8563527 [details] [diff] [review]
fix permissions for non HTTPS sources
Attachment #8563504 - Attachment is obsolete: true
Attachment #8563527 - Flags: review?(cmanchester)
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.
Attachment #8563527 - Flags: review?(cmanchester)
Created attachment 8563663 [details] [diff] [review]
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.
Attachment #8563527 - Attachment is obsolete: true
Attachment #8563663 - Flags: review?(cmanchester)
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.
Attachment #8563663 - Flags: review?(cmanchester) → review+
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.
https://hg.mozilla.org/qa/mozmill-tests/rev/cc4868faaff9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38

Comment 10

3 years ago
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

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

3 years ago
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
(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.
Created attachment 8564153 [details] [diff] [review]
follow-up v1
Attachment #8564153 - Flags: review?(mihaela.velimiroviciu)
Comment on attachment 8564153 [details] [diff] [review]
follow-up v1

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

Looks fine to me.
Attachment #8564153 - Flags: review?(mihaela.velimiroviciu) → review+
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.
Status: REOPENED → ASSIGNED
status-firefox37: --- → affected
Created attachment 8564184 [details] [diff] [review]
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.
Attachment #8564184 - Flags: review?(cmanchester)
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?
Attachment #8564184 - Flags: review?(cmanchester) → review+
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.
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.
status-firefox35: --- → unaffected
status-firefox36: --- → ?
status-firefox37: affected → fixed
The underlying patch for Firefox will not land for the 36.0 release. So all done here.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-firefox36: ? → unaffected
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.