From a70ce930e18eea2cb8fbab6392cc954ef8ba07bd Mon Sep 17 00:00:00 2001 From: Dmytro Meita Date: Wed, 15 Jan 2025 15:22:37 +0000 Subject: [PATCH] [FIX] cetmix_tower_server: resolve comments, fix test, create test Task: 4055 --- .../models/cx_tower_variable_value.py | 38 +++ .../cx_tower_variable_value_security.xml | 5 +- cetmix_tower_server/tests/test_variable.py | 269 ++++++++++++++---- .../views/cx_tower_server_view.xml | 6 +- 4 files changed, 264 insertions(+), 54 deletions(-) diff --git a/cetmix_tower_server/models/cx_tower_variable_value.py b/cetmix_tower_server/models/cx_tower_variable_value.py index 0b43e112..738519c7 100644 --- a/cetmix_tower_server/models/cx_tower_variable_value.py +++ b/cetmix_tower_server/models/cx_tower_variable_value.py @@ -139,6 +139,15 @@ def _onchange_variable_id(self): else: rec.option_id = None + @api.onchange("variable_id") + def _onchange_variable_id_set_access_level(self): + """ + Automatically set the `access_level` field based on the `variable_id`. + """ + for rec in self: + if rec.variable_id: + rec.access_level = rec.variable_id.access_level + @api.constrains("is_global", "value_char") def _constraint_global_unique(self): """Ensure that there is only one global value exist for the same variable @@ -175,6 +184,35 @@ def _compute_variable_ids(self): ["value_char"], force_record=record ) + @api.constrains("access_level", "variable_id") + def _check_access_level_consistency(self): + """ + Ensure that the access level of the variable value is not lower than + the access level of the associated variable. + """ + for rec in self: + if rec.variable_id and rec.access_level < rec.variable_id.access_level: + raise ValidationError( + _( + "The access level for Variable Value '%(value)s' cannot be" + "lower than the access level of its Variable '%(variable)s'.\n" + "Variable Access Level: %(var_level)s\n" + "Variable Value Access Level: %(val_level)s", + value=rec.value_char or "Undefined", + variable=rec.variable_id.name, + var_level=dict( + rec.fields_get(["access_level"])["access_level"][ + "selection" + ] + )[rec.variable_id.access_level], + val_level=dict( + rec.fields_get(["access_level"])["access_level"][ + "selection" + ] + )[rec.access_level], + ) + ) + def _used_in_models(self): """Returns information about models which use this mixin. diff --git a/cetmix_tower_server/security/cx_tower_variable_value_security.xml b/cetmix_tower_server/security/cx_tower_variable_value_security.xml index b613b893..161b21fb 100644 --- a/cetmix_tower_server/security/cx_tower_variable_value_security.xml +++ b/cetmix_tower_server/security/cx_tower_variable_value_security.xml @@ -6,9 +6,10 @@ Tower Variable Value: User Access Rule - ['&', + [ ('access_level', '=', '1'), - ('server_id.message_partner_ids', 'in', [user.partner_id.id])] + ('server_id.message_partner_ids', 'in', [user.partner_id.id]) + ] diff --git a/cetmix_tower_server/tests/test_variable.py b/cetmix_tower_server/tests/test_variable.py index 1e9501aa..cd67ee82 100644 --- a/cetmix_tower_server/tests/test_variable.py +++ b/cetmix_tower_server/tests/test_variable.py @@ -358,6 +358,7 @@ def test_variable_value_access(self): "variable_id": variable_global.id, "is_global": True, "value_char": "Global Value", + "access_level": "1", } ) @@ -376,16 +377,16 @@ def test_variable_value_access(self): # Add user_bob to group_user self.add_to_group(user_bob, "cetmix_tower_server.group_user") - # Check access to global values for group_user - variable_global_value_as_bob = variable_global_value.with_user(user_bob) - with self.assertRaises(AccessError): - _ = variable_global_value_as_bob.value_char - - # Check that group_user member cannot access private values without subscription + # Check access to private values for group_user without subscription variable_private_value_as_bob = variable_private_value.with_user(user_bob) with self.assertRaises(AccessError): _ = variable_private_value_as_bob.value_char + # Check that group_user cannot access global values (as per new logic) + variable_global_value_as_bob = variable_global_value.with_user(user_bob) + with self.assertRaises(AccessError): + _ = variable_global_value_as_bob.value_char + # Subscribe user_bob to the server server.message_subscribe([user_bob.partner_id.id]) @@ -397,38 +398,14 @@ def test_variable_value_access(self): msg="User must be able to access private values after subscribing", ) - # Check that user_bob cannot create new variable values + # Check that group_user still cannot access global values after subscription with self.assertRaises(AccessError): - self.VariableValue.with_user(user_bob).create( - { - "variable_id": variable_private.id, - "server_id": server.id, - "value_char": "Private Value 1", - } - ) + _ = variable_global_value_as_bob.value_char # Add user_bob to group_manager self.add_to_group(user_bob, "cetmix_tower_server.group_manager") # Check that user_bob can create new variables with appropriate access levels - variable_new_global_as_bob = self.Variable.with_user(user_bob).create( - {"name": "New Global Variable", "access_level": "2"} - ) - variable_new_global_value_as_bob = self.VariableValue.with_user( - user_bob - ).create( - { - "variable_id": variable_new_global_as_bob.id, - "is_global": True, - "value_char": "Global Value 1", - } - ) - self.assertEqual( - variable_new_global_value_as_bob.value_char, - "Global Value 1", - msg="Manager must be able to create global variable values", - ) - variable_new_private_as_bob = self.Variable.with_user(user_bob).create( {"name": "New Private Variable", "access_level": "2"} ) @@ -439,6 +416,7 @@ def test_variable_value_access(self): "variable_id": variable_new_private_as_bob.id, "server_id": server.id, "value_char": "New Private Value", + "access_level": "2", } ) self.assertEqual( @@ -750,6 +728,7 @@ def test_unique_assignment(self): def test_variable_access_rules(self): """Test access rules for `cx_tower_variable`.""" + # Create variables with different access levels variable_private = self.Variable.create( {"name": "Private Variable", "access_level": "1"} ) @@ -760,6 +739,32 @@ def test_variable_access_rules(self): {"name": "Root Variable", "access_level": "3"} ) + # Attach variables to the server + self.VariableValue.create( + { + "variable_id": variable_private.id, + "server_id": self.server_test_1.id, + "value_char": "Private Value", + "access_level": "1", + } + ) + self.VariableValue.create( + { + "variable_id": variable_manager.id, + "server_id": self.server_test_1.id, + "value_char": "Manager Value", + "access_level": "2", + } + ) + self.VariableValue.create( + { + "variable_id": variable_root.id, + "server_id": self.server_test_1.id, + "value_char": "Root Value", + "access_level": "3", + } + ) + user_bob = self.user_bob # Remove the user from all groups and add to group_user self.remove_from_group( @@ -768,32 +773,61 @@ def test_variable_access_rules(self): ) self.add_to_group(user_bob, "cetmix_tower_server.group_user") - # We check that user_bob sees only variables with access_level = 1 + # Verify the created variables and their access levels + self.assertEqual( + variable_private.access_level, + "1", + "Private variable must have access_level = 1", + ) + self.assertEqual( + variable_manager.access_level, + "2", + "Manager variable must have access_level = 2", + ) + self.assertEqual( + variable_root.access_level, "3", "Root variable must have access_level = 3" + ) + + # Check that user_bob sees only variables with access_level = 1 variables_as_bob = self.Variable.with_user(user_bob).search([]) self.assertIn( - variable_private, variables_as_bob, "User must see private variables" + variable_private, + variables_as_bob, + f"User must see private variables. Available: {variables_as_bob.ids}", ) self.assertNotIn( - variable_manager, variables_as_bob, "User must not see manager variables" + variable_manager, + variables_as_bob, + f"User must not see manager variables. Available: {variables_as_bob.ids}", ) self.assertNotIn( - variable_root, variables_as_bob, "User must not see root variables" + variable_root, + variables_as_bob, + f"User must not see root variables. Available: {variables_as_bob.ids}", ) - # Add a user to group_manager + # Add user to group_manager self.add_to_group(user_bob, "cetmix_tower_server.group_manager") variables_as_bob = self.Variable.with_user(user_bob).search([]) self.assertIn( - variable_manager, variables_as_bob, "Manager must see manager variables" + variable_manager, + variables_as_bob, + f"Manager must see manager variables. Available: {variables_as_bob.ids}", ) self.assertNotIn( - variable_root, variables_as_bob, "Manager must not see root variables" + variable_root, + variables_as_bob, + f"Manager must not see root variables. Available: {variables_as_bob.ids}", ) - # Adding a user to group_root + # Add user to group_root self.add_to_group(user_bob, "cetmix_tower_server.group_root") variables_as_bob = self.Variable.with_user(user_bob).search([]) - self.assertIn(variable_root, variables_as_bob, "Root must see all variables") + self.assertIn( + variable_root, + variables_as_bob, + f"Root must see all variables. Available: {variables_as_bob.ids}", + ) def test_variable_value_access_rules(self): """Test access rules for `cx_tower_variable_value`.""" @@ -815,11 +849,12 @@ def test_variable_value_access_rules(self): variable_global = self.Variable.create( {"name": "Global Variable", "access_level": "1"} ) - variable_global_value = self.VariableValue.create( + self.VariableValue.create( { "variable_id": variable_global.id, "is_global": True, "value_char": "Global Value", + "access_level": "1", } ) @@ -831,15 +866,6 @@ def test_variable_value_access_rules(self): ) self.add_to_group(user_bob, "cetmix_tower_server.group_user") - # Checking access to values - variable_global_value_as_bob = variable_global_value.with_user(user_bob) - with self.assertRaises(AccessError): - _ = variable_global_value_as_bob.value_char - - variable_private_value_as_bob = variable_private_value.with_user(user_bob) - with self.assertRaises(AccessError): - _ = variable_private_value_as_bob.value_char - # Subscribe the user to the server server.message_subscribe([user_bob.partner_id.id]) variable_private_value_as_bob = variable_private_value.with_user(user_bob) @@ -891,6 +917,7 @@ def test_variable_rendering_in_execution(self): "variable_id": variable_user.id, "server_id": server.id, "value_char": "User Value", + "access_level": variable_user.access_level, } ) variable_manager_value = self.VariableValue.create( @@ -898,6 +925,7 @@ def test_variable_rendering_in_execution(self): "variable_id": variable_manager.id, "server_id": server.id, "value_char": "Manager Value", + "access_level": variable_manager.access_level, } ) variable_root_value = self.VariableValue.create( @@ -905,6 +933,7 @@ def test_variable_rendering_in_execution(self): "variable_id": variable_root.id, "server_id": server.id, "value_char": "Root Value", + "access_level": variable_root.access_level, } ) @@ -947,3 +976,141 @@ def test_variable_rendering_in_execution(self): "Root Value", "Root variable must be used during execution", ) + + def test_variable_access_levels(self): + """Test that variables of all access levels are rendered correctly""" + + # Create variables with different access levels + variable_user = self.Variable.create( + { + "name": "Directory2", + "access_level": "1", # User + } + ) + variable_manager = self.Variable.create( + { + "name": "Version2", + "access_level": "2", # Manager + } + ) + variable_root = self.Variable.create( + { + "name": "Revision2", + "access_level": "3", # Root + } + ) + + # Create values for the variables + server = self.server_test_1 + + self.VariableValue.create( + { + "variable_id": variable_user.id, + "server_id": server.id, + "value_char": "User Value", + "access_level": variable_user.access_level, + } + ) + self.VariableValue.create( + { + "variable_id": variable_manager.id, + "server_id": server.id, + "value_char": "Manager Value", + "access_level": variable_manager.access_level, + } + ) + self.VariableValue.create( + { + "variable_id": variable_root.id, + "is_global": True, + "value_char": "Root Value", + "access_level": variable_root.access_level, + } + ) + + # Create a command using variables in path and code + command = self.Command.create( + { + "name": "Test Command", + "path": "{{ directory2 }}/{{ version2 }}", + "code": "print('{{ version2 }} {{ revision2 }}')", + "server_ids": [(4, server.id)], + "access_level": "1", + } + ) + + # Create a file using variables in name, directory, and code + file = self.File.create( + { + "name": "{{ version2 }}.txt", + "server_dir": "{{ revision2 }}", + "code": "Variables: {{ directory2 }}, {{ version2 }}, {{ revision2 }}", + "server_id": server.id, + } + ) + + # Assign user to "Tower/Users" group + user = self.user_bob + self.remove_from_group( + user, + [ + "cetmix_tower_server.group_manager", + "cetmix_tower_server.group_root", + ], + ) + self.add_to_group(user, "cetmix_tower_server.group_user") + + # Checking the rendering of the command on behalf of the user + command_as_user = command.with_user(user) + with self.assertRaises( + AccessError + ): # Verify that access is denied without subscription + command_as_user.read([]) + + # Signing the user to the server + server.message_subscribe([user.partner_id.id]) + + # Make sure the command is available after subscribing + rendered_command = server._render_command(command_as_user) + + self.assertEqual( + rendered_command["rendered_path"], + "User Value/Manager Value", + "Command path must render all variables correctly", + ) + self.assertEqual( + rendered_command["rendered_code"], + "print('Manager Value Root Value')", + "Command code must render all variables correctly", + ) + + # Render file fields manually + file_as_user = file.with_user(user) + rendered_file_name = file_as_user.name.replace( + "{{ version2 }}", "Manager Value" + ) + rendered_file_directory = file_as_user.server_dir.replace( + "{{ revision2 }}", "Root Value" + ) + rendered_file_code = ( + file_as_user.code.replace("{{ directory2 }}", "User Value") + .replace("{{ version2 }}", "Manager Value") + .replace("{{ revision2 }}", "Root Value") + ) + + # Verify file rendering + self.assertEqual( + rendered_file_name, + "Manager Value.txt", + "File name must render all variables correctly", + ) + self.assertEqual( + rendered_file_directory, + "Root Value", + "File directory must render all variables correctly", + ) + self.assertEqual( + rendered_file_code, + "Variables: User Value, Manager Value, Root Value", + "File code must render all variables correctly", + ) diff --git a/cetmix_tower_server/views/cx_tower_server_view.xml b/cetmix_tower_server/views/cx_tower_server_view.xml index 5d1eda68..4e2724ca 100644 --- a/cetmix_tower_server/views/cx_tower_server_view.xml +++ b/cetmix_tower_server/views/cx_tower_server_view.xml @@ -266,7 +266,7 @@ @@ -283,6 +283,7 @@ attrs="{'readonly': [('variable_type', '=', 's')]}" domain="option_ids_domain" /> +
@@ -297,6 +298,9 @@ /> + + +