RESTier icon indicating copy to clipboard operation
RESTier copied to clipboard

[2.0] Apply Query Filters to ALL entities, even single Navigations

Open randbrown opened this issue 8 years ago • 5 comments

E.g. I have User and UserRole. I have a method called protected IQueryable<User> OnFilterUser(IQueryable<User> entitySet) and that is correctly called when hitting the /Users endpoint. However, when I hit the UserRoles?$expand=User, the User filtering does not kick in. This means one user could craft an expand url from a related table and retrieve another user's data.

Assemblies affected

RESTier 1.0.0-beta

Reproduce steps

Followed instructions per section 2.2

Expected result

Expected that the UserRole.User expanded property would be filtered according to the User filter.

Actual result

The UserRole.User expanded property is not filtered.

randbrown avatar Sep 18 '16 03:09 randbrown

I checked this and verified again, it works well.

You can refer to test case https://github.com/OData/RESTier/blob/master/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Tests/TrippinE2EOnFilterTestCases.cs#L25-L28

And OnFilter in https://github.com/OData/RESTier/blob/master/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Api/TrippinApi.cs#L112-L115

Note OnFilter is filter a IQueryable, not a single user. which means the method should be OnFilterUser(IQueryable<User> u)

If you still have issues, we will need your sample service to recreate as I cannot recreate from our side, our end to end test cases work well.

chinadragon0515 avatar Sep 18 '16 04:09 chinadragon0515

I don't see a test case for my scenario. In all those cases, the expand property contains a collection. In my case, User->UserRoles is 1-to-many. Therefore each UserRole has only a single User property, instead of a collection. My scenario would be similar to the Northwind database Order->Customer relationship. (but I did not find any test cases like this)

*corrected method signature in original message, thanks for pointing that out.

randbrown avatar Sep 18 '16 14:09 randbrown

OK, I see the case you have, filter on a single navigation property but not collection of navigation property, this is not support yet. We only add filter to collection which can be converted as IQueryable.

The url like this is not filter neither ~/UserRoles(key)/User

For single property, we have not worked on it as we need to convert to IQueryable, add filter, then convert back to single value. We do not have a solid plan on this part.

Welcome contribution on this.

chinadragon0515 avatar Sep 19 '16 01:09 chinadragon0515

So here is idea... Instead of converting single property to IQueryable we can build condition expression with test expression extracted from IQueryable we get from OnFilter. This expression(condition) will return visitedNode if test expression resolves to true(ifTrue) or Constant expression with value null of type from visitedNode (ifFalse).

Edit:

My attempt: link to my ConventionBasedQueryExpressionProcessor I used LinqKit to "reduce" invocation expression. If anyone has idea how to build test expression without Invoke (EF does not like this NodeType) - let me know.

mateusz-malicki avatar Apr 20 '17 13:04 mateusz-malicki

Silently ignoring a filter could be a critical security/data corruption issue.

mikepizzo avatar Jun 10 '17 02:06 mikepizzo