AuthorizeHelper will no-op if endpoint routing is used (#10471)

Author: BrennanConroy

src/Analyzers/Analyzers/test/TestFiles/CompilationFeatureDetectorTest/StartupWithUseSignalR.cs

@@ -9,10 +9,12 @@ public class StartupWithUseSignalR
     {
         public void Configure(IApplicationBuilder app)
         {
+#pragma warning disable CS0618 // Type or member is obsolete
             app.UseSignalR(routes =>
             {
 
             });
+#pragma warning restore CS0618 // Type or member is obsolete
         }
     }
 }

src/SignalR/clients/csharp/Client/test/FunctionalTests/VersionStartup.cs

@@ -1,18 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
-using System.IdentityModel.Tokens.Jwt;
-using System.Security.Claims;
-using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.AspNetCore.Builder;
-using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Connections;
 using Microsoft.AspNetCore.SignalR.Protocol;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
-using Microsoft.IdentityModel.Tokens;
-using Newtonsoft.Json;
 
 namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
 {
@@ -33,11 +25,12 @@ public void ConfigureServices(IServiceCollection services)
 
         public void Configure(IApplicationBuilder app)
         {
+            app.UseRouting();
             app.UseAuthentication();
 
-            app.UseSignalR(routes =>
+            app.UseEndpoints(endpoints =>
             {
-                routes.MapHub<VersionHub>("/version");
+                endpoints.MapHub<VersionHub>("/version");
             });
         }
     }

src/SignalR/common/Http.Connections/ref/Microsoft.AspNetCore.Http.Connections.netcoreapp3.0.cs

@@ -12,6 +12,7 @@ public static partial class ConnectionEndpointRouteBuilderExtensions
     }
     public static partial class ConnectionsAppBuilderExtensions
     {
+        [System.ObsoleteAttribute("This method is obsolete and will be removed in a future version. The recommended alternative is to use MapConnections or MapConnectionHandler<TConnectionHandler> inside Microsoft.AspNetCore.Builder.UseEndpoints(...).")]
         public static Microsoft.AspNetCore.Builder.IApplicationBuilder UseConnections(this Microsoft.AspNetCore.Builder.IApplicationBuilder app, System.Action<Microsoft.AspNetCore.Http.Connections.ConnectionsRouteBuilder> configure) { throw null; }
     }
 }
@@ -28,6 +29,7 @@ public partial class ConnectionOptionsSetup : Microsoft.Extensions.Options.IConf
         public ConnectionOptionsSetup() { }
         public void Configure(Microsoft.AspNetCore.Http.Connections.ConnectionOptions options) { }
     }
