Add missing tests for NamedPipes (#72956)

* Add missing tests for NamedPipes

* Address feedback

* Move event wait to happen one line before Connect.

* Bring back previous version of test

* Reword SkipTestException message

* Move NamedPipeTest.CurrentUserOnly.cs to compile for all platforms

* Address feedback

* Remove IsRemoteExecutorSupportedAndOnUnixAndSuperUser

* Keep RemoteExecutor.IsSupported

src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs

@@ -23,22 +23,28 @@ public NamedPipeTest_CurrentUserOnly_Unix(ITestOutputHelper output)
             _output = output;
         }
 
-        private static bool IsRemoteExecutorSupportedAndOnUnixAndSuperUser => RemoteExecutor.IsSupported && PlatformDetection.IsUnixAndSuperUser;
-
-        [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))]
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [OuterLoop("Needs sudo access")]
-        [InlineData(PipeOptions.None, PipeOptions.None)]
-        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly)]
-        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None)]
-        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly)]
+        [InlineData(PipeOptions.None, PipeOptions.None, PipeDirection.In)]
+        [InlineData(PipeOptions.None, PipeOptions.None, PipeDirection.InOut)]
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
         public async Task Connection_UnderDifferentUsers_BehavesAsExpected(
-            PipeOptions serverPipeOptions, PipeOptions clientPipeOptions)
+            PipeOptions serverPipeOptions, PipeOptions clientPipeOptions, PipeDirection clientPipeDirection)
         {
+            bool isRoot = AdminHelpers.IsProcessElevated();
+            if (clientPipeOptions == PipeOptions.CurrentUserOnly && isRoot)
+            {
+                throw new SkipTestException("Current user is root, RemoteExecutor is unable to use a different user for CurrentUserOnly.");
+            }
 
             // Use an absolute path, otherwise, the test can fail if the remote invoker and test runner have
             // different working and/or temp directories.
             string pipeName = "/tmp/" + Path.GetRandomFileName();
-            bool isRoot = Environment.UserName == "root";
 
             _output.WriteLine("Starting as {0} on '{1}'", Environment.UserName, pipeName);
 
@@ -48,9 +54,10 @@ public async Task Connection_UnderDifferentUsers_BehavesAsExpected(
                 Task serverTask = server.WaitForConnectionAsync(CancellationToken.None);
 
                 using (RemoteExecutor.Invoke(
-                    new Action<string, string>(ConnectClientFromRemoteInvoker),
+                    new Action<string, string, string>(ConnectClientFromRemoteInvoker),
                     pipeName,
-                    clientPipeOptions == PipeOptions.CurrentUserOnly && !isRoot ? "true" : "false",
+                    clientPipeOptions == PipeOptions.CurrentUserOnly ? "true" : "false",
+                    clientPipeDirection == PipeDirection.In ? "true" : "false",
                     new RemoteInvokeOptions { RunAsSudo = true }))
                 {
                 }
@@ -62,10 +69,12 @@ public async Task Connection_UnderDifferentUsers_BehavesAsExpected(
             }
         }
 
-        private static void ConnectClientFromRemoteInvoker(string pipeName, string isCurrentUserOnly)
+        private static void ConnectClientFromRemoteInvoker(string pipeName, string isCurrentUserOnly, string isReadOnly)
         {
             PipeOptions pipeOptions = bool.Parse(isCurrentUserOnly) ? PipeOptions.CurrentUserOnly : PipeOptions.None;
-            using (var client = new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, pipeOptions))
+            PipeDirection pipeDirection = bool.Parse(isReadOnly) ? PipeDirection.In : PipeDirection.InOut;
+
+            using (var client = new NamedPipeClientStream(".", pipeName, pipeDirection, pipeOptions))
             {
                 if (pipeOptions == PipeOptions.CurrentUserOnly)
                     Assert.Throws<UnauthorizedAccessException>(() => client.Connect());

src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Windows.cs

@@ -102,8 +102,9 @@ public void RunImpersonated(Action action)
             {
                 WindowsIdentity.RunImpersonated(_testAccountTokenHandle, () =>
                 {
-                    using (WindowsIdentity clientIdentity = WindowsIdentity.GetCurrent())
-                        Assert.NotEqual(serverIdentity.Name, clientIdentity.Name);
+                    using WindowsIdentity clientIdentity = WindowsIdentity.GetCurrent();
+                    Assert.NotEqual(serverIdentity.Name, clientIdentity.Name);
+                    Assert.False(new WindowsPrincipal(clientIdentity).IsInRole(WindowsBuiltInRole.Administrator));
 
                     action();
                 });
@@ -131,14 +132,52 @@ public NamedPipeTest_CurrentUserOnly_Windows(TestAccountImpersonator testAccount
             _testAccountImpersonator = testAccountImpersonator;
         }
 
+        [OuterLoop]
+        [ConditionalFact(nameof(IsAdminOnSupportedWindowsVersions))]
+        public async Task Connection_UnderDifferentUsers_CurrentUserOnlyOnServer_InvalidClientConnectionAttempts_DoNotBlockSuccessfulClient()
+        {
+            string name = PipeStreamConformanceTests.GetUniquePipeName();
+            bool invalidClientShouldStop = false;
+            bool invalidClientIsRunning = false;
+
+            using var server = new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly);
+            Task serverWait = server.WaitForConnectionAsync();
+
+            Task invalidClient = Task.Run(() =>
+            {
+                // invalid non-current user tries to connect to server.
+                _testAccountImpersonator.RunImpersonated(() =>
+                {
+                    while (!Volatile.Read(ref invalidClientShouldStop))
+                    {
+                        using var client = new NamedPipeClientStream(".", name, PipeDirection.In, PipeOptions.Asynchronous);
+                        Assert.Throws<UnauthorizedAccessException>(() => client.Connect());
+                        Volatile.Write(ref invalidClientIsRunning, true);
+                    }
+                });
+            });
+
+            Assert.False(serverWait.IsCompleted);
+            while (!Volatile.Read(ref invalidClientIsRunning)) ; // Wait until the invalid client starts running.
+
+            // valid client tries to connect and succeeds.
+            using var client = new NamedPipeClientStream(".", name, PipeDirection.In, PipeOptions.Asynchronous);
+            client.Connect();
+            await serverWait;
+            Volatile.Write(ref invalidClientShouldStop, true);
+        }
+
         [OuterLoop]
         [ConditionalTheory(nameof(IsAdminOnSupportedWindowsVersions))]
-        [InlineData(PipeOptions.None, PipeOptions.None)]
-        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly)]
-        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None)]
-        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly)]
+        [InlineData(PipeOptions.None, PipeOptions.None, PipeDirection.InOut)] // Fails even without CurrentUserOnly, because under the default pipe ACL, other users are denied Write access, and client is requesting PipeDirection.InOut
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
         public void Connection_UnderDifferentUsers_BehavesAsExpected(
-            PipeOptions serverPipeOptions, PipeOptions clientPipeOptions)
+            PipeOptions serverPipeOptions, PipeOptions clientPipeOptions, PipeDirection clientDirection)
         {
             string name = PipeStreamConformanceTests.GetUniquePipeName();
             using (var cts = new CancellationTokenSource())
@@ -148,16 +187,25 @@ public void Connection_UnderDifferentUsers_BehavesAsExpected(
 
                 _testAccountImpersonator.RunImpersonated(() =>
                 {
-                    using (var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, clientPipeOptions))
+                    using (var client = new NamedPipeClientStream(".", name, clientDirection, clientPipeOptions))
                     {
                         Assert.Throws<UnauthorizedAccessException>(() => client.Connect());
                     }
                 });
 
-                // Server is expected to not have received any request.
-                cts.Cancel();
-                AggregateException e = Assert.Throws<AggregateException>(() => serverTask.Wait(10_000));
-                Assert.IsType<TaskCanceledException>(e.InnerException);
+                if (serverPipeOptions == PipeOptions.None && clientPipeOptions == PipeOptions.CurrentUserOnly && clientDirection == PipeDirection.In)
+                {
+                    // When CurrentUserOnly is only on client side and asks for ReadOnly access, the connection is not rejected
+                    // but we get the UnauthorizedAccessException on the client regardless.
+                    Assert.True(serverTask.IsCompletedSuccessfully);
+                }
+                else
+                {
+                    // Server is expected to not have received any request.
+                    cts.Cancel();
+                    AggregateException ex = Assert.Throws<AggregateException>(() => serverTask.Wait(10_000));
+                    Assert.IsType<TaskCanceledException>(ex.InnerException);
+                }
             }
         }
 

