Merge pull request #183 from canonical/bump-external-idp-lib

Update kratos_external_idp_integrator lib

lib/charms/kratos_external_idp_integrator/v0/kratos_external_provider.py

@@ -5,8 +5,8 @@
 """# Interface library for Kratos external OIDC providers.
 
 This library wraps relation endpoints using the `kratos-external-idp` interface
-and provides a Python API for both requesting Kratos to register the the client credentials for
-communicating with an external provider.
+and provides a Python API for both requesting Kratos to register the client credentials
+and for communicating with an external provider.
 
 ## Getting Started
 
@@ -27,9 +27,6 @@
         limit: 1
 ```
 
-Next add the `jsonschema` python package to your charm's `requirements.txt`, so that the
-library can validate the incoming relation databags.
-
 Then, to initialise the library:
 
 ```python
@@ -129,7 +126,9 @@ def _on_client_config_changed(self, event):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 7
+LIBPATCH = 8
+
+PYDEPS = ["jsonschema"]
 
 DEFAULT_RELATION_NAME = "kratos-external-idp"
 logger = logging.getLogger(__name__)
@@ -633,9 +632,11 @@ def config(self) -> Dict:
             "client_secret": self.client_secret,
             "issuer_url": self.issuer_url,
             "scope": self.scope.split(" "),
-            "mapper_url": base64.b64encode(self.jsonnet_mapper.encode()).decode()
-            if self.jsonnet_mapper
-            else None,
+            "mapper_url": (
+                base64.b64encode(self.jsonnet_mapper.encode()).decode()
+                if self.jsonnet_mapper
+                else None
+            ),
             "microsoft_tenant": self.tenant_id,
             "apple_team_id": self.team_id,
             "apple_private_key_id": self.private_key_id,
@@ -678,10 +679,29 @@ def restore(self, snapshot: Dict) -> None:
         self.relation_id = snapshot["relation_id"]
 
 
+class ClientConfigRemovedEvent(EventBase):
+    """Event to notify the charm that a provider's client config was removed."""
+
+    def __init__(self, handle: Handle, relation_id: str) -> None:
+        super().__init__(handle)
+        self.relation_id = relation_id
+
+    def snapshot(self) -> Dict:
+        """Save event."""
+        return {
+            "relation_id": self.relation_id,
+        }
+
+    def restore(self, snapshot: Dict) -> None:
+        """Restore event."""
+        self.relation_id = snapshot["relation_id"]
+
+
 class ExternalIdpRequirerEvents(ObjectEvents):
     """Event descriptor for events raised by `ExternalIdpRequirerEvents`."""
 
     client_config_changed = EventSource(ClientConfigChangedEvent)
+    client_config_removed = EventSource(ClientConfigRemovedEvent)
 
 
 class ExternalIdpRequirer(Object):
@@ -711,6 +731,7 @@ def _on_provider_endpoint_relation_changed(self, event: RelationEvent) -> None:
         providers = data["providers"]
 
         if len(providers) == 0:
+            self.on.client_config_removed.emit(event.relation.id)
             return
 
         p = self._get_provider(providers[0], event.relation)
