[trunk/7.0] multicompany: error message when creating a stock.move

Bug #1182111 reported by Alexandre Fayolle - camptocamp
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Medium
OpenERP R&D Addons Team 2
OpenERP Community Backports (Addons)
Status tracked in 7.0
7.0
Fix Released
Undecided
Alexandre Fayolle - camptocamp

Bug Description

Steps to reproduce:

* log on a 7.0 instance on runbot as admin
* create a new user with login "test_warehouse" belonging to a child company of "you company", make him warehouse manager, let him access only that company (don't forget to change his password)
* log out, log on as test_warehouse
* go the Warehouse > internal pickings and click on create
* add a new item

What I get:

* an unexpected error message box, which I can remove (by clicking "OK") but which is *very* annoying when it shows up 50x each day

See screenshot.

This error is caused by trying to get the name of a stock location on which the user has no access.

Related branches

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :
Revision history for this message
Twinkle Christian(OpenERP) (tch-openerp) wrote :
Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Medium
status: New → Confirmed
summary: - 7.0 multicompany: error message when creating a stock.move
+ [trunk/7.0] multicompany: error message when creating a stock.move
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

Further investigation shows that the methods _default_location_destination and _default_location_source are at fault : these return a company independent location found with a xml_id and this location is related to base.main_company on which my user has no read access.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

in #3 what I mean by "company independent" is that the location is hard coded, and does not take the user into account.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

3rd method with same bug: on_change_move_type (also in stock.move)

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

probably same bug waiting in mrp/mrp.py

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

When resolving "default values" that could be company-dependent, the logic should be to perform an extra search() on any resolved ID to make sure the user can access the record. For 7.0 this has to be done manually[1], whereas in trunk ir.model.data has a new method for that purpose: check_object_refererence()[2].

In this particular case it seems we can do even better: we could perform a search() on stock.location for the kind of location that is desired, depending on the context. If more than one location is found we can preferably pick the "default value" if it is part of the results (which means the user can access it too!), otherwise we can just pick the first one. If no location of the right type is found we should not return any (i.e. `False`).

What do you think?

[1] This bugfix for example: http://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9069#hr_timesheet/hr_timesheet.py
[2] http://bazaar.launchpad.net/~openerp/openobject-server/trunk/view/head:/openerp/addons/base/ir/ir_model.py#L872

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

actually mrp uses get_object which does check for read permission.

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

@odo-openerp: first thing to note is that get_object_reference performs no check at all, so IMO it is wrong to use it to look for stuff with can have some ir_rules attached. So "quick and dirty patch" I'm working on for v7 uses get_object (and handles the exceptions).

This is consistent with what is done in stock for stock.warehouse (and I'm adding an extra check for the possible ValueError which can be raised in that method)

tags: added: maintenance
Amit Parik (amit-parik)
tags: added: warehouse
Revision history for this message
Martin Trigaux (OpenERP) (mat-openerp) wrote :

Hello guys,

I have merged Alexandre's patch with few modifications (some of the checks were already done in another revision).
Thanks for the patch and report

revno: 9381 [merge]
revision-id: <email address hidden>

Changed in openobject-addons:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.