Closed
Bug 989806
Opened 11 years ago
Closed 11 years ago
Use reviewers certificate when installing apps from the Marketplace reviewer pages
Categories
(Core Graveyard :: DOM: Apps, defect)
Tracking
(firefox31 fixed, firefox32 fixed)
RESOLVED
FIXED
mozilla32
People
(Reporter: marco, Assigned: marco)
References
Details
Attachments
(3 files, 6 obsolete files)
(deleted),
application/zip
|
Details | |
(deleted),
patch
|
marco
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
We need to use the reviewers certificate when installing apps from the Marketplace reviewers page, so that reviewers can install apps and review them.
There's a discussion in bug 889744.
Assignee | ||
Comment 1•11 years ago
|
||
I've used some real apps downloaded from the marketplace, it would be better if we had some example apps.
Assignee: nobody → mar.castelluccio
Status: NEW → ASSIGNED
Attachment #8399174 -
Flags: review?(fabrice)
Assignee | ||
Comment 2•11 years ago
|
||
Fabrice, do you know who could provide sample apps signed by Marketplace?
Comment 3•11 years ago
|
||
Wil, can we get a sample "Hello World" app signed by the marketplace?
Flags: needinfo?(clouserw)
Assignee | ||
Comment 4•11 years ago
|
||
If possible, I'd need:
- a sample privileged app signed by Marketplace;
- a sample packaged app signed by Marketplace;
- a sample packaged app signed with the Marketplace reviewers certificate.
(The results are the same with either privileged or packaged apps, but I'd like to test them both for completeness)
Comment 5•11 years ago
|
||
privileged app signed by marketplace:
https://marketplace.firefox.com/downloads/file/210676/kitchensink-0.2.2.zip
sample packaged app signed by marketplace:
https://marketplace.firefox.com/downloads/file/227650/flashlight-linterna-2.0.zip
sample packaged app signed w/ reviewers cert:
bwalker recommends we don't link to this file in a public bug. I don't see you on IRC so I'll email the file to you.
Flags: needinfo?(clouserw)
Comment 6•11 years ago
|
||
Well, it's gonna end up in a public repo..,
Updated•11 years ago
|
Flags: needinfo?(bwalker)
Comment 7•11 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #6)
> Well, it's gonna end up in a public repo..,
The idea is that we're going to use this in an automated test, right? I guess putting that app here is OK; it's not as though we're revealing the private key we used to sign the app.
Flags: needinfo?(bwalker)
Comment 8•11 years ago
|
||
sample app signed by reviewer cert
Assignee | ||
Comment 9•11 years ago
|
||
Updated with the new sample apps (thank you!).
Attachment #8399174 -
Attachment is obsolete: true
Attachment #8399174 -
Flags: review?(fabrice)
Attachment #8410485 -
Flags: review?(fabrice)
Updated•11 years ago
|
Attachment #8410485 -
Flags: review?(fabrice) → review+
Assignee | ||
Comment 10•11 years ago
|
||
Updated commit message.
Attachment #8410485 -
Attachment is obsolete: true
Attachment #8411739 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 12•11 years ago
|
||
I've written a wrong bug number in the patch (as it was at the beginning a patch for bug 889744), so it's already landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/b589de6c1084
Comment 13•11 years ago
|
||
Backed out for failures in test_marketplace_pkg_install.html:
https://tbpl.mozilla.org/php/getParsedLog.php?id=38404634&tree=Mozilla-Inbound#error0
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5fa1fb553007
Comment 14•11 years ago
|
||
I pushed a version that calls SimpleTest.requestCompleteLog() to tryserver to see if it provides any useful context for the nsIPromptFactory::getPrompt failure that precedes the timeout:
https://tbpl.mozilla.org/?tree=Try&rev=c01b343bef8c
Assignee | ||
Comment 15•11 years ago
|
||
The failure looks similar to bug 948948.
Assignee | ||
Comment 16•11 years ago
|
||
It is failing right after the iframe is added to the DOM ("domParent.appendChild(ifr);" in the installApp function).
Assignee | ||
Comment 17•11 years ago
|
||
Ok, I've fixed the error by setting the "mozbrowser" property of the iframe to "true" and by adding the "browser" permission at the beginning of the test.
Assignee | ||
Comment 18•11 years ago
|
||
It is still failing on B2G Desktop, now because of another reason: the error page "The connection is untrusted" is shown.
Isn't this supposed to be solved by the new certs in build/pgo?
Assignee | ||
Comment 19•11 years ago
|
||
Here's the last version of the patch.
Fails on b2g with "mozbrowsererror: certerror".
I've found two tests that load an https page in an iframe: dom/browser-element/mochitest/browserElement_SecurityChange.js and dom/browser-element/mochitest/browserElement_ErrorSecurity.js.
The former is disabled on Android (bug 766586) and on B2G (no bug is referenced and there isn't a comment explaining why).
The latter is disabled on b2g, there's a comment that says: "Disable due to certificate issue (no bug that I'm aware of)".
I've just found bug 907770, as it turns out until a few days ago HTTPS hosts in b2g and Android mochitests weren't supported. I'm still experiencing the cert issue though.
Attachment #8411739 -
Attachment is obsolete: true
Assignee | ||
Comment 20•11 years ago
|
||
Could you review the mochitest.ini changes?
I think we can land this disabled on b2g until we fix the certificate issue.
Try run is green: https://tbpl.mozilla.org/?tree=Try&rev=546171fce13e
Attachment #8413189 -
Attachment is obsolete: true
Attachment #8413342 -
Flags: review?(ryanvm)
Comment 21•11 years ago
|
||
Comment on attachment 8413342 [details] [diff] [review]
Patch
Assuming your intent was disable the test on all B2G builds (desktop and emulator), this looks correct to me.
Attachment #8413342 -
Flags: review?(ryanvm) → feedback+
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #21)
> Comment on attachment 8413342 [details] [diff] [review]
> Patch
>
> Assuming your intent was disable the test on all B2G builds (desktop and
> emulator), this looks correct to me.
Yes, it has to be disabled on all b2g builds (as the dom/browser-element tests that involve HTTPS hosts).
Assignee | ||
Comment 23•11 years ago
|
||
Rebased (there was a conflict in mochitest.ini).
Attachment #8413342 -
Attachment is obsolete: true
Attachment #8413429 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 24•11 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Comment 25•11 years ago
|
||
Any reason the test wasn't added to the manifest alphabetically?
Assignee | ||
Comment 26•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #25)
> Any reason the test wasn't added to the manifest alphabetically?
No reason, I didn't know there was a ordering rule.
Comment 27•11 years ago
|
||
Just good practice :)
Comment 28•11 years ago
|
||
Sorry, I had to back this out for Android test failures :(
https://hg.mozilla.org/mozilla-central/rev/4d926af89907
https://tbpl.mozilla.org/php/getParsedLog.php?id=38597434&tree=Mozilla-Inbound
Flags: in-testsuite+
Assignee | ||
Comment 29•11 years ago
|
||
It's failing because of the certificate issue on Android too... But it is an intermittent failure (and it looks like 2.2 specific).
I wonder if this is a problem with the mochitest suite or a problem with the mozbrowser iframe.
Assignee | ||
Comment 30•11 years ago
|
||
Looks like tests using mozbrowser may be unreliable on Android (see bug 855543 comment 21).
Let's land with the test disabled on both b2g and Android.
Attachment #8413429 -
Attachment is obsolete: true
Attachment #8413853 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 31•11 years ago
|
||
Keywords: checkin-needed
Comment 32•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 33•11 years ago
|
||
Can we have this patch uplifted to Aurora? Reviewing app submissions on Nightly is a little risky (we might reject for a transient platform bug) and we'd like to get a working solution in place as soon as possible.
Assignee | ||
Comment 34•10 years ago
|
||
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 896620
User impact if declined: Reviewers will not be able to install apps from Marketplace reviewers page (and so will not be able to review apps).
Testing completed (on m-c, etc.): This has been on m-c for a week or so, it landed right after the mozilla-central -> mozilla-aurora transition.
Risk to taking this patch (and alternatives if risky): The actual code change is small, the patch looks big because it contains test apps and new mochitest certificates for a new HTTPS host.
String or IDL/UUID changes made by this patch: None.
Attachment #8418940 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox31:
--- → affected
status-firefox32:
--- → fixed
Updated•10 years ago
|
Attachment #8418940 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 35•10 years ago
|
||
Updated•7 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•