1171792 - An update is attempted from an update notification when it is not possible for the logged in user to update

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Mark Gradisar]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150513174244

Steps to reproduce:

Used automated update pop-up. Mac OS v10.10.3


Actual results:

It appeared to download then requested Restart and I selected Restart Firefox.


Expected results:

New version of Firefox should have installed and started.  I had to manually start Firefox and it was the old version.

Comment 1

[Robert Strong (they/them - no direct email)]

First, please don't reinstall so we can troubleshoot exactly what is going on.

In the Mac finder and in your home directory please open Library/Caches/Mozilla/updates/Applications/Firefox/updates

In that directory there should be files named last-update.log and backup-update.log. Please attach them to this bug using the "Add an attachment" link.

Then look in the 0 directory. If there is a file named update.log please attach it to this bug as well. Thanks!

Comment 2

[Mark Gradisar]

Attachment: zipped last-update.log and backup-update.log

Requested files named last-update.log and backup-update.log in a zip file

Comment 3

[Robert Strong (they/them - no direct email)]

There was an error renaming Contents/_CodeSignature/CodeResources

Could you launch a terminal and cd into /Applications/Firefox.app/Contents/_CodeSignature/
Then type ls -l and copy the output and paste it into a comment in this bug.

For comparison purposes please cd into /Applications/Firefox.app/Contents/Resources/
Then type ls -l application.ini and paste it into a comment in this bug.

Thanks!

Comment 4

[Mark Gradisar]

Attachment: Archive.zip

Resources & Signature ls

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: last-update.log

Comment 6

[Robert Strong (they/them - no direct email)]

Thanks! I think I see what is going on and will have a Mac available tomorrow to verify if my suspicion is correct. Please don't re-install until after I verify it since I might need you to check something else.

Comment 7

[Robert Strong (they/them - no direct email)]

Hi Mark, I think I now have an understanding of the cases where this happens. Can you tell me what the about window displays under the Firefox menu? Thanks

Comment 8

[Mark Gradisar]

Attachment: Screen Shot 2015-06-07 at 7.17.12 AM.png

See attached 'about' window

Comment 9

[Robert Strong (they/them - no direct email)]

Thanks Mark. This bug has two parts. First, there is a permissions check that is missing in the update window code. Second, bug 394984 prevents people that don't have permissions from updating. I'll use this bug to address the first part. The second part has been difficult to fix due to a lack of resources. I've talked this over with a couple of people and I will hopefully be able to work on that bug later this month.

You can resolve this issue on your system by removing the existing install and reinstalling. Keep in mind (per bug 394984) that the person that performs the install will be the only person that can update until bug 394984 is fixed.

Comment 10

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Try push
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd68bce19842

Comment 11

[Robert Strong (they/them - no direct email)]

[Tracking Requested - why for this release]:
This prevents notifying the user that an update is available when they are not able to update.

Comment 12

[Robert Strong (they/them - no direct email)]

Additional try push
https://bugzilla.mozilla.org/show_bug.cgi?id=1171792

Comment 13

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12)
> Additional try push
> https://bugzilla.mozilla.org/show_bug.cgi?id=1171792
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c91e88a55c5b

Comment 14

[Robert Strong (they/them - no direct email)]

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/51056edec2f3

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 8616505 [details] [diff] [review]
patch rev1

