Skip to content
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

Missing documentation for FilterQueryValidator #1651

Open
mloffer opened this issue Oct 19, 2018 · 7 comments
Open

Missing documentation for FilterQueryValidator #1651

mloffer opened this issue Oct 19, 2018 · 7 comments

Comments

@mloffer
Copy link

mloffer commented Oct 19, 2018

WebAPI 2.2 documentation lists very little:
https://docs.microsoft.com/en-us/aspnet/core/web-api/?view=aspnetcore-2.2

OData documentation is out of date:
https://docs.microsoft.com/en-us/aspnet/web-api/overview/odata-support-in-aspnet-web-api/odata-security-guidance
A) FilterQueryValidator requires DefaultQuerySettings to be passed to the base. Where should this come from?
B) This article doesn't show how to use MyFilterQueryValidtor.

I have looked through the samples, but there are no relevant projects:
https://github.com/OData/ODataSamples/tree/master/WebApiCore

Questions:
How do I create one?
How should DefaultQuerySettings be passed in?
How do I use it to validate against a controller action?

Sorry to be so basic, but the documentation for this stack is nearly non-existent. It's hard to even know what to ask.

@KanishManuja-MS
Copy link
Contributor

@mloffer Thanks for the feedback. We are working on making the documentation better. For the time being, for aspnetcore, you can use this in your startup.cs to set the DefaultQuerySetting with the parameters you want -
app.UseMvc(b =>
{
b.SetDefaultQuerySettings(new Microsoft.AspNet.OData.Query.DefaultQuerySettings());
});

If you are using aspnet, then you can use HttpConfiguration extension methods to do the same.

@mloffer
Copy link
Author

mloffer commented Nov 1, 2018

I don't understand what explicitly setting DefaultQuerySettings does here? Isn't this done automatically behind the scenes? What does this have to do with writing a FilterQueryValidator?

@KanishManuja-MS
Copy link
Contributor

@mloffer DefaultQuerySetting in the context of FilterQueryValidator simply checks if filter is enabled as a query option or not. It was not clear to me how are you planning on using the FilterQueryValidator; if you want to use your own then simply new a DefaultQuerySetting object with Enable filter property set to true.

I believe that the framework should take care of this automatically but it seems like you are trying to achieve something else here. Can you please give me more context about what you are trying to do?

@mloffer
Copy link
Author

mloffer commented Nov 2, 2018

This is what I was looking to write. It's fairly similar to the way the AspNet guide says to do it. I've provided a new instance of DefaultQuerySetting like you suggested, but I am unclear on what the implications of this are. Am I at risk of having this attribute ignore global query settings I'm configuring elsewhere in the stack?

Also, is EnableQueryAttribute the right thing to use here? It's just QueryableAttribute in the AspNet guide, but this class doesn't exist in the AspNetCore implementation.

You can see that I would like to throw an error if the ODataQueryOptions.Filter is null, but the ValidateQuery method never gets called if some part of the query isn't provided by the consumer. For instance, I can provide $count=true and I get the error. But simply accessing the route does not enforce this behavior.

namespace AspNetCoreODataFilterQueryValidator
{
    public class FilterTitleAttribute: EnableQueryAttribute
    {
        public override void ValidateQuery(HttpRequest request, ODataQueryOptions queryOptions)
        {
            if(queryOptions.Filter == null)
            {
                throw new ODataException("You must filter on Title to use this route.");
            }

            var defaultQuerySettings = new DefaultQuerySettings { EnableFilter = true };
            queryOptions.Filter.Validator = new MyFilterQueryValidator(defaultQuerySettings);

            base.ValidateQuery(request, queryOptions);
        }
    }

    public class MyFilterQueryValidator : FilterQueryValidator
    {
        public MyFilterQueryValidator(DefaultQuerySettings defaultQuerySettings) : base(defaultQuerySettings) { }

        public override void Validate(FilterQueryOption filterQueryOption, ODataValidationSettings settings)
        {
            var count = filterQueryOption.CountMatches("Title");
            if(count != 1)
            {
                throw new ODataException("You must filter exactly one Title to use this route.");
            }
            base.Validate(filterQueryOption, settings);
        }
    }
}

@KanishManuja-MS
Copy link
Contributor

@mloffer Thanks for the details.
I believe I now understand what you are trying to do.
ODataQueryOption.Filter.Validate only gets fired when there is a $filter query option in the request. By design, FilterQueryValidator does not support checking for absence of the filter rather it checks if the format is correct. With that said, there are a number of ways in which you can accomplish what you want -

  1. Use dependency injection - override ODataQueryValidator.Validate and specify your implementation in configure method of startup.cs.
  2. Throw an exception if queryOptions.Filter is null in the controller method itself. If it is not null, then your validation code will fire.

Let me know if you need more help, I can pull in other experts as well.

Thanks!

@mloffer
Copy link
Author

mloffer commented Nov 2, 2018

That makes sense that ODataQueryOption.Filter.Validate wouldn't fire unless a filter were provided.

My question is why EnableQueryAttribute.ValidateQuery doesn't fire unless some query option is passed, such as $count? After all, it's not called EnableQueryOptionsAttribute. Is there a better attribute to inherit from for this task?

I mean, sure, I could put the null check in every controller action. But that violates DRY principles and makes the code very hard to refactor if that requirement ever changes.

@ashygk
Copy link

ashygk commented Oct 21, 2020

Hi,

I am having issues with invoking the ValidateSingleValuePropertyAccessNode in my FilterQueryValidator. I would like to set specified properties to be used in the filter query and throw validation error if any other properties are specified in the filter query. Anyone please assist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants