[16.0] shopinvader_api_sale: fix access #1494
Conversation
ATM You are forced to add a record rule for _every_ single record involved in the call. Every. Including nested relations. Making that work might be very complicated, especially when those records have nothing to deal w/ partners, meaning that it would be hard to discriminate if the current partner should be allowed to see it. For instance, the product.pricelist. This model has no default record rule, hence the /sales api is broken OOTB and unfortunately you cannot control the pricelist in the schema because is used down the stack to compute prices. Of course this might be the best solution but at least it makes the api work OOTB. In any case, for now this change is only temporary fix for me and a way to raise the problem. As I understand ppl might want to still rely fully on RR, probably we should make this configurable somehow. Maybe via config param? Not sure what's the best w/ the new implementation since there's no s.backend anymore.
| @@ -60,6 +60,7 @@ def get( | |||
| env["shopinvader_api_sale.sales_router.helper"] | |||
| .new({"partner": partner}) | |||
| ._get(sale_id) | |||
There was a problem hiding this comment.
Side note, this way of using the abstract model sounds a bit strange to me. I'd prefer pure @api.model methods that accept the partner as argument. @sebastienbeau
There was a problem hiding this comment.
The aim of passing the partner like that, was to be able to have the "self.partner" without needing of add on every method "partner" as first params.
But any better idea are welcome
| @@ -37,7 +37,7 @@ def _find_open_cart(self, partner_id=None, uuid=None): | |||
| # maybe a current cart exists with another uuid | |||
| domain = self._get_open_cart_domain(partner_id, uuid=None) | |||
| cart = self.search(domain, limit=1) | |||
| return cart | |||
| return cart.sudo() | |||
There was a problem hiding this comment.
I prefer to avoid returning sudo'ed models (and sudo locally where it's needed) to avoid passing around sudoed models that can cause unexpected security holes by navigation through the relations.
| @@ -37,7 +37,7 @@ def _find_open_cart(self, partner_id=None, uuid=None): | |||
| # maybe a current cart exists with another uuid | |||
| domain = self._get_open_cart_domain(partner_id, uuid=None) | |||
| cart = self.search(domain, limit=1) | |||
| return cart | |||
| return cart.sudo() | |||
There was a problem hiding this comment.
I would prefer to call sudo when required than always returning the SO in sudo mode by default.
ATM You are forced to add a record rule for every single record involved in the call. Every. Including nested relations. Making that work might be very complicated, especially when those records have nothing to deal w/ partners, meaning that it would be hard to discriminate if the current partner should be allowed to see it.
For instance, the product.pricelist. This model has no default record rule, hence the /sales api is broken OOTB and unfortunately you cannot control the pricelist in the schema because is used down the stack to compute prices.
Of course this might be the best solution but at least it makes the api work OOTB.
In any case, for now this change is only temporary fix for me and a way to raise the problem. As I understand ppl might want to still rely fully on RR, probably we should make this configurable somehow. Maybe via config param? Not sure what's the best w/ the new implementation since there's no s.backend anymore.