Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ElevatedManagedAction issue #1220

Closed
barnabas90 opened this issue Oct 20, 2022 · 5 comments
Closed

ElevatedManagedAction issue #1220

barnabas90 opened this issue Oct 20, 2022 · 5 comments

Comments

@barnabas90
Copy link

Hi,

Today, I ran into an issue, I used a specific ctor of ElevatedManagedAction and my custom action suddenly got permission issues. Turned out that impersonate=no is not generated with that ctor. I checked the source code, impersonate=false is missing from 2 ctors at line 309 and 326. Is this intended or a bug?

@barnabas90
Copy link
Author

Workaround was to give id in prop init list instead of ctor.

@oleg-shilo
Copy link
Owner

The constructors are used to init the most commonly used properties. For everything else use initializers. Exactly as you did.

@barnabas90
Copy link
Author

Let me approach this from a different perspective.

In wixsharp, we have the ElevatedManagedAction class, which is a specialized version of the ManagedAction class. This ElevatedManagedAction class just presets some properties to their proper values (Impersonate=false, Execute=Execute.deferred), in order to generate a custom action that runs elevated.
With this specialized class, the programmer doesn't have to use the ManagedAction class and set these properties manually all the time when they need to define a custom action that runs elevated, just like as its documentation says:

Defines WiX Managed CustomAction, which is to be run with elevated privileges (UAC).
Any CustomAction, which needs elevation must be run with "Action.Impersonate" set to "false" and "Action.Execute" set to "Execute.deferred". Thus "ElevatedManagedAction" is a full equivalent of "ManagedAction" with appropriately adjusted "Action.Execute" and "Action.Impersonate" during the instantiation.

Here's the problem:
There're 29 overloads of the ctor, 2 of them lack the statement that sets Impersonate to false, for no apparent reason. Therefore the impersonate=no attribute is not set for the custom action in the generated wxs when those ctors are used. This results in a custom action that doesn't run elevated.

That 2 ctors are traps, the programmer expects to get an elevated custom action - but it won't be, and it only gets clear what the problem is, when the programmer checks the generated wxs.

@oleg-shilo
Copy link
Owner

oleg-shilo commented Oct 25, 2022

As a background info...
The original implementation of thess class constructors (and in fact of any WiX entity class) was to cover the most commonly used properties. It is warranted that it will not cover all available properties (e.g. introduced in later WiX) and all the possible combinations of the properties.
In order to avoid "constructors madness" one needs to encourage the use of initializers. 29 constructors are already madness if you ask me.

Now, about your very question about these two constructors. It is an API bug caused by the typo. Fixed now. The fix will be available in the very next release.

And another note. The use of ElevatedManagedAction fits canonical WiX API scenarios. My personal strong preference would be to go with the ManagedProject class and go with the After Install event, which is already elevated for you:

var project = new ManagedProject(...
. . .
project.AfterInstall += (SetupEventArgs e) =>
{
    // do whatever action that requires elevation
};

Providing you want to do your action after install of course.

@barnabas90
Copy link
Author

Thank you for the fix and the note!

oleg-shilo added a commit that referenced this issue Jan 6, 2023
- Issue #1244: The directory Id generated can be too long
- Issue #1223: Non LegacyDummyDirAlgorithm creates C:\ProgramFilesFolder empty folder
- Issue #1220: ElevatedManagedAction issue
- Feature #1204: Feature - RegisterCom class to ease the registration of COM files
- Issue #1203: SilentBootstrapperApplication
- Issue #182 (extended solution): RegistrySearch has "Win64=no" when building x64 installers
- Added Self-executable_Msi sample
- Added WixBootstrapper_EmbeddedUI to demonstrate how to show managed UI if the bundled MSI
- Added sample for customization of the stock Burn UI. Triggered by #1219
- Added sample for "Issue #1218: Dynamic custom UI sequence"
- Resurrected setting user input from BA UI and passing it to the msi. RegistrySearch input is also retained.
- Added validation for `Issue #1075: [FEAT] Add error if LaunchApplicationFromExitDialog using in common Project` error.
- Fixed problem with RegKey being placed in the x86 root XML dir for the x64 project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants