Skip to content

Commit

Permalink
Fix mismatch between user and builtinaccount (#387)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickgw authored Nov 23, 2022
1 parent 3aea926 commit 6c3f2fd
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- ScheduledTask
- No longer conflates resource parameter `BuiltInAccount` and `*-ScheduledTask` parameter `user` - Fixes [Issue #385](https://github.com/dsccommunity/ComputerManagementDsc/issues/385)

### Added

- Computer
Expand Down
32 changes: 24 additions & 8 deletions source/DSCResources/DSC_ScheduledTask/DSC_ScheduledTask.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ function Set-TargetResource
non-null value to be 'LOCAL SERVICE', 'NETWORK SERVICE' or
'SYSTEM'
#>
$username = 'NT AUTHORITY\' + $BuiltInAccount
$username = Set-DomainNameInAccountName -AccountName $BuiltInAccount -DomainName 'NT AUTHORITY'
$registerArguments.Add('User', $username)
$LogonType = 'ServiceAccount'
}
Expand All @@ -804,7 +804,6 @@ function Set-TargetResource
elseif ($PSBoundParameters.ContainsKey('ExecuteAsCredential'))
{
$username = $ExecuteAsCredential.UserName

# If the LogonType is not specified then set it to password
if ([System.String]::IsNullOrEmpty($LogonType))
{
Expand All @@ -829,7 +828,7 @@ function Set-TargetResource
privileges, should we default to 'NT AUTHORITY\LOCAL SERVICE'
instead?
#>
$username = 'NT AUTHORITY\SYSTEM'
$username = Set-DomainNameInAccountName -AccountName 'SYSTEM' -DomainName 'NT AUTHORITY'
$registerArguments.Add('User', $username)
$LogonType = 'ServiceAccount'
}
Expand Down Expand Up @@ -1423,16 +1422,17 @@ function Test-TargetResource

if ($PSBoundParameters.ContainsKey('BuiltInAccount'))
{
$PSBoundParameters.User = $BuiltInAccount
$currentValues.User = $BuiltInAccount
$user = Set-DomainNameInAccountName -AccountName 'SYSTEM' -DomainName 'NT AUTHORITY'
$PSBoundParameters.User = $user
$currentValues.User = $user

$PSBoundParameters.ExecuteAsCredential = $BuiltInAccount
$currentValues.ExecuteAsCredential = $BuiltInAccount

$PSBoundParameters['LogonType'] = 'ServiceAccount'
$currentValues['LogonType'] = 'ServiceAccount'

$PSBoundParameters['BuiltInAccount'] = 'NT AUTHORITY\' + $BuiltInAccount
$PSBoundParameters['BuiltInAccount'] = $BuiltInAccount
}
elseif ($PSBoundParameters.ContainsKey('ExecuteAsCredential'))
{
Expand Down Expand Up @@ -1464,6 +1464,16 @@ function Test-TargetResource
}
else
{
$user = Set-DomainNameInAccountName -AccountName 'SYSTEM' -DomainName 'NT AUTHORITY'
$PSBoundParameters.User = $user
$currentValues.User = $user

$PSBoundParameters.ExecuteAsCredential = 'SYSTEM'
$currentValues.ExecuteAsCredential = 'SYSTEM'

$PSBoundParameters.Add('BuiltInAccount', $BuiltInAccount)
$currentValues.BuiltInAccount = $BuiltInAccount

# Must be running as System, login type is ServiceAccount
$PSBoundParameters['LogonType'] = 'ServiceAccount'
$currentValues['LogonType'] = 'ServiceAccount'
Expand Down Expand Up @@ -1915,9 +1925,15 @@ function Get-CurrentResource
Delay = ConvertTo-TimeSpanStringFromScheduledTaskString -TimeSpan $trigger.Delay
}

if (($result.ContainsKey('LogonType')) -and ($result['LogonType'] -ieq 'ServiceAccount'))
if (
(($result.ContainsKey('LogonType')) -and ($result['LogonType'] -ieq 'ServiceAccount')) -or
$result.Principal.UserID -in @('SYSTEM', 'LOCAL SERVICE', 'NETWORK SERVICE')
)
{
$builtInAccount = Set-DomainNameInAccountName -AccountName $task.Principal.UserId -DomainName 'NT AUTHORITY'
$result.User = Set-DomainNameInAccountName `
-AccountName $task.Principal.UserId `
-DomainName 'NT AUTHORITY'
$builtInAccount = $task.Principal.UserId
$result.Add('BuiltInAccount', $builtInAccount)
}
}
Expand Down
25 changes: 18 additions & 7 deletions tests/Unit/DSC_ScheduledTask.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,15 @@ try
}
}

$testParameters.Add('User', 'WrongUser')

It 'Should Disregard User and Set User to the BuiltInAccount' {
Set-TargetResource @testParameters
Assert-MockCalled -CommandName Register-ScheduledTask -Times 1 -Scope It -ParameterFilter {
$User -ieq ('NT AUTHORITY\' + $testParameters['BuiltInAccount'])
}
}

$testParameters.Add('LogonType', 'Password')

It 'Should overwrite LogonType to "ServiceAccount"' {
Expand All @@ -1799,14 +1808,16 @@ try

Mock -CommandName Get-ScheduledTask -MockWith {
@{
TaskName = $testParameters.TaskName
TaskPath = $testParameters.TaskPath
Actions = @(
Description = '+'
TaskName = $testParameters.TaskName
TaskPath = $testParameters.TaskPath
Actions = @(
[pscustomobject] @{
Execute = $testParameters.ActionExecutable
}
)
Triggers = @(
ActionArguments = '-File "C:\something\right.ps1"'
Triggers = @(
[pscustomobject] @{
Repetition = @{
Duration = "PT$([System.TimeSpan]::Parse($testParameters.RepetitionDuration).TotalHours)H"
Expand All @@ -1817,12 +1828,12 @@ try
}
}
)
Settings = [pscustomobject] @{
Settings = [pscustomobject] @{
Enabled = $true
MultipleInstances = 'IgnoreNew'
}
Principal = [pscustomobject] @{
UserId = 'NT AUTHORITY\' + $testParameters.BuiltInAccount
Principal = [pscustomobject] @{
UserId = $testParameters.BuiltInAccount
LogonType = 'ServiceAccount'
}
}
Expand Down

0 comments on commit 6c3f2fd

Please sign in to comment.