feat: add enablePasswordDB charm configuration option to enable/dis…

…able 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 <[email protected]>

* 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 <[email protected]>

---------

Co-authored-by: Mohammed Alhabib <[email protected]>
Co-authored-by: Phoevos Kalemkeris <[email protected]>

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]

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

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."""

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")