From 392262febfd53d57dc507e6871c53d5c0137b69d Mon Sep 17 00:00:00 2001 From: Phan Trung Thanh Date: Tue, 26 Nov 2024 18:34:23 +0100 Subject: [PATCH] remove the SAML integration and all relevant code --- .woke.yaml | 2 - config.yaml | 3 +- docs/reference/integrations.md | 21 -- lib/charms/saml_integrator/v0/saml.py | 347 --------------------- metadata.yaml | 4 - src-docs/charm.py.md | 24 +- src-docs/charm_state.py.md | 27 +- src-docs/charm_types.py.md | 16 - src-docs/saml_observer.py.md | 75 ----- src/charm.py | 3 - src/charm_state.py | 6 - src/charm_types.py | 13 - src/matrix_auth_observer.py | 1 - src/pebble.py | 3 - src/saml_observer.py | 91 ------ src/synapse/__init__.py | 1 - src/synapse/workload.py | 4 - src/synapse/workload_configuration.py | 94 +----- synapse_rock/attributemaps/login_ubuntu.py | 11 - tests/integration/test_nginx.py | 99 ------ tests/unit/conftest.py | 14 - tests/unit/test_charm.py | 59 ---- tests/unit/test_charm_state.py | 1 - tests/unit/test_synapse_api.py | 2 - tests/unit/test_synapse_workload.py | 139 --------- tox.ini | 2 - 26 files changed, 26 insertions(+), 1036 deletions(-) delete mode 100644 lib/charms/saml_integrator/v0/saml.py delete mode 100644 src-docs/saml_observer.py.md delete mode 100644 src/saml_observer.py delete mode 100644 synapse_rock/attributemaps/login_ubuntu.py delete mode 100644 tests/integration/test_nginx.py diff --git a/.woke.yaml b/.woke.yaml index 83bb1eda..a961a86d 100644 --- a/.woke.yaml +++ b/.woke.yaml @@ -8,7 +8,5 @@ rules: # Ignore "master" - While https://github.com/canonical/redis-k8s-operator/pull/78 # is not merged - name: master - # Ignore "grandfathered" used by SAML configuration. - - name: grandfathered # Ignore "whitelist" used by Synapse configuration. - name: whitelist diff --git a/config.yaml b/config.yaml index dbe5f92f..9e1f0879 100644 --- a/config.yaml +++ b/config.yaml @@ -71,8 +71,7 @@ options: type: string description: | The public-facing base URL that clients use to access this Homeserver. - Defaults to https:///. Only used if there is integration with - SAML integrator charm. + Defaults to https:///. experimental_alive_check: type: string description: Comma separated list of period,threshold and timeout for Synapse diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 1de9a6da..a33bc401 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -105,27 +105,6 @@ Example redis integrate command: juju integrate synapse redis-k8s ``` -### saml - -_Interface_: saml - -_Supported charms_: [saml-integrator](https://charmhub.io/saml-integrator/) - -Integrating Synapse with SAML Integrator provides SAML configuration details so -users can be authenticated in via a SAML server. - -Example saml integrate command: -``` -juju integrate synapse saml-integrator:saml -``` - -Note that `public_baseurl` configuration set the public-facing base URL that -clients use to access this Homeserver. It's used as `entity_id` if set instead of -https://server_name. - -See more information in [Charm Architecture](https://charmhub.io/synapse/docs/explanation-charm-architecture). - - ### smtp _Interface_: smtp diff --git a/lib/charms/saml_integrator/v0/saml.py b/lib/charms/saml_integrator/v0/saml.py deleted file mode 100644 index 8fd1610e..00000000 --- a/lib/charms/saml_integrator/v0/saml.py +++ /dev/null @@ -1,347 +0,0 @@ -#!/usr/bin/env python3 - -# Copyright 2024 Canonical Ltd. -# Licensed under the Apache2.0. See LICENSE file in charm source for details. - -"""Library to manage the relation data for the SAML Integrator charm. - -This library contains the Requires and Provides classes for handling the relation -between an application and a charm providing the `saml`relation. -It also contains a `SamlRelationData` class to wrap the SAML data that will -be shared via the relation. - -### Requirer Charm - -```python - -from charms.saml_integrator.v0 import SamlDataAvailableEvent, SamlRequires - -class SamlRequirerCharm(ops.CharmBase): - def __init__(self, *args): - super().__init__(*args) - self.saml = saml.SamlRequires(self) - self.framework.observe(self.saml.on.saml_data_available, self._handler) - ... - - def _handler(self, events: SamlDataAvailableEvent) -> None: - ... - -``` - -As shown above, the library provides a custom event to handle the scenario in -which new SAML data has been added or updated. - -### Provider Charm - -Following the previous example, this is an example of the provider charm. - -```python -from charms.saml_integrator.v0 import SamlDataAvailableEvent, SamlRequires - -class SamlRequirerCharm(ops.CharmBase): - def __init__(self, *args): - super().__init__(*args) - self.saml = SamlRequires(self) - self.framework.observe(self.saml.on.saml_data_available, self._on_saml_data_available) - ... - - def _on_saml_data_available(self, events: SamlDataAvailableEvent) -> None: - ... - - def __init__(self, *args): - super().__init__(*args) - self.saml = SamlProvides(self) - -``` -The SamlProvides object wraps the list of relations into a `relations` property -and provides an `update_relation_data` method to update the relation data by passing -a `SamlRelationData` data object. -Additionally, SamlRelationData can be used to directly parse the relation data with the -class method `from_relation_data`. -""" - -# The unique Charmhub library identifier, never change it -LIBID = "511cdfa7de3d43568bf9b512f9c9f89d" - -# Increment this major API version when introducing breaking changes -LIBAPI = 0 - -# Increment this PATCH version before using `charmcraft publish-lib` or reset -# to 0 if you are raising the major API version -LIBPATCH = 10 - -# pylint: disable=wrong-import-position -import re -import typing - -import ops -from pydantic import AnyHttpUrl, BaseModel, Field -from pydantic.tools import parse_obj_as - -DEFAULT_RELATION_NAME = "saml" - - -class SamlEndpoint(BaseModel): - """Represent a SAML endpoint. - - Attrs: - name: Endpoint name. - url: Endpoint URL. - binding: Endpoint binding. - response_url: URL to address the response to. - """ - - name: str = Field(..., min_length=1) - url: typing.Optional[AnyHttpUrl] - binding: str = Field(..., min_length=1) - response_url: typing.Optional[AnyHttpUrl] - - def to_relation_data(self) -> typing.Dict[str, str]: - """Convert an instance of SamlEndpoint to the relation representation. - - Returns: - Dict containing the representation. - """ - result: typing.Dict[str, str] = {} - # Get the HTTP method from the SAML binding - http_method = self.binding.split(":")[-1].split("-")[-1].lower() - # Transform name into snakecase - lowercase_name = re.sub(r"(? "SamlEndpoint": - """Initialize a new instance of the SamlEndpoint class from the relation data. - - Args: - relation_data: the relation data. - - Returns: - A SamlEndpoint instance. - """ - url_key = "" - for key in relation_data: - # A key per method and entpoint type that is always present - if key.endswith("_redirect_url") or key.endswith("_post_url"): - url_key = key - # Get endpoint name from the relation data key - lowercase_name = "_".join(url_key.split("_")[:-2]) - name = "".join(x.capitalize() for x in lowercase_name.split("_")) - # Get HTTP method from the relation data key - http_method = url_key.split("_")[-2] - prefix = f"{lowercase_name}_{http_method}_" - return cls( - name=name, - url=( - parse_obj_as(AnyHttpUrl, relation_data[f"{prefix}url"]) - if relation_data[f"{prefix}url"] - else None - ), - binding=relation_data[f"{prefix}binding"], - response_url=( - parse_obj_as(AnyHttpUrl, relation_data[f"{prefix}response_url"]) - if f"{prefix}response_url" in relation_data - else None - ), - ) - - -class SamlRelationData(BaseModel): - """Represent the relation data. - - Attrs: - entity_id: SAML entity ID. - metadata_url: URL to the metadata. - certificates: Tuple of SAML certificates. - endpoints: Tuple of SAML endpoints. - """ - - entity_id: str = Field(..., min_length=1) - metadata_url: typing.Optional[AnyHttpUrl] - certificates: typing.Tuple[str, ...] - endpoints: typing.Tuple[SamlEndpoint, ...] - - def to_relation_data(self) -> typing.Dict[str, str]: - """Convert an instance of SamlDataAvailableEvent to the relation representation. - - Returns: - Dict containing the representation. - """ - result = { - "entity_id": self.entity_id, - "x509certs": ",".join(self.certificates), - } - if self.metadata_url: - result["metadata_url"] = str(self.metadata_url) - for endpoint in self.endpoints: - result.update(endpoint.to_relation_data()) - return result - - @classmethod - def from_relation_data(cls, relation_data: ops.RelationDataContent) -> "SamlRelationData": - """Get a SamlRelationData wrapping the relation data. - - Arguments: - relation_data: the relation data. - - Returns: a SamlRelationData instance with the relation data. - """ - # mypy is not aware of the relation data being present - endpoints = [ - SamlEndpoint.from_relation_data( - { - key2: relation_data.get(key2) # type: ignore - for key2 in relation_data - if key2.startswith("_".join(key.split("_")[:-1])) - } - ) - for key in relation_data - if key.endswith("_redirect_url") or key.endswith("_post_url") - ] - endpoints.sort(key=lambda ep: ep.name) - return cls( - entity_id=relation_data.get("entity_id"), # type: ignore - metadata_url=( - parse_obj_as(AnyHttpUrl, relation_data.get("metadata_url")) - if relation_data.get("metadata_url") - else None - ), # type: ignore - certificates=tuple(relation_data.get("x509certs").split(",")), # type: ignore - endpoints=tuple(endpoints), - ) - - -class SamlDataAvailableEvent(ops.RelationEvent): - """Saml event emitted when relation data has changed. - - Attrs: - saml_relation_data: the SAML relation data - entity_id: SAML entity ID. - metadata_url: URL to the metadata. - certificates: Tuple containing the SAML certificates. - endpoints: Tuple containing the SAML endpoints. - """ - - @property - def saml_relation_data(self) -> SamlRelationData: - """Get a SamlRelationData for the relation data.""" - assert self.relation.app - return SamlRelationData.from_relation_data(self.relation.data[self.relation.app]) - - @property - def entity_id(self) -> str: - """Fetch the SAML entity ID from the relation.""" - return self.saml_relation_data.entity_id - - @property - def metadata_url(self) -> typing.Optional[str]: - """Fetch the SAML metadata URL from the relation.""" - return str(self.saml_relation_data.metadata_url) - - @property - def certificates(self) -> typing.Tuple[str, ...]: - """Fetch the SAML certificates from the relation.""" - return self.saml_relation_data.certificates - - @property - def endpoints(self) -> typing.Tuple[SamlEndpoint, ...]: - """Fetch the SAML endpoints from the relation.""" - return self.saml_relation_data.endpoints - - -class SamlRequiresEvents(ops.CharmEvents): - """SAML events. - - This class defines the events that a SAML requirer can emit. - - Attrs: - saml_data_available: the SamlDataAvailableEvent. - """ - - saml_data_available = ops.EventSource(SamlDataAvailableEvent) - - -class SamlRequires(ops.Object): - """Requirer side of the SAML relation. - - Attrs: - on: events the provider can emit. - """ - - on = SamlRequiresEvents() - - def __init__(self, charm: ops.CharmBase, relation_name: str = DEFAULT_RELATION_NAME) -> None: - """Construct. - - Args: - charm: the provider charm. - relation_name: the relation name. - """ - super().__init__(charm, relation_name) - self.charm = charm - self.relation_name = relation_name - self.framework.observe(charm.on[relation_name].relation_changed, self._on_relation_changed) - - def _on_relation_changed(self, event: ops.RelationChangedEvent) -> None: - """Event emitted when the relation has changed. - - Args: - event: event triggering this handler. - """ - assert event.relation.app - if event.relation.data[event.relation.app]: - self.on.saml_data_available.emit(event.relation, app=event.app, unit=event.unit) - - def get_relation_data(self) -> typing.Optional[SamlRelationData]: - """Retrieve the relation data. - - Returns: - SmtpRelationData: the relation data. - """ - relation = self.model.get_relation(self.relation_name) - if not relation or not relation.app or not relation.data[relation.app]: - return None - return SamlRelationData.from_relation_data(relation.data[relation.app]) - - -class SamlProvides(ops.Object): - """Provider side of the SAML relation. - - Attrs: - relations: list of charm relations. - """ - - def __init__(self, charm: ops.CharmBase, relation_name: str = DEFAULT_RELATION_NAME) -> None: - """Construct. - - Args: - charm: the provider charm. - relation_name: the relation name. - """ - super().__init__(charm, relation_name) - self.charm = charm - self.relation_name = relation_name - - @property - def relations(self) -> typing.List[ops.Relation]: - """The list of Relation instances associated with this relation_name. - - Returns: - List of relations to this charm. - """ - return list(self.model.relations[self.relation_name]) - - def update_relation_data(self, relation: ops.Relation, saml_data: SamlRelationData) -> None: - """Update the relation data. - - Args: - relation: the relation for which to update the data. - saml_data: a SamlRelationData instance wrapping the data to be updated. - """ - relation.data[self.charm.model.app].update(saml_data.to_relation_data()) diff --git a/metadata.yaml b/metadata.yaml index 546dee3b..48fa06e9 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -70,10 +70,6 @@ requires: interface: redis limit: 1 optional: true - saml: - interface: saml - limit: 1 - optional: true smtp: interface: smtp limit: 1 diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 186262b6..ccedf2ab 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -18,7 +18,7 @@ Charm the service. Attrs: on: listen to Redis events. - + ### function `__init__` @@ -75,7 +75,7 @@ Unit that this execution is responsible for. --- - + ### function `build_charm_state` @@ -92,7 +92,7 @@ Build charm state. --- - + ### function `get_main_unit` @@ -109,7 +109,7 @@ Get main unit. --- - + ### function `get_main_unit_address` @@ -126,7 +126,7 @@ Get main unit address. If main unit is None, use unit name. --- - + ### function `get_signing_key` @@ -143,7 +143,7 @@ Get signing key from secret. --- - + ### function `get_unit_number` @@ -167,7 +167,7 @@ Get unit number from unit name. --- - + ### function `instance_map` @@ -184,7 +184,7 @@ Build instance_map config. --- - + ### function `is_main` @@ -202,7 +202,7 @@ Verify if this unit is the main. --- - + ### function `peer_units_total` @@ -219,7 +219,7 @@ Get peer units total. --- - + ### function `reconcile` @@ -239,7 +239,7 @@ This is the main entry for changes that require a restart. --- - + ### function `set_main_unit` @@ -257,7 +257,7 @@ Create/Renew an admin access token and put it in the peer relation. --- - + ### function `set_signing_key` diff --git a/src-docs/charm_state.py.md b/src-docs/charm_state.py.md index 60795c90..fe439b82 100644 --- a/src-docs/charm_state.py.md +++ b/src-docs/charm_state.py.md @@ -8,7 +8,7 @@ State of the Charm. --- - + ## function `inject_charm_state` @@ -84,7 +84,7 @@ Unit that this execution is responsible for. --- - + ### function `build_charm_state` @@ -96,7 +96,7 @@ Build charm state. --- - + ### function `get_charm` @@ -113,7 +113,7 @@ Return the current charm. --- - + ### function `reconcile` @@ -137,7 +137,7 @@ Exception raised when a charm configuration is found to be invalid. Attrs: msg (str): Explanation of the error. - + ### function `__init__` @@ -168,7 +168,6 @@ State of the Charm. - `synapse_config`: synapse configuration. - `datasource`: datasource information. - - `saml_config`: saml configuration. - `smtp_config`: smtp configuration. - `media_config`: media configuration. - `redis_config`: redis configuration. @@ -192,7 +191,7 @@ Get charm proxy information from juju charm environment. --- - + ### classmethod `from_charm` @@ -200,7 +199,6 @@ Get charm proxy information from juju charm environment. from_charm( charm: CharmBase, datasource: Optional[DatasourcePostgreSQL], - saml_config: Optional[SAMLConfiguration], smtp_config: Optional[SMTPConfiguration], media_config: Optional[MediaConfiguration], redis_config: Optional[RedisConfiguration], @@ -217,7 +215,6 @@ Initialize a new instance of the CharmState class from the associated charm. - `charm`: The charm instance associated with this state. - `datasource`: datasource information to be used by Synapse. - - `saml_config`: saml configuration to be used by Synapse. - `smtp_config`: SMTP configuration to be used by Synapse. - `media_config`: Media configuration to be used by Synapse. - `redis_config`: Redis configuration to be used by Synapse. @@ -243,7 +240,7 @@ Protocol that defines a class that returns a CharmBaseWithState. --- - + ### function `get_charm` @@ -307,7 +304,7 @@ Represent Synapse builtin configuration values. --- - + ### classmethod `get_default_notif_from` @@ -331,7 +328,7 @@ Set server_name as default value to notif_from. --- - + ### classmethod `roomids_to_list` @@ -360,7 +357,7 @@ Convert a comma separated list of rooms to list. --- - + ### classmethod `to_pebble_check` @@ -389,7 +386,7 @@ Convert the experimental_alive_check field to pebble check. --- - + ### classmethod `to_yes_or_no` @@ -412,7 +409,7 @@ Convert the report_stats field to yes or no. --- - + ### classmethod `userids_to_list` diff --git a/src-docs/charm_types.py.md b/src-docs/charm_types.py.md index 9b752b21..dfea833e 100644 --- a/src-docs/charm_types.py.md +++ b/src-docs/charm_types.py.md @@ -62,22 +62,6 @@ A named tuple representing Redis configuration. ---- - -## class `SAMLConfiguration` -A named tuple representing a SAML configuration. - - - -**Attributes:** - - - `entity_id`: SAML entity ID. - - `metadata_url`: URL to the metadata. - - - - - --- ## class `SMTPConfiguration` diff --git a/src-docs/saml_observer.py.md b/src-docs/saml_observer.py.md deleted file mode 100644 index 1fb45d9b..00000000 --- a/src-docs/saml_observer.py.md +++ /dev/null @@ -1,75 +0,0 @@ - - - - -# module `saml_observer.py` -The SAML integrator relation observer. - - - ---- - -## class `SAMLObserver` -The SAML Integrator relation observer. - - - -### function `__init__` - -```python -__init__(charm: CharmBaseWithState) -``` - -Initialize the observer and register event handlers. - - - -**Args:** - - - `charm`: The parent charm to attach the observer to. - - ---- - -#### property model - -Shortcut for more simple access the model. - - - ---- - - - -### function `get_charm` - -```python -get_charm() → CharmBaseWithState -``` - -Return the current charm. - - - -**Returns:** - The current charm - ---- - - - -### function `get_relation_as_saml_conf` - -```python -get_relation_as_saml_conf() → Optional[SAMLConfiguration] -``` - -Get SAML data from relation. - - - -**Returns:** - - - `Dict`: Information needed for setting environment variables. - - diff --git a/src/charm.py b/src/charm.py index deae385e..768ecd7b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -29,7 +29,6 @@ from mjolnir import Mjolnir from observability import Observability from redis_observer import RedisObserver -from saml_observer import SAMLObserver from smtp_observer import SMTPObserver from user import User @@ -62,7 +61,6 @@ def __init__(self, *args: typing.Any) -> None: self._matrix_auth = MatrixAuthObserver(self) self._media = MediaObserver(self) self._database = DatabaseObserver(self, relation_name=synapse.SYNAPSE_DB_RELATION_NAME) - self._saml = SAMLObserver(self) self._smtp = SMTPObserver(self) self._redis = RedisObserver(self) self.token_service = AdminAccessTokenService(app=self.app, model=self.model) @@ -107,7 +105,6 @@ def build_charm_state(self) -> CharmState: return CharmState.from_charm( charm=self, datasource=self._database.get_relation_as_datasource(), - saml_config=self._saml.get_relation_as_saml_conf(), smtp_config=self._smtp.get_relation_as_smtp_conf(), media_config=self._media.get_relation_as_media_conf(), redis_config=self._redis.get_relation_as_redis_conf(), diff --git a/src/charm_state.py b/src/charm_state.py index fe4d2a80..fe39404d 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -27,7 +27,6 @@ DatasourcePostgreSQL, MediaConfiguration, RedisConfiguration, - SAMLConfiguration, SMTPConfiguration, ) @@ -347,7 +346,6 @@ class CharmState: # pylint: disable=too-many-instance-attributes Attributes: synapse_config: synapse configuration. datasource: datasource information. - saml_config: saml configuration. smtp_config: smtp configuration. media_config: media configuration. redis_config: redis configuration. @@ -358,7 +356,6 @@ class CharmState: # pylint: disable=too-many-instance-attributes synapse_config: SynapseConfig datasource: typing.Optional[DatasourcePostgreSQL] - saml_config: typing.Optional[SAMLConfiguration] smtp_config: typing.Optional[SMTPConfiguration] media_config: typing.Optional[MediaConfiguration] redis_config: typing.Optional[RedisConfiguration] @@ -388,7 +385,6 @@ def from_charm( cls, charm: ops.CharmBase, datasource: typing.Optional[DatasourcePostgreSQL], - saml_config: typing.Optional[SAMLConfiguration], smtp_config: typing.Optional[SMTPConfiguration], media_config: typing.Optional[MediaConfiguration], redis_config: typing.Optional[RedisConfiguration], @@ -400,7 +396,6 @@ def from_charm( Args: charm: The charm instance associated with this state. datasource: datasource information to be used by Synapse. - saml_config: saml configuration to be used by Synapse. smtp_config: SMTP configuration to be used by Synapse. media_config: Media configuration to be used by Synapse. redis_config: Redis configuration to be used by Synapse. @@ -456,7 +451,6 @@ def from_charm( return cls( synapse_config=valid_synapse_config, datasource=datasource, - saml_config=saml_config, smtp_config=smtp_config, media_config=media_config, redis_config=redis_config, diff --git a/src/charm_types.py b/src/charm_types.py index 2b01c48e..d716c99b 100644 --- a/src/charm_types.py +++ b/src/charm_types.py @@ -28,19 +28,6 @@ class DatasourcePostgreSQL(typing.TypedDict): db: str -@dataclass(frozen=True) -class SAMLConfiguration(typing.TypedDict): - """A named tuple representing a SAML configuration. - - Attributes: - entity_id: SAML entity ID. - metadata_url: URL to the metadata. - """ - - entity_id: str - metadata_url: str - - @dataclass(frozen=True) class SMTPConfiguration(typing.TypedDict): """A named tuple representing SMTP configuration. diff --git a/src/matrix_auth_observer.py b/src/matrix_auth_observer.py index 2e15ecc4..1a01558a 100644 --- a/src/matrix_auth_observer.py +++ b/src/matrix_auth_observer.py @@ -112,7 +112,6 @@ def _get_matrix_auth_provider_data( Returns: MatrixAuthConfiguration instance. """ - # future refactor victim: this is repeated with saml homeserver = ( charm_state.synapse_config.public_baseurl if charm_state.synapse_config.public_baseurl is not None diff --git a/src/pebble.py b/src/pebble.py index 6fcd3837..5e0b8776 100644 --- a/src/pebble.py +++ b/src/pebble.py @@ -341,9 +341,6 @@ def reconcile( # noqa: C901 pylint: disable=too-many-branches,too-many-statemen logger.debug("pebble.change_config: Enabling registration_secrets") synapse.create_registration_secrets_files(container=container, charm_state=charm_state) synapse.enable_registration_secrets(current_synapse_config, charm_state=charm_state) - if charm_state.saml_config is not None: - logger.debug("pebble.change_config: Enabling SAML") - synapse.enable_saml(current_synapse_config, charm_state=charm_state) if charm_state.smtp_config is not None: logger.debug("pebble.change_config: Enabling SMTP") synapse.enable_smtp(current_synapse_config, charm_state=charm_state) diff --git a/src/saml_observer.py b/src/saml_observer.py deleted file mode 100644 index 42e7a0f7..00000000 --- a/src/saml_observer.py +++ /dev/null @@ -1,91 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -"""The SAML integrator relation observer.""" - -import logging -import typing - -import ops -from charms.saml_integrator.v0.saml import SamlDataAvailableEvent, SamlRequires -from ops.charm import RelationBrokenEvent -from ops.framework import Object - -from charm_state import CharmBaseWithState, CharmState, inject_charm_state -from charm_types import SAMLConfiguration - -logger = logging.getLogger(__name__) - - -class SAMLObserver(Object): - """The SAML Integrator relation observer.""" - - _RELATION_NAME = "saml" - - def __init__(self, charm: CharmBaseWithState): - """Initialize the observer and register event handlers. - - Args: - charm: The parent charm to attach the observer to. - """ - super().__init__(charm, "saml-observer") - self._charm = charm - self.saml = SamlRequires(self._charm) - self.framework.observe(self.saml.on.saml_data_available, self._on_saml_data_available) - self.framework.observe( - charm.on[self.saml.relation_name].relation_broken, self._on_relation_broken - ) - - def get_charm(self) -> CharmBaseWithState: - """Return the current charm. - - Returns: - The current charm - """ - return self._charm - - @inject_charm_state - def _on_saml_data_available(self, _: SamlDataAvailableEvent, charm_state: CharmState) -> None: - """Handle SAML data available. - - Args: - charm_state: The charm state. - """ - self.model.unit.status = ops.MaintenanceStatus("Preparing the SAML integration") - logger.debug("_on_saml_data_available emitting reconcile") - self.get_charm().reconcile(charm_state) - - @inject_charm_state - def _on_relation_broken(self, _: RelationBrokenEvent, charm_state: CharmState) -> None: - """Handle SAML data available. - - Args: - charm_state: The charm state. - """ - self.model.unit.status = ops.MaintenanceStatus("Reloading homeserver configuration") - logger.debug("_on_relation_broken emitting reconcile") - self.get_charm().reconcile(charm_state) - - def get_relation_as_saml_conf(self) -> typing.Optional[SAMLConfiguration]: - """Get SAML data from relation. - - Returns: - Dict: Information needed for setting environment variables. - """ - if not self.model.relations.get(self._RELATION_NAME): - return None - - relation_data = {} - relations = list(self._charm.model.relations[self._RELATION_NAME]) - relation_id = relations[0].id - for relation in relations: - relation_data[relation.id] = ( - {key: value for key, value in relation.data[relation.app].items() if key != "data"} - if relation.app - else {} - ) - - return SAMLConfiguration( - entity_id=relation_data[relation_id].get("entity_id", ""), - metadata_url=relation_data[relation_id].get("metadata_url", ""), - ) diff --git a/src/synapse/__init__.py b/src/synapse/__init__.py index bb95f49c..67594b06 100644 --- a/src/synapse/__init__.py +++ b/src/synapse/__init__.py @@ -88,7 +88,6 @@ enable_registration_secrets, enable_replication, enable_room_list_publication_rules, - enable_saml, enable_serve_server_wellknown, enable_smtp, enable_stale_devices_deletion, diff --git a/src/synapse/workload.py b/src/synapse/workload.py index 2b3812b2..5aeb31cf 100644 --- a/src/synapse/workload.py +++ b/src/synapse/workload.py @@ -81,10 +81,6 @@ class CreateMjolnirConfigError(WorkloadError): """Exception raised when something goes wrong while creating mjolnir config.""" -class EnableSAMLError(WorkloadError): - """Exception raised when something goes wrong while enabling SAML.""" - - class EnableSMTPError(WorkloadError): """Exception raised when something goes wrong while enabling SMTP.""" diff --git a/src/synapse/workload_configuration.py b/src/synapse/workload_configuration.py index e56a9406..1e3e2418 100644 --- a/src/synapse/workload_configuration.py +++ b/src/synapse/workload_configuration.py @@ -6,17 +6,10 @@ """Helper module used to manage interactions with Synapse homeserver configuration.""" import logging -import typing from charm_state import CharmState -from .workload import ( - SYNAPSE_EXPORTER_PORT, - EnableMetricsError, - EnableSAMLError, - EnableSMTPError, - WorkloadError, -) +from .workload import SYNAPSE_EXPORTER_PORT, EnableMetricsError, EnableSMTPError, WorkloadError logger = logging.getLogger(__name__) @@ -374,91 +367,6 @@ def enable_synapse_invite_checker(current_yaml: dict, charm_state: CharmState) - raise WorkloadError(str(exc)) from exc -def _create_pysaml2_config(charm_state: CharmState) -> typing.Dict: - """Create config as expected by pysaml2. - - Args: - charm_state: Instance of CharmState. - - Returns: - Pysaml2 configuration. - - Raises: - EnableSAMLError: if SAML configuration is not found. - """ - if charm_state.saml_config is None: - raise EnableSAMLError( - "SAML Configuration not found. " - "Please verify the integration between SAML Integrator and Synapse." - ) - - saml_config = charm_state.saml_config - entity_id = charm_state.synapse_config.public_baseurl - sp_config = { - "metadata": { - "remote": [ - { - "url": saml_config["metadata_url"], - }, - ], - }, - "allow_unknown_attributes": True, - "service": { - "sp": { - "entityId": entity_id, - "allow_unsolicited": True, - }, - }, - } - # login.staging.ubuntu.com and login.ubuntu.com - # dont send uid in SAMLResponse so this will map - # as expected - if "ubuntu.com" in saml_config["metadata_url"]: - sp_config["attribute_map_dir"] = "/usr/local/attributemaps" - - return sp_config - - -def enable_saml(current_yaml: dict, charm_state: CharmState) -> None: - """Change the Synapse configuration to enable SAML. - - Args: - current_yaml: current configuration. - charm_state: Instance of CharmState. - - Raises: - EnableSAMLError: something went wrong enabling SAML. - """ - try: - # enable x_forwarded to pass expected headers - current_listeners = current_yaml["listeners"] - updated_listeners = [ - { - **item, - "x_forwarded": ( - True - if "x_forwarded" in item and not item["x_forwarded"] - else item.get("x_forwarded", False) - ), - } - for item in current_listeners - ] - current_yaml["listeners"] = updated_listeners - current_yaml["saml2_enabled"] = True - current_yaml["saml2_config"] = {} - current_yaml["saml2_config"]["sp_config"] = _create_pysaml2_config(charm_state) - user_mapping_provider_config = { - "config": { - "mxid_source_attribute": "uid", - "grandfathered_mxid_source_attribute": "uid", - "mxid_mapping": "dotreplace", - }, - } - current_yaml["saml2_config"]["user_mapping_provider"] = user_mapping_provider_config - except KeyError as exc: - raise EnableSAMLError(str(exc)) from exc - - def enable_serve_server_wellknown(current_yaml: dict) -> None: """Change the Synapse configuration to enable server wellknown file. diff --git a/synapse_rock/attributemaps/login_ubuntu.py b/synapse_rock/attributemaps/login_ubuntu.py deleted file mode 100644 index 012798e7..00000000 --- a/synapse_rock/attributemaps/login_ubuntu.py +++ /dev/null @@ -1,11 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -MAP = { - "identifier": "urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified", - "fro": { - 'username': 'uid', - 'fullname': 'displayName', - 'email': 'email', - } -} diff --git a/tests/integration/test_nginx.py b/tests/integration/test_nginx.py deleted file mode 100644 index 912ac7e5..00000000 --- a/tests/integration/test_nginx.py +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Integration tests for Synapse charm needing the nginx_integrator_app fixture.""" -import logging -import typing - -import pytest -import requests -from juju.application import Application -from juju.model import Model -from ops.model import ActiveStatus -from saml_test_helper import SamlK8sTestHelper - -import synapse - -# caused by pytest fixtures -# pylint: disable=too-many-arguments - -# mypy has trouble to inferred types for variables that are initialized in subclasses. -ACTIVE_STATUS_NAME = typing.cast(str, ActiveStatus.name) # type: ignore - -logger = logging.getLogger(__name__) - - -@pytest.mark.asyncio -@pytest.mark.usefixtures("nginx_integrator_app") -async def test_saml_auth( # pylint: disable=too-many-locals - model: Model, - model_name: str, - synapse_app: Application, - get_unit_ips: typing.Callable[[str], typing.Awaitable[tuple[str, ...]]], -): - """ - arrange: integrate Synapse with SAML and NGINX charms, upload metadata to samltest.id - and configure public_baseurl. - act: simulate a user authenticating via SAML. - assert: The SAML authentication process is executed successfully. - """ - synapse_config = await synapse_app.get_config() - server_name = synapse_config["server_name"]["value"] - assert server_name - saml_helper = SamlK8sTestHelper.deploy_saml_idp(model_name) - - saml_integrator_app: Application = await model.deploy( - "saml-integrator", - channel="latest/edge", - series="jammy", - trust=True, - ) - await model.wait_for_idle() - saml_helper.prepare_pod(model_name, f"{saml_integrator_app.name}-0") - saml_helper.prepare_pod(model_name, f"{synapse_app.name}-0") - await saml_integrator_app.set_config( - { - "entity_id": f"https://{saml_helper.SAML_HOST}/metadata", - "metadata_url": f"https://{saml_helper.SAML_HOST}/metadata", - } - ) - await model.wait_for_idle(idle_period=30) - await model.add_relation(saml_integrator_app.name, synapse_app.name) - await model.wait_for_idle( - idle_period=30, - apps=[synapse_app.name, saml_integrator_app.name], - status=ACTIVE_STATUS_NAME, - ) - - session = requests.session() - headers = {"Host": server_name} - for unit_ip in await get_unit_ips(synapse_app.name): - response = session.get( - f"http://{unit_ip}:{synapse.SYNAPSE_NGINX_PORT}/_synapse/client/saml2/metadata.xml", - timeout=10, - headers=headers, - ) - assert response.status_code == 200 - saml_helper.register_service_provider(name=server_name, metadata=response.text) - saml_page_path = "_matrix/client/r0/login/sso/redirect/saml" - saml_page_params = "redirectUrl=http%3A%2F%2Flocalhost%2F&org.matrix.msc3824.action=login" - redirect_response = session.get( - f"http://{unit_ip}:{synapse.SYNAPSE_NGINX_PORT}/{saml_page_path}?{saml_page_params}", - verify=False, - timeout=10, - headers=headers, - allow_redirects=False, - ) - assert redirect_response.status_code == 302 - redirect_url = redirect_response.headers["Location"] - saml_response = saml_helper.redirect_sso_login(redirect_url) - assert f"https://{server_name}" in saml_response.url - - url = saml_response.url.replace( - f"https://{server_name}", f"http://{unit_ip}:{synapse.SYNAPSE_NGINX_PORT}" - ) - logged_in_page = session.post(url, data=saml_response.data, headers={"Host": server_name}) - - assert logged_in_page.status_code == 200 - assert "Continue to your account" in logged_in_page.text diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 4bfc67fe..979784e6 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -196,20 +196,6 @@ def start_cmd_handler(argv: list[str]) -> synapse.ExecResult: harness.cleanup() -@pytest.fixture(name="saml_configured") -def saml_configured_fixture(harness: Harness) -> Harness: - """Harness fixture with saml relation configured""" - harness.update_config({"server_name": TEST_SERVER_NAME, "public_baseurl": TEST_SERVER_NAME}) - saml_relation_data = { - "entity_id": "https://login.staging.ubuntu.com", - "metadata_url": "https://login.staging.ubuntu.com/saml/metadata", - } - harness.add_relation("saml", "saml-integrator", app_data=saml_relation_data) - harness.set_can_connect(synapse.SYNAPSE_CONTAINER_NAME, True) - harness.set_leader(True) - return harness - - @pytest.fixture(name="smtp_configured") def smtp_configured_fixture(harness: Harness) -> Harness: """Harness fixture with smtp relation configured""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 432e5911..fe01258b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -7,7 +7,6 @@ import io import json -import typing from unittest.mock import MagicMock import ops @@ -178,24 +177,6 @@ def test_traefik_integration(harness: Harness) -> None: } -def test_saml_integration_container_down(saml_configured: Harness) -> None: - """ - arrange: start the Synapse charm, set server_name, set Synapse container to be down. - act: emit saml_data_available. - assert: Synapse charm should submit the correct status. - """ - harness = saml_configured - harness.begin() - harness.set_can_connect(harness.model.unit.containers[synapse.SYNAPSE_CONTAINER_NAME], False) - relation = harness.charm.framework.model.get_relation("saml", 0) - - harness.charm._saml.saml.on.saml_data_available.emit(relation) - - assert isinstance(harness.model.unit.status, ops.MaintenanceStatus) - assert "Waiting for" in str(harness.model.unit.status) - harness.cleanup() - - def test_smtp_integration_container_down(smtp_configured: Harness) -> None: """ arrange: start the Synapse charm, set server_name, set Synapse container to be down. @@ -420,27 +401,6 @@ def test_redis_configuration_success(redis_configured: Harness, monkeypatch: pyt assert "1010" == str(redis_config["port"]) -def test_saml_enabled_reconcile_pebble_error( - saml_configured: Harness, monkeypatch: pytest.MonkeyPatch -) -> None: - """ - arrange: start the Synapse charm, set server_name, mock pebble to give an error. - act: emit saml_data_available. - assert: Synapse charm should submit the correct status. - """ - harness = saml_configured - harness.begin() - error_message = "Fail" - reconcile_mock = MagicMock(side_effect=PebbleServiceError(error_message)) - monkeypatch.setattr(pebble, "reconcile", reconcile_mock) - - relation = harness.charm.framework.model.get_relation("saml", 0) - harness.charm._saml.saml.on.saml_data_available.emit(relation) - - assert isinstance(harness.model.unit.status, ops.BlockedStatus) - assert error_message in str(harness.model.unit.status) - - def test_smtp_enabled_reconcile_pebble_error( smtp_configured: Harness, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -480,22 +440,3 @@ def test_redis_enabled_reconcile_pebble_error( assert isinstance(harness.model.unit.status, ops.BlockedStatus) assert error_message in str(harness.model.unit.status) - - -def test_saml_on_relation_broken( - saml_configured: Harness, monkeypatch: pytest.MonkeyPatch -) -> None: - """ - arrange: start the Synapse charm with saml integration, set server_name, mock pebble. - act: remove the saml integration. - assert: Synapse charm should correctly reconcile. - """ - harness = saml_configured - harness.begin() - reconcile_mock = MagicMock() - monkeypatch.setattr(pebble, "reconcile", reconcile_mock) - - relation = typing.cast(ops.model.Relation, harness.model.get_relation("saml")) - harness.remove_relation(relation.id) - - reconcile_mock.assert_called_once() diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index 9e920ea1..6f19224b 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -34,7 +34,6 @@ def build_charm_state(self) -> CharmState: return CharmState( synapse_config=synapse_config, datasource=None, - saml_config=None, smtp_config=None, media_config=None, redis_config=None, diff --git a/tests/unit/test_synapse_api.py b/tests/unit/test_synapse_api.py index 9cfb7f03..e39e7f9f 100644 --- a/tests/unit/test_synapse_api.py +++ b/tests/unit/test_synapse_api.py @@ -226,7 +226,6 @@ def test_override_rate_limit_success(monkeypatch: pytest.MonkeyPatch): charm_state = CharmState( synapse_config=synapse_config, datasource=None, - saml_config=None, smtp_config=None, media_config=None, redis_config=None, @@ -265,7 +264,6 @@ def test_override_rate_limit_error(monkeypatch: pytest.MonkeyPatch): charm_state = CharmState( synapse_config=synapse_config, datasource=None, - saml_config=None, smtp_config=None, media_config=None, redis_config=None, diff --git a/tests/unit/test_synapse_workload.py b/tests/unit/test_synapse_workload.py index 913028be..55492af4 100644 --- a/tests/unit/test_synapse_workload.py +++ b/tests/unit/test_synapse_workload.py @@ -18,12 +18,9 @@ from pydantic.v1 import ValidationError import synapse -from charm import SynapseCharm from charm_state import CharmState, SynapseConfig from charm_types import SMTPConfiguration -from .conftest import TEST_SERVER_NAME - def test_allow_public_rooms_over_federation_sucess(config_content: dict[str, typing.Any]): """ @@ -251,7 +248,6 @@ def test_enable_trusted_key_servers_no_action(config_content: dict[str, typing.A content, CharmState( # pylint: disable=duplicate-code datasource=None, - saml_config=None, smtp_config=None, media_config=None, redis_config=None, @@ -342,136 +338,6 @@ def test_enable_forgotten_room_success(config_content: dict[str, typing.Any]): assert yaml.safe_dump(content) == yaml.safe_dump(expected_config_content) -def test_enable_saml_success(): - """ - arrange: set mock container with file. - act: change the configuration file. - assert: new configuration file is pushed and SAML is enabled. - """ - # This test was given as an example in this comment by Ben Hoyt. - # https://github.com/canonical/synapse-operator/pull/19#discussion_r1302486670 - # Arrange: set up harness and container filesystem - harness = Harness(SynapseCharm) - harness.update_config({"server_name": TEST_SERVER_NAME, "public_baseurl": TEST_SERVER_NAME}) - relation_id = harness.add_relation("saml", "saml-integrator") - harness.add_relation_unit(relation_id, "saml-integrator/0") - metadata_url = "https://login.staging.ubuntu.com/saml/metadata" - harness.update_relation_data( - relation_id, - "saml-integrator", - { - "entity_id": "https://login.staging.ubuntu.com", - "metadata_url": metadata_url, - }, - ) - harness.set_can_connect(synapse.SYNAPSE_CONTAINER_NAME, True) - harness.begin() - current_config = """ -listeners: - - type: http - port: 8080 - bind_addresses: - - "::" - x_forwarded: false -""" - - config = yaml.safe_load(current_config) - - synapse.enable_saml(config, harness.charm.build_charm_state()) - - # Assert: ensure config file was written correctly - expected_config_content = { - "listeners": [ - {"type": "http", "x_forwarded": True, "port": 8080, "bind_addresses": ["::"]} - ], - "saml2_enabled": True, - "saml2_config": { - "sp_config": { - "metadata": {"remote": [{"url": metadata_url}]}, - "service": { - "sp": { - "entityId": TEST_SERVER_NAME, - "allow_unsolicited": True, - } - }, - "allow_unknown_attributes": True, - "attribute_map_dir": "/usr/local/attributemaps", - }, - "user_mapping_provider": { - "config": { - "grandfathered_mxid_source_attribute": "uid", - "mxid_source_attribute": "uid", - "mxid_mapping": "dotreplace", - } - }, - }, - } - assert yaml.safe_dump(config) == yaml.safe_dump(expected_config_content) - - -def test_enable_saml_success_no_ubuntu_url(): - """ - arrange: set configuration and saml-integrator relation without ubuntu.com - in metadata_url. - act: enable saml. - assert: SAML configuration is created as expected. - """ - harness = Harness(SynapseCharm) - harness.update_config({"server_name": TEST_SERVER_NAME, "public_baseurl": TEST_SERVER_NAME}) - relation_id = harness.add_relation("saml", "saml-integrator") - harness.add_relation_unit(relation_id, "saml-integrator/0") - metadata_url = "https://login.staging.com/saml/metadata" - harness.update_relation_data( - relation_id, - "saml-integrator", - { - "entity_id": "https://login.staging.com", - "metadata_url": metadata_url, - }, - ) - harness.set_can_connect(synapse.SYNAPSE_CONTAINER_NAME, True) - harness.begin() - current_config = """ -listeners: - - type: http - port: 8080 - bind_addresses: - - "::" - x_forwarded: false -""" - - config = yaml.safe_load(current_config) - - synapse.enable_saml(config, harness.charm.build_charm_state()) - - expected_config_content = { - "listeners": [ - {"type": "http", "x_forwarded": True, "port": 8080, "bind_addresses": ["::"]} - ], - "saml2_enabled": True, - "saml2_config": { - "sp_config": { - "metadata": {"remote": [{"url": metadata_url}]}, - "service": { - "sp": { - "entityId": TEST_SERVER_NAME, - "allow_unsolicited": True, - } - }, - "allow_unknown_attributes": True, - }, - "user_mapping_provider": { - "config": { - "grandfathered_mxid_source_attribute": "uid", - "mxid_source_attribute": "uid", - "mxid_mapping": "dotreplace", - } - }, - }, - } - assert yaml.safe_dump(config) == yaml.safe_dump(expected_config_content) - - def test_get_mjolnir_config_success(): """ arrange: set access token and room id parameters. @@ -536,7 +402,6 @@ def test_enable_smtp_success(config_content: dict[str, typing.Any]): synapse_config = SynapseConfig(**synapse_with_notif_config) # type: ignore[arg-type] charm_state = CharmState( datasource=None, - saml_config=None, smtp_config=SMTP_CONFIGURATION, media_config=None, redis_config=None, @@ -713,7 +578,6 @@ def test_block_non_admin_invites(config_content: dict[str, typing.Any]): synapse_config = SynapseConfig(**block_non_admin_invites) # type: ignore[arg-type] charm_state = CharmState( datasource=None, - saml_config=None, smtp_config=SMTP_CONFIGURATION, redis_config=None, synapse_config=synapse_config, @@ -749,7 +613,6 @@ def test_publish_rooms_allowlist_success(config_content: dict[str, typing.Any]): synapse_config = SynapseConfig(**synapse_with_notif_config) # type: ignore[arg-type] charm_state = CharmState( datasource=None, - saml_config=None, smtp_config=SMTP_CONFIGURATION, redis_config=None, synapse_config=synapse_config, @@ -863,7 +726,6 @@ def test_invite_checker_policy_rooms(config_content: dict[str, typing.Any]): synapse_config = SynapseConfig(**invite_checker_policy_rooms) # type: ignore[arg-type] charm_state = CharmState( datasource=None, - saml_config=None, smtp_config=SMTP_CONFIGURATION, redis_config=None, synapse_config=synapse_config, @@ -906,7 +768,6 @@ def test_invite_checker_blocklist_allowlist_url(config_content: dict[str, typing synapse_config = SynapseConfig(**invite_checker_blocklist_allowlist_url) # type: ignore[arg-type] # noqa: E501 charm_state = CharmState( datasource=None, - saml_config=None, smtp_config=SMTP_CONFIGURATION, redis_config=None, synapse_config=synapse_config, diff --git a/tox.ini b/tox.ini index 0242d5c6..2e93538f 100644 --- a/tox.ini +++ b/tox.ini @@ -67,7 +67,6 @@ deps = requests types-PyYAML types-requests - git+https://github.com/canonical/saml-test-idp.git -r{toxinidir}/requirements.txt commands = pydocstyle {[vars]src_path} @@ -126,7 +125,6 @@ deps = boto3 # Type error problem with newer version of macaroonbakery macaroonbakery==1.3.2 - git+https://github.com/canonical/saml-test-idp.git -r{toxinidir}/requirements.txt commands = pytest -v -x --tb native --ignore={[vars]tst_path}unit --log-cli-level=INFO -s {posargs}