MSC3861: load the issuer and account management URLs from OIDC discovery (#17407)
This will help mitigating any discrepancies between the issuer configured and the one returned by the OIDC provider. This also removes the need for configuring the `account_management_url` explicitely, as it will now be loaded from the OIDC discovery, as per MSC2965. Because we may now fetch stuff for the .well-known/matrix/client endpoint, this also transforms the client well-known resource to be asynchronous.
This commit is contained in:
parent
53a3783750
commit
48303fcbcc
|
@ -0,0 +1 @@
|
|||
MSC3861: load the issuer and account management URLs from OIDC discovery.
|
|
@ -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:
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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.
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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")
|
||||
|
|
Loading…
Reference in New Issue