FileSystem.AccessControl tests not cleaning files properly (#47074)

* 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.

* Temp commit to verify files and dirs deleted without try catch

* Set directory security first, then file security, in DeleteDirectory. Bring back InlineData for test, but only when a Read is included.

* Restore try catch before merging.

* Address suggestions.

* 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.

* NotFound test

* FileShare.None

* InlineData FileMode

* Rename method that creates per platform. Bring back inline data.

* Address suggestions

* Address suggestions

* Address suggestions

* Reverting Verify_FileSecurity_CreateFile to the original arguments

Co-authored-by: carlossanlop <carlossanlop@users.noreply.github.com>

diff --git a/src/libraries/Common/tests/System/IO/TempDirectory.cs b/src/libraries/Common/tests/System/IO/TempDirectory.cs
index c3afec4..93c6d96 100644 (file)
--- 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.
     /// </summary>
-    public sealed class TempDirectory : IDisposable
+    public class TempDirectory : IDisposable
     {
         public const int MaxNameLength = 255;
 
@@ -40,7 +40,7 @@ namespace System.IO
 
         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 431e8e4..22f2e6f 100644 (file)
--- a/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs
+++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/FileSystemAclExtensionsTests.cs
@@ -27,8 +27,8 @@ namespace System.IO
         [Fact]
         public void GetAccessControl_DirectoryInfo_ReturnsValidObject()
         {
-            using var directory = new TempDirectory();
-            DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path);
+            using var directory = new TempAclDirectory();
+            var directoryInfo = new DirectoryInfo(directory.Path);
             DirectorySecurity directorySecurity = directoryInfo.GetAccessControl();
             Assert.NotNull(directorySecurity);
             Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType);
@@ -43,9 +43,9 @@ namespace System.IO
         [Fact]
         public void GetAccessControl_DirectoryInfo_AccessControlSections_ReturnsValidObject()
         {
-            using var directory = new TempDirectory();
-            DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path);
-            AccessControlSections accessControlSections = new AccessControlSections();
+            using var directory = new TempAclDirectory();
+            var directoryInfo = new DirectoryInfo(directory.Path);
+            var accessControlSections = new AccessControlSections();
             DirectorySecurity directorySecurity = directoryInfo.GetAccessControl(accessControlSections);
             Assert.NotNull(directorySecurity);
             Assert.Equal(typeof(FileSystemRights), directorySecurity.AccessRightType);
@@ -60,9 +60,9 @@ namespace System.IO
         [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);
+            var fileInfo = new FileInfo(file.Path);
             FileSecurity fileSecurity = fileInfo.GetAccessControl();
             Assert.NotNull(fileSecurity);
             Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType);
@@ -77,10 +77,10 @@ namespace System.IO
         [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();
+            var fileInfo = new FileInfo(file.Path);
+            var accessControlSections = new AccessControlSections();
             FileSecurity fileSecurity = fileInfo.GetAccessControl(accessControlSections);
             Assert.NotNull(fileSecurity);
             Assert.Equal(typeof(FileSystemRights), fileSecurity.AccessRightType);
@@ -95,7 +95,7 @@ namespace System.IO
         [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);
             FileSecurity fileSecurity = FileSystemAclExtensions.GetAccessControl(fileStream);
@@ -110,36 +110,36 @@ namespace System.IO
         [Fact]
         public void SetAccessControl_DirectoryInfo_DirectorySecurity_InvalidArguments()
         {
-            using var directory = new TempDirectory();
-            DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path);
+            using var directory = new TempAclDirectory();
+            var directoryInfo = new DirectoryInfo(directory.Path);
             AssertExtensions.Throws<ArgumentNullException>("directorySecurity", () => directoryInfo.SetAccessControl(directorySecurity: null));
         }
 
         [Fact]
         public void SetAccessControl_DirectoryInfo_DirectorySecurity_Success()
         {
-            using var directory = new TempDirectory();
-            DirectoryInfo directoryInfo = new DirectoryInfo(directory.Path);
-            DirectorySecurity directorySecurity = new DirectorySecurity();
+            using var directory = new TempAclDirectory();
+            var directoryInfo = new DirectoryInfo(directory.Path);
+            var directorySecurity = new DirectorySecurity();
             directoryInfo.SetAccessControl(directorySecurity);
         }
 
         [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);
+            var fileInfo = new FileInfo(file.Path);
             AssertExtensions.Throws<ArgumentNullException>("fileSecurity", () => fileInfo.SetAccessControl(fileSecurity: null));
         }
 
         [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();
+            var fileInfo = new FileInfo(file.Path);
+            var fileSecurity = new FileSecurity();
             fileInfo.SetAccessControl(fileSecurity);
         }
 
@@ -152,7 +152,7 @@ namespace System.IO
         [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);
             AssertExtensions.Throws<ArgumentNullException>("fileSecurity", () => FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity: null));
