From bf1aa1d81851f14472412867cfd77a7868103aca Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Thu, 20 Sep 2018 12:44:19 -0700 Subject: [PATCH 1/2] Make tests classes and HubMessageType enum package private in the Java client(#2992) --- .../main/java/com/microsoft/aspnet/signalr/HubMessageType.java | 2 +- .../com/microsoft/aspnet/signalr/HandshakeProtocolTest.java | 2 +- .../java/com/microsoft/aspnet/signalr/HubConnectionTest.java | 2 +- .../java/com/microsoft/aspnet/signalr/HubExceptionTest.java | 2 +- .../java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java | 2 +- .../com/microsoft/aspnet/signalr/NegotiateResponseTest.java | 2 +- .../com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java | 2 +- .../com/microsoft/aspnet/signalr/WebSocketTransportTest.java | 2 +- .../aspnet/signalr/WebSocketTransportUrlFormatTest.java | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubMessageType.java b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubMessageType.java index a02075858a..859ce257b0 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubMessageType.java +++ b/clients/java/signalr/src/main/java/com/microsoft/aspnet/signalr/HubMessageType.java @@ -3,7 +3,7 @@ package com.microsoft.aspnet.signalr; -public enum HubMessageType { +enum HubMessageType { INVOCATION(1), STREAM_ITEM(2), COMPLETION(3), diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java index ff2ea82184..69eb504189 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HandshakeProtocolTest.java @@ -7,7 +7,7 @@ import static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.Test; -public class HandshakeProtocolTest { +class HandshakeProtocolTest { @Test public void VerifyCreateHandshakerequestMessage() { diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java index 47acc2178b..d713bad31a 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubConnectionTest.java @@ -14,7 +14,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; -public class HubConnectionTest { +class HubConnectionTest { private static final String RECORD_SEPARATOR = "\u001e"; @Test diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java index 0e306e5890..f6de72d45a 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/HubExceptionTest.java @@ -7,7 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import org.junit.jupiter.api.Test; -public class HubExceptionTest { +class HubExceptionTest { @Test public void VeryHubExceptionMesssageIsSet() { String errorMessage = "This is a HubException"; diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java index 08fe5c1065..ccd4cd33ec 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/JsonHubProtocolTest.java @@ -12,7 +12,7 @@ import java.util.List; import org.junit.jupiter.api.Test; -public class JsonHubProtocolTest { +class JsonHubProtocolTest { private JsonHubProtocol jsonHubProtocol = new JsonHubProtocol(); @Test diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java index 7c2be3b9fc..ec2d4180ca 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/NegotiateResponseTest.java @@ -8,7 +8,7 @@ import static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.Test; -public class NegotiateResponseTest { +class NegotiateResponseTest { @Test public void VerifyNegotiateResponse() { diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java index 9f852c7725..8e96e3a2fe 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/ResolveNegotiateUrlTest.java @@ -11,7 +11,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -public class ResolveNegotiateUrlTest { +class ResolveNegotiateUrlTest { private static Stream protocols() { return Stream.of( Arguments.of("http://example.com/hub/", "http://example.com/hub/negotiate"), diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java index 31116d4d65..d3a2985744 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportTest.java @@ -9,7 +9,7 @@ import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; -public class WebSocketTransportTest { +class WebSocketTransportTest { @Test public void WebsocketThrowsIfItCantConnect() throws Exception { Transport transport = new WebSocketTransport("www.notarealurl12345.fake", new NullLogger()); diff --git a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java index f7df0d956f..d4294cd642 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java +++ b/clients/java/signalr/src/test/java/com/microsoft/aspnet/signalr/WebSocketTransportUrlFormatTest.java @@ -12,7 +12,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -public class WebSocketTransportUrlFormatTest { +class WebSocketTransportUrlFormatTest { private static Stream protocols() { return Stream.of( Arguments.of("http://example.com", "ws://example.com"), From f0a34a4ca46129b8434020dedb8c360e751e2963 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 20 Sep 2018 13:39:25 -0700 Subject: [PATCH 2/2] Clone Windows Identity in LongPolling connections (#2985) --- build/dependencies.props | 1 + .../Internal/HttpConnectionContext.cs | 9 +++++ .../Internal/HttpConnectionDispatcher.cs | 40 +++++++++++++++---- ...crosoft.AspNetCore.Http.Connections.csproj | 1 + 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 60617932b5..11bca983ea 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -69,6 +69,7 @@ 3.1.1 4.3.0 4.5.1 + 4.5.0 4.5.0 4.5.1 2.3.1 diff --git a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs index 3b31756d86..fd9d22490a 100644 --- a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs +++ b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionContext.cs @@ -6,6 +6,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.IO.Pipelines; using System.Security.Claims; +using System.Security.Principal; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; @@ -208,6 +209,14 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal Cancellation?.Dispose(); Cancellation = null; + + if (User != null && User.Identity is WindowsIdentity) + { + foreach (var identity in User.Identities) + { + (identity as IDisposable)?.Dispose(); + } + } } await disposeTask; diff --git a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs index 24e0090986..3b2d467673 100644 --- a/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs +++ b/src/Microsoft.AspNetCore.Http.Connections/Internal/HttpConnectionDispatcher.cs @@ -7,11 +7,11 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.IO.Pipelines; +using System.Security.Claims; +using System.Security.Principal; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Connections.Features; -using Microsoft.AspNetCore.Http.Connections.Features; using Microsoft.AspNetCore.Http.Connections.Internal.Transports; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Internal; @@ -608,9 +608,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal return false; } - // Setup the connection state from the http context - connection.User = context.User; - // Configure transport-specific features. if (transportType == HttpTransportType.LongPolling) { @@ -630,7 +627,15 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal { // Set the request trace identifier to the current http request handling the poll existing.TraceIdentifier = context.TraceIdentifier; - existing.User = context.User; + + // Don't copy the identity if it's a windows identity + // We specifically clone the identity on first poll if it's a windows identity + // If we swapped the new User here we'd have to dispose the old identities which could race with the application + // trying to access the identity. + if (context.User.Identity is WindowsIdentity) + { + existing.User = context.User; + } } } else @@ -638,6 +643,9 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal connection.HttpContext = context; } + // Setup the connection state from the http context + connection.User = connection.HttpContext.User; + // Set the Connection ID on the logging scope so that logs from now on will have the // Connection ID metadata set. logScope.ConnectionId = connection.ConnectionId; @@ -645,6 +653,23 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal return true; } + private static void CloneUser(HttpContext newContext, HttpContext oldContext) + { + if (oldContext.User.Identity is WindowsIdentity) + { + newContext.User = new ClaimsPrincipal(); + + foreach (var identity in oldContext.User.Identities) + { + newContext.User.AddIdentity(identity.Clone()); + } + } + else + { + newContext.User = oldContext.User; + } + } + private static HttpContext CloneHttpContext(HttpContext context) { // The reason we're copying the base features instead of the HttpContext properties is @@ -692,7 +717,8 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal var newHttpContext = new DefaultHttpContext(features); newHttpContext.TraceIdentifier = context.TraceIdentifier; - newHttpContext.User = context.User; + + CloneUser(newHttpContext, context); // Making request services function property could be tricky and expensive as it would require // DI scope per connection. It would also mean that services resolved in middleware leading up to here diff --git a/src/Microsoft.AspNetCore.Http.Connections/Microsoft.AspNetCore.Http.Connections.csproj b/src/Microsoft.AspNetCore.Http.Connections/Microsoft.AspNetCore.Http.Connections.csproj index 44cd96c75a..1cc7e47c44 100644 --- a/src/Microsoft.AspNetCore.Http.Connections/Microsoft.AspNetCore.Http.Connections.csproj +++ b/src/Microsoft.AspNetCore.Http.Connections/Microsoft.AspNetCore.Http.Connections.csproj @@ -29,6 +29,7 @@ +