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: [PATCH] Make importing display name and email optional (#9277) --- changelog.d/9277.feature | 1 + synapse/handlers/register.py | 5 +- synapse/handlers/sso.py | 52 ++++++++++++++++--- .../templates/sso_auth_account_details.html | 23 ++++++++ synapse/rest/synapse/client/pick_username.py | 14 +++-- 5 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 changelog.d/9277.feature diff --git a/changelog.d/9277.feature b/changelog.d/9277.feature new file mode 100644 index 0000000000..c21b197ca1 --- /dev/null +++ b/changelog.d/9277.feature @@ -0,0 +1 @@ +Improve the user experience of setting up an account via single-sign on. diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index a2cf0f6f3e..b20a5d8605 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -14,8 +14,9 @@ # limitations under the License. """Contains functions for registering clients.""" + import logging -from typing import TYPE_CHECKING, List, Optional, Tuple +from typing import TYPE_CHECKING, Iterable, List, Optional, Tuple from synapse import types from synapse.api.constants import MAX_USERID_LENGTH, EventTypes, JoinRules, LoginType @@ -152,7 +153,7 @@ class RegistrationHandler(BaseHandler): user_type: Optional[str] = None, default_display_name: Optional[str] = None, address: Optional[str] = None, - bind_emails: List[str] = [], + bind_emails: Iterable[str] = [], by_admin: bool = False, user_agent_ips: Optional[List[Tuple[str, str]]] = None, ) -> str: 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( diff --git a/synapse/res/templates/sso_auth_account_details.html b/synapse/res/templates/sso_auth_account_details.html index f22b09aec1..105063825a 100644 --- a/synapse/res/templates/sso_auth_account_details.html +++ b/synapse/res/templates/sso_auth_account_details.html @@ -53,6 +53,14 @@ border-top: 1px solid #E9ECF1; padding: 12px; } + .idp-pick-details .check-row { + display: flex; + align-items: center; + } + + .idp-pick-details .check-row .name { + flex: 1; + } .idp-pick-details .use, .idp-pick-details .idp-value { color: #737D8C; @@ -91,16 +99,31 @@

Information from {{ idp.idp_name }}

{% if user_attributes.avatar_url %}
+
+ + + +
{% endif %} {% if user_attributes.display_name %}
+
+ + + +

{{ user_attributes.display_name }}

{% endif %} {% for email in user_attributes.emails %}
+
+ + + +

{{ email }}

{% endfor %} diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py index 27540d3bbe..96077cfcd1 100644 --- a/synapse/rest/synapse/client/pick_username.py +++ b/synapse/rest/synapse/client/pick_username.py @@ -14,7 +14,7 @@ # limitations under the License. import logging -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, List from twisted.web.http import Request from twisted.web.resource import Resource @@ -26,7 +26,7 @@ from synapse.http.server import ( DirectServeJsonResource, respond_with_html, ) -from synapse.http.servlet import parse_string +from synapse.http.servlet import parse_boolean, parse_string from synapse.http.site import SynapseRequest from synapse.util.templates import build_jinja_env @@ -113,11 +113,19 @@ class AccountDetailsResource(DirectServeHtmlResource): try: localpart = parse_string(request, "username", required=True) + use_display_name = parse_boolean(request, "use_display_name", default=False) + + try: + emails_to_use = [ + val.decode("utf-8") for val in request.args.get(b"use_email", []) + ] # type: List[str] + except ValueError: + raise SynapseError(400, "Query parameter use_email must be utf-8") except SynapseError as e: logger.warning("[session %s] bad param: %s", session_id, e) self._sso_handler.render_error(request, "bad_param", e.msg, code=e.code) return await self._sso_handler.handle_submit_username_request( - request, localpart, session_id + request, session_id, localpart, use_display_name, emails_to_use )