@@ -161,10 +161,10 @@ namespace System.IO
         [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);
-            FileSecurity fileSecurity = new FileSecurity();
+            var fileSecurity = new FileSecurity();
             FileSystemAclExtensions.SetAccessControl(fileStream, fileSecurity);
         }
 
@@ -176,80 +176,103 @@ namespace System.IO
         public void DirectoryInfo_Create_NullDirectoryInfo()
         {
             DirectoryInfo info = null;
-            DirectorySecurity security = new DirectorySecurity();
-
-            Assert.Throws<ArgumentNullException>("directoryInfo", () =>
-            {
-                if (PlatformDetection.IsNetFramework)
-                {
-                    FileSystemAclExtensions.Create(info, security);
-                }
-                else
-                {
-                    info.Create(security);
-                }
-            });
+            var security = new DirectorySecurity();
+            Assert.Throws<ArgumentNullException>("directoryInfo", () => CreateDirectoryWithSecurity(info, security));
         }
 
         [Fact]
         public void DirectoryInfo_Create_NullDirectorySecurity()
         {
-            DirectoryInfo info = new DirectoryInfo("path");
-
-            Assert.Throws<ArgumentNullException>("directorySecurity", () =>
-            {
-                if (PlatformDetection.IsNetFramework)
-                {
-                    FileSystemAclExtensions.Create(info, null);
-                }
-                else
-                {
-                    info.Create(null);
-                }
-            });
+            var info = new DirectoryInfo("path");
+            Assert.Throws<ArgumentNullException>("directorySecurity", () => CreateDirectoryWithSecurity(info, null));
         }
 
         [Fact]
         public void DirectoryInfo_Create_NotFound()
         {
-            using var directory = new TempDirectory();
-            string path = Path.Combine(directory.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist");
-            DirectoryInfo info = new DirectoryInfo(path);
-            DirectorySecurity security = new DirectorySecurity();
+            using var tempRootDir = new TempAclDirectory();
+            string dirPath = Path.Combine(tempRootDir.Path, Guid.NewGuid().ToString(), "ParentDoesNotExist");
 
-            Assert.Throws<UnauthorizedAccessException>(() =>
+            var dirInfo = new DirectoryInfo(dirPath);
+            var security = new DirectorySecurity();
+            // Fails because the DirectorySecurity lacks any rights to create parent folder
+            Assert.Throws<UnauthorizedAccessException>(() =>  CreateDirectoryWithSecurity(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 dirInfo = new DirectoryInfo(dirPath);
+            var security = GetDirectorySecurity(FileSystemRights.FullControl);
+            // Succeeds because it creates the missing parent folder
+            CreateDirectoryWithSecurity(dirInfo, security);
+        }
+
+        private void CreateDirectoryWithSecurity(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()
         {
-            DirectorySecurity security = new DirectorySecurity();
-            Verify_DirectoryInfo_Create(security);
+            var 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 DirectoryInfo_Create_DirectorySecurityWithSpecificAccessRule(
-            FileSystemRights rights,
-            AccessControlType controlType)
+        // 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)
         {
-            DirectorySecurity security = GetDirectorySecurity(rights, controlType);
-            Verify_DirectoryInfo_Create(security);
+            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);
+
+            var actualInfo = new DirectoryInfo(dirInfo.FullName);
+            DirectorySecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access);
+            VerifyAccessSecurity(expectedSecurity, actualSecurity);
+        }
+
+        [Theory]
+        [MemberData(nameof(RightsToDeny))]
+        public void DirectoryInfo_Create_DenySpecific_AddAccessRules(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 tempRootDir = new TempAclDirectory();
+            string path = Path.Combine(tempRootDir.Path, "directory");
+            var dirInfo = new DirectoryInfo(path);
+
+            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);
         }
 
         #endregion
@@ -260,58 +283,31 @@ namespace System.IO
         public void FileInfo_Create_NullFileInfo()
         {
             FileInfo info = null;
-            FileSecurity security = new FileSecurity();
+            var security = new FileSecurity();
 
             Assert.Throws<ArgumentNullException>("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);
-                }
-            });
+                CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security));
         }
 
         [Fact]
         public void FileInfo_Create_NullFileSecurity()
         {
-            FileInfo info = new FileInfo("path");
+            var info = new FileInfo("path");
 
             Assert.Throws<ArgumentNullException>("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);
-                }
-            });
+                CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, null));
         }
 
         [Fact]
         public void FileInfo_Create_NotFound()
         {
-            using var directory = new TempDirectory();
-            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<DirectoryNotFoundException>(() =>
-            {
-                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);
-                }
-            });
+                CreateFileWithSecurity(fileInfo, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security));
         }
 
         [Theory]
