From 06a9da424f7fc03f3beb4b5bf4611a75db05b44d Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 5 Jul 2022 14:35:27 -0700 Subject: [PATCH 1/2] Remove Auth validation when Form --- .../src/RequestDelegateFactory.cs | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b508e90b72cc..5c1213d0fbb1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1239,8 +1239,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, return (null, false); } - ThrowIfRequestIsAuthenticated(httpContext); - try { formValue = await httpContext.Request.ReadFormAsync(); @@ -1260,33 +1258,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, return (formValue, true); } - - static void ThrowIfRequestIsAuthenticated(HttpContext httpContext) - { - if (httpContext.Connection.ClientCertificate is not null) - { - throw new BadHttpRequestException( - "Support for binding parameters from an HTTP request's form is not currently supported " + - "if the request is associated with a client certificate. Use of an HTTP request form is " + - "not currently secure for HTTP requests in scenarios which require authentication."); - } - - if (!StringValues.IsNullOrEmpty(httpContext.Request.Headers.Authorization)) - { - throw new BadHttpRequestException( - "Support for binding parameters from an HTTP request's form is not currently supported " + - "if the request contains an \"Authorization\" HTTP request header. Use of an HTTP request form is " + - "not currently secure for HTTP requests in scenarios which require authentication."); - } - - if (!StringValues.IsNullOrEmpty(httpContext.Request.Headers.Cookie)) - { - throw new BadHttpRequestException( - "Support for binding parameters from an HTTP request's form is not currently supported " + - "if the request contains a \"Cookie\" HTTP request header. Use of an HTTP request form is " + - "not currently secure for HTTP requests in scenarios which require authentication."); - } - } } private static Expression GetValueFromProperty(MemberExpression sourceExpression, PropertyInfo itemProperty, string key, Type? returnType = null) From 06d743ea60f42940fc899d50d375684b43fbe64e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 5 Jul 2022 15:03:00 -0700 Subject: [PATCH 2/2] Updating unit tests --- .../test/RequestDelegateFactoryTests.cs | 63 ++++++++----------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 6a2b7c4bfb1c..c25a8ec7de91 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -4479,18 +4479,19 @@ void TestAction(IFormFileCollection formFiles, IFormFile file) } [Theory] - [InlineData("Authorization", "bearer my-token", "Support for binding parameters from an HTTP request's form is not currently supported if the request contains an \"Authorization\" HTTP request header. Use of an HTTP request form is not currently secure for HTTP requests in scenarios which require authentication.")] - [InlineData("Cookie", ".AspNetCore.Auth=abc123", "Support for binding parameters from an HTTP request's form is not currently supported if the request contains a \"Cookie\" HTTP request header. Use of an HTTP request form is not currently secure for HTTP requests in scenarios which require authentication.")] - public async Task RequestDelegateThrowsIfRequestUsingFormContainsSecureHeader( + [InlineData("Authorization", "bearer my-token")] + [InlineData("Cookie", ".AspNetCore.Auth=abc123")] + public async Task RequestDelegatePopulatesFromIFormFileParameterIfRequestContainsSecureHeader( string headerName, - string headerValue, - string expectedMessage) + string headerValue) { - var invoked = false; + IFormFile? fileArgument = null; + TraceIdentifier traceIdArgument = default; - void TestAction(IFormFile file) + void TestAction(IFormFile? file, TraceIdentifier traceId) { - invoked = true; + fileArgument = file; + traceIdArgument = traceId; } var fileContent = new StringContent("hello", Encoding.UTF8, "application/octet-stream"); @@ -4507,34 +4508,30 @@ void TestAction(IFormFile file) httpContext.Request.Headers[headerName] = headerValue; httpContext.Request.Headers["Content-Type"] = "multipart/form-data;boundary=some-boundary"; httpContext.Features.Set(new RequestBodyDetectionFeature(true)); + httpContext.TraceIdentifier = "my-trace-id"; var factoryResult = RequestDelegateFactory.Create(TestAction); var requestDelegate = factoryResult.RequestDelegate; - var badHttpRequestException = await Assert.ThrowsAsync(() => requestDelegate(httpContext)); - - Assert.False(invoked); - - // The httpContext should be untouched. - Assert.False(httpContext.RequestAborted.IsCancellationRequested); - Assert.Equal(200, httpContext.Response.StatusCode); - Assert.False(httpContext.Response.HasStarted); + await requestDelegate(httpContext); - // We don't log bad requests when we throw. - Assert.Empty(TestSink.Writes); + Assert.Equal(httpContext.Request.Form.Files["file"], fileArgument); + Assert.Equal("file.txt", fileArgument!.FileName); + Assert.Equal("file", fileArgument.Name); - Assert.Equal(expectedMessage, badHttpRequestException.Message); - Assert.Equal(400, badHttpRequestException.StatusCode); + Assert.Equal("my-trace-id", traceIdArgument.Id); } [Fact] - public async Task RequestDelegateThrowsIfRequestUsingFormHasClientCertificate() + public async Task RequestDelegatePopulatesFromIFormFileParameterIfRequestHasClientCertificate() { - var invoked = false; + IFormFile? fileArgument = null; + TraceIdentifier traceIdArgument = default; - void TestAction(IFormFile file) + void TestAction(IFormFile? file, TraceIdentifier traceId) { - invoked = true; + fileArgument = file; + traceIdArgument = traceId; } var fileContent = new StringContent("hello", Encoding.UTF8, "application/octet-stream"); @@ -4550,6 +4547,7 @@ void TestAction(IFormFile file) httpContext.Request.Body = stream; httpContext.Request.Headers["Content-Type"] = "multipart/form-data;boundary=some-boundary"; httpContext.Features.Set(new RequestBodyDetectionFeature(true)); + httpContext.TraceIdentifier = "my-trace-id"; #pragma warning disable SYSLIB0026 // Type or member is obsolete var clientCertificate = new X509Certificate2(); @@ -4560,20 +4558,13 @@ void TestAction(IFormFile file) var factoryResult = RequestDelegateFactory.Create(TestAction); var requestDelegate = factoryResult.RequestDelegate; - var badHttpRequestException = await Assert.ThrowsAsync(() => requestDelegate(httpContext)); - - Assert.False(invoked); - - // The httpContext should be untouched. - Assert.False(httpContext.RequestAborted.IsCancellationRequested); - Assert.Equal(200, httpContext.Response.StatusCode); - Assert.False(httpContext.Response.HasStarted); + await requestDelegate(httpContext); - // We don't log bad requests when we throw. - Assert.Empty(TestSink.Writes); + Assert.Equal(httpContext.Request.Form.Files["file"], fileArgument); + Assert.Equal("file.txt", fileArgument!.FileName); + Assert.Equal("file", fileArgument.Name); - Assert.Equal("Support for binding parameters from an HTTP request's form is not currently supported if the request is associated with a client certificate. Use of an HTTP request form is not currently secure for HTTP requests in scenarios which require authentication.", badHttpRequestException.Message); - Assert.Equal(400, badHttpRequestException.StatusCode); + Assert.Equal("my-trace-id", traceIdArgument.Id); } private record struct ParameterListRecordStruct(HttpContext HttpContext, [FromRoute] int Value);