This repository was archived by the owner on Feb 19, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 342
Improve Chocolatey setup as administrator and add Test-ProcessAdminRights helper #486
Closed
jberezanski
wants to merge
15
commits into
chocolatey-archive:master
from
jberezanski:per-machine-install-v6
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
2cdd364
tests: add environment variable handling facilities
jberezanski 78ffeb3
add tests for Chocolatey setup (Initialize-Chocolatey)
jberezanski 0ecec9c
setup tests: conditionally test default installation
jberezanski dfb4372
setup tests: mock UAC status
jberezanski 5698b2f
add Test-AdminRights helper
jberezanski 5785f3f
Test-AdminRights: request WindowsIdentity with minimal access necessary
jberezanski 42fcbbd
enable mocking Test-AdminRights for setup tests
jberezanski fd27621
setup tests: add simulated standard user scenarios
jberezanski daae22d
setup tests: with admin rights, variables should be set at Machine le…
jberezanski 466633c
when running with administrative rights, set environment variables at…
jberezanski ecb5037
setup tests: do not vary by UAC status
jberezanski 3f8de3e
setup tests: fix formatting
jberezanski e8423f2
rename Test-AdminRights to Test-ProcessAdminRights
jberezanski f5c47dd
helpers: add environment variable management abstractions
jberezanski 1314e2c
tests: add environment manipulation mocking facility
jberezanski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| function Get-EnvironmentVariable([string] $Name, [System.EnvironmentVariableTarget] $Scope) { | ||
| [Environment]::GetEnvironmentVariable($Name, $Scope) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| function Get-EnvironmentVariableNames([System.EnvironmentVariableTarget] $Scope) { | ||
| switch ($Scope) { | ||
| 'User' { Get-Item 'HKCU:\Environment' | Select-Object -ExpandProperty Property } | ||
| 'Machine' { Get-Item 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager\Environment' | Select-Object -ExpandProperty Property } | ||
| 'Process' { Get-ChildItem Env:\ | Select-Object -ExpandProperty Key } | ||
| default { throw "Unsupported environment scope: $Scope" } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| function Set-EnvironmentVariable([string] $Name, [string] $Value, [System.EnvironmentVariableTarget] $Scope) { | ||
| [Environment]::SetEnvironmentVariable($Name, $Value, $Scope) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| function Test-ProcessAdminRights { | ||
| <# | ||
| .SYNOPSIS | ||
| Tests whether the current process is running with administrative rights. | ||
|
|
||
| .DESCRIPTION | ||
| This function checks whether the current process has administrative rights | ||
| by checking if the current user identity is a member of the Administrators group. | ||
| It returns $true if the current process is running with administrative rights, | ||
| $false otherwise. | ||
|
|
||
| On Windows Vista and later, with UAC enabled, the returned value represents the | ||
| actual rights available to the process, i.e. if it returns $true, the process is | ||
| running elevated. | ||
|
|
||
| .OUTPUTS | ||
| System.Boolean | ||
|
|
||
| #> | ||
|
|
||
| $currentPrincipal = New-Object Security.Principal.WindowsPrincipal([Security.Principal.WindowsIdentity]::GetCurrent([Security.Principal.TokenAccessLevels]'Query,Duplicate')) | ||
| $isAdmin = $currentPrincipal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) | ||
| Write-Debug "Test-ProcessAdminRights: returning $isAdmin" | ||
| return $isAdmin | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| $here = Split-Path -Parent $MyInvocation.MyCommand.Definition | ||
|
|
||
| Get-ChildItem "$here\mocks" -Filter *.ps1 -Recurse | ForEach-Object { Write-Debug "Importing $($_.FullName)"; . $_.FullName } | ||
|
|
||
| function Get-EnvironmentSnapshot() | ||
| { | ||
| Write-Debug 'Obtaining snapshot of the environment' | ||
| $machineEnv = @{} | ||
| $key = Get-Item 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager\Environment' | ||
| $key.GetValueNames() | ForEach-Object { $machineEnv[$_] = $key.GetValue($_) } | ||
|
|
||
| $userEnv = @{} | ||
| $key = Get-Item 'HKCU:\Environment' | ||
| $key.GetValueNames() | ForEach-Object { $userEnv[$_] = $key.GetValue($_) } | ||
|
|
||
| $processEnv = @{} | ||
| Get-ChildItem Env:\ | ForEach-Object { $processEnv[$_.Key] = $_.Value } | ||
|
|
||
| return New-Object PSCustomObject -Property @{ machine = $machineEnv; user = $userEnv; process = $processEnv } | ||
| } | ||
|
|
||
| function Restore-Environment($state) | ||
| { | ||
| Write-Debug 'Restoring the environment' | ||
| $state.machine.GetEnumerator() | ForEach-Object { | ||
| $current = [Environment]::GetEnvironmentVariable($_.Key, 'Machine') | ||
| if ($current -ne $_.Value) { | ||
| Write-Warning "Restoring value of environment variable $($_.Key) at Machine scope. The need to do that means that some code did not use the environment manipulation functions *-EnvironmentVariable*." | ||
| [Environment]::SetEnvironmentVariable($_.Key, $_.Value, 'Machine') | ||
| } | ||
| } | ||
|
|
||
| $key = Get-Item 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager\Environment' | ||
| $key.GetValueNames() | Where-Object { -not $state.machine.ContainsKey($_) } | ForEach-Object { | ||
| Write-Warning "Deleting environment variable $_ at Machine scope. The need to do that means that some code did not use the environment manipulation functions *-EnvironmentVariable*." | ||
| [Environment]::SetEnvironmentVariable($_, $null, 'Machine') | ||
| } | ||
|
|
||
| $state.user.GetEnumerator() | ForEach-Object { | ||
| $current = [Environment]::GetEnvironmentVariable($_.Key, 'User') | ||
| if ($current -ne $_.Value) { | ||
| Write-Warning "Restoring value of environment variable $($_.Key) at User scope. The need to do that means that some code did not use the environment manipulation functions *-EnvironmentVariable*." | ||
| [Environment]::SetEnvironmentVariable($_.Key, $_.Value, 'User') | ||
| } | ||
| } | ||
|
|
||
| $key = Get-Item 'HKCU:\Environment' | ||
| $key.GetValueNames() | Where-Object { -not $state.user.ContainsKey($_) } | ForEach-Object { | ||
| Write-Warning "Deleting environment variable $_ at User scope. The need to do that means that some code did not use the environment manipulation functions *-EnvironmentVariable*." | ||
| [Environment]::SetEnvironmentVariable($_, $null, 'User') | ||
| } | ||
|
|
||
| $state.process.GetEnumerator() | ForEach-Object { | ||
| $current = [Environment]::GetEnvironmentVariable($_.Key, 'Process') | ||
| if ($current -ne $_.Value) { | ||
| Write-Debug "Restoring value of environment variable $($_.Key) at Process scope" | ||
| [Environment]::SetEnvironmentVariable($_.Key, $_.Value, 'Process') | ||
| } | ||
| } | ||
|
|
||
| Get-ChildItem Env:\ | Select-Object -ExpandProperty Name | Where-Object { -not $state.process.ContainsKey($_) } | ForEach-Object { | ||
| Write-Debug "Deleting environment variable $_ at Process scope" | ||
| [Environment]::SetEnvironmentVariable($_, $null, 'Process') | ||
| } | ||
| } | ||
|
|
||
| function Setup-EnvironmentMockup | ||
| { | ||
| $global:ChocolateyTestEnvironmentVariables = Get-EnvironmentSnapshot | ||
| } | ||
|
|
||
| function Cleanup-EnvironmentMockup | ||
| { | ||
| $global:ChocolateyTestEnvironmentVariables = $null | ||
| } | ||
|
|
||
| function Execute-WithEnvironmentProtection($scriptBlock) | ||
| { | ||
| $savedEnvironment = Get-EnvironmentSnapshot | ||
| try | ||
| { | ||
| Setup-EnvironmentMockup | ||
| try | ||
| { | ||
| & $scriptBlock | ||
| } | ||
| finally | ||
| { | ||
| Cleanup-EnvironmentMockup | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| Restore-Environment $savedEnvironment | ||
| } | ||
| } | ||
|
|
||
| function Add-EnvironmentVariable($name, $value, $targetScope) | ||
| { | ||
| Write-Debug "Setting $name to $value at $targetScope scope" | ||
| Set-EnvironmentVariable -Name $name -Value $Value -Scope $targetScope | ||
| if ($targetScope -eq 'Process') { | ||
| Write-Debug "Current $name value is '$value' (from Process scope)" | ||
| return | ||
| } | ||
| # find lowest scope with $name set and use that value as current | ||
| foreach ($currentScope in @('User', 'Machine')) { | ||
| $valueAtCurrentScope = Get-EnvironmentVariable -Name $name -Scope $currentScope | ||
| if ($valueAtCurrentScope -ne $null) { | ||
| Write-Debug "Current $name value is '$valueAtCurrentScope' (from $currentScope scope)" | ||
| Set-EnvironmentVariable -Name $name -Value $valueAtCurrentScope -Scope Process | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function Remove-EnvironmentVariable($name) | ||
| { | ||
| Write-Debug "Ensuring environment variable $name is not set at any scope" | ||
| 'Machine','User','Process' | ForEach-Object { | ||
| if (-not ([String]::IsNullOrEmpty((Get-EnvironmentVariable -Name $name -Scope $_)))) { | ||
| Write-Debug "Deleting environment variable $name at $_ scope" | ||
| Set-EnvironmentVariable -Name $name -Value $null -Scope $_ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function Add-DirectoryToPath($directory, $scope) | ||
| { | ||
| $curPath = Get-EnvironmentVariable -Name 'PATH' -Scope $scope | ||
| $newPath = ($curPath -split ';' | Where-Object { $_.TrimEnd('\') -ne $directory.TrimEnd('\') }) -join ';' | ||
| if ($newPath -ne $curPath) { | ||
| Write-Debug "Directory $directory is already on PATH at $scope scope" | ||
| } else { | ||
| Write-Debug "Adding directory $directory to PATH at $scope scope" | ||
| if ([String]::IsNullOrEmpty($newPath)) { | ||
| Set-EnvironmentVariable -Name 'PATH' -Value $directory -Scope $scope | ||
| } else { | ||
| Set-EnvironmentVariable -Name 'PATH' -Value "$($newPath.TrimEnd(';'));$directory" -Scope $scope | ||
| } | ||
| } | ||
| if ($scope -ne 'Process') { | ||
| $curPath = Get-EnvironmentVariable -Name 'PATH' -Scope Process | ||
| $newPath = ($curPath -split ';' | Where-Object { $_.TrimEnd('\') -ne $directory.TrimEnd('\') }) -join ';' | ||
| if ($newPath -eq $curPath) { | ||
| Write-Debug "Adding directory $directory to PATH at Process scope" | ||
| if ([String]::IsNullOrEmpty($newPath)) { | ||
| Set-EnvironmentVariable -Name 'PATH' -Value $directory -Scope Process | ||
| } else { | ||
| Set-EnvironmentVariable -Name 'PATH' -Value "$($newPath.TrimEnd(';'));$directory" -Scope Process | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function Remove-DirectoryFromPath($directory) | ||
| { | ||
| Write-Debug "Ensuring directory $directory is not on PATH at any scope" | ||
| 'Machine','User','Process' | ForEach-Object { | ||
| $scope = $_ | ||
| $curPath = Get-EnvironmentVariable -Name 'PATH' -Scope $scope | ||
| $newPath = ($curPath -split ';' | Where-Object { $_.TrimEnd('\') -ne $directory.TrimEnd('\') }) -join ';' | ||
| if ($newPath -ne $curPath) { | ||
| Write-Debug "Removing directory $directory from PATH at $scope scope" | ||
| Set-EnvironmentVariable -Name 'PATH' -Value $newPath -Scope $scope | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function Assert-OnPath($directory, $pathScope) | ||
| { | ||
| $path = Get-EnvironmentVariable -Name 'PATH' -Scope $pathScope | ||
| $dirInPath = $path -split ';' | Where-Object { $_ -eq $directory } | ||
| "$dirInPath" | Should not BeNullOrEmpty | ||
| } | ||
|
|
||
| function Assert-NotOnPath($directory, $pathScope) | ||
| { | ||
| $path = Get-EnvironmentVariable -Name 'PATH' -Scope $pathScope | ||
| $dirInPath = $path -split ';' | Where-Object { $_ -eq $directory } | ||
| "$dirInPath" | Should BeNullOrEmpty | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests that are actually making changes to the environment scare me a bit. A glitch where it doesn't get to clean up is going to leave the system in an unusable state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we could find a way to mock out the actual calls to the system and test the behavior of chocolatey based on what is returned from those calls, I think we can call these unit tests.
Actual hits to the system are considered physical integration tests and should probably be moved somewhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented a very thin abstraction layer, wrapping [Environment]::Get/SetEnvironmentVariable and obtaining a list of environment variable names (needed by one helper). In test code I can now replace that layer with mocks that read and write in-memory state.
I've left the existing backup/restore mechanism in place, first to protect against bugs and code not going through my layer (no such code now, but someone might forget and write a direct call to [Environment] in the future), second to restore process-level environment between test cases.
The restore mechanism will now warn if it detects a change at user or machine level, because that means some code did not use the layer and should be fixed.