From 1bffac487b0809067578eda075859d01c26a59d9 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 15 Jan 2021 20:05:05 -0800 Subject: [PATCH 01/14] FileSystem.AccessControl tests not cleaning files properly The TempDirectory helper class is disposable. The finalizer calls a function that deletes the folders left behind, without throwing exceptions on fail. This protective try catch hid some additional errors that were not caught when these unit tests were first written, so I'm fixing them: - I created a class that inherits from TempDirectory to override the folder deleting method - In the finalizer, I iterate through all the files and folders created by the test, ensure their ACLs give me full control (in case the tests prevent deletion), then attempt to delete the tree. - To verify my changes, I did not put the finalizer code in a try catch, so I could see the errors thrown when attempting to delete the tree. - The deletion exceptions helped me find that GetAccessControl needs to be called with the AccessControlSections.Access argument, otherwise they throw "PrivilegeNotHeldException: The process does not possess the 'SeSecurityPrivilege' privilege which is required for this operation." - I avoided creating files and directories using a FileSecurity or DirectorySecurity that did not have its access rules defined. Files and folders created this way could not be deleted. - I avoided testing AccessControlType.Deny. This would also cause deletion exceptions. - Moved some repeated code into common methods. --- .../Common/tests/System/IO/TempDirectory.cs | 4 +- .../tests/FileSystemAclExtensionsTests.cs | 350 ++++++------------ ...m.IO.FileSystem.AccessControl.Tests.csproj | 1 + .../tests/TempAclDirectory.cs | 42 +++ 4 files changed, 157 insertions(+), 240 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs diff --git a/src/libraries/Common/tests/System/IO/TempDirectory.cs b/src/libraries/Common/tests/System/IO/TempDirectory.cs index c3afec45efb9c7..93c6d96aac875f 100644 --- a/src/libraries/Common/tests/System/IO/TempDirectory.cs +++ b/src/libraries/Common/tests/System/IO/TempDirectory.cs @@ -9,7 +9,7 @@ namespace System.IO /// Represents a temporary directory. Creating an instance creates a directory at the specified path, /// and disposing the instance deletes the directory. /// - public sealed class TempDirectory : IDisposable + public class TempDirectory : IDisposable { public const int MaxNameLength = 255; @@ -40,7 +40,7 @@ public void Dispose() public string GenerateRandomFilePath() => IO.Path.Combine(Path, IO.Path.GetRandomFileName()); - private void DeleteDirectory() + protected virtual void DeleteDirectory() { try { Directory.Delete(Path, recursive: true); } catch { /* Ignore exceptions on disposal paths */ } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 431e8e4154e22c..f3e068c13b965b 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -27,9 +27,9 @@ public void GetAccessControl_DirectoryInfo_InvalidArguments() [Fact] public void GetAccessControl_DirectoryInfo_ReturnsValidObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); - DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(); + DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(AccessControlSections.Access); Assert.NotNull(directorySecurity); Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType); } @@ -43,7 +43,7 @@ public void GetAccessControl_DirectoryInfo_AccessControlSections_InvalidArgument [Fact] public void GetAccessControl_DirectoryInfo_AccessControlSections_ReturnsValidObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); AccessControlSections accessControlSections = new AccessControlSections(); DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(accessControlSections); @@ -60,10 +60,10 @@ public void GetAccessControl_FileInfo_InvalidArguments() [Fact] public void GetAccessControl_FileInfo_ReturnsValidObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); FileInfo fileInfo = new FileInfo(file.Path); - FileSecurity fileSecurity = fileInfo.GetAccessControl(); + FileSecurity fileSecurity = fileInfo.GetAccessControl(AccessControlSections.Access); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); } @@ -77,7 +77,7 @@ public void GetAccessControl_FileInfo_AccessControlSections_InvalidArguments() [Fact] public void GetAccessControl_FileInfo_AccessControlSections_ReturnsValidObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); FileInfo fileInfo = new FileInfo(file.Path); AccessControlSections accessControlSections = new AccessControlSections(); @@ -95,9 +95,9 @@ public void GetAccessControl_Filestream_InvalidArguments() [Fact] public void GetAccessControl_Filestream_ReturnValidObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); FileSecurity fileSecurity = FileSystemAclExtensions.GetAccessControl(fileStream); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); @@ -110,7 +110,7 @@ public void GetAccessControl_Filestream_ReturnValidObject() [Fact] public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); AssertExtensions.Throws("directorySecurity", () => directoryInfo.SetAccessControl(directorySecurity: null)); } @@ -118,7 +118,7 @@ public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments() [Fact] public void SetAccessControl_DirectoryInfo_DirectorySecurity_Success() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); DirectorySecurity directorySecurity = new DirectorySecurity(); directoryInfo.SetAccessControl(directorySecurity); @@ -127,7 +127,7 @@ public void SetAccessControl_DirectoryInfo_DirectorySecurity_Success() [Fact] public void SetAccessControl_FileInfo_FileSecurity_InvalidArguments() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); FileInfo fileInfo = new FileInfo(file.Path); AssertExtensions.Throws("fileSecurity", () => fileInfo.SetAccessControl(fileSecurity: null)); @@ -136,7 +136,7 @@ public void SetAccessControl_FileInfo_FileSecurity_InvalidArguments() [Fact] public void SetAccessControl_FileInfo_FileSecurity_Success() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); FileInfo fileInfo = new FileInfo(file.Path); FileSecurity fileSecurity = new FileSecurity(); @@ -152,18 +152,18 @@ public void SetAccessControl_FileStream_FileSecurity_InvalidArguments() [Fact] public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); AssertExtensions.Throws("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null)); } [Fact] public void SetAccessControl_FileStream_FileSecurity_Success() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); FileSecurity fileSecurity = new FileSecurity(); FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity); } @@ -177,79 +177,58 @@ public void DirectoryInfo_Create_NullDirectoryInfo() { DirectoryInfo info = null; DirectorySecurity security = new DirectorySecurity(); - - Assert.Throws("directoryInfo", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, security); - } - else - { - info.Create(security); - } - }); + Assert.Throws("directoryInfo", () => DirectoryInfo_Create_Framework(info, security)); } [Fact] public void DirectoryInfo_Create_NullDirectorySecurity() { DirectoryInfo info = new DirectoryInfo("path"); - - Assert.Throws("directorySecurity", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, null); - } - else - { - info.Create(null); - } - }); + Assert.Throws("directorySecurity", () => DirectoryInfo_Create_Framework(info, null)); } [Fact] public void DirectoryInfo_Create_NotFound() { - using var directory = new TempDirectory(); - string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist"); + var directory = new TempAclDirectory(); + string path = Path.Combine(directory.Path, "ParentDoesNotExist"); + directory.Dispose(); // Delete parent folder + DirectoryInfo info = new DirectoryInfo(path); - DirectorySecurity security = new DirectorySecurity(); + DirectorySecurity security = GetDirectorySecurity(FileSystemRights.FullControl); + DirectoryInfo_Create_Framework(info, security); + } - Assert.Throws(() => + private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurity security) + { + if (PlatformDetection.IsNetFramework) { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, security); - } - else - { - info.Create(security); - } - }); + FileSystemAclExtensions.Create(info, security); + } + else + { + info.Create(security); + } } [Fact] - public void DirectoryInfo_Create_DefaultDirectorySecurity() + public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule() { - DirectorySecurity security = new DirectorySecurity(); - Verify_DirectoryInfo_Create(security); - } + using var directory = new TempAclDirectory(); + string path = Path.Combine(directory.Path, "directory"); + DirectoryInfo info = new DirectoryInfo(path); - [Theory] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Allow)] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Deny)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Allow)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Deny)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Allow)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Deny)] - public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule( - FileSystemRights rights, - AccessControlType controlType) - { - DirectorySecurity security = GetDirectorySecurity(rights, controlType); - Verify_DirectoryInfo_Create(security); + DirectorySecurity expectedSecurity = GetDirectorySecurity(FileSystemRights.FullControl); + + info.Create(expectedSecurity); + + Assert.True(Directory.Exists(path)); + + DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); + + DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); + + VerifyAccessSecurity(expectedSecurity, actualSecurity); } #endregion @@ -263,16 +242,7 @@ public void FileInfo_Create_NullFileInfo() FileSecurity security = new FileSecurity(); Assert.Throws("fileInfo", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - else - { - info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Fact] @@ -281,37 +251,19 @@ public void FileInfo_Create_NullFileSecurity() FileInfo info = new FileInfo("path"); Assert.Throws("fileSecurity", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, null); - } - else - { - info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, null); - } - }); + FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, null)); } [Fact] public void FileInfo_Create_NotFound() { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "file.txt"); FileInfo info = new FileInfo(path); FileSecurity security = new FileSecurity(); Assert.Throws(() => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - else - { - info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -324,16 +276,7 @@ public void FileInfo_Create_FileSecurity_InvalidFileMode(FileMode invalidMode) FileInfo info = new FileInfo("path"); Assert.Throws("mode", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, invalidMode, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); ; - } - else - { - info.Create(invalidMode, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, invalidMode, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -345,16 +288,7 @@ public void FileInfo_Create_FileSecurity_InvalidFileShare(FileShare invalidFileS FileInfo info = new FileInfo("path"); Assert.Throws("share", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security); - } - else - { - info.Create(FileMode.Create, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -366,16 +300,7 @@ public void FileInfo_Create_FileSecurity_InvalidBufferSize(int invalidBufferSize FileInfo info = new FileInfo("path"); Assert.Throws("bufferSize", () => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, FileMode.Create, FileSystemRights.WriteData, FileShare.Read, invalidBufferSize, FileOptions.None, security); - } - else - { - info.Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, invalidBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, invalidBufferSize, FileOptions.None, security)); } [Theory] @@ -393,36 +318,46 @@ public void FileInfo_Create_FileSecurity_ForbiddenCombo_FileModeFileSystemSecuri FileInfo info = new FileInfo("path"); Assert.Throws(() => - { - if (PlatformDetection.IsNetFramework) - { - FileSystemAclExtensions.Create(info, mode, rights, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - else - { - info.Create(mode, rights, FileShare.Read, DefaultBufferSize, FileOptions.None, security); - } - }); + FileInfo_Create_Framework(info, mode, rights, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } - [Fact] - public void FileInfo_Create_DefaultFileSecurity() + private void FileInfo_Create_Framework(FileInfo info, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity security) { - FileSecurity security = new FileSecurity(); - Verify_FileInfo_Create(security); + if (PlatformDetection.IsNetFramework) + { + FileSystemAclExtensions.Create(info, mode, rights, share, bufferSize, options, security); + } + else + { + info.Create(mode, rights, share, bufferSize, options, security); + } } - [Theory] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Allow)] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Deny)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Allow)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Deny)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Allow)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Deny)] - public void FileInfo_Create_FileSecurity_SpecificAccessRule(FileSystemRights rights, AccessControlType controlType) + [Fact] + public void FileInfo_Create_FileSecurity_SpecificAccessRule() { - FileSecurity security = GetFileSecurity(rights, controlType); - Verify_FileInfo_Create(security); + using var directory = new TempAclDirectory(); + + string path = Path.Combine(directory.Path, "file.txt"); + FileInfo info = new FileInfo(path); + + FileSecurity expectedSecurity = GetFileSecurity(FileSystemRights.FullControl); + + info.Create( + FileMode.Create, + FileSystemRights.FullControl, + FileShare.ReadWrite | FileShare.Delete, + DefaultBufferSize, + FileOptions.None, + expectedSecurity); + + Assert.True(File.Exists(path)); + + FileInfo actualInfo = new FileInfo(info.FullName); + + FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); + + VerifyAccessSecurity(expectedSecurity, actualSecurity); } #endregion @@ -450,46 +385,25 @@ public void DirectorySecurity_CreateDirectory_InvalidPath() } [Fact] - public void DirectorySecurity_CreateDirectory_DefaultDirectorySecurity() - { - DirectorySecurity security = new DirectorySecurity(); - Verify_DirectorySecurity_CreateDirectory(security); - } - - [Theory] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Allow)] - [InlineData(FileSystemRights.ReadAndExecute, AccessControlType.Deny)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Allow)] - [InlineData(FileSystemRights.WriteData, AccessControlType.Deny)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Allow)] - [InlineData(FileSystemRights.FullControl, AccessControlType.Deny)] - public void DirectorySecurity_CreateDirectory_DirectorySecurityWithSpecificAccessRule( - FileSystemRights rights, - AccessControlType controlType) + public void DirectorySecurity_CreateDirectory_DirectorySecurityWithSpecificAccessRule() { - DirectorySecurity security = GetDirectorySecurity(rights, controlType); - Verify_DirectorySecurity_CreateDirectory(security); - } - - [Fact] - public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists() - { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); string path = Path.Combine(directory.Path, "createMe"); - DirectorySecurity basicSecurity = new DirectorySecurity(); - basicSecurity.CreateDirectory(path); + DirectorySecurity expectedSecurity = GetDirectorySecurity(FileSystemRights.FullControl); + + expectedSecurity.CreateDirectory(path); Assert.True(Directory.Exists(path)); - DirectorySecurity specificSecurity = GetDirectorySecurity(FileSystemRights.ExecuteFile, AccessControlType.Deny); + DirectorySecurity basicSecurity = new DirectorySecurity(); - // Already exists, existingDirInfo should have the original basic security, not the new specific security - DirectoryInfo existingDirInfo = specificSecurity.CreateDirectory(path); + // Already exists, existingDirInfo should have the original security, not the new basic security + DirectoryInfo existingDirInfo = basicSecurity.CreateDirectory(path); - DirectorySecurity actualSecurity = existingDirInfo.GetAccessControl(); + DirectorySecurity actualSecurity = existingDirInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(basicSecurity, actualSecurity); + VerifyAccessSecurity(expectedSecurity, actualSecurity); } #endregion @@ -499,39 +413,22 @@ public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists() #region Helper methods - private DirectorySecurity GetDirectorySecurity(FileSystemRights rights, AccessControlType controlType) => GetDirectorySecurity(WellKnownSidType.BuiltinUsersSid, rights, controlType); - - private DirectorySecurity GetDirectorySecurity(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) + private DirectorySecurity GetDirectorySecurity(FileSystemRights rights) { DirectorySecurity security = new DirectorySecurity(); - SecurityIdentifier identity = new SecurityIdentifier(sid, null); - FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, controlType); - security.AddAccessRule(accessRule); - - return security; - } - - private void Verify_DirectoryInfo_Create(DirectorySecurity expectedSecurity) - { - using var directory = new TempDirectory(); - string path = Path.Combine(directory.Path, "directory"); - DirectoryInfo info = new DirectoryInfo(path); - - info.Create(expectedSecurity); - - Assert.True(Directory.Exists(path)); + SecurityIdentifier identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); + FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); - DirectorySecurity actualSecurity = actualInfo.GetAccessControl(); + security.AddAccessRule(accessRule); - VerifyAccessSecurity(expectedSecurity, actualSecurity); + return security; } private void Verify_DirectorySecurity_CreateDirectory(DirectorySecurity expectedSecurity) { - using var directory = new TempDirectory(); + using var directory = new TempAclDirectory(); string path = Path.Combine(directory.Path, "createMe"); expectedSecurity.CreateDirectory(path); @@ -540,45 +437,22 @@ private void Verify_DirectorySecurity_CreateDirectory(DirectorySecurity expected DirectoryInfo actualInfo = new DirectoryInfo(path); - DirectorySecurity actualSecurity = actualInfo.GetAccessControl(); + DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); VerifyAccessSecurity(expectedSecurity, actualSecurity); } - private FileSecurity GetFileSecurity(FileSystemRights rights, AccessControlType controlType) => GetFileSecurity(WellKnownSidType.BuiltinUsersSid, rights, controlType); - - private FileSecurity GetFileSecurity(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType) + private FileSecurity GetFileSecurity(FileSystemRights rights) { FileSecurity security = new FileSecurity(); - SecurityIdentifier identity = new SecurityIdentifier(sid, null); - FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, controlType); - security.AddAccessRule(accessRule); + SecurityIdentifier identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - return security; - } + FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); - private void Verify_FileInfo_Create(FileSecurity expectedSecurity) - { - Verify_FileInfo_Create(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, expectedSecurity); - } - - private void Verify_FileInfo_Create(FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity expectedSecurity) - { - using var directory = new TempDirectory(); - - string path = Path.Combine(directory.Path, "file.txt"); - FileInfo info = new FileInfo(path); - - info.Create(mode, rights, share, bufferSize, options, expectedSecurity); - - Assert.True(File.Exists(path)); - - FileInfo actualInfo = new FileInfo(info.FullName); - - FileSecurity actualSecurity = actualInfo.GetAccessControl(); + security.AddAccessRule(accessRule); - VerifyAccessSecurity(expectedSecurity, actualSecurity); + return security; } private void VerifyAccessSecurity(CommonObjectSecurity expectedSecurity, CommonObjectSecurity actualSecurity) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/System.IO.FileSystem.AccessControl.Tests.csproj b/src/libraries/System.IO.FileSystem.AccessControl/tests/System.IO.FileSystem.AccessControl.Tests.csproj index c3d83e8544137f..8c4fa0e42afcd1 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/System.IO.FileSystem.AccessControl.Tests.csproj +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/System.IO.FileSystem.AccessControl.Tests.csproj @@ -13,6 +13,7 @@ + diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs new file mode 100644 index 00000000000000..194f2fce7ae361 --- /dev/null +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.AccessControl; +using System.Security.Principal; + +namespace System.IO +{ + /// + /// Represents a temporary directory. + /// Disposing will recurse all files and directories inside it, ensure the + /// appropriate access control is set, then delete all of them. + /// + public sealed class TempAclDirectory : TempDirectory + { + protected override void DeleteDirectory() + { + try + { + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); + + foreach (string file in Directory.EnumerateFiles(Path, "*", SearchOption.AllDirectories)) + { + var fileSecurity = new FileSecurity(file, AccessControlSections.Access); + var fileInfo = new FileInfo(file); + fileInfo.SetAccessControl(fileSecurity); + } + + foreach (string directory in Directory.EnumerateDirectories(Path, "*", SearchOption.AllDirectories)) + { + var directorySecurity = new DirectorySecurity(directory, AccessControlSections.Access); + var directoryInfo = new DirectoryInfo(directory); + directoryInfo.SetAccessControl(directorySecurity); + } + + Directory.Delete(Path, recursive: true); + } + catch () { /* Do not throw because we call this on finalize */ } + } + } +} From f527694043af0f285ee5738742d3b0c96500cf11 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Tue, 26 Jan 2021 12:50:35 -0800 Subject: [PATCH 02/14] Temp commit to verify files and dirs deleted without try catch --- .../tests/TempAclDirectory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs index 194f2fce7ae361..01de1eb12da215 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -15,8 +15,8 @@ public sealed class TempAclDirectory : TempDirectory { protected override void DeleteDirectory() { - try - { + //try + //{ var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); @@ -35,8 +35,8 @@ protected override void DeleteDirectory() } Directory.Delete(Path, recursive: true); - } - catch () { /* Do not throw because we call this on finalize */ } + //} + // catch { /* Do not throw because we call this on finalize */ } } } } From 5dfd51f25bf01a6669efac975eacefa99693910c Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Tue, 26 Jan 2021 13:32:33 -0800 Subject: [PATCH 03/14] Set directory security first, then file security, in DeleteDirectory. Bring back InlineData for test, but only when a Read is included. --- .../tests/FileSystemAclExtensionsTests.cs | 13 +++++++++--- .../tests/TempAclDirectory.cs | 21 ++++++++++--------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index f3e068c13b965b..0accc24147bead 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -211,14 +211,21 @@ private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurit } } - [Fact] - public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule() + [Theory] + // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose + [InlineData(FileSystemRights.FullControl)] + [InlineData(FileSystemRights.Read)] + [InlineData(FileSystemRights.Read | FileSystemRights.Write)] + [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] + [InlineData(FileSystemRights.ReadAndExecute)] + [InlineData(FileSystemRights.ReadAttributes | FileSystemRights.ReadData | FileSystemRights.ReadPermissions)] + public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(FileSystemRights rights) { using var directory = new TempAclDirectory(); string path = Path.Combine(directory.Path, "directory"); DirectoryInfo info = new DirectoryInfo(path); - DirectorySecurity expectedSecurity = GetDirectorySecurity(FileSystemRights.FullControl); + DirectorySecurity expectedSecurity = GetDirectorySecurity(rights); info.Create(expectedSecurity); diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs index 01de1eb12da215..7fdd0845da1e0f 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Security.AccessControl; using System.Security.Principal; @@ -20,23 +21,23 @@ protected override void DeleteDirectory() var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); - foreach (string file in Directory.EnumerateFiles(Path, "*", SearchOption.AllDirectories)) + foreach (string dirPath in Directory.EnumerateDirectories(Path, "*", SearchOption.AllDirectories)) { - var fileSecurity = new FileSecurity(file, AccessControlSections.Access); - var fileInfo = new FileInfo(file); - fileInfo.SetAccessControl(fileSecurity); + var dirInfo = new DirectoryInfo(dirPath); + dirInfo.SetAccessControl(new DirectorySecurity(dirPath, AccessControlSections.Access)); } - foreach (string directory in Directory.EnumerateDirectories(Path, "*", SearchOption.AllDirectories)) + foreach (string filePath in Directory.EnumerateFiles(Path, "*", SearchOption.AllDirectories)) { - var directorySecurity = new DirectorySecurity(directory, AccessControlSections.Access); - var directoryInfo = new DirectoryInfo(directory); - directoryInfo.SetAccessControl(directorySecurity); + var fileInfo = new FileInfo(filePath); + fileInfo.SetAccessControl(new FileSecurity(filePath, AccessControlSections.Access)); } - Directory.Delete(Path, recursive: true); + var rootDirInfo = new DirectoryInfo(Path); + rootDirInfo.SetAccessControl(new DirectorySecurity(Path, AccessControlSections.Access)); + rootDirInfo.Delete(recursive: true); //} - // catch { /* Do not throw because we call this on finalize */ } + //catch { /* Do not throw because we call this on finalize */ } } } } From cb4bec400924dc1c4497298fd03e41e5fa31c3f5 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Tue, 26 Jan 2021 16:05:52 -0800 Subject: [PATCH 04/14] Restore try catch before merging. --- .../tests/TempAclDirectory.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs index 7fdd0845da1e0f..0e5060dcf96b01 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -16,8 +16,8 @@ public sealed class TempAclDirectory : TempDirectory { protected override void DeleteDirectory() { - //try - //{ + try + { var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); @@ -36,8 +36,8 @@ protected override void DeleteDirectory() var rootDirInfo = new DirectoryInfo(Path); rootDirInfo.SetAccessControl(new DirectorySecurity(Path, AccessControlSections.Access)); rootDirInfo.Delete(recursive: true); - //} - //catch { /* Do not throw because we call this on finalize */ } + } + catch { /* Do not throw because we call this on finalize */ } } } } From df559c0a1b8907e50e4f35132e94a011fa2e3c7e Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 1 Feb 2021 19:11:29 -0800 Subject: [PATCH 05/14] Address suggestions. --- .../tests/FileSystemAclExtensionsTests.cs | 79 +++++++++++++++++-- .../tests/TempAclDirectory.cs | 12 ++- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 0accc24147bead..59e2d18fa691c5 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -218,7 +218,6 @@ private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurit [InlineData(FileSystemRights.Read | FileSystemRights.Write)] [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] [InlineData(FileSystemRights.ReadAndExecute)] - [InlineData(FileSystemRights.ReadAttributes | FileSystemRights.ReadData | FileSystemRights.ReadPermissions)] public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(FileSystemRights rights) { using var directory = new TempAclDirectory(); @@ -238,6 +237,36 @@ public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(FileSys VerifyAccessSecurity(expectedSecurity, actualSecurity); } + [Theory] + [InlineData(FileSystemRights.TakeOwnership)] + [InlineData(FileSystemRights.Write)] + public void DirectoryInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) + { + var expectedSecurity = new DirectorySecurity(); + + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + var allowAccessRule = new FileSystemAccessRule(identity, FileSystemRights.Read, AccessControlType.Allow); + expectedSecurity.AddAccessRule(allowAccessRule); + + var denyAccessRule = new FileSystemAccessRule(identity, rightsToDeny, AccessControlType.Deny); + expectedSecurity.AddAccessRule(denyAccessRule); + + using var directory = new TempAclDirectory(); + string path = Path.Combine(directory.Path, "directory"); + DirectoryInfo info = new DirectoryInfo(path); + + info.Create(expectedSecurity); + + Assert.True(Directory.Exists(path)); + + DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); + + DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); + + VerifyAccessSecurity(expectedSecurity, actualSecurity); + } + #endregion #region FileInfo Create @@ -350,7 +379,7 @@ public void FileInfo_Create_FileSecurity_SpecificAccessRule() FileSecurity expectedSecurity = GetFileSecurity(FileSystemRights.FullControl); - info.Create( + using FileStream stream = info.Create( FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite | FileShare.Delete, @@ -360,7 +389,45 @@ public void FileInfo_Create_FileSecurity_SpecificAccessRule() Assert.True(File.Exists(path)); - FileInfo actualInfo = new FileInfo(info.FullName); + var actualInfo = new FileInfo(info.FullName); + + FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); + + VerifyAccessSecurity(expectedSecurity, actualSecurity); + } + + + [Theory] + [InlineData(FileSystemRights.TakeOwnership)] + [InlineData(FileSystemRights.Write)] + public void FileInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) + { + var expectedSecurity = new FileSecurity(); + + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + var allowAccessRule = new FileSystemAccessRule(identity, FileSystemRights.Read, AccessControlType.Allow); + expectedSecurity.AddAccessRule(allowAccessRule); + + var denyAccessRule = new FileSystemAccessRule(identity, rightsToDeny, AccessControlType.Deny); + expectedSecurity.AddAccessRule(denyAccessRule); + + using var directory = new TempAclDirectory(); + + string path = Path.Combine(directory.Path, "file.txt"); + var info = new FileInfo(path); + + using FileStream stream = info.Create( + FileMode.Create, + FileSystemRights.FullControl, + FileShare.ReadWrite | FileShare.Delete, + DefaultBufferSize, + FileOptions.None, + expectedSecurity); + + Assert.True(File.Exists(path)); + + var actualInfo = new FileInfo(info.FullName); FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); @@ -392,7 +459,7 @@ public void DirectorySecurity_CreateDirectory_InvalidPath() } [Fact] - public void DirectorySecurity_CreateDirectory_DirectorySecurityWithSpecificAccessRule() + public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists() { using var directory = new TempAclDirectory(); string path = Path.Combine(directory.Path, "createMe"); @@ -423,13 +490,9 @@ public void DirectorySecurity_CreateDirectory_DirectorySecurityWithSpecificAcces private DirectorySecurity GetDirectorySecurity(FileSystemRights rights) { DirectorySecurity security = new DirectorySecurity(); - SecurityIdentifier identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); - security.AddAccessRule(accessRule); - return security; } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs index 0e5060dcf96b01..3844ab2a879946 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -24,17 +24,23 @@ protected override void DeleteDirectory() foreach (string dirPath in Directory.EnumerateDirectories(Path, "*", SearchOption.AllDirectories)) { var dirInfo = new DirectoryInfo(dirPath); - dirInfo.SetAccessControl(new DirectorySecurity(dirPath, AccessControlSections.Access)); + var dirSecurity = new DirectorySecurity(dirPath, AccessControlSections.Access); + dirSecurity.AddAccessRule(accessRule); + dirInfo.SetAccessControl(dirSecurity); } foreach (string filePath in Directory.EnumerateFiles(Path, "*", SearchOption.AllDirectories)) { var fileInfo = new FileInfo(filePath); - fileInfo.SetAccessControl(new FileSecurity(filePath, AccessControlSections.Access)); + var fileSecurity = new FileSecurity(filePath, AccessControlSections.Access); + fileSecurity.AddAccessRule(accessRule); + fileInfo.SetAccessControl(fileSecurity); } var rootDirInfo = new DirectoryInfo(Path); - rootDirInfo.SetAccessControl(new DirectorySecurity(Path, AccessControlSections.Access)); + var rootSecurity = new DirectorySecurity(Path, AccessControlSections.Access); + rootSecurity.AddAccessRule(accessRule); + rootDirInfo.SetAccessControl(rootSecurity); rootDirInfo.Delete(recursive: true); } catch { /* Do not throw because we call this on finalize */ } From ba6bf54a84c2676a9f4aa6cea6fe89bfb3c701a1 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 14:23:05 -0800 Subject: [PATCH 06/14] Bring back tests that consume the parameterless security constructors. Bring back all FileSystemRights with annotations on why some need to be skipped. Ensure DeleteDirectory can successfully reset permissions before deleting all files and folders deleted by the unit test. --- .../tests/FileSystemAclExtensionsTests.cs | 252 ++++++++++-------- .../tests/TempAclDirectory.cs | 49 ++-- 2 files changed, 179 insertions(+), 122 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 59e2d18fa691c5..4ccfa6aeb5bdaa 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -28,7 +28,7 @@ public void GetAccessControl_DirectoryInfo_InvalidArguments() public void GetAccessControl_DirectoryInfo_ReturnsValidObject() { using var directory = new TempAclDirectory(); - DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); + var directoryInfo = new DirectoryInfo(directory.Path); DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(AccessControlSections.Access); Assert.NotNull(directorySecurity); Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType); @@ -44,8 +44,8 @@ public void GetAccessControl_DirectoryInfo_AccessControlSections_InvalidArgument public void GetAccessControl_DirectoryInfo_AccessControlSections_ReturnsValidObject() { using var directory = new TempAclDirectory(); - DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); - AccessControlSections accessControlSections = new AccessControlSections(); + var directoryInfo = new DirectoryInfo(directory.Path); + var accessControlSections = new AccessControlSections(); DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(accessControlSections); Assert.NotNull(directorySecurity); Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType); @@ -62,7 +62,7 @@ public void GetAccessControl_FileInfo_ReturnsValidObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - FileInfo fileInfo = new FileInfo(file.Path); + var fileInfo = new FileInfo(file.Path); FileSecurity fileSecurity = fileInfo.GetAccessControl(AccessControlSections.Access); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); @@ -79,8 +79,8 @@ public void GetAccessControl_FileInfo_AccessControlSections_ReturnsValidObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - FileInfo fileInfo = new FileInfo(file.Path); - AccessControlSections accessControlSections = new AccessControlSections(); + var fileInfo = new FileInfo(file.Path); + var accessControlSections = new AccessControlSections(); FileSecurity fileSecurity = fileInfo.GetAccessControl(accessControlSections); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); @@ -111,7 +111,7 @@ public void GetAccessControl_Filestream_ReturnValidObject() public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments() { using var directory = new TempAclDirectory(); - DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); + var directoryInfo = new DirectoryInfo(directory.Path); AssertExtensions.Throws("directorySecurity", () => directoryInfo.SetAccessControl(directorySecurity: null)); } @@ -119,8 +119,8 @@ public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments() public void SetAccessControl_DirectoryInfo_DirectorySecurity_Success() { using var directory = new TempAclDirectory(); - DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path); - DirectorySecurity directorySecurity = new DirectorySecurity(); + var directoryInfo = new DirectoryInfo(directory.Path); + var directorySecurity = new DirectorySecurity(); directoryInfo.SetAccessControl(directorySecurity); } @@ -129,7 +129,7 @@ public void SetAccessControl_FileInfo_FileSecurity_InvalidArguments() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - FileInfo fileInfo = new FileInfo(file.Path); + var fileInfo = new FileInfo(file.Path); AssertExtensions.Throws("fileSecurity", () => fileInfo.SetAccessControl(fileSecurity: null)); } @@ -138,8 +138,8 @@ public void SetAccessControl_FileInfo_FileSecurity_Success() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - FileInfo fileInfo = new FileInfo(file.Path); - FileSecurity fileSecurity = new FileSecurity(); + var fileInfo = new FileInfo(file.Path); + var fileSecurity = new FileSecurity(); fileInfo.SetAccessControl(fileSecurity); } @@ -164,7 +164,7 @@ public void SetAccessControl_FileStream_FileSecurity_Success() using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); - FileSecurity fileSecurity = new FileSecurity(); + var fileSecurity = new FileSecurity(); FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity); } @@ -176,14 +176,14 @@ public void SetAccessControl_FileStream_FileSecurity_Success() public void DirectoryInfo_Create_NullDirectoryInfo() { DirectoryInfo info = null; - DirectorySecurity security = new DirectorySecurity(); + var security = new DirectorySecurity(); Assert.Throws("directoryInfo", () => DirectoryInfo_Create_Framework(info, security)); } [Fact] public void DirectoryInfo_Create_NullDirectorySecurity() { - DirectoryInfo info = new DirectoryInfo("path"); + var info = new DirectoryInfo("path"); Assert.Throws("directorySecurity", () => DirectoryInfo_Create_Framework(info, null)); } @@ -194,7 +194,7 @@ public void DirectoryInfo_Create_NotFound() string path = Path.Combine(directory.Path, "ParentDoesNotExist"); directory.Dispose(); // Delete parent folder - DirectoryInfo info = new DirectoryInfo(path); + var info = new DirectoryInfo(path); DirectorySecurity security = GetDirectorySecurity(FileSystemRights.FullControl); DirectoryInfo_Create_Framework(info, security); } @@ -211,6 +211,13 @@ private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurit } } + [Fact] + public void DirectoryInfo_Create_DefaultDirectorySecurity() + { + var security = new DirectorySecurity(); + Verify_DirectorySecurity_CreateDirectory(security); + } + [Theory] // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose [InlineData(FileSystemRights.FullControl)] @@ -220,30 +227,25 @@ private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurit [InlineData(FileSystemRights.ReadAndExecute)] public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(FileSystemRights rights) { - using var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "directory"); - DirectoryInfo info = new DirectoryInfo(path); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "directory"); + var dirInfo = new DirectoryInfo(path); DirectorySecurity expectedSecurity = GetDirectorySecurity(rights); + dirInfo.Create(expectedSecurity); + Assert.True(dirInfo.Exists); + tempRootDir.CreatedSubdirectories.Add(dirInfo); - info.Create(expectedSecurity); - - Assert.True(Directory.Exists(path)); - - DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); - + var actualInfo = new DirectoryInfo(dirInfo.FullName); DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(expectedSecurity, actualSecurity); } [Theory] - [InlineData(FileSystemRights.TakeOwnership)] - [InlineData(FileSystemRights.Write)] + [MemberData(nameof(RightsToDeny))] public void DirectoryInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) { var expectedSecurity = new DirectorySecurity(); - var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); var allowAccessRule = new FileSystemAccessRule(identity, FileSystemRights.Read, AccessControlType.Allow); @@ -252,18 +254,16 @@ public void DirectoryInfo_Create_MultipleAddAccessRules(FileSystemRights rightsT var denyAccessRule = new FileSystemAccessRule(identity, rightsToDeny, AccessControlType.Deny); expectedSecurity.AddAccessRule(denyAccessRule); - using var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "directory"); - DirectoryInfo info = new DirectoryInfo(path); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "directory"); + var dirInfo = new DirectoryInfo(path); - info.Create(expectedSecurity); - - Assert.True(Directory.Exists(path)); - - DirectoryInfo actualInfo = new DirectoryInfo(info.FullName); + dirInfo.Create(expectedSecurity); + Assert.True(dirInfo.Exists); + tempRootDir.CreatedSubdirectories.Add(dirInfo); + var actualInfo = new DirectoryInfo(dirInfo.FullName); DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(expectedSecurity, actualSecurity); } @@ -275,7 +275,7 @@ public void DirectoryInfo_Create_MultipleAddAccessRules(FileSystemRights rightsT public void FileInfo_Create_NullFileInfo() { FileInfo info = null; - FileSecurity security = new FileSecurity(); + var security = new FileSecurity(); Assert.Throws("fileInfo", () => FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); @@ -284,7 +284,7 @@ public void FileInfo_Create_NullFileInfo() [Fact] public void FileInfo_Create_NullFileSecurity() { - FileInfo info = new FileInfo("path"); + var info = new FileInfo("path"); Assert.Throws("fileSecurity", () => FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, null)); @@ -293,13 +293,13 @@ public void FileInfo_Create_NullFileSecurity() [Fact] public void FileInfo_Create_NotFound() { - using var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "file.txt"); - FileInfo info = new FileInfo(path); - FileSecurity security = new FileSecurity(); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, Guid.NewGuid().ToString(), "file.txt"); + var fileInfo = new FileInfo(path); + var security = new FileSecurity(); Assert.Throws(() => - FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); + FileInfo_Create_Framework(fileInfo, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -308,8 +308,8 @@ public void FileInfo_Create_NotFound() [InlineData((FileMode)int.MaxValue)] public void FileInfo_Create_FileSecurity_InvalidFileMode(FileMode invalidMode) { - FileSecurity security = new FileSecurity(); - FileInfo info = new FileInfo("path"); + var security = new FileSecurity(); + var info = new FileInfo("path"); Assert.Throws("mode", () => FileInfo_Create_Framework(info, invalidMode, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); @@ -320,8 +320,8 @@ public void FileInfo_Create_FileSecurity_InvalidFileMode(FileMode invalidMode) [InlineData((FileShare)int.MaxValue)] public void FileInfo_Create_FileSecurity_InvalidFileShare(FileShare invalidFileShare) { - FileSecurity security = new FileSecurity(); - FileInfo info = new FileInfo("path"); + var security = new FileSecurity(); + var info = new FileInfo("path"); Assert.Throws("share", () => FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security)); @@ -332,8 +332,8 @@ public void FileInfo_Create_FileSecurity_InvalidFileShare(FileShare invalidFileS [InlineData(0)] public void FileInfo_Create_FileSecurity_InvalidBufferSize(int invalidBufferSize) { - FileSecurity security = new FileSecurity(); - FileInfo info = new FileInfo("path"); + var security = new FileSecurity(); + var info = new FileInfo("path"); Assert.Throws("bufferSize", () => FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, invalidBufferSize, FileOptions.None, security)); @@ -350,13 +350,20 @@ public void FileInfo_Create_FileSecurity_InvalidBufferSize(int invalidBufferSize [InlineData(FileMode.Append, FileSystemRights.ReadData)] public void FileInfo_Create_FileSecurity_ForbiddenCombo_FileModeFileSystemSecurity(FileMode mode, FileSystemRights rights) { - FileSecurity security = new FileSecurity(); - FileInfo info = new FileInfo("path"); + var security = new FileSecurity(); + var info = new FileInfo("path"); Assert.Throws(() => FileInfo_Create_Framework(info, mode, rights, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } + [Fact] + public void FileInfo_Create_DefaultFileSecurity() + { + var security = new FileSecurity(); + Verify_FileSecurity_CreateFile(security); + } + private void FileInfo_Create_Framework(FileInfo info, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity security) { if (PlatformDetection.IsNetFramework) @@ -372,14 +379,13 @@ private void FileInfo_Create_Framework(FileInfo info, FileMode mode, FileSystemR [Fact] public void FileInfo_Create_FileSecurity_SpecificAccessRule() { - using var directory = new TempAclDirectory(); - - string path = Path.Combine(directory.Path, "file.txt"); - FileInfo info = new FileInfo(path); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "file.txt"); + var fileInfo = new FileInfo(path); FileSecurity expectedSecurity = GetFileSecurity(FileSystemRights.FullControl); - using FileStream stream = info.Create( + using FileStream stream = fileInfo.Create( FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite | FileShare.Delete, @@ -387,37 +393,34 @@ public void FileInfo_Create_FileSecurity_SpecificAccessRule() FileOptions.None, expectedSecurity); - Assert.True(File.Exists(path)); - - var actualInfo = new FileInfo(info.FullName); + Assert.True(fileInfo.Exists); + tempRootDir.CreatedSubfiles.Add(fileInfo); + var actualInfo = new FileInfo(fileInfo.FullName); FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(expectedSecurity, actualSecurity); } [Theory] - [InlineData(FileSystemRights.TakeOwnership)] - [InlineData(FileSystemRights.Write)] + [MemberData(nameof(RightsToDeny))] public void FileInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) { var expectedSecurity = new FileSecurity(); var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - var allowAccessRule = new FileSystemAccessRule(identity, FileSystemRights.Read, AccessControlType.Allow); expectedSecurity.AddAccessRule(allowAccessRule); var denyAccessRule = new FileSystemAccessRule(identity, rightsToDeny, AccessControlType.Deny); expectedSecurity.AddAccessRule(denyAccessRule); - using var directory = new TempAclDirectory(); + using var tempRootDir = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "file.txt"); - var info = new FileInfo(path); + string path = Path.Combine(tempRootDir.Path, "file.txt"); + var fileInfo = new FileInfo(path); - using FileStream stream = info.Create( + using FileStream stream = fileInfo.Create( FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite | FileShare.Delete, @@ -425,12 +428,11 @@ public void FileInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny FileOptions.None, expectedSecurity); - Assert.True(File.Exists(path)); - - var actualInfo = new FileInfo(info.FullName); + Assert.True(fileInfo.Exists); + tempRootDir.CreatedSubfiles.Add(fileInfo); + var actualInfo = new FileInfo(fileInfo.FullName); FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(expectedSecurity, actualSecurity); } @@ -452,7 +454,7 @@ public void DirectorySecurity_CreateDirectory_NullSecurity() [Fact] public void DirectorySecurity_CreateDirectory_InvalidPath() { - DirectorySecurity security = new DirectorySecurity(); + var security = new DirectorySecurity(); Assert.Throws("path", () => security.CreateDirectory(null)); Assert.Throws(() => security.CreateDirectory("")); @@ -461,22 +463,19 @@ public void DirectorySecurity_CreateDirectory_InvalidPath() [Fact] public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists() { - using var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "createMe"); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "createMe"); DirectorySecurity expectedSecurity = GetDirectorySecurity(FileSystemRights.FullControl); + DirectoryInfo dirInfo = expectedSecurity.CreateDirectory(path); + Assert.True(dirInfo.Exists); + tempRootDir.CreatedSubdirectories.Add(dirInfo); - expectedSecurity.CreateDirectory(path); - - Assert.True(Directory.Exists(path)); - - DirectorySecurity basicSecurity = new DirectorySecurity(); - + var basicSecurity = new DirectorySecurity(); // Already exists, existingDirInfo should have the original security, not the new basic security DirectoryInfo existingDirInfo = basicSecurity.CreateDirectory(path); DirectorySecurity actualSecurity = existingDirInfo.GetAccessControl(AccessControlSections.Access); - VerifyAccessSecurity(expectedSecurity, actualSecurity); } @@ -487,41 +486,82 @@ public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists() #region Helper methods - private DirectorySecurity GetDirectorySecurity(FileSystemRights rights) - { - DirectorySecurity security = new DirectorySecurity(); - SecurityIdentifier identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); - security.AddAccessRule(accessRule); - return security; + public static IEnumerable RightsToDeny() + { + yield return new object[] { FileSystemRights.AppendData }; + yield return new object[] { FileSystemRights.ChangePermissions }; + // yield return new object[] { FileSystemRights.CreateDirectories }; // CreateDirectories == AppendData + yield return new object[] { FileSystemRights.CreateFiles }; + yield return new object[] { FileSystemRights.Delete }; + yield return new object[] { FileSystemRights.DeleteSubdirectoriesAndFiles }; + yield return new object[] { FileSystemRights.ExecuteFile }; + // yield return new object[] { FileSystemRights.FullControl }; // Contains ReadData, should not deny that + // yield return new object[] { FileSystemRights.ListDirectory }; ListDirectory == ReadData + // yield return new object[] { FileSystemRights.Modify }; // Contains ReadData, should not deny that + // yield return new object[] { FileSystemRights.Read }; // Contains ReadData, should not deny that + // yield return new object[] { FileSystemRights.ReadAndExecute }; // Contains ReadData, should not deny that + yield return new object[] { FileSystemRights.ReadAttributes }; + // yield return new object[] { FileSystemRights.ReadData }; // Minimum right required to delete a file or directory + yield return new object[] { FileSystemRights.ReadExtendedAttributes }; + yield return new object[] { FileSystemRights.ReadPermissions }; + // yield return new object[] { FileSystemRights.Synchronize }; CreateFile always requires Synchronize access + yield return new object[] { FileSystemRights.TakeOwnership }; + //yield return new object[] { FileSystemRights.Traverse }; // Traverse == ExecuteFile + yield return new object[] { FileSystemRights.Write }; + yield return new object[] { FileSystemRights.WriteAttributes }; + // yield return new object[] { FileSystemRights.WriteData }; // WriteData == CreateFiles + yield return new object[] { FileSystemRights.WriteExtendedAttributes }; + } + + private void Verify_FileSecurity_CreateFile(FileSecurity expectedSecurity) + { + Verify_FileSecurity_CreateFile(FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite, DefaultBufferSize, FileOptions.Asynchronous, expectedSecurity); + } + + private void Verify_FileSecurity_CreateFile(FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity expectedSecurity) + { + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "file.txt"); + var fileInfo = new FileInfo(path); + + fileInfo.Create(mode, rights, share, bufferSize, options, expectedSecurity).Dispose(); + Assert.True(fileInfo.Exists); + tempRootDir.CreatedSubfiles.Add(fileInfo); + + var actualFileInfo = new FileInfo(path); + FileSecurity actualSecurity = actualFileInfo.GetAccessControl(AccessControlSections.Access); + VerifyAccessSecurity(expectedSecurity, actualSecurity); } private void Verify_DirectorySecurity_CreateDirectory(DirectorySecurity expectedSecurity) { - using var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "createMe"); + using var tempRootDir = new TempAclDirectory(); + string path = Path.Combine(tempRootDir.Path, "createMe"); + DirectoryInfo dirInfo = expectedSecurity.CreateDirectory(path); + Assert.True(dirInfo.Exists); + tempRootDir.CreatedSubdirectories.Add(dirInfo); - expectedSecurity.CreateDirectory(path); - - Assert.True(Directory.Exists(path)); - - DirectoryInfo actualInfo = new DirectoryInfo(path); - - DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access); + var actualDirInfo = new DirectoryInfo(path); + DirectorySecurity actualSecurity = actualDirInfo.GetAccessControl(AccessControlSections.Access); VerifyAccessSecurity(expectedSecurity, actualSecurity); } - private FileSecurity GetFileSecurity(FileSystemRights rights) + private DirectorySecurity GetDirectorySecurity(FileSystemRights rights) { - FileSecurity security = new FileSecurity(); - - SecurityIdentifier identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - - FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); - + var security = new DirectorySecurity(); + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + var accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); security.AddAccessRule(accessRule); + return security; + } + private FileSecurity GetFileSecurity(FileSystemRights rights) + { + var security = new FileSecurity(); + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + var accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow); + security.AddAccessRule(accessRule); return security; } diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs index 3844ab2a879946..5ecf047eede16d 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs @@ -14,36 +14,53 @@ namespace System.IO /// public sealed class TempAclDirectory : TempDirectory { + internal readonly List CreatedSubdirectories = new(); + internal readonly List CreatedSubfiles = new(); protected override void DeleteDirectory() { try { - var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); - - foreach (string dirPath in Directory.EnumerateDirectories(Path, "*", SearchOption.AllDirectories)) + foreach (DirectoryInfo subdir in CreatedSubdirectories) { - var dirInfo = new DirectoryInfo(dirPath); - var dirSecurity = new DirectorySecurity(dirPath, AccessControlSections.Access); - dirSecurity.AddAccessRule(accessRule); - dirInfo.SetAccessControl(dirSecurity); + ResetFullControlToDirectory(subdir); } - foreach (string filePath in Directory.EnumerateFiles(Path, "*", SearchOption.AllDirectories)) + foreach (FileInfo subfile in CreatedSubfiles) { - var fileInfo = new FileInfo(filePath); - var fileSecurity = new FileSecurity(filePath, AccessControlSections.Access); - fileSecurity.AddAccessRule(accessRule); - fileInfo.SetAccessControl(fileSecurity); + ResetFullControlToFile(subfile); } var rootDirInfo = new DirectoryInfo(Path); - var rootSecurity = new DirectorySecurity(Path, AccessControlSections.Access); - rootSecurity.AddAccessRule(accessRule); - rootDirInfo.SetAccessControl(rootSecurity); + ResetFullControlToDirectory(rootDirInfo); rootDirInfo.Delete(recursive: true); } catch { /* Do not throw because we call this on finalize */ } } + + private void ResetFullControlToDirectory(DirectoryInfo dirInfo) + { + try + { + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); + var security = new DirectorySecurity(dirInfo.FullName, AccessControlSections.Access); + security.AddAccessRule(accessRule); + dirInfo.SetAccessControl(security); + } + catch { /* Skip silently if dir does not exist */ } + } + + private void ResetFullControlToFile(FileInfo fileInfo) + { + try + { + var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + var accessRule = new FileSystemAccessRule(identity, FileSystemRights.FullControl, AccessControlType.Allow); + var security = new FileSecurity(fileInfo.FullName, AccessControlSections.Access); + security.AddAccessRule(accessRule); + fileInfo.SetAccessControl(security); + } + catch { /* Skip silently if file does not exist */ } + } } } From d1e989624abaa2cf8952a665b6fdcf94d7781bab Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 16:30:23 -0800 Subject: [PATCH 07/14] NotFound test --- .../tests/FileSystemAclExtensionsTests.cs | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 4ccfa6aeb5bdaa..a0926552cf810a 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -177,29 +177,41 @@ public void DirectoryInfo_Create_NullDirectoryInfo() { DirectoryInfo info = null; var security = new DirectorySecurity(); - Assert.Throws("directoryInfo", () => DirectoryInfo_Create_Framework(info, security)); + Assert.Throws("directoryInfo", () => DirectoryInfo_Create_SelectFramework(info, security)); } [Fact] public void DirectoryInfo_Create_NullDirectorySecurity() { var info = new DirectoryInfo("path"); - Assert.Throws("directorySecurity", () => DirectoryInfo_Create_Framework(info, null)); + Assert.Throws("directorySecurity", () => DirectoryInfo_Create_SelectFramework(info, null)); } [Fact] public void DirectoryInfo_Create_NotFound() { - var directory = new TempAclDirectory(); - string path = Path.Combine(directory.Path, "ParentDoesNotExist"); - directory.Dispose(); // Delete parent folder + using var tempRootDir = new TempAclDirectory(); + string dirPath = Path.Combine(tempRootDir.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist"); + + var dirInfo = new DirectoryInfo(dirPath); + var security = new DirectorySecurity(); + // Fails because the DirectorySecurity lacks any rights to create parent folder + Assert.Throws(() => DirectoryInfo_Create_SelectFramework(dirInfo, security)); + } + + [Fact] + public void DirectoryInfo_Create_NotFound_FullControl() + { + using var tempRootDir = new TempAclDirectory(); + string dirPath = Path.Combine(tempRootDir.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist"); - var info = new DirectoryInfo(path); - DirectorySecurity security = GetDirectorySecurity(FileSystemRights.FullControl); - DirectoryInfo_Create_Framework(info, security); + var dirInfo = new DirectoryInfo(dirPath); + var security = GetDirectorySecurity(FileSystemRights.FullControl); + // Succeeds because it creates the missing parent folder + DirectoryInfo_Create_SelectFramework(dirInfo, security); } - private void DirectoryInfo_Create_Framework(DirectoryInfo info, DirectorySecurity security) + private void DirectoryInfo_Create_SelectFramework(DirectoryInfo info, DirectorySecurity security) { if (PlatformDetection.IsNetFramework) { From 4f8d3e2fc19d99107d3e92a49d3e6bd53a28e39f Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 16:31:46 -0800 Subject: [PATCH 08/14] FileShare.None --- .../tests/FileSystemAclExtensionsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index a0926552cf810a..4efbe16c48a656 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -154,7 +154,7 @@ public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); + using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.None); AssertExtensions.Throws("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null)); } From 75230bfc11c21e665785978b3979e656e0ac7b6c Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 16:33:08 -0800 Subject: [PATCH 09/14] InlineData FileMode --- .../tests/FileSystemAclExtensionsTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 4efbe16c48a656..1b2321d46b1196 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -149,12 +149,15 @@ public void SetAccessControl_FileStream_FileSecurity_InvalidArguments() Assert.Throws("fileStream", () => FileSystemAclExtensions.SetAccessControl((FileStream)null, fileSecurity: null)); } - [Fact] - public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject() + [Theory] + [InlineData(FileMode.Append)] + [InlineData(FileMode.Open)] + [InlineData(FileMode.OpenOrCreate)] + public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject(FileMode mode) { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, mode, FileAccess.Write, FileShare.None); AssertExtensions.Throws("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null)); } From 8f4e43e4e267256af26b717018d1bc89efc3dde3 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 16:42:05 -0800 Subject: [PATCH 10/14] Rename method that creates per platform. Bring back inline data. --- .../tests/FileSystemAclExtensionsTests.cs | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 1b2321d46b1196..d809f3de7d9782 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -180,14 +180,14 @@ public void DirectoryInfo_Create_NullDirectoryInfo() { DirectoryInfo info = null; var security = new DirectorySecurity(); - Assert.Throws("directoryInfo", () => DirectoryInfo_Create_SelectFramework(info, security)); + Assert.Throws("directoryInfo", () => CreateDirectoryWithSecurity(info, security)); } [Fact] public void DirectoryInfo_Create_NullDirectorySecurity() { var info = new DirectoryInfo("path"); - Assert.Throws("directorySecurity", () => DirectoryInfo_Create_SelectFramework(info, null)); + Assert.Throws("directorySecurity", () => CreateDirectoryWithSecurity(info, null)); } [Fact] @@ -199,7 +199,7 @@ public void DirectoryInfo_Create_NotFound() var dirInfo = new DirectoryInfo(dirPath); var security = new DirectorySecurity(); // Fails because the DirectorySecurity lacks any rights to create parent folder - Assert.Throws(() => DirectoryInfo_Create_SelectFramework(dirInfo, security)); + Assert.Throws(() => CreateDirectoryWithSecurity(dirInfo, security)); } [Fact] @@ -211,10 +211,10 @@ public void DirectoryInfo_Create_NotFound_FullControl() var dirInfo = new DirectoryInfo(dirPath); var security = GetDirectorySecurity(FileSystemRights.FullControl); // Succeeds because it creates the missing parent folder - DirectoryInfo_Create_SelectFramework(dirInfo, security); + CreateDirectoryWithSecurity(dirInfo, security); } - private void DirectoryInfo_Create_SelectFramework(DirectoryInfo info, DirectorySecurity security) + private void CreateDirectoryWithSecurity(DirectoryInfo info, DirectorySecurity security) { if (PlatformDetection.IsNetFramework) { @@ -240,7 +240,7 @@ public void DirectoryInfo_Create_DefaultDirectorySecurity() [InlineData(FileSystemRights.Read | FileSystemRights.Write)] [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] [InlineData(FileSystemRights.ReadAndExecute)] - public void DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(FileSystemRights rights) + public void DirectoryInfo_Create_DirectorySecurity_SpecificAccessRule(FileSystemRights rights) { using var tempRootDir = new TempAclDirectory(); string path = Path.Combine(tempRootDir.Path, "directory"); @@ -293,7 +293,7 @@ public void FileInfo_Create_NullFileInfo() var security = new FileSecurity(); Assert.Throws("fileInfo", () => - FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Fact] @@ -302,7 +302,7 @@ public void FileInfo_Create_NullFileSecurity() var info = new FileInfo("path"); Assert.Throws("fileSecurity", () => - FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, null)); + CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, null)); } [Fact] @@ -314,7 +314,7 @@ public void FileInfo_Create_NotFound() var security = new FileSecurity(); Assert.Throws(() => - FileInfo_Create_Framework(fileInfo, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(fileInfo, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -327,7 +327,7 @@ public void FileInfo_Create_FileSecurity_InvalidFileMode(FileMode invalidMode) var info = new FileInfo("path"); Assert.Throws("mode", () => - FileInfo_Create_Framework(info, invalidMode, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(info, invalidMode, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -339,7 +339,7 @@ public void FileInfo_Create_FileSecurity_InvalidFileShare(FileShare invalidFileS var info = new FileInfo("path"); Assert.Throws("share", () => - FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security)); } [Theory] @@ -351,7 +351,7 @@ public void FileInfo_Create_FileSecurity_InvalidBufferSize(int invalidBufferSize var info = new FileInfo("path"); Assert.Throws("bufferSize", () => - FileInfo_Create_Framework(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, invalidBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, invalidBufferSize, FileOptions.None, security)); } [Theory] @@ -369,7 +369,7 @@ public void FileInfo_Create_FileSecurity_ForbiddenCombo_FileModeFileSystemSecuri var info = new FileInfo("path"); Assert.Throws(() => - FileInfo_Create_Framework(info, mode, rights, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); + CreateFileWithSecurity(info, mode, rights, FileShare.Delete, DefaultBufferSize, FileOptions.None, security)); } [Fact] @@ -379,7 +379,7 @@ public void FileInfo_Create_DefaultFileSecurity() Verify_FileSecurity_CreateFile(security); } - private void FileInfo_Create_Framework(FileInfo info, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity security) + private void CreateFileWithSecurity(FileInfo info, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity security) { if (PlatformDetection.IsNetFramework) { @@ -391,14 +391,20 @@ private void FileInfo_Create_Framework(FileInfo info, FileMode mode, FileSystemR } } - [Fact] - public void FileInfo_Create_FileSecurity_SpecificAccessRule() + [Theory] + // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose + [InlineData(FileSystemRights.FullControl)] + [InlineData(FileSystemRights.Read)] + [InlineData(FileSystemRights.Read | FileSystemRights.Write)] + [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] + [InlineData(FileSystemRights.ReadAndExecute)] + public void FileInfo_Create_FileSecurity_SpecificAccessRule(FileSystemRights rights) { using var tempRootDir = new TempAclDirectory(); string path = Path.Combine(tempRootDir.Path, "file.txt"); var fileInfo = new FileInfo(path); - FileSecurity expectedSecurity = GetFileSecurity(FileSystemRights.FullControl); + FileSecurity expectedSecurity = GetFileSecurity(rights); using FileStream stream = fileInfo.Create( FileMode.Create, From 13c457c984cdeb05ea324a76364366ce6867591b Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 3 Feb 2021 20:31:44 -0800 Subject: [PATCH 11/14] Address suggestions --- .../tests/FileSystemAclExtensionsTests.cs | 53 +++++++++++++------ 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index d809f3de7d9782..08728dd11b91dc 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -97,7 +97,7 @@ public void GetAccessControl_Filestream_ReturnValidObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); + using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.Delete); FileSecurity fileSecurity = FileSystemAclExtensions.GetAccessControl(fileStream); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); @@ -166,7 +166,7 @@ public void SetAccessControl_FileStream_FileSecurity_Success() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.Delete); + using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); var fileSecurity = new FileSecurity(); FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity); } @@ -235,12 +235,8 @@ public void DirectoryInfo_Create_DefaultDirectorySecurity() [Theory] // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose - [InlineData(FileSystemRights.FullControl)] - [InlineData(FileSystemRights.Read)] - [InlineData(FileSystemRights.Read | FileSystemRights.Write)] - [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] - [InlineData(FileSystemRights.ReadAndExecute)] - public void DirectoryInfo_Create_DirectorySecurity_SpecificAccessRule(FileSystemRights rights) + [MemberData(nameof(RightsToAllow))] + public void DirectoryInfo_Create_AllowSpecific_AccessRules(FileSystemRights rights) { using var tempRootDir = new TempAclDirectory(); string path = Path.Combine(tempRootDir.Path, "directory"); @@ -258,7 +254,7 @@ public void DirectoryInfo_Create_DirectorySecurity_SpecificAccessRule(FileSystem [Theory] [MemberData(nameof(RightsToDeny))] - public void DirectoryInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) + public void DirectoryInfo_Create_DenySpecific_AddAccessRules(FileSystemRights rightsToDeny) { var expectedSecurity = new DirectorySecurity(); var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); @@ -393,12 +389,8 @@ private void CreateFileWithSecurity(FileInfo info, FileMode mode, FileSystemRigh [Theory] // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose - [InlineData(FileSystemRights.FullControl)] - [InlineData(FileSystemRights.Read)] - [InlineData(FileSystemRights.Read | FileSystemRights.Write)] - [InlineData(FileSystemRights.Read | FileSystemRights.Write | FileSystemRights.ExecuteFile)] - [InlineData(FileSystemRights.ReadAndExecute)] - public void FileInfo_Create_FileSecurity_SpecificAccessRule(FileSystemRights rights) + [MemberData(nameof(RightsToAllow))] + public void FileInfo_Create_AllowSpecific_AccessRules(FileSystemRights rights) { using var tempRootDir = new TempAclDirectory(); string path = Path.Combine(tempRootDir.Path, "file.txt"); @@ -425,7 +417,7 @@ public void FileInfo_Create_FileSecurity_SpecificAccessRule(FileSystemRights rig [Theory] [MemberData(nameof(RightsToDeny))] - public void FileInfo_Create_MultipleAddAccessRules(FileSystemRights rightsToDeny) + public void FileInfo_Create_DenySpecific_AccessRules(FileSystemRights rightsToDeny) { var expectedSecurity = new FileSecurity(); @@ -525,7 +517,7 @@ public static IEnumerable RightsToDeny() // yield return new object[] { FileSystemRights.ReadData }; // Minimum right required to delete a file or directory yield return new object[] { FileSystemRights.ReadExtendedAttributes }; yield return new object[] { FileSystemRights.ReadPermissions }; - // yield return new object[] { FileSystemRights.Synchronize }; CreateFile always requires Synchronize access + // yield return new object[] { FileSystemRights.Synchronize }; // CreateFile always requires Synchronize access yield return new object[] { FileSystemRights.TakeOwnership }; //yield return new object[] { FileSystemRights.Traverse }; // Traverse == ExecuteFile yield return new object[] { FileSystemRights.Write }; @@ -534,6 +526,33 @@ public static IEnumerable RightsToDeny() yield return new object[] { FileSystemRights.WriteExtendedAttributes }; } + public static IEnumerable RightsToAllow() + { + yield return new object[] { FileSystemRights.AppendData }; + yield return new object[] { FileSystemRights.ChangePermissions }; + // yield return new object[] { FileSystemRights.CreateDirectories }; // CreateDirectories == AppendData + yield return new object[] { FileSystemRights.CreateFiles }; + yield return new object[] { FileSystemRights.Delete }; + yield return new object[] { FileSystemRights.DeleteSubdirectoriesAndFiles }; + yield return new object[] { FileSystemRights.ExecuteFile }; + yield return new object[] { FileSystemRights.FullControl }; + // yield return new object[] { FileSystemRights.ListDirectory }; ListDirectory == ReadData + yield return new object[] { FileSystemRights.Modify }; + yield return new object[] { FileSystemRights.Read }; + yield return new object[] { FileSystemRights.ReadAndExecute }; + yield return new object[] { FileSystemRights.ReadAttributes }; + // yield return new object[] { FileSystemRights.ReadData }; // Minimum right required to delete a file or directory + yield return new object[] { FileSystemRights.ReadExtendedAttributes }; + yield return new object[] { FileSystemRights.ReadPermissions }; + yield return new object[] { FileSystemRights.Synchronize }; + yield return new object[] { FileSystemRights.TakeOwnership }; + // yield return new object[] { FileSystemRights.Traverse }; // Traverse == ExecuteFile + yield return new object[] { FileSystemRights.Write }; + yield return new object[] { FileSystemRights.WriteAttributes }; + // yield return new object[] { FileSystemRights.WriteData }; // WriteData == CreateFiles + yield return new object[] { FileSystemRights.WriteExtendedAttributes }; + } + private void Verify_FileSecurity_CreateFile(FileSecurity expectedSecurity) { Verify_FileSecurity_CreateFile(FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite, DefaultBufferSize, FileOptions.Asynchronous, expectedSecurity); From e702f0597f08dd005cd505f89b0d152d9b674a1d Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 4 Feb 2021 09:53:54 -0800 Subject: [PATCH 12/14] Address suggestions --- .../tests/FileSystemAclExtensionsTests.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 08728dd11b91dc..92298d8e87f641 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -63,7 +63,7 @@ public void GetAccessControl_FileInfo_ReturnsValidObject() using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); var fileInfo = new FileInfo(file.Path); - FileSecurity fileSecurity = fileInfo.GetAccessControl(AccessControlSections.Access); + FileSecurity fileSecurity = fileInfo.GetAccessControl(); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); } @@ -149,15 +149,12 @@ public void SetAccessControl_FileStream_FileSecurity_InvalidArguments() Assert.Throws("fileStream", () => FileSystemAclExtensions.SetAccessControl((FileStream)null, fileSecurity: null)); } - [Theory] - [InlineData(FileMode.Append)] - [InlineData(FileMode.Open)] - [InlineData(FileMode.OpenOrCreate)] - public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject(FileMode mode) + [Fact] + public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, mode, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.None); AssertExtensions.Throws("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null)); } From b716dffbe3a622c6029b1b65a90d904bba005879 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 4 Feb 2021 10:48:50 -0800 Subject: [PATCH 13/14] Address suggestions --- .../tests/FileSystemAclExtensionsTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index 92298d8e87f641..eacfb5013b8441 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -29,7 +29,7 @@ public void GetAccessControl_DirectoryInfo_ReturnsValidObject() { using var directory = new TempAclDirectory(); var directoryInfo = new DirectoryInfo(directory.Path); - DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(AccessControlSections.Access); + DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(); Assert.NotNull(directorySecurity); Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType); } @@ -97,7 +97,7 @@ public void GetAccessControl_Filestream_ReturnValidObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.Delete); + using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); FileSecurity fileSecurity = FileSystemAclExtensions.GetAccessControl(fileStream); Assert.NotNull(fileSecurity); Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType); @@ -154,7 +154,7 @@ public void SetAccessControl_FileStream_FileSecurity_InvalidFileSecurityObject() { using var directory = new TempAclDirectory(); using var file = new TempFile(Path.Combine(directory.Path, "file.txt")); - using FileStream fileStream = File.Open(file.Path, FileMode.Open, FileAccess.Write, FileShare.None); + using FileStream fileStream = File.Open(file.Path, FileMode.Append, FileAccess.Write, FileShare.None); AssertExtensions.Throws("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null)); } @@ -231,7 +231,7 @@ public void DirectoryInfo_Create_DefaultDirectorySecurity() } [Theory] - // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose + // Must have at least one ReadData, otherwise the TempAclDirectory will fail to delete that item on dispose [MemberData(nameof(RightsToAllow))] public void DirectoryInfo_Create_AllowSpecific_AccessRules(FileSystemRights rights) { From 283d478110da208d885200b453c77eb2ba6bfe5e Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 4 Feb 2021 16:53:24 -0800 Subject: [PATCH 14/14] Reverting Verify_FileSecurity_CreateFile to the original arguments --- .../tests/FileSystemAclExtensionsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs index eacfb5013b8441..22f2e6ffdd09cd 100644 --- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs +++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs @@ -552,7 +552,7 @@ public static IEnumerable RightsToAllow() private void Verify_FileSecurity_CreateFile(FileSecurity expectedSecurity) { - Verify_FileSecurity_CreateFile(FileMode.Create, FileSystemRights.FullControl, FileShare.ReadWrite, DefaultBufferSize, FileOptions.Asynchronous, expectedSecurity); + Verify_FileSecurity_CreateFile(FileMode.Create, FileSystemRights.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, expectedSecurity); } private void Verify_FileSecurity_CreateFile(FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity expectedSecurity)