Approval Request Comment
[Feature/regressing bug #]: I've looked but wasn't able to find the regressing bug.
[User impact if declined]: Users that don't have permission to update will attempt to update without ever displaying the manual upgrade page informing them that they should update.
[Describe test coverage new/current, TreeHerder]: Tests in tree
[Risks and why]: Minimal. The client changes are to check if the user can apply updates when there is an update available and there is a corresponding check in the same file for when checking for updates for the same case that has worked for quite some time. There are also tests for both cases in the patch.
[String/UUID change made/needed]: None

Comment 16

[jen strickland :jenstrickland]

[Tracking Requested - why for this release]: tracked for 39, 40, 41, and esr38+, don't have privileges to esr31, will uplift when solution is found.

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Henrik, I wonder if you're the right person to ask. This sounds like a mac-specific regression and it would be good to have coverage for this in the update tests. Who would be the right person to ask to write those tests?

Comment 18

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

Robert, in regards an automated Marionette update test, would it be important to utilize the notification popup here? If that is the case a test would not be possible given that this is a system notification and we don't have access to it. I|m also not sure about the permissions. We run as the same user who installed the build. So we always have the permission to update.

Comment 19

[Robert Strong (they/them - no direct email)]

(In reply to Liz Henry (:lizzard) from comment #17)
> Henrik, I wonder if you're the right person to ask. This sounds like a
> mac-specific regression and it would be good to have coverage for this in
> the update tests. Who would be the right person to ask to write those tests?
Liz, this is not a Mac specific regression.

(In reply to Henrik Skupin (:whimboo) from comment #18)
> Robert, in regards an automated Marionette update test, would it be
> important to utilize the notification popup here? If that is the case a test
> would not be possible given that this is a system notification and we don't
> have access to it. I|m also not sure about the permissions. We run as the
> same user who installed the build. So we always have the permission to
> update.
Henrik, there is already a test in tree and a marionette test isn't needed.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Robert, my mistake on it being a Mac specific problem.  

On tests: we didn't appear to catch this in our current update testing. It sounds like we don't know when this regression happened or which versions it affects.  So it seems logical to ask what new update tests we could add to cover this case. 

Can you help us verify the fix, or describe how to reproduce the problem so QE can try to verify it? As you can imagine it is somewhat unnerving to uplift this with a week and a half to go for beta 39.

Comment 21

[Robert Strong (they/them - no direct email)]

(In reply to Liz Henry (:lizzard) from comment #20)
> Robert, my mistake on it being a Mac specific problem.  
> 
> On tests: we didn't appear to catch this in our current update testing. It
> sounds like we don't know when this regression happened or which versions it
> affects.  So it seems logical to ask what new update tests we could add to
> cover this case. 
Tests are being added in this bug which is why tests didn't catch this bug and hence the in-testsuite+ and the release driver approval request has "Describe test coverage new/current, TreeHerder]: Tests in tree". When I wrote the mochitest chrome tests years ago it was not possible to have a test for this and new code has made it possible.

> Can you help us verify the fix, or describe how to reproduce the problem so
> QE can try to verify it? As you can imagine it is somewhat unnerving to
> uplift this with a week and a half to go for beta 39.
No problem and I always do after the uplift request is approved since the time is better spent elsewhere if the uplift request is not approved.

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8616505 [details] [diff] [review]
patch rev1

Approved for uplift to aurora and beta. rstrong will verify on both 39 and 40 and has added test coverage.

Comment 23

[Robert Strong (they/them - no direct email)]

Sheriffs, I'll land this on the branches so I can first verify the patch applies cleanly and the tests pass.

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/51056edec2f3

Comment 25

[Robert Strong (they/them - no direct email)]

Patch applies cleanly and tests pass on aurora. Patch didn't apply cleanly to beta and esr38 as I expected and I already have patches for them. I'll submit them after I verify the tests pass.

Comment 26

[Robert Strong (they/them - no direct email)]

Attachment: beta patch

All tests pass

BTW: I have verified that without the client changes test_0172_notify_noPerms_manual.xul fails as it should and passes with the client changes.

Comment 27

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/b807b5614da3

Pushed to mozilla-beta
https://hg.mozilla.org/releases/mozilla-beta/rev/038ab776f75f

Comment 28

[Robert Strong (they/them - no direct email)]

Comment on attachment 8616505 [details] [diff] [review]
patch rev1

I'll submit a patch that applies cleanly to esr38 after I verify the tests pass and request esr38 on it.

Comment 29

[Robert Strong (they/them - no direct email)]

Attachment: patch esr38

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Some users won't be notified of updates.
User impact if declined: Users that don't have permission to update will attempt to update without ever displaying the manual upgrade page informing them that they should update.
Fix Landed on Version: 39
Risk to taking this patch (and alternatives if risky): Minimal. The client changes are to check if the user can apply updates when there is an update available and there is a corresponding check in the same file for when checking for updates for the same case that has worked for quite some time. There are also tests for both cases in the patch.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 30

[Robert Strong (they/them - no direct email)]

I built and ran the tests on esr38 as well so all should be good with the esr38 patch

Comment 31

[Robert Strong (they/them - no direct email)]

Steps to reproduce:

On a Windows system install a version of Firefox that has this patch and has an update available for it in the default location. Create a Windows account that is not a member of the administrators group and is a member of the users group (typically the default when creating an account on recent version of Windows). Login as that user and launch Firefox. Immediately open tools -> options -> advanced -> update and uncheck "Use a background service to install updates". Wait until the background update check notifies you that an update is available by displaying the large update ui.

Without the patch if you try to update via the ui it should try and end up displaying the UAC prompt where it asks you to enter a username and password.

With the patch it should inform you that there is an update available and provide a link to download it.

Comment 35

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8620787 [details] [diff] [review]
patch esr38

Approving uplift to ESR 39 (38.1.0) as this will impact firefox update scenarios esp. the background service update option.

Comment 36

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Adding a qe-verify flag so we test the fix on this one in time for ESR 38.1.0 release.

Comment 37

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-esr38
https://hg.mozilla.org/releases/mozilla-esr38/rev/dbd824d3a383

Comment 40

[Cornel Ionce [:noni] [Hubs QA]]

Verified this issue as fixed using Latest Nightly 41.0a1, latest Aurora 40.0a2 and Firefox 39 beta 7.
Tested on Windows 7 64-bit and Mac OS X 10.10.4 with the STR from comment 31 and got the expected results.

Leaving this bug resolved until I'll verify it using esr38.

Comment 41

[Robert Strong (they/them - no direct email)]

Thanks!

Comment 42

[Cornel Ionce [:noni] [Hubs QA]]

It seems that ESR is not quite fixed. I've tried to update from ESR 38.0 using a Guest account and it still tries to update once (the update is downloaded, browser restarted to apply update but still 38.0).

Afterwards, no notification of an update was received in more than an hour.

If the user wants to update manually, he is notified that an update is available to download. Another issue here is that the download link points to the Release instead of ESR.

Prints: http://imgur.com/a/DtNKw

Comment 43

[Robert Strong (they/them - no direct email)]

(In reply to Cornel Ionce [QA] from comment #42)
> It seems that ESR is not quite fixed.
Can you provide the link to the build you are using to test and the steps you performed?

Comment 44

[Cornel Ionce [:noni] [Hubs QA]]

I've downloaded the ESR 38.0 build from: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/38.0esr/mac/en-US/

1. Install the ESR 38.0 build from the main account.
2. Switch to a Guest account.
3. Open ESR 38.0 and wait for the update notification.
4. Restart the browser after the update was downloaded, as required.(at this point the update was not installed)
5. Go to Firefox -> About Firefox (Update available for download notification)

Note: I've tested on Mac OS X 10.10.4.

Comment 45

[Robert Strong (they/them - no direct email)]

The build available from
http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/38.0esr/mac/en-US/

is from May 5th so I highly doubt it has the fix.

Comment 46

[Robert Strong (they/them - no direct email)]

To verify this first you will need a build that includes the patch from this bug. I installed the builds available here.

http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.1.0esr-candidates/build1/mac/en-US/

http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/38.1.0esr-candidates/build1/win32/en-US/

Since the latest esr builds don't have an update available a custom update will need to be used.
After launching the build installed from the above urls open about:config.
Add a new string preference with the name app.update.url.override and a value of
http://exchangecode.com/robert/work/snippets/bug1171792.xml

You should be able to verify the bug now using the steps used previously though you might need to restart. I already verified Windows works as expected.

Comment 47

[Cornel Ionce [:noni] [Hubs QA]]

By creating the " app.update.url.override" string and setting it to "http://exchangecode.com/robert/work/snippets/bug1171792.xml" I was able to confirm this fix for ESR 38.

Screenshot: http://i.imgur.com/1QgpoIP.jpg

Thanks Robert!

Comment 48

[Robert Strong (they/them - no direct email)]

Thank you Cornel!