[FIX] web: filter cached inaccessible x2many records in web_read

[fw-bot]

Description of the issue/feature this PR addresses:
web_read on x2many fields can reuse cached ids after write/web_save. Some of these cached ids may be inaccessible with the current record rules/context (cache pollution).

Example:

• Context:

  • Two companies exist: Company A and Company B.
  • Two users exist: User A and User B.
  • User A can only access Company A (company_ids=[A], company_id=A).
  • User B is linked to both companies (company_ids=[A, B], company_id=A).
  • The "res.company" record rule is the standard one: [('id', 'in', company_ids)] (company_ids comes from allowed_company_ids).
  • User A edits User B and saves the form.

• Steps:

  • User A performs a web_read to load User B: company_ids contains only Company A.
  • User A performs web_save (write + internal web_read in the same request): cached ids [A, B] are reused and the code attempts to read Company B.

Current behavior before PR (without fix):
After saving a form with an x2many field, web_save calls write and then web_read. In this flow, web_read can include inaccessible x2many ids from cache and raise an AccessError.

Desired behavior after PR is merged:
x2many records are re-filtered with current read rules before formatting, and inaccessible ids are removed from values_list.

Forward-Port-Of: #250904

[robodoo]

[fw-bot]

This PR targets 18.0 and is part of the forward-port chain. Further PRs will be created up to master.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

[fw-bot]

@cro-odoo @rco-odoo ci/runbot failed on this forward-port PR

[rco-odoo]

@cro-odoo the failing tests may be because this call to search() crashes. I suggest to wrap it into a try/except, so that some AccessError turns co_records into an empty recordset. That's the behavior we now have in master.

[fw-bot]

@cro-odoo @rco-odoo this PR has become a normal PR because head updated from d06ddc4f9201b0b159d12c137f0e595e96db5334 to 6f2933446e657f42e6df3b5f344542c6003b4f8e. It must be merged to be forward-ported further.

[fw-bot]

@cro-odoo @rco-odoo this forward port of #250904 is awaiting action (not merged or closed).

[cro-odoo]

hello @rco-odoo
Due to incompatibility of my patch with the model account.code.mapping, I use the idea of @kmagusiak (#250904 (comment)) ==> using _filtered_access(read) and apply your suggestion of "try/except"

[cro-odoo]

Hello future reviewer,

I adjusted the previous patch "a bit more" after investigating the failed tests.

The original bug is still cache pollution: "web_read" may reuse inaccessible x2many ids and should not expose or crash on them.

Here is the new logic to remove polluted x2many values:

1. the order case: search(..., order=...) to get an ordered and "filtered" recordset. (could be replaced by .sorted from saas-18.4 ==> [FIX] web: filter cached inaccessible x2many records in web_read #250904 (comment))
2. the fields info/value case: filtering polluted x2many values, not ordering ==> we can't do a search([('id', 'in', ...)]) because sometimes is not allowed (e.g. account.code.mapping in which search() method raises an UserError).

So to filter, we use _filtered_access('read') (to be changed to clear_access_cache in 19.1 #250904 (comment)) but using it without checking if the user has access to the model introduces a regression in hr_appraisal app (see TestHrAppraisalRequestUi.test_send_appraisal_request_by_email_flow)

• manager_ids is a x2many field to hr.employee and the UI tour user is a base.group_user user (not hr user) ==> that user does not have a read access on hr.employee model, so _filtered_access('read') removes all managers from values. The field is required, so the form can't be saved and the modal action no longer opens.

So my idea is to only apply _filtered_access('read') in the fields section when the user has general read access on the comodel, otherwise co-records keep the relation ids unchanged.

This is a bigger patch than the first version, so I would like your feedback @rco-odoo or @kmagusiak on this PR: Is still this look ok for you ? Since you have a more global view on the ORM methods / behaviors.

[kmagusiak]

Before deciding, let's see future issues.
@robodoo fw=skipci

If that changes the behaviour too much, I would opt to revert the change in 17.0 and abandon that change as too risky.
If all is fine, this should be backported to 17.0.
Maybe some nitpicks about the order of if statements.

[robodoo]

Not waiting for CI to create followup forward-ports.