hivesolutions / appier

Joyful Python Web App development
http://appier.hive.pt
Apache License 2.0
126 stars 22 forks source link

feat: parse eager string #57

Closed joao-conde closed 1 year ago

joao-conde commented 1 year ago

Parse the eager string query param in find types as well, to enable eager param usage.

@joamag

coveralls commented 1 year ago

Coverage Status

Coverage: 64.624% (-0.02%) from 64.639% when pulling 03871feb5a90ff2cd6a6264b7cc9746059d21c48 on joao-conde:jc/appier-object-eager into ea0f7955d55ad019815556f507efd7139c3280f5 on hivesolutions:master.

coveralls commented 1 year ago

Coverage Status

Coverage: 64.624% (-0.02%) from 64.639% when pulling 03871feb5a90ff2cd6a6264b7cc9746059d21c48 on joao-conde:jc/appier-object-eager into ea0f7955d55ad019815556f507efd7139c3280f5 on hivesolutions:master.

joamag commented 1 year ago

This can create security issues where by just using the eager param one can access information outside of the action method (controller method) scope. I belive we need to discuss this in a more profound discussion....

joamag commented 1 year ago

I believe the secret to overcoming this security issue is to create a whitelist of eager references that can be accessed at the controller method level.

joamag commented 1 year ago

@joao-conde give me some time to come up with the specifics of the solution. And I'll merge this one ;)

joao-conde commented 1 year ago

@joamag following the python zen we could also "treat as adults" (or something similar) users of appier, and instead of a whitelist approach, follow a black list one, meaning by default I could call eager on every field.

joamag commented 1 year ago

@joamag following the python zen we could also "treat as adults" (or something similar) users of appier, and instead of a whitelist approach, follow a black list one, meaning by default I could call eager on every field.

The whole whitelist thing is more related to things like: If we add a new reference field in an Appier Model we would have to explicitly blacklist it in every single action method that does not allow access to the reference. As opposed to whitelisting where an explicit grant has been made so you don't need to update any action method that does not require access to it. It seems to me to be more logical and probably avoids security issues coming from mistakes.

joao-conde commented 1 year ago

Makes sense. Some of the appier model fields are already using for example eager=True, would this be enough to mark them as "safe" to be eagerly retrieved?

joamag commented 1 year ago

Makes sense. Some of the appier model fields are already using for example eager=True, would this be enough to mark them as "safe" to be eagerly retrieved?

Doing that will retrieve those references by default when doing a get() or find(), you don't need to do anything else they will always be eagerly retrieved.

you can check more info here https://appier.hive.pt/doc/models.md#models