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

Rethink installer and how it sets stuff up (reduce need for admin) #10126

Closed
crutkas opened this issue Mar 9, 2021 · 13 comments
Closed

Rethink installer and how it sets stuff up (reduce need for admin) #10126

crutkas opened this issue Mar 9, 2021 · 13 comments
Assignees
Labels
Area-Enterprise Issues relevant to large enterprises, SCCM, run as admin restrictions, ... Area-Runner The PowerToys main executable Area-Setup/Install Refers to installation mechanism Cost-Large Large work item - 3+ days worth of work (chances are needs to be broken down) Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@crutkas
Copy link
Member

crutkas commented Mar 9, 2021

Right now we have a hybrid of logic between the settings app / runner and the installer. Most is duplicate.

What i think we should do is move most to a common library and have, on uninstall, these be removed, else they are maintained and invoked by the actual application.

  • Run at Startup -> we have two modes here, one for admin and another for normal users to prevent UAC prompts on start up
  • Reg context menus -> This doesn't require admin rights for image resizer / power rename
  • Reg file associates -> come up with way to prompt users on first load to register.
    • this will be must more powerful once the Monaco preview pane comes online
  • better user prompts for non-supported scenarios (Example, user installs on Win8.1)

Ultimately, these changes should simplify the installer as well as the upgrade process. Chances are there are a few other items that need to be here as well but i think how we currently handle our installer needs to be rethought as we grow and do even more complex scenarios

Workback schedule

.49

.51

  • Move runtimes to MSI
  • Verify HKCU vs HKLM for some of the quirks when running as admin

.53

  • No more elevation
  • Remove Bootstrapper

Plan discussed: ETA Nov 2021 release

  1. Image Resizer context menu
  2. PowerRename context menu
  3. File reg for file explore thumbnails <- based on settings, looks to need elevation
  4. File reg Preview pane items <- based on settings, looks to need elevation

What I’m proposing is a new process that handles

  1. Adding context menus
  2. File reg for thumbnails
  3. File reg for preview panes

This exe would be called when a user

  1. Changes a setting that will force us to change
  2. When we detect on first time / upgrade we don’t have hooks (dialog to confirm)
  3. Uninstall and we have reg’ed hooks
    a. If a user cancels the UAC, we need to validate.

This allows for a single centralized spot for logic, simplifies the Wix installer file, and will allow us to have an easier way to enable the Monaco 150 files.

@crutkas crutkas added Area-Setup/Install Refers to installation mechanism Area-Runner The PowerToys main executable Cost-Large Large work item - 3+ days worth of work (chances are needs to be broken down) Area-Enterprise Issues relevant to large enterprises, SCCM, run as admin restrictions, ... labels Mar 9, 2021
@crutkas crutkas added this to the 2020 Stability Release milestone Mar 9, 2021
@ghost ghost added the Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams label Mar 9, 2021
@enricogior
Copy link
Contributor

One important thing to keep in mind: if we allow to install PowerToys in a user folder (not in the Program Files) we must remove the option to run it as admin because it would open a security vulnerability very easy to exploit.

In order to mitigate the vulnerability we would have to:

  • don't load the modules at run time, and link them at build time
  • in the runner, validate each external binary signature before running them elevated
  • prompt the UAC every time PowerToys starts elevated (aka stop using the scheduled task)

@crutkas
Copy link
Member Author

crutkas commented Mar 9, 2021

Why not detect if valid location and enable functionality

no matter what the startup task will have split logic.

@enricogior
Copy link
Contributor

@crutkas

Why not detect if valid location and enable functionality

It will not mitigate the vulnerability unless is a location that requires admin privileges to write.

@htcfreek
Copy link
Collaborator

htcfreek commented Mar 9, 2021

I don't like the installation in user context. This makes deployment in company environments with sccm (or others) complicated/nearly impossible.

@crutkas crutkas removed the Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams label Mar 11, 2021
@enricogior
Copy link
Contributor

@crutkas
is this issue a tracker for the specifications or for the actual work?
If it's for the actual work, it cannot be done in 0.37 since we have other work that has higher priority.

@prakharb5
Copy link

Agree with this issue. Currently when updating, installer triggers the UAC prompt thrice (though the last one seems to be of app). Instead, a single prompt should be used. Say, a process triggers UAC prompt and gets root access, and then it manages and processes the upgrade (deleting files, making change to context menu, run at startup - all 3 by one UAC prompt).

@enricogior
Copy link
Contributor

@Cyberdroid1
what you describe is a different issue and it's tracked by #8828

@crutkas
Copy link
Member Author

crutkas commented Apr 2, 2021

lets use this as the talking point, break work apart from there as i know this is a larger item

@yuyoyuppe
Copy link
Collaborator

We also want to have a checkbox during uninstallation to remove the settings as well.

@crutkas
Copy link
Member Author

crutkas commented Jan 6, 2022

can we maybe close this issue and reconsitute what work is left now?

@yuyoyuppe
Copy link
Collaborator

Sure. To sum up: we've reworked the installer, fixed some issues and migrated to standard WiX bootstrapper. However, there is still unresolved stuff which doesn't let us to fully transition to per user install, and also we're blocked by dotnet runtime dependency requiring elevation.

@yuyoyuppe yuyoyuppe added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development labels Jan 13, 2022
@yuyoyuppe
Copy link
Collaborator

yuyoyuppe commented Jan 13, 2022

xref #15516 and #15517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Enterprise Issues relevant to large enterprises, SCCM, run as admin restrictions, ... Area-Runner The PowerToys main executable Area-Setup/Install Refers to installation mechanism Cost-Large Large work item - 3+ days worth of work (chances are needs to be broken down) Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

6 participants