ir_rule should apply to the result of a Model.write() in some cases

There is a particular use case worth considering here: rules that restrict writing based on records 'state'.

With this type of rules in place, the user would be unable to change a record from a state with write access to the next state, where he only only have read access.

Hi Daniel,

do you know of any module that actually depends on this bug/feature in the way that you describe? That particular use case is usually implemented by setting groups on a workflow state in combination with the states attribute dictionary on fields or simply using attrs in view definitions.

Cheers,
Stefan.

Hi Stefan,

No standard module uses this method. I think this kind of use case is not addressed in any standard module.
Using attrs in views is necessary when using both methods.( It's a workaround for, IMHO, a missing feature in OpenERP).

But they don't replace the state-based ir_rules that add a more robust security layer and prevent unintended changes through direct access to the ORM (using an XML-RPC call for instance).
AFAIK there is no other way to enforce that.

Focusing on the problem described in the bug report, maybe the intended behaviour can be achieved customizing the create()/write() methods.

Hi Daniel,

if both 'pre' write and 'post' write have their separate purposes, then maybe OpenERP should eventually allow marking the rules for either of them, so having perm_pre_write and perm_post_write flags instead of just perm_write.

For now I think the inconsistency between not allowing the user to create a resource with property A but still allowing to modify an existing resource to the same result is a pretty obvious defect that should be addressed.

I have to agree that there shouldn't be an inconsistency between what you can achieve through a write() and a create().
I think this qualifies as a "security vulnerability".

Hello,

I have to agree with Daniel here, because this behavior is indeed by design, pretty much for the use cases he mentioned.
It is common for rules to restrict write or read access based on the ownership of a record, for example the "Sales - See own leads only" group prevents seeing leads that are not assigned to you.
Even if you can't see your colleagues' leads, you should be able to transfer one of your leads to your colleagues - and the system should allow that.

So yes, there are use cases where this is important, and we should not change this is stable versions.

Now it is true that administrators might want to have more control over this gray area in the future. We have discussed this many times internally, and we always came to the conclusion that adding more permission types (such as perm_pre_write/perm_post_write) would needlessly complexify the already complicated OpenERP ACLs.
An idea that came up was to automatically decide whether an access rules check is necessary at the end of a write() operation based on the other operations that the relevant access rules apply to.
As you said, it is not quite consistent that a rule will prevent you to create a certain record, but will allow you to modify it to reach the same state. Hence we could say that a `write-restricting` rule would be checked *also* at the end of the write() operation if and only if it applies to `create` as well.
Or to `read`, I'm not sure. Indeed it's also surprising that sometimes you cannot read what you can create, but it might still make sense for a certain kind of automated operations: in 7.0 a contact form system in the portal will allow anonymous users to post contact requests (i.e. leads), but they must not be allowed to read any request over RPC, even the ones they've just created (it's like a black box).

So probably having a post-write check for rules that also restrict 'write' is the most logical behavior.
What do you think?

Thanks for raising attention to this gray area of the OpenERP security model!

PS: I'll set the bug to Confirmed/Low, think we can still target it to 7.0. I don't think we can allow this behavior to change in 6.1.
PPS: Please don't set unconfirmed bugs to Fix Committed - even though it looks like the most appropriate status when a merge proposal is awaiting review, it might confuse others into thinking the bug is confirmed and the solution was decided upon.

Typo: in the previous comment, "for rules that also restrict 'write'" was supposed to be "for rules that also restrict 'create'", sorry.

I'm also adding the Security flag to this bug, as it's clearly security related.

Hi Olivier,

thanks for clarifying. I will have a look at a solution in the suggested direction.

Cheers,
Stefan.