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

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

Open
randbrown opened this issue Sep 18, 2016 · 5 comments
Open
Labels
Milestone

Comments

@randbrown
Copy link

randbrown commented Sep 18, 2016

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.

@chinadragon0515
Copy link
Contributor

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 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.

@randbrown
Copy link
Author

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.

@chinadragon0515
Copy link
Contributor

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 chinadragon0515 changed the title Filter method not getting called when target is an expand navigation property Filter method not getting called when target is an expand single navigation property Sep 19, 2016
@mateusz-malicki
Copy link
Contributor

mateusz-malicki commented Apr 20, 2017

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.

@mikepizzo mikepizzo added the P2 label Jun 10, 2017
@mikepizzo
Copy link
Member

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

@robertmclaws robertmclaws added this to the 2.0 milestone Dec 5, 2023
@robertmclaws robertmclaws changed the title Filter method not getting called when target is an expand single navigation property [2.0] Apply Query Filters to ALL entities, even single Navigations Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

7 participants