diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index 0fdc6dd9e7..05aa76d6a6 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -444,14 +444,16 @@ class SsoHandler:
if not user_id:
attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
- if attributes.localpart is None:
- # the mapper doesn't return a username. bail out with a redirect to
- # the username picker.
- await self._redirect_to_username_picker(
+ next_step_url = self._get_url_for_next_new_user_step(
+ attributes=attributes
+ )
+ if next_step_url:
+ await self._redirect_to_next_new_user_step(
auth_provider_id,
remote_user_id,
attributes,
client_redirect_url,
+ next_step_url,
extra_login_attributes,
)
@@ -530,18 +532,53 @@ class SsoHandler:
)
return attributes
- async def _redirect_to_username_picker(
+ def _get_url_for_next_new_user_step(
+ self,
+ attributes: Optional[UserAttributes] = None,
+ session: Optional[UsernameMappingSession] = None,
+ ) -> bytes:
+ """Returns the URL to redirect to for the next step of new user registration
+
+ Given attributes from the user mapping provider or a UsernameMappingSession,
+ returns the URL to redirect to for the next step of the registration flow.
+
+ Args:
+ attributes: the user attributes returned by the user mapping provider,
+ from before a UsernameMappingSession has begun.
+
+ session: an active UsernameMappingSession, possibly with some of its
+ attributes chosen by the user.
+
+ Returns:
+ The URL to redirect to, or an empty value if no redirect is necessary
+ """
+ # Must provide either attributes or session, not both
+ assert (attributes is not None) != (session is not None)
+
+ if (attributes and attributes.localpart is None) or (
+ session and session.chosen_localpart is None
+ ):
+ return b"/_synapse/client/pick_username/account_details"
+ elif self._consent_at_registration and not (
+ session and session.terms_accepted_version
+ ):
+ return b"/_synapse/client/new_user_consent"
+ else:
+ return b"/_synapse/client/sso_register" if session else b""
+
+ async def _redirect_to_next_new_user_step(
self,
auth_provider_id: str,
remote_user_id: str,
attributes: UserAttributes,
client_redirect_url: str,
+ next_step_url: bytes,
extra_login_attributes: Optional[JsonDict],
) -> NoReturn:
"""Creates a UsernameMappingSession and redirects the browser
- Called if the user mapping provider doesn't return a localpart for a new user.
- Raises a RedirectException which redirects the browser to the username picker.
+ Called if the user mapping provider doesn't return complete information for a new user.
+ Raises a RedirectException which redirects the browser to a specified URL.
Args:
auth_provider_id: A unique identifier for this SSO provider, e.g.
@@ -554,12 +591,15 @@ class SsoHandler:
client_redirect_url: The redirect URL passed in by the client, which we
will eventually redirect back to.
+ next_step_url: The URL to redirect to for the next step of the new user flow.
+
extra_login_attributes: An optional dictionary of extra
attributes to be provided to the client in the login response.
Raises:
RedirectException
"""
+ # TODO: If needed, allow using/looking up an existing session here.
session_id = random_string(16)
now = self._clock.time_msec()
session = UsernameMappingSession(
@@ -570,13 +610,18 @@ class SsoHandler:
client_redirect_url=client_redirect_url,
expiry_time_ms=now + self._MAPPING_SESSION_VALIDITY_PERIOD_MS,
extra_login_attributes=extra_login_attributes,
+ # Treat the localpart returned by the user mapping provider as though
+ # it was chosen by the user. If it's None, it must be chosen eventually.
+ chosen_localpart=attributes.localpart,
+ # TODO: Consider letting the user mapping provider specify defaults for
+ # other user-chosen attributes.
)
self._username_mapping_sessions[session_id] = session
logger.info("Recorded registration session id %s", session_id)
- # Set the cookie and redirect to the username picker
- e = RedirectException(b"/_synapse/client/pick_username/account_details")
+ # Set the cookie and redirect to the next step
+ e = RedirectException(next_step_url)
e.cookies.append(
b"%s=%s; path=/"
% (USERNAME_MAPPING_SESSION_COOKIE_NAME, session_id.encode("ascii"))
@@ -805,16 +850,9 @@ class SsoHandler:
)
session.emails_to_use = filtered_emails
- # we may now need to collect consent from the user, in which case, redirect
- # to the consent-extraction-unit
- if self._consent_at_registration:
- redirect_url = b"/_synapse/client/new_user_consent"
-
- # otherwise, redirect to the completion page
- else:
- redirect_url = b"/_synapse/client/sso_register"
-
- respond_with_redirect(request, redirect_url)
+ respond_with_redirect(
+ request, self._get_url_for_next_new_user_step(session=session)
+ )
async def handle_terms_accepted(
self, request: Request, session_id: str, terms_version: str
@@ -842,8 +880,9 @@ class SsoHandler:
session.terms_accepted_version = terms_version
- # we're done; now we can register the user
- respond_with_redirect(request, b"/_synapse/client/sso_register")
+ respond_with_redirect(
+ request, self._get_url_for_next_new_user_step(session=session)
+ )
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.
diff --git a/synapse/rest/synapse/client/new_user_consent.py b/synapse/rest/synapse/client/new_user_consent.py
index fc62a09b7f..3869d18003 100644
--- a/synapse/rest/synapse/client/new_user_consent.py
+++ b/synapse/rest/synapse/client/new_user_consent.py
@@ -63,8 +63,8 @@ class NewUserConsentResource(DirectServeHtmlResource):
self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code)
return
- # It should be impossible to get here without having first been through
- # the pick-a-username step, which ensures chosen_localpart gets set.
+ # It should be impossible to get here without either the user or the mapping provider
+ # having chosen a username, which ensures chosen_localpart gets set.
if not session.chosen_localpart:
logger.warning("Session has no user name selected")
self._sso_handler.render_error(
|
