Browse Source

fix(security2): make the path overloads of AddFileSystemAccessRule and AddFileSystemAuditRule write at once

AddFileSystemAccessRule(string, List<IdentityReference2>, ...) and
AddFileSystemAuditRule(string, List<IdentityReference2>, ...) were
iterator methods (yield return), so nothing happened until the caller
enumerated the result: called as a statement, or assigned to a variable
that the caller never reads, they changed nothing. Their siblings, the
overloads for FileSystemInfo and FileSystemSecurity2 with a list, and
the single-account overloads, already built a list and wrote at once.

Both overloads now build the list and add the entries before returning,
like their siblings; a call that is never enumerated now adds the
entries just the same. An exception that adding an entry raises changes
only in timing: it is now thrown by the call itself, not by the later
enumeration of its result. The returned collection still holds one rule
per account, in the order of the account list, for both a file path and
a folder path.

Public signatures are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/131/head
Raimund Andree 2 weeks ago
parent
commit
a0362c359c
  1. 16
      Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs
  2. 16
      Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs
  3. 25
      Tests/ObjectApis.Tests.ps1

16
Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.AddFileSystemAccessRules.cs

@ -87,17 +87,27 @@ namespace Security2
return ace; return ace;
} }
/// <summary>
/// Adds one access entry per account to the item at the path, and returns them in the order of
/// <paramref name="accounts"/>. Before 5.0.0, this overload was an iterator (a yield return in the loop
/// below), so nothing happened until the caller enumerated the result; called as a statement, or assigned to
/// a variable that is never read, it changed nothing. It now builds the list and writes the entries before
/// returning, like its FileSystemInfo and FileSystemSecurity2 siblings; an exception that adding an entry
/// raises is now thrown by this call, not by the later enumeration.
/// </summary>
public static IEnumerable<FileSystemAccessRule2> AddFileSystemAccessRule(string path, List<IdentityReference2> accounts, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags) public static IEnumerable<FileSystemAccessRule2> AddFileSystemAccessRule(string path, List<IdentityReference2> accounts, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags)
{ {
if (type == AccessControlType.Allow) if (type == AccessControlType.Allow)
rights = rights | FileSystemRights2.Synchronize; rights = rights | FileSystemRights2.Synchronize;
var aces = new List<FileSystemAccessRule2>();
if (File.Exists(path)) if (File.Exists(path))
{ {
var item = new FileInfo(path); var item = new FileInfo(path);
foreach (var account in accounts) foreach (var account in accounts)
{ {
yield return AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags); aces.Add(AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags));
} }
} }
else else
@ -105,9 +115,11 @@ namespace Security2
var item = new DirectoryInfo(path); var item = new DirectoryInfo(path);
foreach (var account in accounts) foreach (var account in accounts)
{ {
yield return AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags); aces.Add(AddFileSystemAccessRule(item, account, rights, type, inheritanceFlags, propagationFlags));
} }
} }
return aces;
} }
public static void AddFileSystemAccessRule(FileSystemAccessRule2 rule) public static void AddFileSystemAccessRule(FileSystemAccessRule2 rule)

16
Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.AddFileSystemAuditRules.cs

@ -78,14 +78,24 @@ namespace Security2
return aces; return aces;
} }
/// <summary>
/// Adds one audit entry per account to the item at the path, and returns them in the order of
/// <paramref name="accounts"/>. Before 5.0.0, this overload was an iterator (a yield return in the loop
/// below), so nothing happened until the caller enumerated the result; called as a statement, or assigned to
/// a variable that is never read, it changed nothing. It now builds the list and writes the entries before
/// returning, like its FileSystemInfo and FileSystemSecurity2 siblings; an exception that adding an entry
/// raises is now thrown by this call, not by the later enumeration.
/// </summary>
public static IEnumerable<FileSystemAuditRule2> AddFileSystemAuditRule(string path, List<IdentityReference2> accounts, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags) public static IEnumerable<FileSystemAuditRule2> AddFileSystemAuditRule(string path, List<IdentityReference2> accounts, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags)
{ {
var aces = new List<FileSystemAuditRule2>();
if (File.Exists(path)) if (File.Exists(path))
{ {
var item = new FileInfo(path); var item = new FileInfo(path);
foreach (var account in accounts) foreach (var account in accounts)
{ {
yield return AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags); aces.Add(AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags));
} }
} }
else else
@ -93,9 +103,11 @@ namespace Security2
var item = new DirectoryInfo(path); var item = new DirectoryInfo(path);
foreach (var account in accounts) foreach (var account in accounts)
{ {
yield return AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags); aces.Add(AddFileSystemAuditRule(item, account, rights, type, inheritanceFlags, propagationFlags));
} }
} }
return aces;
} }
public static void AddFileSystemAuditRule(FileSystemAuditRule2 rule) public static void AddFileSystemAuditRule(FileSystemAuditRule2 rule)