@@ -741,6 +762,18 @@ def set_relation_registered_provider(
             return
         relation.data[self.model.app].update(data)
 
+    def remove_relation_registered_provider(self, relation_id: int) -> None:
+        """Delete the provider info from the databag."""
+        if not self._charm.unit.is_leader():
+            return
+
+        relation = self.model.get_relation(
+            relation_name=self._relation_name, relation_id=relation_id
+        )
+        if not relation:
+            return
+        relation.data[self.model.app].clear()
+
     def get_providers(self) -> List:
         """Iterate over the relations and fetch all providers."""
         providers = []

src/charm.py

@@ -37,6 +37,7 @@
 from charms.kratos.v0.kratos_info import KratosInfoProvider
 from charms.kratos_external_idp_integrator.v0.kratos_external_provider import (
     ClientConfigChangedEvent,
+    ClientConfigRemovedEvent,
     ExternalIdpRequirer,
 )
 from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer, PromtailDigestError
@@ -229,6 +230,9 @@ def __init__(self, *args: Any) -> None:
         self.framework.observe(
             self.external_provider.on.client_config_changed, self._on_client_config_changed
         )
+        self.framework.observe(
+            self.external_provider.on.client_config_removed, self._on_client_config_removed
+        )
 
         self.framework.observe(self.on.get_identity_action, self._on_get_identity_action)
         self.framework.observe(self.on.delete_identity_action, self._on_delete_identity_action)
@@ -881,17 +885,22 @@ def _on_client_config_changed(self, event: ClientConfigChangedEvent) -> None:
 
         self.unit.status = MaintenanceStatus(f"Adding external provider: {event.provider}")
 
-        if not self._migration_is_needed():
-            self._handle_status_update_config(event)
-
-            self.external_provider.set_relation_registered_provider(
-                join(public_url, f"self-service/methods/oidc/callback/{event.provider_id}"),
-                event.provider_id,
-                event.relation_id,
-            )
-        else:
+        if self._migration_is_needed():
             event.defer()
             logger.info("Database is not created. Deferring event.")
+            return
+
+        self._handle_status_update_config(event)
+        self.external_provider.set_relation_registered_provider(
+            join(public_url, f"self-service/methods/oidc/callback/{event.provider_id}"),
+            event.provider_id,
+            event.relation_id,
+        )
+
+    def _on_client_config_removed(self, event: ClientConfigRemovedEvent) -> None:
+        self.unit.status = MaintenanceStatus("Removing external provider")
+        self._handle_status_update_config(event)
+        self.external_provider.remove_relation_registered_provider(event.relation_id)
 
     def _on_get_identity_action(self, event: ActionEvent) -> None:
         if not self._kratos_service_is_running:

tests/unit/test_charm.py

@@ -178,6 +178,7 @@ def setup_external_provider_relation(harness: Harness) -> tuple[int, dict]:
     data = {
         "client_id": "client_id",
         "provider": "generic",
+        "label": "generic",
         "secret_backend": "relation",
         "client_secret": "client_secret",
         "issuer_url": "https://example.com/oidc",
@@ -714,7 +715,7 @@ def test_on_config_changed_when_no_dns_available(harness: Harness) -> None:
     assert isinstance(harness.charm.unit.status, BlockedStatus)
 
 
-def test_on_config_changed_with_ingress(
+def test_on_client_config_changed_with_ingress(
     harness: Harness,
     mocked_container: Container,
     mocked_migration_is_needed: MagicMock,
@@ -781,9 +782,9 @@ def test_on_config_changed_with_ingress(
                                 "client_id": data["client_id"],
                                 "client_secret": data["client_secret"],
                                 "issuer_url": data["issuer_url"],
-                                "label": "generic",
                                 "mapper_url": "file:///etc/config/kratos/default_schema.jsonnet",
                                 "provider": data["provider"],
+                                "label": data["label"],
                                 "scope": data["scope"].split(" "),
                             },
                         ],
@@ -818,6 +819,138 @@ def test_on_config_changed_with_ingress(
     assert app_data[0]["redirect_uri"].startswith(expected_redirect_url)
 
 
+def test_on_client_config_relation_removed_with_ingress(
+    harness: Harness,
+    mocked_container: Container,
+    mocked_migration_is_needed: MagicMock,
+    mocked_kratos_configmap: MagicMock,
+) -> None:
+    setup_peer_relation(harness)
+    setup_postgres_relation(harness)
+    setup_ingress_relation(harness, "public")
+    (_, login_databag) = setup_login_ui_relation(harness)
+
+    relation_id, _ = setup_external_provider_relation(harness)
+    harness.remove_relation(relation_id)
+    harness.set_leader(True)
+    container = harness.model.unit.get_container(CONTAINER_NAME)
+    harness.charm.on.kratos_pebble_ready.emit(container)
+
+    expected_config = {
+        "log": {
+            "level": "info",
+            "format": "json",
+        },
+        "identity": {
+            "default_schema_id": "social_user_v0",
+            "schemas": [
+                {"id": "admin_v0", "url": "base64://something"},
+                {
+                    "id": "social_user_v0",
+                    "url": "base64://something",
+                },
+            ],
+        },
+        "selfservice": {
+            "allowed_return_urls": [
+                "https://public/",
+            ],
+            "default_browser_return_url": login_databag["login_url"],
+            "flows": {
+                "error": {
+                    "ui_url": login_databag["error_url"],
+                },
+                "login": {
+                    "ui_url": login_databag["login_url"],
+                },
+            },
+        },
+        "courier": {
+            "smtp": {"connection_uri": "smtps://test:test@mailslurper:1025/?skip_ssl_verify=true"}
+        },
+        "serve": {
+            "public": {
+                "cors": {
+                    "enabled": True,
+                },
+            },
+        },
+    }
+
+    configmap = mocked_kratos_configmap.update.call_args_list[-1][0][0]
+    config = configmap["kratos.yaml"]
+    validate_config(
+        expected_config, yaml.safe_load(config), validate_schemas=False, validate_mappers=False
+    )
+
+
+def test_on_client_config_data_removed_with_ingress(
+    harness: Harness,
+    mocked_container: Container,
+    mocked_migration_is_needed: MagicMock,
+    mocked_kratos_configmap: MagicMock,
+) -> None:
+    setup_peer_relation(harness)
+    setup_postgres_relation(harness)
+    setup_ingress_relation(harness, "public")
+    (_, login_databag) = setup_login_ui_relation(harness)
+
+    relation_id, _ = setup_external_provider_relation(harness)
+    harness.update_relation_data(relation_id, "kratos-external-idp-integrator", {"providers": ""})
+    container = harness.model.unit.get_container(CONTAINER_NAME)
+    harness.charm.on.leader_elected.emit()
+    harness.charm.on.kratos_pebble_ready.emit(container)
+
+    expected_config = {
+        "log": {
+            "level": "info",
+            "format": "json",
+        },
+        "identity": {
+            "default_schema_id": "social_user_v0",
+            "schemas": [
+                {"id": "admin_v0", "url": "base64://something"},
+                {
+                    "id": "social_user_v0",
+                    "url": "base64://something",
+                },
+            ],
+        },
+        "selfservice": {
+            "allowed_return_urls": [
+                "https://public/",
+            ],
+            "default_browser_return_url": login_databag["login_url"],
+            "flows": {
+                "error": {
+                    "ui_url": login_databag["error_url"],
+                },
+                "login": {
+                    "ui_url": login_databag["login_url"],
+                },
+            },
+        },
+        "courier": {
+            "smtp": {"connection_uri": "smtps://test:test@mailslurper:1025/?skip_ssl_verify=true"}
+        },
+        "serve": {
+            "public": {
+                "cors": {
+                    "enabled": True,
+                },
+            },
+        },
+    }
+
+    configmap = mocked_kratos_configmap.update.call_args_list[-1][0][0]
+    config = configmap["kratos.yaml"]
+    validate_config(
+        expected_config, yaml.safe_load(config), validate_schemas=False, validate_mappers=False
+    )
+
+    assert harness.get_relation_data(relation_id, harness.charm.app) == {}
+
+
 def test_on_config_changed_with_hydra(
     harness: Harness, mocked_migration_is_needed: MagicMock, mocked_kratos_configmap: MagicMock
 ) -> None: