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

Fallback to run as admin after install, if running PT as user not possible #16089

Merged
merged 5 commits into from
Feb 8, 2022

Conversation

stefansjfw
Copy link
Collaborator

@stefansjfw stefansjfw commented Feb 4, 2022

Summary of the Pull Request

What is this about:

What is included in the PR:

How does someone test / validate:

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@stefansjfw stefansjfw force-pushed the stefan/fix_run_as_non_admin_user branch from 56d2f2b to 6b6edf5 Compare February 4, 2022 16:44
restart_same_elevation();
result = -1;
Logger::warn("Scheduled restart failed. Trying restart as admin as fallback...");
restart_elevated();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are trying to restart elevated? Seems strange that it would work for the issue people are having. How about doing a debug build to verify this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it locally. The issue can easily be reproduced if you just try to install PowerToys on non-admin user.

The issue is happening because how we're trying to run non-elevated from elevated admin owned process. So we try to run non-elevated, if it fails, run elevated as it was working before.

src/runner/main.cpp Outdated Show resolved Hide resolved
src/runner/main.cpp Outdated Show resolved Hide resolved
src/runner/main.cpp Outdated Show resolved Hide resolved
src/runner/restart_elevated.cpp Outdated Show resolved Hide resolved
stefansjfw and others added 5 commits February 4, 2022 19:27
Co-authored-by: Andrey Nekrasov <yuyoyuppe@users.noreply.github.com>
Co-authored-by: Andrey Nekrasov <yuyoyuppe@users.noreply.github.com>
Co-authored-by: Andrey Nekrasov <yuyoyuppe@users.noreply.github.com>
@stefansjfw stefansjfw force-pushed the stefan/fix_run_as_non_admin_user branch from c14ddcf to db6baf9 Compare February 4, 2022 18:30
Copy link
Contributor

@yuyoyuppe yuyoyuppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@stefansjfw stefansjfw merged commit 11bb7cc into main Feb 8, 2022
@stefansjfw stefansjfw deleted the stefan/fix_run_as_non_admin_user branch February 8, 2022 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants