From ba2370b91f5a272e978f0ce0cabf95f0aada0d46 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 8 Dec 2020 14:02:06 +0000 Subject: [PATCH 01/16] Implement a username picker for synapse Fixes #8876 --- docs/sso_mapping_providers.md | 28 ++- synapse/app/homeserver.py | 2 + synapse/handlers/sso.py | 194 ++++++++++++++++++- synapse/res/username_picker/index.html | 20 ++ synapse/res/username_picker/script.js | 116 +++++++++++ synapse/res/username_picker/style.css | 179 +++++++++++++++++ synapse/rest/synapse/client/pick_username.py | 88 +++++++++ 7 files changed, 609 insertions(+), 18 deletions(-) create mode 100644 synapse/res/username_picker/index.html create mode 100644 synapse/res/username_picker/script.js create mode 100644 synapse/res/username_picker/style.css create mode 100644 synapse/rest/synapse/client/pick_username.py diff --git a/docs/sso_mapping_providers.md b/docs/sso_mapping_providers.md index 7714b1d844f5..e1d6ede7bac3 100644 --- a/docs/sso_mapping_providers.md +++ b/docs/sso_mapping_providers.md @@ -15,12 +15,18 @@ where SAML mapping providers come into play. SSO mapping providers are currently supported for OpenID and SAML SSO configurations. Please see the details below for how to implement your own. -It is the responsibility of the mapping provider to normalise the SSO attributes -and map them to a valid Matrix ID. The -[specification for Matrix IDs](https://matrix.org/docs/spec/appendices#user-identifiers) -has some information about what is considered valid. Alternately an easy way to -ensure it is valid is to use a Synapse utility function: -`synapse.types.map_username_to_mxid_localpart`. +It is up to the mapping provider whether the user should be assigned a predefined +Matrix ID based on the SSO attributes, or if the user should be allowed to +choose their own username. + +In the first case - where users are automatically allocated a Matrix ID - it is +the responsibility of the mapping provider to normalise the SSO attributes and +map them to a valid Matrix ID. The [specification for Matrix +IDs](https://matrix.org/docs/spec/appendices#user-identifiers) has some +information about what is considered valid. + +If the mapping provider does not assign a Matrix ID, then Synapse will +automatically serve an HTML page allowing the user to pick their own username. External mapping providers are provided to Synapse in the form of an external Python module. You can retrieve this module from [PyPI](https://pypi.org) or elsewhere, @@ -80,8 +86,9 @@ A custom mapping provider must specify the following methods: with failures=1. The method should then return a different `localpart` value, such as `john.doe1`. - Returns a dictionary with two keys: - - localpart: A required string, used to generate the Matrix ID. - - displayname: An optional string, the display name for the user. + - `localpart`: A string, used to generate the Matrix ID. If this is + `None`, the user is prompted to pick their own username. + - `displayname`: An optional string, the display name for the user. * `get_extra_attributes(self, userinfo, token)` - This method must be async. - Arguments: @@ -165,12 +172,13 @@ A custom mapping provider must specify the following methods: redirected to. - This method must return a dictionary, which will then be used by Synapse to build a new user. The following keys are allowed: - * `mxid_localpart` - Required. The mxid localpart of the new user. + * `mxid_localpart` - The mxid localpart of the new user. If this is + `None`, the user is prompted to pick their own username. * `displayname` - The displayname of the new user. If not provided, will default to the value of `mxid_localpart`. * `emails` - A list of emails for the new user. If not provided, will default to an empty list. - + Alternatively it can raise a `synapse.api.errors.RedirectException` to redirect the user to another page. This is useful to prompt the user for additional information, e.g. if you want them to provide their own username. diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index bbb740783801..8d9b53be534f 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -63,6 +63,7 @@ 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_username import pick_username_resource from synapse.rest.well_known import WellKnownResource from synapse.server import HomeServer from synapse.storage import DataStore @@ -192,6 +193,7 @@ def _configure_named_resource(self, name, compress=False): "/_matrix/client/versions": client_resource, "/.well-known/matrix/client": WellKnownResource(self), "/_synapse/admin": AdminRestResource(self), + "/_synapse/client/pick_username": pick_username_resource(self), } ) diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index f054b66a53da..ba9b15fcdbf0 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -13,17 +13,19 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional +from typing import TYPE_CHECKING, Awaitable, Callable, Dict, List, Optional import attr +from typing_extensions import NoReturn from twisted.web.http import Request -from synapse.api.errors import RedirectException +from synapse.api.errors import RedirectException, SynapseError from synapse.http.server import respond_with_html from synapse.http.site import SynapseRequest from synapse.types import JsonDict, UserID, contains_invalid_mxid_characters from synapse.util.async_helpers import Linearizer +from synapse.util.stringutils import random_string if TYPE_CHECKING: from synapse.server import HomeServer @@ -40,16 +42,52 @@ class MappingException(Exception): @attr.s class UserAttributes: - localpart = attr.ib(type=str) + # the localpart of the mxid that the mapper has assigned to the user. + # if `None`, the mapper has not picked a userid, and the user should be prompted to + # 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)) +@attr.s +class UsernameMappingSession: + """Data we track about SSO sessions""" + + # A unique identifier for this SSO provider, e.g. "oidc" or "saml". + auth_provider_id = attr.ib(type=str) + + # user ID on the IdP server + remote_user_id = attr.ib(type=str) + + # attributes returned by the ID mapper + display_name = attr.ib(type=Optional[str]) + emails = attr.ib(type=List[str]) + + # An optional dictionary of extra attributes to be provided to the client in the + # login response. + extra_login_attributes = attr.ib(type=Optional[JsonDict]) + + # where to redirect the client back to + client_redirect_url = attr.ib(type=str) + + # expiry time for the session, in milliseconds + expiry_time_ms = attr.ib(type=int) + + +# the HTTP cookie used to track the mapping session id +USERNAME_MAPPING_SESSION_COOKIE_NAME = b"username_mapping_session" + + class SsoHandler: # The number of attempts to ask the mapping provider for when generating an MXID. _MAP_USERNAME_RETRIES = 1000 + # the time a UsernameMappingSession remains valid for + _MAPPING_SESSION_VALIDITY_PERIOD_MS = 15 * 60 * 1000 + def __init__(self, hs: "HomeServer"): + self._clock = hs.get_clock() self._store = hs.get_datastore() self._server_name = hs.hostname self._registration_handler = hs.get_registration_handler() @@ -59,6 +97,9 @@ def __init__(self, hs: "HomeServer"): # a lock on the mappings self._mapping_lock = Linearizer(name="sso_user_mapping", clock=hs.get_clock()) + # a map from session id to session data + self._username_mapping_sessions = {} # type: Dict[str, UsernameMappingSession] + def render_error( self, request, error: str, error_description: Optional[str] = None ) -> None: @@ -206,6 +247,18 @@ async def complete_sso_login_request( # Otherwise, generate a new user. if not user_id: attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper) + + if attributes.localpart is None: + # the mapper didn't return a username. bail out with a redirect to + # the username picker. + await self._redirect_to_username_picker( + auth_provider_id, + remote_user_id, + attributes, + client_redirect_url, + extra_login_attributes, + ) + user_id = await self._register_mapped_user( attributes, auth_provider_id, @@ -243,10 +296,8 @@ async def _call_attribute_mapper( ) if not attributes.localpart: - raise MappingException( - "Error parsing SSO response: SSO mapping provider plugin " - "did not return a localpart value" - ) + # the mapper has not picked a localpart + return attributes # Check if this mxid already exists user_id = UserID(attributes.localpart, self._server_name).to_string() @@ -261,6 +312,37 @@ async def _call_attribute_mapper( ) return attributes + async def _redirect_to_username_picker( + self, + auth_provider_id: str, + remote_user_id: str, + attributes: UserAttributes, + client_redirect_url: str, + extra_login_attributes: Optional[JsonDict], + ) -> NoReturn: + session_id = random_string(16) + now = self._clock.time_msec() + session = UsernameMappingSession( + auth_provider_id=auth_provider_id, + remote_user_id=remote_user_id, + display_name=attributes.display_name, + emails=attributes.emails, + client_redirect_url=client_redirect_url, + expiry_time_ms=now + self._MAPPING_SESSION_VALIDITY_PERIOD_MS, + extra_login_attributes=extra_login_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") + e.cookies.append( + b"%s=%s; path=/" + % (USERNAME_MAPPING_SESSION_COOKIE_NAME, session_id.encode("ascii")) + ) + raise e + async def _register_mapped_user( self, attributes: UserAttributes, @@ -271,7 +353,9 @@ async def _register_mapped_user( ) -> str: # Since the localpart is provided via a potentially untrusted module, # ensure the MXID is valid before registering. - if contains_invalid_mxid_characters(attributes.localpart): + if not attributes.localpart or contains_invalid_mxid_characters( + attributes.localpart + ): raise MappingException("localpart is invalid: %s" % (attributes.localpart,)) logger.debug("Mapped SSO user to local part %s", attributes.localpart) @@ -326,3 +410,97 @@ async def complete_sso_ui_auth_request( await self._auth_handler.complete_sso_ui_auth( user_id, ui_auth_session_id, request ) + + async def check_username_availability( + self, localpart: str, session_id: str, + ) -> bool: + """Handle an "is username available" callback check + + Args: + localpart: desired localpart + session_id: the session id for the username picker + Returns: + True if the username is available + Raises: + SynapseError if the localpart is invalid or the session is unknown + """ + + # 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") + + logger.info( + "[session %s] Checking for availability of username %s", + session_id, + localpart, + ) + + if contains_invalid_mxid_characters(localpart): + raise SynapseError(400, "localpart is invalid: %s" % (localpart,)) + user_id = UserID(localpart, self._server_name).to_string() + user_infos = await self._store.get_users_by_id_case_insensitive(user_id) + + logger.info("[session %s] users: %s", session_id, user_infos) + return not user_infos + + async def handle_submit_username_request( + self, request: SynapseRequest, localpart: str, session_id: str + ) -> None: + 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") + + logger.info("[session %s] Registering localpart %s", session_id, localpart) + + attributes = UserAttributes( + localpart=localpart, + display_name=session.display_name, + emails=session.emails, + ) + + user_id = await self._register_mapped_user( + attributes, + session.auth_provider_id, + session.remote_user_id, + request.get_user_agent(""), + request.getClientIP(), + ) + + logger.info("[session %s] Registered userid %s", session_id, user_id) + + # delete the mapping session and the cookie + del self._username_mapping_sessions[session_id] + + # delete the cookie + request.addCookie( + USERNAME_MAPPING_SESSION_COOKIE_NAME, + b"", + expires=b"Thu, 01 Jan 1970 00:00:00 GMT", + path=b"/", + ) + + await self._auth_handler.complete_sso_login( + user_id, + request, + session.client_redirect_url, + session.extra_login_attributes, + ) + + def _expire_old_sessions(self): + to_expire = [] + now = int(self._clock.time_msec()) + + for session_id, session in self._username_mapping_sessions.items(): + if session.expiry_time_ms <= now: + to_expire.append(session_id) + + for session_id in to_expire: + logger.info("Expiring mapping session %s", session_id) + del self._username_mapping_sessions[session_id] diff --git a/synapse/res/username_picker/index.html b/synapse/res/username_picker/index.html new file mode 100644 index 000000000000..c6463a8175b4 --- /dev/null +++ b/synapse/res/username_picker/index.html @@ -0,0 +1,20 @@ + + + + Synapse Login + + + +
+
+ + + +
+ + +
+ + diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js new file mode 100644 index 000000000000..669def8474fb --- /dev/null +++ b/synapse/res/username_picker/script.js @@ -0,0 +1,116 @@ +let inputField = document.getElementById("field-username"); +let inputForm = document.getElementById("form"); +let submitButton = document.getElementById("button-submit"); +let message = document.getElementById("message"); + +// Remove input field placeholder if the text field is not empty +let switchClass = function(input) { + if (input.value.length > 0) { + input.classList.add('has-contents'); + } + else { + input.classList.remove('has-contents'); + } +}; + +// Submit username and receive response +let showMessage = function(messageText) { + // Unhide the message text + message.classList.remove("hidden"); + + message.innerHTML = messageText; +}; + +let onResponse = function(response, success) { + // Display message + showMessage(response); + + if(success) { + inputForm.submit(); + return; + } + + // Enable submit button and input field + submitButton.classList.remove('button--disabled'); + submitButton.value = "Submit" +}; + +let allowedUsernameCharacters = RegExp("[^a-z0-9\\.\\_\\=\\-\\/]"); +let usernameIsValid = function(username) { + return !allowedUsernameCharacters.test(username); +} +let allowedCharactersString = "" + +"lowercase letters, " + +"digits, " + +"., " + +"_, " + +"-, " + +"/, " + +"="; + +let buildQueryString = function(params) { + return Object.keys(params) + .map(k => encodeURIComponent(k) + '=' + encodeURIComponent(params[k])) + .join('&'); +} + +let submitUsername = function(username) { + if(username.length == 0) { + onResponse("Please enter a username.", false); + return; + } + if(!usernameIsValid(username)) { + onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString, false); + return; + } + + let check_uri = 'check?' + buildQueryString({"username": username}); + fetch(check_uri, { + "credentials": "include", + }).then((response) => { + if(!response.ok) { + // for non-200 responses, raise the body of the response as an exception + return response.text().then((text) => { throw text }); + } else { + return response.json() + } + }).then((json) => { + if(json.error) { + throw json.error; + } else if(json.available) { + onResponse("Success. Please wait a moment for your browser to redirect.", true); + } else { + onResponse("This username is not available, please choose another.", false); + } + }).catch((err) => { + onResponse("Error checking username availability: " + err, false); + }); +} + +let clickSubmit = function() { + if(submitButton.classList.contains('button--disabled')) { return; } + + // Disable submit button and input field + submitButton.classList.add('button--disabled'); + + // Submit username + submitButton.value = "Checking..."; + submitUsername(inputField.value); +}; + +submitButton.onclick = clickSubmit; + +// Listen for events on inputField +inputField.addEventListener('keypress', function(event) { + // Listen for Enter on input field + if(event.which === 13) { + event.preventDefault(); + clickSubmit(); + return true; + } + switchClass(inputField); +}); +inputField.addEventListener('change', function() { + switchClass(inputField); +}); + diff --git a/synapse/res/username_picker/style.css b/synapse/res/username_picker/style.css new file mode 100644 index 000000000000..472ee919f73c --- /dev/null +++ b/synapse/res/username_picker/style.css @@ -0,0 +1,179 @@ +body { + background: #ededf0; + color: #737373; + font-family: "Open Sans", sans-serif; + letter-spacing: 0.03em; + margin: 0; + padding: 0; + display: grid; + grid-template-rows: auto 1fr; } + +.card { + background-color: #fff; + padding: 2em; + position: relative; + margin: auto; + width: 100%; + box-shadow: 0 0.25em 0.25em 0 rgba(210, 210, 210, 0.5); + border-radius: 0.125em; } + @media (min-width: 25em) { + .card { + max-width: 26em; + padding: 2.5em; } } + @supports (display: grid) { + .card { + grid-row-start: 2; } + @media (min-height: 50em) { + .card { + top: -3em; + /* compensate for negative margin for footer links */ } } } + .card__back { + margin-bottom: 1em; } + .card__heading { + font-size: 1.4em; + font-weight: 400; + text-transform: capitalize; + padding-left: 2.125em; + position: relative; + min-height: 1.5em; } + .card__heading--iconless { + padding-left: 0; } + .card__heading img { + width: 1.5em; + height: 1.5em; + position: absolute; + left: 0; + top: 0; } + .card__heading--success { + color: #12bc00; } + .card__heading--error { + color: #ff0039; } + .card [data-screen]:focus { + outline: none; } + +* { + box-sizing: border-box; } + +form { + margin: 0; } + form * { + font-family: inherit; } + +label { + margin: 2em 0; + display: block; } + +input[type="text"], +input[type="email"], +input[type="password"] { + font-size: 100%; + background-color: #ededf0; + border: 1px solid #fff; + border-radius: .2em; + padding: .5em .9em; + display: block; + width: 100%; + margin-bottom: 1em; } + input[type="text"]:hover, input[type="text"]:focus, + input[type="email"]:hover, + input[type="email"]:focus, + input[type="password"]:hover, + input[type="password"]:focus { + border: 1px solid #0060df; + outline: none; } + .focus-styles input[type="text"]:focus, .focus-styles + input[type="email"]:focus, .focus-styles + input[type="password"]:focus { + border-color: transparent; } + +.form__input { + position: relative; } + p + .form__input { + margin-top: 2.5em; + /* leave space to fit a paragraph above a field */ } + .form__input label { + margin: 0; + position: absolute; + top: .5em; + left: .9em; } + .form__input input:focus + label, + .form__input input.has-contents + label { + position: absolute; + top: -1.5em; + color: #0060df; + font-weight: bold; } + .form__input input:focus + label > span, + .form__input input.has-contents + label > span { + font-size: 0.75em; } + +html, +body { + height: 100%; } + +.button { + text-align: center; + text-decoration: none; + padding: .93em 2em; + display: block; + font-size: 87.5%; + letter-spacing: .04em; + line-height: 1.57; + font-family: inherit; + border-radius: 2em; + background-color: #0060df; + color: #fff; + border: 1px solid transparent; + transition: background-color .1s ease-in-out; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + cursor: pointer; } + .button:hover { + background-color: #fff; + color: #0060df; + border-color: currentColor; + text-decoration: none; } + .button:active { + background-color: #0060df; + color: #fff; + border-color: #0060df; } + .button--full-width { + width: 100%; } + .button--secondary { + border-color: #b1b1b3; + background-color: transparent; + color: #000; + text-transform: none; } + .button--secondary:hover { + background-color: #000; + color: #fff; + border-color: transparent; } + .button--secondary:hover svg > path { + fill: #fff; } + .button--secondary:active { + background-color: transparent; + border-color: #000; + color: #000; } + .button--disabled { + border-color: #fff; + background-color: transparent; + color: #000; + text-transform: none; } + .button--disabled:hover { + background-color: #fff; + color: #000; + border-color: transparent; } + +.hidden { + display: none; } + +.tooltip { + background-color: #f9f9fa; + padding: 1em; + margin: 1em 0; } + .tooltip p:last-child { + margin-bottom: 0; } + .tooltip a:last-child { + margin-left: .5em; } + .tooltip:target { + display: block; } diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py new file mode 100644 index 000000000000..2a3a86b9cd51 --- /dev/null +++ b/synapse/rest/synapse/client/pick_username.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 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. +from typing import TYPE_CHECKING + +import pkg_resources + +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.http.server import DirectServeHtmlResource, DirectServeJsonResource +from synapse.http.servlet import parse_string +from synapse.http.site import SynapseRequest + +if TYPE_CHECKING: + from synapse.server import HomeServer + + +def pick_username_resource(hs: "HomeServer") -> Resource: + """Factory method to generate the username picker resource. + + This resource gets mounted under /_synapse/client/pick_username. The top-level + resource is just a File resource which serves up the static files in the resources + "res" directory, but it has a couple of children: + + * "submit", which does the mechanics of registering the new user, and redirects the + browser back to the client URL + + * "check": checks if a userid is free. + """ + + # XXX should we make this path customisable so that admins can restyle it? + base_path = pkg_resources.resource_filename("synapse", "res/username_picker") + + res = File(base_path) + res.putChild(b"submit", SubmitResource(hs)) + res.putChild(b"check", AvailabilityCheckResource(hs)) + + return res + + +class AvailabilityCheckResource(DirectServeJsonResource): + def __init__(self, hs: "HomeServer"): + super().__init__() + self._sso_handler = hs.get_sso_handler() + + 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") + + is_available = await self._sso_handler.check_username_availability( + localpart, session_id.decode("ascii", errors="replace") + ) + return 200, {"available": is_available} + + +class SubmitResource(DirectServeHtmlResource): + def __init__(self, hs: "HomeServer"): + super().__init__() + self._sso_handler = hs.get_sso_handler() + + 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") + + await self._sso_handler.handle_submit_username_request( + request, localpart, session_id.decode("ascii", errors="replace") + ) From f6841f00ec9cb28315574c80d25b32fe4272a931 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 15 Dec 2020 11:57:26 +0000 Subject: [PATCH 02/16] Make `localpart_template` optional for OIDC mapping provider --- docs/sample_config.yaml | 5 +-- synapse/config/oidc_config.py | 5 +-- synapse/handlers/oidc_handler.py | 53 +++++++++++++++----------------- synapse/types.py | 8 +++-- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 75a01094d57b..cba4b97de173 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1825,9 +1825,10 @@ oidc_config: # * user: The claims returned by the UserInfo Endpoint and/or in the ID # Token # - # This must be configured if using the default mapping provider. + # If this is not set, the user will be prompted to choose their + # own username. # - localpart_template: "{{ user.preferred_username }}" + #localpart_template: "{{ user.preferred_username }}" # Jinja2 template for the display name to set on first login. # diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py index 1abf8ed405a0..4e3055282d58 100644 --- a/synapse/config/oidc_config.py +++ b/synapse/config/oidc_config.py @@ -203,9 +203,10 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): # * user: The claims returned by the UserInfo Endpoint and/or in the ID # Token # - # This must be configured if using the default mapping provider. + # If this is not set, the user will be prompted to choose their + # own username. # - localpart_template: "{{{{ user.preferred_username }}}}" + #localpart_template: "{{{{ user.preferred_username }}}}" # Jinja2 template for the display name to set on first login. # diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index cbd11a138266..80112d51af1a 100644 --- a/synapse/handlers/oidc_handler.py +++ b/synapse/handlers/oidc_handler.py @@ -947,7 +947,7 @@ def _remote_id_from_userinfo(self, userinfo: UserInfo) -> str: UserAttributeDict = TypedDict( - "UserAttributeDict", {"localpart": str, "display_name": Optional[str]} + "UserAttributeDict", {"localpart": Optional[str], "display_name": Optional[str]} ) C = TypeVar("C") @@ -1029,7 +1029,7 @@ def jinja_finalize(thing): @attr.s class JinjaOidcMappingConfig: subject_claim = attr.ib() # type: str - localpart_template = attr.ib() # type: Template + localpart_template = attr.ib(type=Optional["Template"]) display_name_template = attr.ib() # type: Optional[Template] extra_attributes = attr.ib() # type: Dict[str, Template] @@ -1047,18 +1047,14 @@ def __init__(self, config: JinjaOidcMappingConfig): def parse_config(config: dict) -> JinjaOidcMappingConfig: subject_claim = config.get("subject_claim", "sub") - if "localpart_template" not in config: - raise ConfigError( - "missing key: oidc_config.user_mapping_provider.config.localpart_template" - ) - - try: - localpart_template = env.from_string(config["localpart_template"]) - except Exception as e: - raise ConfigError( - "invalid jinja template for oidc_config.user_mapping_provider.config.localpart_template: %r" - % (e,) - ) + localpart_template = None # type: Optional[Template] + if "localpart_template" in config: + try: + localpart_template = env.from_string(config["localpart_template"]) + except Exception as e: + raise ConfigError( + "invalid jinja template", path=["localpart_template"] + ) from e display_name_template = None # type: Optional[Template] if "display_name_template" in config: @@ -1066,26 +1062,22 @@ def parse_config(config: dict) -> JinjaOidcMappingConfig: display_name_template = env.from_string(config["display_name_template"]) except Exception as e: raise ConfigError( - "invalid jinja template for oidc_config.user_mapping_provider.config.display_name_template: %r" - % (e,) - ) + "invalid jinja template", path=["display_name_template"] + ) from e extra_attributes = {} # type Dict[str, Template] if "extra_attributes" in config: extra_attributes_config = config.get("extra_attributes") or {} if not isinstance(extra_attributes_config, dict): - raise ConfigError( - "oidc_config.user_mapping_provider.config.extra_attributes must be a dict" - ) + raise ConfigError("must be a dict", path=["extra_attributes"]) for key, value in extra_attributes_config.items(): try: extra_attributes[key] = env.from_string(value) except Exception as e: raise ConfigError( - "invalid jinja template for oidc_config.user_mapping_provider.config.extra_attributes.%s: %r" - % (key, e) - ) + "invalid jinja template", path=["extra_attributes", key] + ) from e return JinjaOidcMappingConfig( subject_claim=subject_claim, @@ -1100,14 +1092,17 @@ def get_remote_user_id(self, userinfo: UserInfo) -> str: async def map_user_attributes( self, userinfo: UserInfo, token: Token, failures: int ) -> UserAttributeDict: - localpart = self._config.localpart_template.render(user=userinfo).strip() + localpart = None + + if self._config.localpart_template: + localpart = self._config.localpart_template.render(user=userinfo).strip() - # Ensure only valid characters are included in the MXID. - localpart = map_username_to_mxid_localpart(localpart) + # Ensure only valid characters are included in the MXID. + localpart = map_username_to_mxid_localpart(localpart) - # Append suffix integer if last call to this function failed to produce - # a usable mxid. - localpart += str(failures) if failures else "" + # Append suffix integer if last call to this function failed to produce + # a usable mxid. + localpart += str(failures) if failures else "" display_name = None # type: Optional[str] if self._config.display_name_template is not None: diff --git a/synapse/types.py b/synapse/types.py index 3ab6bdbe0626..c7d4e9580917 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -349,15 +349,17 @@ def contains_invalid_mxid_characters(localpart: str) -> bool: ) -def map_username_to_mxid_localpart(username, case_sensitive=False): +def map_username_to_mxid_localpart( + username: Union[str, bytes], case_sensitive: bool = False +) -> str: """Map a username onto a string suitable for a MXID This follows the algorithm laid out at https://matrix.org/docs/spec/appendices.html#mapping-from-other-character-sets. Args: - username (unicode|bytes): username to be mapped - case_sensitive (bool): true if TEST and test should be mapped + username: username to be mapped + case_sensitive: true if TEST and test should be mapped onto different mxids Returns: From 7097e9d8f5dc97a1df70d2936975a0a2fab2698e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 15 Dec 2020 23:50:13 +0000 Subject: [PATCH 03/16] Add a test for the username picker flow ... along with a couple of extras for the regular flow. --- tests/handlers/test_oidc.py | 143 +++++++++++++++++++++++++++++++++++- tests/unittest.py | 8 +- 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index c54f1c5797c0..7890dcad13bf 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -13,14 +13,21 @@ # See the License for the specific language governing permissions and # limitations under the License. import json -from urllib.parse import parse_qs, urlparse +import re +from typing import Dict +from urllib.parse import parse_qs, urlencode, urlparse from mock import ANY, Mock, patch import pymacaroons +from twisted.web.resource import Resource + +from synapse.api.errors import RedirectException from synapse.handlers.oidc_handler import OidcError from synapse.handlers.sso import MappingException +from synapse.rest.client.v1 import login +from synapse.rest.synapse.client.pick_username import pick_username_resource from synapse.server import HomeServer from synapse.types import UserID @@ -793,6 +800,140 @@ def test_map_userinfo_to_user_retries(self): "mapping_error", "Unable to generate a Matrix ID from the SSO response" ) + def test_empty_localpart(self): + """Attempts to map onto an empty localpart should be rejected.""" + userinfo = { + "sub": "tester", + "username": "", + } + self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) + self.assertRenderedError("mapping_error", "localpart is invalid: ") + + @override_config( + { + "oidc_config": { + "user_mapping_provider": { + "config": {"localpart_template": "{{ user.username }}"} + } + } + } + ) + def test_null_localpart(self): + """Mapping onto a null localpart via an empty OIDC attribute should be rejected""" + userinfo = { + "sub": "tester", + "username": None, + } + self.get_success(_make_callback_with_userinfo(self.hs, userinfo)) + self.assertRenderedError("mapping_error", "localpart is invalid: ") + + +class UsernamePickerTestCase(HomeserverTestCase): + servlets = [login.register_servlets] + + def default_config(self): + config = super().default_config() + config["public_baseurl"] = BASE_URL + oidc_config = { + "enabled": True, + "client_id": CLIENT_ID, + "client_secret": CLIENT_SECRET, + "issuer": ISSUER, + "scopes": SCOPES, + "user_mapping_provider": { + "config": {"display_name_template": "{{ user.displayname }}"} + }, + } + + # Update this config with what's in the default config so that + # override_config works as expected. + oidc_config.update(config.get("oidc_config", {})) + config["oidc_config"] = oidc_config + + # whitelist this client URI so we redirect straight to it rather than + # serving a confirmation page + config["sso"] = {"client_whitelist": ["https://whitelisted.client"]} + return config + + def create_resource_dict(self) -> Dict[str, Resource]: + d = super().create_resource_dict() + d["/_synapse/client/pick_username"] = pick_username_resource(self.hs) + return d + + def test_username_picker(self): + """Test the happy path of a username picker flow.""" + client_redirect_url = "https://whitelisted.client" + + # first of all, mock up an OIDC callback to the OidcHandler, which should + # raise a RedirectException + userinfo = {"sub": "tester", "displayname": "Jonny"} + f = self.get_failure( + _make_callback_with_userinfo( + self.hs, userinfo, client_redirect_url=client_redirect_url + ), + RedirectException, + ) + + # check the Location and cookies returned by the RedirectException + self.assertEqual(f.value.location, b"/_synapse/client/pick_username") + cookieheader = f.value.cookies[0] + regex = re.compile(b"^username_mapping_session=([a-zA-Z]+);") + m = regex.search(cookieheader) + if not m: + self.fail("cookie header %s does not match %s" % (cookieheader, regex)) + + # introspect the sso handler a bit to check that the username mapping session + # looks ok. + session_id = m.group(1).decode("ascii") + username_mapping_sessions = self.hs.get_sso_handler()._username_mapping_sessions + self.assertIn( + session_id, username_mapping_sessions, "session id not found in map" + ) + session = username_mapping_sessions[session_id] + self.assertEqual(session.remote_user_id, "tester") + self.assertEqual(session.display_name, "Jonny") + self.assertEqual(session.client_redirect_url, client_redirect_url) + + # the expiry time should be about 15 minutes away + expected_expiry = self.clock.time_msec() + (15 * 60 * 1000) + self.assertGreaterEqual(session.expiry_time_ms, expected_expiry - 1000) + self.assertLessEqual(session.expiry_time_ms, expected_expiry + 1000) + + # Now, submit a username to the username picker, which should serve a redirect + # back to the client + submit_path = f.value.location + b"/submit" + content = urlencode({b"username": b"bobby"}).encode("utf8") + chan = self.make_request( + "POST", + path=submit_path, + content=content, + content_is_form=True, + custom_headers=[ + ("Cookie", cookieheader), + # old versions of twisted don't do form-parsing without a valid + # content-length header. + ("Content-Length", str(len(content))), + ], + ) + self.assertEqual(chan.code, 302, chan.result) + location_headers = chan.headers.getRawHeaders("Location") + self.assertEqual( + location_headers[0][: len(client_redirect_url)], client_redirect_url + ) + + # fish the login token out of the redirect uri + parts = urlparse(location_headers[0]) + query = parse_qs(parts.query) + login_token = query["loginToken"][0] + + # finally, submit the matrix login token to the login API, which gives us our + # matrix access token, mxid, and device id. + chan = self.make_request( + "POST", "/login", content={"type": "m.login.token", "token": login_token}, + ) + self.assertEqual(chan.code, 200, chan.result) + self.assertEqual(chan.json_body["user_id"], "@bobby:test") + async def _make_callback_with_userinfo( hs: HomeServer, userinfo: dict, client_redirect_url: str = "http://client/redirect" diff --git a/tests/unittest.py b/tests/unittest.py index 39e5e7b85c74..af7f752c5a69 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -20,7 +20,7 @@ import inspect import logging import time -from typing import Dict, Optional, Type, TypeVar, Union +from typing import Dict, Iterable, Optional, Tuple, Type, TypeVar, Union from mock import Mock, patch @@ -383,6 +383,9 @@ def make_request( federation_auth_origin: str = None, content_is_form: bool = False, await_result: bool = True, + custom_headers: Optional[ + Iterable[Tuple[Union[bytes, str], Union[bytes, str]]] + ] = None, ) -> FakeChannel: """ Create a SynapseRequest at the path using the method and containing the @@ -405,6 +408,8 @@ def make_request( true (the default), will pump the test reactor until the the renderer tells the channel the request is finished. + custom_headers: (name, value) pairs to add as request headers + Returns: The FakeChannel object which stores the result of the request. """ @@ -420,6 +425,7 @@ def make_request( federation_auth_origin, content_is_form, await_result, + custom_headers, ) def setup_test_homeserver(self, *args, **kwargs): From 352aebfcfb182fc5e341254cf33c94390c2442ce Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 15 Dec 2020 23:51:22 +0000 Subject: [PATCH 04/16] newsfile --- changelog.d/8942.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8942.feature diff --git a/changelog.d/8942.feature b/changelog.d/8942.feature new file mode 100644 index 000000000000..d450ef4998d9 --- /dev/null +++ b/changelog.d/8942.feature @@ -0,0 +1 @@ +Add support for allowing users to pick their own user ID during a single-sign-on login. From e529562fbf73334faf32ef0589d82aae5e67c0b5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 17 Dec 2020 17:13:29 +0000 Subject: [PATCH 05/16] Apply suggestions from code review Co-authored-by: Patrick Cloke --- synapse/handlers/sso.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index ba9b15fcdbf0..5f19554e528d 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -50,7 +50,7 @@ class UserAttributes: emails = attr.ib(type=List[str], default=attr.Factory(list)) -@attr.s +@attr.s(slots=True) class UsernameMappingSession: """Data we track about SSO sessions""" @@ -249,7 +249,7 @@ async def complete_sso_login_request( attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper) if attributes.localpart is None: - # the mapper didn't return a username. bail out with a redirect to + # the mapper doesn't return a username. bail out with a redirect to # the username picker. await self._redirect_to_username_picker( auth_provider_id, From c99aafa53f327117c8924fbee158ea46cd2f3bfe Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 17:43:33 +0000 Subject: [PATCH 06/16] address review comments --- synapse/handlers/oidc_handler.py | 8 +-- synapse/handlers/sso.py | 60 ++++++++++++++++++++ synapse/rest/synapse/client/pick_username.py | 4 +- tests/handlers/test_oidc.py | 6 +- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py index 80112d51af1a..709f8dfc1316 100644 --- a/synapse/handlers/oidc_handler.py +++ b/synapse/handlers/oidc_handler.py @@ -1028,10 +1028,10 @@ def jinja_finalize(thing): @attr.s class JinjaOidcMappingConfig: - subject_claim = attr.ib() # type: str - localpart_template = attr.ib(type=Optional["Template"]) - display_name_template = attr.ib() # type: Optional[Template] - extra_attributes = attr.ib() # type: Dict[str, Template] + subject_claim = attr.ib(type=str) + localpart_template = attr.ib(type=Optional[Template]) + display_name_template = attr.ib(type=Optional[Template]) + extra_attributes = attr.ib(type=Dict[str, Template]) class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]): diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index 5f19554e528d..548b02211b4f 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -320,6 +320,28 @@ async def _redirect_to_username_picker( client_redirect_url: str, 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. + + Args: + auth_provider_id: A unique identifier for this SSO provider, e.g. + "oidc" or "saml". + + remote_user_id: The unique identifier from the SSO provider. + + attributes: the user attributes returned by the user mapping provider. + + client_redirect_url: The redirect URL passed in by the client, which we + will eventually redirect back to. + + extra_login_attributes: An optional dictionary of extra + attributes to be provided to the client in the login response. + + Raises: + RedirectException + """ session_id = random_string(16) now = self._clock.time_msec() session = UsernameMappingSession( @@ -351,6 +373,33 @@ async def _register_mapped_user( user_agent: str, ip_address: str, ) -> str: + """Register a new SSO user. + + This is called once we have successfully mapped the remote user id onto a local + user id, one way or another. + + Args: + attributes: user attributes returned by the user mapping provider, + including a non-empty localpart. + + auth_provider_id: A unique identifier for this SSO provider, e.g. + "oidc" or "saml". + + remote_user_id: The unique identifier from the SSO provider. + + user_agent: The user-agent in the HTTP request (used for potential + shadow-banning.) + + ip_address: The IP address of the requester (used for potential + shadow-banning.) + + Raises: + a MappingException if the localpart is invalid. + + a SynapseError with code 400 and errcode Codes.USER_IN_USE if the localpart + is already taken. + """ + # Since the localpart is provided via a potentially untrusted module, # ensure the MXID is valid before registering. if not attributes.localpart or contains_invalid_mxid_characters( @@ -451,6 +500,15 @@ async def check_username_availability( async def handle_submit_username_request( self, request: SynapseRequest, localpart: str, session_id: str ) -> None: + """Handle a request to the username-picker 'submit' endpoint + + Will serve an HTTP response to the request. + + Args: + request: HTTP request + 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: @@ -465,6 +523,8 @@ async def handle_submit_username_request( emails=session.emails, ) + # the following will raise a 400 error if the username has been taken in the + # meantime. user_id = await self._register_mapped_user( attributes, session.auth_provider_id, diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py index 2a3a86b9cd51..d3b6803e657a 100644 --- a/synapse/rest/synapse/client/pick_username.py +++ b/synapse/rest/synapse/client/pick_username.py @@ -37,8 +37,8 @@ def pick_username_resource(hs: "HomeServer") -> Resource: resource is just a File resource which serves up the static files in the resources "res" directory, but it has a couple of children: - * "submit", which does the mechanics of registering the new user, and redirects the - browser back to the client URL + * "submit", which does the mechanics of registering the new user, and redirects the + browser back to the client URL * "check": checks if a userid is free. """ diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py index 7890dcad13bf..368d600b33f2 100644 --- a/tests/handlers/test_oidc.py +++ b/tests/handlers/test_oidc.py @@ -896,8 +896,7 @@ def test_username_picker(self): # the expiry time should be about 15 minutes away expected_expiry = self.clock.time_msec() + (15 * 60 * 1000) - self.assertGreaterEqual(session.expiry_time_ms, expected_expiry - 1000) - self.assertLessEqual(session.expiry_time_ms, expected_expiry + 1000) + 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 @@ -917,11 +916,12 @@ def test_username_picker(self): ) self.assertEqual(chan.code, 302, chan.result) location_headers = chan.headers.getRawHeaders("Location") + # ensure that the returned location starts with the requested redirect URL self.assertEqual( location_headers[0][: len(client_redirect_url)], client_redirect_url ) - # fish the login token out of the redirect uri + # fish the login token out of the returned redirect uri parts = urlparse(location_headers[0]) query = parse_qs(parts.query) login_token = query["loginToken"][0] From e27b91086fb5fe1bb503b3129ce4006f3f4ed481 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 18:29:09 +0000 Subject: [PATCH 07/16] strip out mozilla-specific CSS --- synapse/res/username_picker/index.html | 5 +- synapse/res/username_picker/style.css | 180 ++----------------------- 2 files changed, 16 insertions(+), 169 deletions(-) diff --git a/synapse/res/username_picker/index.html b/synapse/res/username_picker/index.html index c6463a8175b4..11098df421b6 100644 --- a/synapse/res/username_picker/index.html +++ b/synapse/res/username_picker/index.html @@ -7,12 +7,11 @@
+ -
+
diff --git a/synapse/res/username_picker/style.css b/synapse/res/username_picker/style.css index 472ee919f73c..745bd4c684ef 100644 --- a/synapse/res/username_picker/style.css +++ b/synapse/res/username_picker/style.css @@ -1,179 +1,27 @@ -body { - background: #ededf0; - color: #737373; - font-family: "Open Sans", sans-serif; - letter-spacing: 0.03em; - margin: 0; - padding: 0; - display: grid; - grid-template-rows: auto 1fr; } - -.card { - background-color: #fff; - padding: 2em; - position: relative; - margin: auto; - width: 100%; - box-shadow: 0 0.25em 0.25em 0 rgba(210, 210, 210, 0.5); - border-radius: 0.125em; } - @media (min-width: 25em) { - .card { - max-width: 26em; - padding: 2.5em; } } - @supports (display: grid) { - .card { - grid-row-start: 2; } - @media (min-height: 50em) { - .card { - top: -3em; - /* compensate for negative margin for footer links */ } } } - .card__back { - margin-bottom: 1em; } - .card__heading { - font-size: 1.4em; - font-weight: 400; - text-transform: capitalize; - padding-left: 2.125em; - position: relative; - min-height: 1.5em; } - .card__heading--iconless { - padding-left: 0; } - .card__heading img { - width: 1.5em; - height: 1.5em; - position: absolute; - left: 0; - top: 0; } - .card__heading--success { - color: #12bc00; } - .card__heading--error { - color: #ff0039; } - .card [data-screen]:focus { - outline: none; } - -* { - box-sizing: border-box; } - -form { - margin: 0; } - form * { - font-family: inherit; } - -label { - margin: 2em 0; - display: block; } - -input[type="text"], -input[type="email"], -input[type="password"] { +input[type="text"] { font-size: 100%; background-color: #ededf0; border: 1px solid #fff; border-radius: .2em; padding: .5em .9em; display: block; - width: 100%; - margin-bottom: 1em; } - input[type="text"]:hover, input[type="text"]:focus, - input[type="email"]:hover, - input[type="email"]:focus, - input[type="password"]:hover, - input[type="password"]:focus { - border: 1px solid #0060df; - outline: none; } - .focus-styles input[type="text"]:focus, .focus-styles - input[type="email"]:focus, .focus-styles - input[type="password"]:focus { - border-color: transparent; } + width: 26em; +} -.form__input { - position: relative; } - p + .form__input { - margin-top: 2.5em; - /* leave space to fit a paragraph above a field */ } - .form__input label { - margin: 0; - position: absolute; - top: .5em; - left: .9em; } - .form__input input:focus + label, - .form__input input.has-contents + label { - position: absolute; - top: -1.5em; - color: #0060df; - font-weight: bold; } - .form__input input:focus + label > span, - .form__input input.has-contents + label > span { - font-size: 0.75em; } - -html, -body { - height: 100%; } - -.button { - text-align: center; - text-decoration: none; - padding: .93em 2em; - display: block; - font-size: 87.5%; - letter-spacing: .04em; - line-height: 1.57; - font-family: inherit; - border-radius: 2em; - background-color: #0060df; - color: #fff; - border: 1px solid transparent; - transition: background-color .1s ease-in-out; - -webkit-appearance: none; - -moz-appearance: none; - appearance: none; - cursor: pointer; } - .button:hover { - background-color: #fff; - color: #0060df; - border-color: currentColor; - text-decoration: none; } - .button:active { - background-color: #0060df; - color: #fff; - border-color: #0060df; } - .button--full-width { - width: 100%; } - .button--secondary { - border-color: #b1b1b3; - background-color: transparent; - color: #000; - text-transform: none; } - .button--secondary:hover { - background-color: #000; - color: #fff; - border-color: transparent; } - .button--secondary:hover svg > path { - fill: #fff; } - .button--secondary:active { - background-color: transparent; - border-color: #000; - color: #000; } - .button--disabled { - border-color: #fff; - background-color: transparent; - color: #000; - text-transform: none; } - .button--disabled:hover { - background-color: #fff; - color: #000; - border-color: transparent; } +.button--disabled { + border-color: #fff; + background-color: transparent; + color: #000; + text-transform: none; +} .hidden { - display: none; } + display: none; +} .tooltip { background-color: #f9f9fa; padding: 1em; - margin: 1em 0; } - .tooltip p:last-child { - margin-bottom: 0; } - .tooltip a:last-child { - margin-left: .5em; } - .tooltip:target { - display: block; } + margin: 1em 0; +} + From 2603c1d79dd3d29fa83ee9f658af3e8e3345767a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 20:47:23 +0000 Subject: [PATCH 08/16] Switch to named functions --- synapse/res/username_picker/script.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 669def8474fb..34ab7bfebec4 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -4,7 +4,7 @@ let submitButton = document.getElementById("button-submit"); let message = document.getElementById("message"); // Remove input field placeholder if the text field is not empty -let switchClass = function(input) { +function switchClass(input) { if (input.value.length > 0) { input.classList.add('has-contents'); } @@ -14,14 +14,14 @@ let switchClass = function(input) { }; // Submit username and receive response -let showMessage = function(messageText) { +function showMessage(messageText) { // Unhide the message text message.classList.remove("hidden"); message.innerHTML = messageText; }; -let onResponse = function(response, success) { +function onResponse(response, success) { // Display message showMessage(response); @@ -36,7 +36,7 @@ let onResponse = function(response, success) { }; let allowedUsernameCharacters = RegExp("[^a-z0-9\\.\\_\\=\\-\\/]"); -let usernameIsValid = function(username) { +function usernameIsValid(username) { return !allowedUsernameCharacters.test(username); } let allowedCharactersString = "" + @@ -48,13 +48,13 @@ let allowedCharactersString = "" + "/, " + "="; -let buildQueryString = function(params) { +function buildQueryString(params) { return Object.keys(params) .map(k => encodeURIComponent(k) + '=' + encodeURIComponent(params[k])) .join('&'); } -let submitUsername = function(username) { +function submitUsername(username) { if(username.length == 0) { onResponse("Please enter a username.", false); return; @@ -87,7 +87,7 @@ let submitUsername = function(username) { }); } -let clickSubmit = function() { +function clickSubmit() { if(submitButton.classList.contains('button--disabled')) { return; } // Disable submit button and input field @@ -113,4 +113,3 @@ inputField.addEventListener('keypress', function(event) { inputField.addEventListener('change', function() { switchClass(inputField); }); - From 955cdb457354e5ea7db77881dc6403d5376b4be1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 20:48:55 +0000 Subject: [PATCH 09/16] fix indentation four legs good, two legs^Wspaces bad --- synapse/res/username_picker/script.js | 82 +++++++++++++-------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 34ab7bfebec4..a81ddd300b71 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -5,39 +5,39 @@ let message = document.getElementById("message"); // Remove input field placeholder if the text field is not empty function switchClass(input) { - if (input.value.length > 0) { - input.classList.add('has-contents'); - } - else { - input.classList.remove('has-contents'); - } + if (input.value.length > 0) { + input.classList.add('has-contents'); + } + else { + input.classList.remove('has-contents'); + } }; // Submit username and receive response function showMessage(messageText) { - // Unhide the message text - message.classList.remove("hidden"); + // Unhide the message text + message.classList.remove("hidden"); - message.innerHTML = messageText; + message.innerHTML = messageText; }; function onResponse(response, success) { - // Display message - showMessage(response); + // Display message + showMessage(response); - if(success) { - inputForm.submit(); - return; - } + if(success) { + inputForm.submit(); + return; + } - // Enable submit button and input field - submitButton.classList.remove('button--disabled'); - submitButton.value = "Submit" + // Enable submit button and input field + submitButton.classList.remove('button--disabled'); + submitButton.value = "Submit"; }; let allowedUsernameCharacters = RegExp("[^a-z0-9\\.\\_\\=\\-\\/]"); function usernameIsValid(username) { - return !allowedUsernameCharacters.test(username); + return !allowedUsernameCharacters.test(username); } let allowedCharactersString = "" + "lowercase letters, " + @@ -55,14 +55,14 @@ function buildQueryString(params) { } function submitUsername(username) { - if(username.length == 0) { - onResponse("Please enter a username.", false); - return; - } - if(!usernameIsValid(username)) { - onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString, false); - return; - } + if(username.length == 0) { + onResponse("Please enter a username.", false); + return; + } + if(!usernameIsValid(username)) { + onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString, false); + return; + } let check_uri = 'check?' + buildQueryString({"username": username}); fetch(check_uri, { @@ -88,28 +88,28 @@ function submitUsername(username) { } function clickSubmit() { - if(submitButton.classList.contains('button--disabled')) { return; } + if(submitButton.classList.contains('button--disabled')) { return; } - // Disable submit button and input field - submitButton.classList.add('button--disabled'); + // Disable submit button and input field + submitButton.classList.add('button--disabled'); - // Submit username - submitButton.value = "Checking..."; - submitUsername(inputField.value); + // Submit username + submitButton.value = "Checking..."; + submitUsername(inputField.value); }; submitButton.onclick = clickSubmit; // Listen for events on inputField inputField.addEventListener('keypress', function(event) { - // Listen for Enter on input field - if(event.which === 13) { - event.preventDefault(); - clickSubmit(); - return true; - } - switchClass(inputField); + // Listen for Enter on input field + if(event.which === 13) { + event.preventDefault(); + clickSubmit(); + return true; + } + switchClass(inputField); }); inputField.addEventListener('change', function() { - switchClass(inputField); + switchClass(inputField); }); From a7b1428fb4c1d722e2c4eb8770cb6dab281f83f7 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 20:49:57 +0000 Subject: [PATCH 10/16] fix some missing semicolons --- synapse/res/username_picker/script.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index a81ddd300b71..97cfe87196d7 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -70,9 +70,9 @@ function submitUsername(username) { }).then((response) => { if(!response.ok) { // for non-200 responses, raise the body of the response as an exception - return response.text().then((text) => { throw text }); + return response.text().then((text) => { throw text; }); } else { - return response.json() + return response.json(); } }).then((json) => { if(json.error) { From 50a893362f7627108e19197dec117134b55bc7c9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 21:08:29 +0000 Subject: [PATCH 11/16] Remove unused "has-contents" CSS class --- synapse/res/username_picker/script.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 97cfe87196d7..9b7e6b90489b 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -3,16 +3,6 @@ let inputForm = document.getElementById("form"); let submitButton = document.getElementById("button-submit"); let message = document.getElementById("message"); -// Remove input field placeholder if the text field is not empty -function switchClass(input) { - if (input.value.length > 0) { - input.classList.add('has-contents'); - } - else { - input.classList.remove('has-contents'); - } -}; - // Submit username and receive response function showMessage(messageText) { // Unhide the message text @@ -108,8 +98,4 @@ inputField.addEventListener('keypress', function(event) { clickSubmit(); return true; } - switchClass(inputField); -}); -inputField.addEventListener('change', function() { - switchClass(inputField); }); From 3b357674efabd0754741e462c87c77b7fad75d1c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 21:14:08 +0000 Subject: [PATCH 12/16] Use an `onsubmit` on the form rather than keypress listener, etc --- synapse/res/username_picker/index.html | 2 +- synapse/res/username_picker/script.js | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/synapse/res/username_picker/index.html b/synapse/res/username_picker/index.html index 11098df421b6..37ea8bb6d847 100644 --- a/synapse/res/username_picker/index.html +++ b/synapse/res/username_picker/index.html @@ -9,7 +9,7 @@
- +
diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 9b7e6b90489b..bc961519a8b1 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -16,6 +16,8 @@ function onResponse(response, success) { showMessage(response); if(success) { + // remove the event handler before re-submitting the form. + delete inputForm.onsubmit; inputForm.submit(); return; } @@ -78,6 +80,7 @@ function submitUsername(username) { } function clickSubmit() { + event.preventDefault(); if(submitButton.classList.contains('button--disabled')) { return; } // Disable submit button and input field @@ -88,14 +91,4 @@ function clickSubmit() { submitUsername(inputField.value); }; -submitButton.onclick = clickSubmit; - -// Listen for events on inputField -inputField.addEventListener('keypress', function(event) { - // Listen for Enter on input field - if(event.which === 13) { - event.preventDefault(); - clickSubmit(); - return true; - } -}); +inputForm.onsubmit = clickSubmit; From 1b172dabfe99997097ca8244eb5e642e8605bab0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 21:32:41 +0000 Subject: [PATCH 13/16] fallback for non-`fetch` browsers --- synapse/res/username_picker/script.js | 33 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index bc961519a8b1..463ae2e34f3c 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -11,17 +11,18 @@ function showMessage(messageText) { message.innerHTML = messageText; }; -function onResponse(response, success) { +function doSubmit() { + showMessage("Success. Please wait a moment for your browser to redirect."); + + // remove the event handler before re-submitting the form. + delete inputForm.onsubmit; + inputForm.submit(); +} + +function onResponse(response) { // Display message showMessage(response); - if(success) { - // remove the event handler before re-submitting the form. - delete inputForm.onsubmit; - inputForm.submit(); - return; - } - // Enable submit button and input field submitButton.classList.remove('button--disabled'); submitButton.value = "Submit"; @@ -48,11 +49,17 @@ function buildQueryString(params) { function submitUsername(username) { if(username.length == 0) { - onResponse("Please enter a username.", false); + onResponse("Please enter a username."); return; } if(!usernameIsValid(username)) { - onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString, false); + onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString); + return; + } + + // if this browser doesn't support fetch, skip the availability check. + if(!window.fetch) { + doSubmit(); return; } @@ -70,12 +77,12 @@ function submitUsername(username) { if(json.error) { throw json.error; } else if(json.available) { - onResponse("Success. Please wait a moment for your browser to redirect.", true); + doSubmit(); } else { - onResponse("This username is not available, please choose another.", false); + onResponse("This username is not available, please choose another."); } }).catch((err) => { - onResponse("Error checking username availability: " + err, false); + onResponse("Error checking username availability: " + err); }); } From bc67f1572188677d3c49a35ca21a79a2cc2e3de8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 21:45:31 +0000 Subject: [PATCH 14/16] fix up 'credentials' param --- synapse/res/username_picker/script.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 463ae2e34f3c..9eb899997fd4 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -65,7 +65,8 @@ function submitUsername(username) { let check_uri = 'check?' + buildQueryString({"username": username}); fetch(check_uri, { - "credentials": "include", + // include the cookie + "credentials": "same-origin", }).then((response) => { if(!response.ok) { // for non-200 responses, raise the body of the response as an exception From 20a10bad06d4a7bd0dc0c617298b1229c66741cd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 17 Dec 2020 21:45:42 +0000 Subject: [PATCH 15/16] avoid html injection --- synapse/res/username_picker/script.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 9eb899997fd4..5212e01ea702 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -8,7 +8,7 @@ function showMessage(messageText) { // Unhide the message text message.classList.remove("hidden"); - message.innerHTML = messageText; + message.textContent = messageText; }; function doSubmit() { From c659e610b5ffc592ace0588884829a158c5a6d77 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 18 Dec 2020 13:30:41 +0000 Subject: [PATCH 16/16] Remove HTML from response message --- synapse/res/username_picker/script.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js index 5212e01ea702..416a7c6f41dd 100644 --- a/synapse/res/username_picker/script.js +++ b/synapse/res/username_picker/script.js @@ -32,14 +32,7 @@ let allowedUsernameCharacters = RegExp("[^a-z0-9\\.\\_\\=\\-\\/]"); function usernameIsValid(username) { return !allowedUsernameCharacters.test(username); } -let allowedCharactersString = "" + -"lowercase letters, " + -"digits, " + -"., " + -"_, " + -"-, " + -"/, " + -"="; +let allowedCharactersString = "lowercase letters, digits, ., _, -, /, ="; function buildQueryString(params) { return Object.keys(params)