[15.0][MIG] base_user_role_company #241
Conversation
| Enable User Roles depending on the Companies selected. | ||
|
|
||
| A company specific Role will only be enabled | ||
| if it is set for **all** the currently selected companies. |
There was a problem hiding this comment.
@dreispt this isn't true since at least v14. To me it would make sense to adapt the code accordingly, or do you as maintainer say better adapt the readme?
There was a problem hiding this comment.
@hbrunn agree that the current README does not reflect what the code does.
The current code will poses a threat from a security standpoint. If you are assigned to Sales Manager role in company A only, you can still access to all sales manager functions in company B by just enabling this company together with company A.
I think that it's necessary to adapt the code to the README.
There was a problem hiding this comment.
@hbrunn agree that the current README does not reflect what the code does.
The current code will poses a threat from a security standpoint. If you are assigned to Sales Manager role in company A only, you can still access to all sales manager functions in company B by just enabling this company together with company A.
I think that it's necessary to adapt the code to the README.
There was a problem hiding this comment.
Is there some news about this?
There was a problem hiding this comment.
I think introducing a field last_cids on res.users that we write every time we rewrite groups should work. Then we can restrict the incoming allowed companies to the intersection with last_cids, and the company switcher then has to call a controller that does updating last_cids + group update in one transaction.
With this we should be able to guarantee you never have groups assigned for companies that shouldn't.
There was a problem hiding this comment.
Hello here!
I work for an Odoo integrator. I think this module could be quite useful for some clients. I subscribe to the analysis that it doesn't do what it's expected to as soon it is considered as a security module (which is how I see it and how I think it could be useful).
I'm a functional guy so I'm probably missing many technical aspects. Anyways, here is my take about what the module should do :
- Regarding the Menus / Views / Access Rights, consider the intersection of what the companies ticked in the selector give access to (through the groups associated to the roles and their inherited groups). Example with Access Rights, if you have access to those two groups through companies ticked in the selector :
=> Then you should only get read and write access to sales orders and sale orders lines - Regarding the record rules : apply an "AND" rule. In the example below we have 2 groups of record rules :
=> In this example, since we have a record rule restricting the modification to one's own sales and the other to the sales in "draft" state (quotations), if the companies in the selector give access to those two groups, the modification should only be allowed for one's own sales in "draft" state.
What's your opinion ? Happy to discuss this topic with you !
There was a problem hiding this comment.
what you describe is changing how roles work in the first place. That would involve changing the base module, so very much out of scope here.
There was a problem hiding this comment.
Hello @hbrunn ,
Thanks for reading my post and coming back to me!
Do you have other solutions in mind ?
There was a problem hiding this comment.
what I wrote above, but that also won't happen in this PR I think
|
/ocabot migration base_user_role_company |
|
There's no issue in this repo with the title 'Migration to version 15.0' and the milestone 15.0, so not possible to add the comment. |
|
@hbrunn Looks good. Perhaps a small glitch (not associated with the migration) is that when you access to the users associated to a role, from the role, the company assigned to the user is not visible.
It is only visible from the user:
|
|
@JordiBForgeFlow I think I can slip this into this PR if nobody objects? Do you have an opinion about #241 (review) ? |
|
@hbrunn I would add the change as a separate commit, so that we can fwp and backport. |
Currently translated at 100.0% (11 of 11 strings) Translation: server-backend-14.0/server-backend-14.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-14-0/server-backend-14-0-base_user_role_company/it/
Issue found on logout / relogin. The user groups were applied correctly, but the main menu showed apps the user did not have access to. This was related to the menu caching mechanisn, that was disabled here.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: server-backend-14.0/server-backend-14.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-14-0/server-backend-14-0-base_user_role_company/
Currently translated at 90.9% (10 of 11 strings) Translation: server-backend-14.0/server-backend-14.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-14-0/server-backend-14-0-base_user_role_company/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: server-backend-16.0/server-backend-16.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/
Currently translated at 100.0% (8 of 8 strings) Translation: server-backend-16.0/server-backend-16.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/es/
Currently translated at 100.0% (8 of 8 strings) Translation: server-backend-16.0/server-backend-16.0-base_user_role_company Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/pt/
when this role is set for all currently active companies
hbrunn
force-pushed
the
15.0-mig-base_user_role_company
branch
from
c432df7
to
28c9cec
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
dreispt
removed
the
stale
this is a backport from v16 as it contains patches the previous v15 PRs (#139, #187) didn't