@@ -320,20 +316,11 @@ namespace System.IO
         [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<ArgumentOutOfRangeException>("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);
-                }
-            });
+                CreateFileWithSecurity(info, invalidMode, FileSystemRights.WriteData, FileShare.Delete, DefaultBufferSize, FileOptions.None, security));
         }
 
         [Theory]
@@ -341,20 +328,11 @@ namespace System.IO
         [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<ArgumentOutOfRangeException>("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);
-                }
-            });
+                CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, invalidFileShare, DefaultBufferSize, FileOptions.None, security));
         }
 
         [Theory]
@@ -362,20 +340,11 @@ namespace System.IO
         [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<ArgumentOutOfRangeException>("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);
-                }
-            });
+                CreateFileWithSecurity(info, FileMode.CreateNew, FileSystemRights.WriteData, FileShare.Delete, invalidBufferSize, FileOptions.None, security));
         }
 
         [Theory]
@@ -389,40 +358,92 @@ namespace System.IO
         [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<ArgumentException>(() =>
-            {
-                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);
-                }
-            });
+                CreateFileWithSecurity(info, mode, rights, FileShare.Delete, DefaultBufferSize, FileOptions.None, security));
         }
 
         [Fact]
         public void FileInfo_Create_DefaultFileSecurity()
         {
-            FileSecurity security = new FileSecurity();
-            Verify_FileInfo_Create(security);
+            var security = new FileSecurity();
+            Verify_FileSecurity_CreateFile(security);
+        }
+
+        private void CreateFileWithSecurity(FileInfo info, FileMode mode, FileSystemRights rights, FileShare share, int bufferSize, FileOptions options, FileSecurity security)
+        {
+            if (PlatformDetection.IsNetFramework)
+            {
+                FileSystemAclExtensions.Create(info, mode, rights, share, bufferSize, options, security);
+            }
+            else
+            {
+                info.Create(mode, rights, share, bufferSize, options, security);
+            }
+        }
+
+        [Theory]
+        // Must have at least one Read, otherwise the TempAclDirectory will fail to delete that item on dispose
+        [MemberData(nameof(RightsToAllow))]
+        public void FileInfo_Create_AllowSpecific_AccessRules(FileSystemRights rights)
+        {
+            using var tempRootDir = new TempAclDirectory();
+            string path = Path.Combine(tempRootDir.Path, "file.txt");
+            var fileInfo = new FileInfo(path);
+
+            FileSecurity expectedSecurity = GetFileSecurity(rights);
+
+            using FileStream stream = fileInfo.Create(
+                FileMode.Create,
+                FileSystemRights.FullControl,
+                FileShare.ReadWrite | FileShare.Delete,
+                DefaultBufferSize,
+                FileOptions.None,
+                expectedSecurity);
+
+            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.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)
+        [MemberData(nameof(RightsToDeny))]
+        public void FileInfo_Create_DenySpecific_AccessRules(FileSystemRights rightsToDeny)
         {
-            FileSecurity security = GetFileSecurity(rights, controlType);
-            Verify_FileInfo_Create(security);
+            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 tempRootDir = new TempAclDirectory();
+
+            string path = Path.Combine(tempRootDir.Path, "file.txt");
+            var fileInfo = new FileInfo(path);
+
+            using FileStream stream = fileInfo.Create(
+                FileMode.Create,
+                FileSystemRights.FullControl,
+                FileShare.ReadWrite | FileShare.Delete,
+                DefaultBufferSize,
+                FileOptions.None,
+                expectedSecurity);
+
+            Assert.True(fileInfo.Exists);
+            tempRootDir.CreatedSubfiles.Add(fileInfo);
+
+            var actualInfo = new FileInfo(fileInfo.FullName);
+            FileSecurity actualSecurity = actualInfo.GetAccessControl(AccessControlSections.Access);
+            VerifyAccessSecurity(expectedSecurity, actualSecurity);
         }
 
         #endregion
@@ -443,53 +464,29 @@ namespace System.IO
         [Fact]
         public void DirectorySecurity_CreateDirectory_InvalidPath()
         {
-            DirectorySecurity security = new DirectorySecurity();
+            var security = new DirectorySecurity();
 
             Assert.Throws<ArgumentNullException>("path", () => security.CreateDirectory(null));
             Assert.Throws<ArgumentException>(() => security.CreateDirectory(""));
         }
 
         [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)
