dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.47k stars 10.03k forks source link

Issue with replacing IObjectModelValidator #30938

Closed amoorthaemer closed 3 years ago

amoorthaemer commented 3 years ago

Hi,

I followed your example on disabling validation (see: https://docs.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-5.0) and noticed some strange side-behavior, e.g. model validation on classes was indeed disabled, but validation on action (method) parameters (f.e. passed by query) resulted in a bad request (HTTP 400)

In short: a POST with a model in the body functioned as documented, but a GET with query parameters didn't.

the example code tested:

public class NullObjectModelValidator : IObjectModelValidator
{
    public void Validate(ActionContext actionContext,
        ValidationStateDictionary validationState, string prefix, object model)
    {

    }
}

... and ...

services.AddSingleton<IObjectModelValidator, NullObjectModelValidator>();

Is this by design or indeed a bug?

TIA

Bert

pranavkm commented 3 years ago

@amoorthaemer we weren't aware that particular feature was documented. We're going to have a look at that. In the meanwhile, could you elaborate on why you need to turn off all validation in MVC? It opens up your app to security vulnerabilities.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

amoorthaemer commented 3 years ago

@pranavkm well I'm not opening model validation. Just wanted to move it to a different handler. I stumbled upon this example and wanted to use it to accomplish my goals. The thing is that when I'm using validation attributes ASPNET Core returns a 400 but the response isn't formatted according RFC 7807. Because I have modelled all other 400's using ProblemDetails as the response it becomes very "difficult" to parse the response in a generic way., Maybe there is another solution, but I didn't find it. Maybe you can elaborate on this?

TIA

/Cheers Bert

amoorthaemer commented 3 years ago

@pranavkm Sorry, my bad. I had a new InvalidModelStateResponseFactory and this returned the wrong model. But, still the IObjectModelValidator shouldn't behave as it does now. Besides that, you can close this.

Thanks

Bert

MaceWindu commented 3 years ago

@amoorthaemer , example in documentation is not correct, as it leaves model in unvalidated state, which doesn't work at least with [FromBody] and [FromHeaders] parameters.

Validate body should look like that:

void IObjectModelValidator.Validate(ActionContext actionContext, ValidationStateDictionary validationState, string prefix, object model)
{
    foreach (var entry in actionContext.ModelState.Values)
    {
        // or ModelValidationState.Valid
        entry.ValidationState = ModelValidationState.Skipped;
    }
}
pranavkm commented 3 years ago

Thanks, I think @MaceWindu's response should address this. I've removed the section from our doc article, it's not something we would recommend for most users, so I'll consider this issue resolved.