Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support the new /auth_metadata endpoint defined in MSC2965. #18093

Merged
merged 3 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/18093.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support the new `/auth_metadata` endpoint defined in [MSC2965](https://github.com/matrix-org/matrix-spec-proposals/pull/2965).
6 changes: 6 additions & 0 deletions synapse/api/auth/msc3861_delegated.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ async def account_management_url(self) -> Optional[str]:
logger.warning("Failed to load metadata:", exc_info=True)
return None

async def auth_metadata(self) -> Dict[str, Any]:
"""
Returns the auth metadata dict
"""
return await self._issuer_metadata.get()

async def _introspection_endpoint(self) -> str:
"""
Returns the introspection endpoint of the issuer
Expand Down
6 changes: 3 additions & 3 deletions synapse/rest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
account_validity,
appservice_ping,
auth,
auth_issuer,
auth_metadata,
capabilities,
delayed_events,
devices,
Expand Down Expand Up @@ -121,7 +121,7 @@
mutual_rooms.register_servlets,
login_token_request.register_servlets,
rendezvous.register_servlets,
auth_issuer.register_servlets,
auth_metadata.register_servlets,
)

SERVLET_GROUPS: Dict[str, Iterable[RegisterServletsFunc]] = {
Expand Down Expand Up @@ -187,7 +187,7 @@ def register_servlets(
mutual_rooms.register_servlets,
login_token_request.register_servlets,
rendezvous.register_servlets,
auth_issuer.register_servlets,
auth_metadata.register_servlets,
]:
continue

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
class AuthIssuerServlet(RestServlet):
"""
Advertises what OpenID Connect issuer clients should use to authorise users.
This endpoint was defined in a previous iteration of MSC2965, and is still
used by some clients.
"""

PATTERNS = client_patterns(
Expand Down Expand Up @@ -63,7 +65,42 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
)


class AuthMetadataServlet(RestServlet):
"""
Advertises the OAuth 2.0 server metadata for the homeserver.
"""

PATTERNS = client_patterns(
"/org.matrix.msc2965/auth_metadata$",
unstable=True,
releases=(),
)

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:
# 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, await auth.auth_metadata()
else:
# Wouldn't expect this to be reached: the servlet shouldn't have been
# registered. Still, fail gracefully if we are registered for some reason.
raise SynapseError(
404,
"OIDC discovery has not been configured on this homeserver",
Codes.NOT_FOUND,
)
Comment on lines +95 to +99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec would prefer M_UNRECOGNIZED with 404. https://spec.matrix.org/v1.13/client-server-api/#common-error-codes



def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
# We use the MSC3861 values as they are used by multiple MSCs
if hs.config.experimental.msc3861.enabled:
AuthIssuerServlet(hs).register(http_server)
AuthMetadataServlet(hs).register(http_server)
79 changes: 0 additions & 79 deletions tests/rest/client/test_auth_issuer.py

This file was deleted.

140 changes: 140 additions & 0 deletions tests/rest/client/test_auth_metadata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
#
# This file is licensed under the Affero General Public License (AGPL) version 3.
#
# Copyright 2023 The Matrix.org Foundation C.I.C
# Copyright (C) 2023-2025 New Vector, Ltd
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# See the GNU Affero General Public License for more details:
# <https://www.gnu.org/licenses/agpl-3.0.html>.
#
# Originally licensed under the Apache License, Version 2.0:
# <http://www.apache.org/licenses/LICENSE-2.0>.
#
# [This file includes modifications made by New Vector Limited]
#
from http import HTTPStatus
from unittest.mock import AsyncMock

from synapse.rest.client import auth_metadata

from tests.unittest import HomeserverTestCase, override_config, skip_unless
from tests.utils import HAS_AUTHLIB

ISSUER = "https://account.example.com/"


class AuthIssuerTestCase(HomeserverTestCase):
servlets = [
auth_metadata.register_servlets,
]

def test_returns_404_when_msc3861_disabled(self) -> None:
# Make an unauthenticated request for the discovery info.
channel = self.make_request(
"GET",
"/_matrix/client/unstable/org.matrix.msc2965/auth_issuer",
)
self.assertEqual(channel.code, HTTPStatus.NOT_FOUND)

@skip_unless(HAS_AUTHLIB, "requires authlib")
@override_config(
{
"disable_registration": True,
"experimental_features": {
"msc3861": {
"enabled": True,
"issuer": ISSUER,
"client_id": "David Lister",
"client_auth_method": "client_secret_post",
"client_secret": "Who shot Mister Burns?",
}
},
}
)
def test_returns_issuer_when_oidc_enabled(self) -> None:
# 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()


class AuthMetadataTestCase(HomeserverTestCase):
servlets = [
auth_metadata.register_servlets,
]

def test_returns_404_when_msc3861_disabled(self) -> None:
# Make an unauthenticated request for the discovery info.
channel = self.make_request(
"GET",
"/_matrix/client/unstable/org.matrix.msc2965/auth_metadata",
)
self.assertEqual(channel.code, HTTPStatus.NOT_FOUND)

@skip_unless(HAS_AUTHLIB, "requires authlib")
@override_config(
{
"disable_registration": True,
"experimental_features": {
"msc3861": {
"enabled": True,
"issuer": ISSUER,
"client_id": "David Lister",
"client_auth_method": "client_secret_post",
"client_secret": "Who shot Mister Burns?",
}
},
}
)
def test_returns_issuer_when_oidc_enabled(self) -> None:
# Patch the HTTP client to return the issuer metadata
req_mock = AsyncMock(
return_value={
"issuer": ISSUER,
"authorization_endpoint": "https://example.com/auth",
"token_endpoint": "https://example.com/token",
}
)
self.hs.get_proxied_http_client().get_json = req_mock # type: ignore[method-assign]
Comment on lines +118 to +125
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mocking a request Synapse is supposed to be making externally, right? This one?

response = await self._http_client.get_json(url)

Just checking that we're not mocking the test http client and then immediately checking it returns the mocked data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's what's happening


channel = self.make_request(
"GET",
"/_matrix/client/unstable/org.matrix.msc2965/auth_metadata",
)

self.assertEqual(channel.code, HTTPStatus.OK)
self.assertEqual(
channel.json_body,
{
"issuer": ISSUER,
"authorization_endpoint": "https://example.com/auth",
"token_endpoint": "https://example.com/token",
},
)
Loading