Skip to content

Remove Auth validation when Form File upload #42586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1239,8 +1239,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
return (null, false);
}

ThrowIfRequestIsAuthenticated(httpContext);

try
{
formValue = await httpContext.Request.ReadFormAsync();
Expand All @@ -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)
Expand Down
63 changes: 27 additions & 36 deletions src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
httpContext.TraceIdentifier = "my-trace-id";

var factoryResult = RequestDelegateFactory.Create(TestAction);
var requestDelegate = factoryResult.RequestDelegate;

var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => 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");
Expand All @@ -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<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
httpContext.TraceIdentifier = "my-trace-id";

#pragma warning disable SYSLIB0026 // Type or member is obsolete
var clientCertificate = new X509Certificate2();
Expand All @@ -4560,20 +4558,13 @@ void TestAction(IFormFile file)
var factoryResult = RequestDelegateFactory.Create(TestAction);
var requestDelegate = factoryResult.RequestDelegate;

var badHttpRequestException = await Assert.ThrowsAsync<BadHttpRequestException>(() => 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);
Expand Down