Fix SAML entity_id usage (#37)

canonical · Sep 14, 2023 · 74a8a52 · 74a8a52
1 parent 0e9bf07
commit 74a8a52
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 11 deletions.
diff --git a/config.yaml b/config.yaml
@@ -17,5 +17,5 @@ options:
     type: string
     description: |
       The public-facing base URL that clients use to access this Homeserver.
-      Defaults to https://<server_name>/. Only used if there is integration with
-      SAML integrator charm.
+      It's used as entity_id if set instead of https://server_name. Only used if
+      there is integration with SAML integrator charm.
diff --git a/src/synapse/workload.py b/src/synapse/workload.py
@@ -186,6 +186,11 @@ def _create_pysaml2_config(charm_state: CharmState) -> typing.Dict:
         )
 
     saml_config = charm_state.saml_config
+    entity_id = (
+        charm_state.public_baseurl
+        if charm_state.public_baseurl is not None
+        else f"https://{charm_state.server_name}"
+    )
     sp_config = {
         "metadata": {
             "remote": [
@@ -197,12 +202,12 @@ def _create_pysaml2_config(charm_state: CharmState) -> typing.Dict:
         "allow_unknown_attributes": True,
         "service": {
             "sp": {
-                "entityId": saml_config["entity_id"],
+                "entityId": entity_id,
                 "allow_unsolicited": True,
             },
         },
     }
-    # login.staging.canonical.com and login.canonical.com
+    # login.staging.ubuntu.com and login.ubuntu.com
     # dont send uid in SAMLResponse so this will map
     # fullname to uid
     if "ubuntu.com" in saml_config["metadata_url"]:

diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -219,7 +219,7 @@ async def grafana_app_fixture(
         app = await model.deploy(
             "grafana-k8s",
             application_name=grafana_app_name,
-            channel="1.0/edge",
+            channel="stable",
             trust=True,
         )
         await model.wait_for_idle(raise_on_blocked=True, status=ACTIVE_STATUS_NAME)
@@ -244,7 +244,7 @@ async def deploy_prometheus_fixture(
         app = await model.deploy(
             "prometheus-k8s",
             application_name=prometheus_app_name,
-            channel="1.0/edge",
+            channel="stable",
             trust=True,
         )
         await model.wait_for_idle(raise_on_blocked=True, status=ACTIVE_STATUS_NAME)

diff --git a/tests/unit/test_synapse_workload.py b/tests/unit/test_synapse_workload.py
@@ -81,12 +81,13 @@ def test_enable_saml_success():
     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": "https://login.staging.ubuntu.com/saml/metadata",
+            "metadata_url": metadata_url,
         },
     )
     harness.set_can_connect(SYNAPSE_CONTAINER_NAME, True)
@@ -118,12 +119,10 @@ def test_enable_saml_success():
         "saml2_enabled": True,
         "saml2_config": {
             "sp_config": {
-                "metadata": {
-                    "remote": [{"url": "https://login.staging.ubuntu.com/saml/metadata"}]
-                },
+                "metadata": {"remote": [{"url": metadata_url}]},
                 "service": {
                     "sp": {
-                        "entityId": "https://login.staging.ubuntu.com",
+                        "entityId": TEST_SERVER_NAME,
                         "allow_unsolicited": True,
                     }
                 },