diff --git a/addons/web/i18n/web.pot b/addons/web/i18n/web.pot index 8e1dd26e9b1..f5decb24748 100644 --- a/addons/web/i18n/web.pot +++ b/addons/web/i18n/web.pot @@ -5972,6 +5972,21 @@ msgid "" "You may not believe it,
but the application is actually loading..." msgstr "" +#. module: web +#. odoo-javascript +#: code:addons/web/static/src/views/fields/properties/property_tags.js:0 +#, python-format +msgid "You need to be able to edit parent first to add property tags" +msgstr "" + +#. module: web +#. odoo-javascript +#: code:addons/web/static/src/views/fields/properties/properties_field.js:0 +#: code:addons/web/static/src/views/fields/properties/properties_field.js:0 +#, python-format +msgid "You need to be able to edit parent first to configure property fields" +msgstr "" + #. module: web #. odoo-javascript #: code:addons/web/static/src/legacy/js/views/basic/basic_controller.js:0 diff --git a/addons/web/static/src/views/fields/properties/properties_field.js b/addons/web/static/src/views/fields/properties/properties_field.js index 891fb4ea66d..5f94f3e9d02 100644 --- a/addons/web/static/src/views/fields/properties/properties_field.js +++ b/addons/web/static/src/views/fields/properties/properties_field.js @@ -218,9 +218,15 @@ export class PropertiesField extends Component { * @param {event} event * @param {string} propertyName */ - onPropertyEdit(event, propertyName) { + async onPropertyEdit(event, propertyName) { event.stopPropagation(); event.preventDefault(); + if (!await this.checkDefinitionWriteAccess()) { + this.notification.add(_lt("You need to be able to edit parent first to configure property fields"), { + type: "warning", + }); + return; + } if (event.target.classList.contains("disabled")) { // remove the glitch if we click on the edit button // while the popover is already opened @@ -278,7 +284,13 @@ export class PropertiesField extends Component { this.dialogService.add(ConfirmationDialog, dialogProps); } - onPropertyCreate() { + async onPropertyCreate() { + if (!await this.checkDefinitionWriteAccess()) { + this.notification.add(_lt("You need to be able to edit parent first to configure property fields"), { + type: "warning", + }); + return; + } const propertiesDefinitions = this.propertiesList || []; if ( @@ -306,6 +318,28 @@ export class PropertiesField extends Component { this.props.record.update({ [this.props.name]: propertiesDefinitions }); } + /** + * Verify that we can write on properties, + * if we don't have access for parent + */ + async checkDefinitionWriteAccess() { + const definitionRecordId = this.props.record.data[this.definitionRecordField][0]; + const definitionRecordModel = this.props.record.fields[this.definitionRecordField].relation; + try { + await this.orm.call( + definitionRecordModel, + "check_access_rule", + [definitionRecordId], + { + operation: "write", + } + ); + } catch (_e) { // eslint-disable-line no-unused-vars + return false; + } + return true; + } + /** * The tags list has been changed. * If `newValue` is given, update the property value as well. @@ -473,6 +507,7 @@ export class PropertiesField extends Component { this.popover.open(target, { readonly: this.props.readonly || !this.state.canChangeDefinition, canChangeDefinition: this.state.canChangeDefinition, + checkDefinitionWriteAccess: () => this.checkDefinitionWriteAccess(), propertyDefinition: this.propertiesList.find( (property) => property.name === currentName(propertyName) ), diff --git a/addons/web/static/src/views/fields/properties/properties_field.xml b/addons/web/static/src/views/fields/properties/properties_field.xml index 04f78de3fc3..efa1333720f 100644 --- a/addons/web/static/src/views/fields/properties/properties_field.xml +++ b/addons/web/static/src/views/fields/properties/properties_field.xml @@ -31,6 +31,7 @@ diff --git a/addons/web/static/src/views/fields/properties/property_tags.js b/addons/web/static/src/views/fields/properties/property_tags.js index d55ab0719bf..ddfc80732a2 100644 --- a/addons/web/static/src/views/fields/properties/property_tags.js +++ b/addons/web/static/src/views/fields/properties/property_tags.js @@ -188,11 +188,18 @@ export class PropertyTags extends Component { * * @param {string} newLabel */ - onTagCreate(newLabel) { + async onTagCreate(newLabel) { if (!newLabel || !newLabel.length) { return; } + if (!await this.props.checkDefinitionWriteAccess()) { + this.notification.add(_lt("You need to be able to edit parent first to add property tags"), { + type: "warning", + }); + return; + } + const newValue = newLabel ? newLabel.toLowerCase().replace(" ", "_") : ""; const existingTag = this.props.tags.find((tag) => tag[0] === newValue); @@ -292,6 +299,7 @@ PropertyTags.props = { deleteAction: { type: String }, readonly: { type: Boolean, optional: true }, canChangeTags: { type: Boolean, optional: true }, + checkDefinitionWriteAccess: { type: Function, optional: true }, // Select a new value onValueChange: { type: Function, optional: true }, // Change the tags definition (can also receive a second diff --git a/addons/web/static/src/views/fields/properties/property_value.js b/addons/web/static/src/views/fields/properties/property_value.js index 45e4a4786f8..1a5cd0446f5 100644 --- a/addons/web/static/src/views/fields/properties/property_value.js +++ b/addons/web/static/src/views/fields/properties/property_value.js @@ -359,6 +359,7 @@ PropertyValue.props = { context: { type: Object }, readonly: { type: Boolean, optional: true }, canChangeDefinition: { type: Boolean, optional: true }, + checkDefinitionWriteAccess: { type: Function, optional: true }, selection: { type: Array, optional: true }, tags: { type: Array, optional: true }, onChange: { type: Function, optional: true }, diff --git a/addons/web/static/src/views/fields/properties/property_value.xml b/addons/web/static/src/views/fields/properties/property_value.xml index f7ab6bf1ca5..fb3750079f4 100644 --- a/addons/web/static/src/views/fields/properties/property_value.xml +++ b/addons/web/static/src/views/fields/properties/property_value.xml @@ -66,6 +66,7 @@ deleteAction="'value'" readonly="props.readonly" canChangeTags="props.canChangeDefinition" + checkDefinitionWriteAccess.bind="props.checkDefinitionWriteAccess" onValueChange.bind="onValueChange" onTagsChange.bind="props.onTagsChange"/> diff --git a/addons/web/static/tests/views/fields/properties_field_tests.js b/addons/web/static/tests/views/fields/properties_field_tests.js index 2c5eb8065c0..96d5534be3a 100644 --- a/addons/web/static/tests/views/fields/properties_field_tests.js +++ b/addons/web/static/tests/views/fields/properties_field_tests.js @@ -281,7 +281,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: access to parent", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -378,7 +378,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: add a new property", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -435,7 +435,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: selection", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -573,7 +573,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: float and integer", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -642,7 +642,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: move properties", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -717,7 +717,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: tags", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -837,7 +837,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: many2one", async function (assert) { async function mockRPC(route, { method, model, args, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } else if (method === "get_available_models" && model === "ir.model") { return [ @@ -923,7 +923,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: many2many", async function (assert) { async function mockRPC(route, { method, model, args, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } else if (method === "get_available_models" && model === "ir.model") { return [ @@ -1026,7 +1026,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: many2one 'Search more...'", async function (assert) { async function mockRPC(route, { method, model }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } else if (method === "display_name_for" && model === "ir.model") { return [ @@ -1223,7 +1223,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: name reset", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } else if (method === "get_available_models" && model === "ir.model") { return [ @@ -1361,7 +1361,7 @@ QUnit.module("Fields", (hooks) => { */ QUnit.test("properties: default value", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -1428,7 +1428,7 @@ QUnit.module("Fields", (hooks) => { "properties: form view and falsy domain, properties are not empty", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } @@ -1482,7 +1482,7 @@ QUnit.module("Fields", (hooks) => { "properties: form view and falsy domain, properties are empty", async function (assert) { async function mockRPC(route, { method, model, kwargs }) { - if (method === "check_access_rights") { + if (["check_access_rights", "check_access_rule"].includes(method)) { return true; } } diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index 5f0986c5b84..85712aeea26 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -1383,31 +1383,42 @@ class PropertiesCase(TransactionCase): self.assertEqual(expected_properties, sql_properties) @mute_logger('odoo.fields') + @users('test') def test_properties_field_security(self): """Check the access right related to the Properties fields.""" - MultiTag = type(self.env['test_new_api.multi.tag']) - - def _mocked_check_access_rights(operation, raise_exception=True): + def _mocked_check_access_rights(records, operation, raise_exception=True): + if records.env.su: # called with SUDO + return True if raise_exception: raise AccessError('') return False + message = self.message_1.with_user(self.test_user) + # a user read a properties with a many2one to a record he doesn't have access to tag = self.env['test_new_api.multi.tag'].create({'name': 'Test Tag'}) - self.message_1.attributes = [{ + + message.attributes = [{ 'name': 'test', 'type': 'many2one', 'comodel': 'test_new_api.multi.tag', 'value': [tag.id, 'Tag'], 'definition_changed': True, }] - values = self.message_1.read(['attributes'])[0]['attributes'][0] + values = message.read(['attributes'])[0]['attributes'][0] self.assertEqual(values['value'], (tag.id, 'Test Tag')) self.env.invalidate_all() - with patch.object(MultiTag, 'check_access_rights', side_effect=_mocked_check_access_rights): - values = self.message_1.read(['attributes'])[0]['attributes'][0] + with patch('odoo.addons.test_new_api.models.test_new_api.MultiTag.check_access_rights', _mocked_check_access_rights): + values = message.read(['attributes'])[0]['attributes'][0] self.assertEqual(values['value'], (tag.id, None)) + # a user read a properties with a many2one to a record + # but doesn't have access to its parent + self.env.invalidate_all() + with patch('odoo.addons.test_new_api.models.test_new_api.Discussion.check_access_rights', _mocked_check_access_rights): + values = message.read(['attributes'])[0]['attributes'][0] + self.assertEqual(values['value'], (tag.id, 'Test Tag')) + def _get_sql_properties(self, message): self.env.flush_all()