+    [System.ObsoleteAttribute("This class is obsolete and will be removed in a future version. The recommended alternative is to use MapConnection and MapConnectionHandler<TConnectionHandler> inside Microsoft.AspNetCore.Builder.UseEndpoints(...).")]
     public partial class ConnectionsRouteBuilder
     {
         internal ConnectionsRouteBuilder() { }

src/SignalR/common/Http.Connections/src/ConnectionEndpointRouteBuilderExtensions.cs

@@ -3,8 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Connections;
 using Microsoft.AspNetCore.Http.Connections;
 using Microsoft.AspNetCore.Http.Connections.Internal;
@@ -48,20 +46,14 @@ public static IEndpointConventionBuilder MapConnectionHandler<TConnectionHandler
         public static IEndpointConventionBuilder MapConnectionHandler<TConnectionHandler>(this IEndpointRouteBuilder endpoints, string pattern, Action<HttpConnectionDispatcherOptions> configureOptions) where TConnectionHandler : ConnectionHandler
         {
             var options = new HttpConnectionDispatcherOptions();
-            // REVIEW: WE should consider removing this and instead just relying on the
-            // AuthorizationMiddleware
-            var attributes = typeof(TConnectionHandler).GetCustomAttributes(inherit: true);
-            foreach (var attribute in attributes.OfType<AuthorizeAttribute>())
-            {
-                options.AuthorizationData.Add(attribute);
-            }
             configureOptions?.Invoke(options);
 
             var conventionBuilder = endpoints.MapConnections(pattern, options, b =>
             {
                 b.UseConnectionHandler<TConnectionHandler>();
             });
 
+            var attributes = typeof(TConnectionHandler).GetCustomAttributes(inherit: true);
             conventionBuilder.Add(e =>
             {
                 // Add all attributes on the ConnectionHandler has metadata (this will allow for things like)
@@ -93,7 +85,7 @@ public static IEndpointConventionBuilder MapConnections(this IEndpointRouteBuild
             var connectionDelegate = connectionBuilder.Build();
 
             // REVIEW: Consider expanding the internals of the dispatcher as endpoint routes instead of
-            // using if statemants we can let the matcher handle
+            // using if statements we can let the matcher handle
 
             var conventionBuilders = new List<IEndpointConventionBuilder>();
 

src/SignalR/common/Http.Connections/src/ConnectionsAppBuilderExtensions.cs

@@ -3,9 +3,6 @@
 
 using System;
 using Microsoft.AspNetCore.Http.Connections;
-using Microsoft.AspNetCore.Http.Connections.Internal;
-using Microsoft.AspNetCore.Routing;
-using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.AspNetCore.Builder
 {
@@ -16,25 +13,29 @@ public static class ConnectionsAppBuilderExtensions
     {
         /// <summary>
         /// Adds support for ASP.NET Core Connection Handlers to the <see cref="IApplicationBuilder"/> request execution pipeline.
+        /// <para>
+        /// This method is obsolete and will be removed in a future version.
+        /// The recommended alternative is to use MapConnections or MapConnectionHandler&#60;TConnectionHandler&#62; inside Microsoft.AspNetCore.Builder.UseEndpoints(...).
+        /// </para>
         /// </summary>
         /// <param name="app">The <see cref="IApplicationBuilder"/>.</param>
         /// <param name="configure">A callback to configure connection routes.</param>
         /// <returns>The same instance of the <see cref="IApplicationBuilder"/> for chaining.</returns>
+        [Obsolete("This method is obsolete and will be removed in a future version. The recommended alternative is to use MapConnections or MapConnectionHandler<TConnectionHandler> inside Microsoft.AspNetCore.Builder.UseEndpoints(...).")]
         public static IApplicationBuilder UseConnections(this IApplicationBuilder app, Action<ConnectionsRouteBuilder> configure)
         {
             if (configure == null)
             {
                 throw new ArgumentNullException(nameof(configure));
             }
 
-            var dispatcher = app.ApplicationServices.GetRequiredService<HttpConnectionDispatcher>();
-
-            var routes = new RouteBuilder(app);
-
-            configure(new ConnectionsRouteBuilder(routes, dispatcher));
-
             app.UseWebSockets();
-            app.UseRouter(routes.Build());
+            app.UseRouting();
+            app.UseAuthorization();
+            app.UseEndpoints(endpoints =>
+            {
+                configure(new ConnectionsRouteBuilder(endpoints));
+            });
             return app;
         }
     }

src/SignalR/common/Http.Connections/src/ConnectionsRouteBuilder.cs

@@ -2,26 +2,27 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Reflection;
-using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Connections;
-using Microsoft.AspNetCore.Http.Connections.Internal;
 using Microsoft.AspNetCore.Routing;
 
 namespace Microsoft.AspNetCore.Http.Connections
 {
     /// <summary>
     /// Maps routes to ASP.NET Core Connection Handlers.
+    /// <para>
+    /// This class is obsolete and will be removed in a future version.
+    /// The recommended alternative is to use MapConnection and MapConnectionHandler&#60;TConnectionHandler&#62; inside Microsoft.AspNetCore.Builder.UseEndpoints(...).
+    /// </para>
     /// </summary>
+    [Obsolete("This class is obsolete and will be removed in a future version. The recommended alternative is to use MapConnection and MapConnectionHandler<TConnectionHandler> inside Microsoft.AspNetCore.Builder.UseEndpoints(...).")]
     public class ConnectionsRouteBuilder
     {
-        private readonly HttpConnectionDispatcher _dispatcher;
-        private readonly RouteBuilder _routes;
+        private readonly IEndpointRouteBuilder _endpoints;
 
-        internal ConnectionsRouteBuilder(RouteBuilder routes, HttpConnectionDispatcher dispatcher)
+        internal ConnectionsRouteBuilder(IEndpointRouteBuilder endpoints)
         {
-            _routes = routes;
-            _dispatcher = dispatcher;
+            _endpoints = endpoints;
         }
 
         /// <summary>
@@ -38,45 +39,24 @@ public void MapConnections(PathString path, Action<IConnectionBuilder> configure
         /// <param name="path">The request path.</param>
         /// <param name="options">Options used to configure the connection.</param>
         /// <param name="configure">A callback to configure the connection.</param>
-        public void MapConnections(PathString path, HttpConnectionDispatcherOptions options, Action<IConnectionBuilder> configure)
-        {
-            var connectionBuilder = new ConnectionBuilder(_routes.ServiceProvider);
-            configure(connectionBuilder);
-            var socket = connectionBuilder.Build();
-            _routes.MapRoute(path, c => _dispatcher.ExecuteAsync(c, options, socket));
-            _routes.MapRoute(path + "/negotiate", c => _dispatcher.ExecuteNegotiateAsync(c, options));
-        }
+        public void MapConnections(PathString path, HttpConnectionDispatcherOptions options, Action<IConnectionBuilder> configure) =>
+            _endpoints.MapConnections(path, options, configure);
 
         /// <summary>
         /// Maps incoming requests with the specified path to the provided connection pipeline.
         /// </summary>
         /// <typeparam name="TConnectionHandler">The <see cref="ConnectionHandler"/> type.</typeparam>
         /// <param name="path">The request path.</param>
-        public void MapConnectionHandler<TConnectionHandler>(PathString path) where TConnectionHandler : ConnectionHandler
-        {
+        public void MapConnectionHandler<TConnectionHandler>(PathString path) where TConnectionHandler : ConnectionHandler =>
             MapConnectionHandler<TConnectionHandler>(path, configureOptions: null);
-        }
 
         /// <summary>
         /// Maps incoming requests with the specified path to the provided connection pipeline.
         /// </summary>
         /// <typeparam name="TConnectionHandler">The <see cref="ConnectionHandler"/> type.</typeparam>
         /// <param name="path">The request path.</param>
         /// <param name="configureOptions">A callback to configure dispatcher options.</param>
-        public void MapConnectionHandler<TConnectionHandler>(PathString path, Action<HttpConnectionDispatcherOptions> configureOptions) where TConnectionHandler : ConnectionHandler
-        {
-            var authorizeAttributes = typeof(TConnectionHandler).GetCustomAttributes<AuthorizeAttribute>(inherit: true);
-            var options = new HttpConnectionDispatcherOptions();
-            foreach (var attribute in authorizeAttributes)
-            {
-                options.AuthorizationData.Add(attribute);
-            }
-            configureOptions?.Invoke(options);
-
-            MapConnections(path, options, builder =>
-            {
-                builder.UseConnectionHandler<TConnectionHandler>();
-            });
-        }
+        public void MapConnectionHandler<TConnectionHandler>(PathString path, Action<HttpConnectionDispatcherOptions> configureOptions) where TConnectionHandler : ConnectionHandler =>
+            _endpoints.MapConnectionHandler<TConnectionHandler>(path, configureOptions);
     }
 }

src/SignalR/common/Http.Connections/src/Internal/AuthorizeHelper.cs

@@ -61,11 +61,6 @@ public async Task ExecuteAsync(HttpContext context, HttpConnectionDispatcherOpti
             var logScope = new ConnectionLogScope(GetConnectionId(context));
             using (_logger.BeginScope(logScope))
             {
-                if (!await AuthorizeHelper.AuthorizeAsync(context, options.AuthorizationData))
-                {
-                    return;
-                }
-
                 if (HttpMethods.IsPost(context.Request.Method))
                 {
                     // POST /{path}
@@ -95,11 +90,6 @@ public async Task ExecuteNegotiateAsync(HttpContext context, HttpConnectionDispa
             var logScope = new ConnectionLogScope(connectionId: string.Empty);
             using (_logger.BeginScope(logScope))
             {
-                if (!await AuthorizeHelper.AuthorizeAsync(context, options.AuthorizationData))
-                {
-                    return;
-                }
-
                 if (HttpMethods.IsPost(context.Request.Method))
                 {
                     // POST /{path}/negotiate

src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs

@@ -14,6 +14,7 @@
     <Compile Include="$(SignalRSharedSourceRoot)WebSocketExtensions.cs" Link="WebSocketExtensions.cs" />
     <Compile Include="$(SignalRSharedSourceRoot)StreamExtensions.cs" Link="StreamExtensions.cs" />
     <Compile Include="$(SignalRSharedSourceRoot)DuplexPipe.cs" Link="DuplexPipe.cs" />
+    <Compile Include="$(SignalRSharedSourceRoot)TaskCache.cs" Link="Internal\TaskCache.cs" />
   </ItemGroup>
 
   <ItemGroup>