diff --git a/changelog.d/17407.misc b/changelog.d/17407.misc
new file mode 100644
index 0000000000..9ed6e61a5b
--- /dev/null
+++ b/changelog.d/17407.misc
@@ -0,0 +1 @@
+MSC3861: load the issuer and account management URLs from OIDC discovery.
diff --git a/synapse/api/auth/msc3861_delegated.py b/synapse/api/auth/msc3861_delegated.py
index 7361666c77..6bd845c7e3 100644
--- a/synapse/api/auth/msc3861_delegated.py
+++ b/synapse/api/auth/msc3861_delegated.py
@@ -121,7 +121,9 @@ class MSC3861DelegatedAuth(BaseAuth):
self._hostname = hs.hostname
self._admin_token = self._config.admin_token
- self._issuer_metadata = RetryOnExceptionCachedCall(self._load_metadata)
+ self._issuer_metadata = RetryOnExceptionCachedCall[OpenIDProviderMetadata](
+ self._load_metadata
+ )
if isinstance(auth_method, PrivateKeyJWTWithKid):
# Use the JWK as the client secret when using the private_key_jwt method
@@ -145,6 +147,33 @@ class MSC3861DelegatedAuth(BaseAuth):
# metadata.validate_introspection_endpoint()
return metadata
+ async def issuer(self) -> str:
+ """
+ Get the configured issuer
+
+ This will use the issuer value set in the metadata,
+ falling back to the one set in the config if not set in the metadata
+ """
+ metadata = await self._issuer_metadata.get()
+ return metadata.issuer or self._config.issuer
+
+ async def account_management_url(self) -> Optional[str]:
+ """
+ Get the configured account management URL
+
+ This will discover the account management URL from the issuer if it's not set in the config
+ """
+ if self._config.account_management_url is not None:
+ return self._config.account_management_url
+
+ try:
+ metadata = await self._issuer_metadata.get()
+ return metadata.get("account_management_uri", None)
+ # We don't want to raise here if we can't load the metadata
+ except Exception:
+ logger.warning("Failed to load metadata:", exc_info=True)
+ return None
+
async def _introspection_endpoint(self) -> str:
"""
Returns the introspection endpoint of the issuer
@@ -154,7 +183,7 @@ class MSC3861DelegatedAuth(BaseAuth):
if self._config.introspection_endpoint is not None:
return self._config.introspection_endpoint
- metadata = await self._load_metadata()
+ metadata = await self._issuer_metadata.get()
return metadata.get("introspection_endpoint")
async def _introspect_token(self, token: str) -> IntrospectionToken:
diff --git a/synapse/rest/client/auth.py b/synapse/rest/client/auth.py
index 32eeecd662..b8dca7c797 100644
--- a/synapse/rest/client/auth.py
+++ b/synapse/rest/client/auth.py
@@ -20,7 +20,7 @@
#
import logging
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, cast
from twisted.web.server import Request
@@ -70,11 +70,17 @@ class AuthRestServlet(RestServlet):
self.hs.config.experimental.msc3861.enabled
and stagetype == "org.matrix.cross_signing_reset"
):
- config = self.hs.config.experimental.msc3861
- if config.account_management_url is not None:
- url = f"{config.account_management_url}?action=org.matrix.cross_signing_reset"
+ # If MSC3861 is enabled, we can assume self._auth is an instance of MSC3861DelegatedAuth
+ # We import lazily here because of the authlib requirement
+ from synapse.api.auth.msc3861_delegated import MSC3861DelegatedAuth
+
+ auth = cast(MSC3861DelegatedAuth, self.auth)
+
+ url = await auth.account_management_url()
+ if url is not None:
+ url = f"{url}?action=org.matrix.cross_signing_reset"
else:
- url = config.issuer
+ url = await auth.issuer()
respond_with_redirect(request, str.encode(url))
if stagetype == LoginType.RECAPTCHA:
diff --git a/synapse/rest/client/auth_issuer.py b/synapse/rest/client/auth_issuer.py
index 77b9720956..acd0399d85 100644
--- a/synapse/rest/client/auth_issuer.py
+++ b/synapse/rest/client/auth_issuer.py
@@ -13,7 +13,7 @@
# limitations under the License.
import logging
import typing
-from typing import Tuple
+from typing import Tuple, cast
from synapse.api.errors import Codes, SynapseError
from synapse.http.server import HttpServer
@@ -43,10 +43,16 @@ class AuthIssuerServlet(RestServlet):
def __init__(self, hs: "HomeServer"):
super().__init__()
self._config = hs.config
+ self._auth = hs.get_auth()
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
if self._config.experimental.msc3861.enabled:
- return 200, {"issuer": self._config.experimental.msc3861.issuer}
+ # If MSC3861 is enabled, we can assume self._auth is an instance of MSC3861DelegatedAuth
+ # We import lazily here because of the authlib requirement
+ from synapse.api.auth.msc3861_delegated import MSC3861DelegatedAuth
+
+ auth = cast(MSC3861DelegatedAuth, self._auth)
+ return 200, {"issuer": await auth.issuer()}
else:
# Wouldn't expect this to be reached: the servelet shouldn't have been
# registered. Still, fail gracefully if we are registered for some reason.
diff --git a/synapse/rest/client/keys.py b/synapse/rest/client/keys.py
index a33eb6c1f2..7025662fdc 100644
--- a/synapse/rest/client/keys.py
+++ b/synapse/rest/client/keys.py
@@ -23,7 +23,7 @@
import logging
import re
from collections import Counter
-from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple, cast
from synapse.api.errors import (
InteractiveAuthIncompleteError,
@@ -406,11 +406,17 @@ class SigningKeyUploadServlet(RestServlet):
# explicitly mark the master key as replaceable.
if self.hs.config.experimental.msc3861.enabled:
if not master_key_updatable_without_uia:
- config = self.hs.config.experimental.msc3861
- if config.account_management_url is not None:
- url = f"{config.account_management_url}?action=org.matrix.cross_signing_reset"
+ # If MSC3861 is enabled, we can assume self.auth is an instance of MSC3861DelegatedAuth
+ # We import lazily here because of the authlib requirement
+ from synapse.api.auth.msc3861_delegated import MSC3861DelegatedAuth
+
+ auth = cast(MSC3861DelegatedAuth, self.auth)
+
+ uri = await auth.account_management_url()
+ if uri is not None:
+ url = f"{uri}?action=org.matrix.cross_signing_reset"
else:
- url = config.issuer
+ url = await auth.issuer()
# We use a dummy session ID as this isn't really a UIA flow, but we
# reuse the same API shape for better client compatibility.
diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py
index ae691bcdba..03b1e7edc4 100644
--- a/synapse/rest/client/login.py
+++ b/synapse/rest/client/login.py
@@ -268,7 +268,7 @@ class LoginRestServlet(RestServlet):
approval_notice_medium=ApprovalNoticeMedium.NONE,
)
- well_known_data = self._well_known_builder.get_well_known()
+ well_known_data = await self._well_known_builder.get_well_known()
if well_known_data:
result["well_known"] = well_known_data
return 200, result
diff --git a/synapse/rest/well_known.py b/synapse/rest/well_known.py
index d0ca8ca46b..989e570671 100644
--- a/synapse/rest/well_known.py
+++ b/synapse/rest/well_known.py
@@ -18,12 +18,13 @@
#
#
import logging
-from typing import TYPE_CHECKING, Optional
+from typing import TYPE_CHECKING, Optional, Tuple, cast
from twisted.web.resource import Resource
from twisted.web.server import Request
-from synapse.http.server import set_cors_headers
+from synapse.api.errors import NotFoundError
+from synapse.http.server import DirectServeJsonResource
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict
from synapse.util import json_encoder
@@ -38,8 +39,9 @@ logger = logging.getLogger(__name__)
class WellKnownBuilder:
def __init__(self, hs: "HomeServer"):
self._config = hs.config
+ self._auth = hs.get_auth()
- def get_well_known(self) -> Optional[JsonDict]:
+ async def get_well_known(self) -> Optional[JsonDict]:
if not self._config.server.serve_client_wellknown:
return None
@@ -52,13 +54,20 @@ class WellKnownBuilder:
# We use the MSC3861 values as they are used by multiple MSCs
if self._config.experimental.msc3861.enabled:
+ # If MSC3861 is enabled, we can assume self._auth is an instance of MSC3861DelegatedAuth
+ # We import lazily here because of the authlib requirement
+ from synapse.api.auth.msc3861_delegated import MSC3861DelegatedAuth
+
+ auth = cast(MSC3861DelegatedAuth, self._auth)
+
result["org.matrix.msc2965.authentication"] = {
- "issuer": self._config.experimental.msc3861.issuer
+ "issuer": await auth.issuer(),
}
- if self._config.experimental.msc3861.account_management_url is not None:
+ account_management_url = await auth.account_management_url()
+ if account_management_url is not None:
result["org.matrix.msc2965.authentication"][
"account"
- ] = self._config.experimental.msc3861.account_management_url
+ ] = account_management_url
if self._config.server.extra_well_known_client_content:
for (
@@ -71,26 +80,22 @@ class WellKnownBuilder:
return result
-class ClientWellKnownResource(Resource):
+class ClientWellKnownResource(DirectServeJsonResource):
"""A Twisted web resource which renders the .well-known/matrix/client file"""
isLeaf = 1
def __init__(self, hs: "HomeServer"):
- Resource.__init__(self)
+ super().__init__()
self._well_known_builder = WellKnownBuilder(hs)
- def render_GET(self, request: SynapseRequest) -> bytes:
- set_cors_headers(request)
- r = self._well_known_builder.get_well_known()
+ async def _async_render_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
+ r = await self._well_known_builder.get_well_known()
if not r:
- request.setResponseCode(404)
- request.setHeader(b"Content-Type", b"text/plain")
- return b".well-known not available"
+ raise NotFoundError(".well-known not available")
logger.debug("returning: %s", r)
- request.setHeader(b"Content-Type", b"application/json")
- return json_encoder.encode(r).encode("utf-8")
+ return 200, r
class ServerWellKnownResource(Resource):
diff --git a/tests/rest/client/test_auth_issuer.py b/tests/rest/client/test_auth_issuer.py
index 964baeec32..299475a35c 100644
--- a/tests/rest/client/test_auth_issuer.py
+++ b/tests/rest/client/test_auth_issuer.py
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from http import HTTPStatus
+from unittest.mock import AsyncMock
from synapse.rest.client import auth_issuer
@@ -50,10 +51,27 @@ class AuthIssuerTestCase(HomeserverTestCase):
}
)
def test_returns_issuer_when_oidc_enabled(self) -> None:
- # Make an unauthenticated request for the discovery info.
+ # Patch the HTTP client to return the issuer metadata
+ req_mock = AsyncMock(return_value={"issuer": ISSUER})
+ self.hs.get_proxied_http_client().get_json = req_mock # type: ignore[method-assign]
+
+ channel = self.make_request(
+ "GET",
+ "/_matrix/client/unstable/org.matrix.msc2965/auth_issuer",
+ )
+
+ self.assertEqual(channel.code, HTTPStatus.OK)
+ self.assertEqual(channel.json_body, {"issuer": ISSUER})
+
+ req_mock.assert_called_with("https://account.example.com/.well-known/openid-configuration")
+ req_mock.reset_mock()
+
+ # Second call it should use the cached value
channel = self.make_request(
"GET",
"/_matrix/client/unstable/org.matrix.msc2965/auth_issuer",
)
+
self.assertEqual(channel.code, HTTPStatus.OK)
self.assertEqual(channel.json_body, {"issuer": ISSUER})
+ req_mock.assert_not_called()
diff --git a/tests/rest/test_well_known.py b/tests/rest/test_well_known.py
index e166c13bc1..ac992766e8 100644
--- a/tests/rest/test_well_known.py
+++ b/tests/rest/test_well_known.py
@@ -17,6 +17,8 @@
# [This file includes modifications made by New Vector Limited]
#
#
+from unittest.mock import AsyncMock
+
from twisted.web.resource import Resource
from synapse.rest.well_known import well_known_resource
@@ -112,7 +114,6 @@ class WellKnownTests(unittest.HomeserverTestCase):
"msc3861": {
"enabled": True,
"issuer": "https://issuer",
- "account_management_url": "https://my-account.issuer",
"client_id": "id",
"client_auth_method": "client_secret_post",
"client_secret": "secret",
@@ -122,18 +123,26 @@ class WellKnownTests(unittest.HomeserverTestCase):
}
)
def test_client_well_known_msc3861_oauth_delegation(self) -> None:
- channel = self.make_request(
- "GET", "/.well-known/matrix/client", shorthand=False
- )
+ # Patch the HTTP client to return the issuer metadata
+ req_mock = AsyncMock(return_value={"issuer": "https://issuer", "account_management_uri": "https://my-account.issuer"})
+ self.hs.get_proxied_http_client().get_json = req_mock # type: ignore[method-assign]
- self.assertEqual(channel.code, 200)
- self.assertEqual(
- channel.json_body,
- {
- "m.homeserver": {"base_url": "https://homeserver/"},
- "org.matrix.msc2965.authentication": {
- "issuer": "https://issuer",
- "account": "https://my-account.issuer",
+ for _ in range(2):
+ channel = self.make_request(
+ "GET", "/.well-known/matrix/client", shorthand=False
+ )
+
+ self.assertEqual(channel.code, 200)
+ self.assertEqual(
+ channel.json_body,
+ {
+ "m.homeserver": {"base_url": "https://homeserver/"},
+ "org.matrix.msc2965.authentication": {
+ "issuer": "https://issuer",
+ "account": "https://my-account.issuer",
+ },
},
- },
- )
+ )
+
+ # It should have been called exactly once, because it gets cached
+ req_mock.assert_called_once_with("https://issuer/.well-known/openid-configuration")
|
