1333789 - Firefox installer should not request UAC privilege when installing to a user directory

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Mike Kaply [:mkaply]]

If you use an INI file to specify the Firefox install directory to a user directory, we should not ask for UAC admin privilege since we do not need it for the install.

See:

https://wiki.mozilla.org/Installer:Command_Line_Arguments

Comment 1

[Masatoshi Kimura [:emk]]

I think this bug is valid even if no INI file is used. The current method (cancel the UAC dialog) is not discoverable. The installer should request an elevation only after user specified the install path.

Comment 2

[Molly Howell (she/her) (mostly inactive)]

:mkaply, would you be able to elaborate on the impact of this bug? I'm not sure what priority to assign this because I don't know how it actually affects people trying to do silent/INI installs.

The interactive case that :emk brought up probably should have its own bug, because it's going to require new UI and therefore be a bigger project than the silent case. It might also get a different priority level.

Comment 3

[Masatoshi Kimura [:emk]]

(In reply to Matt Howell [:mhowell] from comment #2)
> The interactive case that :emk brought up probably should have its own bug,
> because it's going to require new UI and therefore be a bigger project than
> the silent case. It might also get a different priority level.

OK, filed bug 1337218.

Comment 4

[Mike Kaply [:mkaply]]

> :mkaply, would you be able to elaborate on the impact of this bug? I'm not sure what priority to assign this because I don't know how it actually affects people trying to do silent/INI installs.

Sure. I should have given more context. We're working with some partners that install Firefox as part of other software installs. Our current installer always requests UAC privileges which means that we lose a certain amount of users the moment that popup is displayed.

The assertion is that if they could simply install Firefox into the user directory without displaying the UAC dialog, we'd be less likely to lose that install.

So I'm wondering if we can be smart in our NSIS file and do RequestExecutionLevel admin only if the install directory is Program Files. This would obviously affect registry entries as well, for control panel uninstall and our other stuff.

Comment 5

[Molly Howell (she/her) (mostly inactive)]

Okay, thank you. So it sounds like all we need to do to handle the INI install case is defer trying to elevate until we know that the target directory isn't writable otherwise. That's easy enough to do that I'll just go ahead and put a patch up. For interactive installs there's more to do because we currently use the elevation state to determine what the default directory should be, so we need some other way to deal with that (thanks to :emk for filing that separately).

Comment 6

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Review commit: https://reviewboard.mozilla.org/r/110426/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/110426/

Comment 7

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/110426/diff/1-2/

Comment 8

[Mike Kaply [:mkaply]]

I got this comment from the partner as well. Posting it just in case it is relevant.

Regarding the UAC install
The INI configuration also needs MaintenanceService=false, since the Maintenance Service will install into Program Files (it ignores InstallDirectoryPath). Might be useful information for the ticket: checking if elevation is required based on the values of InstallDirectoryPath and MaintenanceService.

Comment 9

[Molly Howell (she/her) (mostly inactive)]

(In reply to Mike Kaply [:mkaply] from comment #8)
> I got this comment from the partner as well. Posting it just in case it is
> relevant.
> 
> Regarding the UAC install
> The INI configuration also needs MaintenanceService=false, since the
> Maintenance Service will install into Program Files (it ignores
> InstallDirectoryPath). Might be useful information for the ticket: checking
> if elevation is required based on the values of InstallDirectoryPath and
> MaintenanceService.

Hadn't thought of that. I'll amend the patch to address this case as well.

Comment 10

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/110426/diff/2-3/

Comment 11

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

https://reviewboard.mozilla.org/r/110426/#review112214

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa534d18c2d2
Try to avoid UAC prompts during automated installations using INI files. r=agashlin

Comment 13

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

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

Comment 14

[Mike Kaply [:mkaply]]

Matt:

Is there any chance you'd consider this safe enough to move all the way up to Firefox 52?

We have a partner rollout scheduled and this would make them very happy.

Comment 15

[Molly Howell (she/her) (mostly inactive)]

I don't think it's high risk, since it's a small change to a relatively uncommonly used code path. But I had a chat with the reviewer and we would both feel better (and probably so would relman) if you or the partner could manually verify that the change a) does solve their need, and b) doesn't fail spectacularly in the process. Today's nightly installer should have this change in it, so you can use it to test. Thanks.

Comment 16

[Mike Kaply [:mkaply]]

Definitely going to test once it's live. Leaving the needinfo to remind me.

Comment 17

[Mike Kaply [:mkaply]]

Partner has verified this works for them. I'm going to request uplift.

Comment 18

[Mike Kaply [:mkaply]]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Approval Request Comment
[Feature/Bug causing the regression]: Allow silent installer to install to User directoy
[User impact if declined]: No user impacted. Requested by partner. Partner unable to do silent install (which could affect adoption).
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Follow instruction here: https://wiki.mozilla.org/Installer:Command_Line_Arguments for a silent install. Specify a user directory and verify there is no UAC prompt.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Extremely low.
[Why is the change risky/not risky?]: Only affects silent/config.ini install which has very low usage.
[String changes made/needed]:

Comment 19

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Thanks, Mike!
We should request aurora approval as well, if we need beta.

See comment 18 for approval request comment.

Comment 20

[Gerry Chang [:gchang]]

Hi Brindusa, 
Could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 21

[Brindusa Tot, Desktop QA]

Verified as fixed on latest Nightly 54.0a1, build ID 20170215030342 on Windows 10 x64. Tested with instructions and  INI configuration file specified at https://wiki.mozilla.org/Installer:Command_Line_Arguments, and make sure to have set MaintenanceService=false into the ini file.

Comment 22

[Gerry Chang [:gchang]]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

Fix a silent install issue and was verified in nightly. Aurora53+.

Comment 23

[Julien Cristau [:jcristau]]

Comment on attachment 8834541 [details]
Bug 1333789 - Try to avoid UAC prompts during automated installations using INI files.

tweak windows install script to avoid unneeded prompt, beta52+

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/84ece89d7cc0

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fac7be108937

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/fac7be108937

Comment 27

[Brindusa Tot, Desktop QA]

Verified as fixed on Windows 10 x64 with Release Candidate 52.0-build2 and 52.0esr-build1.

Tested with instructions from comment 18 and INI configuration file specified at https://wiki.mozilla.org/Installer:Command_Line_Arguments, and make sure to have set MaintenanceService=false into the INI file.

Comment 28

[Andrei Vaida [:avaida]]

Removing the qe-verify flag, since this bug has been confirmed fixed on Fx54 (see Comment 21), Fx52 and Fx52esr (see Comment 27).