[IMP] *: do not force admins to be app admins
For bugfix purposes, app administration groups have been given to (implied by) the "Settings" group because without those rights, opening/saving the settings crashed. 1) Do not load hidden view content This commit uses the conditional inheritance of views (depending on user groups) to avoid loading unnecessary view & record content client-side. This improves performance for admins without the specific application admin rights, but also fixes the main bugfix problem, caused by the webclient querying name_get for the records in relational fields content. Example: sale_management adds a res.config.settings field to specify the default sale.order.template for the current company. If a 'Settings' user without 'sale.group_sale_manager' opens the settings, he won't see this setting, but if a default template is specified for the current company, the webclient will still request the name_get of this template to the server, because the field was present in the view, only hidden with a groups attribute. With this commit change in sale, the field won't be in the view unless you have the Sale manager group, avoiding the error/traceback/bug. 2) Remove implied application administration groups Do not force the specific application groups on all 'Settings' user, they globally do not need those rights, and if they need it, they can add it to their account themselves. 3) Add a test to make sure settings user are able to manage settings. 4) Enforce 'settings' -> 'access rights' -> 'internal user' groups As the previous test highlighted some 'false positives' because it considered a settings user unable to read `crm.team` and `stock.warehouse` records, we also took the opportunity to enforce the fact that 'Settings' & 'Access rights' users must be internal users. It makes no sense for a portal/public user to have access to the settings, and didn't work anyway. Part-of: odoo/odoo#91909
This commit is contained in:
Victor Feyens
@@ -78,10 +78,6 @@
|
||||
<field name="groups_id" eval="[(4, ref('account.group_account_invoice'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="base.group_system" model="res.groups">
|
||||
<field name="implied_ids" eval="[(4, ref('account.group_account_manager'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="group_warning_account" model="res.groups">
|
||||
<field name="name">A warning can be set on a partner (Account)</field>
|
||||
<field name="category_id" ref="base.module_category_hidden"/>
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="40"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('account.group_account_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<field name="country_code" invisible="1"/>
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="5"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('sales_team.group_sale_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="CRM" string="CRM" data-key="crm" groups="sales_team.group_sale_manager">
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="65"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('event.group_event_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Events" string="Events" data-key="event" groups="event.group_event_manager">
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="90"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('fleet.fleet_group_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Fleet" id="fleet" string="Fleet" data-key="fleet" groups="fleet.fleet_group_manager">
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="70"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('hr.group_hr_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Employees" string="Employees" data-key="hr" groups="hr.group_hr_manager">
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="80"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('hr_attendance.group_hr_attendance_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Attendances" string="Attendances" data-key="hr_attendance" groups="hr_attendance.group_hr_attendance_manager">
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="85"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('hr_expense.group_hr_expense_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Expenses" string="Expenses" data-key="hr_expense" groups="hr_expense.group_hr_expense_manager">
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="75"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('hr_recruitment.group_hr_recruitment_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Recruitment" string="Recruitment" data-key="hr_recruitment" groups="hr_recruitment.group_hr_recruitment_manager">
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="55"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('hr_timesheet.group_timesheet_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//form" position="attributes">
|
||||
<attribute name="js_class">hr_timesheet_config_form</attribute>
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="90"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('lunch.group_lunch_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Lunch" string="Lunch" data-key="lunch" groups="lunch.group_lunch_manager">
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="60"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('mass_mailing.group_mass_mailing_user'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Email Marketing" string="Email Marketing" data-key="mass_mailing" groups="mass_mailing.group_mass_mailing_user">
|
||||
|
||||
@@ -40,10 +40,6 @@
|
||||
<field name="category_id" ref="base.module_category_hidden"/>
|
||||
</record>
|
||||
|
||||
<record id="base.group_system" model="res.groups">
|
||||
<field name="implied_ids" eval="[(4, ref('mrp.group_mrp_manager'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="group_mrp_workorder_dependencies" model="res.groups">
|
||||
<field name="name">Use Operation Dependencies</field>
|
||||
<field name="category_id" ref="base.module_category_hidden"/>
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="35"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form" />
|
||||
<field name="groups_id" eval="[(4, ref('mrp.group_mrp_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Manufacturing" string="Manufacturing" data-key="mrp" groups="mrp.group_mrp_manager">
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="95"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form" />
|
||||
<field name="groups_id" eval="[(4, ref('point_of_sale.group_pos_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<field name="pos_selectable_categ_ids" invisible="1"/>
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="50"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form" />
|
||||
<field name="groups_id" eval="[(4, ref('project.group_project_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Project" string="Project" data-key="project" groups="project.group_project_manager">
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="25"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('purchase.group_purchase_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Purchase" string="Purchase" data-key="purchase" groups="purchase.group_purchase_manager">
|
||||
|
||||
@@ -5,7 +5,8 @@
|
||||
<field name="name">res.config.settings.view.form.inherit.sale</field>
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="10"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form" />
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('sales_team.group_sale_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block o_not_app"
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="30"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form" />
|
||||
<field name="groups_id" eval="[(4, ref('stock.group_stock_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside" >
|
||||
<div class="app_settings_block" data-string="Inventory" string="Inventory" data-key="stock" groups="stock.group_stock_manager">
|
||||
|
||||
@@ -23,10 +23,6 @@
|
||||
<field name="groups_id" eval="[(4, ref('website.group_website_designer'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="base.group_system" model="res.groups">
|
||||
<field name="implied_ids" eval="[(4, ref('website.group_website_designer'))]"/>
|
||||
</record>
|
||||
|
||||
<data noupdate="1">
|
||||
|
||||
<record id="website_designer_edit_qweb" model="ir.rule">
|
||||
|
||||
@@ -1,15 +1,24 @@
|
||||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<odoo>
|
||||
<record id="res_config_settings_view_form_inherit_auth_signup" model="ir.ui.view">
|
||||
<field name="name">res.config.settings.view.form.inherit.website</field>
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="inherit_id" ref="auth_signup.res_config_settings_view_form"/>
|
||||
<field name="arch" type="xml">
|
||||
<!-- Remove customer accounts setting from general settings tab -->
|
||||
<!-- It must not be in the view at all to make sure settings can be saved
|
||||
(because auth_signup_uninvited is specified as required) -->
|
||||
<xpath expr="//div[@id='login_documents']" position="replace"></xpath>
|
||||
</field>
|
||||
</record>
|
||||
|
||||
<record id="res_config_settings_view_form" model="ir.ui.view">
|
||||
<field name="name">res.config.settings.view.form.inherit.website</field>
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="priority" eval="20"/>
|
||||
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('website.group_website_designer'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<!-- Remove customer accounts setting from general settings tab -->
|
||||
<xpath expr="//div[@id='login_documents']" position="attributes">
|
||||
<attribute name="invisible">1</attribute>
|
||||
</xpath>
|
||||
<xpath expr="//div[hasclass('settings')]" position="inside">
|
||||
<div class="app_settings_block" data-string="Website" string="Website" data-key="website" groups="website.group_website_designer">
|
||||
<div class="row o_settings_container mb-0 mt-0 ml-0 mw-100"
|
||||
|
||||
@@ -24,9 +24,9 @@
|
||||
<field name="inherit_id" ref="sale.res_config_settings_view_form"/>
|
||||
<field name="arch" type="xml">
|
||||
<!-- Remove customer accounts setting from sales settings tab -->
|
||||
<xpath expr="//div[@id='auth_signup_documents']" position="attributes">
|
||||
<attribute name="invisible">1</attribute>
|
||||
</xpath>
|
||||
<!-- It must not be in the view at all to make sure settings can be saved
|
||||
(because auth_signup_uninvited is specified as required) -->
|
||||
<xpath expr="//div[@id='auth_signup_documents']" position="replace"></xpath>
|
||||
</field>
|
||||
</record>
|
||||
|
||||
|
||||
@@ -20,10 +20,6 @@
|
||||
<field name="groups_id" eval="[(4,ref('group_website_slides_manager'))]"/>
|
||||
</record>
|
||||
|
||||
<record id="base.group_system" model="res.groups">
|
||||
<field name="implied_ids" eval="[(4, ref('group_website_slides_manager'))]"/>
|
||||
</record>
|
||||
|
||||
<data noupdate="1">
|
||||
<!-- CHANNEL -->
|
||||
<record id="rule_slide_channel_global" model="ir.rule">
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
<field name="name">res.config.settings.view.form.inherit.website.slides</field>
|
||||
<field name="model">res.config.settings</field>
|
||||
<field name="inherit_id" ref="website.res_config_settings_view_form"/>
|
||||
<field name="groups_id" eval="[(4, ref('website_slides.group_website_slides_manager'), 0)]"/>
|
||||
<field name="arch" type="xml">
|
||||
<xpath expr="//div[@id='website_marketing_automation']" position="after">
|
||||
<div class="col-12 col-lg-6 o_setting_box" id="slides_install_setting">
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
-->
|
||||
<record model="res.groups" id="group_erp_manager">
|
||||
<field name="name">Access Rights</field>
|
||||
<field name="implied_ids" eval="[Command.link(ref('group_user'))]"/>
|
||||
</record>
|
||||
|
||||
<record model="res.groups" id="group_system">
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from collections import defaultdict
|
||||
import logging
|
||||
|
||||
from odoo import exceptions
|
||||
from odoo.tests.common import TransactionCase, tagged
|
||||
from odoo import exceptions, Command
|
||||
from odoo.tests.common import Form, TransactionCase, tagged
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -97,3 +98,86 @@ class TestResConfigExecute(TransactionCase):
|
||||
for config_settings in all_config_settings:
|
||||
_logger.info("Testing %s" % (config_settings.name))
|
||||
self.env[config_settings.name].create({}).execute()
|
||||
|
||||
def test_settings_access(self):
|
||||
"""Check that settings user are able to open & save settings
|
||||
|
||||
Also check that user with settings rights + any one of the groups restricting
|
||||
a conditional view inheritance of res.config.settings view is also able to
|
||||
open & save the settings (considering the added conditional content)
|
||||
"""
|
||||
ResUsers = self.env['res.users']
|
||||
group_system = self.env.ref('base.group_system')
|
||||
self.settings_view = self.env.ref('base.res_config_settings_view_form')
|
||||
settings_only_user = ResUsers.create({
|
||||
'name': 'Sleepy Joe',
|
||||
'login': 'sleepy',
|
||||
'groups_id': [Command.link(group_system.id)],
|
||||
})
|
||||
|
||||
_logger.info("Testing settings access for group %s", group_system.full_name)
|
||||
forbidden_models = self._test_user_settings_fields_access(settings_only_user)
|
||||
self._test_user_settings_view_save(settings_only_user)
|
||||
|
||||
for model in forbidden_models:
|
||||
_logger.warning("Settings user doesn\'t have read access to the model %s", model)
|
||||
|
||||
settings_view_conditional_groups = self.env['ir.ui.view'].search([
|
||||
('model', '=', 'res.config.settings'),
|
||||
]).groups_id
|
||||
|
||||
for group in settings_view_conditional_groups:
|
||||
group_name = group.full_name
|
||||
_logger.info("Testing settings access for group %s", group_name)
|
||||
create_values = {
|
||||
'name': f'Test {group_name}',
|
||||
'login': group_name,
|
||||
'groups_id': [Command.link(group_system.id), Command.link(group.id)]
|
||||
}
|
||||
user = ResUsers.create(create_values)
|
||||
self._test_user_settings_view_save(user)
|
||||
forbidden_models_fields = self._test_user_settings_fields_access(user)
|
||||
|
||||
for model, fields in forbidden_models_fields.items():
|
||||
_logger.warning(
|
||||
"Settings + %s user doesn\'t have read access to the model %s"
|
||||
"linked to settings records by the field(s) %s",
|
||||
group_name, model, ", ".join(str(field) for field in fields)
|
||||
)
|
||||
|
||||
def _test_user_settings_fields_access(self, user):
|
||||
"""Verify that settings user are able to create & save settings."""
|
||||
settings = self.env['res.config.settings'].with_user(user).create({})
|
||||
|
||||
# Save the settings
|
||||
settings.set_values()
|
||||
|
||||
# Check user has access to all models of relational fields in view
|
||||
# because the webclient makes a name_get request for all specified records
|
||||
# even if they are not shown to the user.
|
||||
settings_view_arch = self.settings_view.with_user(user)._get_combined_arch()
|
||||
seen_fields = set()
|
||||
for node in settings_view_arch.iterdescendants(tag='field'):
|
||||
seen_fields.add(node.get('name'))
|
||||
|
||||
models_to_check = defaultdict(set)
|
||||
for field_name in seen_fields:
|
||||
field = settings._fields[field_name]
|
||||
if field.relational:
|
||||
models_to_check[field.comodel_name].add(field)
|
||||
|
||||
forbidden_models_fields = defaultdict(set)
|
||||
for model in models_to_check:
|
||||
has_read_access = self.env[model].with_user(user).check_access_rights(
|
||||
'read', raise_exception=False)
|
||||
if not has_read_access:
|
||||
forbidden_models_fields[model] = models_to_check[model]
|
||||
|
||||
return forbidden_models_fields
|
||||
|
||||
def _test_user_settings_view_save(self, user):
|
||||
"""Verify that settings user are able to save the settings form."""
|
||||
ResConfigSettings = self.env['res.config.settings'].with_user(user)
|
||||
|
||||
settings_form = Form(ResConfigSettings)
|
||||
settings_form.save()
|
||||
|
||||
@@ -33,7 +33,7 @@ class TestActionBindings(common.TransactionCase):
|
||||
)
|
||||
|
||||
# add a group on an action, and check that it is not returned
|
||||
group = self.env.ref('base.group_user')
|
||||
group = self.env.ref('base.group_system')
|
||||
action2.groups_id += group
|
||||
self.env.user.groups_id -= group
|
||||
|
||||
|
||||
Reference in New Issue
Block a user