From 85c56b5a679add887ec9716f176949561dca581b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 1 Feb 2021 17:30:42 +0000 Subject: Make importing display name and email optional (#9277) --- synapse/handlers/sso.py | 52 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 8 deletions(-) (limited to 'synapse/handlers/sso.py') diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index ff4750999a..d7ca2918f8 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -14,7 +14,16 @@ # limitations under the License. import abc import logging -from typing import TYPE_CHECKING, Awaitable, Callable, Dict, List, Mapping, Optional +from typing import ( + TYPE_CHECKING, + Awaitable, + Callable, + Dict, + Iterable, + Mapping, + Optional, + Set, +) from urllib.parse import urlencode import attr @@ -29,7 +38,7 @@ from synapse.handlers.ui_auth import UIAuthSessionDataConstants from synapse.http import get_request_user_agent 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.types import Collection, JsonDict, UserID, contains_invalid_mxid_characters from synapse.util.async_helpers import Linearizer from synapse.util.stringutils import random_string @@ -115,7 +124,7 @@ class UserAttributes: # enter one. localpart = attr.ib(type=Optional[str]) display_name = attr.ib(type=Optional[str], default=None) - emails = attr.ib(type=List[str], default=attr.Factory(list)) + emails = attr.ib(type=Collection[str], default=attr.Factory(list)) @attr.s(slots=True) @@ -130,7 +139,7 @@ class UsernameMappingSession: # attributes returned by the ID mapper display_name = attr.ib(type=Optional[str]) - emails = attr.ib(type=List[str]) + emails = attr.ib(type=Collection[str]) # An optional dictionary of extra attributes to be provided to the client in the # login response. @@ -144,6 +153,8 @@ class UsernameMappingSession: # choices made by the user chosen_localpart = attr.ib(type=Optional[str], default=None) + use_display_name = attr.ib(type=bool, default=True) + emails_to_use = attr.ib(type=Collection[str], default=()) # the HTTP cookie used to track the mapping session id @@ -710,7 +721,12 @@ class SsoHandler: return not user_infos async def handle_submit_username_request( - self, request: SynapseRequest, localpart: str, session_id: str + self, + request: SynapseRequest, + session_id: str, + localpart: str, + use_display_name: bool, + emails_to_use: Iterable[str], ) -> None: """Handle a request to the username-picker 'submit' endpoint @@ -720,11 +736,30 @@ class SsoHandler: request: HTTP request localpart: localpart requested by the user session_id: ID of the username mapping session, extracted from a cookie + use_display_name: whether the user wants to use the suggested display name + emails_to_use: emails that the user would like to use """ session = self.get_mapping_session(session_id) # update the session with the user's choices session.chosen_localpart = localpart + session.use_display_name = use_display_name + + emails_from_idp = set(session.emails) + filtered_emails = set() # type: Set[str] + + # we iterate through the list rather than just building a set conjunction, so + # that we can log attempts to use unknown addresses + for email in emails_to_use: + if email in emails_from_idp: + filtered_emails.add(email) + else: + logger.warning( + "[session %s] ignoring user request to use unknown email address %r", + session_id, + email, + ) + session.emails_to_use = filtered_emails # we're done; now we can register the user respond_with_redirect(request, b"/_synapse/client/sso_register") @@ -747,11 +782,12 @@ class SsoHandler: ) attributes = UserAttributes( - localpart=session.chosen_localpart, - display_name=session.display_name, - emails=session.emails, + localpart=session.chosen_localpart, emails=session.emails_to_use, ) + if session.use_display_name: + attributes.display_name = session.display_name + # the following will raise a 400 error if the username has been taken in the # meantime. user_id = await self._register_mapped_user( -- cgit 1.5.1 From c543bf87ecf295fa68311beabd1dc013288a2e98 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 1 Feb 2021 18:37:41 +0000 Subject: Collect terms consent from the user during SSO registration (#9276) --- changelog.d/9276.feature | 1 + docs/sample_config.yaml | 22 ++++++ docs/workers.md | 1 + synapse/config/sso.py | 22 ++++++ synapse/handlers/register.py | 2 + synapse/handlers/sso.py | 44 +++++++++++ synapse/res/templates/sso_new_user_consent.html | 39 ++++++++++ synapse/rest/synapse/client/__init__.py | 2 + synapse/rest/synapse/client/new_user_consent.py | 97 +++++++++++++++++++++++++ 9 files changed, 230 insertions(+) create mode 100644 changelog.d/9276.feature create mode 100644 synapse/res/templates/sso_new_user_consent.html create mode 100644 synapse/rest/synapse/client/new_user_consent.py (limited to 'synapse/handlers/sso.py') diff --git a/changelog.d/9276.feature b/changelog.d/9276.feature new file mode 100644 index 0000000000..c21b197ca1 --- /dev/null +++ b/changelog.d/9276.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 eec082ca8c..15e9746696 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -2003,6 +2003,28 @@ sso: # # * username: the localpart of the user's chosen user id # + # * HTML page allowing the user to consent to the server's terms and + # conditions. This is only shown for new users, and only if + # `user_consent.require_at_registration` is set. + # + # When rendering, this template is given the following variables: + # + # * server_name: the homeserver's name. + # + # * user_id: the user's matrix proposed ID. + # + # * user_profile.display_name: the user's proposed display name, if any. + # + # * consent_version: the version of the terms that the user will be + # shown + # + # * terms_url: a link to the page showing the terms. + # + # The template should render a form which submits the following fields: + # + # * accepted_version: the version of the terms accepted by the user + # (ie, 'consent_version' from the input variables). + # # * HTML page for a confirmation step before redirecting back to the client # with the login token: 'sso_redirect_confirm.html'. # diff --git a/docs/workers.md b/docs/workers.md index 6b8887de36..0da805c333 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -259,6 +259,7 @@ using): ^/_matrix/client/(api/v1|r0|unstable)/login/sso/redirect ^/_synapse/client/pick_idp$ ^/_synapse/client/pick_username + ^/_synapse/client/new_user_consent$ ^/_synapse/client/sso_register$ # OpenID Connect requests. diff --git a/synapse/config/sso.py b/synapse/config/sso.py index bf82183cdc..939eeac6de 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -158,6 +158,28 @@ class SSOConfig(Config): # # * username: the localpart of the user's chosen user id # + # * HTML page allowing the user to consent to the server's terms and + # conditions. This is only shown for new users, and only if + # `user_consent.require_at_registration` is set. + # + # When rendering, this template is given the following variables: + # + # * server_name: the homeserver's name. + # + # * user_id: the user's matrix proposed ID. + # + # * user_profile.display_name: the user's proposed display name, if any. + # + # * consent_version: the version of the terms that the user will be + # shown + # + # * terms_url: a link to the page showing the terms. + # + # The template should render a form which submits the following fields: + # + # * accepted_version: the version of the terms accepted by the user + # (ie, 'consent_version' from the input variables). + # # * HTML page for a confirmation step before redirecting back to the client # with the login token: 'sso_redirect_confirm.html'. # diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index b20a5d8605..49b085269b 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -694,6 +694,8 @@ class RegistrationHandler(BaseHandler): access_token: The access token of the newly logged in device, or None if `inhibit_login` enabled. """ + # TODO: 3pid registration can actually happen on the workers. Consider + # refactoring it. if self.hs.config.worker_app: await self._post_registration_client( user_id=user_id, auth_result=auth_result, access_token=access_token diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index d7ca2918f8..b450668f1c 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -155,6 +155,7 @@ class UsernameMappingSession: chosen_localpart = attr.ib(type=Optional[str], default=None) use_display_name = attr.ib(type=bool, default=True) emails_to_use = attr.ib(type=Collection[str], default=()) + terms_accepted_version = attr.ib(type=Optional[str], default=None) # the HTTP cookie used to track the mapping session id @@ -190,6 +191,8 @@ class SsoHandler: # map from idp_id to SsoIdentityProvider self._identity_providers = {} # type: Dict[str, SsoIdentityProvider] + self._consent_at_registration = hs.config.consent.user_consent_at_registration + def register_identity_provider(self, p: SsoIdentityProvider): p_id = p.idp_id assert p_id not in self._identity_providers @@ -761,6 +764,38 @@ 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) + + async def handle_terms_accepted( + self, request: Request, session_id: str, terms_version: str + ): + """Handle a request to the new-user 'consent' endpoint + + Will serve an HTTP response to the request. + + Args: + request: HTTP request + session_id: ID of the username mapping session, extracted from a cookie + terms_version: the version of the terms which the user viewed and consented + to + """ + logger.info( + "[session %s] User consented to terms version %s", + session_id, + terms_version, + ) + session = self.get_mapping_session(session_id) + session.terms_accepted_version = terms_version + # we're done; now we can register the user respond_with_redirect(request, b"/_synapse/client/sso_register") @@ -816,6 +851,15 @@ class SsoHandler: path=b"/", ) + auth_result = {} + if session.terms_accepted_version: + # TODO: make this less awful. + auth_result[LoginType.TERMS] = True + + await self._registration_handler.post_registration_actions( + user_id, auth_result, access_token=None + ) + await self._auth_handler.complete_sso_login( user_id, request, diff --git a/synapse/res/templates/sso_new_user_consent.html b/synapse/res/templates/sso_new_user_consent.html new file mode 100644 index 0000000000..8c33787c54 --- /dev/null +++ b/synapse/res/templates/sso_new_user_consent.html @@ -0,0 +1,39 @@ + + + + + SSO redirect confirmation + + + + +
+

Your account is nearly ready

+

Agree to the terms to create your account.

+
+
+ +
+ +
+
{{ user_profile.display_name }}
+
{{ user_id }}
+
+
+ + +
+ + diff --git a/synapse/rest/synapse/client/__init__.py b/synapse/rest/synapse/client/__init__.py index 6acbc03d73..02310c1900 100644 --- a/synapse/rest/synapse/client/__init__.py +++ b/synapse/rest/synapse/client/__init__.py @@ -17,6 +17,7 @@ from typing import TYPE_CHECKING, Mapping from twisted.web.resource import Resource +from synapse.rest.synapse.client.new_user_consent import NewUserConsentResource 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 @@ -39,6 +40,7 @@ def build_synapse_client_resource_tree(hs: "HomeServer") -> Mapping[str, Resourc # 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/new_user_consent": NewUserConsentResource(hs), "/_synapse/client/sso_register": SsoRegisterResource(hs), } diff --git a/synapse/rest/synapse/client/new_user_consent.py b/synapse/rest/synapse/client/new_user_consent.py new file mode 100644 index 0000000000..b2e0f93810 --- /dev/null +++ b/synapse/rest/synapse/client/new_user_consent.py @@ -0,0 +1,97 @@ +# -*- 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, respond_with_html +from synapse.http.servlet import parse_string +from synapse.types import UserID +from synapse.util.templates import build_jinja_env + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class NewUserConsentResource(DirectServeHtmlResource): + """A resource which collects consent to the server's terms from a new user + + This resource gets mounted at /_synapse/client/new_user_consent, and is shown + when we are automatically creating a new user due to an SSO login. + + It shows a template which prompts the user to go and read the Ts and Cs, and click + a clickybox if they have done so. + """ + + def __init__(self, hs: "HomeServer"): + super().__init__() + self._sso_handler = hs.get_sso_handler() + self._server_name = hs.hostname + self._consent_version = hs.config.consent.user_consent_version + + def template_search_dirs(): + if hs.config.sso.sso_template_dir: + yield hs.config.sso.sso_template_dir + yield hs.config.sso.default_template_dir + + self._jinja_env = build_jinja_env(template_search_dirs(), hs.config) + + async def _async_render_GET(self, request: Request) -> None: + try: + session_id = get_username_mapping_session_cookie_from_request(request) + session = self._sso_handler.get_mapping_session(session_id) + except SynapseError as e: + logger.warning("Error fetching session: %s", e) + self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code) + return + + user_id = UserID(session.chosen_localpart, self._server_name) + user_profile = { + "display_name": session.display_name, + } + + template_params = { + "user_id": user_id.to_string(), + "user_profile": user_profile, + "consent_version": self._consent_version, + "terms_url": "/_matrix/consent?v=%s" % (self._consent_version,), + } + + template = self._jinja_env.get_template("sso_new_user_consent.html") + html = template.render(template_params) + respond_with_html(request, 200, html) + + async def _async_render_POST(self, request: Request): + 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 + + try: + accepted_version = parse_string(request, "accepted_version", required=True) + except SynapseError as e: + self._sso_handler.render_error(request, "bad_param", e.msg, code=e.code) + return + + await self._sso_handler.handle_terms_accepted( + request, session_id, accepted_version + ) -- cgit 1.5.1