Tryton - Issues

 

Issue7502

Title Access error when a button depends on a field that the user does not have access to
Priority feature Status in-progress
Superseder Nosy List ced, pokoli, reviewbot
Type behavior Components account_invoice, trytond
Assigned To pokoli Keywords review
Reviews 47511002,48461004
View: 47511002, 48461004

Created on 2018-06-05.17:39:39 by pokoli, last changed by reviewbot.

Messages
review48461004 updated at https://codereview.tryton.org/48461004/#ps60001
msg41316 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-08.09:03:34
review48461004 is not enough to fix the problem. The button should be removed from the XML just like the fields.
review48461004 updated at https://codereview.tryton.org/48461004/#ps40001
review48461004 updated at https://codereview.tryton.org/48461004/#ps20001
msg41275 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-06.13:24:04
The problem is not the changeset 15ae4cf806b5 but the changeset 80fbc1dacc95 that added depends on the cancel_move to the button.
So I agree that we should be careful about adding depends on button, especially for cosmetic states like invisible.
But the ModelView._view_look_dom_arch should remove buttons that depends on field that can not be display because of the access rights.
review48461004 updated at https://codereview.tryton.org/48461004/#ps1
msg41272 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-06-06.12:48:57
For the concrete case we are facing the problem is that is not possible to move a cancelled invoice with a cancel move back to draft. The was introduced on changeset 15ae4cf806b5

If the invoice has a cancel move showing the button will make the post method fail as it will try to remove the invoice.move which will be already posted and it's not possible to delete a posted move. 

So I think it's safe to remove it.
msg41269 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-06.12:15:40
Now, it makes sense. But I'm worry that removing such field will break the behavior of the application. If a field depends on another, it is because it is needed to properly work like for an on_change, required states or digits.
I guess the problem is indeed that the button has a depends that is not strictly required.
review47511002 updated at https://codereview.tryton.org/47511002/#ps40001
msg41265 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-06-06.11:48:11
The issue is that the fields dependant fields are added without taking in account if the user is allowed to read them. Removing them fields fixed the issue.
msg41253 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-05.18:13:18
But does it tries to read this field, it should have been removed?
New review47511002 at https://codereview.tryton.org/47511002/#ps20001
msg41251 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-06-05.17:53:19
The client tries to read the cancel_move field of the invoice which is not allowed (as the user does not have access to account.move model) so an access error is raised.
msg41250 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-06-05.17:50:46
I do not understand where is the access error?
msg41249 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-06-05.17:39:38
We have defined a new user group which have access to the following models:

- account.invoice
- account.invoice.line
- account.invoice.tax
- account.invoice.line-account.tax

When a user of this group opens a cancelled invoice it gets an access error because it can not access the account.move field. This is because the draft button is readonly if the invoice has a cancel move [1] 

Since issue6919 the server now on which field the buttons depends. This can be used to remove the button from the view if the user does not have access to all dependant fields like we do for field dependencies. 

[1] http://hg.tryton.org/modules/account_invoice/file/41b515ac4843/invoice.py#l292
History
Date User Action Args
2018-06-11 12:52:48reviewbotsetmessages: + msg41328
2018-06-08 09:03:35cedsetstatus: testing -> in-progress
messages: + msg41316
2018-06-07 13:02:45reviewbotsetmessages: + msg41303
2018-06-06 16:35:09reviewbotsetmessages: + msg41285
2018-06-06 13:24:05cedsetmessages: + msg41275
2018-06-06 13:03:33reviewbotsetmessages: + msg41273
2018-06-06 12:49:26pokolisetreviews: 47511002 -> 47511002,48461004
2018-06-06 12:48:57pokolisetstatus: in-progress -> testing
component: + account_invoice
messages: + msg41272
2018-06-06 12:15:40cedsetmessages: + msg41269
2018-06-06 12:02:42reviewbotsetmessages: + msg41267

Showing 10 items. Show all history (warning: this could be VERY long)