src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.cs

@@ -54,13 +54,19 @@ public static void CreateServer_ConnectClient_UsingUnixAbsolutePath()
         }
 
         [Theory]
-        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly)]
-        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None)]
-        public static void Connection_UnderSameUser_SingleSide_CurrentUserOnly_Works(PipeOptions serverPipeOptions, PipeOptions clientPipeOptions)
+        [InlineData(PipeOptions.None, PipeOptions.None, PipeDirection.In)]
+        [InlineData(PipeOptions.None, PipeOptions.None, PipeDirection.InOut)]
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.None, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.None, PipeDirection.InOut)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.In)]
+        [InlineData(PipeOptions.CurrentUserOnly, PipeOptions.CurrentUserOnly, PipeDirection.InOut)]
+        public static void Connection_UnderSameUser_CurrentUserOnly_Works(PipeOptions serverPipeOptions, PipeOptions clientPipeOptions, PipeDirection clientDirection)
         {
             string name = PipeStreamConformanceTests.GetUniquePipeName();
             using (var server = new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, serverPipeOptions))
-            using (var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, clientPipeOptions))
+            using (var client = new NamedPipeClientStream(".", name, clientDirection, clientPipeOptions))
             {
                 Task[] tasks = new[]
                 {

src/libraries/System.IO.Pipes/tests/System.IO.Pipes.Tests.csproj

@@ -18,13 +18,13 @@
     <Compile Include="NamedPipeTests\NamedPipeTest.CreateServer.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.CreateClient.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.CrossProcess.cs" />
+    <Compile Include="NamedPipeTests\NamedPipeTest.CurrentUserOnly.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.Specific.cs" />
     <Compile Include="PipeStream.ProtectedMethods.Tests.cs" />
     <Compile Include="PipeStreamConformanceTests.cs" />
     <Compile Include="$(CommonTestPath)System\Threading\Tasks\TaskTimeoutExtensions.cs" Link="Common\System\Threading\Tasks\TaskTimeoutExtensions.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'windows'">
-    <Compile Include="NamedPipeTests\NamedPipeTest.CurrentUserOnly.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.CurrentUserOnly.Windows.cs" />
     <Compile Include="NamedPipeTests\NamedPipeTest.RunAsClient.Windows.cs" />
     <Compile Include="InteropTest.Windows.cs" />