From f2c1560eca1e2160087a280261ca78d0708ad721 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 29 Jan 2021 16:38:29 +0000 Subject: Ratelimit invites by room and target user (#9258) --- tests/handlers/test_federation.py | 93 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) (limited to 'tests/handlers') diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index 0b24b89a2e..74503112f5 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -16,7 +16,7 @@ import logging from unittest import TestCase from synapse.api.constants import EventTypes -from synapse.api.errors import AuthError, Codes, SynapseError +from synapse.api.errors import AuthError, Codes, LimitExceededError, SynapseError from synapse.api.room_versions import RoomVersions from synapse.events import EventBase from synapse.federation.federation_base import event_from_pdu_json @@ -191,6 +191,97 @@ class FederationTestCase(unittest.HomeserverTestCase): self.assertEqual(sg, sg2) + @unittest.override_config( + {"rc_invites": {"per_room": {"per_second": 0.5, "burst_count": 3}}} + ) + def test_invite_by_room_ratelimit(self): + """Tests that invites from federation in a room are actually rate-limited. + """ + other_server = "otherserver" + other_user = "@otheruser:" + other_server + + # create the room + user_id = self.register_user("kermit", "test") + tok = self.login("kermit", "test") + room_id = self.helper.create_room_as(room_creator=user_id, tok=tok) + room_version = self.get_success(self.store.get_room_version(room_id)) + + def create_invite_for(local_user): + return event_from_pdu_json( + { + "type": EventTypes.Member, + "content": {"membership": "invite"}, + "room_id": room_id, + "sender": other_user, + "state_key": local_user, + "depth": 32, + "prev_events": [], + "auth_events": [], + "origin_server_ts": self.clock.time_msec(), + }, + room_version, + ) + + for i in range(3): + self.get_success( + self.handler.on_invite_request( + other_server, + create_invite_for("@user-%d:test" % (i,)), + room_version, + ) + ) + + self.get_failure( + self.handler.on_invite_request( + other_server, create_invite_for("@user-4:test"), room_version, + ), + exc=LimitExceededError, + ) + + @unittest.override_config( + {"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}} + ) + def test_invite_by_user_ratelimit(self): + """Tests that invites from federation to a particular user are + actually rate-limited. + """ + other_server = "otherserver" + other_user = "@otheruser:" + other_server + + # create the room + user_id = self.register_user("kermit", "test") + tok = self.login("kermit", "test") + + def create_invite(): + room_id = self.helper.create_room_as(room_creator=user_id, tok=tok) + room_version = self.get_success(self.store.get_room_version(room_id)) + return event_from_pdu_json( + { + "type": EventTypes.Member, + "content": {"membership": "invite"}, + "room_id": room_id, + "sender": other_user, + "state_key": "@user:test", + "depth": 32, + "prev_events": [], + "auth_events": [], + "origin_server_ts": self.clock.time_msec(), + }, + room_version, + ) + + for i in range(3): + event = create_invite() + self.get_success( + self.handler.on_invite_request(other_server, event, event.room_version,) + ) + + event = create_invite() + self.get_failure( + self.handler.on_invite_request(other_server, event, event.room_version,), + exc=LimitExceededError, + ) + def _build_and_send_join_event(self, other_server, other_user, room_id): join_event = self.get_success( self.handler.on_make_join_request(other_server, room_id, other_user) -- cgit 1.5.1 From 8aed29dc615bee75019fc526a5c91cdc2638b665 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 1 Feb 2021 15:50:56 +0000 Subject: Improve styling and wording of SSO redirect confirm template (#9272) --- changelog.d/9272.feature | 1 + docs/sample_config.yaml | 14 ++++- synapse/config/sso.py | 14 ++++- synapse/handlers/auth.py | 24 ++++++- synapse/handlers/sso.py | 10 ++- synapse/module_api/__init__.py | 10 ++- synapse/res/templates/sso.css | 83 +++++++++++++++++++++++++ synapse/res/templates/sso_redirect_confirm.html | 34 ++++++++-- tests/handlers/test_cas.py | 8 +-- tests/handlers/test_oidc.py | 24 ++++--- tests/handlers/test_saml.py | 8 +-- 11 files changed, 200 insertions(+), 30 deletions(-) create mode 100644 changelog.d/9272.feature create mode 100644 synapse/res/templates/sso.css (limited to 'tests/handlers') diff --git a/changelog.d/9272.feature b/changelog.d/9272.feature new file mode 100644 index 0000000000..c21b197ca1 --- /dev/null +++ b/changelog.d/9272.feature @@ -0,0 +1 @@ +Improve the user experience of setting up an account via single-sign on. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 8777e3254d..05506a7787 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1971,7 +1971,8 @@ sso: # * HTML page for a confirmation step before redirecting back to the client # with the login token: 'sso_redirect_confirm.html'. # - # When rendering, this template is given three variables: + # When rendering, this template is given the following variables: + # # * redirect_url: the URL the user is about to be redirected to. Needs # manual escaping (see # https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping). @@ -1984,6 +1985,17 @@ sso: # # * server_name: the homeserver's name. # + # * new_user: a boolean indicating whether this is the user's first time + # logging in. + # + # * user_id: the user's matrix ID. + # + # * user_profile.avatar_url: an MXC URI for the user's avatar, if any. + # None if the user has not set an avatar. + # + # * user_profile.display_name: the user's display name. None if the user + # has not set a display name. + # # * HTML page which notifies the user that they are authenticating to confirm # an operation on their account during the user interactive authentication # process: 'sso_auth_confirm.html'. diff --git a/synapse/config/sso.py b/synapse/config/sso.py index 59be825532..a470112ed4 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -127,7 +127,8 @@ class SSOConfig(Config): # * HTML page for a confirmation step before redirecting back to the client # with the login token: 'sso_redirect_confirm.html'. # - # When rendering, this template is given three variables: + # When rendering, this template is given the following variables: + # # * redirect_url: the URL the user is about to be redirected to. Needs # manual escaping (see # https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping). @@ -140,6 +141,17 @@ class SSOConfig(Config): # # * server_name: the homeserver's name. # + # * new_user: a boolean indicating whether this is the user's first time + # logging in. + # + # * user_id: the user's matrix ID. + # + # * user_profile.avatar_url: an MXC URI for the user's avatar, if any. + # None if the user has not set an avatar. + # + # * user_profile.display_name: the user's display name. None if the user + # has not set a display name. + # # * HTML page which notifies the user that they are authenticating to confirm # an operation on their account during the user interactive authentication # process: 'sso_auth_confirm.html'. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 0e98db22b3..c722a4afa8 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -61,6 +61,7 @@ from synapse.http.site import SynapseRequest from synapse.logging.context import defer_to_thread from synapse.metrics.background_process_metrics import run_as_background_process from synapse.module_api import ModuleApi +from synapse.storage.roommember import ProfileInfo from synapse.types import JsonDict, Requester, UserID from synapse.util import stringutils as stringutils from synapse.util.async_helpers import maybe_awaitable @@ -1396,6 +1397,7 @@ class AuthHandler(BaseHandler): request: Request, client_redirect_url: str, extra_attributes: Optional[JsonDict] = None, + new_user: bool = False, ): """Having figured out a mxid for this user, complete the HTTP request @@ -1406,6 +1408,8 @@ class AuthHandler(BaseHandler): process. extra_attributes: Extra attributes which will be passed to the client during successful login. Must be JSON serializable. + new_user: True if we should use wording appropriate to a user who has just + registered. """ # If the account has been deactivated, do not proceed with the login # flow. @@ -1414,8 +1418,17 @@ class AuthHandler(BaseHandler): respond_with_html(request, 403, self._sso_account_deactivated_template) return + profile = await self.store.get_profileinfo( + UserID.from_string(registered_user_id).localpart + ) + self._complete_sso_login( - registered_user_id, request, client_redirect_url, extra_attributes + registered_user_id, + request, + client_redirect_url, + extra_attributes, + new_user=new_user, + user_profile_data=profile, ) def _complete_sso_login( @@ -1424,12 +1437,18 @@ class AuthHandler(BaseHandler): request: Request, client_redirect_url: str, extra_attributes: Optional[JsonDict] = None, + new_user: bool = False, + user_profile_data: Optional[ProfileInfo] = None, ): """ The synchronous portion of complete_sso_login. This exists purely for backwards compatibility of synapse.module_api.ModuleApi. """ + + if user_profile_data is None: + user_profile_data = ProfileInfo(None, None) + # Store any extra attributes which will be passed in the login response. # Note that this is per-user so it may overwrite a previous value, this # is considered OK since the newest SSO attributes should be most valid. @@ -1467,6 +1486,9 @@ class AuthHandler(BaseHandler): display_url=redirect_url_no_params, redirect_url=redirect_url, server_name=self._server_name, + new_user=new_user, + user_id=registered_user_id, + user_profile=user_profile_data, ) respond_with_html(request, 200, html) diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index 50c5ae142a..ceaeb5a376 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -391,6 +391,8 @@ class SsoHandler: to an additional page. (e.g. to prompt for more information) """ + new_user = False + # grab a lock while we try to find a mapping for this user. This seems... # optimistic, especially for implementations that end up redirecting to # interstitial pages. @@ -431,9 +433,14 @@ class SsoHandler: get_request_user_agent(request), request.getClientIP(), ) + new_user = True await self._auth_handler.complete_sso_login( - user_id, request, client_redirect_url, extra_login_attributes + user_id, + request, + client_redirect_url, + extra_login_attributes, + new_user=new_user, ) async def _call_attribute_mapper( @@ -778,6 +785,7 @@ class SsoHandler: request, session.client_redirect_url, session.extra_login_attributes, + new_user=True, ) def _expire_old_sessions(self): diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 72ab5750cc..401d577293 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -279,7 +279,11 @@ class ModuleApi: ) async def complete_sso_login_async( - self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str + self, + registered_user_id: str, + request: SynapseRequest, + client_redirect_url: str, + new_user: bool = False, ): """Complete a SSO login by redirecting the user to a page to confirm whether they want their access token sent to `client_redirect_url`, or redirect them to that @@ -291,9 +295,11 @@ class ModuleApi: request: The request to respond to. client_redirect_url: The URL to which to offer to redirect the user (or to redirect them directly if whitelisted). + new_user: set to true to use wording for the consent appropriate to a user + who has just registered. """ await self._auth_handler.complete_sso_login( - registered_user_id, request, client_redirect_url, + registered_user_id, request, client_redirect_url, new_user=new_user ) @defer.inlineCallbacks diff --git a/synapse/res/templates/sso.css b/synapse/res/templates/sso.css new file mode 100644 index 0000000000..ff9dc94032 --- /dev/null +++ b/synapse/res/templates/sso.css @@ -0,0 +1,83 @@ +body { + font-family: "Inter", "Helvetica", "Arial", sans-serif; + font-size: 14px; + color: #17191C; +} + +header { + max-width: 480px; + width: 100%; + margin: 24px auto; + text-align: center; +} + +header p { + color: #737D8C; + line-height: 24px; +} + +h1 { + font-size: 24px; +} + +h2 { + font-size: 14px; +} + +h2 img { + vertical-align: middle; + margin-right: 8px; + width: 24px; + height: 24px; +} + +label { + cursor: pointer; +} + +main { + max-width: 360px; + width: 100%; + margin: 24px auto; +} + +.primary-button { + border: none; + text-decoration: none; + padding: 12px; + color: white; + background-color: #418DED; + font-weight: bold; + display: block; + border-radius: 12px; + width: 100%; + margin: 16px 0; + cursor: pointer; + text-align: center; +} + +.profile { + display: flex; + justify-content: center; + margin: 24px 0; +} + +.profile .avatar { + width: 36px; + height: 36px; + border-radius: 100%; + display: block; + margin-right: 8px; +} + +.profile .display-name { + font-weight: bold; + margin-bottom: 4px; +} +.profile .user-id { + color: #737D8C; +} + +.profile .display-name, .profile .user-id { + line-height: 18px; +} \ No newline at end of file diff --git a/synapse/res/templates/sso_redirect_confirm.html b/synapse/res/templates/sso_redirect_confirm.html index 20a15e1e74..ce4f573848 100644 --- a/synapse/res/templates/sso_redirect_confirm.html +++ b/synapse/res/templates/sso_redirect_confirm.html @@ -3,12 +3,34 @@ SSO redirect confirmation + + -

The application at {{ display_url | e }} is requesting full access to your {{ server_name }} Matrix account.

-

If you don't recognise this address, you should ignore this and close this tab.

-

- I trust this address -

+
+ {% if new_user %} +

Your account is now ready

+

You've made your account on {{ server_name | e }}.

+ {% else %} +

Log in

+ {% endif %} +

Continue to confirm you trust {{ display_url | e }}.

+
+
+ {% if user_profile.avatar_url %} +
+ +
+ {% if user_profile.display_name %} +
{{ user_profile.display_name | e }}
+ {% endif %} +
{{ user_id | e }}
+
+
+ {% endif %} + Continue +
- \ No newline at end of file + diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py index c37bb6440e..7baf224f7e 100644 --- a/tests/handlers/test_cas.py +++ b/tests/handlers/test_cas.py @@ -62,7 +62,7 @@ class CasHandlerTestCase(HomeserverTestCase): # check that the auth handler got called as expected auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "redirect_uri", None + "@test_user:test", request, "redirect_uri", None, new_user=True ) def test_map_cas_user_to_existing_user(self): @@ -85,7 +85,7 @@ class CasHandlerTestCase(HomeserverTestCase): # check that the auth handler got called as expected auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "redirect_uri", None + "@test_user:test", request, "redirect_uri", None, new_user=False ) # Subsequent calls should map to the same mxid. @@ -94,7 +94,7 @@ class CasHandlerTestCase(HomeserverTestCase): self.handler._handle_cas_response(request, cas_response, "redirect_uri", "") ) auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "redirect_uri", None + "@test_user:test", request, "redirect_uri", None, new_user=False ) def test_map_cas_user_to_invalid_localpart(self): @@ -112,7 +112,7 @@ class CasHandlerTestCase(HomeserverTestCase): # check that the auth handler got called as expected auth_handler.complete_sso_login.assert_called_once_with( - "@f=c3=b6=c3=b6:test", request, "redirect_uri", None + "@f=c3=b6=c3=b6:test", request, "redirect_uri", None, new_user=True ) diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index b3dfa40d25..d8f90b9a80 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -419,7 +419,7 @@ class OidcHandlerTestCase(HomeserverTestCase): self.get_success(self.handler.handle_oidc_callback(request)) auth_handler.complete_sso_login.assert_called_once_with( - expected_user_id, request, client_redirect_url, None, + expected_user_id, request, client_redirect_url, None, new_user=True ) self.provider._exchange_code.assert_called_once_with(code) self.provider._parse_id_token.assert_called_once_with(token, nonce=nonce) @@ -450,7 +450,7 @@ class OidcHandlerTestCase(HomeserverTestCase): self.get_success(self.handler.handle_oidc_callback(request)) auth_handler.complete_sso_login.assert_called_once_with( - expected_user_id, request, client_redirect_url, None, + expected_user_id, request, client_redirect_url, None, new_user=False ) self.provider._exchange_code.assert_called_once_with(code) self.provider._parse_id_token.assert_not_called() @@ -623,7 +623,11 @@ class OidcHandlerTestCase(HomeserverTestCase): self.get_success(self.handler.handle_oidc_callback(request)) auth_handler.complete_sso_login.assert_called_once_with( - "@foo:test", request, client_redirect_url, {"phone": "1234567"}, + "@foo:test", + request, + client_redirect_url, + {"phone": "1234567"}, + new_user=True, ) def test_map_userinfo_to_user(self): @@ -637,7 +641,7 @@ class OidcHandlerTestCase(HomeserverTestCase): } self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", ANY, ANY, None, + "@test_user:test", ANY, ANY, None, new_user=True ) auth_handler.complete_sso_login.reset_mock() @@ -648,7 +652,7 @@ class OidcHandlerTestCase(HomeserverTestCase): } self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - "@test_user_2:test", ANY, ANY, None, + "@test_user_2:test", ANY, ANY, None, new_user=True ) auth_handler.complete_sso_login.reset_mock() @@ -685,14 +689,14 @@ class OidcHandlerTestCase(HomeserverTestCase): } self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - user.to_string(), ANY, ANY, None, + user.to_string(), ANY, ANY, None, new_user=False ) auth_handler.complete_sso_login.reset_mock() # Subsequent calls should map to the same mxid. self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - user.to_string(), ANY, ANY, None, + user.to_string(), ANY, ANY, None, new_user=False ) auth_handler.complete_sso_login.reset_mock() @@ -707,7 +711,7 @@ class OidcHandlerTestCase(HomeserverTestCase): } self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - user.to_string(), ANY, ANY, None, + user.to_string(), ANY, ANY, None, new_user=False ) auth_handler.complete_sso_login.reset_mock() @@ -743,7 +747,7 @@ class OidcHandlerTestCase(HomeserverTestCase): self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) auth_handler.complete_sso_login.assert_called_once_with( - "@TEST_USER_2:test", ANY, ANY, None, + "@TEST_USER_2:test", ANY, ANY, None, new_user=False ) def test_map_userinfo_to_invalid_localpart(self): @@ -779,7 +783,7 @@ class OidcHandlerTestCase(HomeserverTestCase): # test_user is already taken, so test_user1 gets registered instead. auth_handler.complete_sso_login.assert_called_once_with( - "@test_user1:test", ANY, ANY, None, + "@test_user1:test", ANY, ANY, None, new_user=True ) auth_handler.complete_sso_login.reset_mock() diff --git a/tests/handlers/test_saml.py b/tests/handlers/test_saml.py index 261c7083d1..a8d6c0f617 100644 --- a/tests/handlers/test_saml.py +++ b/tests/handlers/test_saml.py @@ -131,7 +131,7 @@ class SamlHandlerTestCase(HomeserverTestCase): # check that the auth handler got called as expected auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "redirect_uri", None + "@test_user:test", request, "redirect_uri", None, new_user=True ) @override_config({"saml2_config": {"grandfathered_mxid_source_attribute": "mxid"}}) @@ -157,7 +157,7 @@ class SamlHandlerTestCase(HomeserverTestCase): # check that the auth handler got called as expected auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "", None + "@test_user:test", request, "", None, new_user=False ) # Subsequent calls should map to the same mxid. @@ -166,7 +166,7 @@ class SamlHandlerTestCase(HomeserverTestCase): self.handler._handle_authn_response(request, saml_response, "") ) auth_handler.complete_sso_login.assert_called_once_with( - "@test_user:test", request, "", None + "@test_user:test", request, "", None, new_user=False ) def test_map_saml_response_to_invalid_localpart(self): @@ -214,7 +214,7 @@ class SamlHandlerTestCase(HomeserverTestCase): # test_user is already taken, so test_user1 gets registered instead. auth_handler.complete_sso_login.assert_called_once_with( - "@test_user1:test", request, "", None + "@test_user1:test", request, "", None, new_user=True ) auth_handler.complete_sso_login.reset_mock() -- cgit 1.5.1 From 846b9d3df033be1043710e49e89bcba68722071e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 1 Feb 2021 22:56:01 +0000 Subject: Put OIDC callback URI under /_synapse/client. (#9288) --- CHANGES.md | 4 +++ UPGRADE.rst | 13 ++++++++- changelog.d/9288.feature | 1 + docs/openid.md | 19 ++++++------- docs/workers.md | 2 +- synapse/config/oidc_config.py | 2 +- synapse/handlers/oidc_handler.py | 8 +++--- synapse/rest/oidc/__init__.py | 27 ------------------- synapse/rest/oidc/callback_resource.py | 30 --------------------- synapse/rest/synapse/client/__init__.py | 4 +-- synapse/rest/synapse/client/oidc/__init__.py | 31 ++++++++++++++++++++++ .../rest/synapse/client/oidc/callback_resource.py | 30 +++++++++++++++++++++ tests/handlers/test_oidc.py | 15 +++++------ 13 files changed, 102 insertions(+), 84 deletions(-) create mode 100644 changelog.d/9288.feature delete mode 100644 synapse/rest/oidc/__init__.py delete mode 100644 synapse/rest/oidc/callback_resource.py create mode 100644 synapse/rest/synapse/client/oidc/__init__.py create mode 100644 synapse/rest/synapse/client/oidc/callback_resource.py (limited to 'tests/handlers') diff --git a/CHANGES.md b/CHANGES.md index fcd782fa94..e9ff14a03d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,10 @@ Unreleased Note that this release includes a change in Synapse to use Redis as a cache ─ as well as a pub/sub mechanism ─ if Redis support is enabled. No action is needed by server administrators, and we do not expect resource usage of the Redis instance to change dramatically. +This release also changes the callback URI for OpenID Connect (OIDC) identity +providers. If your server is configured to use single sign-on via an +OIDC/OAuth2 IdP, you may need to make configuration changes. Please review +[UPGRADE.rst](UPGRADE.rst) for more details on these changes. Synapse 1.26.0 (2021-01-27) =========================== diff --git a/UPGRADE.rst b/UPGRADE.rst index eea0322695..d00f718cae 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -88,6 +88,17 @@ for example: Upgrading to v1.27.0 ==================== +Changes to callback URI for OAuth2 / OpenID Connect +--------------------------------------------------- + +This version changes the URI used for callbacks from OAuth2 identity providers. If +your server is configured for single sign-on via an OpenID Connect or OAuth2 identity +provider, you will need to add ``[synapse public baseurl]/_synapse/client/oidc/callback`` +to the list of permitted "redirect URIs" at the identity provider. + +See `docs/openid.md `_ for more information on setting up OpenID +Connect. + Changes to HTML templates ------------------------- @@ -235,7 +246,7 @@ shown below: return {"localpart": localpart} -Removal historical Synapse Admin API +Removal historical Synapse Admin API ------------------------------------ Historically, the Synapse Admin API has been accessible under: diff --git a/changelog.d/9288.feature b/changelog.d/9288.feature new file mode 100644 index 0000000000..efde69fb3c --- /dev/null +++ b/changelog.d/9288.feature @@ -0,0 +1 @@ +Update the redirect URI for OIDC authentication. diff --git a/docs/openid.md b/docs/openid.md index 3d07220967..9d19368845 100644 --- a/docs/openid.md +++ b/docs/openid.md @@ -54,7 +54,8 @@ Here are a few configs for providers that should work with Synapse. ### Microsoft Azure Active Directory Azure AD can act as an OpenID Connect Provider. Register a new application under *App registrations* in the Azure AD management console. The RedirectURI for your -application should point to your matrix server: `[synapse public baseurl]/_synapse/oidc/callback` +application should point to your matrix server: +`[synapse public baseurl]/_synapse/client/oidc/callback` Go to *Certificates & secrets* and register a new client secret. Make note of your Directory (tenant) ID as it will be used in the Azure links. @@ -94,7 +95,7 @@ staticClients: - id: synapse secret: secret redirectURIs: - - '[synapse public baseurl]/_synapse/oidc/callback' + - '[synapse public baseurl]/_synapse/client/oidc/callback' name: 'Synapse' ``` @@ -140,7 +141,7 @@ Follow the [Getting Started Guide](https://www.keycloak.org/getting-started) to | Enabled | `On` | | Client Protocol | `openid-connect` | | Access Type | `confidential` | -| Valid Redirect URIs | `[synapse public baseurl]/_synapse/oidc/callback` | +| Valid Redirect URIs | `[synapse public baseurl]/_synapse/client/oidc/callback` | 5. Click `Save` 6. On the Credentials tab, update the fields: @@ -168,7 +169,7 @@ oidc_providers: ### [Auth0][auth0] 1. Create a regular web application for Synapse -2. Set the Allowed Callback URLs to `[synapse public baseurl]/_synapse/oidc/callback` +2. Set the Allowed Callback URLs to `[synapse public baseurl]/_synapse/client/oidc/callback` 3. Add a rule to add the `preferred_username` claim.
Code sample @@ -217,7 +218,7 @@ login mechanism needs an attribute to uniquely identify users, and that endpoint does not return a `sub` property, an alternative `subject_claim` has to be set. 1. Create a new OAuth application: https://github.com/settings/applications/new. -2. Set the callback URL to `[synapse public baseurl]/_synapse/oidc/callback`. +2. Set the callback URL to `[synapse public baseurl]/_synapse/client/oidc/callback`. Synapse config: @@ -262,13 +263,13 @@ oidc_providers: display_name_template: "{{ user.name }}" ``` 4. Back in the Google console, add this Authorized redirect URI: `[synapse - public baseurl]/_synapse/oidc/callback`. + public baseurl]/_synapse/client/oidc/callback`. ### Twitch 1. Setup a developer account on [Twitch](https://dev.twitch.tv/) 2. Obtain the OAuth 2.0 credentials by [creating an app](https://dev.twitch.tv/console/apps/) -3. Add this OAuth Redirect URL: `[synapse public baseurl]/_synapse/oidc/callback` +3. Add this OAuth Redirect URL: `[synapse public baseurl]/_synapse/client/oidc/callback` Synapse config: @@ -290,7 +291,7 @@ oidc_providers: 1. Create a [new application](https://gitlab.com/profile/applications). 2. Add the `read_user` and `openid` scopes. -3. Add this Callback URL: `[synapse public baseurl]/_synapse/oidc/callback` +3. Add this Callback URL: `[synapse public baseurl]/_synapse/client/oidc/callback` Synapse config: @@ -323,7 +324,7 @@ one so requires a little more configuration. 2. Once the app is created, add "Facebook Login" and choose "Web". You don't need to go through the whole form here. 3. In the left-hand menu, open "Products"/"Facebook Login"/"Settings". - * Add `[synapse public baseurl]/_synapse/oidc/callback` as an OAuth Redirect + * Add `[synapse public baseurl]/_synapse/client/oidc/callback` as an OAuth Redirect URL. 4. In the left-hand menu, open "Settings/Basic". Here you can copy the "App ID" and "App Secret" for use below. diff --git a/docs/workers.md b/docs/workers.md index c36549c621..c4a6c79238 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -266,7 +266,7 @@ using): ^/_synapse/client/sso_register$ # OpenID Connect requests. - ^/_synapse/oidc/callback$ + ^/_synapse/client/oidc/callback$ # SAML requests. ^/_matrix/saml2/authn_response$ diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index bb122ef182..4c24c50629 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -53,7 +53,7 @@ class OIDCConfig(Config): "Multiple OIDC providers have the idp_id %r." % idp_id ) - self.oidc_callback_url = self.public_baseurl + "_synapse/oidc/callback" + self.oidc_callback_url = self.public_baseurl + "_synapse/client/oidc/callback" @property def oidc_enabled(self) -> bool: diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index ca647fa78f..71008ec50d 100644 --- a/synapse/handlers/oidc_handler.py +++ b/synapse/handlers/oidc_handler.py @@ -102,7 +102,7 @@ class OidcHandler: ) from e async def handle_oidc_callback(self, request: SynapseRequest) -> None: - """Handle an incoming request to /_synapse/oidc/callback + """Handle an incoming request to /_synapse/client/oidc/callback Since we might want to display OIDC-related errors in a user-friendly way, we don't raise SynapseError from here. Instead, we call @@ -643,7 +643,7 @@ class OidcProvider: - ``client_id``: the client ID set in ``oidc_config.client_id`` - ``response_type``: ``code`` - - ``redirect_uri``: the callback URL ; ``{base url}/_synapse/oidc/callback`` + - ``redirect_uri``: the callback URL ; ``{base url}/_synapse/client/oidc/callback`` - ``scope``: the list of scopes set in ``oidc_config.scopes`` - ``state``: a random string - ``nonce``: a random string @@ -684,7 +684,7 @@ class OidcProvider: request.addCookie( SESSION_COOKIE_NAME, cookie, - path="/_synapse/oidc", + path="/_synapse/client/oidc", max_age="3600", httpOnly=True, sameSite="lax", @@ -705,7 +705,7 @@ class OidcProvider: async def handle_oidc_callback( self, request: SynapseRequest, session_data: "OidcSessionData", code: str ) -> None: - """Handle an incoming request to /_synapse/oidc/callback + """Handle an incoming request to /_synapse/client/oidc/callback By this time we have already validated the session on the synapse side, and now need to do the provider-specific operations. This includes: diff --git a/synapse/rest/oidc/__init__.py b/synapse/rest/oidc/__init__.py deleted file mode 100644 index d958dd65bb..0000000000 --- a/synapse/rest/oidc/__init__.py +++ /dev/null @@ -1,27 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2020 Quentin Gliech -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import logging - -from twisted.web.resource import Resource - -from synapse.rest.oidc.callback_resource import OIDCCallbackResource - -logger = logging.getLogger(__name__) - - -class OIDCResource(Resource): - def __init__(self, hs): - Resource.__init__(self) - self.putChild(b"callback", OIDCCallbackResource(hs)) diff --git a/synapse/rest/oidc/callback_resource.py b/synapse/rest/oidc/callback_resource.py deleted file mode 100644 index f7a0bc4bdb..0000000000 --- a/synapse/rest/oidc/callback_resource.py +++ /dev/null @@ -1,30 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2020 Quentin Gliech -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import logging - -from synapse.http.server import DirectServeHtmlResource - -logger = logging.getLogger(__name__) - - -class OIDCCallbackResource(DirectServeHtmlResource): - isLeaf = 1 - - def __init__(self, hs): - super().__init__() - self._oidc_handler = hs.get_oidc_handler() - - async def _async_render_GET(self, request): - await self._oidc_handler.handle_oidc_callback(request) diff --git a/synapse/rest/synapse/client/__init__.py b/synapse/rest/synapse/client/__init__.py index 02310c1900..381baf9729 100644 --- a/synapse/rest/synapse/client/__init__.py +++ b/synapse/rest/synapse/client/__init__.py @@ -47,9 +47,9 @@ def build_synapse_client_resource_tree(hs: "HomeServer") -> Mapping[str, Resourc # provider-specific SSO bits. Only load these if they are enabled, since they # rely on optional dependencies. if hs.config.oidc_enabled: - from synapse.rest.oidc import OIDCResource + from synapse.rest.synapse.client.oidc import OIDCResource - resources["/_synapse/oidc"] = OIDCResource(hs) + resources["/_synapse/client/oidc"] = OIDCResource(hs) if hs.config.saml2_enabled: from synapse.rest.saml2 import SAML2Resource diff --git a/synapse/rest/synapse/client/oidc/__init__.py b/synapse/rest/synapse/client/oidc/__init__.py new file mode 100644 index 0000000000..64c0deb75d --- /dev/null +++ b/synapse/rest/synapse/client/oidc/__init__.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Quentin Gliech +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging + +from twisted.web.resource import Resource + +from synapse.rest.synapse.client.oidc.callback_resource import OIDCCallbackResource + +logger = logging.getLogger(__name__) + + +class OIDCResource(Resource): + def __init__(self, hs): + Resource.__init__(self) + self.putChild(b"callback", OIDCCallbackResource(hs)) + + +__all__ = ["OIDCResource"] diff --git a/synapse/rest/synapse/client/oidc/callback_resource.py b/synapse/rest/synapse/client/oidc/callback_resource.py new file mode 100644 index 0000000000..f7a0bc4bdb --- /dev/null +++ b/synapse/rest/synapse/client/oidc/callback_resource.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Quentin Gliech +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import logging + +from synapse.http.server import DirectServeHtmlResource + +logger = logging.getLogger(__name__) + + +class OIDCCallbackResource(DirectServeHtmlResource): + isLeaf = 1 + + def __init__(self, hs): + super().__init__() + self._oidc_handler = hs.get_oidc_handler() + + async def _async_render_GET(self, request): + await self._oidc_handler.handle_oidc_callback(request) diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index d8f90b9a80..ad20400b1d 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -40,7 +40,7 @@ ISSUER = "https://issuer/" CLIENT_ID = "test-client-id" CLIENT_SECRET = "test-client-secret" BASE_URL = "https://synapse/" -CALLBACK_URL = BASE_URL + "_synapse/oidc/callback" +CALLBACK_URL = BASE_URL + "_synapse/client/oidc/callback" SCOPES = ["openid"] AUTHORIZATION_ENDPOINT = ISSUER + "authorize" @@ -58,12 +58,6 @@ COMMON_CONFIG = { } -# The cookie name and path don't really matter, just that it has to be coherent -# between the callback & redirect handlers. -COOKIE_NAME = b"oidc_session" -COOKIE_PATH = "/_synapse/oidc" - - class TestMappingProvider: @staticmethod def parse_config(config): @@ -340,8 +334,11 @@ class OidcHandlerTestCase(HomeserverTestCase): # For some reason, call.args does not work with python3.5 args = calls[0][0] kwargs = calls[0][1] - self.assertEqual(args[0], COOKIE_NAME) - self.assertEqual(kwargs["path"], COOKIE_PATH) + + # The cookie name and path don't really matter, just that it has to be coherent + # between the callback & redirect handlers. + self.assertEqual(args[0], b"oidc_session") + self.assertEqual(kwargs["path"], "/_synapse/client/oidc") cookie = args[1] macaroon = pymacaroons.Macaroon.deserialize(cookie) -- cgit 1.5.1