Ask consent on SSO registration with default mxid (#10733)

Fixes #10732: consent flow skipped during SSO user registration if username is left at default

Signed-off-by: Andrew Ferrazzutti fair@miscworks.net
This commit is contained in:
AndrewFerr 2021-09-10 05:36:45 -04:00 committed by GitHub
parent 7f0565e029
commit 0c0da36a68
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 63 additions and 23 deletions

1
changelog.d/10733.bugfix Normal file
View File

@ -0,0 +1 @@
Allow user registration via SSO to require consent tracking for SSO mapping providers that don't prompt for Matrix ID selection. Contributed by @AndrewFerr.

View File

@ -444,14 +444,16 @@ class SsoHandler:
if not user_id: if not user_id:
attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper) attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
if attributes.localpart is None: next_step_url = self._get_url_for_next_new_user_step(
# the mapper doesn't return a username. bail out with a redirect to attributes=attributes
# the username picker. )
await self._redirect_to_username_picker( if next_step_url:
await self._redirect_to_next_new_user_step(
auth_provider_id, auth_provider_id,
remote_user_id, remote_user_id,
attributes, attributes,
client_redirect_url, client_redirect_url,
next_step_url,
extra_login_attributes, extra_login_attributes,
) )
@ -530,18 +532,53 @@ class SsoHandler:
) )
return attributes 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, self,
auth_provider_id: str, auth_provider_id: str,
remote_user_id: str, remote_user_id: str,
attributes: UserAttributes, attributes: UserAttributes,
client_redirect_url: str, client_redirect_url: str,
next_step_url: bytes,
extra_login_attributes: Optional[JsonDict], extra_login_attributes: Optional[JsonDict],
) -> NoReturn: ) -> NoReturn:
"""Creates a UsernameMappingSession and redirects the browser """Creates a UsernameMappingSession and redirects the browser
Called if the user mapping provider doesn't return a localpart for a new user. Called if the user mapping provider doesn't return complete information for a new user.
Raises a RedirectException which redirects the browser to the username picker. Raises a RedirectException which redirects the browser to a specified URL.
Args: Args:
auth_provider_id: A unique identifier for this SSO provider, e.g. 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 client_redirect_url: The redirect URL passed in by the client, which we
will eventually redirect back to. 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 extra_login_attributes: An optional dictionary of extra
attributes to be provided to the client in the login response. attributes to be provided to the client in the login response.
Raises: Raises:
RedirectException RedirectException
""" """
# TODO: If needed, allow using/looking up an existing session here.
session_id = random_string(16) session_id = random_string(16)
now = self._clock.time_msec() now = self._clock.time_msec()
session = UsernameMappingSession( session = UsernameMappingSession(
@ -570,13 +610,18 @@ class SsoHandler:
client_redirect_url=client_redirect_url, client_redirect_url=client_redirect_url,
expiry_time_ms=now + self._MAPPING_SESSION_VALIDITY_PERIOD_MS, expiry_time_ms=now + self._MAPPING_SESSION_VALIDITY_PERIOD_MS,
extra_login_attributes=extra_login_attributes, 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 self._username_mapping_sessions[session_id] = session
logger.info("Recorded registration session id %s", session_id) logger.info("Recorded registration session id %s", session_id)
# Set the cookie and redirect to the username picker # Set the cookie and redirect to the next step
e = RedirectException(b"/_synapse/client/pick_username/account_details") e = RedirectException(next_step_url)
e.cookies.append( e.cookies.append(
b"%s=%s; path=/" b"%s=%s; path=/"
% (USERNAME_MAPPING_SESSION_COOKIE_NAME, session_id.encode("ascii")) % (USERNAME_MAPPING_SESSION_COOKIE_NAME, session_id.encode("ascii"))
@ -805,16 +850,9 @@ class SsoHandler:
) )
session.emails_to_use = filtered_emails session.emails_to_use = filtered_emails
# we may now need to collect consent from the user, in which case, redirect respond_with_redirect(
# to the consent-extraction-unit request, self._get_url_for_next_new_user_step(session=session)
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)
async def handle_terms_accepted( async def handle_terms_accepted(
self, request: Request, session_id: str, terms_version: str self, request: Request, session_id: str, terms_version: str
@ -842,8 +880,9 @@ class SsoHandler:
session.terms_accepted_version = terms_version session.terms_accepted_version = terms_version
# we're done; now we can register the user respond_with_redirect(
respond_with_redirect(request, b"/_synapse/client/sso_register") request, self._get_url_for_next_new_user_step(session=session)
)
async def register_sso_user(self, request: Request, session_id: str) -> None: 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. """Called once we have all the info we need to register a new user.

View File

@ -63,8 +63,8 @@ class NewUserConsentResource(DirectServeHtmlResource):
self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code) self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code)
return return
# It should be impossible to get here without having first been through # It should be impossible to get here without either the user or the mapping provider
# the pick-a-username step, which ensures chosen_localpart gets set. # having chosen a username, which ensures chosen_localpart gets set.
if not session.chosen_localpart: if not session.chosen_localpart:
logger.warning("Session has no user name selected") logger.warning("Session has no user name selected")
self._sso_handler.render_error( self._sso_handler.render_error(