-
Notifications
You must be signed in to change notification settings - Fork 503
Support the new /auth_metadata endpoint defined in MSC2965.
#18093
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Support the new `/auth_metadata` endpoint defined in MSC2965. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
|
|
@@ -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 servelet shouldn't have been | ||
sandhose marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spec would prefer |
||
|
|
||
|
|
||
| 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) | ||
This file was deleted.
| 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? synapse/synapse/api/auth/msc3861_delegated.py Line 145 in ca290d3
Just checking that we're not mocking the test http client and then immediately checking it returns the mocked data.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||||
| }, | ||||
| ) | ||||
Uh oh!
There was an error while loading. Please reload this page.