[FIX] base_tier_validation: allow sudo writes

[yajo]

Fix #875 by always allowing superuser to write.

@moduon MT-5997

[OCA-git-bot]

Hi @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

[LoisRForgeFlow]

Hi @yajo

What is the process that needs to bypass the validation?

The problem I see with this change is that it is a bit to wide of a fix. For instance, it will allow procurements to extend POs under validation even when the procurement comes from a regular user. See https://github.com/odoo/odoo/blob/9fd48fcaa04e884871789abb7f19dacef49a2a8c/addons/purchase_stock/models/stock_rule.py#L139

[yajo]

What is the process that needs to bypass the validation?

You can see the full report in #875. The process is the standard notifications creation cron, along with the need to auto-generate a portal token for some records. It's an Odoo standard process (you might see other modules in the traceback, but they don't affect this issue).

For instance, it will allow procurements to extend POs under validation even when the procurement comes from a regular user. See https://github.com/odoo/odoo/blob/9fd48fcaa04e884871789abb7f19dacef49a2a8c/addons/purchase_stock/models/stock_rule.py#L139

AFAICS, the purchase_tier_validation module validates only purchase.order, not purchase.order.line, so IIUC this problem you say wouldn't happen.

Do you see other cases where this would be dangerous?

FWIW, reading this module code, it seems to me like it could be implemented as an extension to the rules system, where write permissions are extended by the validation state of a record. A sensible refactor would be to override check_access_rule() instead of overriding write(). And just like the rules system, IMHO it makes sense that sudo() overpasses them.

[fcvalgar]

I'm testing in an EE client environment with the following modules installed and have not seen any problems with any of the flows used by these modules.

[LoisRForgeFlow]

@yajo

You can see the full report in #875.

Thanks, it was in the description, too quick reading... 🤦‍♂️

AFAICS, the purchase_tier_validation module validates only purchase.order, not purchase.order.line, so IIUC this problem you say wouldn't happen.

EDIT: I did a wrong test, it appear to go throught, which was unexpected for me. I'll review.

EDIT2: So I tested v15 and the issue also happens, a procurement can extend a PO under validation, this was for sure not the intended behavior. So we have one more issue to fix...

In any case, from my point of view since there are sudos in several places in odoo core that you cannot control, I'm not 100% sure that the patch you propose is the path to go.

[yajo]

Yeah, it's a tricky case.

I could follow this other approach from #875 (comment):

1. Add portal to the dependencies of base_tier_validation
2. Override _portal_ensure_token().
3. Call super with_context(skip_validation_check=True)

However I feel like it's the wrong approach.

I mean: by default, when Odoo does some action with sudo, it usually means that it should happen always, no matter if you can or not write to the record. Thus, there are only a few sudos around, like the case at hand.

So I think it makes sense to assume that the tier validation modules should cover those cases explicitly if needed. I pushed a change to avoid skipping validations when skip_validation_check context is explicitly set to False.

But you're the maintainer. If you prefer the other solution, just tell me and I'll adapt the PR. It will involve depending on portal, though.