EndpointMiddleware does not invoke endpoint (#10650)

* EndpointMiddleware does not invoke endpoint

Fixes: #10649

The bug is that the endpoint delegate is not invoked when
SuppressCheckForUnhandledSecurityMetadata is set to true. This option is
provided so that can you suppress the security checks done by routing,
but right now what it does is suppress the entire middleware.

We had tests for the supression cases, but they didn't actually validate
that the middleware did any work, just that we don't throw. Fixed that.

* harden tests

src/Http/Routing/src/EndpointMiddleware.cs

@@ -36,22 +36,19 @@ namespace Microsoft.AspNetCore.Routing
             var endpoint = httpContext.Features.Get<IEndpointFeature>()?.Endpoint;
             if (endpoint?.RequestDelegate != null)
             {
-                if (_routeOptions.SuppressCheckForUnhandledSecurityMetadata)
+                if (!_routeOptions.SuppressCheckForUnhandledSecurityMetadata)
                 {
-                    // User opted out of this check.
-                    return Task.CompletedTask;
-                }
+                    if (endpoint.Metadata.GetMetadata<IAuthorizeData>() != null &&
+                        !httpContext.Items.ContainsKey(AuthorizationMiddlewareInvokedKey))
+                    {
+                        ThrowMissingAuthMiddlewareException(endpoint);
+                    }
 
-                if (endpoint.Metadata.GetMetadata<IAuthorizeData>() != null &&
-                    !httpContext.Items.ContainsKey(AuthorizationMiddlewareInvokedKey))
-                {
-                    ThrowMissingAuthMiddlewareException(endpoint);
-                }
-
-                if (endpoint.Metadata.GetMetadata<ICorsMetadata>() != null &&
-                    !httpContext.Items.ContainsKey(CorsMiddlewareInvokedKey))
-                {
-                    ThrowMissingCorsMiddlewareException(endpoint);
+                    if (endpoint.Metadata.GetMetadata<ICorsMetadata>() != null &&
+                        !httpContext.Items.ContainsKey(CorsMiddlewareInvokedKey))
+                    {
+                        ThrowMissingCorsMiddlewareException(endpoint);
+                    }
                 }
 
                 Log.ExecutingEndpoint(_logger, endpoint);

src/Http/Routing/test/UnitTests/EndpointMiddlewareTest.cs

@@ -25,8 +25,10 @@ namespace Microsoft.AspNetCore.Routing
             var httpContext = new DefaultHttpContext();
             httpContext.RequestServices = new ServiceProvider();
 
+            var calledNext = false;
             RequestDelegate next = (c) =>
             {
+                calledNext = true;
                 return Task.CompletedTask;
             };
 
@@ -35,7 +37,8 @@ namespace Microsoft.AspNetCore.Routing
             // Act
             await middleware.Invoke(httpContext);
 
-            // Assert - does not throw
+            // Assert
+            Assert.True(calledNext);
         }
 
         [Fact]
@@ -46,37 +49,10 @@ namespace Microsoft.AspNetCore.Routing
             httpContext.RequestServices = new ServiceProvider();
             httpContext.SetEndpoint(null);
 
+            var calledNext = false;
             RequestDelegate next = (c) =>
             {
-                return Task.CompletedTask;
-            };
-
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, RouteOptions);
-
-            // Act
-            await middleware.Invoke(httpContext);
-
-            // Assert - does not throw
-        }
-
-        [Fact]
-        public async Task Invoke_WithEndpoint_InvokesDelegate()
-        {
-            // Arrange
-            var httpContext = new DefaultHttpContext();
-            httpContext.RequestServices = new ServiceProvider();
-
-            var invoked = false;
-            RequestDelegate endpointFunc = (c) =>
-            {
-                invoked = true;
-                return Task.CompletedTask;
-            };
-
-            httpContext.SetEndpoint(new Endpoint(endpointFunc, EndpointMetadataCollection.Empty, "Test"));
-
-            RequestDelegate next = (c) =>
-            {
+                calledNext = true;
                 return Task.CompletedTask;
             };
 
@@ -86,7 +62,37 @@ namespace Microsoft.AspNetCore.Routing
             await middleware.Invoke(httpContext);
 
             // Assert
-            Assert.True(invoked);
+            Assert.True(calledNext);
+        }
+
+        [Fact]
+        public async Task Invoke_WithEndpoint_InvokesDelegate()
+        {
+            // Arrange
+            var httpContext = new DefaultHttpContext();
+            httpContext.RequestServices = new ServiceProvider();
+
+            var calledEndpoint = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                calledEndpoint = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.SetEndpoint(new Endpoint(endpointFunc, EndpointMetadataCollection.Empty, "Test"));
+
+            RequestDelegate next = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, RouteOptions);
+
+            // Act
+            await middleware.Invoke(httpContext);
+
+            // Assert
+            Assert.True(calledEndpoint);
         }
 
         [Fact]
