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

Fix mismatch between user and builtinaccount #387

Merged
merged 26 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
99bcac5
Fix mismatch between user and builtinaccount
nickgw May 21, 2022
5feb6fd
Updating changelog
nickgw May 21, 2022
b009bae
Dont exceed 100char
nickgw May 21, 2022
a49b884
remove test
nickgw May 22, 2022
14b6455
remove bad whitespce, handle when logontype is set incorrectly
nickgw May 22, 2022
874ca5e
combine ifs
nickgw May 22, 2022
22914f4
Merge remote-tracking branch 'origin/main' into ngermany_fixuservsbui…
nickgw Jun 3, 2022
49a6401
Fixing builtinuser assignment
nickgw Jun 3, 2022
ee9648b
updating test
nickgw Jun 3, 2022
fdc5381
updating code
nickgw Jul 23, 2022
93b7a15
Uncommenting out test
nickgw Jul 23, 2022
dfb4163
Trigger CI
nickgw Oct 4, 2022
a6edb9c
Pass User too
nickgw Oct 4, 2022
dece569
bolster test
nickgw Oct 4, 2022
674b7c5
Add tests
nickgw Oct 4, 2022
a05afba
Update test to use test-target resource
nickgw Oct 5, 2022
666b530
moving get-scheduledtask up
nickgw Oct 5, 2022
ede5740
trying to mock schdtask
nickgw Oct 5, 2022
083474a
remove erroneous f
nickgw Oct 10, 2022
7c451cf
adding removed tests
nickgw Oct 29, 2022
d1ba4ee
add new test
nickgw Oct 29, 2022
ed04ba8
remove bad tests
nickgw Oct 29, 2022
5e56c3b
Merge branch 'ngermany_fixuservsbuiltinaccount' of https://github.com…
nickgw Oct 29, 2022
a9e44e2
Merge branch 'main' into ngermany_fixuservsbuiltinaccount
PlagueHO Nov 20, 2022
71ae2a0
Merge branch 'ngermany_fixuservsbuiltinaccount' of https://github.com…
nickgw Nov 22, 2022
73dce2d
Updating based on review
nickgw Nov 22, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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