25
Tests/ObjectApis.Tests.ps1

@ -395,9 +395,11 @@ Describe 'Access rule helpers that take a path' {
$entries[0].FileSystemRights | Should -Be ([System.Security.AccessControl.FileSystemRights] 'ReadData, Synchronize') $entries[0].FileSystemRights | Should -Be ([System.Security.AccessControl.FileSystemRights] 'ReadData, Synchronize')
} }
# The overload that takes a path returns an iterator, so the caller must enumerate the result to write the entries. # Before 5.0.0, the overload that takes a path was an iterator (yield return) that changed nothing until the
# The overloads that take an item write them at once; this test doesn't pin the difference. # caller enumerated the result, so a call as a statement, or an assignment that the caller never reads, wrote
It 'Should add the entries of several accounts to a <Kind> by its path when the result is enumerated' -ForEach @( # nothing; the overloads that take an item or a FileSystemSecurity2 were never lazy. It now writes at once, like
# its siblings, so the entries exist before $pending is ever touched.
It 'Should add the entries of several accounts to a <Kind> by its path at once, before the result is ever enumerated' -ForEach @(
@{ Kind = 'file'; Directory = $false } @{ Kind = 'file'; Directory = $false }
@{ Kind = 'folder'; Directory = $true } @{ Kind = 'folder'; Directory = $true }
) { ) {
@ -408,9 +410,13 @@ Describe 'Access rule helpers that take a path' {
$path, $accounts, [Security2.FileSystemRights2]::ReadData, $allow, $noInheritance, $noPropagation $path, $accounts, [Security2.FileSystemRights2]::ReadData, $allow, $noInheritance, $noPropagation
) )
@($pending) | Should -HaveCount 2
@(Get-ExplicitEntries -Path $path) | Should -HaveCount 1 @(Get-ExplicitEntries -Path $path) | Should -HaveCount 1
@(Get-ExplicitEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1 @(Get-ExplicitEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1
$added = @($pending)
$added | Should -HaveCount 2
$added[0].Account.Sid | Should -BeExactly $identity.Sid
$added[1].Account.Sid | Should -BeExactly $users.Sid
} }
It 'Should add the deny entries of several accounts without Synchronize when the result is enumerated' { It 'Should add the deny entries of several accounts without Synchronize when the result is enumerated' {
@ -589,7 +595,10 @@ Describe 'Audit rule helpers that take a path' -Skip:(-not $holdsSecurityPrivile
$found[0].FullName | Should -BeExactly $path $found[0].FullName | Should -BeExactly $path
} }
It 'Should add the entries of several accounts to a <Kind> by its path when the result is enumerated, and remove them again' -ForEach @( # Before 5.0.0, the overload that takes a path was an iterator (yield return) that changed nothing until the
# caller enumerated the result; it now writes at once, like its siblings, so the entries exist before $pending is
# ever touched. This overload needs the Security privilege like its neighbors, so the whole Describe skips without it.
It 'Should add the entries of several accounts to a <Kind> by its path at once, before the result is ever enumerated, and remove them again' -ForEach @(
@{ Kind = 'file'; Directory = $false } @{ Kind = 'file'; Directory = $false }
@{ Kind = 'folder'; Directory = $true } @{ Kind = 'folder'; Directory = $true }
) { ) {
@ -602,9 +611,13 @@ Describe 'Audit rule helpers that take a path' -Skip:(-not $holdsSecurityPrivile
$path, $accounts, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation $path, $accounts, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation
) )
@($pending) | Should -HaveCount 2
@(Get-AuditEntries -Path $path) | Should -HaveCount 1 @(Get-AuditEntries -Path $path) | Should -HaveCount 1
@(Get-AuditEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1 @(Get-AuditEntries -Path $path -Account 'S-1-5-32-545') | Should -HaveCount 1
$added = @($pending)
$added | Should -HaveCount 2
$added[0].Account.Sid | Should -BeExactly $identity.Sid
$added[1].Account.Sid | Should -BeExactly $users.Sid
[Security2.FileSystemAuditRule2]::RemoveFileSystemAuditRule( [Security2.FileSystemAuditRule2]::RemoveFileSystemAuditRule(
$path, $identity, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation, $true $path, $identity, [Security2.FileSystemRights2]::Delete, $success, $noInheritance, $noPropagation, $true
) )

Loading…
Cancel
Save