@@ -101,9 +107,14 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
+            RequestDelegate throwIfCalled = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
 
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, RouteOptions);
+            httpContext.SetEndpoint(new Endpoint(throwIfCalled, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, throwIfCalled, RouteOptions);
 
             // Act & Assert
             var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => middleware.Invoke(httpContext));
@@ -121,16 +132,29 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
+            var calledEndpoint = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                calledEndpoint = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.SetEndpoint(new Endpoint(endpointFunc, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
 
             httpContext.Items[EndpointMiddleware.AuthorizationMiddlewareInvokedKey] = true;
 
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, RouteOptions);
+            RequestDelegate next = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
 
-            // Act & Assert
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, RouteOptions);
+
+            // Act
             await middleware.Invoke(httpContext);
 
-            // If we got this far, we can sound the everything's OK alarm.
+            // Assert
+            Assert.True(calledEndpoint);
         }
 
         [Fact]
@@ -142,13 +166,29 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
+            var calledEndpoint = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                calledEndpoint = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.SetEndpoint(new Endpoint(endpointFunc, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
 
             var routeOptions = Options.Create(new RouteOptions { SuppressCheckForUnhandledSecurityMetadata = true });
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, routeOptions);
 
-            // Act & Assert
+            RequestDelegate next = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, routeOptions);
+
+            // Act
             await middleware.Invoke(httpContext);
+
+            // Assert
+            Assert.True(calledEndpoint);
         }
 
         [Fact]
@@ -163,9 +203,14 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<ICorsMetadata>()), "Test"));
+            RequestDelegate throwIfCalled = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
 
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, RouteOptions);
+            httpContext.SetEndpoint(new Endpoint(throwIfCalled, new EndpointMetadataCollection(Mock.Of<ICorsMetadata>()), "Test"));
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, throwIfCalled, RouteOptions);
 
             // Act & Assert
             var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => middleware.Invoke(httpContext));
@@ -183,16 +228,29 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<ICorsMetadata>()), "Test"));
+            var calledEndpoint = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                calledEndpoint = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.SetEndpoint(new Endpoint(endpointFunc, new EndpointMetadataCollection(Mock.Of<ICorsMetadata>()), "Test"));
 
             httpContext.Items[EndpointMiddleware.CorsMiddlewareInvokedKey] = true;
 
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, RouteOptions);
+            RequestDelegate next = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
 
-            // Act & Assert
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, RouteOptions);
+
+            // Act
             await middleware.Invoke(httpContext);
 
-            // If we got this far, we can sound the everything's OK alarm.
+            // Assert
+            Assert.True(calledEndpoint);
         }
 
         [Fact]
@@ -204,13 +262,29 @@ namespace Microsoft.AspNetCore.Routing
                 RequestServices = new ServiceProvider()
             };
 
-            httpContext.SetEndpoint(new Endpoint(_ => Task.CompletedTask, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
+            var calledEndpoint = false;
+            RequestDelegate endpointFunc = (c) =>
+            {
+                calledEndpoint = true;
+                return Task.CompletedTask;
+            };
+
+            httpContext.SetEndpoint(new Endpoint(endpointFunc, new EndpointMetadataCollection(Mock.Of<IAuthorizeData>()), "Test"));
 
             var routeOptions = Options.Create(new RouteOptions { SuppressCheckForUnhandledSecurityMetadata = true });
-            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, _ => Task.CompletedTask, routeOptions);
 
-            // Act & Assert
+            RequestDelegate next = (c) =>
+            {
+                throw new InvalidTimeZoneException("Should not be called");
+            };
+
+            var middleware = new EndpointMiddleware(NullLogger<EndpointMiddleware>.Instance, next, routeOptions);
+
+            // Act
             await middleware.Invoke(httpContext);
+
+            // Assert
+            Assert.True(calledEndpoint);
         }
 
         private class ServiceProvider : IServiceProvider