diff --git a/Security2/FileSystem/FileSystemSecurity2.cs b/Security2/FileSystem/FileSystemSecurity2.cs index cf905ba..9e29088 100644 --- a/Security2/FileSystem/FileSystemSecurity2.cs +++ b/Security2/FileSystem/FileSystemSecurity2.cs @@ -22,9 +22,12 @@ namespace Security2 set { item = value; } } - public string FullName { get { return item.FullName; } } + // An object that wraps a descriptor directly, such as the conversion from FileSecurity or DirectorySecurity, + // has no item and so no path to report; null reads more naturally here than an empty string, which would look + // like the path of an item whose name happens to be empty. + public string FullName { get { return item != null ? item.FullName : null; } } - public string Name { get { return item.Name; } } + public string Name { get { return item != null ? item.Name : null; } } public bool IsFile { get { return isFile; } } @@ -39,6 +42,18 @@ namespace Security2 RememberSections(); } + // Wraps a descriptor that a caller created directly, such as the conversions from FileSecurity and + // DirectorySecurity do; the object has no item. The descriptor is treated as if every section had been read, + // since there was no partial read: WriteChanges compares against it as a whole. + private FileSystemSecurity2(FileSystemSecurity sd) + { + this.sd = sd; + isFile = sd is FileSecurity; + sections = AccessControlSections.All; + + RememberSections(); + } + public FileSystemSecurity2(FileSystemInfo item) { this.item = item; @@ -195,6 +210,8 @@ namespace Security2 // that is read alone, and writing them back fails for an owner that the user cannot assign (#34). public void Write() { + RequireItem(); + SetSecurity(item, sd, sections); RememberSections(); @@ -204,6 +221,8 @@ namespace Security2 // unchanged owner that the user cannot assign isn't written back (#34). Without a change, it writes nothing. internal void WriteChanges() { + RequireItem(); + var changedSections = ChangedSections; if (changedSections == AccessControlSections.None) { @@ -215,6 +234,20 @@ namespace Security2 RememberSections(); } + // Write() and WriteChanges() write to the item that the descriptor was read from; an object that a conversion + // from FileSecurity or DirectorySecurity created has none. Without this check they would fail with a + // NullReferenceException deep in SetSecurity instead of saying what is missing. + private void RequireItem() + { + if (item == null) + { + throw new InvalidOperationException( + "This security descriptor has no item to write to: it was created directly from a FileSecurity or " + + "DirectorySecurity object, not read from a file or folder. Set the Item property first, or call " + + "Write(FileSystemInfo) or Write(string) to choose where to write it."); + } + } + // Writes the sections that the descriptor was read with to another item, like Write(). public void Write(FileSystemInfo item) { @@ -243,14 +276,17 @@ namespace Security2 #region Conversion // The descriptor is in sd. Before 5.0.0-rc6, these conversions returned fields that were never set, so they - // gave null, and the hash code threw a NullReferenceException. + // gave null, and the hash code threw a NullReferenceException. Before 5.0.0, the conversions the other way + // (from FileSecurity/DirectorySecurity) ignored the descriptor that was passed and returned an object built + // from an empty path, which always threw; they now keep the descriptor that was passed, see the private + // constructor above. public static implicit operator FileSecurity(FileSystemSecurity2 fs2) { return fs2.sd as FileSecurity; } public static implicit operator FileSystemSecurity2(FileSecurity fs) { - return new FileSystemSecurity2(new FileInfo("")); + return new FileSystemSecurity2((FileSystemSecurity)fs); } public static implicit operator DirectorySecurity(FileSystemSecurity2 fs2) @@ -259,7 +295,7 @@ namespace Security2 } public static implicit operator FileSystemSecurity2(DirectorySecurity fs) { - return new FileSystemSecurity2(new DirectoryInfo("")); + return new FileSystemSecurity2((FileSystemSecurity)fs); } // Like the descriptors of .NET, two objects are equal when they wrap the same descriptor; a descriptor of .NET diff --git a/Tests/ObjectApis.Tests.ps1 b/Tests/ObjectApis.Tests.ps1 index 575e34c..eafb0e1 100644 --- a/Tests/ObjectApis.Tests.ps1 +++ b/Tests/ObjectApis.Tests.ps1 @@ -797,6 +797,99 @@ Describe 'Owner and descriptor objects' { } } +# Before 5.0.0, both conversions ignored the descriptor that was passed and returned a FileSystemSecurity2 that wrapped +# a FileInfo or a DirectoryInfo built from an empty path, which AlphaFS rejects, so the conversion always threw. The +# object that the fixed conversion returns has no item: FullName and Name, which have no path to report, are null, +# which reads most naturally as "nothing to name". +Describe 'The FileSystemSecurity2 conversions from FileSecurity and DirectorySecurity' { + BeforeAll { + function New-RawFileSystemSecurity { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseShouldProcessForStateChangingFunctions', '', Justification = 'Builds an in-memory descriptor only.' + )] + param ([string] $Kind) + + $raw = New-Object -TypeName "System.Security.AccessControl.$Kind" + $rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $sid, [System.Security.AccessControl.FileSystemRights]::ReadData, + [System.Security.AccessControl.AccessControlType]::Allow + ) + $raw.AddAccessRule($rule) + $raw + } + } + + It 'Should wrap the descriptor that was passed, report IsFile as , and round-trip the same instance' -ForEach @( + @{ Kind = 'FileSecurity'; IsFile = $true } + @{ Kind = 'DirectorySecurity'; IsFile = $false } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + $targetType = [type] "System.Security.AccessControl.$Kind" + + $wrapped = [Security2.FileSystemSecurity2] $raw + + [object]::ReferenceEquals($wrapped.SecurityDescriptor, $raw) | Should -BeTrue + $wrapped.IsFile | Should -Be $IsFile + [object]::ReferenceEquals(($wrapped -as $targetType), $raw) | Should -BeTrue + $wrapped.FullName | Should -Be $null + $wrapped.Name | Should -Be $null + } + + It 'Should compare two conversions of the same descriptor as equal, and of different descriptors as unequal' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $raw = New-RawFileSystemSecurity -Kind $Kind + + $first = [Security2.FileSystemSecurity2] $raw + $second = [Security2.FileSystemSecurity2] $raw + $other = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $first.Equals($second) | Should -BeTrue + $first.GetHashCode() | Should -Be $second.GetHashCode() + $first.Equals($other) | Should -BeFalse + } + + It 'Should throw an InvalidOperationException, not a NullReferenceException, when a without an item is written' -ForEach @( + @{ Kind = 'FileSecurity' } + @{ Kind = 'DirectorySecurity' } + ) { + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $failure = { $wrapped.Write() } | Should -Throw -PassThru + + $failure.Exception.GetBaseException() | Should -BeOfType [System.InvalidOperationException] + $failure.Exception.GetBaseException().Message | Should -BeLike '*item*' + } + + It 'Should write a without an item once the caller sets Item' -ForEach @( + @{ Kind = 'FileSecurity'; Directory = $false } + @{ Kind = 'DirectorySecurity'; Directory = $true } + ) { + $path = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertedWriteItem' -Directory:$Directory + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + $wrapped.Item = Get-Item2 -Path $path + + $wrapped.Write() + + @((Get-Acl -LiteralPath $path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | + Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 + } + + It 'Should write a without an item directly to a path' -ForEach @( + @{ Kind = 'FileSecurity'; Directory = $false } + @{ Kind = 'DirectorySecurity'; Directory = $true } + ) { + $path = New-TestSandboxItem -Sandbox $sandbox -Name 'ConvertedWritePath' -Directory:$Directory + $wrapped = [Security2.FileSystemSecurity2] (New-RawFileSystemSecurity -Kind $Kind) + + $wrapped.Write([string] $path) + + @((Get-Acl -LiteralPath $path).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | + Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-1-0' }) | Should -HaveCount 1 + } +} + Describe 'Generic access rights and identity errors' { It 'Should map the generic mask to the file system rights ' -ForEach @( @{ Mask = '80000000'; Expected = '00120089' }