From 4d5d62be3ddefd024e3d3adbaba2e9c45c83f851 Mon Sep 17 00:00:00 2001 From: LuminatiHD <91634405+LuminatiHD@users.noreply.github.com> Date: Fri, 21 Jun 2024 08:13:26 +0200 Subject: [PATCH 1/7] fix converge idempotency tests (#133) * fix converge idempotency tests * remove idempotency stage in molecule since we test it in converge --- .../converge.yml | 42 +++++++++++++++++-- .../molecule.yml | 1 - 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/molecule/system_high_availability_settings/converge.yml b/molecule/system_high_availability_settings/converge.yml index 155f515e..1126f8c7 100644 --- a/molecule/system_high_availability_settings/converge.yml +++ b/molecule/system_high_availability_settings/converge.yml @@ -47,7 +47,7 @@ remote_system_username: opnsense remote_system_password: v3rys3cure services_to_synchronize: "{{ services }}" - tags: all_plugs + changed_when: false - name: Converge - get config ansible.builtin.slurp: @@ -72,18 +72,36 @@ - "'on' in ( current_config.content | b64decode )" loop: "{{ services }}" + - name: Converge - check idempotence + puzzle.opnsense.system_high_availability_settings: + synchronize_interface: LAN + synchronize_config_to_ip: 224.0.0.240 + synchronize_peer_ip: 224.0.0.241 + disable_preempt: true + disconnect_dialup_interfaces: true + synchronize_states: true + remote_system_username: opnsense + remote_system_password: v3rys3cure + services_to_synchronize: "{{ services }}" + register: idempotence + + - name: Converge - Check that parameters stay unaffected + ansible.builtin.assert: + that: + - idempotence.changed == false + - name: Converge - Change nothing puzzle.opnsense.system_high_availability_settings: synchronize_interface: LAN disable_preempt: true disconnect_dialup_interfaces: true synchronize_states: true - register: reset_all + register: change_nothing - name: Converge - Check that parameters stay unaffected ansible.builtin.assert: that: - - reset_all.changed == false + - change_nothing.changed == false - name: Converge - Remove all services, set disable_preempt, disconnect_dialup_interfaces, synchronize_states to false puzzle.opnsense.system_high_availability_settings: @@ -93,6 +111,7 @@ disconnect_dialup_interfaces: false synchronize_states: false register: synch_remove_services + changed_when: false - name: Converge - get config ansible.builtin.slurp: @@ -112,6 +131,20 @@ - "'on' not in ( current_config.content | b64decode )" - "'on' not in ( current_config.content | b64decode )" + - name: Converge - check idempotence + puzzle.opnsense.system_high_availability_settings: + synchronize_interface: LAN + services_to_synchronize: [] + disable_preempt: false + disconnect_dialup_interfaces: false + synchronize_states: false + register: idempotence + + - name: Converge - Check that parameters stay unaffected + ansible.builtin.assert: + that: + - idempotence.changed == false + - name: Converge - Enable unsupported service puzzle.opnsense.system_high_availability_settings: synchronize_interface: LAN @@ -128,6 +161,7 @@ - name: Converge - Get OPNsense version ansible.builtin.command: opnsense-version -O register: opnsense_version_cmd + changed_when: false - name: Converge - Set OPNsense version fact ansible.builtin.set_fact: @@ -139,6 +173,7 @@ services_to_synchronize: Web Proxy register: unsupported_service_output ignore_errors: true + changed_when: false - name: Converge - Check success on supported versions ansible.builtin.assert: @@ -162,6 +197,7 @@ - WireGuard register: unsupported_service_output ignore_errors: true + changed_when: false - name: Converge - Check success on supported versions ansible.builtin.assert: diff --git a/molecule/system_high_availability_settings/molecule.yml b/molecule/system_high_availability_settings/molecule.yml index 18ecbbb2..d7bb77b4 100644 --- a/molecule/system_high_availability_settings/molecule.yml +++ b/molecule/system_high_availability_settings/molecule.yml @@ -7,7 +7,6 @@ scenario: - syntax - create - converge - - idempotence - verify - destroy From 656164698e2aa1930853ee049d617f56c102f68a Mon Sep 17 00:00:00 2001 From: KiLLuuuhh <74672908+KiLLuuuhh@users.noreply.github.com> Date: Fri, 28 Jun 2024 15:06:22 +0200 Subject: [PATCH 2/7] fix-apikeys-attribute-as-list (#130) --- .../130-fix-apikeys-attribute-as-list.yml | 4 + molecule/system_access_users/converge.yml | 41 +- .../module_utils/system_access_users_utils.py | 599 ++++++++++-------- plugins/modules/system_access_users.py | 25 +- .../test_system_access_users_utils.py | 541 ++++++++++------ 5 files changed, 725 insertions(+), 485 deletions(-) create mode 100644 changelogs/fragments/130-fix-apikeys-attribute-as-list.yml diff --git a/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml b/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml new file mode 100644 index 00000000..98e3d742 --- /dev/null +++ b/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml @@ -0,0 +1,4 @@ +--- +bugfixes: + - system_access_users - apikeys parameters are now passed as a list of dicts + - system_access_users - apikeys are changed if updated \ No newline at end of file diff --git a/molecule/system_access_users/converge.yml b/molecule/system_access_users/converge.yml index 605ed2dd..7b551e1d 100644 --- a/molecule/system_access_users/converge.yml +++ b/molecule/system_access_users/converge.yml @@ -12,6 +12,7 @@ puzzle.opnsense.system_access_users: username: test_user_1 password: test_password_1 + full_name: "Test User 1: Test minimum requirements User Creation" # Test User minimum requirements disabled - name: "Test User 2: Test disabled User Creation" @@ -117,9 +118,9 @@ full_name: "Test User 14: Test User Creation with not existing group as list" groups: - test + register: test_user_14_result ignore_errors: yes - - name: "Verify that the user creation failed due to non-existing group" ansible.builtin.assert: that: @@ -156,7 +157,7 @@ puzzle.opnsense.system_access_users: username: test_user_18 password: test_password_18 - authorizedkeys: test_authorized_key + authorizedkeys: test_authorized_key_mit_fabio full_name: "Test User 18: Test User Creation with authorizedkeys" # Test User with empty api_keys @@ -164,7 +165,9 @@ puzzle.opnsense.system_access_users: username: test_user_19 password: test_password_19 - apikeys: "" + apikeys: + - key: "" + secret: "" full_name: "Test User 19: Test User Creation with empty api_keys" register: api_keys_result @@ -180,7 +183,9 @@ puzzle.opnsense.system_access_users: username: test_user_20 password: test_password_20 - apikeys: "TEST_API_KEY" + apikeys: + - key: "TEST_API_KEY" + secret: "O0OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" full_name: "Test User 20: Test User Creation with too short api_keys" register: test_user_20_result ignore_errors: yes @@ -197,7 +202,9 @@ puzzle.opnsense.system_access_users: username: test_user_21 password: test_password_21 - apikeys: "TEST_API_KEY_WITH_RANDOM_CHARS_UNTIL_80_zo5Y3bUpOQFfbQnAOB6GqbHsPAP9Jqbjofnqu9xc" + apikeys: + - key: "TEST_API_KEY_WITH_RANDOM_CHARS_UNTIL_80_zo5Y3bUpOQFfbQnAOB6GqbHsPAP9Jqbjofnqu900" + secret: "O0OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" full_name: "Test User 21: Test User Creation with valid api_keys" register: api_keys_result @@ -208,20 +215,22 @@ - "'generated_apikeys' in api_keys_result" - api_keys_result.generated_apikeys | length > 0 - # Test User password escaping - - name: "Test User 22: Test password escaping" + # Test User password escaping with backslash + - name: "Test User 22: Test password escaping with backslash" puzzle.opnsense.system_access_users: username: test_user_22 password: test_password_22\ + full_name: "Test User 22: Test password escaping with backslash" shell: /bin/sh groups: - admins - # Test User password escaping - - name: "Test User 23: Test password escaping" + # Test User password escaping with dash + - name: "Test User 23: Test password escaping with dash" puzzle.opnsense.system_access_users: username: test_user_23 password: test_password_23' + full_name: "Test User 23: Test password escaping with dash" shell: /bin/sh groups: - admins @@ -239,3 +248,17 @@ that: - "'syntax error, unexpected identifier \"cost\", expecting \")\" in Command line code on line 1' not in (current_config.content | b64decode | string)" - "'syntax error, unexpected single-quoted string \",PASSWORD_BCRYPT,[ \", expecting \")\" in Command line code on line 1' not in (current_config.content | b64decode | string)" + + # Test User with apikeys as list + - name: "Test User 24: Test User Creation with apikeys as list" + puzzle.opnsense.system_access_users: + username: test_user_24 + password: test_password_24 + full_name: "Test User 24: Test User Creation with apikeys as list" + groups: + - admins + apikeys: + - key: "TEST_API_KEY_WITH_RANDOM_CHARS_UNTIL_80_zo5Y3bUpOQFfbQnAOB6GqbHsPAP9Jqbjofnqu900" + secret: "O0OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" + - key: "TEST_API_KEY_WITH_RANDOM_CHARS_UNTIL_80_zo5Y3bUpOQFfbQnAOB6GqbHsPAP9Jqbjofnqu911" + secret: "111Qc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" \ No newline at end of file diff --git a/plugins/module_utils/system_access_users_utils.py b/plugins/module_utils/system_access_users_utils.py index 63fc5f24..ad9187db 100644 --- a/plugins/module_utils/system_access_users_utils.py +++ b/plugins/module_utils/system_access_users_utils.py @@ -1,5 +1,6 @@ # Copyright: (c) 2024, Puzzle ITC, Kilian Soltermann # GNU General Public License v3.0+ (see LICENSE or https://www.gnu.org/licenses/gpl-3.0.txt) +# pylint: disable=too-many-lines """ This module manages user and group configurations within an OPNsense system. It provides @@ -26,9 +27,8 @@ """ -import dataclasses -from dataclasses import dataclass, asdict, fields, field -from typing import List, Optional, Dict, Any +from dataclasses import dataclass, asdict +from typing import List, Optional, Dict import base64 import os import binascii @@ -39,68 +39,109 @@ xml_utils, opnsense_utils, ) + from ansible_collections.puzzle.opnsense.plugins.module_utils.config_utils import ( OPNsenseModuleConfig, ) -from ansible_collections.puzzle.opnsense.plugins.module_utils.enum_utils import ListEnum -class OPNSenseGroupNotFoundError(Exception): +class OPNsenseCryptReturnError(Exception): """ - Exception raised when an OPNsense group is not found. + Exception raised when the return value of the instance is not what is expected """ -class OPNSenseNotValidBase64APIKeyError(Exception): +class OPNsenseGroupNotFoundError(Exception): """ - Exception raised when a not valid base32 api code is provided + Exception raised when an OPNsense group is not found. """ -class OPNSenseCryptReturnError(Exception): +class OPNsenseHashVerifyReturnError(Exception): """ Exception raised when the return value of the instance is not what is expected """ -class OPNSensePasswordVerifyReturnError(Exception): +class OPNsenseNotValidBase64APIKeyError(Exception): """ - Exception raised when the return value of the instance is not what is expected + Exception raised when a not valid base32 api code is provided """ -def password_verify(existing_user_password: str, password: Optional[str]) -> bool: +def hash_verify(existing_hashed_string: str, plain_string: Optional[str]) -> bool: """ - Verify if provided password matches the stored password using OPNsense's PHP command. + Verifies if a plain string matches an existing hashed string. Args: - existing_user_password (str): The hashed password stored in the XML config. - password (str): The plaintext password to verify. + existing_hashed_string (str): The existing hashed string to verify against. + plain_string (Optional[str]): The plain string to verify. Returns: - bool: True if passwords match, False otherwise. + bool: True if the plain string matches the hashed string, otherwise False. Raises: - OPNSensePasswordVerifyReturnError: If an error occurs during verification. + OPNsenseHashVerifyReturnError: If an error occurs during hash verification. """ - if password is None: + if plain_string is None: return False - # check if current password matches hash - password_matches = opnsense_utils.run_command( + # escape plain_string + escaped_string = plain_string.replace("\\", "\\\\").replace("'", "\\'") + + # check if current plain_string matches hash + hash_matches = opnsense_utils.run_command( php_requirements=[], - command=f"password_verify('{password}','{existing_user_password}');", + command=f"var_dump(password_verify('{escaped_string}','{existing_hashed_string}'));", ) - if password_matches.get("stderr"): - raise OPNSensePasswordVerifyReturnError("error encounterd verifying password") + if hash_matches.get("stderr"): + raise OPNsenseHashVerifyReturnError( + f"error encounterd verifying hash {hash_matches.get('stderr')}" + ) + + # if return code of hash_matches is true, it's a match + return hash_matches.get("stdout") == "bool(true)" + + +def apikeys_verify(existing_apikeys: List[Dict], apikeys: Optional[List[Dict]]) -> bool: + """ + Verifies if a list of API keys matches existing API keys. + + Args: + existing_apikeys (List[Dict]): List of existing API keys with 'key' and hashed 'secret'. + apikeys (List[Dict]): List of new API keys with 'key' and plain 'secret'. - # if return code of password_matches not equals 1, it's a match - if password_matches.get("stdout") != "1": + Returns: + bool: True if all new API keys match the existing ones, otherwise False. + """ + + if apikeys is None: + return False + + # if the List[Dict] matches, return True + if apikeys == existing_apikeys: return True - return False + existing_keys_and_secrets = { + apikey["key"]: apikey["secret"] for apikey in existing_apikeys + } + + for apikey in apikeys: + key = apikey["key"] + plain_secret = apikey["secret"] + + if key not in existing_keys_and_secrets: + # Key does not exist + return False + + if not hash_verify(existing_keys_and_secrets[key], plain_secret): + # Secret does not match + return False + + # If all keys and secrets match + return True @dataclass @@ -207,125 +248,45 @@ def remove_user(self, user: "User") -> None: self.member.remove(user.uid) -# pylint: disable=too-many-instance-attributes -@dataclass class User: """ - Represents a User entity with various attributes. - - Args: - name (str): The username of the user. - password (Optional[str]): The user's password. - scope (Optional[str]): The scope of the user, default is "User". - descr (Optional[str]): A description of the user, if available. - ipsecpsk (Optional[str]): IPsec pre-shared key, if applicable. - otp_seed (Optional[str]): OTP seed for two-factor authentication, if used. - shell (Optional[str]): The user's login shell, if specified. - uid (Optional[str]): The user's unique identifier. - disabled (bool): Whether the user is disabled (default is False). - full_name (Optional[str]): The user's full name, if available. - email (Optional[str]): The user's email address, if provided. - comment (Optional[str]): Additional comments or information about the user. - landing_page (Optional[str]): The landing page for the user, if specified. - expires (Optional[str]): The expiration date for the user, if set. - authorizedkeys (Optional[str]): Authorized SSH keys for the user, if applicable. - cert (Optional[str]): Certificate information for the user, if relevant. - apikeys (Optional[list[str]]): API key associated with the user, if any. Will be generated - if "" is provided - groupname (Optional[list[str]]): List of group names the user belongs to, if any. + A class representing a user with various attributes and functionalities. Methods: - __eq__(self, other): Compare two User objects, excluding sensitive fields. - to_etree(self): Convert User instance to an XML Element. - from_ansible_module_params(cls, params): Create a User from Ansible module parameters. - from_xml(element): Create a User from an XML Element. - __post_init__(self): Process post-initialization tasks. - set_otp_seed(self, otp_seed=None): Generate or encode OTP seed. - set_apikeys(self, apikeys=None): Generate or set API keys. - set_authorizedkeys(self, authorizedkeys=None): Encode authorized SSH keys. - - The User class is designed to represent user entities with various attributes commonly used in - system configurations. It provides methods for comparing, converting to XML, creating from - Ansible module parameters, and creating from XML representations. - """ - - name: str - password: Optional[str] = None - scope: Optional[str] = "User" - descr: Optional[str] = None - ipsecpsk: Optional[str] = None - otp_seed: Optional[str] = None - shell: str = "/sbin/nologin" - uid: Optional[str] = None - disabled: bool = False - full_name: Optional[str] = None - email: Optional[str] = None - comment: Optional[str] = None - landing_page: Optional[str] = None - expires: Optional[str] = None - authorizedkeys: Optional[str] = None - cert: Optional[str] = None # will be handled in seperate module - apikeys: Optional[List[str]] = None - groupname: Optional[List[str]] = None - - extra_attrs: Dict[str, Any] = field(default_factory=dict, repr=False) - - def __init__(self, **kwargs): - _attr_names: set[str] = {f.name for f in dataclasses.fields(self)} - _extra_attrs: dict = {} - for key, value in kwargs.items(): - if key in _attr_names: - setattr(self, key, value) - continue + __init__(**kwargs): + Initializes a User object with given attributes. - _extra_attrs[key] = value - self.extra_attrs = _extra_attrs + __eq__(other): + Compares two User objects for equality. - def __eq__(self, other) -> bool: - if not isinstance(other, User): - return False + generate_hashed_secret(secret: str) -> str: + Generates a hashed secret using the crypt function. - if not hasattr(self, "password") or not hasattr(other, "password"): - return False + _apikeys_from_xml(apikeys: dict) -> List[Dict]: + Converts API keys from an XML format to a list of dictionaries. - for _field in fields(self): - if _field.name in ["uid", "otp_seed", "apikeys"]: - continue + from_xml(element: Element) -> "User": + Converts an XML element into a User object. - if _field.name == "password" and not password_verify( - existing_user_password=getattr(other, _field.name), - password=self.password, - ): - return False + generate_apikeys(apikeys: List[dict] = None) -> List[dict]: + Generates API keys, encoding them as necessary. - # if value is not equal return False - if getattr(self, _field.name) != getattr(other, _field.name): - return False + set_otp_seed(otp_seed: str = None) -> str: + Sets or generates an OTP seed. - return True + encode_authorizedkeys(authorizedkeys: Optional[str] = None) -> Optional[str]: + Encodes authorized SSH keys as base64. - def set_otp_seed(self, otp_seed: str = None) -> str: - """ - Generates and returns a base32-encoded OTP seed. + to_etree() -> Element: + Converts the User object to an XML element. - Args: - otp_seed (str, optional): Existing OTP seed to encode (default: None). - - Returns: - str: Base32-encoded OTP seed. - - If no OTP seed is provided, a random seed is generated and encoded as base32. - """ - - if otp_seed is None: - otp_seed = os.urandom(20) - - return base64.b32encode(otp_seed.encode("utf-8")).decode("utf-8") + from_ansible_module_params(cls, params: dict) -> "User": + Creates a User instance from Ansible module parameters. + """ - def _generate_hashed_secret(self, secret: str) -> str: - """ - function to generate hashed secrets using crypt - """ + @staticmethod + def generate_hashed_secret(secret: str) -> str: + """Generates a hashed secret using the crypt function.""" # load requirements php_requirements = [] @@ -341,17 +302,17 @@ def _generate_hashed_secret(self, secret: str) -> str: # check if stderr returns value if hashed_secret_value.get("stderr"): - raise OPNSenseCryptReturnError("error encounterd while creating secret") + raise OPNsenseCryptReturnError("error encounterd while creating secret") # validate secret if ( hashed_secret_value.get("stdout").startswith("$6$") - and len(hashed_secret_value.get("stdout")) == 90 + and len(hashed_secret_value.get("stdout")) >= 90 ): return hashed_secret_value.get("stdout") # if validation fails, - raise OPNSenseCryptReturnError( + raise OPNsenseCryptReturnError( f""" validation of the secret failed! Secret must start with $6$ and have a min length of 90 @@ -359,42 +320,137 @@ def _generate_hashed_secret(self, secret: str) -> str: """ ) - def set_apikeys(self, apikeys: list = None) -> list: + def __init__(self, **kwargs): + # set default attributes + self.authorizedkeys = kwargs.get("authorizedkeys", None) + self.disabled = kwargs.get("disabled", False) + self.expires = kwargs.get("expires", None) + self.ipsecpsk = kwargs.get("ipsecpsk", None) + self.otp_seed = kwargs.get("otp_seed", None) + self.scope = kwargs.get("scope", "user") + + for key, value in kwargs.items(): + setattr(self, key, value) + + def __eq__(self, other) -> bool: + if not isinstance(other, User): + return False + + if self.__dict__.get("password") and other.__dict__.get("password"): + if not self.__dict__["password"] == other.__dict__["password"]: + if not hash_verify( + existing_hashed_string=self.__dict__["password"], + plain_string=other.__dict__["password"], + ): + return False + + if self.__dict__.get("apikeys") and other.__dict__.get("apikeys"): + if not apikeys_verify( + existing_apikeys=self.__dict__["apikeys"], + apikeys=other.__dict__["apikeys"], + ): + return False + + self_dict = { + k: v for k, v in self.__dict__.items() if k not in ["apikeys", "password"] + } + other_dict = { + k: v for k, v in other.__dict__.items() if k not in ["apikeys", "password"] + } + + return self_dict == other_dict + + @staticmethod + def _apikeys_from_xml(apikeys: dict) -> List[Dict]: + if isinstance(apikeys, str): + return [{}] + + api_keys = [] + if isinstance(apikeys, list): + for pair in apikeys: + item = pair.get("item", {}) + api_keys.append({"key": item.get("key"), "secret": item.get("secret")}) + elif apikeys.get("item"): + item = apikeys.get("item", {}) + api_keys.append({"key": item.get("key"), "secret": item.get("secret")}) + + return api_keys + + @staticmethod + def from_xml(element: Element) -> "User": """ - Generates a list of dictionaries, each containing a 'key' and a 'secret'. - If apikeys is provided, each element in apikeys is used as the 'key', - and a new 'secret' is generated. - If apikeys is not provided or is an empty list, a single 'key'-'secret' pair is generated. + Converts an XML element into a User object. - Args: - apikeys (list, optional): A list of strings to be used as the 'key' part of each pair. + Parameters: + element (Element): An XML element representing a user, with child elements + for each user attribute. Returns: - list: A list of dictionaries, where each dictionary has a 'key' and a 'secret'. + User: A User object initialized with the data extracted from the XML element. + + This method extracts data from an XML element, handling different data types appropriately, + such as converting single group names into a list and interpreting the + 'disabled' field as a boolean. """ - api_keys = [] + user_dict: dict = xml_utils.etree_to_dict(element)["user"] + + if "groupname" in user_dict and user_dict["groupname"] is None: + user_dict["groupname"] = [] - # Check if apikeys is None or contains an empty string - if apikeys is None or "" in apikeys: - key = base64.b64encode(os.urandom(60)).decode("utf-8") - secret = base64.b64encode(os.urandom(60)).decode("utf-8") - api_keys.append({"key": key, "secret": secret}) - else: - for api_key in apikeys: - secret = base64.b64encode(os.urandom(60)).decode("utf-8") + if "groupname" in user_dict and isinstance(user_dict["groupname"], str): + user_dict["groupname"] = [user_dict["groupname"]] + + # Handle 'disabled' element + user_dict["disabled"] = user_dict.get("disabled", "0") == "1" + + # handle apikeys element + if user_dict.get("apikeys"): + user_dict["apikeys"] = User._apikeys_from_xml(user_dict.get("apikeys")) + + return User(**user_dict) + + @staticmethod + def generate_apikeys(apikeys: List[dict] = None) -> List[dict]: + """Generates or validates API keys.""" + + api_keys: List[dict] = [] + + for apikey in apikeys: + # Check if key and secret are provided + if not apikey["key"]: + key = base64.b64encode(os.urandom(60)).decode("utf-8") + + if not apikey["secret"]: + secret = base64.b64encode(os.urandom(60)).decode("utf-8") + + api_keys.append({"key": key, "secret": secret}) + else: try: - base64.b64decode(api_key) - api_keys.append({"key": api_key, "secret": secret}) + base64.b64decode(apikey["key"]) + base64.b64decode(apikey["secret"]) + + api_keys.append(apikey) + except binascii.Error as binascii_error_message: - raise OPNSenseNotValidBase64APIKeyError( - f"The API key: {api_key} is not a valid base64 string. " + raise OPNsenseNotValidBase64APIKeyError( + f"The API key: {apikey} is not a valid base64 string. " f"Error: {str(binascii_error_message)}" ) from binascii_error_message return api_keys - def set_authorizedkeys(self, authorizedkeys: str = None) -> Optional[str]: + @staticmethod + def set_otp_seed(otp_seed: str = None) -> str: + """Sets or generates an OTP seed.""" + + if otp_seed is None or otp_seed == "": + return base64.b32encode(os.urandom(20).encode("utf-8")).decode("utf-8") + + return otp_seed + + @staticmethod + def encode_authorizedkeys(authorizedkeys: Optional[str] = None) -> Optional[str]: """ Encodes the authorized SSH keys as base32. @@ -414,27 +470,10 @@ def set_authorizedkeys(self, authorizedkeys: str = None) -> Optional[str]: return None def to_etree(self) -> Element: - """ - Converts the User instance to an XML Element. - - This method serializes the User object into an XML Element, filtering out - None or False values except for specific fields. It handles special cases - for fields that are instances of ListEnum by converting their values to - their corresponding enum values. Boolean values are converted to "1" for - True, and fields with None values are removed unless they are part of a - predefined list of exceptions. + """Converts the User object into an XML element.""" + user_dict: dict = self.__dict__.copy() - Returns: - Element: An XML Element representing the serialized User object, ready - for inclusion in an XML document. - - This approach ensures that the XML representation is compact and adheres to - the expected schema, with consideration for optional fields and data types. - """ - - user_dict: dict = asdict(self) - - for user_key, user_val in user_dict.copy().items(): + for user_key, user_val in list(user_dict.items()): if user_val is None and user_key in [ "expires", "ipsecpsk", @@ -444,12 +483,11 @@ def to_etree(self) -> Element: continue if isinstance(user_val, list) and user_key == "apikeys": - # Modify the apikeys directly into the list of items user_dict[user_key] = [ { "item": { key_name: ( - self._generate_hashed_secret(secret_value) + User.generate_hashed_secret(secret_value) if key_name == "secret" and not secret_value.startswith("$6$") else secret_value @@ -460,9 +498,6 @@ def to_etree(self) -> Element: for api_key_dict in user_val ] - if issubclass(type(user_val), ListEnum): - user_dict[user_key] = user_val.value - elif user_val is None or user_val is False: del user_dict[user_key] continue @@ -470,10 +505,6 @@ def to_etree(self) -> Element: elif isinstance(user_val, bool): user_dict[user_key] = "1" - for key, value in self.extra_attrs.items(): - user_dict[key] = value - - del user_dict["extra_attrs"] element: Element = xml_utils.dict_to_etree("user", user_dict)[0] return element @@ -503,8 +534,8 @@ def from_ansible_module_params(cls, params: dict) -> "User": "scope": params.get("scope"), "ipsecpsk": params.get("ipsecpsk"), "otp_seed": ( - cls.set_otp_seed(cls, otp_seed=params.get("otp_seed")) - if params.get("otp_seed") is not None + User.set_otp_seed(otp_seed=params.get("otp_seed")) + if params.get("otp_seed") else None ), "shell": params.get("shell"), @@ -514,59 +545,25 @@ def from_ansible_module_params(cls, params: dict) -> "User": "comment": params.get("comment"), "landing_page": params.get("landing_page"), "expires": params.get("expires"), - "groupname": params.get("groups"), - "authorizedkeys": ( - cls.set_authorizedkeys(cls, authorizedkeys=params.get("authorizedkeys")) - if params.get("authorizedkeys") - else None + "authorizedkeys": User.encode_authorizedkeys( + authorizedkeys=params.get("authorizedkeys", None) ), "cert": params.get("cert"), "apikeys": ( - cls.set_apikeys(cls, apikeys=params.get("apikeys")) + User.generate_apikeys(apikeys=params.get("apikeys")) if params.get("apikeys") else None ), } + if params.get("groups", None): + user_dict["groupname"] = params["groups"] user_dict = { key: value for key, value in user_dict.items() if value is not None } return cls(**user_dict) - @staticmethod - def from_xml(element: Element) -> "User": - """ - Converts an XML element into a User object. - - Parameters: - element (Element): An XML element representing a user, with child elements - for each user attribute. - - Returns: - User: A User object initialized with the data extracted from the XML element. - - This method extracts data from an XML element, handling different data types appropriately, - such as converting single group names into a list and interpreting the - 'disabled' field as a boolean. - """ - - user_dict: dict = xml_utils.etree_to_dict(element)["user"] - - if "groupname" in user_dict and isinstance(user_dict["groupname"], str): - user_dict["groupname"] = [user_dict["groupname"]] - - # Handle 'disabled' element - user_dict["disabled"] = user_dict.get("disabled", "0") == "1" - - if "apikeys" in user_dict and isinstance(user_dict["apikeys"], str): - apikeys_elements = user_dict["apikeys"].get("item", []) - if isinstance(apikeys_elements, dict): - apikeys_elements = [apikeys_elements] - user_dict["apikeys"] = [item["key"] for item in apikeys_elements] - - return User(**user_dict) - class UserSet(OPNsenseModuleConfig): """ @@ -712,9 +709,76 @@ def changed(self) -> bool: This property should be consulted before performing a save operation to avoid unnecessary writes to the system configuration when no changes have been made. """ - return self._load_users() != self._users or self._load_groups() != self._groups + def set_user_password(self, user: User) -> None: + """ + Sets the user's password using specified PHP and configuration functions. + """ + + # load requirements + php_requirements = self._config_maps["password"]["php_requirements"] + + # load requirements + configure_function_dict = self._config_maps["password"]["configure_functions"] + configure_function_key = "password" + configure_function = configure_function_dict[configure_function_key]["name"] + configure_params = configure_function_dict[configure_function_key][ + "configure_params" + ] + + # sanitize and escape password + escaped_password = user.password.replace("\\", "\\\\").replace("'", "\\'") + + # format parameters + formatted_params = [ + ( + param.replace("'password'", f"'{escaped_password}'") + if "password" in param + else param + ) + for param in configure_params + ] + + # set user password + user.password = opnsense_utils.run_function( + php_requirements=php_requirements, + configure_function=configure_function, + configure_params=formatted_params, + ).get("stdout") + + # since "password" is no longer needed and to avoid the configure_functions in + # the save() method, it can be popped + self._config_maps.pop("password") + + @staticmethod + def set_api_keys_secret(user: User) -> None: + """ + Sets the API keys for a user, hashing the 'secret' key if not already hashed. + + Args: + user (User): The user object containing API keys to be processed. + + Returns: + None + + The function iterates over the user's API keys and hashes the 'secret' key using + User.generate_hashed_secret if the key name is 'secret' and the value does not already start + with "$6$". Other keys and values are left unchanged. + """ + + user.apikeys = [ + { + key_name: ( + User.generate_hashed_secret(secret_value) + if key_name == "secret" and not secret_value.startswith("$6$") + else secret_value + ) + for key_name, secret_value in api_key_dict.items() + } + for api_key_dict in user.apikeys + ] + def _update_user_groups(self, user: User, existing_user: Optional[User] = None): """ Manages the association of a user with specified groups, either by updating the groups of an @@ -731,20 +795,28 @@ def _update_user_groups(self, user: User, existing_user: Optional[User] = None): the existing group memberships need to be updated. Raises: - OPNSenseGroupNotFoundError: If a specified group does not exist on the instance, this + OPNsenseGroupNotFoundError: If a specified group does not exist on the instance, this exception is raised, indicating the need for corrective action or error handling. """ target_user = existing_user if existing_user else user - if user.groupname is None or not hasattr(user, "groupname"): + if not hasattr(existing_user, "groupname") and not hasattr(user, "groupname"): + return + # ansible_user : no groupname + # ansible_user: groupname < existing_user + if ( + hasattr(existing_user, "groupname") and existing_user.groupname + ) and not hasattr(user, "groupname"): for existing_group in self._groups: if existing_group.check_if_user_in_group(target_user): existing_group.remove_user(target_user) - if target_user.groupname: + + if target_user.__dict__.get("groupname"): target_user.groupname.remove(existing_group.name) if not target_user.groupname: - target_user.groupname = None + target_user.groupname = [] + return # Exit the method after removing the user from all groups. # Convert groupname to a list if it's not already. @@ -764,51 +836,10 @@ def _update_user_groups(self, user: User, existing_user: Optional[User] = None): if not group_found: # Group was not found, raise an exception - raise OPNSenseGroupNotFoundError( + raise OPNsenseGroupNotFoundError( f"Group '{group_name}' not found on Instance" ) - def set_user_password(self, user: User) -> None: - """ - Sets the user's password using specified PHP and configuration functions. - """ - - # load requirements - php_requirements = self._config_maps["password"]["php_requirements"] - - # load requirements - configure_function_dict = self._config_maps["password"]["configure_functions"] - configure_function_key = next( - (key for key in configure_function_dict if key != "name"), None - ) - configure_function = configure_function_dict[configure_function_key]["name"] - configure_params = configure_function_dict[configure_function_key][ - "configure_params" - ] - - # sanitize and escape password - escaped_password = user.password.replace("\\", "\\\\").replace("'", "\\'") - - # format parameters - formatted_params = [ - ( - param.replace("'password'", f"'{escaped_password}'") - if "password" in param - else param - ) - for param in configure_params - ] - - # set user password - user.password = opnsense_utils.run_function( - php_requirements=php_requirements, - configure_function=configure_function, - configure_params=formatted_params, - ).get("stdout") - - # since "password" is no longer needed, it can be popped - self._config_maps.pop("password") - def add_or_update(self, user: User) -> None: """ Adds a new user to the system or updates an existing user's information, ensuring that group @@ -843,15 +874,19 @@ def add_or_update(self, user: User) -> None: next_uid: Element = self.get("uid") if existing_user: - if not password_verify( - existing_user_password=existing_user.password, password=user.password + if not hash_verify( + existing_hashed_string=existing_user.password, + plain_string=user.password, ): self.set_user_password(user) + else: + user.__dict__.pop("password") - # since we don't want the clear-type password to be set, - # and it is clear a update is not needed, we remove it from the update - if "password" in user.__dict__: - del user.__dict__["password"] + if hasattr(user, "apikeys"): + if not apikeys_verify( + existing_apikeys=existing_user.apikeys, apikeys=user.apikeys + ): + self.set_api_keys_secret(user) # Update groups if needed self._update_user_groups(user, existing_user) @@ -862,15 +897,17 @@ def add_or_update(self, user: User) -> None: return self.set_user_password(user) + # Assign UID if not set - if not user.uid: + if not hasattr(user, "uid"): user.uid = next_uid.text # Increase the next_uid self.set(value=str(int(next_uid.text) + 1), setting="uid") - if user.groupname: + if hasattr(user, "groupname"): # Update groups for the new user self._update_user_groups(user) + # Add the new user self._users.append(user) diff --git a/plugins/modules/system_access_users.py b/plugins/modules/system_access_users.py index 3bd91e6e..412939f6 100644 --- a/plugins/modules/system_access_users.py +++ b/plugins/modules/system_access_users.py @@ -66,7 +66,7 @@ type: str expires: description: - - The expiration date for the OPNsense user account. + - "Leave blank if the account shouldn't expire, otherwise enter the expiration date in the following format: mm/dd/yyyy" required: false type: str groups: @@ -80,7 +80,7 @@ - A list of apikeys for an OPNsense User. Generates new apikey if "" is provided. required: false type: list - elements: str + elements: dict otp_seed: description: - The otp_seed of a OPNsense user. @@ -153,8 +153,8 @@ from ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils import ( User, UserSet, - OPNSenseGroupNotFoundError, - OPNSenseNotValidBase64APIKeyError, + OPNsenseGroupNotFoundError, + OPNsenseNotValidBase64APIKeyError, ) @@ -184,7 +184,7 @@ def main(): "apikeys": { "type": "list", "required": False, - "elements": "str", + "elements": "dict", "no_log": False, }, "scope": {"type": "str", "required": False}, @@ -238,15 +238,8 @@ def main(): user_set.save() result["opnsense_configure_output"] = user_set.apply_settings() - if ansible_user.apikeys: - result["generated_apikeys"] = [] - for new_generated_api_key in ansible_user.apikeys: - result["generated_apikeys"].append( - f"key={new_generated_api_key['key']}" - ) - result["generated_apikeys"].append( - f"secret={new_generated_api_key['secret']}" - ) + if hasattr(ansible_user, "apikeys"): + result["generated_apikeys"] = ansible_user.apikeys for cmd_result in result["opnsense_configure_output"]: if cmd_result["rc"] != 0: @@ -256,10 +249,10 @@ def main(): ) module.exit_json(**result) - except OPNSenseGroupNotFoundError as opnsense_group_not_found_error_error_message: + except OPNsenseGroupNotFoundError as opnsense_group_not_found_error_error_message: module.fail_json(msg=str(opnsense_group_not_found_error_error_message)) except ( - OPNSenseNotValidBase64APIKeyError + OPNsenseNotValidBase64APIKeyError ) as opnsense_not_valid_base64_apikey_error_message: module.fail_json(msg=str(opnsense_not_valid_base64_apikey_error_message)) diff --git a/tests/unit/plugins/module_utils/test_system_access_users_utils.py b/tests/unit/plugins/module_utils/test_system_access_users_utils.py index 539f30b8..fda86cf9 100644 --- a/tests/unit/plugins/module_utils/test_system_access_users_utils.py +++ b/tests/unit/plugins/module_utils/test_system_access_users_utils.py @@ -12,13 +12,13 @@ from ansible_collections.puzzle.opnsense.plugins.module_utils import xml_utils from ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils import ( - Group, User, UserSet, - password_verify, - OPNSenseGroupNotFoundError, - OPNSenseCryptReturnError, - OPNSensePasswordVerifyReturnError, + Group, + OPNsenseCryptReturnError, + OPNsenseGroupNotFoundError, + OPNsenseHashVerifyReturnError, + hash_verify, ) from ansible_collections.puzzle.opnsense.plugins.module_utils.module_index import ( VERSION_MAP, @@ -83,7 +83,7 @@ AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy $6$$f8zJvXeCng1iaUCaq8KLvg4tJbGQ.qWKmfgcpytflpGF4AXc4U.N8/TiczM6fu741KBB2PwWUC0k7fzet8asq0 - + vagrantvagrant /bin/sh @@ -109,7 +109,6 @@ system 1999 0 - 1000 2004 2005 2006 @@ -122,7 +121,6 @@ test_group test_group system - 1000 2004 2021 2000 @@ -175,6 +173,7 @@ def test_user_from_xml(): assert test_user.otp_seed is None assert test_user.shell == "/bin/sh" assert test_user.uid == "1000" + assert not hasattr(test_user, "api_keys") def test_user_to_etree(): @@ -195,10 +194,11 @@ def test_user_to_etree(): assert xml_utils.elements_equal(test_element, orig_user) +# attribute apikeys tests def test_user_with_api_key_from_xml(): test_etree_opnsense: Element = ElementTree.fromstring(TEST_XML) - test_etree_user: Element = list(list(test_etree_opnsense)[0])[3] + test_etree_user: Element = list(list(test_etree_opnsense))[0][3] test_user: User = User.from_xml(test_etree_user) assert test_user.name == "test_user_1" @@ -209,11 +209,11 @@ def test_user_with_api_key_from_xml(): assert test_user.scope == "user" assert test_user.descr == "test_user_1" assert ( - test_user.apikeys["item"]["secret"] + test_user.apikeys[0]["secret"] == "$6$$f8zJvXeCng1iaUCaq8KLvg4tJbGQ.qWKmfgcpytflpGF4AXc4U.N8/TiczM6fu741KBB2PwWUC0k7fzet8asq0" ) assert ( - test_user.apikeys["item"]["key"] + test_user.apikeys[0]["key"] == "AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy" ) assert test_user.expires is None @@ -224,20 +224,23 @@ def test_user_with_api_key_from_xml(): assert test_user.uid == "1001" +# Function user_from_ansible_params attributes Unit-Tests + + def test_user_from_ansible_module_params_simple(sample_config_path): test_params: dict = { "username": "vagrant", "password": "vagrant", + "disabled": True, "scope": "user", "full_name": "vagrant box management", "shell": "/bin/sh", "uid": "1000", } - new_test_user: User = User.from_ansible_module_params(test_params) - assert new_test_user.name == "vagrant" assert new_test_user.password == "vagrant" + assert new_test_user.disabled == 1 assert new_test_user.scope == "user" assert new_test_user.descr == "vagrant box management" assert new_test_user.expires is None @@ -247,35 +250,54 @@ def test_user_from_ansible_module_params_simple(sample_config_path): assert new_test_user.uid == "1000" -@patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.version_utils.get_opnsense_version", - return_value="OPNsense Test", -) -@patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", - return_value=True, -) -@patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) -def test_user_set_load_simple_user( - mocked_version_utils: MagicMock, mock_password_verify: MagicMock, sample_config_path -): - with UserSet(sample_config_path) as user_set: - assert len(user_set._users) == 3 - user_set.save() +def test_new_user_from_ansible_module_params_required(sample_config_path): + test_params: dict = { + "username": "not_existing_user", + "password": "vagrant", + } + new_test_user: User = User.from_ansible_module_params(test_params) + assert new_test_user.name == "not_existing_user" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.expires is None + assert new_test_user.authorizedkeys is None + assert new_test_user.ipsecpsk is None + + +def test_new_user_from_ansible_module_params_additional_parameters(sample_config_path): + test_params: dict = { + "username": "not_existing_user", + "password": "vagrant", + "full_name": "new not_existing_user", + "email": "test@test.com", + "comment": "this is a test", + "landing_page": "/test.html", + "shell": "/bin/sh", + "expires": "11/17/2024", + } + new_test_user: User = User.from_ansible_module_params(test_params) + assert new_test_user.name == "not_existing_user" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.descr == "new not_existing_user" + assert new_test_user.email == "test@test.com" + assert new_test_user.comment == "this is a test" + assert new_test_user.landing_page == "/test.html" + assert new_test_user.shell == "/bin/sh" + assert new_test_user.expires == "11/17/2024" + assert new_test_user.authorizedkeys is None + assert new_test_user.ipsecpsk is None def test_group_from_xml(): test_etree_opnsense: Element = ElementTree.fromstring(TEST_XML) - test_etree_group: Element = list(list(test_etree_opnsense)[0])[5] test_group: Group = Group.from_xml(test_etree_group) - assert test_group.name == "admins" assert test_group.description == "System Administrators" assert test_group.scope == "system" assert test_group.member == [ "0", - "1000", "2004", "2005", "2006", @@ -286,44 +308,6 @@ def test_group_from_xml(): assert test_group.gid == "1999" -@patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.version_utils.get_opnsense_version", - return_value="OPNsense Test", -) -@patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", - return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", -) -@patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", - return_value=True, -) -@patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) -def test_user_set_add_group( - mocked_version_utils: MagicMock, - mock_set_password: MagicMock, - mock_password_verify: MagicMock, - sample_config_path, -): - with UserSet(sample_config_path) as user_set: - test_user: User = user_set.find(name="vagrant") - test_user.groupname = ["admins"] - - user_set.add_or_update(test_user) - - assert user_set.changed - - user_set.save() - - with UserSet(sample_config_path) as new_user_set: - new_test_user: User = new_user_set.find(name="vagrant") - - assert new_test_user.groupname == ["admins"] - assert "1000" in new_user_set._groups[0].member - - new_user_set.save() - - def test_user_from_ansible_module_params_with_group(sample_config_path): test_params: dict = { "username": "vagrant", @@ -334,9 +318,7 @@ def test_user_from_ansible_module_params_with_group(sample_config_path): "uid": "1000", "groups": ["admins"], } - new_test_user: User = User.from_ansible_module_params(test_params) - assert new_test_user.name == "vagrant" assert new_test_user.password == "vagrant" assert new_test_user.scope == "user" @@ -358,7 +340,7 @@ def test_user_from_ansible_module_params_with_group(sample_config_path): return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) @@ -377,22 +359,14 @@ def test_user_from_ansible_module_params_with_group_as_string( "uid": "1000", "groups": ["test_group"], } - with UserSet(sample_config_path) as user_set: test_user = User.from_ansible_module_params(test_params) - user_set.add_or_update(test_user) - assert user_set.changed user_set.save() - with UserSet(sample_config_path) as new_user_set: new_test_user = new_user_set.find(name="test") - - # Adjust the assertions based on the actual implementation of your User and UserSet classes - assert "test_group" in new_test_user.groupname - new_user_set.save() @@ -405,7 +379,7 @@ def test_user_from_ansible_module_params_with_group_as_string( return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) @@ -424,25 +398,18 @@ def test_user_from_ansible_module_params_with_multiple_groups_as_list( "uid": "1000", "groups": ["admins", "test_group"], } - with UserSet(sample_config_path) as user_set: test_user = User.from_ansible_module_params(test_params) - user_set.add_or_update(test_user) - assert user_set.changed user_set.save() - with UserSet(sample_config_path) as new_user_set: new_test_user = new_user_set.find(name="test") - # Adjust the assertions based on the actual implementation of your User and UserSet classes - assert ( "admins" in new_test_user.groupname and "test_group" in new_test_user.groupname ) - new_user_set.save() @@ -455,7 +422,7 @@ def test_user_from_ansible_module_params_with_multiple_groups_as_list( return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) @@ -473,20 +440,14 @@ def test_user_from_ansible_module_params_with_no_groups( "shell": "/bin/sh", "uid": "1000", } - with UserSet(sample_config_path) as user_set: test_user = User.from_ansible_module_params(test_params) - user_set.add_or_update(test_user) - assert user_set.changed user_set.save() - with UserSet(sample_config_path) as new_user_set: new_test_user = new_user_set.find(name="test") - assert new_test_user.name == "test" - new_user_set.save() @@ -499,7 +460,7 @@ def test_user_from_ansible_module_params_with_no_groups( return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) @@ -518,50 +479,54 @@ def test_user_from_ansible_module_params_with_not_existing_group( "uid": "1000", "groups": ["not_existing_group"], } - with UserSet(sample_config_path) as user_set: - with pytest.raises(OPNSenseGroupNotFoundError) as excinfo: + with pytest.raises(OPNsenseGroupNotFoundError) as excinfo: test_user = User.from_ansible_module_params(test_params) - user_set.add_or_update(test_user) - user_set.save() - assert "Group 'not_existing_group' not found on Instance" in str(excinfo.value) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.version_utils.get_opnsense_version", + return_value="OPNsense Test", +) @patch( "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.User.set_authorizedkeys", - return_value="3J35EY37QTNXFFEECJGZ32WVYQC5W4GZ", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", + return_value=True, ) -def test_user_from_ansible_module_params_with_authorizedkeys( - mock_set_set_authorizedkeys, mock_set_password, sample_config_path +@patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) +def test_user_from_ansible_module_params_single_group_removal( + mock_set_password, + mock_get_version, + mock_password_verify: MagicMock, + sample_config_path, ): - test_params: dict = { - "username": "vagrant", - "password": "vagrant", + test_params = { + "username": "test_user_23", + "password": "test_password_23", "scope": "user", - "full_name": "vagrant box management", + "full_name": "[ ANSIBLE ]", "shell": "/bin/sh", - "uid": "1000", - "authorizedkeys": "test_authorizedkey", + "uid": "2021", + "groups": [], } - - new_test_user: User = User.from_ansible_module_params(test_params) - - assert new_test_user.name == "vagrant" - assert new_test_user.password == "vagrant" - assert new_test_user.scope == "user" - assert new_test_user.descr == "vagrant box management" - assert new_test_user.expires is None - assert new_test_user.authorizedkeys == "3J35EY37QTNXFFEECJGZ32WVYQC5W4GZ" - assert new_test_user.ipsecpsk is None - assert new_test_user.shell == "/bin/sh" - assert new_test_user.uid == "1000" + with UserSet(sample_config_path) as user_set: + test_user = User.from_ansible_module_params(test_params) + user_set.add_or_update(test_user) + assert user_set.changed + user_set.save() + with UserSet(sample_config_path) as new_user_set: + all_groups = new_user_set._load_groups() + test_user: User = user_set.find(name="test_user_23") + test_group = all_groups[1] + assert "2021" not in test_group.member + assert not test_user.groupname + new_user_set.save() @patch( @@ -573,11 +538,11 @@ def test_user_from_ansible_module_params_with_authorizedkeys( return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) -def test_user_from_ansible_module_params_single_group_removal( +def test_user_from_ansible_module_params_single_group_removal_no_param( mock_set_password, mock_get_version, mock_password_verify: MagicMock, @@ -591,25 +556,20 @@ def test_user_from_ansible_module_params_single_group_removal( "shell": "/bin/sh", "uid": "2021", } - with UserSet(sample_config_path) as user_set: test_user = User.from_ansible_module_params(test_params) - + assert not hasattr(test_user, "groupname") + # assert len(test_user.groupname) == 0 user_set.add_or_update(test_user) - assert user_set.changed user_set.save() - - with UserSet(sample_config_path) as new_user_set: - all_groups = new_user_set._load_groups() - test_user: User = user_set.find(name="test_user_23") - - test_group = all_groups[1] - - assert "2021" not in test_group.member - assert not test_user.groupname - - new_user_set.save() + # with UserSet(sample_config_path) as new_user_set: + # all_groups = new_user_set._load_groups() + # test_user: User = user_set.find(name="test_user_23") + # test_group = all_groups[1] + # assert "2021" in test_group.member + # assert test_user.groupname + # new_user_set.save() @patch( @@ -621,7 +581,7 @@ def test_user_from_ansible_module_params_single_group_removal( return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", ) @patch( - "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.password_verify", + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", return_value=True, ) @patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) @@ -638,39 +598,275 @@ def test_user_from_ansible_module_params_multiple_group_removal( "full_name": "vagrant box management", "shell": "/bin/sh", "uid": "1000", + "groups": [], } - with UserSet(sample_config_path) as user_set: test_user = User.from_ansible_module_params(test_params) - user_set.add_or_update(test_user) - assert user_set.changed user_set.save() - with UserSet(sample_config_path) as new_user_set: all_groups = new_user_set._load_groups() - admin_group = all_groups[0] test_group = all_groups[1] - assert "1000" not in admin_group.member assert "1000" not in test_group.member - new_user_set.save() @patch( "ansible_collections.puzzle.opnsense.plugins.module_utils.opnsense_utils.run_function" ) -def test_generate_hashed_secret_success(mock_run_function): +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.version_utils.get_opnsense_version", + return_value="OPNsense Test", +) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", + return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", +) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", + return_value=True, +) +@patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) +def test_new_user_from_ansible_module_params_with_multiple_api_keys( + mock_set_password: MagicMock, + mock_get_version: MagicMock, + mock_password_verify: MagicMock, + mock_run_function: MagicMock, + sample_config_path, +): + test_params: dict = { + "username": "test_user_2", + "password": "test_password_2", + "scope": "user", + "full_name": "test_user_2", + "shell": "/bin/sh", + "uid": "1000", + "apikeys": [ + { + "key": "AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy", + "secret": "O6OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==", + }, + { + "key": "BMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy", + "secret": "lvU6lbOscmeunpWVHFfDlj4yF4DVp7uXOuH3770BkH0Tf4w4XFB/GKJw6+RzJPtoauKkHoPz/1y0NT0SRn3vqQ==", + }, + ], + } + mock_run_function.return_value = { "stdout": "$6$somerandomsalt$hashedsecretvalue1234567890123456789012345678901234567890123456789054583", "stderr": None, } + with UserSet(sample_config_path) as user_set: + new_test_user: User = User.from_ansible_module_params(test_params) + user_set.add_or_update(new_test_user) + user_set.save() + + assert new_test_user.name == "test_user_2" + assert new_test_user.password == "test_password_2" + assert new_test_user.scope == "user" + assert new_test_user.descr == "test_user_2" + assert new_test_user.expires is None + assert len(new_test_user.apikeys) == 2 + assert ( + new_test_user.apikeys[0]["key"] + == "AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy" + ) + assert ( + new_test_user.apikeys[0]["secret"] + == "O6OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" + ) + assert ( + new_test_user.apikeys[1]["key"] + == "BMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy" + ) + assert ( + new_test_user.apikeys[1]["secret"] + == "lvU6lbOscmeunpWVHFfDlj4yF4DVp7uXOuH3770BkH0Tf4w4XFB/GKJw6+RzJPtoauKkHoPz/1y0NT0SRn3vqQ==" + ) + assert new_test_user.ipsecpsk is None + assert new_test_user.shell == "/bin/sh" + assert new_test_user.uid == "1000" + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", + return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", +) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.User.encode_authorizedkeys", + return_value="3J35EY37QTNXFFEECJGZ32WVYQC5W4GZ", +) +def test_existing_user_from_ansible_module_params_with_multiple_api_keys( + mock_set_encode_authorizedkeys, mock_set_password, sample_config_path +): + test_params: dict = { + "username": "vagrant", + "password": "vagrant", + "scope": "user", + "full_name": "vagrant box management", + "shell": "/bin/sh", + "uid": "1000", + "apikeys": [ + { + "key": "AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy", + "secret": "O6OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==", + }, + { + "key": "BMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy", + "secret": "lvU6lbOscmeunpWVHFfDlj4yF4DVp7uXOuH3770BkH0Tf4w4XFB/GKJw6+RzJPtoauKkHoPz/1y0NT0SRn3vqQ==", + }, + ], + } + new_test_user: User = User.from_ansible_module_params(test_params) + assert new_test_user.name == "vagrant" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.descr == "vagrant box management" + assert new_test_user.expires is None + assert len(new_test_user.apikeys) == 2 + assert ( + new_test_user.apikeys[0]["key"] + == "AMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy" + ) + assert ( + new_test_user.apikeys[0]["secret"] + == "O6OQc0uNZ1w/ihSAVGyPbPzXmBhOt1hUpytSMU2NGdQfQWYlSDFtwY4xAquJtJLPQS0cN6conp59QGf5+icYvQ==" + ) + assert ( + new_test_user.apikeys[1]["key"] + == "BMC39xLYvfD7PyaemZrIVuaWBIdRQVS9NgEHFWzW7+xj0ExFY+07/Vz6HcmUVkJkjb8N0Cg7yEdESvNy" + ) + assert ( + new_test_user.apikeys[1]["secret"] + == "lvU6lbOscmeunpWVHFfDlj4yF4DVp7uXOuH3770BkH0Tf4w4XFB/GKJw6+RzJPtoauKkHoPz/1y0NT0SRn3vqQ==" + ) + assert new_test_user.ipsecpsk is None + assert new_test_user.shell == "/bin/sh" + assert new_test_user.uid == "1000" + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.version_utils.get_opnsense_version", + return_value="OPNsense Test", +) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.hash_verify", + return_value=True, +) +@patch.dict(in_dict=VERSION_MAP, values=TEST_VERSION_MAP, clear=True) +def test_user_set_load_simple_user( + mocked_version_utils: MagicMock, mock_password_verify: MagicMock, sample_config_path +): + with UserSet(sample_config_path) as user_set: + assert len(user_set._users) == 3 + user_set.save() + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", + return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", +) +def test_user_from_ansible_module_params_with_provided_otp_seed( + mock_set_password, sample_config_path +): + test_params: dict = { + "username": "vagrant", + "password": "vagrant", + "scope": "user", + "full_name": "vagrant box management", + "shell": "/bin/sh", + "uid": "1000", + "otp_seed": "some_seed", + } + new_test_user: User = User.from_ansible_module_params(test_params) + assert new_test_user.name == "vagrant" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.descr == "vagrant box management" + assert new_test_user.expires is None + assert new_test_user.otp_seed == "some_seed" + assert new_test_user.ipsecpsk is None + assert new_test_user.shell == "/bin/sh" + assert new_test_user.uid == "1000" + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", + return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", +) +def test_user_from_ansible_module_params_with_generated_otp_seed( + mock_set_password, sample_config_path +): + test_params: dict = { + "username": "new_user", + "password": "vagrant", + "scope": "user", + "full_name": "vagrant box management", + "shell": "/bin/sh", + "uid": "1000", + "otp_seed": "", + } + + new_test_user: User = User.from_ansible_module_params(test_params) + + assert new_test_user.name == "new_user" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.descr == "vagrant box management" + assert new_test_user.expires is None + assert new_test_user.otp_seed is None + assert new_test_user.ipsecpsk is None + assert new_test_user.shell == "/bin/sh" + assert new_test_user.uid == "1000" + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.UserSet.set_user_password", + return_value="$2y$10$1BvUdvwM.a.dJACwfeNfAOgNT6Cqc4cKZ2F6byyvY8hIK9I8fn36O", +) +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.system_access_users_utils.User.encode_authorizedkeys", + return_value="3J35EY37QTNXFFEECJGZ32WVYQC5W4GZ", +) +def test_user_from_ansible_module_params_with_authorizedkeys( + mock_set_encode_authorizedkeys, mock_set_password, sample_config_path +): + test_params: dict = { + "username": "vagrant", + "password": "vagrant", + "scope": "user", + "full_name": "vagrant box management", + "shell": "/bin/sh", + "uid": "1000", + "authorizedkeys": "test_authorizedkey", + } + new_test_user: User = User.from_ansible_module_params(test_params) + assert new_test_user.name == "vagrant" + assert new_test_user.password == "vagrant" + assert new_test_user.scope == "user" + assert new_test_user.descr == "vagrant box management" + assert new_test_user.expires is None + assert new_test_user.authorizedkeys == "3J35EY37QTNXFFEECJGZ32WVYQC5W4GZ" + assert new_test_user.ipsecpsk is None + assert new_test_user.shell == "/bin/sh" + assert new_test_user.uid == "1000" + + +@patch( + "ansible_collections.puzzle.opnsense.plugins.module_utils.opnsense_utils.run_function" +) +def test_generate_hashed_secret_success(mock_run_function): + mock_run_function.return_value = { + "stdout": "$6$somerandomsalt$hashedsecretvalue1234567890123456789012345678901234567890123456789054583", + "stderr": None, + } user = User(name="test", password="test") - result = user._generate_hashed_secret("password123") + result = user.generate_hashed_secret("password123") assert ( result == "$6$somerandomsalt$hashedsecretvalue1234567890123456789012345678901234567890123456789054583" @@ -685,11 +881,9 @@ def test_generate_hashed_secret_failure_invalid_hash(mock_run_function): "stdout": "$5$somerandomsalt$shortvalue", "stderr": None, } - user = User(name="test", password="test") - with pytest.raises(OPNSenseCryptReturnError) as excinfo: - user._generate_hashed_secret("password123") - + with pytest.raises(OPNsenseCryptReturnError) as excinfo: + user.generate_hashed_secret("password123") assert "validation of the secret failed!" in str(excinfo.value) @@ -698,11 +892,9 @@ def test_generate_hashed_secret_failure_invalid_hash(mock_run_function): ) def test_generate_hashed_secret_error_in_crypt(mock_run_function): mock_run_function.return_value = {"stdout": "", "stderr": "error in crypt function"} - user = User(name="test", password="test") - with pytest.raises(OPNSenseCryptReturnError) as excinfo: - user._generate_hashed_secret("password123") - + with pytest.raises(OPNsenseCryptReturnError) as excinfo: + user.generate_hashed_secret("password123") assert "error encounterd while creating secret" in str(excinfo.value) @@ -710,19 +902,16 @@ def test_generate_hashed_secret_error_in_crypt(mock_run_function): "ansible_collections.puzzle.opnsense.plugins.module_utils.opnsense_utils.run_command" ) def test_password_verify_returns_true_on_match(mock_run_command: MagicMock): - # Mock the return value of the run_command to simulate a password match mock_run_command.return_value = { - "stdout": "", + "stdout": "bool(true)", "stderr": None, } - # Call the function with test data - test_password_matches = password_verify( - password="test_password_1", - existing_user_password="$2y$11$pSYTZcD0o23JSfksEekwKOnWM1o3Ih9vp7OOQN.v35E1rag49cEc6", + test_password_matches = hash_verify( + plain_string="test_password_1", + existing_hashed_string="$2y$11$pSYTZcD0o23JSfksEekwKOnWM1o3Ih9vp7OOQN.v35E1rag49cEc6", ) - # Assert that the function returns True for a password match assert test_password_matches @@ -731,19 +920,16 @@ def test_password_verify_returns_true_on_match(mock_run_command: MagicMock): "ansible_collections.puzzle.opnsense.plugins.module_utils.opnsense_utils.run_command" ) def test_password_verify_returns_false_on_difference(mock_run_command: MagicMock): - # Mock the return value of the run_command to simulate a password match mock_run_command.return_value = { "stdout": "1", "stderr": None, } - # Call the function with test data - test_password_matches = password_verify( - password="test_password_1", - existing_user_password="$2y$11$pSYTZcD0o23JSfksEe1231345h9vp7OOQN.v35E1rag49cEc6", + test_password_matches = hash_verify( + plain_string="test_password_1", + existing_hashed_string="$2y$11$pSYTZcD0o23JSfksEe1231345h9vp7OOQN.v35E1rag49cEc6", ) - # Assert that the function returns True for a password match assert not test_password_matches @@ -751,21 +937,18 @@ def test_password_verify_returns_false_on_difference(mock_run_command: MagicMock @patch( "ansible_collections.puzzle.opnsense.plugins.module_utils.opnsense_utils.run_command" ) -def test_password_verify_returns_OPNSensePasswordVerifyReturnError( +def test_password_verify_returns_OPNsenseHashVerifyReturnError( mock_run_command: MagicMock, ): - # Mock the return value of the run_command to simulate a password match mock_run_command.return_value = { "stdout": None, "stderr": "this an error", } - - with pytest.raises(OPNSensePasswordVerifyReturnError) as excinfo: + with pytest.raises(OPNsenseHashVerifyReturnError) as excinfo: # Call the function with test data - test_password_matches = password_verify( - password="test_password_1", - existing_user_password="$2y$11$pSYTZcD0o23JSfksEekwKOnWM1o3Ih9vp7OOQN.v35E1rag49cEc6", + test_password_matches = hash_verify( + plain_string="test_password_1", + existing_hashed_string="$2y$11$pSYTZcD0o23JSfksEekwKOnWM1o3Ih9vp7OOQN.v35E1rag49cEc6", ) - - assert "error encounterd verifying passwor" in str(excinfo.value) + assert "error encounterd verifying hash" in str(excinfo.value) From d18887a36d1d1abd3ae68f0829eb8a49f9fe88c9 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna <33524186+DonGiovanni83@users.noreply.github.com> Date: Fri, 28 Jun 2024 15:10:57 +0200 Subject: [PATCH 3/7] Release 1.2.0 (#135) * Set version_added in HA module * Generate changelog --- CHANGELOG.rst | 26 ++++++++++++++++ changelogs/changelog.yaml | 30 +++++++++++++++++++ ...17-remove-user-shell-attribute-as-enum.yml | 3 -- ...ent-password-sanitation-before-hashing.yml | 7 ----- .../124-fix-password-module-index-entry.yml | 4 --- ...lude-plugin-interfaces-for-assignments.yml | 3 -- .../130-fix-apikeys-attribute-as-list.yml | 4 --- ...tion-handling-in-opnsenseconfigcontext.yml | 3 -- .../system_high_availability_settings.py | 1 + 9 files changed, 57 insertions(+), 24 deletions(-) delete mode 100644 changelogs/fragments/117-remove-user-shell-attribute-as-enum.yml delete mode 100644 changelogs/fragments/122-implement-password-sanitation-before-hashing.yml delete mode 100644 changelogs/fragments/124-fix-password-module-index-entry.yml delete mode 100644 changelogs/fragments/129-include-plugin-interfaces-for-assignments.yml delete mode 100644 changelogs/fragments/130-fix-apikeys-attribute-as-list.yml delete mode 100644 changelogs/fragments/132-improve-exception-handling-in-opnsenseconfigcontext.yml diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f9c0dbc1..dc5b1b34 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,32 @@ OPNsense Collection Release Notes .. contents:: Topics +v1.2.0 +====== + +Minor Changes +------------- + +- config_utils - Raise exceptions that occur within an OPNsenseConfigContext with traceback. +- system_access_users - Enhanced group removal handling + +Bugfixes +-------- + +- interfaces_assignments - Include plugin interfaces such as VLAN, VXLANs etc. in validations. +- module_index - Password entry now matches configure_function structure. +- system_access_users - Introduced password sanitization to fix parsing errors. +- system_access_users - Introduced password verification to fix passwords not being updated. +- system_access_users - Remove the UserLoginEnum type to prevent strict validation. +- system_access_users - Updated set_user_password dict calls in order to work with the newly introduced structure +- system_access_users - apikeys are changed if updated +- system_access_users - apikeys parameters are now passed as a list of dicts + +New Modules +----------- + +- system_high_availability_settings - Configure high availability settings + v1.1.1 ====== diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index b0bf0234..cf81ac60 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -37,3 +37,33 @@ releases: fragments: - 112-fix-user-extra-attributes-in-system_access_users.yml release_date: '2024-04-24' + 1.2.0: + changes: + bugfixes: + - interfaces_assignments - Include plugin interfaces such as VLAN, VXLANs etc. + in validations. + - module_index - Password entry now matches configure_function structure. + - system_access_users - Introduced password sanitization to fix parsing errors. + - system_access_users - Introduced password verification to fix passwords not + being updated. + - system_access_users - Remove the UserLoginEnum type to prevent strict validation. + - system_access_users - Updated set_user_password dict calls in order to work + with the newly introduced structure + - system_access_users - apikeys are changed if updated + - system_access_users - apikeys parameters are now passed as a list of dicts + minor_changes: + - config_utils - Raise exceptions that occur within an OPNsenseConfigContext + with traceback. + - system_access_users - Enhanced group removal handling + fragments: + - 117-remove-user-shell-attribute-as-enum.yml + - 122-implement-password-sanitation-before-hashing.yml + - 124-fix-password-module-index-entry.yml + - 129-include-plugin-interfaces-for-assignments.yml + - 130-fix-apikeys-attribute-as-list.yml + - 132-improve-exception-handling-in-opnsenseconfigcontext.yml + modules: + - description: Configure high availability settings + name: system_high_availability_settings + namespace: '' + release_date: '2024-06-28' diff --git a/changelogs/fragments/117-remove-user-shell-attribute-as-enum.yml b/changelogs/fragments/117-remove-user-shell-attribute-as-enum.yml deleted file mode 100644 index 5623066f..00000000 --- a/changelogs/fragments/117-remove-user-shell-attribute-as-enum.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -bugfixes: - - system_access_users - Remove the UserLoginEnum type to prevent strict validation. diff --git a/changelogs/fragments/122-implement-password-sanitation-before-hashing.yml b/changelogs/fragments/122-implement-password-sanitation-before-hashing.yml deleted file mode 100644 index 89a64aa3..00000000 --- a/changelogs/fragments/122-implement-password-sanitation-before-hashing.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -bugfixes: - - system_access_users - Introduced password sanitization to fix parsing errors. - - system_access_users - Introduced password verification to fix passwords not being updated. - -minor_changes: - - system_access_users - Enhanced group removal handling diff --git a/changelogs/fragments/124-fix-password-module-index-entry.yml b/changelogs/fragments/124-fix-password-module-index-entry.yml deleted file mode 100644 index 66ce263e..00000000 --- a/changelogs/fragments/124-fix-password-module-index-entry.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -bugfixes: - - module_index - Password entry now matches configure_function structure. - - system_access_users - Updated set_user_password dict calls in order to work with the newly introduced structure diff --git a/changelogs/fragments/129-include-plugin-interfaces-for-assignments.yml b/changelogs/fragments/129-include-plugin-interfaces-for-assignments.yml deleted file mode 100644 index f1a8b01f..00000000 --- a/changelogs/fragments/129-include-plugin-interfaces-for-assignments.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -bugfixes: - - interfaces_assignments - Include plugin interfaces such as VLAN, VXLANs etc. in validations. diff --git a/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml b/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml deleted file mode 100644 index 98e3d742..00000000 --- a/changelogs/fragments/130-fix-apikeys-attribute-as-list.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -bugfixes: - - system_access_users - apikeys parameters are now passed as a list of dicts - - system_access_users - apikeys are changed if updated \ No newline at end of file diff --git a/changelogs/fragments/132-improve-exception-handling-in-opnsenseconfigcontext.yml b/changelogs/fragments/132-improve-exception-handling-in-opnsenseconfigcontext.yml deleted file mode 100644 index 94c52b25..00000000 --- a/changelogs/fragments/132-improve-exception-handling-in-opnsenseconfigcontext.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -minor_changes: - - config_utils - Raise exceptions that occur within an OPNsenseConfigContext with traceback. diff --git a/plugins/modules/system_high_availability_settings.py b/plugins/modules/system_high_availability_settings.py index b428d475..eaff98c4 100644 --- a/plugins/modules/system_high_availability_settings.py +++ b/plugins/modules/system_high_availability_settings.py @@ -16,6 +16,7 @@ author: - Yoan Müller (@LuminatiHD) module: system_high_availability_settings +version_added: "1.2.0" short_description: Configure high availability settings description: - Module to configure high availability system settings From a17fc2c1a2ab692e0634f6065c2953837e1463b8 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna <33524186+DonGiovanni83@users.noreply.github.com> Date: Fri, 28 Jun 2024 15:19:06 +0200 Subject: [PATCH 4/7] Update galaxy meta (#136) --- galaxy.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/galaxy.yml b/galaxy.yml index c96e3c32..6f25b839 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -2,7 +2,7 @@ namespace: puzzle name: opnsense -version: 1.1.1 +version: 1.2.0 readme: README.md authors: - Lukas Grimm (github.com/ombre8) @@ -10,6 +10,7 @@ authors: - Philipp Matti (github.com/acteru) - Reto Kupferschmid (github.com/rekup) - Kilian Soltermann (github.com/killuuuhh) + - Yoan Müller (github.com/LuminatiHD) description: Ansible collection for OPNsense license_file: LICENSE tags: From 5f26407eb7fac4b3c284f4f1435e80201859f681 Mon Sep 17 00:00:00 2001 From: Tim Herren Date: Wed, 10 Jul 2024 09:38:27 +0200 Subject: [PATCH 5/7] fix ip-address key and fix intendation (#141) --- plugins/modules/firewall_rules.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/modules/firewall_rules.py b/plugins/modules/firewall_rules.py index ad5794b8..3d484a10 100644 --- a/plugins/modules/firewall_rules.py +++ b/plugins/modules/firewall_rules.py @@ -274,7 +274,7 @@ required: false type: str state: - description: Weather rule should be added or removed. + description: Whether rule should be added or removed. required: false type: str default: present @@ -290,7 +290,7 @@ interface: lan source: destination: - port: 22 + port: 22 action: block - name: Allow all access from RFC1918 networks to this host @@ -298,7 +298,7 @@ interface: lan action: pass source: - ip: 192.168.0.0/16 + address: 192.168.0.0/16 destination: ''' From 556290b550a254b94818b3306a5cea900966cc66 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna <33524186+DonGiovanni83@users.noreply.github.com> Date: Mon, 5 Aug 2024 10:40:23 +0200 Subject: [PATCH 6/7] Add extra attributes to firewall rules (#143) * Add extra attributes to firewall rules * Add docstrings to test * Remove redundant idempodene tests * Use loops in fw rules converge for better readability * Remove unproper supported statetype * Add persistence test for extra attrs to fw rules * Set statetype as extra attr * Fix test linting * Add changelog fragment --- ...add-extra-attributes-to-firewall-rules.yml | 3 + molecule/firewall_rules/converge.yml | 1242 ++++------------- plugins/module_utils/firewall_rules_utils.py | 39 +- .../module_utils/test_firewall_rules_utils.py | 53 +- 4 files changed, 309 insertions(+), 1028 deletions(-) create mode 100644 changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml diff --git a/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml b/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml new file mode 100644 index 00000000..615c7d6b --- /dev/null +++ b/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - firewall_rules_utils - Handle additional XML attributes fo the firewall rule objects from the config. \ No newline at end of file diff --git a/molecule/firewall_rules/converge.yml b/molecule/firewall_rules/converge.yml index 5255d0f0..fbda4709 100644 --- a/molecule/firewall_rules/converge.yml +++ b/molecule/firewall_rules/converge.yml @@ -3,1037 +3,269 @@ hosts: all become: true tasks: - # Test basic functionality with different actions - - name: "Action: Test pass action" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - description: "New Test pass Rule" - source: - destination: - - - name: "Action: Test block action" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'block' - description: "New Test block Rule" - - - name: "Action: Test reject action" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'reject' - description: "New Test reject Rule" - - # Test basic functionality of the disabled button - - name: "Disabled: Test disabled button" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - description: "New Test disabled pass Rule" - disabled: true - - # Test basic functionality of the disabled quick button - - name: "Quick: Test pass Rule with quick disabled" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - quick: false - description: "New Test pass Rule with quick disabled" - - # Test different Interfaces - - name: "Interface: Test pass Rule" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - description: "New Test pass Rule of Interface lan" - - - name: "Interface: Test pass Rule" - puzzle.opnsense.firewall_rules: - interface: 'lo0' - action: 'pass' - description: "New Test pass Rule of Interface Loopback" - - - name: "Interface: Test pass Rule" - puzzle.opnsense.firewall_rules: - interface: 'openvpn' - action: 'pass' - description: "New Test pass Rule of Interface OpenVPN" - - - name: "Interface: Test pass Rule" - puzzle.opnsense.firewall_rules: - interface: 'opt2' - action: 'pass' - description: "New Test pass Rule of Interface VAGRANT" - - # Test different Directions - - name: "Direction: Test pass Rule with Direction in" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - direction: in - description: "New Test pass Rule with Direction in" - - - name: "Direction: Test pass Rule with Direction out" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - direction: out - description: "New Test pass Rule with Direction out" - - # Test different IPProtocols - - name: "IPProtocol: Test pass Rule with IPProtocol IPv4" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - ipprotocol: 'inet' - description: "New Test pass Rule with IPv4" - - - name: "IPProtocol: Test pass Rule with IPProtocol IPv6" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - ipprotocol: 'inet6' - description: "New Test pass Rule with IPProtocol IPv6" - - - name: "IPProtocol: Test pass Rule with IPProtocol IPv4 + IPv6" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - ipprotocol: 'inet46' - description: "New Test pass Rule with IPProtocol IPv4 + IPv6" - - # Test different Protocols - - name: "Protocol: Test pass Rule with Protocol any" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'any' - description: "New Test pass Rule with Protocol any" - - - name: "Protocol: Test pass Rule with Protocol tcp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'tcp' - description: "New Test pass Rule with Protocol tcp" - - - name: "Protocol: Test pass Rule with Protocol udp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'udp' - description: "New Test pass Rule with Protocol udp" - - - name: "Protocol: Test pass Rule with Protocol tcp/udp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'tcp/udp' - description: "New Test pass Rule with Protocol tcp/udp" - - - name: "Protocol: Test pass Rule with Protocol icmp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'icmp' - description: "New Test pass Rule with Protocol icmp" - - - name: "Protocol: Test pass Rule with Protocol esp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'esp' - description: "New Test pass Rule with Protocol esp" - - - name: "Protocol: Test pass Rule with Protocol ah" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ah' - description: "New Test pass Rule with Protocol ah" - - - name: "Protocol: Test pass Rule with Protocol gre" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'gre' - description: "New Test pass Rule with Protocol gre" - - - name: "Protocol: Test pass Rule with Protocol igmp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'igmp' - description: "New Test pass Rule with Protocol igmp" - - - name: "Protocol: Test pass Rule with Protocol pim" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pim' - description: "New Test pass Rule with Protocol pim" - - - name: "Protocol: Test pass Rule with Protocol ospf" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ospf' - description: "New Test pass Rule with Protocol ospf" - - - name: "Protocol: Test pass Rule with Protocol ggp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ggp' - description: "New Test pass Rule with Protocol ggp" - - - name: "Protocol: Test pass Rule with Protocol ipencap" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipencap' - description: "New Test pass Rule with Protocol ipencap" - - - name: "Protocol: Test pass Rule with Protocol st2" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'st2' - description: "New Test pass Rule with Protocol st2" - - - name: "Protocol: Test pass Rule with Protocol cbt" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'cbt' - description: "New Test pass Rule with Protocol cbt" - - - name: "Protocol: Test pass Rule with Protocol egp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'egp' - description: "New Test pass Rule with Protocol egp" - - - name: "Protocol: Test pass Rule with Protocol igp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'igp' - description: "New Test pass Rule with Protocol igp" - - - name: "Protocol: Test pass Rule with Protocol bbn-rcc" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'bbn-rcc' - description: "New Test pass Rule with Protocol bbn-rcc" - - - name: "Protocol: Test pass Rule with Protocol nvp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'nvp' - description: "New Test pass Rule with Protocol nvp" - - - name: "Protocol: Test pass Rule with Protocol pup" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pup' - description: "New Test pass Rule with Protocol pup" - - - name: "Protocol: Test pass Rule with Protocol argus" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'argus' - description: "New Test pass Rule with Protocol argus" - - - name: "Protocol: Test pass Rule with Protocol emcon" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'emcon' - description: "New Test pass Rule with Protocol emcon" - - - name: "Protocol: Test pass Rule with Protocol xnet" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'xnet' - description: "New Test pass Rule with Protocol xnet" - - - name: "Protocol: Test pass Rule with Protocol chaos" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'chaos' - description: "New Test pass Rule with Protocol chaos" - - - name: "Protocol: Test pass Rule with Protocol mux" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'mux' - description: "New Test pass Rule with Protocol mux" - - - name: "Protocol: Test pass Rule with Protocol dcn" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'dcn' - description: "New Test pass Rule with Protocol dcn" - - - name: "Protocol: Test pass Rule with Protocol hmp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'hmp' - description: "New Test pass Rule with Protocol hmp" - - - name: "Protocol: Test pass Rule with Protocol prm" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'prm' - description: "New Test pass Rule with Protocol prm" - - - name: "Protocol: Test pass Rule with Protocol xns-idp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'xns-idp' - description: "New Test pass Rule with Protocol xns-idp" - - - name: "Protocol: Test pass Rule with Protocol trunk-1" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'trunk-1' - description: "New Test pass Rule with Protocol trunk-1" - - - name: "Protocol: Test pass Rule with Protocol trunk-2" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'trunk-2' - description: "New Test pass Rule with Protocol trunk-2" - - - name: "Protocol: Test pass Rule with Protocol leaf-1" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'leaf-1' - description: "New Test pass Rule with Protocol leaf-1" - - - name: "Protocol: Test pass Rule with Protocol leaf-2" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'leaf-2' - description: "New Test pass Rule with Protocol leaf-2" - - - name: "Protocol: Test pass Rule with Protocol rdp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'rdp' - description: "New Test pass Rule with Protocol rdp" - - - name: "Protocol: Test pass Rule with Protocol irtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'irtp' - description: "New Test pass Rule with Protocol irtp" - - - name: "Protocol: Test pass Rule with Protocol iso-tp4" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'iso-tp4' - description: "New Test pass Rule with Protocol iso-tp4" - - - name: "Protocol: Test pass Rule with Protocol netblt" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'netblt' - description: "New Test pass Rule with Protocol netblt" - - - name: "Protocol: Test pass Rule with Protocol mfe-nsp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'mfe-nsp' - description: "New Test pass Rule with Protocol mfe-nsp" - - - name: "Protocol: Test pass Rule with Protocol merit-inp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'merit-inp' - description: "New Test pass Rule with Protocol merit-inp" - - - name: "Protocol: Test pass Rule with Protocol dccp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'dccp' - description: "New Test pass Rule with Protocol dccp" - - - name: "Protocol: Test pass Rule with Protocol 3pc" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: '3pc' - description: "New Test pass Rule with Protocol 3pc" - - - name: "Protocol: Test pass Rule with Protocol idpr" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'idpr' - description: "New Test pass Rule with Protocol idpr" - - - name: "Protocol: Test pass Rule with Protocol xtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'xtp' - description: "New Test pass Rule with Protocol xtp" - - - name: "Protocol: Test pass Rule with Protocol ddp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ddp' - description: "New Test pass Rule with Protocol ddp" - - - name: "Protocol: Test pass Rule with Protocol idpr-cmtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'idpr-cmtp' - description: "New Test pass Rule with Protocol idpr-cmtp" - - - name: "Protocol: Test pass Rule with Protocol tp++" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'tp++' - description: "New Test pass Rule with Protocol tp++" - - - name: "Protocol: Test pass Rule with Protocol il" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'il' - description: "New Test pass Rule with Protocol il" - - - name: "Protocol: Test pass Rule with Protocol ipv6" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipv6' - description: "New Test pass Rule with Protocol ipv6" - - - name: "Protocol: Test pass Rule with Protocol sdrp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sdrp' - description: "New Test pass Rule with Protocol sdrp" - - - name: "Protocol: Test pass Rule with Protocol idrp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'idrp' - description: "New Test pass Rule with Protocol idrp" - - - name: "Protocol: Test pass Rule with Protocol rsvp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'rsvp' - description: "New Test pass Rule with Protocol rsvp" - - - name: "Protocol: Test pass Rule with Protocol dsr" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'dsr' - description: "New Test pass Rule with Protocol dsr" - - - name: "Protocol: Test pass Rule with Protocol bna" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'bna' - description: "New Test pass Rule with Protocol bna" - - - name: "Protocol: Test pass Rule with Protocol i-nlsp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'i-nlsp' - description: "New Test pass Rule with Protocol i-nlsp" - - - name: "Protocol: Test pass Rule with Protocol swipe" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'swipe' - description: "New Test pass Rule with Protocol swipe" - - - name: "Protocol: Test pass Rule with Protocol narp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'narp' - description: "New Test pass Rule with Protocol narp" - - - name: "Protocol: Test pass Rule with Protocol mobile" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'mobile' - description: "New Test pass Rule with Protocol mobile" - - - name: "Protocol: Test pass Rule with Protocol tlsp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'tlsp' - description: "New Test pass Rule with Protocol tlsp" - - - - name: "Protocol: Test pass Rule with Protocol skip" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'skip' - description: "New Test pass Rule with Protocol skip" - - - name: "Protocol: Test pass Rule with Protocol ipv6-icmp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipv6-icmp' - description: "New Test pass Rule with Protocol ipv6-icmp" - - - name: "Protocol: Test pass Rule with Protocol cftp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'cftp' - description: "New Test pass Rule with Protocol cftp" - - - name: "Protocol: Test pass Rule with Protocol sat-expak" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sat-expak' - description: "New Test pass Rule with Protocol sat-expak" - - - name: "Protocol: Test pass Rule with Protocol kryptolan" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'kryptolan' - description: "New Test pass Rule with Protocol kryptolan" - - - name: "Protocol: Test pass Rule with Protocol rvd" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'rvd' - description: "New Test pass Rule with Protocol rvd" - - - name: "Protocol: Test pass Rule with Protocol ippc" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ippc' - description: "New Test pass Rule with Protocol ippc" - - - name: "Protocol: Test pass Rule with Protocol sat-mon" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sat-mon' - description: "New Test pass Rule with Protocol sat-mon" - - - name: "Protocol: Test pass Rule with Protocol visa" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'visa' - description: "New Test pass Rule with Protocol visa" - - - name: "Protocol: Test pass Rule with Protocol ipcv" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipcv' - description: "New Test pass Rule with Protocol ipcv" - - - name: "Protocol: Test pass Rule with Protocol cpnx" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'cpnx' - description: "New Test pass Rule with Protocol cpnx" - - - name: "Protocol: Test pass Rule with Protocol cphb" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'cphb' - description: "New Test pass Rule with Protocol cphb" - - - name: "Protocol: Test pass Rule with Protocol wsn" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'wsn' - description: "New Test pass Rule with Protocol wsn" - - - name: "Protocol: Test pass Rule with Protocol pvp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pvp' - description: "New Test pass Rule with Protocol pvp" - - - name: "Protocol: Test pass Rule with Protocol br-sat-mon" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'br-sat-mon' - description: "New Test pass Rule with Protocol br-sat-mon" - - - name: "Protocol: Test pass Rule with Protocol sun-nd" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sun-nd' - description: "New Test pass Rule with Protocol sun-nd" - - - name: "Protocol: Test pass Rule with Protocol wb-mon" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'wb-mon' - description: "New Test pass Rule with Protocol wb-mon" - - - name: "Protocol: Test pass Rule with Protocol wb-expak" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'wb-expak' - description: "New Test pass Rule with Protocol wb-expak" - - - name: "Protocol: Test pass Rule with Protocol iso-ip" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'iso-ip' - description: "New Test pass Rule with Protocol iso-ip" - - - name: "Protocol: Test pass Rule with Protocol vmtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'vmtp' - description: "New Test pass Rule with Protocol vmtp" - - - name: "Protocol: Test pass Rule with Protocol secure-vmtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'secure-vmtp' - description: "New Test pass Rule with Protocol secure-vmtp" - - - name: "Protocol: Test pass Rule with Protocol vines" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'vines' - description: "New Test pass Rule with Protocol vines" - - - name: "Protocol: Test pass Rule with Protocol ttp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ttp' - description: "New Test pass Rule with Protocol ttp" - - - name: "Protocol: Test pass Rule with Protocol nsfnet-igp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'nsfnet-igp' - description: "New Test pass Rule with Protocol nsfnet-igp" - - - name: "Protocol: Test pass Rule with Protocol dgp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'dgp' - description: "New Test pass Rule with Protocol dgp" - - - name: "Protocol: Test pass Rule with Protocol tcf" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'tcf' - description: "New Test pass Rule with Protocol tcf" - - - name: "Protocol: Test pass Rule with Protocol eigrp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'eigrp' - description: "New Test pass Rule with Protocol eigrp" - - - name: "Protocol: Test pass Rule with Protocol sprite-rpc" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sprite-rpc' - description: "New Test pass Rule with Protocol sprite-rpc" - - - name: "Protocol: Test pass Rule with Protocol larp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'larp' - description: "New Test pass Rule with Protocol larp" - - - name: "Protocol: Test pass Rule with Protocol mtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'mtp' - description: "New Test pass Rule with Protocol mtp" - - - name: "Protocol: Test pass Rule with Protocol ax.25" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ax.25' - description: "New Test pass Rule with Protocol ax.25" - - - name: "Protocol: Test pass Rule with Protocol ipip" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipip' - description: "New Test pass Rule with Protocol ipip" - - - name: "Protocol: Test pass Rule with Protocol micp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'micp' - description: "New Test pass Rule with Protocol micp" - - - name: "Protocol: Test pass Rule with Protocol scc-sp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'scc-sp' - description: "New Test pass Rule with Protocol scc-sp" - - - name: "Protocol: Test pass Rule with Protocol etherip" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'etherip' - description: "New Test pass Rule with Protocol etherip" - - - name: "Protocol: Test pass Rule with Protocol encap" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'encap' - description: "New Test pass Rule with Protocol encap" - - - name: "Protocol: Test pass Rule with Protocol gmtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'gmtp' - description: "New Test pass Rule with Protocol gmtp" - - - name: "Protocol: Test pass Rule with Protocol ifmp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ifmp' - description: "New Test pass Rule with Protocol ifmp" - - - name: "Protocol: Test pass Rule with Protocol pnni" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pnni' - description: "New Test pass Rule with Protocol pnni" - - - name: "Protocol: Test pass Rule with Protocol aris" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'aris' - description: "New Test pass Rule with Protocol aris" - - - name: "Protocol: Test pass Rule with Protocol scps" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'scps' - description: "New Test pass Rule with Protocol scps" - - - name: "Protocol: Test pass Rule with Protocol qnx" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'qnx' - description: "New Test pass Rule with Protocol qnx" - - - name: "Protocol: Test pass Rule with Protocol a/n" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'a/n' - description: "New Test pass Rule with Protocol a/n" - - - name: "Protocol: Test pass Rule with Protocol ipcomp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipcomp' - description: "New Test pass Rule with Protocol ipcomp" - - - name: "Protocol: Test pass Rule with Protocol snp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'snp' - description: "New Test pass Rule with Protocol snp" - - - name: "Protocol: Test pass Rule with Protocol compaq-peer" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'compaq-peer' - description: "New Test pass Rule with Protocol compaq-peer" - - - name: "Protocol: Test pass Rule with Protocol ipx-in-ip" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ipx-in-ip' - description: "New Test pass Rule with Protocol ipx-in-ip" - - - name: "Protocol: Test pass Rule with Protocol carp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'carp' - description: "New Test pass Rule with Protocol carp" - - - name: "Protocol: Test pass Rule with Protocol pgm" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pgm' - description: "New Test pass Rule with Protocol pgm" - - - name: "Protocol: Test pass Rule with Protocol l2tp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'l2tp' - description: "New Test pass Rule with Protocol l2tp" - - - name: "Protocol: Test pass Rule with Protocol ddx" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ddx' - description: "New Test pass Rule with Protocol ddx" - - - name: "Protocol: Test pass Rule with Protocol iatp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'iatp' - description: "New Test pass Rule with Protocol iatp" - - - name: "Protocol: Test pass Rule with Protocol stp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'stp' - description: "New Test pass Rule with Protocol stp" - - - name: "Protocol: Test pass Rule with Protocol srp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'srp' - description: "New Test pass Rule with Protocol srp" - - - name: "Protocol: Test pass Rule with Protocol uti" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'uti' - description: "New Test pass Rule with Protocol uti" - - - name: "Protocol: Test pass Rule with Protocol smp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'smp' - description: "New Test pass Rule with Protocol smp" - - - name: "Protocol: Test pass Rule with Protocol sm" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sm' - description: "New Test pass Rule with Protocol sm" - - - name: "Protocol: Test pass Rule with Protocol ptp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'ptp' - description: "New Test pass Rule with Protocol ptp" - - - name: "Protocol: Test pass Rule with Protocol isis" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'isis' - description: "New Test pass Rule with Protocol isis" - - - name: "Protocol: Test pass Rule with Protocol crtp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'crtp' - description: "New Test pass Rule with Protocol crtp" - - - name: "Protocol: Test pass Rule with Protocol crudp" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'crudp' - description: "New Test pass Rule with Protocol crudp" - - - name: "Protocol: Test pass Rule with Protocol sps" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'sps' - description: "New Test pass Rule with Protocol sps" + # this rule should have the extra attributes 'create' updated' + # 'tag' 'statetype' and 'disablereplyto' persisted until the end + # of the execution of this playbook. + - name: "Create rule with 'disablereplyto' and a tag" + ansible.builtin.blockinfile: + path: /conf/config.xml + insertbefore: "" + content: | + + pass + lan + inet + test-local-tag + keep state + in + 1 + 1 + [ ANSIBLE ] - Test extra attributes + + 1 + + + 1 + + + vagrant@10.0.2.2 + + /firewall_rules_edit.php made changes + + + vagrant@10.0.2.2 + + /firewall_rules_edit.php made changes + + - - name: "Protocol: Test pass Rule with Protocol pipe" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'pipe' - description: "New Test pass Rule with Protocol pipe" - - name: "Protocol: Test pass Rule with Protocol sctp" + # Test basic functionality with different actions + - name: "Action: Test pass action" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'sctp' - description: "New Test pass Rule with Protocol sctp" + description: "New Test pass Rule" + source: + destination: - - name: "Protocol: Test pass Rule with Protocol fc" + - name: "Action: Test block action" puzzle.opnsense.firewall_rules: interface: 'lan' - action: 'pass' - protocol: 'fc' - description: "New Test pass Rule with Protocol fc" + action: 'block' + description: "New Test block Rule" - - name: "Protocol: Test pass Rule with Protocol rsvp-e2e-ignore" + - name: "Action: Test reject action" puzzle.opnsense.firewall_rules: interface: 'lan' - action: 'pass' - protocol: 'rsvp-e2e-ignore' - description: "New Test pass Rule with Protocol rsvp-e2e-ignore" + action: 'reject' + description: "New Test reject Rule" - - name: "Protocol: Test pass Rule with Protocol udplite" + # Test basic functionality of the disabled button + - name: "Disabled: Test disabled button" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'udplite' - description: "New Test pass Rule with Protocol udplite" + description: "New Test disabled pass Rule" + disabled: true - - name: "Protocol: Test pass Rule with Protocol mpls-in-ip" + # Test basic functionality of the disabled quick button + - name: "Quick: Test pass Rule with quick disabled" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'mpls-in-ip' - description: "New Test pass Rule with Protocol mpls-in-ip" + quick: false + description: "New Test pass Rule with quick disabled" - - name: "Protocol: Test pass Rule with Protocol manet" + # Test different Interfaces + - name: "Interface: Test pass Rules" puzzle.opnsense.firewall_rules: - interface: 'lan' + interface: "{{ item }}" action: 'pass' - protocol: 'manet' - description: "New Test pass Rule with Protocol manet" + description: "New Test pass Rule of Interface {{ item }}" + loop: + - "lan" + - "lo0" + - "openvpn" + - "opt2" - - name: "Protocol: Test pass Rule with Protocol hip" + # Test different Directions + - name: "Direction: Test pass Rule with Direction in" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'hip' - description: "New Test pass Rule with Protocol hip" + direction: in + description: "New Test pass Rule with Direction in" - - name: "Protocol: Test pass Rule with Protocol shim6" + - name: "Direction: Test pass Rule with Direction out" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'shim6' - description: "New Test pass Rule with Protocol shim6" + direction: out + description: "New Test pass Rule with Direction out" - - name: "Protocol: Test pass Rule with Protocol wesp" + # Test different IPProtocols + - name: "IPProtocol: Test pass Rule with IPProtocol IPv4" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'wesp' - description: "New Test pass Rule with Protocol wesp" + ipprotocol: 'inet' + description: "New Test pass Rule with IPv4" - - name: "Protocol: Test pass Rule with Protocol rohc" + - name: "IPProtocol: Test pass Rule with IPProtocol IPv6" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'rohc' - description: "New Test pass Rule with Protocol rohc" + ipprotocol: 'inet6' + description: "New Test pass Rule with IPProtocol IPv6" - - name: "Protocol: Test pass Rule with Protocol pfsync" + - name: "IPProtocol: Test pass Rule with IPProtocol IPv4 + IPv6" puzzle.opnsense.firewall_rules: interface: 'lan' action: 'pass' - protocol: 'pfsync' - description: "New Test pass Rule with Protocol pfsync" + ipprotocol: 'inet46' + description: "New Test pass Rule with IPProtocol IPv4 + IPv6" - - name: "Protocol: Test pass Rule with Protocol divert" - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - protocol: 'divert' - description: "New Test pass Rule with Protocol divert" + # Test different Protocols + - name: "Protocol: Test pass rule for all protocols" + puzzle.opnsense.firewall_rules: + interface: "lan" + action: "pass" + protocol: "{{ item }}" + loop: + - "any" + - "tcp" + - "udp" + - "tcp/udp" + - "icmp" + - "esp" + - "ah" + - "gre" + - "igmp" + - "pim" + - "ospf" + - "ggp" + - "ipencap" + - "st2" + - "cbt" + - "egp" + - "igp" + - "bbn-rcc" + - "nvp" + - "pup" + - "argus" + - "emcon" + - "xnet" + - "chaos" + - "mux" + - "dcn" + - "hmp" + - "prm" + - "xns-idp" + - "trunk-1" + - "trunk-2" + - "leaf-1" + - "leaf-2" + - "rdp" + - "irtp" + - "iso-tp4" + - "netblt" + - "mfe-nsp" + - "merit-inp" + - "dccp" + - "3pc" + - "idpr" + - "xtp" + - "ddp" + - "idpr-cmtp" + - "tp++" + - "il" + - "ipv6" + - "sdrp" + - "idrp" + - "rsvp" + - "dsr" + - "bna" + - "i-nlsp" + - "swipe" + - "narp" + - "mobile" + - "tlsp" + - "skip" + - "ipv6-icmp" + - "cftp" + - "sat-expak" + - "kryptolan" + - "rvd" + - "ippc" + - "sat-mon" + - "visa" + - "ipcv" + - "cpnx" + - "cphb" + - "wsn" + - "pvp" + - "br-sat-mon" + - "sun-nd" + - "wb-mon" + - "wb-expak" + - "iso-ip" + - "vmtp" + - "secure-vmtp" + - "vines" + - "ttp" + - "nsfnet-igp" + - "dgp" + - "tcf" + - "eigrp" + - "sprite-rpc" + - "larp" + - "mtp" + - "ax.25" + - "ipip" + - "micp" + - "scc-sp" + - "etherip" + - "encap" + - "gmtp" + - "ifmp" + - "pnni" + - "aris" + - "scps" + - "qnx" + - "a/n" + - "ipcomp" + - "snp" + - "compaq-peer" + - "ipx-in-ip" + - "carp" + - "pgm" + - "l2tp" + - "ddx" + - "iatp" + - "stp" + - "srp" + - "uti" + - "smp" + - "sm" + - "ptp" + - "isis" + - "crtp" + - "crudp" + - "sps" + - "pipe" + - "sctp" + - "fc" + - "rsvp-e2e-ignore" + - "udplite" + - "mpls-in-ip" + - "manet" + - "hip" + - "shim6" + - "wesp" + - "rohc" + - "pfsync" + - "divert" # Source / Invert: Test basic functionality of the source/invert button - name: "Source / Invert: Test basic functionality of the source/invert button" @@ -1152,22 +384,18 @@ # TODO add support for Advanced features: No XMLRPC Sync, Schedule and Gateway # TODO add support for Advanced Options - # Idempotency test - - name: Apply rule twice and check for idempotency - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - source: - address: '192.168.0.0/16' - register: first_apply - - name: Re-apply same rule - puzzle.opnsense.firewall_rules: - interface: 'lan' - action: 'pass' - source: - address: '192.168.0.0/16' - register: second_apply - - name: Assert no change on second apply - ansible.builtin.assert: - that: - - not second_apply.changed + - name: Test extra argument persistence + block: + + - name: Read the config + ansible.builtin.slurp: + src: /conf/config.xml + register: current_config + + - name: "Check that the extra attributes are still present" + ansible.builtin.assert: + that: + - "'keep state' in ( current_config.content | b64decode )" + - "'' in ( current_config.content | b64decode )" + - "'test-local-tag' in ( current_config.content | b64decode )" + - "'1' in ( current_config.content | b64decode )" \ No newline at end of file diff --git a/plugins/module_utils/firewall_rules_utils.py b/plugins/module_utils/firewall_rules_utils.py index 76807ad0..549e72b0 100644 --- a/plugins/module_utils/firewall_rules_utils.py +++ b/plugins/module_utils/firewall_rules_utils.py @@ -3,6 +3,7 @@ """ Utilities for firewall_rules module related operations. """ +import dataclasses from dataclasses import dataclass, asdict, field from typing import List, Optional from xml.etree.ElementTree import Element @@ -11,7 +12,6 @@ from ansible_collections.puzzle.opnsense.plugins.module_utils.config_utils import ( OPNsenseModuleConfig, ) - from ansible_collections.puzzle.opnsense.plugins.module_utils.enum_utils import ListEnum @@ -296,9 +296,8 @@ class FirewallRule: disabled: bool = False log: bool = False category: Optional[str] = None - statetype: FirewallRuleStateType = FirewallRuleStateType.KEEP_STATE - # TODO ChangeLog + extra_attributes: dict = field(default_factory=dict) def __post_init__(self): # Manually define the fields and their expected types @@ -306,7 +305,6 @@ def __post_init__(self): "type": FirewallRuleAction, "ipprotocol": IPProtocol, "protocol": FirewallRuleProtocol, - "statetype": FirewallRuleStateType, "direction": FirewallRuleDirection, } @@ -354,8 +352,11 @@ def to_etree(self) -> Element: """ rule_dict: dict = asdict(self) del rule_dict["uuid"] + del rule_dict["extra_attributes"] for rule_key, rule_val in rule_dict.copy().items(): + if rule_key == "extra_attributes": + continue if rule_key == "quick": if rule_val: del rule_dict[rule_key] @@ -371,6 +372,9 @@ def to_etree(self) -> Element: elif isinstance(rule_val, bool): rule_dict[rule_key] = "1" + for extra_key, extra_val in self.extra_attributes.items(): + rule_dict[extra_key] = extra_val + element: Element = xml_utils.dict_to_etree("rule", rule_dict)[0] if self.uuid: @@ -446,8 +450,8 @@ def from_ansible_module_params(cls, params: dict) -> "FirewallRule": return cls(**rule_dict) - @staticmethod - def from_xml(element: Element) -> "FirewallRule": + @classmethod + def from_xml(cls, element: Element) -> "FirewallRule": """ Converts an XML element into a FirewallRule object. @@ -489,20 +493,25 @@ def from_xml(element: Element) -> "FirewallRule": uuid=element.attrib.get("uuid"), ) - # TODO ignore changelog for now - rule_dict.pop("updated", None) - rule_dict.pop("created", None) - - rule_dict.pop("source") - rule_dict.pop("destination") - source: FirewallRuleTarget = FirewallRuleTarget.from_xml( + rule_dict["source"] = FirewallRuleTarget.from_xml( "source", element.find("./source") ) - destination: FirewallRuleTarget = FirewallRuleTarget.from_xml( + rule_dict["destination"] = FirewallRuleTarget.from_xml( "destination", element.find("./destination") ) - return FirewallRule(source=source, destination=destination, **rule_dict) + class_attribute_names: List[str] = list( + map(lambda f: f.name, dataclasses.fields(cls)) + ) + + rule_dict["extra_attributes"] = {} + + for k, v in rule_dict.copy().items(): + if k not in class_attribute_names: + rule_dict["extra_attributes"][k] = v + rule_dict.pop(k) + + return FirewallRule(**rule_dict) class FirewallRuleSet(OPNsenseModuleConfig): diff --git a/tests/unit/plugins/module_utils/test_firewall_rules_utils.py b/tests/unit/plugins/module_utils/test_firewall_rules_utils.py index 9ca23e53..3f4c2f8f 100644 --- a/tests/unit/plugins/module_utils/test_firewall_rules_utils.py +++ b/tests/unit/plugins/module_utils/test_firewall_rules_utils.py @@ -13,7 +13,6 @@ from xml.etree.ElementTree import Element import pytest - from ansible_collections.puzzle.opnsense.plugins.module_utils import xml_utils from ansible_collections.puzzle.opnsense.plugins.module_utils.firewall_rules_utils import ( FirewallRuleAction, @@ -21,7 +20,6 @@ FirewallRule, IPProtocol, FirewallRuleProtocol, - FirewallRuleStateType, FirewallRuleTarget, ) from ansible_collections.puzzle.opnsense.plugins.module_utils.module_index import ( @@ -75,6 +73,24 @@ 22 + + pass + wan + inet + keep state + Allow SSH access + tcp + + + + + + 22 + + + this is an extra attribute + + pass wan @@ -161,7 +177,6 @@ def test_firewall_rule_from_xml(): assert test_rule.type == FirewallRuleAction.PASS assert test_rule.interface == "wan" assert test_rule.ipprotocol == IPProtocol.IPv4 - assert test_rule.statetype == FirewallRuleStateType.KEEP_STATE assert test_rule.descr == "Allow SSH access" assert test_rule.protocol == FirewallRuleProtocol.TCP assert test_rule.source.port == "any" @@ -193,9 +208,8 @@ def test_firewall_rule_to_etree(): protocol=FirewallRuleProtocol.TCP, source=FirewallRuleTarget("source"), destination=FirewallRuleTarget("destination", port="22"), - statetype=FirewallRuleStateType.KEEP_STATE, ) - + test_rule.extra_attributes["statetype"] = "keep state" test_element = test_rule.to_etree() orig_etree: Element = ElementTree.fromstring(TEST_XML) @@ -207,6 +221,33 @@ def test_firewall_rule_to_etree(): ) +def test_firewall_rule_to_etree_with_extra_attributes(): + """ + An extra non dataclass relevant field in the xml should be + persisted. + """ + test_rule: FirewallRule = FirewallRule( + interface="wan", + type=FirewallRuleAction.PASS, + descr="Allow SSH access", + ipprotocol=IPProtocol.IPv4, + protocol=FirewallRuleProtocol.TCP, + source=FirewallRuleTarget("source"), + destination=FirewallRuleTarget("destination", port="22"), + extra_attributes={"extra": "this is an extra attribute"}, + ) + + test_rule.extra_attributes["statetype"] = "keep state" + test_element = test_rule.to_etree() + orig_etree: Element = ElementTree.fromstring(TEST_XML) + orig_rule: Element = orig_etree.find("filter")[1] + + assert elements_equal(test_element, orig_rule), ( + f"{xml_utils.etree_to_dict(test_element)}\n" + f"{xml_utils.etree_to_dict(orig_rule)}" + ) + + def test_firewall_rule_from_ansible_module_params_simple(): """ Test FirewallRule instantiation form simple Ansible parameters. @@ -257,7 +298,7 @@ def test_rule_set_load_simple_rules( Test correct loading of FirewallRuleSet from XML config without changes. """ with FirewallRuleSet(sample_config_path) as rule_set: - assert len(rule_set._rules) == 4 + assert len(rule_set._rules) == 5 rule_set.save() From 2a99e1ec03aec2dd4a1b47012c918ee159d3549a Mon Sep 17 00:00:00 2001 From: Fabio Bertagna <33524186+DonGiovanni83@users.noreply.github.com> Date: Tue, 6 Aug 2024 15:16:15 +0200 Subject: [PATCH 7/7] Prepare v1.2.1 release (#145) * Prepare v1.2.1 release * Fix typo in changelog --- CHANGELOG.rst | 8 ++++++++ changelogs/changelog.yaml | 8 ++++++++ .../143-add-extra-attributes-to-firewall-rules.yml | 3 --- galaxy.yml | 2 +- 4 files changed, 17 insertions(+), 4 deletions(-) delete mode 100644 changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml diff --git a/CHANGELOG.rst b/CHANGELOG.rst index dc5b1b34..8ba869d7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,14 @@ OPNsense Collection Release Notes .. contents:: Topics +v1.2.1 +====== + +Bugfixes +-------- + +- firewall_rules_utils - Handle additional XML attributes for the firewall rule objects from the config. + v1.2.0 ====== diff --git a/changelogs/changelog.yaml b/changelogs/changelog.yaml index cf81ac60..fe62c4f1 100644 --- a/changelogs/changelog.yaml +++ b/changelogs/changelog.yaml @@ -67,3 +67,11 @@ releases: name: system_high_availability_settings namespace: '' release_date: '2024-06-28' + 1.2.1: + changes: + bugfixes: + - firewall_rules_utils - Handle additional XML attributes for the firewall rule + objects from the config. + fragments: + - 143-add-extra-attributes-to-firewall-rules.yml + release_date: '2024-08-05' diff --git a/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml b/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml deleted file mode 100644 index 615c7d6b..00000000 --- a/changelogs/fragments/143-add-extra-attributes-to-firewall-rules.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -bugfixes: - - firewall_rules_utils - Handle additional XML attributes fo the firewall rule objects from the config. \ No newline at end of file diff --git a/galaxy.yml b/galaxy.yml index 6f25b839..83064a60 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -2,7 +2,7 @@ namespace: puzzle name: opnsense -version: 1.2.0 +version: 1.2.1 readme: README.md authors: - Lukas Grimm (github.com/ombre8)