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

Separate target user #50

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

peter1rhodes
Copy link

On my work laptop, I have a standard user that I use regularly, but also a local admin account that lets me install stuff. I have therefore modified the install script to allow installation for another user.

I am very open to feedback, as I am a complete Powershell beginner. It is likely that I have not done things in the best way. However, the install script worked well for me

Default to the current user to preserve standard behaviour
Default to the current user to preserve existing behaviour
install.ps1 Outdated

echo "Changing settings for user $targetUser with SID $targetUserSID"

$Env:LocalAppData = "C:\Users\$targetUser\AppData\Local"
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason on why C: is hard coded here? Not everyone installs Windows on that drive.

Copy link
Author

Choose a reason for hiding this comment

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

No that's a great point. I'll have a look for a better way, do you have any suggestions before I look into it?

Copy link
Owner

Choose a reason for hiding this comment

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

You might try to see if systemroot environment variable can be used.

uninstall.ps1 Outdated
$targetUserObject = New-Object System.Security.Principal.NTAccount($targetUser)
$targetUserSID = $targetUserObject.Translate([System.Security.Principal.SecurityIdentifier]).value

$Env:LocalAppData = "C:\Users\$targetUser\AppData\Local"
Copy link
Owner

Choose a reason for hiding this comment

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

Same here with hard coded drive name.

@peter1rhodes peter1rhodes requested a review from lextm September 23, 2021 08:29
@peter1rhodes
Copy link
Author

I've opted for $Env:HOMEDRIVE as it seems a clean way to get there

@rwasef1830
Copy link

@peter1rhodes look up the profile path from registry, some people change the drive that contains the windows profile and home drive may not be the same as the system drive. don't assume all account profiles are in the same drive..

see : HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList

There's probably an easier Win32 call to get this info... or some ready made .NET API.

@lextm
Copy link
Owner

lextm commented Sep 23, 2021

HOMEDRIVE can be a bad option, as examples like this show.

@peter1rhodes
Copy link
Author

Thank you both @rwasef1830 and @lextm. I have found a way to get to the correct user profile directory using HKEY_USERS[SID]\Voltatile Environment which seems to work well.

@peter1rhodes
Copy link
Author

I've refactored the key parts to use Volatile Environment for both LocalAppData and for the USERPROFILE path. I have also moved all that logic for finding the required paths for the target user to a new function Get-TargetUser, in a new file called TargetUser.psm1, which is then imported by both install.ps1 and uninstall.ps1. The Get-TargetUser function prompts the user to select a user for which to install the context menus, defaulting to the current user if Enter is hit without input. It then returns a custom object containing the required info about the targetUser (their username, SID, LocalAppData path and USERPROFILE path), which can then easily be referenced by the calling scripts.

Copy link
Owner

@lextm lextm left a comment

Choose a reason for hiding this comment

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

Looks good, but I don't have a Windows machine at hand to verify. Once I can test them out, I will accept the PR and merge the changes.

@lextm lextm self-assigned this Oct 14, 2021
@AMArostegui
Copy link

I've tried this PR on my computer and it works

@Prodigio
Copy link

Prodigio commented Jul 1, 2024

Tried this PR as well, did work for my non-admin account on Windows 10.

EDIT: in order to run this script, one needs to set Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass in Powershell, otherwise the TargetUser.psm1 cannot be executed.

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.

5 participants