-        {
-            DirectorySecurity security = GetDirectorySecurity(rights, controlType);
-            Verify_DirectorySecurity_CreateDirectory(security);
-        }
-
-        [Fact]
         public void DirectorySecurity_CreateDirectory_DirectoryAlreadyExists()
         {
-            using var directory = new TempDirectory();
-            string path = Path.Combine(directory.Path, "createMe");
-
-            DirectorySecurity basicSecurity = new DirectorySecurity();
-            basicSecurity.CreateDirectory(path);
+            using var tempRootDir = new TempAclDirectory();
+            string path = Path.Combine(tempRootDir.Path, "createMe");
 
-            Assert.True(Directory.Exists(path));
+            DirectorySecurity expectedSecurity = GetDirectorySecurity(FileSystemRights.FullControl);
+            DirectoryInfo dirInfo = expectedSecurity.CreateDirectory(path);
+            Assert.True(dirInfo.Exists);
+            tempRootDir.CreatedSubdirectories.Add(dirInfo);
 
-            DirectorySecurity specificSecurity = GetDirectorySecurity(FileSystemRights.ExecuteFile, AccessControlType.Deny);
+            var basicSecurity = new DirectorySecurity();
+            // Already exists, existingDirInfo should have the original security, not the new basic security
+            DirectoryInfo existingDirInfo = basicSecurity.CreateDirectory(path);
 
-            // Already exists, existingDirInfo should have the original basic security, not the new specific security
-            DirectoryInfo existingDirInfo = specificSecurity.CreateDirectory(path);
-
-            DirectorySecurity actualSecurity = existingDirInfo.GetAccessControl();
-
-            VerifyAccessSecurity(basicSecurity, actualSecurity);
+            DirectorySecurity actualSecurity = existingDirInfo.GetAccessControl(AccessControlSections.Access);
+            VerifyAccessSecurity(expectedSecurity, actualSecurity);
         }
 
         #endregion
@@ -499,86 +496,110 @@ namespace System.IO
 
         #region Helper methods
 
-        private DirectorySecurity GetDirectorySecurity(FileSystemRights rights, AccessControlType controlType) => GetDirectorySecurity(WellKnownSidType.BuiltinUsersSid, rights, controlType);
-
-        private DirectorySecurity GetDirectorySecurity(WellKnownSidType sid, FileSystemRights rights, AccessControlType controlType)
-        {
-            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));
-
-            DirectoryInfo actualInfo = new DirectoryInfo(info.FullName);
-
-            DirectorySecurity actualSecurity = actualInfo.GetAccessControl();
-
+        public static IEnumerable<object[]> 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 };
+        }
+
+        public static IEnumerable<object[]> 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.WriteData, FileShare.Read, DefaultBufferSize, FileOptions.None, 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 TempDirectory();
-            string path = Path.Combine(directory.Path, "createMe");
-
-            expectedSecurity.CreateDirectory(path);
-
-            Assert.True(Directory.Exists(path));
-
-            DirectoryInfo actualInfo = new DirectoryInfo(path);
+            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);
 
-            DirectorySecurity actualSecurity = actualInfo.GetAccessControl();
+            var actualDirInfo = new DirectoryInfo(path);
+            DirectorySecurity actualSecurity = actualDirInfo.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 DirectorySecurity GetDirectorySecurity(FileSystemRights rights)
         {
-            FileSecurity security = new FileSecurity();
-
-            SecurityIdentifier identity = new SecurityIdentifier(sid, null);
-            FileSystemAccessRule accessRule = new FileSystemAccessRule(identity, rights, controlType);
+            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 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)
+        private FileSecurity GetFileSecurity(FileSystemRights rights)
         {
-            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();
-
-            VerifyAccessSecurity(expectedSecurity, actualSecurity);
+            var security = new FileSecurity();
+            var identity = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null);
+            var accessRule = new FileSystemAccessRule(identity, rights, AccessControlType.Allow);
+            security.AddAccessRule(accessRule);
+            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 c3d83e8..8c4fa0e 100644 (file)
--- 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 @@
     <Compile Include="FileSystemAuditRuleTests.cs" />
     <Compile Include="FileSystemSecurityTests.cs" />
     <Compile Include="Helpers.cs" />
+    <Compile Include="TempAclDirectory.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\src\System.IO.FileSystem.AccessControl.csproj" SkipUseReferenceAssembly="true" />

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 (file)
index 0000000..5ecf047
--- /dev/null
+++ b/src/libraries/System.IO.FileSystem.AccessControl/tests/TempAclDirectory.cs
@@ -0,0 +1,66 @@
+// 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;
+
+namespace System.IO
+{
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public sealed class TempAclDirectory : TempDirectory
+    {
+        internal readonly List<DirectoryInfo> CreatedSubdirectories = new();
+        internal readonly List<FileInfo> CreatedSubfiles = new();
+        protected override void DeleteDirectory()
+        {
+            try
+            {
+                foreach (DirectoryInfo subdir in CreatedSubdirectories)
+                {
+                    ResetFullControlToDirectory(subdir);
+                }
+
+                foreach (FileInfo subfile in CreatedSubfiles)
+                {
+                    ResetFullControlToFile(subfile);
+                }
+
+                var rootDirInfo = new DirectoryInfo(Path);
+                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 */ }
+        }
+    }
+}