From b03df235002d34c7edbf7ccb12e101a4b10d7dac Mon Sep 17 00:00:00 2001 From: Bart Koelman <104792814+bart-vmware@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:01:27 +0100 Subject: [PATCH] Check exposure/enabled for HTTP verb match in management endpoints Instead of mapping endpoints in ASP.NET routing per verb, always map all verbs and then filter inside middleware. Advantages: - Returns 404 when not exposed/enabled with invalid verb, instead of 405 - Can change verbs at runtime Disadvantages: - Verb information is lost in route mappings endpoint (always shows all verbs) - Entries in route mappings with all verbs disabled are no longer hidden --- .../src/Endpoint/ActuatorEndpointMapper.cs | 37 ++---- .../Endpoint/Middleware/EndpointMiddleware.cs | 22 +++- .../HttpVerbInConventionalRoutingTest.cs | 123 +++++++++++++++++- .../Refresh/HttpVerbInEndpointRoutingTest.cs | 119 +++++++++++++++++ .../RouteMappings/EndpointMiddlewareTest.cs | 14 +- 5 files changed, 282 insertions(+), 33 deletions(-) diff --git a/src/Management/src/Endpoint/ActuatorEndpointMapper.cs b/src/Management/src/Endpoint/ActuatorEndpointMapper.cs index 4301f40d81..5171b98800 100755 --- a/src/Management/src/Endpoint/ActuatorEndpointMapper.cs +++ b/src/Management/src/Endpoint/ActuatorEndpointMapper.cs @@ -46,39 +46,30 @@ public void Map(IEndpointRouteBuilder endpointRouteBuilder, ActuatorConventionBu ArgumentNullException.ThrowIfNull(endpointRouteBuilder); ArgumentNullException.ThrowIfNull(actuatorConventionBuilder); - InnerMap(middleware => endpointRouteBuilder.CreateApplicationBuilder().UseMiddleware(middleware.GetType()).Build(), - (middleware, requestPath, pipeline) => - { - HashSet allowedVerbs = middleware.EndpointOptions.GetSafeAllowedVerbs(); - - if (allowedVerbs.Count > 0) - { - IEndpointConventionBuilder endpointConventionBuilder = endpointRouteBuilder.MapMethods(requestPath, allowedVerbs, pipeline); + InnerMap(middleware => endpointRouteBuilder.CreateApplicationBuilder().UseMiddleware(middleware.GetType()).Build(), (requestPath, pipeline) => + { + IEndpointConventionBuilder endpointConventionBuilder = endpointRouteBuilder.Map(requestPath, pipeline); - foreach (Action configureAction in _conventionOptionsMonitor.CurrentValue.ConfigureActions) - { - configureAction(endpointConventionBuilder); - } + foreach (Action configureAction in _conventionOptionsMonitor.CurrentValue.ConfigureActions) + { + configureAction(endpointConventionBuilder); + } - actuatorConventionBuilder.TrackTarget(endpointConventionBuilder); - } - }); + actuatorConventionBuilder.TrackTarget(endpointConventionBuilder); + }); } public void Map(IRouteBuilder routeBuilder) { ArgumentNullException.ThrowIfNull(routeBuilder); - InnerMap(middleware => routeBuilder.ApplicationBuilder.New().UseMiddleware(middleware.GetType()).Build(), (middleware, requestPath, pipeline) => + InnerMap(middleware => routeBuilder.ApplicationBuilder.New().UseMiddleware(middleware.GetType()).Build(), (requestPath, pipeline) => { - foreach (string verb in middleware.EndpointOptions.GetSafeAllowedVerbs()) - { - routeBuilder.MapVerb(verb, requestPath, pipeline); - } + routeBuilder.MapRoute(requestPath, pipeline); }); } - private void InnerMap(Func createPipeline, Action applyMapping) + private void InnerMap(Func createPipeline, Action applyMapping) { var collection = new HashSet(); @@ -95,7 +86,7 @@ private void InnerMap(Func createPipeline, } private void MapEndpoints(HashSet collection, string? baseRequestPath, IEnumerable middlewares, - Func createPipeline, Action applyMapping) + Func createPipeline, Action applyMapping) { foreach (IEndpointMiddleware middleware in middlewares) { @@ -105,7 +96,7 @@ private void MapEndpoints(HashSet collection, string? baseRequestPath, I if (collection.Add(requestPath)) { - applyMapping(middleware, requestPath, pipeline); + applyMapping(requestPath, pipeline); } else { diff --git a/src/Management/src/Endpoint/Middleware/EndpointMiddleware.cs b/src/Management/src/Endpoint/Middleware/EndpointMiddleware.cs index f75b9a81e9..7b001a9643 100644 --- a/src/Management/src/Endpoint/Middleware/EndpointMiddleware.cs +++ b/src/Management/src/Endpoint/Middleware/EndpointMiddleware.cs @@ -55,16 +55,32 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate? next) { ArgumentNullException.ThrowIfNull(context); + bool notFound = true; + bool verbNotAllowed = false; + if (ShouldInvoke(context.Request.Path)) { - TResult result = await InvokeEndpointHandlerAsync(context, context.RequestAborted); - await WriteResponseAsync(result, context, context.RequestAborted); + HashSet allowedVerbs = EndpointOptions.GetSafeAllowedVerbs(); + + notFound = allowedVerbs.Count == 0; + verbNotAllowed = !allowedVerbs.Contains(context.Request.Method); } - else + + if (notFound) { // Terminal middleware context.Response.StatusCode = (int)HttpStatusCode.NotFound; } + else if (verbNotAllowed) + { + // Terminal middleware + context.Response.StatusCode = (int)HttpStatusCode.MethodNotAllowed; + } + else + { + TResult result = await InvokeEndpointHandlerAsync(context, context.RequestAborted); + await WriteResponseAsync(result, context, context.RequestAborted); + } } protected abstract Task InvokeEndpointHandlerAsync(HttpContext context, CancellationToken cancellationToken); diff --git a/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInConventionalRoutingTest.cs b/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInConventionalRoutingTest.cs index 013faec173..a77be61c90 100644 --- a/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInConventionalRoutingTest.cs +++ b/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInConventionalRoutingTest.cs @@ -35,12 +35,67 @@ public async Task Allows_only_POST_requests() var requestUri = new Uri("/actuator/refresh", UriKind.Relative); HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); - getResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + getResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); postResponse.StatusCode.Should().Be(HttpStatusCode.OK); } + [Fact] + public async Task Can_be_configured_to_unexposed() + { + var appSettings = new Dictionary + { + ["Management:Endpoints:Actuator:Exposure:Include:0"] = string.Empty + }; + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddInMemoryCollection(appSettings); + builder.Services.AddControllersWithViews(options => options.EnableEndpointRouting = false); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.UseMvc(routes => routes.MapRoute("default", "{controller=Home}/{action=Index}/{id?}")); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task Can_be_configured_to_disabled() + { + var appSettings = new Dictionary + { + ["Management:Endpoints:Actuator:Exposure:Include:0"] = "refresh", + ["Management:Endpoints:Refresh:Enabled"] = "false" + }; + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddInMemoryCollection(appSettings); + builder.Services.AddControllersWithViews(options => options.EnableEndpointRouting = false); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.UseMvc(routes => routes.MapRoute("default", "{controller=Home}/{action=Index}/{id?}")); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + [Fact] public async Task Can_be_configured_to_allow_no_verbs() { @@ -94,7 +149,7 @@ public async Task Can_be_configured_to_allow_only_GET_requests() getResponse.StatusCode.Should().Be(HttpStatusCode.OK); HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); - postResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + postResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); } [Fact] @@ -125,4 +180,68 @@ public async Task Can_be_configured_to_allow_both_GET_and_POST_requests() HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); postResponse.StatusCode.Should().Be(HttpStatusCode.OK); } + + [Fact] + public async Task Can_change_allowed_verbs_at_runtime() + { + const string fileName = "appsettings.json"; + MemoryFileProvider fileProvider = new(); + + fileProvider.IncludeFile(fileName, """ + { + "Management": { + "Endpoints": { + "Actuator": { + "Exposure": { + "Include": ["refresh"] + } + } + } + } + } + """); + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddJsonFile(fileProvider, fileName, false, true); + builder.Services.AddControllersWithViews(options => options.EnableEndpointRouting = false); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.UseMvc(routes => routes.MapRoute("default", "{controller=Home}/{action=Index}/{id?}")); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.OK); + + fileProvider.ReplaceFile(fileName, """ + { + "Management": { + "Endpoints": { + "Actuator": { + "Exposure": { + "Include": ["refresh"] + } + }, + "Refresh": { + "AllowedVerbs": ["GET"] + } + } + } + } + """); + + fileProvider.NotifyChanged(); + + getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.OK); + + postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + } } diff --git a/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInEndpointRoutingTest.cs b/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInEndpointRoutingTest.cs index be2b60626a..2430eaa9a0 100644 --- a/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInEndpointRoutingTest.cs +++ b/src/Management/test/Endpoint.Test/Actuators/Refresh/HttpVerbInEndpointRoutingTest.cs @@ -41,6 +41,61 @@ public async Task Allows_only_POST_requests() postResponse.StatusCode.Should().Be(HttpStatusCode.OK); } + [Fact] + public async Task Can_be_configured_to_unexposed() + { + var appSettings = new Dictionary + { + ["Management:Endpoints:Actuator:Exposure:Include:0"] = string.Empty + }; + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddInMemoryCollection(appSettings); + builder.Services.AddControllersWithViews(); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.MapDefaultControllerRoute(); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + + [Fact] + public async Task Can_be_configured_to_disabled() + { + var appSettings = new Dictionary + { + ["Management:Endpoints:Actuator:Exposure:Include:0"] = "refresh", + ["Management:Endpoints:Refresh:Enabled"] = "false" + }; + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddInMemoryCollection(appSettings); + builder.Services.AddControllersWithViews(); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.MapDefaultControllerRoute(); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.NotFound); + } + [Fact] public async Task Can_be_configured_to_allow_no_verbs() { @@ -125,4 +180,68 @@ public async Task Can_be_configured_to_allow_both_GET_and_POST_requests() HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); postResponse.StatusCode.Should().Be(HttpStatusCode.OK); } + + [Fact] + public async Task Can_change_allowed_verbs_at_runtime() + { + const string fileName = "appsettings.json"; + MemoryFileProvider fileProvider = new(); + + fileProvider.IncludeFile(fileName, """ + { + "Management": { + "Endpoints": { + "Actuator": { + "Exposure": { + "Include": ["refresh"] + } + } + } + } + } + """); + + WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create(); + builder.Configuration.AddJsonFile(fileProvider, fileName, false, true); + builder.Services.AddControllersWithViews(); + builder.Services.AddRefreshActuator(); + + await using WebApplication app = builder.Build(); + app.MapDefaultControllerRoute(); + await app.StartAsync(); + + using HttpClient httpClient = app.GetTestClient(); + var requestUri = new Uri("/actuator/refresh", UriKind.Relative); + + HttpResponseMessage getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + + HttpResponseMessage postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.OK); + + fileProvider.ReplaceFile(fileName, """ + { + "Management": { + "Endpoints": { + "Actuator": { + "Exposure": { + "Include": ["refresh"] + } + }, + "Refresh": { + "AllowedVerbs": ["GET"] + } + } + } + } + """); + + fileProvider.NotifyChanged(); + + getResponse = await httpClient.GetAsync(requestUri); + getResponse.StatusCode.Should().Be(HttpStatusCode.OK); + + postResponse = await httpClient.PostAsync(requestUri, null); + postResponse.StatusCode.Should().Be(HttpStatusCode.MethodNotAllowed); + } } diff --git a/src/Management/test/Endpoint.Test/Actuators/RouteMappings/EndpointMiddlewareTest.cs b/src/Management/test/Endpoint.Test/Actuators/RouteMappings/EndpointMiddlewareTest.cs index 3f8943140b..8a3c09af42 100644 --- a/src/Management/test/Endpoint.Test/Actuators/RouteMappings/EndpointMiddlewareTest.cs +++ b/src/Management/test/Endpoint.Test/Actuators/RouteMappings/EndpointMiddlewareTest.cs @@ -336,11 +336,11 @@ public async Task RouteMappingsActuator_ConventionalRouting_ReturnsExpectedData( }, { "handler": "CoreRouteHandler", - "predicate": "{[/actuator/mappings],methods=[Get]}" + "predicate": "{[/actuator/mappings],methods=[GET || PUT || POST || DELETE || HEAD || OPTIONS]}" }, { "handler": "CoreRouteHandler", - "predicate": "{[/actuator/refresh],methods=[Post]}" + "predicate": "{[/actuator/refresh],methods=[GET || PUT || POST || DELETE || HEAD || OPTIONS]}" } ] } @@ -351,7 +351,7 @@ public async Task RouteMappingsActuator_ConventionalRouting_ReturnsExpectedData( """); response = await client.GetAsync(new Uri("http://localhost/actuator/refresh")); - Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + Assert.Equal(HttpStatusCode.MethodNotAllowed, response.StatusCode); response = await client.PostAsync(new Uri("http://localhost/actuator/refresh"), null); Assert.Equal(HttpStatusCode.OK, response.StatusCode); @@ -439,8 +439,12 @@ public async Task RouteMappingsActuator_ConventionalRouting_CanTurnOffAllVerbs() }, { "handler": "CoreRouteHandler", - "predicate": "{[/actuator/mappings],methods=[Get]}" - } + "predicate": "{[/actuator/mappings],methods=[GET || PUT || POST || DELETE || HEAD || OPTIONS]}" + }, + { + "handler": "CoreRouteHandler", + "predicate": "{[/actuator/refresh],methods=[GET || PUT || POST || DELETE || HEAD || OPTIONS]}" + } ] } }