feat(server): API query filter

[squakez]

• Extended the filtering feature to work with all data types, not only with String
• Fixed an issue that was not allowing to use Object super type
• Added test cases for ReflectionUtils
• Added test cases for ReflectiveFilterer

Fixes #8177 and closes ENTESB-13247

In order to ease the review, the idea is to be able to call an API by providing a query filter such as api/v1/extensions?query=extensionType=Libraries,status=Installed - this one, specifically was not working before this PR as the code only worked for String fields.

[syndesisio-bot]

@squakez The bot could not transition the ticket automatically, please update this Jira ticket manually: https://issues.redhat.com/browse/ENTESB-13247

[zregvart]

Several issues with this:

• feature creep: this is a generic solution to non-generic problem, if use case requires filtering implement a filter with the logic that's required, don't make a generic solution that's going to be used for 1% of use cases
• uses more reflection: we need to minimize our use of reflection not expand it
• increases techdept: we don't want to extend our use of JSONDB we wish to minimise it as much as possible
• security issue: we need to be really careful about what methods get invoked and how we pass the data from the user, this could lead to arbitrary code execution, such as the one seen in CVE-2010-1622

I would also appreciate if you didn't create ENTESB issues for technical work, you're removing community from the discussion in doing so.

[squakez]

Thanks for you review @zregvart.

Several issues with this:

* feature creep: this is a generic solution to non-generic problem, if use case requires filtering implement a filter with the logic that's required, don't make a generic solution that's going to be used for 1% of use cases

Just consider that I am not introducing anything new but leveraging an existing feature already provided by Syndesis. You can already provide the query parameter to all handlers implementing a Lister (see here). I fixed (or at least, tried to) something broken and was aiming to use it.

* uses more reflection: we need to minimize our use of reflection not expand it

Not adding anything on that front, just fixing something as recently done for example with a sorting issue (see history)

* increases techdept: we don't want to extend our use of JSONDB we wish to minimise it as much as possible

Reusing what's already there, no new usage of JSONDB.

* security issue: we need to be really careful about what methods get invoked and how we pass the data from the user, this could lead to arbitrary code execution, such as the one seen in CVE-2010-1622

If there is some possible exploit we should log a separate issue and work to fix it.

I would also appreciate if you didn't create ENTESB issues for technical work, you're removing community from the discussion in doing so.

Sure, I should have created and linked one. Just logged here #8177

[KurtStam]

@squakez can you give a bit of context around ENTESB-13247? It does not seem to be a Product requirement as you opened the issue yourself.

[zregvart]

@squakez your definition of broken is another persons definition of intentional limitation aimed at not allowing every possible scenario as that leads to more issues than solutions. This will introduce a security issue I suggest that you study the mentioned CVE. If you need certain type of filtering add it to the API that requires it. I will argue this no longer, do with the information I've given you as you wish.

[squakez]

@squakez can you give a bit of context around ENTESB-13247? It does not seem to be a Product requirement as you opened the issue yourself.

Thanks @KurtStam, that's a subtask of ENTESB-11534. I'd like to introduce a filter in order to ease the labour of UI and I thought leveraging existing Syndesis feature was better that reinventing the wheel.

[KurtStam]

Thanks @squakez I appreciate that you're trying to add to existing code but the issue is that this existing code is marked as tech debt. So adding to it problematic as @zregvart mentions. Maybe you can give it some thought to see if there is a way not to touch this?

[squakez]

Thanks @KurtStam. Absolutely, I've requested a review for that reason. We should mark as @Deprecated all those classes that are considered tech debt or that need a design review, otherwise any new developer can get confused on what's really worth dedicate effort to. Also, I've seen that those classes have been recently updated to fix other issues.

@zregvart , I do understand your security concerns, there's no reason to assume that tone. I'm open to improve based on others experience and suggestions. In this case I think there is no risk of arbitrary execution, I've made some test, but I understand you're not open to exchange opinions.

I will make an ad-hoc development or leave the UI to read the whole list of extensions as it's already doing.