From fbb2b841fca608d55f72d298953a1496a534b99f Mon Sep 17 00:00:00 2001 From: Daniela Plascencia Date: Tue, 8 Aug 2023 15:29:23 +0200 Subject: [PATCH] feat: add `enablePasswordDB` charm configuration option to enable/disable static password login (#149) (#153) * feat: add `enablePasswordDB` charm configuration option to enable/disable static password login (#149) Add enable-password-db charm configuration option to enable/disable static password login. Fixes #76 Co-authored-by: Mohammed Alhabib <124865446+alhama7a@users.noreply.github.com> * build: unpin setuptools and pip versions from charmcraft.yaml file * refactor: generate dex-auth configuration outside of update_layer(), use CheckFailedError Generate dex-auth configuration outside of update_layer() will allow us to have error handling when checking for certain conditions of the configurations without cluttering too much the update_layer() method. This commit also leverages CheckFailedError to set the unit status to blocked when the static login has been disabled and no connectors configuration has been provieded. Lastly, this commit introduces a change for raising CheckFailedError in some methods and handling the exception in main(). * tests: add test case for checking BlockedStatus when disabling static login * tests: add test cases for checking helper method and config changes when enabling/disabling static login Co-authored-by: Phoevos Kalemkeris --------- Co-authored-by: Mohammed Alhabib <124865446+alhama7a@users.noreply.github.com> Co-authored-by: Phoevos Kalemkeris --- charmcraft.yaml | 3 +- config.yaml | 4 ++ src/charm.py | 137 ++++++++++++++++++++++----------------- tests/unit/test_charm.py | 110 ++++++++++++++++++++++++++++++- 4 files changed, 189 insertions(+), 65 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index c5ee591c..d7980afc 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -11,5 +11,4 @@ bases: channel: "20.04" parts: charm: - # do not use these versions due to pypa/setuptools_scm#713 - charm-python-packages: [setuptools!=62.2.0, pip!=22.1] + charm-python-packages: [setuptools, pip] diff --git a/config.yaml b/config.yaml index dc239c9d..6007db1a 100644 --- a/config.yaml +++ b/config.yaml @@ -2,6 +2,10 @@ # See LICENSE file for licensing details. options: + enable-password-db: + type: boolean + default: true + description: Allows dex to keep a list of passwords which can be used to login to dex port: type: int default: 5556 diff --git a/src/charm.py b/src/charm.py index 416ed06c..9a2b3e63 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,7 +15,7 @@ from ops.charm import CharmBase from ops.framework import StoredState from ops.main import main -from ops.model import ActiveStatus, MaintenanceStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.pebble import Layer from serialized_data_interface import NoVersionsListed, get_interface @@ -96,56 +96,12 @@ def _dex_auth_layer(self) -> Layer: def _update_layer(self) -> None: """Updates the Pebble configuration layer if changed.""" - try: - self._check_container_connection() - except CheckFailedError as err: - self.model.unit.status = err.status - return - - # Get OIDC client info - oidc = self._get_interface("oidc-client") - - if oidc: - oidc_client_info = list(oidc.get_data().values()) - else: - oidc_client_info = [] - - # Load config values as convenient variables - connectors = yaml.safe_load(self.model.config["connectors"]) - port = self.model.config["port"] - public_url = self.model.config["public-url"].lower() - if not public_url.startswith(("http://", "https://")): - public_url = f"http://{public_url}" + # Check container connection + self._check_container_connection() - static_username = self.model.config["static-username"] or self.state.username - static_password = self.model.config["static-password"] or self.state.password - static_password = static_password.encode("utf-8") - hashed = bcrypt.hashpw(static_password, self.state.salt).decode("utf-8") - - static_config = { - "enablePasswordDB": True, - "staticPasswords": [ - { - "email": static_username, - "hash": hashed, - "username": static_username, - "userID": self.state.user_id, - } - ], - } - - config = yaml.dump( - { - "issuer": f"{public_url}/dex", - "storage": {"type": "kubernetes", "config": {"inCluster": True}}, - "web": {"http": f"0.0.0.0:{port}"}, - "logger": {"level": "debug", "format": "text"}, - "oauth2": {"skipApprovalScreen": True}, - "staticClients": oidc_client_info, - "connectors": connectors, - **static_config, - } - ) + # Generate dex-auth configuration to be passed to the pebble layer + # so the service is (re)started. + dex_auth_config = self._generate_dex_auth_config() # Get current layer current_layer = self._container.get_plan() @@ -158,8 +114,8 @@ def _update_layer(self) -> None: # Get current dex config current_config = self._container.pull(self._dex_config_path).read() - if current_config != config: - self._container.push(self._dex_config_path, config, make_dirs=True) + if current_config != dex_auth_config: + self._container.push(self._dex_config_path, dex_auth_config, make_dirs=True) self.logger.info("Updated dex config") # Using restart due to https://github.com/canonical/dex-auth-operator/issues/63 @@ -168,17 +124,13 @@ def _update_layer(self) -> None: def main(self, event): try: self._check_leader() + self.model.unit.status = MaintenanceStatus("Configuring dex charm") + self.ensure_state() + self._update_layer() + self.handle_ingress() except CheckFailedError as err: self.model.unit.status = err.status return - - self.model.unit.status = MaintenanceStatus("Configuring dex charm") - self.ensure_state() - - self._update_layer() - - self.handle_ingress() - self.model.unit.status = ActiveStatus() def ensure_state(self): @@ -229,6 +181,71 @@ def _get_interface(self, interface_name): return interface + def _generate_dex_auth_config(self) -> str: + """Returns dex-auth configuration to be passed when (re)starting the dex-auth service. + + Raises: + CheckFailedError: when static login is disabled and no connectors are configured. + """ + # Get OIDC client info + oidc = self._get_interface("oidc-client") + if oidc: + oidc_client_info = list(oidc.get_data().values()) + else: + oidc_client_info = [] + + # Load config values as convenient variables + connectors = yaml.safe_load(self.model.config["connectors"]) + port = self.model.config["port"] + public_url = self.model.config["public-url"].lower() + if not public_url.startswith(("http://", "https://")): + public_url = f"http://{public_url}" + + enable_password_db = self.model.config["enable-password-db"] + static_config = { + "staticPasswords": [], + } + + # The dex-auth service cannot be started correctly when the static + # login is disabled, but no connector configuration is provided. + if not enable_password_db and not connectors: + raise CheckFailedError( + "Please add a connectors configuration to proceed without a static login.", + BlockedStatus, + ) + + if enable_password_db: + static_username = self.model.config["static-username"] or self.state.username + static_password = self.model.config["static-password"] or self.state.password + static_password = static_password.encode("utf-8") + hashed = bcrypt.hashpw(static_password, self.state.salt).decode("utf-8") + static_config = { + "staticPasswords": [ + { + "email": static_username, + "hash": hashed, + "username": static_username, + "userID": self.state.user_id, + } + ], + } + + config = yaml.dump( + { + "issuer": f"{public_url}/dex", + "storage": {"type": "kubernetes", "config": {"inCluster": True}}, + "web": {"http": f"0.0.0.0:{port}"}, + "logger": {"level": "debug", "format": "text"}, + "oauth2": {"skipApprovalScreen": True}, + "staticClients": oidc_client_info, + "connectors": connectors, + "enablePasswordDB": enable_password_db, + **static_config, + } + ) + + return config + class CheckFailedError(Exception): """Raise this exception if one of the checks in main fails.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f726779b..9e7764c1 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,12 +3,16 @@ from unittest.mock import patch +import ops import pytest import yaml -from ops.model import ActiveStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness -from charm import Operator +from charm import CheckFailedError, Operator + +# SIMULATE_CAN_CONNECT is needed when using ops<2 +ops.testing.SIMULATE_CAN_CONNECT = True @pytest.fixture @@ -40,6 +44,7 @@ def ensure_state(self): def test_install_event(update, harness): harness.set_leader(True) harness.begin() + harness.set_can_connect("dex", True) harness.charm.on.install.emit() update.assert_called() @@ -54,16 +59,115 @@ def test_install_event(update, harness): assert isinstance(harness.charm.model.unit.status, ActiveStatus) +@patch("charm.KubernetesServicePatch", lambda x, y: None) +def test_generate_dex_auth_config_raises(harness): + """Check the method raises when static login is disabled and no connectors are provided.""" + harness.begin() + config_updates = { + "enable-password-db": False, + "port": 5555, + "public-url": "dummy.url", + } + + harness.update_config(config_updates) + + with pytest.raises(CheckFailedError) as error: + harness.charm._generate_dex_auth_config() + assert ( + error.value.msg + == "Please add a connectors configuration to proceed without a static login." + ) + assert error.value.status_type == BlockedStatus + + +@pytest.mark.parametrize( + "dex_config", + ( + { + "enable-password-db": False, + "port": 5555, + "public-url": "dummy.url", + "connectors": "test-connector", + }, + { + "enable-password-db": True, + "port": 5555, + "public-url": "dummy.url", + "static-username": "new-user", + "static-password": "new-pass", + }, + ), +) +@patch("charm.Operator._update_layer") +@patch.object(Operator, "ensure_state", ensure_state) +@patch("charm.KubernetesServicePatch", lambda x, y: None) +def test_generate_dex_auth_config_returns(update_layer, dex_config, harness): + """Check the method returns dex-auth configuration when different settings are provided.""" + harness.set_leader(True) + harness.begin() + harness.set_can_connect("dex", True) + + harness.update_config(dex_config) + + test_configuration = harness.charm._generate_dex_auth_config() + assert test_configuration is not None + + test_configuration_dict = yaml.safe_load(test_configuration) + assert ( + yaml.safe_load(harness.model.config["connectors"]) == test_configuration_dict["connectors"] + ) + assert ( + harness.model.config["enable-password-db"] == test_configuration_dict["enablePasswordDB"] + ) + + static_passwords = test_configuration_dict.get("staticPasswords") + assert isinstance(static_passwords, list) + if not harness.model.config["static-username"]: + assert len(static_passwords) == 0 + else: + assert len(static_passwords) == 1 + assert harness.model.config["static-username"] == static_passwords[0].get("username") + + +@patch("charm.KubernetesServicePatch", lambda x, y: None) +def test_disable_static_login_no_connector_blocked_status(harness): + harness.set_leader(True) + harness.begin() + harness.set_can_connect("dex", True) + + config_updates = { + "enable-password-db": False, + "port": 5555, + "public-url": "dummy.url", + } + + harness.update_config(config_updates) + assert isinstance(harness.charm.model.unit.status, BlockedStatus) + + @patch("charm.KubernetesServicePatch", lambda x, y: None) @patch("charm.Operator._update_layer") def test_config_changed(update, harness): harness.set_leader(True) harness.begin() - harness.update_config({"static-username": "new-user"}) + config_updates = { + "enable-password-db": False, + "port": 5555, + "public-url": "dummy.url", + "connectors": "connector01", + "static-username": "new-user", + "static-password": "new-pass", + } + + harness.update_config(config_updates) update.assert_called() + new_config = harness.model.config + + assert new_config == config_updates + @patch("charm.KubernetesServicePatch", lambda x, y: None) @patch("charm.Operator._update_layer")