From f78d07bf005f7212bcc74256721677a3b255ea0e Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Mon, 1 Feb 2021 13:15:51 +0000
Subject: [PATCH 1/4] Split out a separate endpoint to complete SSO
registration (#9262)
There are going to be a couple of paths to get to the final step of SSO reg, and I want the URL in the browser to consistent. So, let's move the final step onto a separate path, which we redirect to.
---
changelog.d/9262.feature | 1 +
synapse/app/homeserver.py | 2 +
synapse/handlers/sso.py | 81 ++++++++++++++++----
synapse/http/server.py | 7 ++
synapse/rest/synapse/client/pick_username.py | 16 ++--
synapse/rest/synapse/client/sso_register.py | 50 ++++++++++++
tests/rest/client/v1/test_login.py | 14 +++-
7 files changed, 145 insertions(+), 26 deletions(-)
create mode 100644 changelog.d/9262.feature
create mode 100644 synapse/rest/synapse/client/sso_register.py
diff --git a/changelog.d/9262.feature b/changelog.d/9262.feature
new file mode 100644
index 0000000000..c21b197ca1
--- /dev/null
+++ b/changelog.d/9262.feature
@@ -0,0 +1 @@
+Improve the user experience of setting up an account via single-sign on.
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index 57a2f5237c..86d6f73674 100644
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -62,6 +62,7 @@ from synapse.rest.health import HealthResource
from synapse.rest.key.v2 import KeyApiV2Resource
from synapse.rest.synapse.client.pick_idp import PickIdpResource
from synapse.rest.synapse.client.pick_username import pick_username_resource
+from synapse.rest.synapse.client.sso_register import SsoRegisterResource
from synapse.rest.well_known import WellKnownResource
from synapse.server import HomeServer
from synapse.storage import DataStore
@@ -192,6 +193,7 @@ class SynapseHomeServer(HomeServer):
"/_synapse/admin": AdminRestResource(self),
"/_synapse/client/pick_username": pick_username_resource(self),
"/_synapse/client/pick_idp": PickIdpResource(self),
+ "/_synapse/client/sso_register": SsoRegisterResource(self),
}
)
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index 3308b037d2..50c5ae142a 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -21,12 +21,13 @@ import attr
from typing_extensions import NoReturn, Protocol
from twisted.web.http import Request
+from twisted.web.iweb import IRequest
from synapse.api.constants import LoginType
from synapse.api.errors import Codes, NotFoundError, RedirectException, SynapseError
from synapse.handlers.ui_auth import UIAuthSessionDataConstants
from synapse.http import get_request_user_agent
-from synapse.http.server import respond_with_html
+from synapse.http.server import respond_with_html, respond_with_redirect
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict, UserID, contains_invalid_mxid_characters
from synapse.util.async_helpers import Linearizer
@@ -141,6 +142,9 @@ class UsernameMappingSession:
# expiry time for the session, in milliseconds
expiry_time_ms = attr.ib(type=int)
+ # choices made by the user
+ chosen_localpart = attr.ib(type=Optional[str], default=None)
+
# the HTTP cookie used to track the mapping session id
USERNAME_MAPPING_SESSION_COOKIE_NAME = b"username_mapping_session"
@@ -647,6 +651,25 @@ class SsoHandler:
)
respond_with_html(request, 200, html)
+ def get_mapping_session(self, session_id: str) -> UsernameMappingSession:
+ """Look up the given username mapping session
+
+ If it is not found, raises a SynapseError with an http code of 400
+
+ Args:
+ session_id: session to look up
+ Returns:
+ active mapping session
+ Raises:
+ SynapseError if the session is not found/has expired
+ """
+ self._expire_old_sessions()
+ session = self._username_mapping_sessions.get(session_id)
+ if session:
+ return session
+ logger.info("Couldn't find session id %s", session_id)
+ raise SynapseError(400, "unknown session")
+
async def check_username_availability(
self, localpart: str, session_id: str,
) -> bool:
@@ -663,12 +686,7 @@ class SsoHandler:
# make sure that there is a valid mapping session, to stop people dictionary-
# scanning for accounts
-
- self._expire_old_sessions()
- session = self._username_mapping_sessions.get(session_id)
- if not session:
- logger.info("Couldn't find session id %s", session_id)
- raise SynapseError(400, "unknown session")
+ self.get_mapping_session(session_id)
logger.info(
"[session %s] Checking for availability of username %s",
@@ -696,16 +714,33 @@ class SsoHandler:
localpart: localpart requested by the user
session_id: ID of the username mapping session, extracted from a cookie
"""
- self._expire_old_sessions()
- session = self._username_mapping_sessions.get(session_id)
- if not session:
- logger.info("Couldn't find session id %s", session_id)
- raise SynapseError(400, "unknown session")
+ session = self.get_mapping_session(session_id)
- logger.info("[session %s] Registering localpart %s", session_id, localpart)
+ # update the session with the user's choices
+ session.chosen_localpart = localpart
+
+ # we're done; now we can register the user
+ respond_with_redirect(request, b"/_synapse/client/sso_register")
+
+ async def register_sso_user(self, request: Request, session_id: str) -> None:
+ """Called once we have all the info we need to register a new user.
+
+ Does so and serves an HTTP response
+
+ Args:
+ request: HTTP request
+ session_id: ID of the username mapping session, extracted from a cookie
+ """
+ session = self.get_mapping_session(session_id)
+
+ logger.info(
+ "[session %s] Registering localpart %s",
+ session_id,
+ session.chosen_localpart,
+ )
attributes = UserAttributes(
- localpart=localpart,
+ localpart=session.chosen_localpart,
display_name=session.display_name,
emails=session.emails,
)
@@ -720,7 +755,12 @@ class SsoHandler:
request.getClientIP(),
)
- logger.info("[session %s] Registered userid %s", session_id, user_id)
+ logger.info(
+ "[session %s] Registered userid %s with attributes %s",
+ session_id,
+ user_id,
+ attributes,
+ )
# delete the mapping session and the cookie
del self._username_mapping_sessions[session_id]
@@ -751,3 +791,14 @@ class SsoHandler:
for session_id in to_expire:
logger.info("Expiring mapping session %s", session_id)
del self._username_mapping_sessions[session_id]
+
+
+def get_username_mapping_session_cookie_from_request(request: IRequest) -> str:
+ """Extract the session ID from the cookie
+
+ Raises a SynapseError if the cookie isn't found
+ """
+ session_id = request.getCookie(USERNAME_MAPPING_SESSION_COOKIE_NAME)
+ if not session_id:
+ raise SynapseError(code=400, msg="missing session_id")
+ return session_id.decode("ascii", errors="replace")
diff --git a/synapse/http/server.py b/synapse/http/server.py
index d69d579b3a..8249732b27 100644
--- a/synapse/http/server.py
+++ b/synapse/http/server.py
@@ -761,6 +761,13 @@ def set_clickjacking_protection_headers(request: Request):
request.setHeader(b"Content-Security-Policy", b"frame-ancestors 'none';")
+def respond_with_redirect(request: Request, url: bytes) -> None:
+ """Write a 302 response to the request, if it is still alive."""
+ logger.debug("Redirect to %s", url.decode("utf-8"))
+ request.redirect(url)
+ finish_request(request)
+
+
def finish_request(request: Request):
""" Finish writing the response to the request.
diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py
index d3b6803e65..1bc737bad0 100644
--- a/synapse/rest/synapse/client/pick_username.py
+++ b/synapse/rest/synapse/client/pick_username.py
@@ -12,6 +12,7 @@
# 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.
+
from typing import TYPE_CHECKING
import pkg_resources
@@ -20,8 +21,7 @@ from twisted.web.http import Request
from twisted.web.resource import Resource
from twisted.web.static import File
-from synapse.api.errors import SynapseError
-from synapse.handlers.sso import USERNAME_MAPPING_SESSION_COOKIE_NAME
+from synapse.handlers.sso import get_username_mapping_session_cookie_from_request
from synapse.http.server import DirectServeHtmlResource, DirectServeJsonResource
from synapse.http.servlet import parse_string
from synapse.http.site import SynapseRequest
@@ -61,12 +61,10 @@ class AvailabilityCheckResource(DirectServeJsonResource):
async def _async_render_GET(self, request: Request):
localpart = parse_string(request, "username", required=True)
- session_id = request.getCookie(USERNAME_MAPPING_SESSION_COOKIE_NAME)
- if not session_id:
- raise SynapseError(code=400, msg="missing session_id")
+ session_id = get_username_mapping_session_cookie_from_request(request)
is_available = await self._sso_handler.check_username_availability(
- localpart, session_id.decode("ascii", errors="replace")
+ localpart, session_id
)
return 200, {"available": is_available}
@@ -79,10 +77,8 @@ class SubmitResource(DirectServeHtmlResource):
async def _async_render_POST(self, request: SynapseRequest):
localpart = parse_string(request, "username", required=True)
- session_id = request.getCookie(USERNAME_MAPPING_SESSION_COOKIE_NAME)
- if not session_id:
- raise SynapseError(code=400, msg="missing session_id")
+ session_id = get_username_mapping_session_cookie_from_request(request)
await self._sso_handler.handle_submit_username_request(
- request, localpart, session_id.decode("ascii", errors="replace")
+ request, localpart, session_id
)
diff --git a/synapse/rest/synapse/client/sso_register.py b/synapse/rest/synapse/client/sso_register.py
new file mode 100644
index 0000000000..dfefeb7796
--- /dev/null
+++ b/synapse/rest/synapse/client/sso_register.py
@@ -0,0 +1,50 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 The Matrix.org Foundation C.I.C.
+#
+# 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 typing import TYPE_CHECKING
+
+from twisted.web.http import Request
+
+from synapse.api.errors import SynapseError
+from synapse.handlers.sso import get_username_mapping_session_cookie_from_request
+from synapse.http.server import DirectServeHtmlResource
+
+if TYPE_CHECKING:
+ from synapse.server import HomeServer
+
+logger = logging.getLogger(__name__)
+
+
+class SsoRegisterResource(DirectServeHtmlResource):
+ """A resource which completes SSO registration
+
+ This resource gets mounted at /_synapse/client/sso_register, and is shown
+ after we collect username and/or consent for a new SSO user. It (finally) registers
+ the user, and confirms redirect to the client
+ """
+
+ def __init__(self, hs: "HomeServer"):
+ super().__init__()
+ self._sso_handler = hs.get_sso_handler()
+
+ async def _async_render_GET(self, request: Request) -> None:
+ try:
+ session_id = get_username_mapping_session_cookie_from_request(request)
+ except SynapseError as e:
+ logger.warning("Error fetching session cookie: %s", e)
+ self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code)
+ return
+ await self._sso_handler.register_sso_user(request, session_id)
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
index e2bb945453..f01215ed1c 100644
--- a/tests/rest/client/v1/test_login.py
+++ b/tests/rest/client/v1/test_login.py
@@ -31,6 +31,7 @@ from synapse.rest.client.v2_alpha import devices, register
from synapse.rest.client.v2_alpha.account import WhoamiRestServlet
from synapse.rest.synapse.client.pick_idp import PickIdpResource
from synapse.rest.synapse.client.pick_username import pick_username_resource
+from synapse.rest.synapse.client.sso_register import SsoRegisterResource
from synapse.types import create_requester
from tests import unittest
@@ -1215,6 +1216,7 @@ class UsernamePickerTestCase(HomeserverTestCase):
d = super().create_resource_dict()
d["/_synapse/client/pick_username"] = pick_username_resource(self.hs)
+ d["/_synapse/client/sso_register"] = SsoRegisterResource(self.hs)
d["/_synapse/oidc"] = OIDCResource(self.hs)
return d
@@ -1253,7 +1255,7 @@ class UsernamePickerTestCase(HomeserverTestCase):
self.assertApproximates(session.expiry_time_ms, expected_expiry, tolerance=1000)
# Now, submit a username to the username picker, which should serve a redirect
- # back to the client
+ # to the completion page
submit_path = picker_url + "/submit"
content = urlencode({b"username": b"bobby"}).encode("utf8")
chan = self.make_request(
@@ -1270,6 +1272,16 @@ class UsernamePickerTestCase(HomeserverTestCase):
)
self.assertEqual(chan.code, 302, chan.result)
location_headers = chan.headers.getRawHeaders("Location")
+
+ # send a request to the completion page, which should 302 to the client redirectUrl
+ chan = self.make_request(
+ "GET",
+ path=location_headers[0],
+ custom_headers=[("Cookie", "username_mapping_session=" + session_id)],
+ )
+ self.assertEqual(chan.code, 302, chan.result)
+ location_headers = chan.headers.getRawHeaders("Location")
+
# ensure that the returned location matches the requested redirect URL
path, query = location_headers[0].split("?", 1)
self.assertEqual(path, "https://x")
From 9c715a5f1981891815c124353ba15cf4d17bf9bb Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Mon, 1 Feb 2021 15:47:59 +0000
Subject: [PATCH 2/4] Fix SSO on workers (#9271)
Fixes #8966.
* Factor out build_synapse_client_resource_tree
Start a function which will mount resources common to all workers.
* Move sso init into build_synapse_client_resource_tree
... so that we don't have to do it for each worker
* Fix SSO-login-via-a-worker
Expose the SSO login endpoints on workers, like the documentation says.
* Update workers config for new endpoints
Add documentation for endpoints recently added (#8942, #9017, #9262)
* remove submit_token from workers endpoints list
this *doesn't* work on workers (yet).
* changelog
* Add a comment about the odd path for SAML2Resource
---
changelog.d/9271.bugfix | 1 +
docs/workers.md | 18 ++++---
synapse/app/generic_worker.py | 11 +++--
synapse/app/homeserver.py | 18 +------
synapse/rest/synapse/client/__init__.py | 49 ++++++++++++++++++-
.../storage/databases/main/registration.py | 40 +++++++--------
tests/rest/client/v1/test_login.py | 15 ++----
tests/rest/client/v2_alpha/test_auth.py | 6 +--
8 files changed, 93 insertions(+), 65 deletions(-)
create mode 100644 changelog.d/9271.bugfix
diff --git a/changelog.d/9271.bugfix b/changelog.d/9271.bugfix
new file mode 100644
index 0000000000..ef30c6570f
--- /dev/null
+++ b/changelog.d/9271.bugfix
@@ -0,0 +1 @@
+Fix single-sign-on when the endpoints are routed to synapse workers.
diff --git a/docs/workers.md b/docs/workers.md
index d01683681f..6b8887de36 100644
--- a/docs/workers.md
+++ b/docs/workers.md
@@ -225,7 +225,6 @@ expressions:
^/_matrix/client/(api/v1|r0|unstable)/joined_groups$
^/_matrix/client/(api/v1|r0|unstable)/publicised_groups$
^/_matrix/client/(api/v1|r0|unstable)/publicised_groups/
- ^/_synapse/client/password_reset/email/submit_token$
# Registration/login requests
^/_matrix/client/(api/v1|r0|unstable)/login$
@@ -256,25 +255,28 @@ Additionally, the following endpoints should be included if Synapse is configure
to use SSO (you only need to include the ones for whichever SSO provider you're
using):
+ # for all SSO providers
+ ^/_matrix/client/(api/v1|r0|unstable)/login/sso/redirect
+ ^/_synapse/client/pick_idp$
+ ^/_synapse/client/pick_username
+ ^/_synapse/client/sso_register$
+
# OpenID Connect requests.
- ^/_matrix/client/(api/v1|r0|unstable)/login/sso/redirect$
^/_synapse/oidc/callback$
# SAML requests.
- ^/_matrix/client/(api/v1|r0|unstable)/login/sso/redirect$
^/_matrix/saml2/authn_response$
# CAS requests.
- ^/_matrix/client/(api/v1|r0|unstable)/login/(cas|sso)/redirect$
^/_matrix/client/(api/v1|r0|unstable)/login/cas/ticket$
+Ensure that all SSO logins go to a single process.
+For multiple workers not handling the SSO endpoints properly, see
+[#7530](https://github.com/matrix-org/synapse/issues/7530).
+
Note that a HTTP listener with `client` and `federation` resources must be
configured in the `worker_listeners` option in the worker config.
-Ensure that all SSO logins go to a single process (usually the main process).
-For multiple workers not handling the SSO endpoints properly, see
-[#7530](https://github.com/matrix-org/synapse/issues/7530).
-
#### Load balancing
It is possible to run multiple instances of this worker app, with incoming requests
diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py
index e60988fa4a..516f2464b4 100644
--- a/synapse/app/generic_worker.py
+++ b/synapse/app/generic_worker.py
@@ -22,6 +22,7 @@ from typing import Dict, Iterable, Optional, Set
from typing_extensions import ContextManager
from twisted.internet import address
+from twisted.web.resource import IResource
import synapse
import synapse.events
@@ -90,9 +91,8 @@ from synapse.replication.tcp.streams import (
ToDeviceStream,
)
from synapse.rest.admin import register_servlets_for_media_repo
-from synapse.rest.client.v1 import events, room
+from synapse.rest.client.v1 import events, login, room
from synapse.rest.client.v1.initial_sync import InitialSyncRestServlet
-from synapse.rest.client.v1.login import LoginRestServlet
from synapse.rest.client.v1.profile import (
ProfileAvatarURLRestServlet,
ProfileDisplaynameRestServlet,
@@ -127,6 +127,7 @@ from synapse.rest.client.v2_alpha.sendtodevice import SendToDeviceRestServlet
from synapse.rest.client.versions import VersionsRestServlet
from synapse.rest.health import HealthResource
from synapse.rest.key.v2 import KeyApiV2Resource
+from synapse.rest.synapse.client import build_synapse_client_resource_tree
from synapse.server import HomeServer, cache_in_self
from synapse.storage.databases.main.censor_events import CensorEventsStore
from synapse.storage.databases.main.client_ips import ClientIpWorkerStore
@@ -507,7 +508,7 @@ class GenericWorkerServer(HomeServer):
site_tag = port
# We always include a health resource.
- resources = {"/health": HealthResource()}
+ resources = {"/health": HealthResource()} # type: Dict[str, IResource]
for res in listener_config.http_options.resources:
for name in res.names:
@@ -517,7 +518,7 @@ class GenericWorkerServer(HomeServer):
resource = JsonResource(self, canonical_json=False)
RegisterRestServlet(self).register(resource)
- LoginRestServlet(self).register(resource)
+ login.register_servlets(self, resource)
ThreepidRestServlet(self).register(resource)
DevicesRestServlet(self).register(resource)
KeyQueryServlet(self).register(resource)
@@ -557,6 +558,8 @@ class GenericWorkerServer(HomeServer):
groups.register_servlets(self, resource)
resources.update({CLIENT_API_PREFIX: resource})
+
+ resources.update(build_synapse_client_resource_tree(self))
elif name == "federation":
resources.update({FEDERATION_PREFIX: TransportLayerServer(self)})
elif name == "media":
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index 86d6f73674..244657cb88 100644
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -60,9 +60,7 @@ from synapse.rest import ClientRestResource
from synapse.rest.admin import AdminRestResource
from synapse.rest.health import HealthResource
from synapse.rest.key.v2 import KeyApiV2Resource
-from synapse.rest.synapse.client.pick_idp import PickIdpResource
-from synapse.rest.synapse.client.pick_username import pick_username_resource
-from synapse.rest.synapse.client.sso_register import SsoRegisterResource
+from synapse.rest.synapse.client import build_synapse_client_resource_tree
from synapse.rest.well_known import WellKnownResource
from synapse.server import HomeServer
from synapse.storage import DataStore
@@ -191,22 +189,10 @@ class SynapseHomeServer(HomeServer):
"/_matrix/client/versions": client_resource,
"/.well-known/matrix/client": WellKnownResource(self),
"/_synapse/admin": AdminRestResource(self),
- "/_synapse/client/pick_username": pick_username_resource(self),
- "/_synapse/client/pick_idp": PickIdpResource(self),
- "/_synapse/client/sso_register": SsoRegisterResource(self),
+ **build_synapse_client_resource_tree(self),
}
)
- if self.get_config().oidc_enabled:
- from synapse.rest.oidc import OIDCResource
-
- resources["/_synapse/oidc"] = OIDCResource(self)
-
- if self.get_config().saml2_enabled:
- from synapse.rest.saml2 import SAML2Resource
-
- resources["/_matrix/saml2"] = SAML2Resource(self)
-
if self.get_config().threepid_behaviour_email == ThreepidBehaviour.LOCAL:
from synapse.rest.synapse.client.password_reset import (
PasswordResetSubmitTokenResource,
diff --git a/synapse/rest/synapse/client/__init__.py b/synapse/rest/synapse/client/__init__.py
index c0b733488b..6acbc03d73 100644
--- a/synapse/rest/synapse/client/__init__.py
+++ b/synapse/rest/synapse/client/__init__.py
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
-# Copyright 2020 The Matrix.org Foundation C.I.C.
+# Copyright 2021 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@@ -12,3 +12,50 @@
# 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.
+
+from typing import TYPE_CHECKING, Mapping
+
+from twisted.web.resource import Resource
+
+from synapse.rest.synapse.client.pick_idp import PickIdpResource
+from synapse.rest.synapse.client.pick_username import pick_username_resource
+from synapse.rest.synapse.client.sso_register import SsoRegisterResource
+
+if TYPE_CHECKING:
+ from synapse.server import HomeServer
+
+
+def build_synapse_client_resource_tree(hs: "HomeServer") -> Mapping[str, Resource]:
+ """Builds a resource tree to include synapse-specific client resources
+
+ These are resources which should be loaded on all workers which expose a C-S API:
+ ie, the main process, and any generic workers so configured.
+
+ Returns:
+ map from path to Resource.
+ """
+ resources = {
+ # SSO bits. These are always loaded, whether or not SSO login is actually
+ # enabled (they just won't work very well if it's not)
+ "/_synapse/client/pick_idp": PickIdpResource(hs),
+ "/_synapse/client/pick_username": pick_username_resource(hs),
+ "/_synapse/client/sso_register": SsoRegisterResource(hs),
+ }
+
+ # 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
+
+ resources["/_synapse/oidc"] = OIDCResource(hs)
+
+ if hs.config.saml2_enabled:
+ from synapse.rest.saml2 import SAML2Resource
+
+ # This is mounted under '/_matrix' for backwards-compatibility.
+ resources["/_matrix/saml2"] = SAML2Resource(hs)
+
+ return resources
+
+
+__all__ = ["build_synapse_client_resource_tree"]
diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py
index 8d05288ed4..14c0878d81 100644
--- a/synapse/storage/databases/main/registration.py
+++ b/synapse/storage/databases/main/registration.py
@@ -443,6 +443,26 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
return await self.db_pool.runInteraction("get_users_by_id_case_insensitive", f)
+ async def record_user_external_id(
+ self, auth_provider: str, external_id: str, user_id: str
+ ) -> None:
+ """Record a mapping from an external user id to a mxid
+
+ Args:
+ auth_provider: identifier for the remote auth provider
+ external_id: id on that system
+ user_id: complete mxid that it is mapped to
+ """
+ await self.db_pool.simple_insert(
+ table="user_external_ids",
+ values={
+ "auth_provider": auth_provider,
+ "external_id": external_id,
+ "user_id": user_id,
+ },
+ desc="record_user_external_id",
+ )
+
async def get_user_by_external_id(
self, auth_provider: str, external_id: str
) -> Optional[str]:
@@ -1371,26 +1391,6 @@ class RegistrationStore(StatsStore, RegistrationBackgroundUpdateStore):
self._invalidate_cache_and_stream(txn, self.get_user_by_id, (user_id,))
- async def record_user_external_id(
- self, auth_provider: str, external_id: str, user_id: str
- ) -> None:
- """Record a mapping from an external user id to a mxid
-
- Args:
- auth_provider: identifier for the remote auth provider
- external_id: id on that system
- user_id: complete mxid that it is mapped to
- """
- await self.db_pool.simple_insert(
- table="user_external_ids",
- values={
- "auth_provider": auth_provider,
- "external_id": external_id,
- "user_id": user_id,
- },
- desc="record_user_external_id",
- )
-
async def user_set_password_hash(
self, user_id: str, password_hash: Optional[str]
) -> None:
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
index f01215ed1c..ded22a9767 100644
--- a/tests/rest/client/v1/test_login.py
+++ b/tests/rest/client/v1/test_login.py
@@ -29,9 +29,7 @@ from synapse.appservice import ApplicationService
from synapse.rest.client.v1 import login, logout
from synapse.rest.client.v2_alpha import devices, register
from synapse.rest.client.v2_alpha.account import WhoamiRestServlet
-from synapse.rest.synapse.client.pick_idp import PickIdpResource
-from synapse.rest.synapse.client.pick_username import pick_username_resource
-from synapse.rest.synapse.client.sso_register import SsoRegisterResource
+from synapse.rest.synapse.client import build_synapse_client_resource_tree
from synapse.types import create_requester
from tests import unittest
@@ -424,11 +422,8 @@ class MultiSSOTestCase(unittest.HomeserverTestCase):
return config
def create_resource_dict(self) -> Dict[str, Resource]:
- from synapse.rest.oidc import OIDCResource
-
d = super().create_resource_dict()
- d["/_synapse/client/pick_idp"] = PickIdpResource(self.hs)
- d["/_synapse/oidc"] = OIDCResource(self.hs)
+ d.update(build_synapse_client_resource_tree(self.hs))
return d
def test_get_login_flows(self):
@@ -1212,12 +1207,8 @@ class UsernamePickerTestCase(HomeserverTestCase):
return config
def create_resource_dict(self) -> Dict[str, Resource]:
- from synapse.rest.oidc import OIDCResource
-
d = super().create_resource_dict()
- d["/_synapse/client/pick_username"] = pick_username_resource(self.hs)
- d["/_synapse/client/sso_register"] = SsoRegisterResource(self.hs)
- d["/_synapse/oidc"] = OIDCResource(self.hs)
+ d.update(build_synapse_client_resource_tree(self.hs))
return d
def test_username_picker(self):
diff --git a/tests/rest/client/v2_alpha/test_auth.py b/tests/rest/client/v2_alpha/test_auth.py
index a6488a3d29..3f50c56745 100644
--- a/tests/rest/client/v2_alpha/test_auth.py
+++ b/tests/rest/client/v2_alpha/test_auth.py
@@ -22,7 +22,7 @@ from synapse.api.constants import LoginType
from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker
from synapse.rest.client.v1 import login
from synapse.rest.client.v2_alpha import auth, devices, register
-from synapse.rest.oidc import OIDCResource
+from synapse.rest.synapse.client import build_synapse_client_resource_tree
from synapse.types import JsonDict, UserID
from tests import unittest
@@ -173,9 +173,7 @@ class UIAuthTests(unittest.HomeserverTestCase):
def create_resource_dict(self):
resource_dict = super().create_resource_dict()
- if HAS_OIDC:
- # mount the OIDC resource at /_synapse/oidc
- resource_dict["/_synapse/oidc"] = OIDCResource(self.hs)
+ resource_dict.update(build_synapse_client_resource_tree(self.hs))
return resource_dict
def prepare(self, reactor, clock, hs):
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: [PATCH 3/4] 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 +++++++++++++++++++
.../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
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 }}
+
+