From 5856f815471625170d67df83b1f02b913e8bf7e5 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 3 Jun 2021 21:59:12 +0100 Subject: [PATCH 01/52] Hard-coded token authenticated registration Signed-off-by: Callum Brown --- docs/sample_config.yaml | 4 ++ synapse/api/constants.py | 1 + synapse/config/registration.py | 7 +++ synapse/handlers/ui_auth/checkers.py | 24 ++++++++ synapse/rest/client/v2_alpha/register.py | 5 ++ tests/rest/client/v2_alpha/test_register.py | 64 +++++++++++++++++++++ 6 files changed, 105 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 7b97f73a296b..db9ced0ec528 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1202,6 +1202,10 @@ url_preview_accept_language: # #enable_3pid_lookup: true +# Require users to submit a token during registration +# +#registrations_require_token: false + # If set, allows registration of standard or admin accounts by anyone who # has the shared secret, even if registration is otherwise disabled. # diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 3940da5c8880..71823b121e6c 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -73,6 +73,7 @@ class LoginType: TERMS = "m.login.terms" SSO = "m.login.sso" DUMMY = "m.login.dummy" + REGISTRATION_TOKEN = "org.matrix.msc3231.login.registration_token" # This is used in the `type` parameter for /register when called by diff --git a/synapse/config/registration.py b/synapse/config/registration.py index d9dc55a0c3c5..19816377eb2d 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -33,6 +33,9 @@ def read_config(self, config, **kwargs): self.registrations_require_3pid = config.get("registrations_require_3pid", []) self.allowed_local_3pids = config.get("allowed_local_3pids", []) self.enable_3pid_lookup = config.get("enable_3pid_lookup", True) + self.registrations_require_token = config.get( + "registrations_require_token", False + ) self.registration_shared_secret = config.get("registration_shared_secret") self.bcrypt_rounds = config.get("bcrypt_rounds", 12) @@ -178,6 +181,10 @@ def generate_config_section(self, generate_secrets=False, **kwargs): # #enable_3pid_lookup: true + # Require users to submit a token during registration + # + #registrations_require_token: false + # If set, allows registration of standard or admin accounts by anyone who # has the shared secret, even if registration is otherwise disabled. # diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 5414ce77d83c..05c1942e80c9 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -237,11 +237,35 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: return await self._check_threepid("msisdn", authdict) +class RegistrationTokenAuthChecker(UserInteractiveAuthChecker): + AUTH_TYPE = LoginType.REGISTRATION_TOKEN + + def __init__(self, hs: "HomeServer"): + super().__init__(hs) + self._enabled = bool(hs.config.registrations_require_token) + + def is_enabled(self) -> bool: + return self._enabled + + async def check_auth(self, authdict: dict, clientip: str) -> Any: + if "token" not in authdict: + raise LoginError(400, "Missing token", Codes.MISSING_PARAM) + + valid_tokens = ["abcd"] + if authdict["token"] in valid_tokens: + return True + else: + raise LoginError( + 401, "Invalid registration token", errcode=Codes.UNAUTHORIZED + ) + + INTERACTIVE_AUTH_CHECKERS = [ DummyAuthChecker, TermsAuthChecker, RecaptchaAuthChecker, EmailIdentityAuthChecker, MsisdnAuthChecker, + RegistrationTokenAuthChecker, ] """A list of UserInteractiveAuthChecker classes""" diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index a30a5df1b195..ee1ee422b306 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -816,6 +816,11 @@ def _calculate_registration_flows( for flow in flows: flow.insert(0, LoginType.RECAPTCHA) + # Prepend registration token to all flows if we're requiring a token + if config.registrations_require_token: + for flow in flows: + flow.insert(0, LoginType.REGISTRATION_TOKEN) + return flows diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 1cad5f00eb20..3fa767254a2e 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -205,6 +205,70 @@ def test_POST_ratelimiting(self): self.assertEquals(channel.result["code"], b"200", channel.result) + @unittest.override_config({"registrations_require_token": True}) + def test_POST_registrations_require_token(self): + username = "kermit" + device_id = "frogfone" + token = "abcd" + params = { + "username": username, + "password": "monkey", + "device_id": device_id, + } + + # Request without params to get flows and session + channel = self.make_request(b"POST", self.url, b"{}") + self.assertEquals(channel.result["code"], b"401", channel.result) + flows = channel.json_body["flows"] + # Synapse adds a dummy stage to differentiate flows where otherwise one + # flow would be a subset of another flow. + self.assertCountEqual( + [[LoginType.REGISTRATION_TOKEN, LoginType.DUMMY]], + (f["stages"] for f in flows), + ) + session = channel.json_body["session"] + + # Do the registration token stage and check it has completed + params["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session, + } + request_data = json.dumps(params) + channel = self.make_request(b"POST", self.url, request_data) + self.assertEquals(channel.result["code"], b"401", channel.result) + completed = channel.json_body["completed"] + self.assertCountEqual([LoginType.REGISTRATION_TOKEN], completed) + + # Do the m.login.dummy stage and check registration was successful + params["auth"] = { + "type": LoginType.DUMMY, + "session": session, + } + request_data = json.dumps(params) + channel = self.make_request(b"POST", self.url, request_data) + det_data = { + "user_id": "@{}:{}".format(username, self.hs.hostname), + "home_server": self.hs.hostname, + "device_id": device_id, + } + self.assertEquals(channel.result["code"], b"200", channel.result) + self.assertDictContainsSubset(det_data, channel.json_body) + + @unittest.override_config({"registrations_require_token": True}) + def test_POST_registration_token_invalid(self): + token = "1234" + params = { + "auth": { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + } + } + request_data = json.dumps(params) + channel = self.make_request(b"POST", self.url, request_data) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + def test_advertised_flows(self): channel = self.make_request(b"POST", self.url, b"{}") self.assertEquals(channel.result["code"], b"401", channel.result) From 5f21580da154307ee93672c21db929cd189a4676 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 10 Jun 2021 13:26:26 +0100 Subject: [PATCH 02/52] Create registration_tokens table Signed-off-by: Callum Brown --- .../delta/58/29create_registration_tokens.sql | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 synapse/storage/schema/main/delta/58/29create_registration_tokens.sql diff --git a/synapse/storage/schema/main/delta/58/29create_registration_tokens.sql b/synapse/storage/schema/main/delta/58/29create_registration_tokens.sql new file mode 100644 index 000000000000..d11844b53025 --- /dev/null +++ b/synapse/storage/schema/main/delta/58/29create_registration_tokens.sql @@ -0,0 +1,23 @@ +/* Copyright 2021 Callum Brown + * + * 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. + */ + +CREATE TABLE IF NOT EXISTS registration_tokens( + token TEXT NOT NULL, -- The token that can be used for authentication. + total_uses INT, -- The total number of times this token can be used. NULL if no limit. + pending INT, -- The number of in progress registrations using this token. NULL if total_uses is NULL. + completed INT NOT NULL, -- The number of times this token has been used to complete a registration. + expiry_time BIGINT, -- The latest time this token will be valid (epoch time in milliseconds). NULL if token doesn't expire. + UNIQUE (token) +); From 2b8726cf18a0a8510c68da902edd760f2651862c Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 11 Jun 2021 11:43:43 +0100 Subject: [PATCH 03/52] Check in database to validate registration token Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 6 ++-- .../storage/databases/main/registration.py | 29 +++++++++++++++++++ tests/rest/client/v2_alpha/test_register.py | 12 ++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 05c1942e80c9..b83eaf6959ac 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -243,6 +243,7 @@ class RegistrationTokenAuthChecker(UserInteractiveAuthChecker): def __init__(self, hs: "HomeServer"): super().__init__(hs) self._enabled = bool(hs.config.registrations_require_token) + self.store = hs.get_datastore() def is_enabled(self) -> bool: return self._enabled @@ -250,9 +251,10 @@ def is_enabled(self) -> bool: async def check_auth(self, authdict: dict, clientip: str) -> Any: if "token" not in authdict: raise LoginError(400, "Missing token", Codes.MISSING_PARAM) + if not isinstance(authdict["token"], str): + raise LoginError(400, "Token must be a string", Codes.INVALID_PARAM) - valid_tokens = ["abcd"] - if authdict["token"] in valid_tokens: + if await self.store.registration_token_is_valid(authdict["token"]): return True else: raise LoginError( diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index e5c5cf8ff065..7c3e68e907da 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1760,6 +1760,35 @@ def start_or_continue_validation_session_txn(txn): start_or_continue_validation_session_txn, ) + async def registration_token_is_valid(self, token: str) -> bool: + """Checks if a token can be used to authenticate a registration. + + Args: + token: The registration token to be checked + """ + res = await self.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["total_uses", "pending", "completed", "expiry_time"], + allow_none=True, + ) + + # Check if the token exists + if res is None: + return False + + # Check if the token has expired + now = self._clock.time_msec() + if res["expiry_time"] and res["expiry_time"] < now: + return False + + # Check if the token has been used up + if res["total_uses"] and res["pending"] + res["completed"] >= res["total_uses"]: + return False + + # Otherwise, the token is valid + return True + def find_max_generated_user_id_localpart(cur: Cursor) -> int: """ diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 3fa767254a2e..d0f76250f032 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -210,6 +210,18 @@ def test_POST_registrations_require_token(self): username = "kermit" device_id = "frogfone" token = "abcd" + self.get_success( + self.hs.get_datastore().db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "total_uses": None, + "pending": None, + "completed": 0, + "expiry_time": None, + }, + ) + ) params = { "username": username, "password": "monkey", From 5b1ec0bf28659241c5acfd94fbc75f078e8c0a6b Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Mon, 14 Jun 2021 09:58:11 +0100 Subject: [PATCH 04/52] Increment `completed` when registration token used Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 7 +++++++ synapse/rest/client/v2_alpha/register.py | 9 +++++++++ .../storage/databases/main/registration.py | 19 +++++++++++++++++++ tests/rest/client/v2_alpha/test_register.py | 13 ++++++++++++- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index b83eaf6959ac..dd9424fbb28d 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -255,6 +255,13 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: raise LoginError(400, "Token must be a string", Codes.INVALID_PARAM) if await self.store.registration_token_is_valid(authdict["token"]): + # Store the token in the UIA session, so that once registration + # is complete `completed` can be incremented. + await self.store.set_ui_auth_session_data( + authdict["session"], + "registration_token", + authdict["token"], + ) return True else: raise LoginError( diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index ee1ee422b306..fda79c4e4e0a 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -669,6 +669,15 @@ async def on_POST(self, request): ) if registered: + # Check if a token was used to authenticate registration + registration_token = await self.auth_handler.get_session_data( + session_id, + "registration_token", + ) + if registration_token: + # Increment the `completed` counter for the token + await self.store.use_registration_token(registration_token) + await self.registration_handler.post_registration_actions( user_id=registered_user_id, auth_result=auth_result, diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 7c3e68e907da..3e0b706448c6 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1789,6 +1789,25 @@ async def registration_token_is_valid(self, token: str) -> bool: # Otherwise, the token is valid return True + async def use_registration_token(self, token: str) -> None: + """Increment the completed registrations counter for a token. + + Args: + token: The registration token to be 'used' + """ + res = await self.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["pending", "completed", "expiry_time"], + desc="use_registration_token", + ) + await self.db_pool.simple_update_one( + "registration_tokens", + keyvalues={"token": token}, + updatevalues={"completed": res["completed"] + 1}, + desc="use_registration_token", + ) + def find_max_generated_user_id_localpart(cur: Cursor) -> int: """ diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index d0f76250f032..4779f5dbbadf 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -210,8 +210,9 @@ def test_POST_registrations_require_token(self): username = "kermit" device_id = "frogfone" token = "abcd" + store = self.hs.get_datastore() self.get_success( - self.hs.get_datastore().db_pool.simple_insert( + store.db_pool.simple_insert( "registration_tokens", { "token": token, @@ -267,6 +268,16 @@ def test_POST_registrations_require_token(self): self.assertEquals(channel.result["code"], b"200", channel.result) self.assertDictContainsSubset(det_data, channel.json_body) + # Check the `completed` counter has been incremented. + completed = self.get_success( + store.db_pool.simple_select_one_onecol( + "registration_tokens", + keyvalues={"token": token}, + retcol="completed", + ) + ) + self.assertEquals(completed, 1) + @unittest.override_config({"registrations_require_token": True}) def test_POST_registration_token_invalid(self): token = "1234" From 15e576923ef8b130ec667aeebaf24dafae55e4d2 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Mon, 14 Jun 2021 10:24:48 +0100 Subject: [PATCH 05/52] Rename total_uses to uses_allowed Signed-off-by: Callum Brown --- synapse/storage/databases/main/registration.py | 4 ++-- .../999create_registration_tokens.sql} | 2 +- tests/rest/client/v2_alpha/test_register.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename synapse/storage/schema/main/delta/{58/29create_registration_tokens.sql => 59/999create_registration_tokens.sql} (91%) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 3e0b706448c6..01288f4167c1 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1769,7 +1769,7 @@ async def registration_token_is_valid(self, token: str) -> bool: res = await self.db_pool.simple_select_one( "registration_tokens", keyvalues={"token": token}, - retcols=["total_uses", "pending", "completed", "expiry_time"], + retcols=["uses_allowed", "pending", "completed", "expiry_time"], allow_none=True, ) @@ -1783,7 +1783,7 @@ async def registration_token_is_valid(self, token: str) -> bool: return False # Check if the token has been used up - if res["total_uses"] and res["pending"] + res["completed"] >= res["total_uses"]: + if res["uses_allowed"] and res["pending"] + res["completed"] >= res["uses_allowed"]: return False # Otherwise, the token is valid diff --git a/synapse/storage/schema/main/delta/58/29create_registration_tokens.sql b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql similarity index 91% rename from synapse/storage/schema/main/delta/58/29create_registration_tokens.sql rename to synapse/storage/schema/main/delta/59/999create_registration_tokens.sql index d11844b53025..b903c1acb118 100644 --- a/synapse/storage/schema/main/delta/58/29create_registration_tokens.sql +++ b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql @@ -15,7 +15,7 @@ CREATE TABLE IF NOT EXISTS registration_tokens( token TEXT NOT NULL, -- The token that can be used for authentication. - total_uses INT, -- The total number of times this token can be used. NULL if no limit. + uses_allowed INT, -- The total number of times this token can be used. NULL if no limit. pending INT, -- The number of in progress registrations using this token. NULL if total_uses is NULL. completed INT NOT NULL, -- The number of times this token has been used to complete a registration. expiry_time BIGINT, -- The latest time this token will be valid (epoch time in milliseconds). NULL if token doesn't expire. diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 4779f5dbbadf..13913029e002 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -216,7 +216,7 @@ def test_POST_registrations_require_token(self): "registration_tokens", { "token": token, - "total_uses": None, + "uses_allowed": None, "pending": None, "completed": 0, "expiry_time": None, From 9c502b0762297de9c3335d87eef61addd428b6f2 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Mon, 14 Jun 2021 16:56:54 +0100 Subject: [PATCH 06/52] Improve unit tests Signed-off-by: Callum Brown --- tests/rest/client/v2_alpha/test_register.py | 35 +++++++++++++++------ 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 13913029e002..df7ff51f6b14 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -229,8 +229,8 @@ def test_POST_registrations_require_token(self): "device_id": device_id, } - # Request without params to get flows and session - channel = self.make_request(b"POST", self.url, b"{}") + # Request without auth to get flows and session + channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) flows = channel.json_body["flows"] # Synapse adds a dummy stage to differentiate flows where otherwise one @@ -280,15 +280,32 @@ def test_POST_registrations_require_token(self): @unittest.override_config({"registrations_require_token": True}) def test_POST_registration_token_invalid(self): - token = "1234" params = { - "auth": { - "type": LoginType.REGISTRATION_TOKEN, - "token": token, - } + "username": "kermit", + "password": "monkey", } - request_data = json.dumps(params) - channel = self.make_request(b"POST", self.url, request_data) + # Request without auth to get session + channel = self.make_request(b"POST", self.url, json.dumps(params)) + session = channel.json_body["session"] + + # Test with token param missing (invalid) + params["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "session": session, + } + channel = self.make_request(b"POST", self.url, json.dumps(params)) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.MISSING_PARAM) + + # Test with non-string (invalid) + params["auth"]["token"] = 1234 + channel = self.make_request(b"POST", self.url, json.dumps(params)) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Test with unknown token (invalid) + params["auth"]["token"] = "1234" + channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) From e7754a9e075e4e6f7e31e13cdb5f592eea51d438 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Mon, 14 Jun 2021 17:52:56 +0100 Subject: [PATCH 07/52] Increment pending while registration in progress Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 29 ++++++- .../storage/databases/main/registration.py | 26 +++++- .../59/999create_registration_tokens.sql | 2 +- tests/rest/client/v2_alpha/test_register.py | 85 +++++++++++++++++-- 4 files changed, 129 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index dd9424fbb28d..0f21f1585b26 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -18,7 +18,7 @@ from twisted.web.client import PartialDownloadError from synapse.api.constants import LoginType -from synapse.api.errors import Codes, LoginError, SynapseError +from synapse.api.errors import Codes, LoginError, StoreError, SynapseError from synapse.config.emailconfig import ThreepidBehaviour from synapse.util import json_decoder @@ -253,14 +253,35 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: raise LoginError(400, "Missing token", Codes.MISSING_PARAM) if not isinstance(authdict["token"], str): raise LoginError(400, "Token must be a string", Codes.INVALID_PARAM) + if "session" not in authdict: + raise SynapseError(400, "Missing UIA session", Codes.MISSING_PARAM) + session = authdict["session"] - if await self.store.registration_token_is_valid(authdict["token"]): + # If the LoginType.REGISTRATION_TOKEN stage has already been completed, + # return early to avoid incrementing `pending` again. + # TODO: check stored token matches provided token? + try: + stored_token = await self.store.get_ui_auth_session_data( + session, + "registration_token", + None, + ) + except StoreError: + raise SynapseError(400, "Unknown session ID: {}".format(session)) + if stored_token: + return True + + token = authdict["token"] + if await self.store.registration_token_is_valid(token): + # Increment pending counter, so that if token has limited uses it + # can't be used up by someone else in the meantime. + await self.store.set_registration_token_pending(token) # Store the token in the UIA session, so that once registration # is complete `completed` can be incremented. await self.store.set_ui_auth_session_data( - authdict["session"], + session, "registration_token", - authdict["token"], + token, ) return True else: diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 01288f4167c1..37cc568ebf42 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1789,6 +1789,25 @@ async def registration_token_is_valid(self, token: str) -> bool: # Otherwise, the token is valid return True + async def set_registration_token_pending(self, token: str) -> None: + """Increment the pending registrations counter for a token. + + Args: + token: The registration token pending use + """ + pending = await self.db_pool.simple_select_one_onecol( + "registration_tokens", + keyvalues={"token": token}, + retcol="pending", + desc="set_registration_token_pending", + ) + await self.db_pool.simple_update_one( + "registration_tokens", + keyvalues={"token": token}, + updatevalues={"pending": pending + 1}, + desc="set_registration_token_pending", + ) + async def use_registration_token(self, token: str) -> None: """Increment the completed registrations counter for a token. @@ -1798,13 +1817,16 @@ async def use_registration_token(self, token: str) -> None: res = await self.db_pool.simple_select_one( "registration_tokens", keyvalues={"token": token}, - retcols=["pending", "completed", "expiry_time"], + retcols=["pending", "completed"], desc="use_registration_token", ) await self.db_pool.simple_update_one( "registration_tokens", keyvalues={"token": token}, - updatevalues={"completed": res["completed"] + 1}, + updatevalues={ + "completed": res["completed"] + 1, + "pending": res["pending"] - 1, + }, desc="use_registration_token", ) diff --git a/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql index b903c1acb118..a9f34c7d6702 100644 --- a/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql +++ b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql @@ -16,7 +16,7 @@ CREATE TABLE IF NOT EXISTS registration_tokens( token TEXT NOT NULL, -- The token that can be used for authentication. uses_allowed INT, -- The total number of times this token can be used. NULL if no limit. - pending INT, -- The number of in progress registrations using this token. NULL if total_uses is NULL. + pending INT NOT NULL, -- The number of in progress registrations using this token. completed INT NOT NULL, -- The number of times this token has been used to complete a registration. expiry_time BIGINT, -- The latest time this token will be valid (epoch time in milliseconds). NULL if token doesn't expire. UNIQUE (token) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index df7ff51f6b14..5b3ce1797a12 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -217,7 +217,7 @@ def test_POST_registrations_require_token(self): { "token": token, "uses_allowed": None, - "pending": None, + "pending": 0, "completed": 0, "expiry_time": None, }, @@ -268,15 +268,16 @@ def test_POST_registrations_require_token(self): self.assertEquals(channel.result["code"], b"200", channel.result) self.assertDictContainsSubset(det_data, channel.json_body) - # Check the `completed` counter has been incremented. - completed = self.get_success( - store.db_pool.simple_select_one_onecol( + # Check the `completed` counter has been incremented and pending is 0 + res = self.get_success( + store.db_pool.simple_select_one( "registration_tokens", keyvalues={"token": token}, - retcol="completed", + retcols=["pending", "completed"], ) ) - self.assertEquals(completed, 1) + self.assertEquals(res["completed"], 1) + self.assertEquals(res["pending"], 0) @unittest.override_config({"registrations_require_token": True}) def test_POST_registration_token_invalid(self): @@ -309,6 +310,78 @@ def test_POST_registration_token_invalid(self): self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + @unittest.override_config({"registrations_require_token": True}) + def test_POST_registration_token_limit_uses(self): + token = "abcd" + store = self.hs.get_datastore() + # Create token that can be used once + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": 1, + "pending": 0, + "completed": 0, + "expiry_time": None, + }, + ) + ) + params1 = {"username": "bert", "password": "monkey"} + params2 = {"username": "ernie", "password": "monkey"} + # Do 2 requests without auth to get two session IDs + channel1 = self.make_request(b"POST", self.url, json.dumps(params1)) + session1 = channel1.json_body["session"] + channel2 = self.make_request(b"POST", self.url, json.dumps(params2)) + session2 = channel2.json_body["session"] + + # Use token with session1 and check `pending` is 1 + params1["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session1, + } + self.make_request(b"POST", self.url, json.dumps(params1)) + # Repeat request to make sure pending isn't set increased again + self.make_request(b"POST", self.url, json.dumps(params1)) + pending = self.get_success( + store.db_pool.simple_select_one_onecol( + "registration_tokens", + keyvalues={"token": token}, + retcol="pending", + ) + ) + self.assertEquals(pending, 1) + + # Check auth fails when using token with session2 + params2["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session2, + } + channel = self.make_request(b"POST", self.url, json.dumps(params2)) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + + # Complete registration with session1 + params1["auth"]["type"] = LoginType.DUMMY + self.make_request(b"POST", self.url, json.dumps(params1)) + # Check pending=0 and completed=1 + res = self.get_success( + store.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["pending", "completed"], + ) + ) + self.assertEquals(res["pending"], 0) + self.assertEquals(res["completed"], 1) + + # Check auth still fails when using token with session2 + channel = self.make_request(b"POST", self.url, json.dumps(params2)) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + def test_advertised_flows(self): channel = self.make_request(b"POST", self.url, b"{}") self.assertEquals(channel.result["code"], b"401", channel.result) From ef05a6d11580df54cccdf392c1424547fed3ad2a Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Wed, 16 Jun 2021 10:24:38 +0100 Subject: [PATCH 08/52] Add unit test for registration token expiry Signed-off-by: Callum Brown --- tests/rest/client/v2_alpha/test_register.py | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 5b3ce1797a12..356b74131784 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -382,6 +382,53 @@ def test_POST_registration_token_limit_uses(self): self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + @unittest.override_config({"registrations_require_token": True}) + def test_registration_token_expiry(self): + token = "abcd" + now = self.hs.get_clock().time_msec() + store = self.hs.get_datastore() + # Create token that expired yesterday + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": None, + "pending": 0, + "completed": 0, + "expiry_time": now - 24 * 60 * 60 * 1000, + }, + ) + ) + params = {"username": "kermit", "password": "monkey"} + # Request without auth to get session + channel = self.make_request(b"POST", self.url, json.dumps(params)) + session = channel.json_body["session"] + + # Check authentication fails with expired token + params["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session, + } + channel = self.make_request(b"POST", self.url, json.dumps(params)) + self.assertEquals(channel.result["code"], b"401", channel.result) + self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + + # Update token so it expires tomorrow + self.get_success( + store.db_pool.simple_update_one( + "registration_tokens", + keyvalues={"token": token}, + updatevalues={"expiry_time": now + 24 * 60 * 60 * 1000}, + ) + ) + + # Check authentication succeeds + channel = self.make_request(b"POST", self.url, json.dumps(params)) + completed = channel.json_body["completed"] + self.assertCountEqual([LoginType.REGISTRATION_TOKEN], completed) + def test_advertised_flows(self): channel = self.make_request(b"POST", self.url, b"{}") self.assertEquals(channel.result["code"], b"401", channel.result) From 53f0e050bf8915989f046378e55fe27d353aebe9 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Wed, 16 Jun 2021 19:57:21 +0100 Subject: [PATCH 09/52] Fix config file related bits Signed-off-by: Callum Brown --- docs/sample_config.yaml | 8 ++++++-- synapse/config/registration.py | 12 ++++++++---- synapse/handlers/ui_auth/checkers.py | 2 +- synapse/rest/client/v2_alpha/register.py | 2 +- tests/rest/client/v2_alpha/test_register.py | 12 ++++++------ 5 files changed, 22 insertions(+), 14 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index db9ced0ec528..82b55f28214f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1202,9 +1202,13 @@ url_preview_accept_language: # #enable_3pid_lookup: true -# Require users to submit a token during registration +# Require users to submit a token during registration. +# Tokens can be generated with the admin API (once it exists). +# Note that `enable_registration` must be set to `true`. +# Disabling this option will not delete any tokens previously generated. +# Defaults to false. Uncomment the following to require tokens: # -#registrations_require_token: false +#registration_requires_token: true # If set, allows registration of standard or admin accounts by anyone who # has the shared secret, even if registration is otherwise disabled. diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 19816377eb2d..d1525ab1facc 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -33,8 +33,8 @@ def read_config(self, config, **kwargs): self.registrations_require_3pid = config.get("registrations_require_3pid", []) self.allowed_local_3pids = config.get("allowed_local_3pids", []) self.enable_3pid_lookup = config.get("enable_3pid_lookup", True) - self.registrations_require_token = config.get( - "registrations_require_token", False + self.registration_requires_token = config.get( + "registration_requires_token", False ) self.registration_shared_secret = config.get("registration_shared_secret") @@ -181,9 +181,13 @@ def generate_config_section(self, generate_secrets=False, **kwargs): # #enable_3pid_lookup: true - # Require users to submit a token during registration + # Require users to submit a token during registration. + # Tokens can be generated with the admin API (once it exists). + # Note that `enable_registration` must be set to `true`. + # Disabling this option will not delete any tokens previously generated. + # Defaults to false. Uncomment the following to require tokens: # - #registrations_require_token: false + #registration_requires_token: true # If set, allows registration of standard or admin accounts by anyone who # has the shared secret, even if registration is otherwise disabled. diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 0f21f1585b26..7fd34ff103e5 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -242,7 +242,7 @@ class RegistrationTokenAuthChecker(UserInteractiveAuthChecker): def __init__(self, hs: "HomeServer"): super().__init__(hs) - self._enabled = bool(hs.config.registrations_require_token) + self._enabled = bool(hs.config.registration_requires_token) self.store = hs.get_datastore() def is_enabled(self) -> bool: diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index fda79c4e4e0a..febe3b5d5a6c 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -826,7 +826,7 @@ def _calculate_registration_flows( flow.insert(0, LoginType.RECAPTCHA) # Prepend registration token to all flows if we're requiring a token - if config.registrations_require_token: + if config.registration_requires_token: for flow in flows: flow.insert(0, LoginType.REGISTRATION_TOKEN) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 356b74131784..2fff268f9e94 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -205,8 +205,8 @@ def test_POST_ratelimiting(self): self.assertEquals(channel.result["code"], b"200", channel.result) - @unittest.override_config({"registrations_require_token": True}) - def test_POST_registrations_require_token(self): + @override_config({"registration_requires_token": True}) + def test_POST_registration_requires_token(self): username = "kermit" device_id = "frogfone" token = "abcd" @@ -279,7 +279,7 @@ def test_POST_registrations_require_token(self): self.assertEquals(res["completed"], 1) self.assertEquals(res["pending"], 0) - @unittest.override_config({"registrations_require_token": True}) + @override_config({"registration_requires_token": True}) def test_POST_registration_token_invalid(self): params = { "username": "kermit", @@ -310,7 +310,7 @@ def test_POST_registration_token_invalid(self): self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) - @unittest.override_config({"registrations_require_token": True}) + @override_config({"registration_requires_token": True}) def test_POST_registration_token_limit_uses(self): token = "abcd" store = self.hs.get_datastore() @@ -382,8 +382,8 @@ def test_POST_registration_token_limit_uses(self): self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) - @unittest.override_config({"registrations_require_token": True}) - def test_registration_token_expiry(self): + @override_config({"registration_requires_token": True}) + def test_POST_registration_token_expiry(self): token = "abcd" now = self.hs.get_clock().time_msec() store = self.hs.get_datastore() From 78831914f780ce53937ee1e68511da254312832f Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Wed, 16 Jun 2021 20:32:23 +0100 Subject: [PATCH 10/52] Run connected database ops in same transaction Signed-off-by: Callum Brown --- .../storage/databases/main/registration.py | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 37cc568ebf42..aa83fcf9e2d5 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1795,17 +1795,23 @@ async def set_registration_token_pending(self, token: str) -> None: Args: token: The registration token pending use """ - pending = await self.db_pool.simple_select_one_onecol( - "registration_tokens", - keyvalues={"token": token}, - retcol="pending", - desc="set_registration_token_pending", - ) - await self.db_pool.simple_update_one( - "registration_tokens", - keyvalues={"token": token}, - updatevalues={"pending": pending + 1}, - desc="set_registration_token_pending", + + def _set_registration_token_pending_txn(txn): + pending = self.db_pool.simple_select_one_onecol_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcol="pending", + ) + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues={"pending": pending + 1}, + ) + + return await self.db_pool.runInteraction( + "set_registration_token_pending", _set_registration_token_pending_txn ) async def use_registration_token(self, token: str) -> None: @@ -1814,20 +1820,28 @@ async def use_registration_token(self, token: str) -> None: Args: token: The registration token to be 'used' """ - res = await self.db_pool.simple_select_one( - "registration_tokens", - keyvalues={"token": token}, - retcols=["pending", "completed"], - desc="use_registration_token", - ) - await self.db_pool.simple_update_one( - "registration_tokens", - keyvalues={"token": token}, - updatevalues={ - "completed": res["completed"] + 1, - "pending": res["pending"] - 1, - }, - desc="use_registration_token", + + def _use_registration_token_txn(txn): + res = self.db_pool.simple_select_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcols=["pending", "completed"], + ) + + # Decrement pending and increment completed + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues={ + "completed": res["completed"] + 1, + "pending": res["pending"] - 1, + }, + ) + + return await self.db_pool.runInteraction( + "use_registration_token", _use_registration_token_txn ) From 1debc22a5f8f08b8f918e5b0c5dc55605906e7b8 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 17 Jun 2021 11:12:01 +0100 Subject: [PATCH 11/52] Fix some formatting problems Signed-off-by: Callum Brown --- synapse/storage/databases/main/registration.py | 10 ++++++++-- .../main/delta/59/999create_registration_tokens.sql | 4 ++-- tests/rest/client/v2_alpha/test_register.py | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index aa83fcf9e2d5..8a3ab82694bf 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1783,7 +1783,10 @@ async def registration_token_is_valid(self, token: str) -> bool: return False # Check if the token has been used up - if res["uses_allowed"] and res["pending"] + res["completed"] >= res["uses_allowed"]: + if ( + res["uses_allowed"] + and res["pending"] + res["completed"] >= res["uses_allowed"] + ): return False # Otherwise, the token is valid @@ -1815,7 +1818,10 @@ def _set_registration_token_pending_txn(txn): ) async def use_registration_token(self, token: str) -> None: - """Increment the completed registrations counter for a token. + """Complete a use of the given registration token. + + The `pending` counter will be decremented, and the `completed` + counter will be incremented. Args: token: The registration token to be 'used' diff --git a/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql index a9f34c7d6702..ee6cf958f4f3 100644 --- a/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql +++ b/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql @@ -16,8 +16,8 @@ CREATE TABLE IF NOT EXISTS registration_tokens( token TEXT NOT NULL, -- The token that can be used for authentication. uses_allowed INT, -- The total number of times this token can be used. NULL if no limit. - pending INT NOT NULL, -- The number of in progress registrations using this token. - completed INT NOT NULL, -- The number of times this token has been used to complete a registration. + pending INT NOT NULL, -- The number of in progress registrations using this token. + completed INT NOT NULL, -- The number of times this token has been used to complete a registration. expiry_time BIGINT, -- The latest time this token will be valid (epoch time in milliseconds). NULL if token doesn't expire. UNIQUE (token) ); diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 2fff268f9e94..5e6a4ed955d3 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -342,7 +342,7 @@ def test_POST_registration_token_limit_uses(self): "session": session1, } self.make_request(b"POST", self.url, json.dumps(params1)) - # Repeat request to make sure pending isn't set increased again + # Repeat request to make sure pending isn't increased again self.make_request(b"POST", self.url, json.dumps(params1)) pending = self.get_success( store.db_pool.simple_select_one_onecol( From 6ac376d9d3da92908e2575799d38cb4357529326 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 17 Jun 2021 11:43:14 +0100 Subject: [PATCH 12/52] Test `completed` is empty when auth should fail Signed-off-by: Callum Brown --- tests/rest/client/v2_alpha/test_register.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 5e6a4ed955d3..10b59295d8fc 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -297,18 +297,21 @@ def test_POST_registration_token_invalid(self): channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.MISSING_PARAM) + self.assertEquals(channel.json_body["completed"], []) # Test with non-string (invalid) params["auth"]["token"] = 1234 channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.INVALID_PARAM) + self.assertEquals(channel.json_body["completed"], []) # Test with unknown token (invalid) params["auth"]["token"] = "1234" channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + self.assertEquals(channel.json_body["completed"], []) @override_config({"registration_requires_token": True}) def test_POST_registration_token_limit_uses(self): @@ -362,6 +365,7 @@ def test_POST_registration_token_limit_uses(self): channel = self.make_request(b"POST", self.url, json.dumps(params2)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + self.assertEquals(channel.json_body["completed"], []) # Complete registration with session1 params1["auth"]["type"] = LoginType.DUMMY @@ -381,6 +385,7 @@ def test_POST_registration_token_limit_uses(self): channel = self.make_request(b"POST", self.url, json.dumps(params2)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + self.assertEquals(channel.json_body["completed"], []) @override_config({"registration_requires_token": True}) def test_POST_registration_token_expiry(self): @@ -414,6 +419,7 @@ def test_POST_registration_token_expiry(self): channel = self.make_request(b"POST", self.url, json.dumps(params)) self.assertEquals(channel.result["code"], b"401", channel.result) self.assertEquals(channel.json_body["errcode"], Codes.UNAUTHORIZED) + self.assertEquals(channel.json_body["completed"], []) # Update token so it expires tomorrow self.get_success( From dfa8feccdeafb61ec0cb5470554d5eccdd3fcdd3 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 17 Jun 2021 12:02:32 +0100 Subject: [PATCH 13/52] Override type of simple_select_one_txn Signed-off-by: Callum Brown --- synapse/storage/databases/main/registration.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 8a3ab82694bf..e5809d0296c7 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1828,12 +1828,16 @@ async def use_registration_token(self, token: str) -> None: """ def _use_registration_token_txn(txn): - res = self.db_pool.simple_select_one_txn( + # Normally, res is Optional[Dict[str, Any]]. + # Override type because the return type is only optional if + # allow_none is True, and we don't want mypy throwing errors + # about None not being indexable. + res: Dict[str, Any] = self.db_pool.simple_select_one_txn( txn, "registration_tokens", keyvalues={"token": token}, retcols=["pending", "completed"], - ) + ) # type: ignore # Decrement pending and increment completed self.db_pool.simple_update_one_txn( From c89d786f82c111f6b50de46851e8db4549820478 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 17 Jun 2021 12:24:48 +0100 Subject: [PATCH 14/52] Raise error if token changes during UIA Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 7fd34ff103e5..8b1ca3a55890 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -250,16 +250,19 @@ def is_enabled(self) -> bool: async def check_auth(self, authdict: dict, clientip: str) -> Any: if "token" not in authdict: - raise LoginError(400, "Missing token", Codes.MISSING_PARAM) + raise LoginError(400, "Missing registration token", Codes.MISSING_PARAM) if not isinstance(authdict["token"], str): - raise LoginError(400, "Token must be a string", Codes.INVALID_PARAM) + raise LoginError( + 400, "Registration token must be a string", Codes.INVALID_PARAM + ) if "session" not in authdict: raise SynapseError(400, "Missing UIA session", Codes.MISSING_PARAM) + session = authdict["session"] + token = authdict["token"] # If the LoginType.REGISTRATION_TOKEN stage has already been completed, # return early to avoid incrementing `pending` again. - # TODO: check stored token matches provided token? try: stored_token = await self.store.get_ui_auth_session_data( session, @@ -268,10 +271,15 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: ) except StoreError: raise SynapseError(400, "Unknown session ID: {}".format(session)) + if stored_token: - return True + if token != stored_token: + raise SynapseError( + 400, "Registration token has changed", Codes.INVALID_PARAM + ) + else: + return True - token = authdict["token"] if await self.store.registration_token_is_valid(token): # Increment pending counter, so that if token has limited uses it # can't be used up by someone else in the meantime. From e7bd00a1cd150d5ae1568455b298cbf6cc3d8aea Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 18 Jun 2021 14:53:21 +0100 Subject: [PATCH 15/52] Add validity checking endpoint Signed-off-by: Callum Brown --- synapse/rest/client/v2_alpha/register.py | 28 ++++++++++++++ tests/rest/client/v2_alpha/test_register.py | 42 +++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index febe3b5d5a6c..5d477ae1c592 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -377,6 +377,33 @@ async def on_GET(self, request): return 200, {"available": True} +class RegistrationTokenValidityRestServlet(RestServlet): + PATTERNS = client_patterns( + "/org.matrix.msc3231/register/{}/validity".format(LoginType.REGISTRATION_TOKEN), + unstable=True, + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super().__init__() + self.hs = hs + self.store = hs.get_datastore() + + async def on_GET(self, request): + if not self.hs.config.enable_registration: + raise SynapseError( + 403, "Registration has been disabled", errcode=Codes.FORBIDDEN + ) + + token = parse_string(request, "token", required=True) + valid = await self.store.registration_token_is_valid(token) + + return 200, {"valid": valid} + + class RegisterRestServlet(RestServlet): PATTERNS = client_patterns("/register$") @@ -838,4 +865,5 @@ def register_servlets(hs, http_server): MsisdnRegisterRequestTokenRestServlet(hs).register(http_server) UsernameAvailabilityRestServlet(hs).register(http_server) RegistrationSubmitTokenServlet(hs).register(http_server) + RegistrationTokenValidityRestServlet(hs).register(http_server) RegisterRestServlet(hs).register(http_server) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 10b59295d8fc..a48d9808d2b1 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -973,3 +973,45 @@ def test_background_job(self): self.assertGreaterEqual(res, now_ms + self.validity_period - self.max_delta) self.assertLessEqual(res, now_ms + self.validity_period) + + +class RegistrationTokenValidityRestServletTestCase(unittest.HomeserverTestCase): + servlets = [register.register_servlets] + url = "/_matrix/client/unstable/org.matrix.msc3231/register/org.matrix.msc3231.login.registration_token/validity" + + def default_config(self): + config = super().default_config() + config["registration_requires_token"] = True + return config + + def test_GET_token_valid(self): + token = "abcd" + store = self.hs.get_datastore() + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": None, + "pending": 0, + "completed": 0, + "expiry_time": None, + }, + ) + ) + + channel = self.make_request( + b"GET", + self.url + "?token={}".format(token), + ) + self.assertEquals(channel.result["code"], b"200", channel.result) + self.assertEquals(channel.json_body["valid"], True) + + def test_GET_token_invalid(self): + token = "1234" + channel = self.make_request( + b"GET", + self.url + "?token={}".format(token), + ) + self.assertEquals(channel.result["code"], b"200", channel.result) + self.assertEquals(channel.json_body["valid"], False) From d6704fd496dfaab6e01db7e50228bd3c273feece Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Sun, 20 Jun 2021 13:33:58 +0100 Subject: [PATCH 16/52] Use AuthHandler methods for accessing UIA session rather than using storage methods directly. Also use UIAuthSessionDataConstants. Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/__init__.py | 5 +++++ synapse/handlers/ui_auth/checkers.py | 24 ++++++++++++------------ synapse/rest/client/v2_alpha/register.py | 2 +- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/ui_auth/__init__.py b/synapse/handlers/ui_auth/__init__.py index 4c3b669faeef..54f4164a9a6f 100644 --- a/synapse/handlers/ui_auth/__init__.py +++ b/synapse/handlers/ui_auth/__init__.py @@ -34,3 +34,8 @@ class UIAuthSessionDataConstants: # used by validate_user_via_ui_auth to store the mxid of the user we are validating # for. REQUEST_USER_ID = "request_user_id" + + # used during registration to store the registration used (if required) so that + # - we can prevent a token being used twice by one session + # - we can 'use up' the token after registration has successfully completed + REGISTRATION_TOKEN = "org.matrix.msc3231.login.registration_token" diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 8b1ca3a55890..738a73723e2e 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -18,7 +18,7 @@ from twisted.web.client import PartialDownloadError from synapse.api.constants import LoginType -from synapse.api.errors import Codes, LoginError, StoreError, SynapseError +from synapse.api.errors import Codes, LoginError, SynapseError from synapse.config.emailconfig import ThreepidBehaviour from synapse.util import json_decoder @@ -242,6 +242,7 @@ class RegistrationTokenAuthChecker(UserInteractiveAuthChecker): def __init__(self, hs: "HomeServer"): super().__init__(hs) + self.hs = hs self._enabled = bool(hs.config.registration_requires_token) self.store = hs.get_datastore() @@ -258,20 +259,19 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: if "session" not in authdict: raise SynapseError(400, "Missing UIA session", Codes.MISSING_PARAM) + # Import here to avoid a cyclic dependency + from synapse.handlers.ui_auth import UIAuthSessionDataConstants + + # Retrieve the auth handler here to avoid a cyclic dependency + auth_handler = self.hs.get_auth_handler() session = authdict["session"] token = authdict["token"] # If the LoginType.REGISTRATION_TOKEN stage has already been completed, # return early to avoid incrementing `pending` again. - try: - stored_token = await self.store.get_ui_auth_session_data( - session, - "registration_token", - None, - ) - except StoreError: - raise SynapseError(400, "Unknown session ID: {}".format(session)) - + stored_token = await auth_handler.get_session_data( + session, UIAuthSessionDataConstants.REGISTRATION_TOKEN + ) if stored_token: if token != stored_token: raise SynapseError( @@ -286,9 +286,9 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: await self.store.set_registration_token_pending(token) # Store the token in the UIA session, so that once registration # is complete `completed` can be incremented. - await self.store.set_ui_auth_session_data( + await auth_handler.set_session_data( session, - "registration_token", + UIAuthSessionDataConstants.REGISTRATION_TOKEN, token, ) return True diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 5d477ae1c592..9a96f0fead76 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -699,7 +699,7 @@ async def on_POST(self, request): # Check if a token was used to authenticate registration registration_token = await self.auth_handler.get_session_data( session_id, - "registration_token", + UIAuthSessionDataConstants.REGISTRATION_TOKEN, ) if registration_token: # Increment the `completed` counter for the token From 003e67d1ab5d64ea54f3c125ccd2b7a03e18cddc Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 29 Jun 2021 14:34:05 +0100 Subject: [PATCH 17/52] Rate limit validity checking endpoint Signed-off-by: Callum Brown --- docs/sample_config.yaml | 6 +++++ synapse/config/ratelimiting.py | 11 +++++++++ synapse/rest/client/v2_alpha/register.py | 9 +++++++ tests/rest/client/v2_alpha/test_register.py | 26 +++++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 82b55f28214f..9b783e9c8c02 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -832,6 +832,8 @@ log_config: "CONFDIR/SERVERNAME.log.config" # is using # - one for registration that ratelimits registration requests based on the # client's IP address. +# - one for checking the validity of registration tokens that ratelimits +# requests based on the client's IP address. # - one for login that ratelimits login requests based on the client's IP # address. # - one for login that ratelimits login requests based on the account the @@ -860,6 +862,10 @@ log_config: "CONFDIR/SERVERNAME.log.config" # per_second: 0.17 # burst_count: 3 # +#rc_registration_token_validity: +# per_second: 0.1 +# burst_count: 5 +# #rc_login: # address: # per_second: 0.17 diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index 7a8d5851c40b..f856327bd821 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -79,6 +79,11 @@ def read_config(self, config, **kwargs): self.rc_registration = RateLimitConfig(config.get("rc_registration", {})) + self.rc_registration_token_validity = RateLimitConfig( + config.get("rc_registration_token_validity", {}), + defaults={"per_second": 0.1, "burst_count": 5}, + ) + rc_login_config = config.get("rc_login", {}) self.rc_login_address = RateLimitConfig(rc_login_config.get("address", {})) self.rc_login_account = RateLimitConfig(rc_login_config.get("account", {})) @@ -143,6 +148,8 @@ def generate_config_section(self, **kwargs): # is using # - one for registration that ratelimits registration requests based on the # client's IP address. + # - one for checking the validity of registration tokens that ratelimits + # requests based on the client's IP address. # - one for login that ratelimits login requests based on the client's IP # address. # - one for login that ratelimits login requests based on the account the @@ -171,6 +178,10 @@ def generate_config_section(self, **kwargs): # per_second: 0.17 # burst_count: 3 # + #rc_registration_token_validity: + # per_second: 0.1 + # burst_count: 5 + # #rc_login: # address: # per_second: 0.17 diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 9a96f0fead76..31ffc563d2f1 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -28,6 +28,7 @@ ThreepidValidationError, UnrecognizedRequestError, ) +from synapse.api.ratelimiting import Ratelimiter from synapse.config import ConfigError from synapse.config.captcha import CaptchaConfig from synapse.config.consent import ConsentConfig @@ -391,8 +392,16 @@ def __init__(self, hs): super().__init__() self.hs = hs self.store = hs.get_datastore() + self.ratelimiter = Ratelimiter( + store=self.store, + clock=hs.get_clock(), + rate_hz=hs.config.ratelimiting.rc_registration_token_validity.per_second, + burst_count=hs.config.ratelimiting.rc_registration_token_validity.burst_count, + ) async def on_GET(self, request): + await self.ratelimiter.ratelimit(None, (request.getClientIP(),)) + if not self.hs.config.enable_registration: raise SynapseError( 403, "Registration has been disabled", errcode=Codes.FORBIDDEN diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index a48d9808d2b1..a53bec147b4a 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -1015,3 +1015,29 @@ def test_GET_token_invalid(self): ) self.assertEquals(channel.result["code"], b"200", channel.result) self.assertEquals(channel.json_body["valid"], False) + + @override_config( + {"rc_registration_token_validity": {"per_second": 0.1, "burst_count": 5}} + ) + def test_GET_ratelimiting(self): + token = "1234" + + for i in range(0, 6): + channel = self.make_request( + b"GET", + self.url + "?token={}".format(token), + ) + + if i == 5: + self.assertEquals(channel.result["code"], b"429", channel.result) + retry_after_ms = int(channel.json_body["retry_after_ms"]) + else: + self.assertEquals(channel.result["code"], b"200", channel.result) + + self.reactor.advance(retry_after_ms / 1000.0 + 1.0) + + channel = self.make_request( + b"GET", + self.url + "?token={}".format(token), + ) + self.assertEquals(channel.result["code"], b"200", channel.result) From 3c51680c6450713a2fc3212ae81732c17ca78476 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 29 Jun 2021 16:11:31 +0100 Subject: [PATCH 18/52] Use LoginError rather than SynapseError in checker So error fields are merged into UIA response. (Failures are due to authentication) Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 738a73723e2e..4de0fe4137dd 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -257,7 +257,7 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: 400, "Registration token must be a string", Codes.INVALID_PARAM ) if "session" not in authdict: - raise SynapseError(400, "Missing UIA session", Codes.MISSING_PARAM) + raise LoginError(400, "Missing UIA session", Codes.MISSING_PARAM) # Import here to avoid a cyclic dependency from synapse.handlers.ui_auth import UIAuthSessionDataConstants @@ -274,7 +274,7 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: ) if stored_token: if token != stored_token: - raise SynapseError( + raise LoginError( 400, "Registration token has changed", Codes.INVALID_PARAM ) else: From af90be7db3fa7f2a045278946aaf7dce599c124d Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Wed, 30 Jun 2021 16:49:02 +0100 Subject: [PATCH 19/52] Add fallback Signed-off-by: Callum Brown --- synapse/config/registration.py | 2 ++ synapse/res/templates/registration_token.html | 20 +++++++++++++ synapse/rest/client/v2_alpha/auth.py | 30 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 synapse/res/templates/registration_token.html diff --git a/synapse/config/registration.py b/synapse/config/registration.py index d1525ab1facc..f5247c5da023 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -122,6 +122,8 @@ def read_config(self, config, **kwargs): session_lifetime = self.parse_duration(session_lifetime) self.session_lifetime = session_lifetime + # The fallback template used for authenticating using a registration token + self.registration_token_template = self.read_template("registration_token.html") # The success template used during fallback auth. self.fallback_success_template = self.read_template("auth_success.html") diff --git a/synapse/res/templates/registration_token.html b/synapse/res/templates/registration_token.html new file mode 100644 index 000000000000..346ba8de4657 --- /dev/null +++ b/synapse/res/templates/registration_token.html @@ -0,0 +1,20 @@ + + +Authentication + + + + +
+
+

+ Please enter a registration token. +

+ + + +
+
+ + diff --git a/synapse/rest/client/v2_alpha/auth.py b/synapse/rest/client/v2_alpha/auth.py index 6ea1b50a625e..4b3c3ad7bc69 100644 --- a/synapse/rest/client/v2_alpha/auth.py +++ b/synapse/rest/client/v2_alpha/auth.py @@ -46,6 +46,7 @@ def __init__(self, hs: "HomeServer"): self.registration_handler = hs.get_registration_handler() self.recaptcha_template = hs.config.recaptcha_template self.terms_template = hs.config.terms_template + self.registration_token_template = hs.config.registration_token_template self.success_template = hs.config.fallback_success_template async def on_GET(self, request, stagetype): @@ -74,6 +75,14 @@ async def on_GET(self, request, stagetype): # re-authenticate with their SSO provider. html = await self.auth_handler.start_sso_ui_auth(request, session) + elif stagetype == LoginType.REGISTRATION_TOKEN: + html = self.registration_token_template.render( + session=session, + myurl="{}/r0/auth/{}/fallback/web".format( + CLIENT_API_PREFIX, LoginType.REGISTRATION_TOKEN + ), + ) + else: raise SynapseError(404, "Unknown auth stage type") @@ -108,6 +117,7 @@ async def on_POST(self, request, stagetype): % (CLIENT_API_PREFIX, LoginType.RECAPTCHA), sitekey=self.hs.config.recaptcha_public_key, ) + elif stagetype == LoginType.TERMS: authdict = {"session": session} @@ -128,9 +138,29 @@ async def on_POST(self, request, stagetype): myurl="%s/r0/auth/%s/fallback/web" % (CLIENT_API_PREFIX, LoginType.TERMS), ) + elif stagetype == LoginType.SSO: # The SSO fallback workflow should not post here, raise SynapseError(404, "Fallback SSO auth does not support POST requests.") + + elif stagetype == LoginType.REGISTRATION_TOKEN: + token = parse_string(request, "token") + authdict = {"session": session, "token": token} + + success = await self.auth_handler.add_oob_auth( + LoginType.REGISTRATION_TOKEN, authdict, request.getClientIP() + ) + + if success: + html = self.success_template.render() + else: + html = self.registration_token_template.render( + session=session, + myurl="{}/r0/auth/{}/fallback/web".format( + CLIENT_API_PREFIX, LoginType.REGISTRATION_TOKEN + ), + ) + else: raise SynapseError(404, "Unknown auth stage type") From 1552b70b29e07b8f539913881591fd29a4b95f35 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 6 Jul 2021 12:32:35 +0100 Subject: [PATCH 20/52] Docs for currently non-existent admin API Signed-off-by: Callum Brown --- docs/SUMMARY.md | 3 +- docs/admin_api/registration_tokens.md | 289 ++++++++++++++++++++++++++ 2 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 docs/admin_api/registration_tokens.md diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 8f39ae027001..3f9108adad20 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -52,6 +52,7 @@ - [Purge History](admin_api/purge_history_api.md) - [Purge Rooms](admin_api/purge_room.md) - [Register Users](admin_api/register_api.md) + - [Registration Tokens](admin_api/registration_tokens.md) - [Manipulate Room Membership](admin_api/room_membership.md) - [Rooms](admin_api/rooms.md) - [Server Notices](admin_api/server_notices.md) @@ -84,4 +85,4 @@ - [Scripts]() # Other - - [Dependency Deprecation Policy](deprecation_policy.md) \ No newline at end of file + - [Dependency Deprecation Policy](deprecation_policy.md) diff --git a/docs/admin_api/registration_tokens.md b/docs/admin_api/registration_tokens.md new file mode 100644 index 000000000000..403c68e1817d --- /dev/null +++ b/docs/admin_api/registration_tokens.md @@ -0,0 +1,289 @@ +# Registration Tokens + +This API allows you to manage tokens which can be used to authenticate +registration requests, as proposed in [MSC3231](https://github.com/govynnus/matrix-doc/blob/token-registration/proposals/3231-token-authenticated-registration.md). +To use it, you will need to enable the `registration_requires_token` config +option, and authenticate by providing an `access_token` for a server admin: +see [Admin API](../../usage/administration/admin_api). + + +## Registration token objects + +Most endpoints make use of JSON objects that contain details about tokens. +These objects have the following fields: +- `token`: The token which can be used to authenticate registration. +- `uses_allowed`: The number of times the token can be used to complete a + registration before it becomes invalid. +- `pending`: The number of pending uses the token has. When someone uses + the token to authenticate themselves, the pending counter is incremented + so that the token is not used more than the permitted number of times. + When the person completes registration the pending counter is decremented. +- `completed`: The number of times the token has been used to successfully + complete a registration. +- `expiry_time`: The latest time the token is valid. Given as the number of + milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). + A more human friendly format will be provided at some point, but in the + meantime you can remove the milliseconds and use the `date` command. + For example, `date -d '@1625394937'`. + + +## List all tokens + +Lists all tokens and details about them. If the request is successful, the top +level JSON object will have a `registration_tokens` key which is an array of +registration token objects. + +``` +GET /_synapse/admin/v1/registration_tokens +``` + +Optional query parameters: +- `valid`: `true` or `false`. If `true`, only valid tokens are returned. + If `false`, only invalid tokens are returned. If omitted, all tokens are + returned regardless of validity. + +Example: + +``` +GET /_synapse/admin/v1/registration_tokens +``` +``` +200 OK + +{ + "registration_tokens": [ + { + "token": "abcd", + "uses_allowed": 3, + "pending": 0, + "completed": 1, + "expiry_time": null + }, + { + "token": "pqrs", + "uses_allowed": 2, + "pending": 1, + "completed": 1, + "expiry_time": null + }, + { + "token": "wxyz", + "uses_allowed": null, + "pending": 0, + "completed": 9, + "expiry_time": 1625394937000 // 2021-07-04 10:35:37 UTC + } + ] +} +``` + +Example using the `valid` query parameter: + +``` +GET /_synapse/admin/v1/registration_tokens?valid=false +``` +``` +200 OK + +{ + "registration_tokens": [ + { + "token": "pqrs", + "uses_allowed": 2, + "pending": 1, + "completed": 1, + "expiry_time": null + }, + { + "token": "wxyz", + "uses_allowed": null, + "pending": 0, + "completed": 9, + "expiry_time": 1625394937000 // 2021-07-04 10:35:37 UTC + } + ] +} +``` + + +## Get one token + +Get details about a single token. If the request is successful, the response +body will be a registration token object. + +``` +GET /_synapse/admin/v1/registration_tokens/ +``` + +Path parameters: +- `token`: The registration token to return details of. + +Example: + +``` +GET /_synapse/admin/v1/registration_tokens/abcd +``` +``` +200 OK + +{ + "token": "abcd", + "uses_allowed": 3, + "pending": 0, + "completed": 1, + "expiry_time": null +} +``` + + +## Create token + +Create a new registration token. If the request is successful, the newly created +token will be returned as a registration token object in the response body. + +``` +POST /_synapse/admin/v1/registration_tokens/new +``` + +The request body must be a JSON object and can contain the following fields: +- `token`: The registration token. A string of no more than 64 characters that + consists only of characters matched by the regex `[A-Za-z0-9-_]`. + Default: randomly generated. +- `uses_allowed`: The integer number of times the token can be used to complete + a registration before it becomes invalid. + Default: `null` (unlimited uses). +- `expiry_time`: The latest time the token is valid. Given as the number of + milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). + You could use, for example, `date '+%s000' -d 'tomorrow'`. + Default: `null` (token does not expire). +- `length`: The length of the token randomly generated if `token` is not + specified. Must be between 1 and 64 inclusive. Default: `16`. + +If a field is omitted the default is used. + +Example using defaults: + +``` +POST /_synapse/admin/v1/registration_tokens/new + +{} +``` +``` +200 OK + +{ + "token": "0M-9jbkf2t_Tgiw1", + "uses_allowed": null, + "pending": 0, + "completed": 0, + "expiry_time": null +} +``` + +Example specifying some fields: + +``` +POST /_synapse/admin/v1/registration_tokens/new + +{ + "token": "defg", + "uses_allowed": 1 +} +``` +``` +200 OK + +{ + "token": "defg", + "uses_allowed": 1, + "pending": 0, + "completed": 0, + "expiry_time": null +} +``` + + +## Update token + +Update the number of allowed uses or expiry time of a token. If the request is +successful, the updated token will be returned as a registration token object +in the response body. + +``` +PUT /_synapse/admin/v1/registration_tokens/ +``` + +Path parameters: +- `token`: The registration token to update. + +The request body must be a JSON object and can contain the following fields: +- `uses_allowed`: The integer number of times the token can be used to complete + a registration before it becomes invalid. +- `expiry_time`: The latest time the token is valid. Given as the number of + milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). + +If a field is omitted its value is not modified. + +Example: + +``` +PUT /_synapse/admin/v1/registration_tokens/defg + +{ + "expiry_time": 4781243146000 // 2121-07-06 11:05:46 UTC +} +``` +``` +200 OK + +{ + "token": "defg", + "uses_allowed": 1, + "pending": 0, + "completed": 0, + "expiry_time": 4781243146000 +} +``` + + +## Delete token + +Delete a registration token. If the request is successful, the response body +will be empty. + +``` +DELETE /_synapse/admin/v1/registration_tokens/ +``` + +Path parameters: +- `token`: The registration token to delete. + +Example: + +``` +DELETE /_synapse/admin/v1/registration_tokens/wxyz +``` +``` +200 OK +``` + + +## Errors + +If a request fails a "standard error response" will be returned as defined in +the [Matrix Client-Server API specification](https://matrix.org/docs/spec/client_server/r0.6.1#api-standards). + +For example, if the token specified in a path parameter does not exist a +`404 Not Found` error will be returned. + +``` +GET /_synapse/admin/v1/registration_tokens/1234 +``` +``` +404 Not Found + +{ + "errcode": "M_NOT_FOUND", + "error": "No such registration token: 1234" +} +``` From 4df4a6e66303bd67fdcc83a989c9541aeb052ccb Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Sat, 10 Jul 2021 13:45:33 +0100 Subject: [PATCH 21/52] Implement admin API Signed-off-by: Callum Brown --- docs/admin_api/registration_tokens.md | 4 +- synapse/rest/admin/__init__.py | 8 + synapse/rest/admin/registration_tokens.py | 195 +++++++ .../storage/databases/main/registration.py | 160 ++++++ tests/rest/admin/test_registration_tokens.py | 517 ++++++++++++++++++ 5 files changed, 883 insertions(+), 1 deletion(-) create mode 100644 synapse/rest/admin/registration_tokens.py create mode 100644 tests/rest/admin/test_registration_tokens.py diff --git a/docs/admin_api/registration_tokens.md b/docs/admin_api/registration_tokens.md index 403c68e1817d..49b2a8935741 100644 --- a/docs/admin_api/registration_tokens.md +++ b/docs/admin_api/registration_tokens.md @@ -249,7 +249,7 @@ PUT /_synapse/admin/v1/registration_tokens/defg ## Delete token Delete a registration token. If the request is successful, the response body -will be empty. +will be an empty JSON object. ``` DELETE /_synapse/admin/v1/registration_tokens/ @@ -265,6 +265,8 @@ DELETE /_synapse/admin/v1/registration_tokens/wxyz ``` ``` 200 OK + +{} ``` diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index abf749b001ab..d2b93b80437e 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -37,6 +37,11 @@ from synapse.rest.admin.groups import DeleteGroupAdminRestServlet from synapse.rest.admin.media import ListMediaInRoom, register_servlets_for_media_repo from synapse.rest.admin.purge_room_servlet import PurgeRoomServlet +from synapse.rest.admin.registration_tokens import ( + ListRegistrationTokensRestServlet, + NewRegistrationTokenRestServlet, + RegistrationTokenRestServlet, +) from synapse.rest.admin.rooms import ( DeleteRoomRestServlet, ForwardExtremitiesRestServlet, @@ -241,6 +246,9 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: ForwardExtremitiesRestServlet(hs).register(http_server) RoomEventContextServlet(hs).register(http_server) RateLimitRestServlet(hs).register(http_server) + ListRegistrationTokensRestServlet(hs).register(http_server) + NewRegistrationTokenRestServlet(hs).register(http_server) + RegistrationTokenRestServlet(hs).register(http_server) def register_servlets_for_client_rest_resource( diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py new file mode 100644 index 000000000000..4b1cf0dbf67b --- /dev/null +++ b/synapse/rest/admin/registration_tokens.py @@ -0,0 +1,195 @@ +# Copyright 2021 Callum Brown +# +# 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 +import random +import string +from typing import TYPE_CHECKING, Tuple + +from synapse.api.errors import Codes, NotFoundError, SynapseError +from synapse.http.servlet import ( + RestServlet, + parse_boolean, + parse_json_object_from_request, +) +from synapse.http.site import SynapseRequest +from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin +from synapse.types import JsonDict + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class ListRegistrationTokensRestServlet(RestServlet): + """List registration tokens.""" + + PATTERNS = admin_patterns("/registration_tokens$") + + def __init__(self, hs: "HomeServer"): + self.hs = hs + self.auth = hs.get_auth() + self.store = hs.get_datastore() + + async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self.auth, request) + valid = parse_boolean(request, "valid") + token_list = await self.store.get_registration_tokens(valid) + return 200, {"registration_tokens": token_list} + + +class NewRegistrationTokenRestServlet(RestServlet): + """Create a new registration token.""" + + PATTERNS = admin_patterns("/registration_tokens/new$") + + def __init__(self, hs: "HomeServer"): + self.hs = hs + self.auth = hs.get_auth() + self.store = hs.get_datastore() + # A string of all the characters allowed to be in a registration_token + self.allowed_chars = string.ascii_letters + string.digits + "-_" + self.allowed_chars_set = set(self.allowed_chars) + + async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self.auth, request) + body = parse_json_object_from_request(request) + + if "token" in body: + token = body["token"] + if not isinstance(token, str): + raise SynapseError(400, "token must be a string", Codes.INVALID_PARAM) + if len(token) > 64: + raise SynapseError( + 400, + "token must be no longer than 64 characters long", + Codes.INVALID_PARAM, + ) + if not set(token).issubset(self.allowed_chars_set): + raise SynapseError( + 400, + "token must consist only of characters matched by the regex [A-Za-z0-9-_]", + Codes.INVALID_PARAM, + ) + + else: + # Get length of token to generate (default is 16) + length = body.get("length", 16) + if not isinstance(length, int): + raise SynapseError( + 400, "length must be an integer", Codes.INVALID_PARAM + ) + if length > 64: + raise SynapseError( + 400, "length must not be greater than 64", Codes.INVALID_PARAM + ) + + # Generate token + token = "".join(random.choices(self.allowed_chars, k=length)) + + uses_allowed = body.get("uses_allowed", None) + if not isinstance(uses_allowed, (int, type(None))): + raise SynapseError( + 400, "uses_allowed must be an integer or null", Codes.INVALID_PARAM + ) + + expiry_time = body.get("expiry_time", None) + if not isinstance(expiry_time, (int, type(None))): + raise SynapseError( + 400, "expiry_time must be an integer or null", Codes.INVALID_PARAM + ) + + res = await self.store.create_registration_token( + token, uses_allowed, expiry_time + ) + if not res: + # Creation failed, probably the token already exists + raise SynapseError( + 400, "Token already exists: {}".format(token), Codes.INVALID_PARAM + ) + + resp = { + "token": token, + "uses_allowed": uses_allowed, + "pending": 0, + "completed": 0, + "expiry_time": expiry_time, + } + return 200, resp + + +class RegistrationTokenRestServlet(RestServlet): + """Retrieve, update, or delete the given token.""" + + PATTERNS = admin_patterns("/registration_tokens/(?P[^/]*)$") + + def __init__(self, hs: "HomeServer"): + self.hs = hs + self.auth = hs.get_auth() + self.store = hs.get_datastore() + + async def on_GET(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDict]: + """Retrieve a registration token.""" + await assert_requester_is_admin(self.auth, request) + token_info = await self.store.get_one_registration_token(token) + + # If no result return a 404 + if token_info is None: + raise NotFoundError("No such registration token: {}".format(token)) + + return 200, token_info + + async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDict]: + """Update a registration token.""" + await assert_requester_is_admin(self.auth, request) + body = parse_json_object_from_request(request) + updatevalues = {} + + # Only add uses_allowed to updatevalues if it is present and valid + if "uses_allowed" in body: + if not isinstance(body["uses_allowed"], (int, type(None))): + raise SynapseError( + 400, "uses_allowed must be an integer or null", Codes.INVALID_PARAM + ) + updatevalues["uses_allowed"] = body["uses_allowed"] + + if "expiry_time" in body: + if not isinstance(body["expiry_time"], (int, type(None))): + raise SynapseError( + 400, "expiry_time must be an integer or null", Codes.INVALID_PARAM + ) + updatevalues["expiry_time"] = body["expiry_time"] + + if len(updatevalues) == 0: + raise SynapseError(400, "Nothing to update", Codes.MISSING_PARAM) + + token_info = await self.store.update_registration_token(token, updatevalues) + + # If no result return a 404 + if token_info is None: + raise NotFoundError("No such registration token: {}".format(token)) + + return 200, token_info + + async def on_DELETE( + self, request: SynapseRequest, token: str + ) -> Tuple[int, JsonDict]: + """Delete a registration token.""" + await assert_requester_is_admin(self.auth, request) + + if await self.store.delete_registration_token(token): + return 200, {} + else: + raise NotFoundError("No such registration token: {}".format(token)) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index e5809d0296c7..1056b092c5ea 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1854,6 +1854,166 @@ def _use_registration_token_txn(txn): "use_registration_token", _use_registration_token_txn ) + async def get_registration_tokens( + self, valid: Optional[bool] = None + ) -> List[Dict[str, Any]]: + """List all registration tokens. Used by the admin API. + + Args: + valid: If True, only valid tokens are returned. + If False, only invalid tokens are returned. + Default is None: return all tokens regardless of validity. + + Returns: + A list of dicts, each containing details of a token. + """ + + def select_registration_tokens_txn(txn, now: int, valid: Optional[bool]): + if valid is None: + # Return all tokens regardless of validity + txn.execute("SELECT * FROM registration_tokens") + + elif valid: + # Select valid tokens only + sql = ( + "SELECT * FROM registration_tokens WHERE " + "(uses_allowed > pending + completed OR uses_allowed IS NULL)" + "AND (expiry_time > ? OR expiry_time IS NULL)" + ) + txn.execute(sql, [now]) + + else: + # Select invalid tokens only + sql = ( + "SELECT * FROM registration_tokens WHERE " + "uses_allowed <= pending + completed OR expiry_time < ?" + ) + txn.execute(sql, [now]) + + return self.db_pool.cursor_to_dict(txn) + + return await self.db_pool.runInteraction( + "select_registration_tokens", + select_registration_tokens_txn, + self._clock.time_msec(), + valid, + ) + + async def get_one_registration_token(self, token: str) -> Optional[Dict[str, Any]]: + """Get info about the given registration token. Used by the admin API. + + Args: + token: The token to retrieve information about. + + Returns: + A dict, or None if token doesn't exist. + """ + return await self.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["token", "uses_allowed", "pending", "completed", "expiry_time"], + allow_none=True, + desc="get_one_registration_token", + ) + + async def create_registration_token( + self, token: str, uses_allowed: Optional[int], expiry_time: Optional[int] + ) -> bool: + """Create a new registration token. Used by the admin API. + + Args: + token: The token to create. + uses_allowed: The number of times the token can be used to complete + a registration before it becomes invalid. A value of None indicates + unlimited uses. + expiry_time: The latest time the token is valid. Given as the + number of milliseconds since 1970-01-01 00:00:00 UTC. A value of + None indicates that the token does not expire. + + Returns: + Whether the row was inserted or not. + """ + return await self.db_pool.simple_insert( + "registration_tokens", + values={ + "token": token, + "uses_allowed": uses_allowed, + "pending": 0, + "completed": 0, + "expiry_time": expiry_time, + }, + or_ignore=True, + desc="create_registration_token", + ) + + async def update_registration_token( + self, token: str, updatevalues: Dict[str, Optional[int]] + ) -> Optional[Dict[str, Any]]: + """Update a registration token. Used by the admin API. + + Args: + token: The token to update. + updatevalues: A dict with the fields to update. E.g.: + `{"uses_allowed": 3}` to update just uses_allowed, or + `{"uses_allowed": 3, "expiry_time": None}` to update both. + This is passed straight to simple_update_one. + + Returns: + A dict with all info about the token, or None if token doesn't exist. + """ + + def _update_registration_token_txn(txn): + try: + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues=updatevalues, + ) + except StoreError: + # Update failed because token does not exist + return None + + # Get all info about the token so it can be sent in the response + return self.db_pool.simple_select_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcols=[ + "token", + "uses_allowed", + "pending", + "completed", + "expiry_time", + ], + allow_none=True, + ) + + return await self.db_pool.runInteraction( + "update_registration_token", _update_registration_token_txn + ) + + async def delete_registration_token(self, token: str) -> bool: + """Delete a registration token. Used by the admin API. + + Args: + token: The token to delete. + + Returns: + Whether the token was successfully deleted or not. + """ + try: + await self.db_pool.simple_delete_one( + "registration_tokens", + keyvalues={"token": token}, + desc="delete_registration_token", + ) + except StoreError: + # Deletion failed because token does not exist + return False + + return True + def find_max_generated_user_id_localpart(cur: Cursor) -> int: """ diff --git a/tests/rest/admin/test_registration_tokens.py b/tests/rest/admin/test_registration_tokens.py new file mode 100644 index 000000000000..4eff70862a8c --- /dev/null +++ b/tests/rest/admin/test_registration_tokens.py @@ -0,0 +1,517 @@ +# Copyright 2021 Callum Brown +# +# 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 synapse.rest.admin +from synapse.api.errors import Codes +from synapse.rest.client.v1 import login + +from tests import unittest + + +class ManageRegistrationTokensTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs): + self.admin_user = self.register_user("admin", "pass", admin=True) + self.admin_user_tok = self.login("admin", "pass") + + self.other_user = self.register_user("user", "pass") + self.other_user_tok = self.login("user", "pass") + + self.url = "/_synapse/admin/v1/registration_tokens" + + def _new_token(self, data): + """Helper function to create a token.""" + channel = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + return channel.json_body["token"] + + # CREATION + + def test_create_no_auth(self): + """Try to create a token without authentication.""" + channel = self.make_request("POST", self.url + "/new", {}) + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_create_requester_not_admin(self): + """Try to create a token while not an admin.""" + channel = self.make_request( + "POST", + self.url + "/new", + {}, + access_token=self.other_user_tok, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_create_using_defaults(self): + """Create a token using all the defaults.""" + channel = self.make_request( + "POST", + self.url + "/new", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["token"]), 16) + self.assertIsNone(channel.json_body["uses_allowed"]) + self.assertIsNone(channel.json_body["expiry_time"]) + self.assertEqual(channel.json_body["pending"], 0) + self.assertEqual(channel.json_body["completed"], 0) + + def test_create_specifying_fields(self): + """Create a token specifying the value of all fields.""" + data = { + "token": "abcd", + "uses_allowed": 1, + "expiry_time": 1626427432000, + } + + channel = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["token"], "abcd") + self.assertEqual(channel.json_body["uses_allowed"], 1) + self.assertEqual(channel.json_body["expiry_time"], 1626427432000) + self.assertEqual(channel.json_body["pending"], 0) + self.assertEqual(channel.json_body["completed"], 0) + + def test_create_with_null_value(self): + """Create a token specifying unlimited uses and no expiry.""" + data = { + "uses_allowed": None, + "expiry_time": None, + } + + channel = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["token"]), 16) + self.assertIsNone(channel.json_body["uses_allowed"]) + self.assertIsNone(channel.json_body["expiry_time"]) + self.assertEqual(channel.json_body["pending"], 0) + self.assertEqual(channel.json_body["completed"], 0) + + def test_create_token_too_long(self): + """Check token longer than 64 chars is invalid.""" + data = {"token": "a" * 65} + + channel = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + def test_create_token_invalid_chars(self): + """Check you can't create token with invalid characters.""" + data = { + "token": "abc/def", + } + + channel = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + def test_create_token_already_exists(self): + """Check you can't create token that already exists.""" + data = { + "token": "abcd", + } + + channel1 = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel1.result["code"]), msg=channel1.result["body"]) + + channel2 = self.make_request( + "POST", + self.url + "/new", + data, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel2.result["code"]), msg=channel2.result["body"]) + self.assertEqual(channel2.json_body["errcode"], Codes.INVALID_PARAM) + + # UPDATING + + def test_update_no_auth(self): + """Try to update a token without authentication.""" + channel = self.make_request( + "PUT", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + ) + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_update_requester_not_admin(self): + """Try to update a token while not an admin.""" + channel = self.make_request( + "PUT", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + access_token=self.other_user_tok, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_update_non_existent(self): + """Try to update a token that doesn't exist.""" + channel = self.make_request( + "PUT", + self.url + "/1234", + {"uses_allowed": 1}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND) + + def test_update_uses_allowed(self): + """Test updating just uses_allowed.""" + # Create new token using default values + token = self._new_token({}) + + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"uses_allowed": 1}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["uses_allowed"], 1) + self.assertIsNone(channel.json_body["expiry_time"]) + + def test_update_expiry_time(self): + """Test updating just expiry_time.""" + # Create new token using default values + token = self._new_token({}) + + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"expiry_time": 1626430124000}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["expiry_time"], 1626430124000) + self.assertIsNone(channel.json_body["uses_allowed"]) + + def test_update_both(self): + """Test updating both uses_allowed and expiry_time.""" + # Create new token using default values + token = self._new_token({}) + + data = { + "uses_allowed": 1, + "expiry_time": 1626430124000, + } + + channel = self.make_request( + "PUT", + self.url + "/" + token, + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["uses_allowed"], 1) + self.assertEqual(channel.json_body["expiry_time"], 1626430124000) + + def test_update_invalid_type(self): + """Test using invalid types doesn't work.""" + # Create new token using default values + token = self._new_token({}) + + data = { + "uses_allowed": False, + "expiry_time": "1626430124000", + } + + channel = self.make_request( + "PUT", + self.url + "/" + token, + data, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # DELETING + + def test_delete_no_auth(self): + """Try to delete a token without authentication.""" + channel = self.make_request( + "DELETE", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + ) + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_delete_requester_not_admin(self): + """Try to delete a token while not an admin.""" + channel = self.make_request( + "DELETE", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + access_token=self.other_user_tok, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_delete_non_existent(self): + """Try to delete a token that doesn't exist.""" + channel = self.make_request( + "DELETE", + self.url + "/1234", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND) + + def test_delete(self): + """Test deleting a token.""" + # Create new token using default values + token = self._new_token({}) + + channel = self.make_request( + "DELETE", + self.url + "/" + token, + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + + # GETTING ONE + + def test_get_no_auth(self): + """Try to get a token without authentication.""" + channel = self.make_request( + "GET", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + ) + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_get_requester_not_admin(self): + """Try to get a token while not an admin.""" + channel = self.make_request( + "GET", + self.url + "/1234", # Token doesn't exist but that doesn't matter + {}, + access_token=self.other_user_tok, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_get_non_existent(self): + """Try to get a token that doesn't exist.""" + channel = self.make_request( + "GET", + self.url + "/1234", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.NOT_FOUND) + + def test_get(self): + """Test getting a token.""" + # Create new token using default values + token = self._new_token({}) + + channel = self.make_request( + "GET", + self.url + "/" + token, + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["token"], token) + self.assertIsNone(channel.json_body["uses_allowed"]) + self.assertIsNone(channel.json_body["expiry_time"]) + self.assertEqual(channel.json_body["pending"], 0) + self.assertEqual(channel.json_body["completed"], 0) + + # LISTING + + def test_list_no_auth(self): + """Try to list tokens without authentication.""" + channel = self.make_request("GET", self.url, {}) + self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_list_requester_not_admin(self): + """Try to list tokens while not an admin.""" + channel = self.make_request( + "GET", + self.url, + {}, + access_token=self.other_user_tok, + ) + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_list_all(self): + """Test listing all tokens.""" + # Create new token using default values + token = self._new_token({}) + + channel = self.make_request( + "GET", + self.url, + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["registration_tokens"]), 1) + token_info = channel.json_body["registration_tokens"][0] + self.assertEqual(token_info["token"], token) + self.assertIsNone(token_info["uses_allowed"]) + self.assertIsNone(token_info["expiry_time"]) + self.assertEqual(token_info["pending"], 0) + self.assertEqual(token_info["completed"], 0) + + def test_list_invalid_query_parameter(self): + """Test with `valid` query parameter not `true` or `false`.""" + channel = self.make_request( + "GET", + self.url + "?valid=x", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + + def test_list_valid(self): + """Test listing just valid tokens.""" + now = self.hs.get_clock().time_msec() + # Create always valid token + token_valid_1 = self._new_token({}) + # Create token that hasn't been used up + token_valid_2 = self._new_token({"uses_allowed": 1}) + # Create token that has expired + self._new_token({"expiry_time": now - 10000}) + # Create token that has been used up but hasn't expired + # Can't use API because it doesn't allow changing pending or completed + store = self.hs.get_datastore() + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": "invalid_2", + "uses_allowed": 2, + "pending": 1, + "completed": 1, + "expiry_time": now + 1000000, + }, + ) + ) + + channel = self.make_request( + "GET", + self.url + "?valid=true", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["registration_tokens"]), 2) + token_info_1 = channel.json_body["registration_tokens"][0] + token_info_2 = channel.json_body["registration_tokens"][1] + self.assertIn(token_info_1["token"], (token_valid_1, token_valid_2)) + self.assertIn(token_info_2["token"], (token_valid_1, token_valid_2)) + + def test_list_invalid(self): + """Test listing just invalid tokens.""" + now = self.hs.get_clock().time_msec() + # Create always valid token + self._new_token({}) + # Create token that hasn't been used up + self._new_token({"uses_allowed": 1}) + # Create token that has expired + token_invalid_1 = self._new_token({"expiry_time": now - 10000}) + # Create token that has been used up but hasn't expired + # Can't use API because it doesn't allow changing pending or completed + token_invalid_2 = "invalid_2" + store = self.hs.get_datastore() + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token_invalid_2, + "uses_allowed": 2, + "pending": 1, + "completed": 1, + "expiry_time": now + 1000000, + }, + ) + ) + + channel = self.make_request( + "GET", + self.url + "?valid=false", + {}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["registration_tokens"]), 2) + token_info_1 = channel.json_body["registration_tokens"][0] + token_info_2 = channel.json_body["registration_tokens"][1] + self.assertIn(token_info_1["token"], (token_invalid_1, token_invalid_2)) + self.assertIn(token_info_2["token"], (token_invalid_1, token_invalid_2)) From 6901eee9a766d6d7d07eaecaec57784ddfd22625 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Mon, 19 Jul 2021 13:51:22 +0100 Subject: [PATCH 22/52] Move admin api docs to correct location Thanks to dklimpel Signed-off-by: Callum Brown --- docs/SUMMARY.md | 2 +- .../{ => usage/administration}/admin_api/registration_tokens.md | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/{ => usage/administration}/admin_api/registration_tokens.md (100%) diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 3f9108adad20..35d0096bf5e1 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -52,7 +52,7 @@ - [Purge History](admin_api/purge_history_api.md) - [Purge Rooms](admin_api/purge_room.md) - [Register Users](admin_api/register_api.md) - - [Registration Tokens](admin_api/registration_tokens.md) + - [Registration Tokens](usage/administration/admin_api/registration_tokens.md) - [Manipulate Room Membership](admin_api/room_membership.md) - [Rooms](admin_api/rooms.md) - [Server Notices](admin_api/server_notices.md) diff --git a/docs/admin_api/registration_tokens.md b/docs/usage/administration/admin_api/registration_tokens.md similarity index 100% rename from docs/admin_api/registration_tokens.md rename to docs/usage/administration/admin_api/registration_tokens.md From 93f752d017dc76b1a154d50ad9954590c7ad7389 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 20 Jul 2021 14:11:26 +0100 Subject: [PATCH 23/52] Include general API shape in docstrings Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 102 +++++++++++++++++++++- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 4b1cf0dbf67b..f97316f08698 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -34,7 +34,36 @@ class ListRegistrationTokensRestServlet(RestServlet): - """List registration tokens.""" + """List registration tokens. + + To list all tokens: + + GET /_synapse/admin/v1/registration_tokens + + 200 OK + + { + "registration_tokens": [ + { + "token": "abcd", + "uses_allowed": 3, + "pending": 0, + "completed": 1, + "expiry_time": null + }, + { + "token": "wxyz", + "uses_allowed": null, + "pending": 0, + "completed": 9, + "expiry_time": 1625394937000 + } + ] + } + + The optional query parameter `valid` is used to the response to only + valid or invalid tokens. It can be `true` or `false`. + """ PATTERNS = admin_patterns("/registration_tokens$") @@ -51,7 +80,29 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: class NewRegistrationTokenRestServlet(RestServlet): - """Create a new registration token.""" + """Create a new registration token. + + For example, to create a token specifying some fields: + + POST /_synapse/admin/v1/registration_tokens/new + + { + "token": "defg", + "uses_allowed": 1 + } + + 200 OK + + { + "token": "defg", + "uses_allowed": 1, + "pending": 0, + "completed": 0, + "expiry_time": null + } + + Defaults are used for any fields not specified. + """ PATTERNS = admin_patterns("/registration_tokens/new$") @@ -131,7 +182,52 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: class RegistrationTokenRestServlet(RestServlet): - """Retrieve, update, or delete the given token.""" + """Retrieve, update, or delete the given token. + + For example, + + to retrieve a token: + + GET /_synapse/admin/v1/registration_tokens/abcd + + 200 OK + + { + "token": "abcd", + "uses_allowed": 3, + "pending": 0, + "completed": 1, + "expiry_time": null + } + + + to update a token: + + PUT /_synapse/admin/v1/registration_tokens/defg + + { + "expiry_time": 4781243146000 + } + + 200 OK + + { + "token": "defg", + "uses_allowed": 1, + "pending": 0, + "completed": 0, + "expiry_time": 4781243146000 + } + + + to delete a token: + + DELETE /_synapse/admin/v1/registration_tokens/wxyz + + 200 OK + + {} + """ PATTERNS = admin_patterns("/registration_tokens/(?P[^/]*)$") From b2bf3acc496556a19722fd5bfe3a8d24a6fe1b4e Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 20 Jul 2021 16:14:59 +0100 Subject: [PATCH 24/52] More input validation when creating and updating Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 43 ++- tests/rest/admin/test_registration_tokens.py | 320 ++++++++++++++----- 2 files changed, 273 insertions(+), 90 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index f97316f08698..0782f0b85b86 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -110,6 +110,7 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.auth = hs.get_auth() self.store = hs.get_datastore() + self.clock = hs.get_clock() # A string of all the characters allowed to be in a registration_token self.allowed_chars = string.ascii_letters + string.digits + "-_" self.allowed_chars_set = set(self.allowed_chars) @@ -122,10 +123,10 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: token = body["token"] if not isinstance(token, str): raise SynapseError(400, "token must be a string", Codes.INVALID_PARAM) - if len(token) > 64: + if not (0 < len(token) <= 64): raise SynapseError( 400, - "token must be no longer than 64 characters long", + "token must not be empty and must not be longer than 64 characters", Codes.INVALID_PARAM, ) if not set(token).issubset(self.allowed_chars_set): @@ -142,18 +143,25 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: raise SynapseError( 400, "length must be an integer", Codes.INVALID_PARAM ) - if length > 64: + if not (0 < length <= 64): raise SynapseError( - 400, "length must not be greater than 64", Codes.INVALID_PARAM + 400, + "length must be greater than zero and not greater than 64", + Codes.INVALID_PARAM, ) # Generate token token = "".join(random.choices(self.allowed_chars, k=length)) uses_allowed = body.get("uses_allowed", None) - if not isinstance(uses_allowed, (int, type(None))): + if not ( + uses_allowed is None + or (isinstance(uses_allowed, int) and uses_allowed >= 0) + ): raise SynapseError( - 400, "uses_allowed must be an integer or null", Codes.INVALID_PARAM + 400, + "uses_allowed must be a non-negative integer or null", + Codes.INVALID_PARAM, ) expiry_time = body.get("expiry_time", None) @@ -161,6 +169,10 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: raise SynapseError( 400, "expiry_time must be an integer or null", Codes.INVALID_PARAM ) + if isinstance(expiry_time, int) and expiry_time < self.clock.time_msec(): + raise SynapseError( + 400, "expiry_time must not be in the past", Codes.INVALID_PARAM + ) res = await self.store.create_registration_token( token, uses_allowed, expiry_time @@ -233,6 +245,7 @@ class RegistrationTokenRestServlet(RestServlet): def __init__(self, hs: "HomeServer"): self.hs = hs + self.clock = hs.get_clock() self.auth = hs.get_auth() self.store = hs.get_datastore() @@ -255,18 +268,26 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # Only add uses_allowed to updatevalues if it is present and valid if "uses_allowed" in body: - if not isinstance(body["uses_allowed"], (int, type(None))): + ua = body["uses_allowed"] + if not (ua is None or (isinstance(ua, int) and ua >= 0)): raise SynapseError( - 400, "uses_allowed must be an integer or null", Codes.INVALID_PARAM + 400, + "uses_allowed must be a non-negative integer or null", + Codes.INVALID_PARAM, ) - updatevalues["uses_allowed"] = body["uses_allowed"] + updatevalues["uses_allowed"] = ua if "expiry_time" in body: - if not isinstance(body["expiry_time"], (int, type(None))): + et = body["expiry_time"] + if not isinstance(et, (int, type(None))): raise SynapseError( 400, "expiry_time must be an integer or null", Codes.INVALID_PARAM ) - updatevalues["expiry_time"] = body["expiry_time"] + if isinstance(et, int) and et < self.clock.time_msec(): + raise SynapseError( + 400, "expiry_time must not be in the past", Codes.INVALID_PARAM + ) + updatevalues["expiry_time"] = et if len(updatevalues) == 0: raise SynapseError(400, "Nothing to update", Codes.MISSING_PARAM) diff --git a/tests/rest/admin/test_registration_tokens.py b/tests/rest/admin/test_registration_tokens.py index 4eff70862a8c..a74be6cde3d1 100644 --- a/tests/rest/admin/test_registration_tokens.py +++ b/tests/rest/admin/test_registration_tokens.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import random +import string + import synapse.rest.admin from synapse.api.errors import Codes from synapse.rest.client.v1 import login @@ -26,6 +29,7 @@ class ManageRegistrationTokensTestCase(unittest.HomeserverTestCase): ] def prepare(self, reactor, clock, hs): + self.store = hs.get_datastore() self.admin_user = self.register_user("admin", "pass", admin=True) self.admin_user_tok = self.login("admin", "pass") @@ -34,15 +38,25 @@ def prepare(self, reactor, clock, hs): self.url = "/_synapse/admin/v1/registration_tokens" - def _new_token(self, data): + def _new_token(self, **kwargs): """Helper function to create a token.""" - channel = self.make_request( - "POST", - self.url + "/new", - data, - access_token=self.admin_user_tok, + token = kwargs.get( + "token", + "".join(random.choices(string.ascii_letters, k=8)), + ) + self.get_success( + self.store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": kwargs.get("uses_allowed", None), + "pending": kwargs.get("pending", 0), + "completed": kwargs.get("completed", 0), + "expiry_time": kwargs.get("expiry_time", None), + }, + ) ) - return channel.json_body["token"] + return token # CREATION @@ -84,7 +98,7 @@ def test_create_specifying_fields(self): data = { "token": "abcd", "uses_allowed": 1, - "expiry_time": 1626427432000, + "expiry_time": self.clock.time_msec() + 1000000, } channel = self.make_request( @@ -97,7 +111,7 @@ def test_create_specifying_fields(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["token"], "abcd") self.assertEqual(channel.json_body["uses_allowed"], 1) - self.assertEqual(channel.json_body["expiry_time"], 1626427432000) + self.assertEqual(channel.json_body["expiry_time"], data["expiry_time"]) self.assertEqual(channel.json_body["pending"], 0) self.assertEqual(channel.json_body["completed"], 0) @@ -175,6 +189,112 @@ def test_create_token_already_exists(self): self.assertEqual(400, int(channel2.result["code"]), msg=channel2.result["body"]) self.assertEqual(channel2.json_body["errcode"], Codes.INVALID_PARAM) + def test_create_uses_allowed(self): + """Check you can only create a token with good values for uses_allowed.""" + # Should work with 0 (token is invalid from the start) + channel = self.make_request( + "POST", + self.url + "/new", + {"uses_allowed": 0}, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["uses_allowed"], 0) + + # Should fail with negative integer + channel = self.make_request( + "POST", + self.url + "/new", + {"uses_allowed": -5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with float + channel = self.make_request( + "POST", + self.url + "/new", + {"uses_allowed": 1.5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + def test_create_expiry_time(self): + """Check you can't create a token with an invalid expiry_time.""" + # Should fail with a time in the past + channel = self.make_request( + "POST", + self.url + "/new", + {"expiry_time": self.clock.time_msec() - 10000}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with float + channel = self.make_request( + "POST", + self.url + "/new", + {"expiry_time": self.clock.time_msec() + 1000000.5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + def test_create_length(self): + """Check you can only generate a token with a valid length.""" + # Should work with 64 + channel = self.make_request( + "POST", + self.url + "/new", + {"length": 64}, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(len(channel.json_body["token"]), 64) + + # Should fail with 0 + channel = self.make_request( + "POST", + self.url + "/new", + {"length": 0}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with a negative integer + channel = self.make_request( + "POST", + self.url + "/new", + {"length": -5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with a float + channel = self.make_request( + "POST", + self.url + "/new", + {"length": 8.5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with 65 + channel = self.make_request( + "POST", + self.url + "/new", + {"length": 65}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + # UPDATING def test_update_no_auth(self): @@ -213,43 +333,119 @@ def test_update_non_existent(self): def test_update_uses_allowed(self): """Test updating just uses_allowed.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() + # Should succeed with 1 channel = self.make_request( "PUT", self.url + "/" + token, {"uses_allowed": 1}, access_token=self.admin_user_tok, ) - self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["uses_allowed"], 1) self.assertIsNone(channel.json_body["expiry_time"]) + # Should succeed with 0 (makes token invalid) + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"uses_allowed": 0}, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["uses_allowed"], 0) + self.assertIsNone(channel.json_body["expiry_time"]) + + # Should succeed with null + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"uses_allowed": None}, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertIsNone(channel.json_body["uses_allowed"]) + self.assertIsNone(channel.json_body["expiry_time"]) + + # Should fail with a float + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"uses_allowed": 1.5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail with a negative integer + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"uses_allowed": -5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + def test_update_expiry_time(self): """Test updating just expiry_time.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() + new_expiry_time = self.clock.time_msec() + 1000000 + # Should succeed with a time in the future channel = self.make_request( "PUT", self.url + "/" + token, - {"expiry_time": 1626430124000}, + {"expiry_time": new_expiry_time}, access_token=self.admin_user_tok, ) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["expiry_time"], new_expiry_time) + self.assertIsNone(channel.json_body["uses_allowed"]) + # Should succeed with null + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"expiry_time": None}, + access_token=self.admin_user_tok, + ) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(channel.json_body["expiry_time"], 1626430124000) + self.assertIsNone(channel.json_body["expiry_time"]) self.assertIsNone(channel.json_body["uses_allowed"]) + # Should fail with a time in the past + past_time = self.clock.time_msec() - 10000 + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"expiry_time": past_time}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + + # Should fail a float + channel = self.make_request( + "PUT", + self.url + "/" + token, + {"expiry_time": new_expiry_time + 0.5}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + def test_update_both(self): """Test updating both uses_allowed and expiry_time.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() + new_expiry_time = self.clock.time_msec() + 1000000 data = { "uses_allowed": 1, - "expiry_time": 1626430124000, + "expiry_time": new_expiry_time, } channel = self.make_request( @@ -261,12 +457,12 @@ def test_update_both(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["uses_allowed"], 1) - self.assertEqual(channel.json_body["expiry_time"], 1626430124000) + self.assertEqual(channel.json_body["expiry_time"], new_expiry_time) def test_update_invalid_type(self): """Test using invalid types doesn't work.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() data = { "uses_allowed": False, @@ -321,7 +517,7 @@ def test_delete_non_existent(self): def test_delete(self): """Test deleting a token.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() channel = self.make_request( "DELETE", @@ -370,7 +566,7 @@ def test_get_non_existent(self): def test_get(self): """Test getting a token.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() channel = self.make_request( "GET", @@ -408,7 +604,7 @@ def test_list_requester_not_admin(self): def test_list_all(self): """Test listing all tokens.""" # Create new token using default values - token = self._new_token({}) + token = self._new_token() channel = self.make_request( "GET", @@ -437,34 +633,32 @@ def test_list_invalid_query_parameter(self): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) - def test_list_valid(self): - """Test listing just valid tokens.""" + def _test_list_query_parameter(self, valid): + """Helper used to test both valid=true and valid=false.""" + # Create 2 valid and 2 invalid tokens. now = self.hs.get_clock().time_msec() # Create always valid token - token_valid_1 = self._new_token({}) + valid1 = self._new_token() # Create token that hasn't been used up - token_valid_2 = self._new_token({"uses_allowed": 1}) + valid2 = self._new_token(uses_allowed=1) # Create token that has expired - self._new_token({"expiry_time": now - 10000}) + invalid1 = self._new_token(expiry_time=now - 10000) # Create token that has been used up but hasn't expired - # Can't use API because it doesn't allow changing pending or completed - store = self.hs.get_datastore() - self.get_success( - store.db_pool.simple_insert( - "registration_tokens", - { - "token": "invalid_2", - "uses_allowed": 2, - "pending": 1, - "completed": 1, - "expiry_time": now + 1000000, - }, - ) + invalid2 = self._new_token( + uses_allowed=2, + pending=1, + completed=1, + expiry_time=now + 1000000, ) + if valid == "true": + tokens = [valid1, valid2] + else: + tokens = [invalid1, invalid2] + channel = self.make_request( "GET", - self.url + "?valid=true", + self.url + "?valid=" + valid, {}, access_token=self.admin_user_tok, ) @@ -473,45 +667,13 @@ def test_list_valid(self): self.assertEqual(len(channel.json_body["registration_tokens"]), 2) token_info_1 = channel.json_body["registration_tokens"][0] token_info_2 = channel.json_body["registration_tokens"][1] - self.assertIn(token_info_1["token"], (token_valid_1, token_valid_2)) - self.assertIn(token_info_2["token"], (token_valid_1, token_valid_2)) + self.assertIn(token_info_1["token"], tokens) + self.assertIn(token_info_2["token"], tokens) + + def test_list_valid(self): + """Test listing just valid tokens.""" + self._test_list_query_parameter(valid="true") def test_list_invalid(self): """Test listing just invalid tokens.""" - now = self.hs.get_clock().time_msec() - # Create always valid token - self._new_token({}) - # Create token that hasn't been used up - self._new_token({"uses_allowed": 1}) - # Create token that has expired - token_invalid_1 = self._new_token({"expiry_time": now - 10000}) - # Create token that has been used up but hasn't expired - # Can't use API because it doesn't allow changing pending or completed - token_invalid_2 = "invalid_2" - store = self.hs.get_datastore() - self.get_success( - store.db_pool.simple_insert( - "registration_tokens", - { - "token": token_invalid_2, - "uses_allowed": 2, - "pending": 1, - "completed": 1, - "expiry_time": now + 1000000, - }, - ) - ) - - channel = self.make_request( - "GET", - self.url + "?valid=false", - {}, - access_token=self.admin_user_tok, - ) - - self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(len(channel.json_body["registration_tokens"]), 2) - token_info_1 = channel.json_body["registration_tokens"][0] - token_info_2 = channel.json_body["registration_tokens"][1] - self.assertIn(token_info_1["token"], (token_invalid_1, token_invalid_2)) - self.assertIn(token_info_2["token"], (token_invalid_1, token_invalid_2)) + self._test_list_query_parameter(valid="false") From 5d5bdef8d73881ba5b466c876737d446a026a19a Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 20 Jul 2021 16:19:25 +0100 Subject: [PATCH 25/52] Add space to SQL query Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/storage/databases/main/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 1056b092c5ea..75c7ffbeb3b2 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1877,7 +1877,7 @@ def select_registration_tokens_txn(txn, now: int, valid: Optional[bool]): # Select valid tokens only sql = ( "SELECT * FROM registration_tokens WHERE " - "(uses_allowed > pending + completed OR uses_allowed IS NULL)" + "(uses_allowed > pending + completed OR uses_allowed IS NULL) " "AND (expiry_time > ? OR expiry_time IS NULL)" ) txn.execute(sql, [now]) From b61c7f697284d0860b8a1c1eabe1c130c5b81c9e Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 20 Jul 2021 16:21:22 +0100 Subject: [PATCH 26/52] Fix SQL query for invalid tokens If expiry_time == now, by the time the token is returned the token will have expired. Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/storage/databases/main/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 75c7ffbeb3b2..7d2eb81fcce5 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1886,7 +1886,7 @@ def select_registration_tokens_txn(txn, now: int, valid: Optional[bool]): # Select invalid tokens only sql = ( "SELECT * FROM registration_tokens WHERE " - "uses_allowed <= pending + completed OR expiry_time < ?" + "uses_allowed <= pending + completed OR expiry_time <= ?" ) txn.execute(sql, [now]) From e7495e61a9a50ee49ac9e8259094444b1bf05b55 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 22 Jul 2021 14:29:52 +0100 Subject: [PATCH 27/52] Decrease pending when UIA session expires Signed-off-by: Callum Brown --- synapse/handlers/ui_auth/checkers.py | 7 +- synapse/rest/client/v2_alpha/register.py | 7 ++ synapse/storage/databases/main/ui_auth.py | 43 +++++++++++ tests/rest/client/v2_alpha/test_register.py | 86 +++++++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 4de0fe4137dd..e53ae8609a32 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -278,7 +278,7 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: 400, "Registration token has changed", Codes.INVALID_PARAM ) else: - return True + return token if await self.store.registration_token_is_valid(token): # Increment pending counter, so that if token has limited uses it @@ -291,7 +291,10 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: UIAuthSessionDataConstants.REGISTRATION_TOKEN, token, ) - return True + # The token will be stored as the result of the authentication stage + # in ui_auth_sessions_credentials. This allows the pending counter + # for tokens to be decremented when expired sessions are deleted. + return token else: raise LoginError( 401, "Invalid registration token", errcode=Codes.UNAUTHORIZED diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 31ffc563d2f1..cee7bb477d54 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -713,6 +713,13 @@ async def on_POST(self, request): if registration_token: # Increment the `completed` counter for the token await self.store.use_registration_token(registration_token) + # Indicate that the token has been successfully used so that + # pending is not decremented again when expiring old UIA sessions. + await self.store.mark_ui_auth_stage_complete( + session_id, + LoginType.REGISTRATION_TOKEN, + True, + ) await self.registration_handler.post_registration_actions( user_id=registered_user_id, diff --git a/synapse/storage/databases/main/ui_auth.py b/synapse/storage/databases/main/ui_auth.py index 22c05cdde7df..36297cdf130e 100644 --- a/synapse/storage/databases/main/ui_auth.py +++ b/synapse/storage/databases/main/ui_auth.py @@ -15,6 +15,7 @@ import attr +from synapse.api.constants import LoginType from synapse.api.errors import StoreError from synapse.storage._base import SQLBaseStore, db_to_json from synapse.storage.database import LoggingTransaction @@ -329,6 +330,48 @@ def _delete_old_ui_auth_sessions_txn( keyvalues={}, ) + # If a registration token was used, decrement the pending counter + # before deleting the session. + rows = self.db_pool.simple_select_many_txn( + txn, + table="ui_auth_sessions_credentials", + column="session_id", + iterable=session_ids, + keyvalues={"stage_type": LoginType.REGISTRATION_TOKEN}, + retcols=["result"], + ) + + # Get the tokens used and how much pending needs to be decremented by. + token_counts: Dict[str, int] = {} + for r in rows: + # If registration was successfully completed, the result of the + # registration token stage for that session will be True. + # If a token was used to authenticate, but registration was + # never completed, the result will be the token used. + token = db_to_json(r["result"]) + if isinstance(token, str): + token_counts[token] = token_counts.get(token, 0) + 1 + + # Update the `pending` counters. + if len(token_counts) > 0: + token_rows = self.db_pool.simple_select_many_txn( + txn, + table="registration_tokens", + column="token", + iterable=list(token_counts.keys()), + keyvalues={}, + retcols=["token", "pending"], + ) + for token_row in token_rows: + token = token_row["token"] + new_pending = token_row["pending"] - token_counts[token] + self.db_pool.simple_update_one_txn( + txn, + table="registration_tokens", + keyvalues={"token": token}, + updatevalues={"pending": new_pending}, + ) + # Delete the corresponding completed credentials. self.db_pool.simple_delete_many_txn( txn, diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index a53bec147b4a..89a3201c51cf 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -25,6 +25,7 @@ from synapse.appservice import ApplicationService from synapse.rest.client.v1 import login, logout from synapse.rest.client.v2_alpha import account, account_validity, register, sync +from synapse.storage._base import db_to_json from tests import unittest from tests.unittest import override_config @@ -435,6 +436,91 @@ def test_POST_registration_token_expiry(self): completed = channel.json_body["completed"] self.assertCountEqual([LoginType.REGISTRATION_TOKEN], completed) + @override_config({"registration_requires_token": True}) + def test_POST_registration_token_session_expiry(self): + token = "abcd" + store = self.hs.get_datastore() + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": None, + "pending": 0, + "completed": 0, + "expiry_time": None, + }, + ) + ) + + # Do 2 requests without auth to get two session IDs + params1 = {"username": "bert", "password": "monkey"} + params2 = {"username": "ernie", "password": "monkey"} + channel1 = self.make_request(b"POST", self.url, json.dumps(params1)) + session1 = channel1.json_body["session"] + channel2 = self.make_request(b"POST", self.url, json.dumps(params2)) + session2 = channel2.json_body["session"] + + # Use token with both sessions + params1["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session1, + } + self.make_request(b"POST", self.url, json.dumps(params1)) + + params2["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session2, + } + self.make_request(b"POST", self.url, json.dumps(params2)) + + # Complete registration with session1 + params1["auth"]["type"] = LoginType.DUMMY + self.make_request(b"POST", self.url, json.dumps(params1)) + + # Check `result` of registration token stage for session1 is `True` + result1 = self.get_success( + store.db_pool.simple_select_one_onecol( + "ui_auth_sessions_credentials", + keyvalues={ + "session_id": session1, + "stage_type": LoginType.REGISTRATION_TOKEN, + }, + retcol="result", + ) + ) + self.assertTrue(db_to_json(result1)) + + # Check `result` for session2 is the token used + result2 = self.get_success( + store.db_pool.simple_select_one_onecol( + "ui_auth_sessions_credentials", + keyvalues={ + "session_id": session2, + "stage_type": LoginType.REGISTRATION_TOKEN, + }, + retcol="result", + ) + ) + self.assertEquals(db_to_json(result2), token) + + # Delete both sessions (mimics expiry) + self.get_success( + store.delete_old_ui_auth_sessions(self.hs.get_clock().time_msec()) + ) + + # Check pending is now 0 + pending = self.get_success( + store.db_pool.simple_select_one_onecol( + "registration_tokens", + keyvalues={"token": token}, + retcol="pending", + ) + ) + self.assertEquals(pending, 0) + def test_advertised_flows(self): channel = self.make_request(b"POST", self.url, b"{}") self.assertEquals(channel.result["code"], b"401", channel.result) From 39d24d267a786f4f7db03cb92b2ba4dfa7eead37 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 23 Jul 2021 14:58:16 +0100 Subject: [PATCH 28/52] Add type to test argument Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- tests/rest/admin/test_registration_tokens.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/admin/test_registration_tokens.py b/tests/rest/admin/test_registration_tokens.py index a74be6cde3d1..2519f4cd40ed 100644 --- a/tests/rest/admin/test_registration_tokens.py +++ b/tests/rest/admin/test_registration_tokens.py @@ -633,7 +633,7 @@ def test_list_invalid_query_parameter(self): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) - def _test_list_query_parameter(self, valid): + def _test_list_query_parameter(self, valid: str): """Helper used to test both valid=true and valid=false.""" # Create 2 valid and 2 invalid tokens. now = self.hs.get_clock().time_msec() From 70cc9d23674988e8b8cad0d0e73711e688e464a4 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 23 Jul 2021 15:11:54 +0100 Subject: [PATCH 29/52] Add test for session expiry with deleted token Signed-off-by: Callum Brown --- tests/rest/client/v2_alpha/test_register.py | 50 +++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 89a3201c51cf..d9519443c659 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -438,6 +438,7 @@ def test_POST_registration_token_expiry(self): @override_config({"registration_requires_token": True}) def test_POST_registration_token_session_expiry(self): + """Test `pending` is decremented when an uncompleted session expires.""" token = "abcd" store = self.hs.get_datastore() self.get_success( @@ -521,6 +522,55 @@ def test_POST_registration_token_session_expiry(self): ) self.assertEquals(pending, 0) + @override_config({"registration_requires_token": True}) + def test_POST_registration_token_session_expiry_deleted_token(self): + """Test session expiry doesn't break when the token is deleted. + + 1. Start but don't complete UIA with a registration token + 2. Delete the token from the database + 3. Expire the session + """ + token = "abcd" + store = self.hs.get_datastore() + self.get_success( + store.db_pool.simple_insert( + "registration_tokens", + { + "token": token, + "uses_allowed": None, + "pending": 0, + "completed": 0, + "expiry_time": None, + }, + ) + ) + + # Do request without auth to get a session ID + params = {"username": "kermit", "password": "monkey"} + channel = self.make_request(b"POST", self.url, json.dumps(params)) + session = channel.json_body["session"] + + # Use token + params["auth"] = { + "type": LoginType.REGISTRATION_TOKEN, + "token": token, + "session": session, + } + self.make_request(b"POST", self.url, json.dumps(params)) + + # Delete token + result1 = self.get_success( + store.db_pool.simple_delete_one( + "registration_tokens", + keyvalues={"token": token}, + ) + ) + + # Delete session (mimics expiry) + self.get_success( + store.delete_old_ui_auth_sessions(self.hs.get_clock().time_msec()) + ) + def test_advertised_flows(self): channel = self.make_request(b"POST", self.url, b"{}") self.assertEquals(channel.result["code"], b"401", channel.result) From 09f65726a9fffdd501f40cce81bcccddea1d1b85 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 14:01:17 +0100 Subject: [PATCH 30/52] Use f-strings rather than str.format() Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 8 ++++---- synapse/rest/client/v2_alpha/auth.py | 8 ++------ synapse/rest/client/v2_alpha/register.py | 2 +- tests/rest/client/v2_alpha/test_register.py | 10 +++++----- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 0782f0b85b86..5a6ba2eb02e6 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -180,7 +180,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: if not res: # Creation failed, probably the token already exists raise SynapseError( - 400, "Token already exists: {}".format(token), Codes.INVALID_PARAM + 400, f"Token already exists: {token}", Codes.INVALID_PARAM ) resp = { @@ -256,7 +256,7 @@ async def on_GET(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # If no result return a 404 if token_info is None: - raise NotFoundError("No such registration token: {}".format(token)) + raise NotFoundError(f"No such registration token: {token}") return 200, token_info @@ -296,7 +296,7 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # If no result return a 404 if token_info is None: - raise NotFoundError("No such registration token: {}".format(token)) + raise NotFoundError("No such registration token: {token}") return 200, token_info @@ -309,4 +309,4 @@ async def on_DELETE( if await self.store.delete_registration_token(token): return 200, {} else: - raise NotFoundError("No such registration token: {}".format(token)) + raise NotFoundError("No such registration token: {token}") diff --git a/synapse/rest/client/v2_alpha/auth.py b/synapse/rest/client/v2_alpha/auth.py index 4b3c3ad7bc69..62318b364fb9 100644 --- a/synapse/rest/client/v2_alpha/auth.py +++ b/synapse/rest/client/v2_alpha/auth.py @@ -78,9 +78,7 @@ async def on_GET(self, request, stagetype): elif stagetype == LoginType.REGISTRATION_TOKEN: html = self.registration_token_template.render( session=session, - myurl="{}/r0/auth/{}/fallback/web".format( - CLIENT_API_PREFIX, LoginType.REGISTRATION_TOKEN - ), + myurl=f"{CLIENT_API_PREFIX}/r0/auth/{LoginType.REGISTRATION_TOKEN}/fallback/web", ) else: @@ -156,9 +154,7 @@ async def on_POST(self, request, stagetype): else: html = self.registration_token_template.render( session=session, - myurl="{}/r0/auth/{}/fallback/web".format( - CLIENT_API_PREFIX, LoginType.REGISTRATION_TOKEN - ), + myurl=f"{CLIENT_API_PREFIX}/r0/auth/{LoginType.REGISTRATION_TOKEN}/fallback/web", ) else: diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index cee7bb477d54..e174feed242c 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -380,7 +380,7 @@ async def on_GET(self, request): class RegistrationTokenValidityRestServlet(RestServlet): PATTERNS = client_patterns( - "/org.matrix.msc3231/register/{}/validity".format(LoginType.REGISTRATION_TOKEN), + f"/org.matrix.msc3231/register/{LoginType.REGISTRATION_TOKEN}/validity", unstable=True, ) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index d9519443c659..8015e50f8e59 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -262,7 +262,7 @@ def test_POST_registration_requires_token(self): request_data = json.dumps(params) channel = self.make_request(b"POST", self.url, request_data) det_data = { - "user_id": "@{}:{}".format(username, self.hs.hostname), + "user_id": f"@{username}:{self.hs.hostname}", "home_server": self.hs.hostname, "device_id": device_id, } @@ -1138,7 +1138,7 @@ def test_GET_token_valid(self): channel = self.make_request( b"GET", - self.url + "?token={}".format(token), + f"{self.url}?token={token}", ) self.assertEquals(channel.result["code"], b"200", channel.result) self.assertEquals(channel.json_body["valid"], True) @@ -1147,7 +1147,7 @@ def test_GET_token_invalid(self): token = "1234" channel = self.make_request( b"GET", - self.url + "?token={}".format(token), + f"{self.url}?token={token}", ) self.assertEquals(channel.result["code"], b"200", channel.result) self.assertEquals(channel.json_body["valid"], False) @@ -1161,7 +1161,7 @@ def test_GET_ratelimiting(self): for i in range(0, 6): channel = self.make_request( b"GET", - self.url + "?token={}".format(token), + f"{self.url}?token={token}", ) if i == 5: @@ -1174,6 +1174,6 @@ def test_GET_ratelimiting(self): channel = self.make_request( b"GET", - self.url + "?token={}".format(token), + f"{self.url}?token={token}", ) self.assertEquals(channel.result["code"], b"200", channel.result) From 36adec4969b4f033c26804bffa5e626874474e91 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 14:03:14 +0100 Subject: [PATCH 31/52] Update docs/usage/administration/admin_api/registration_tokens.md Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- docs/usage/administration/admin_api/registration_tokens.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/usage/administration/admin_api/registration_tokens.md b/docs/usage/administration/admin_api/registration_tokens.md index 49b2a8935741..95bfb8bb9dd2 100644 --- a/docs/usage/administration/admin_api/registration_tokens.md +++ b/docs/usage/administration/admin_api/registration_tokens.md @@ -17,7 +17,8 @@ These objects have the following fields: - `pending`: The number of pending uses the token has. When someone uses the token to authenticate themselves, the pending counter is incremented so that the token is not used more than the permitted number of times. - When the person completes registration the pending counter is decremented. + When the person completes registration the pending counter is decremented, + and the completed counter is incremented. - `completed`: The number of times the token has been used to successfully complete a registration. - `expiry_time`: The latest time the token is valid. Given as the number of From 7e539f51eeff90fd4a72891ca7ec308a2803eb83 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 14:07:57 +0100 Subject: [PATCH 32/52] Use more descriptive name Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 5a6ba2eb02e6..d0616169011d 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -264,9 +264,9 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi """Update a registration token.""" await assert_requester_is_admin(self.auth, request) body = parse_json_object_from_request(request) - updatevalues = {} + new_attributes = {} - # Only add uses_allowed to updatevalues if it is present and valid + # Only add uses_allowed to new_attributes if it is present and valid if "uses_allowed" in body: ua = body["uses_allowed"] if not (ua is None or (isinstance(ua, int) and ua >= 0)): @@ -275,7 +275,7 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi "uses_allowed must be a non-negative integer or null", Codes.INVALID_PARAM, ) - updatevalues["uses_allowed"] = ua + new_attributes["uses_allowed"] = ua if "expiry_time" in body: et = body["expiry_time"] @@ -287,12 +287,12 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi raise SynapseError( 400, "expiry_time must not be in the past", Codes.INVALID_PARAM ) - updatevalues["expiry_time"] = et + new_attributes["expiry_time"] = et - if len(updatevalues) == 0: + if len(new_attributes) == 0: raise SynapseError(400, "Nothing to update", Codes.MISSING_PARAM) - token_info = await self.store.update_registration_token(token, updatevalues) + token_info = await self.store.update_registration_token(token, new_attributes) # If no result return a 404 if token_info is None: From 1cf29c941b54468f2501714c2c969547e245b69b Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 14:45:12 +0100 Subject: [PATCH 33/52] Return 200 when nothing to update When a request to update a registration token wouldn't actually change anything, return 200 OK rather than an error. Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index d0616169011d..7c6f5f387de1 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -290,9 +290,10 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi new_attributes["expiry_time"] = et if len(new_attributes) == 0: - raise SynapseError(400, "Nothing to update", Codes.MISSING_PARAM) - - token_info = await self.store.update_registration_token(token, new_attributes) + # Nothing to update, get token info to return + token_info = await self.store.get_one_registration_token(token) + else: + token_info = await self.store.update_registration_token(token, new_attributes) # If no result return a 404 if token_info is None: From 7f9efcd6e3e2db83335e7bd77cec1d33732e40fd Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 14:52:56 +0100 Subject: [PATCH 34/52] Remove unneeded else and add missing f Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 7c6f5f387de1..74acf16bdaaa 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -297,7 +297,7 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # If no result return a 404 if token_info is None: - raise NotFoundError("No such registration token: {token}") + raise NotFoundError(f"No such registration token: {token}") return 200, token_info @@ -309,5 +309,5 @@ async def on_DELETE( if await self.store.delete_registration_token(token): return 200, {} - else: - raise NotFoundError("No such registration token: {token}") + + raise NotFoundError(f"No such registration token: {token}") From e9435f8f6c9ba1497f689e3963c9c005c1d71dfc Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Tue, 27 Jul 2021 15:09:22 +0100 Subject: [PATCH 35/52] Run linter Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 4 +++- tests/rest/client/v2_alpha/test_register.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 74acf16bdaaa..4c1a6f43a8df 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -293,7 +293,9 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # Nothing to update, get token info to return token_info = await self.store.get_one_registration_token(token) else: - token_info = await self.store.update_registration_token(token, new_attributes) + token_info = await self.store.update_registration_token( + token, new_attributes + ) # If no result return a 404 if token_info is None: diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 8015e50f8e59..f8b454af18d5 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -559,7 +559,7 @@ def test_POST_registration_token_session_expiry_deleted_token(self): self.make_request(b"POST", self.url, json.dumps(params)) # Delete token - result1 = self.get_success( + self.get_success( store.db_pool.simple_delete_one( "registration_tokens", keyvalues={"token": token}, From f6e483157fafa202df762ea3bf91e5018532ae8b Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 12 Aug 2021 18:18:54 +0100 Subject: [PATCH 36/52] Add uses_allowed to updating example in docstring Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index 4c1a6f43a8df..5be9438c6349 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -218,6 +218,7 @@ class RegistrationTokenRestServlet(RestServlet): PUT /_synapse/admin/v1/registration_tokens/defg { + "uses_allowed": 5, "expiry_time": 4781243146000 } @@ -225,7 +226,7 @@ class RegistrationTokenRestServlet(RestServlet): { "token": "defg", - "uses_allowed": 1, + "uses_allowed": 5, "pending": 0, "completed": 0, "expiry_time": 4781243146000 From 7208760c808693f3255a0452d72fa67924eedcf5 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 12 Aug 2021 18:21:19 +0100 Subject: [PATCH 37/52] Add return values to docstring Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/storage/databases/main/registration.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 7d2eb81fcce5..6c8b26846922 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1765,6 +1765,8 @@ async def registration_token_is_valid(self, token: str) -> bool: Args: token: The registration token to be checked + Returns: + True if the token is valid, False otherwise. """ res = await self.db_pool.simple_select_one( "registration_tokens", From 47b883721afef93238293a7351684bb194f2ca59 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 12 Aug 2021 18:47:47 +0100 Subject: [PATCH 38/52] Add docstring to validity checking endpoint and set releases=() to prevent endpoint being available on /_matrix/client/r0/... Signed-off-by: Callum Brown --- synapse/rest/client/v2_alpha/register.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index e174feed242c..b4218c6aacc0 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -379,8 +379,22 @@ async def on_GET(self, request): class RegistrationTokenValidityRestServlet(RestServlet): + """Check the validity of a registration token. + + Example: + + GET /_matrix/client/unstable/org.matrix.msc3231/register/org.matrix.msc3231.login.registration_token/validity?token=abcd + + 200 OK + + { + "valid": true + } + """ + PATTERNS = client_patterns( f"/org.matrix.msc3231/register/{LoginType.REGISTRATION_TOKEN}/validity", + releases=(), unstable=True, ) From b76099e682de74e0526f9d0cc41e44067c266797 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 12 Aug 2021 20:31:20 +0100 Subject: [PATCH 39/52] Move functions into RegistrationWorkerStore /register is supported by workers so make sure worker processes can call registration token related store functions. Signed-off-by: Callum Brown --- .../storage/databases/main/registration.py | 512 +++++++++--------- 1 file changed, 256 insertions(+), 256 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 6c8b26846922..b404daa77fc8 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1072,6 +1072,262 @@ async def update_access_token_last_validated(self, token_id: int) -> None: desc="update_access_token_last_validated", ) + async def registration_token_is_valid(self, token: str) -> bool: + """Checks if a token can be used to authenticate a registration. + + Args: + token: The registration token to be checked + Returns: + True if the token is valid, False otherwise. + """ + res = await self.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["uses_allowed", "pending", "completed", "expiry_time"], + allow_none=True, + ) + + # Check if the token exists + if res is None: + return False + + # Check if the token has expired + now = self._clock.time_msec() + if res["expiry_time"] and res["expiry_time"] < now: + return False + + # Check if the token has been used up + if ( + res["uses_allowed"] + and res["pending"] + res["completed"] >= res["uses_allowed"] + ): + return False + + # Otherwise, the token is valid + return True + + async def set_registration_token_pending(self, token: str) -> None: + """Increment the pending registrations counter for a token. + + Args: + token: The registration token pending use + """ + + def _set_registration_token_pending_txn(txn): + pending = self.db_pool.simple_select_one_onecol_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcol="pending", + ) + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues={"pending": pending + 1}, + ) + + return await self.db_pool.runInteraction( + "set_registration_token_pending", _set_registration_token_pending_txn + ) + + async def use_registration_token(self, token: str) -> None: + """Complete a use of the given registration token. + + The `pending` counter will be decremented, and the `completed` + counter will be incremented. + + Args: + token: The registration token to be 'used' + """ + + def _use_registration_token_txn(txn): + # Normally, res is Optional[Dict[str, Any]]. + # Override type because the return type is only optional if + # allow_none is True, and we don't want mypy throwing errors + # about None not being indexable. + res: Dict[str, Any] = self.db_pool.simple_select_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcols=["pending", "completed"], + ) # type: ignore + + # Decrement pending and increment completed + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues={ + "completed": res["completed"] + 1, + "pending": res["pending"] - 1, + }, + ) + + return await self.db_pool.runInteraction( + "use_registration_token", _use_registration_token_txn + ) + + async def get_registration_tokens( + self, valid: Optional[bool] = None + ) -> List[Dict[str, Any]]: + """List all registration tokens. Used by the admin API. + + Args: + valid: If True, only valid tokens are returned. + If False, only invalid tokens are returned. + Default is None: return all tokens regardless of validity. + + Returns: + A list of dicts, each containing details of a token. + """ + + def select_registration_tokens_txn(txn, now: int, valid: Optional[bool]): + if valid is None: + # Return all tokens regardless of validity + txn.execute("SELECT * FROM registration_tokens") + + elif valid: + # Select valid tokens only + sql = ( + "SELECT * FROM registration_tokens WHERE " + "(uses_allowed > pending + completed OR uses_allowed IS NULL) " + "AND (expiry_time > ? OR expiry_time IS NULL)" + ) + txn.execute(sql, [now]) + + else: + # Select invalid tokens only + sql = ( + "SELECT * FROM registration_tokens WHERE " + "uses_allowed <= pending + completed OR expiry_time <= ?" + ) + txn.execute(sql, [now]) + + return self.db_pool.cursor_to_dict(txn) + + return await self.db_pool.runInteraction( + "select_registration_tokens", + select_registration_tokens_txn, + self._clock.time_msec(), + valid, + ) + + async def get_one_registration_token(self, token: str) -> Optional[Dict[str, Any]]: + """Get info about the given registration token. Used by the admin API. + + Args: + token: The token to retrieve information about. + + Returns: + A dict, or None if token doesn't exist. + """ + return await self.db_pool.simple_select_one( + "registration_tokens", + keyvalues={"token": token}, + retcols=["token", "uses_allowed", "pending", "completed", "expiry_time"], + allow_none=True, + desc="get_one_registration_token", + ) + + async def create_registration_token( + self, token: str, uses_allowed: Optional[int], expiry_time: Optional[int] + ) -> bool: + """Create a new registration token. Used by the admin API. + + Args: + token: The token to create. + uses_allowed: The number of times the token can be used to complete + a registration before it becomes invalid. A value of None indicates + unlimited uses. + expiry_time: The latest time the token is valid. Given as the + number of milliseconds since 1970-01-01 00:00:00 UTC. A value of + None indicates that the token does not expire. + + Returns: + Whether the row was inserted or not. + """ + return await self.db_pool.simple_insert( + "registration_tokens", + values={ + "token": token, + "uses_allowed": uses_allowed, + "pending": 0, + "completed": 0, + "expiry_time": expiry_time, + }, + or_ignore=True, + desc="create_registration_token", + ) + + async def update_registration_token( + self, token: str, updatevalues: Dict[str, Optional[int]] + ) -> Optional[Dict[str, Any]]: + """Update a registration token. Used by the admin API. + + Args: + token: The token to update. + updatevalues: A dict with the fields to update. E.g.: + `{"uses_allowed": 3}` to update just uses_allowed, or + `{"uses_allowed": 3, "expiry_time": None}` to update both. + This is passed straight to simple_update_one. + + Returns: + A dict with all info about the token, or None if token doesn't exist. + """ + + def _update_registration_token_txn(txn): + try: + self.db_pool.simple_update_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + updatevalues=updatevalues, + ) + except StoreError: + # Update failed because token does not exist + return None + + # Get all info about the token so it can be sent in the response + return self.db_pool.simple_select_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcols=[ + "token", + "uses_allowed", + "pending", + "completed", + "expiry_time", + ], + allow_none=True, + ) + + return await self.db_pool.runInteraction( + "update_registration_token", _update_registration_token_txn + ) + + async def delete_registration_token(self, token: str) -> bool: + """Delete a registration token. Used by the admin API. + + Args: + token: The token to delete. + + Returns: + Whether the token was successfully deleted or not. + """ + try: + await self.db_pool.simple_delete_one( + "registration_tokens", + keyvalues={"token": token}, + desc="delete_registration_token", + ) + except StoreError: + # Deletion failed because token does not exist + return False + + return True + class RegistrationBackgroundUpdateStore(RegistrationWorkerStore): def __init__( @@ -1760,262 +2016,6 @@ def start_or_continue_validation_session_txn(txn): start_or_continue_validation_session_txn, ) - async def registration_token_is_valid(self, token: str) -> bool: - """Checks if a token can be used to authenticate a registration. - - Args: - token: The registration token to be checked - Returns: - True if the token is valid, False otherwise. - """ - res = await self.db_pool.simple_select_one( - "registration_tokens", - keyvalues={"token": token}, - retcols=["uses_allowed", "pending", "completed", "expiry_time"], - allow_none=True, - ) - - # Check if the token exists - if res is None: - return False - - # Check if the token has expired - now = self._clock.time_msec() - if res["expiry_time"] and res["expiry_time"] < now: - return False - - # Check if the token has been used up - if ( - res["uses_allowed"] - and res["pending"] + res["completed"] >= res["uses_allowed"] - ): - return False - - # Otherwise, the token is valid - return True - - async def set_registration_token_pending(self, token: str) -> None: - """Increment the pending registrations counter for a token. - - Args: - token: The registration token pending use - """ - - def _set_registration_token_pending_txn(txn): - pending = self.db_pool.simple_select_one_onecol_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - retcol="pending", - ) - self.db_pool.simple_update_one_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - updatevalues={"pending": pending + 1}, - ) - - return await self.db_pool.runInteraction( - "set_registration_token_pending", _set_registration_token_pending_txn - ) - - async def use_registration_token(self, token: str) -> None: - """Complete a use of the given registration token. - - The `pending` counter will be decremented, and the `completed` - counter will be incremented. - - Args: - token: The registration token to be 'used' - """ - - def _use_registration_token_txn(txn): - # Normally, res is Optional[Dict[str, Any]]. - # Override type because the return type is only optional if - # allow_none is True, and we don't want mypy throwing errors - # about None not being indexable. - res: Dict[str, Any] = self.db_pool.simple_select_one_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - retcols=["pending", "completed"], - ) # type: ignore - - # Decrement pending and increment completed - self.db_pool.simple_update_one_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - updatevalues={ - "completed": res["completed"] + 1, - "pending": res["pending"] - 1, - }, - ) - - return await self.db_pool.runInteraction( - "use_registration_token", _use_registration_token_txn - ) - - async def get_registration_tokens( - self, valid: Optional[bool] = None - ) -> List[Dict[str, Any]]: - """List all registration tokens. Used by the admin API. - - Args: - valid: If True, only valid tokens are returned. - If False, only invalid tokens are returned. - Default is None: return all tokens regardless of validity. - - Returns: - A list of dicts, each containing details of a token. - """ - - def select_registration_tokens_txn(txn, now: int, valid: Optional[bool]): - if valid is None: - # Return all tokens regardless of validity - txn.execute("SELECT * FROM registration_tokens") - - elif valid: - # Select valid tokens only - sql = ( - "SELECT * FROM registration_tokens WHERE " - "(uses_allowed > pending + completed OR uses_allowed IS NULL) " - "AND (expiry_time > ? OR expiry_time IS NULL)" - ) - txn.execute(sql, [now]) - - else: - # Select invalid tokens only - sql = ( - "SELECT * FROM registration_tokens WHERE " - "uses_allowed <= pending + completed OR expiry_time <= ?" - ) - txn.execute(sql, [now]) - - return self.db_pool.cursor_to_dict(txn) - - return await self.db_pool.runInteraction( - "select_registration_tokens", - select_registration_tokens_txn, - self._clock.time_msec(), - valid, - ) - - async def get_one_registration_token(self, token: str) -> Optional[Dict[str, Any]]: - """Get info about the given registration token. Used by the admin API. - - Args: - token: The token to retrieve information about. - - Returns: - A dict, or None if token doesn't exist. - """ - return await self.db_pool.simple_select_one( - "registration_tokens", - keyvalues={"token": token}, - retcols=["token", "uses_allowed", "pending", "completed", "expiry_time"], - allow_none=True, - desc="get_one_registration_token", - ) - - async def create_registration_token( - self, token: str, uses_allowed: Optional[int], expiry_time: Optional[int] - ) -> bool: - """Create a new registration token. Used by the admin API. - - Args: - token: The token to create. - uses_allowed: The number of times the token can be used to complete - a registration before it becomes invalid. A value of None indicates - unlimited uses. - expiry_time: The latest time the token is valid. Given as the - number of milliseconds since 1970-01-01 00:00:00 UTC. A value of - None indicates that the token does not expire. - - Returns: - Whether the row was inserted or not. - """ - return await self.db_pool.simple_insert( - "registration_tokens", - values={ - "token": token, - "uses_allowed": uses_allowed, - "pending": 0, - "completed": 0, - "expiry_time": expiry_time, - }, - or_ignore=True, - desc="create_registration_token", - ) - - async def update_registration_token( - self, token: str, updatevalues: Dict[str, Optional[int]] - ) -> Optional[Dict[str, Any]]: - """Update a registration token. Used by the admin API. - - Args: - token: The token to update. - updatevalues: A dict with the fields to update. E.g.: - `{"uses_allowed": 3}` to update just uses_allowed, or - `{"uses_allowed": 3, "expiry_time": None}` to update both. - This is passed straight to simple_update_one. - - Returns: - A dict with all info about the token, or None if token doesn't exist. - """ - - def _update_registration_token_txn(txn): - try: - self.db_pool.simple_update_one_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - updatevalues=updatevalues, - ) - except StoreError: - # Update failed because token does not exist - return None - - # Get all info about the token so it can be sent in the response - return self.db_pool.simple_select_one_txn( - txn, - "registration_tokens", - keyvalues={"token": token}, - retcols=[ - "token", - "uses_allowed", - "pending", - "completed", - "expiry_time", - ], - allow_none=True, - ) - - return await self.db_pool.runInteraction( - "update_registration_token", _update_registration_token_txn - ) - - async def delete_registration_token(self, token: str) -> bool: - """Delete a registration token. Used by the admin API. - - Args: - token: The token to delete. - - Returns: - Whether the token was successfully deleted or not. - """ - try: - await self.db_pool.simple_delete_one( - "registration_tokens", - keyvalues={"token": token}, - desc="delete_registration_token", - ) - except StoreError: - # Deletion failed because token does not exist - return False - - return True - def find_max_generated_user_id_localpart(cur: Cursor) -> int: """ From c6cb80bd3ca0501a3eea410d4b85e419632b62c0 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 19 Aug 2021 16:42:49 +0100 Subject: [PATCH 40/52] Add link to admin API docs in config file The link will not work until the code is merged. Signed-off-by: Callum Brown --- synapse/config/registration.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 54d88e787224..7cffdacfa5ce 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -206,7 +206,8 @@ def generate_config_section(self, generate_secrets=False, **kwargs): #enable_3pid_lookup: true # Require users to submit a token during registration. - # Tokens can be generated with the admin API (once it exists). + # Tokens can be managed using the admin API: + # https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/registration_tokens.html # Note that `enable_registration` must be set to `true`. # Disabling this option will not delete any tokens previously generated. # Defaults to false. Uncomment the following to require tokens: From ba22ffd160841e56ff9bee2309144557513e32fc Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 19 Aug 2021 16:51:57 +0100 Subject: [PATCH 41/52] Move table creation SQL to latest delta Signed-off-by: Callum Brown --- .../02create_registration_tokens.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename synapse/storage/schema/main/delta/{59/999create_registration_tokens.sql => 62/02create_registration_tokens.sql} (100%) diff --git a/synapse/storage/schema/main/delta/59/999create_registration_tokens.sql b/synapse/storage/schema/main/delta/62/02create_registration_tokens.sql similarity index 100% rename from synapse/storage/schema/main/delta/59/999create_registration_tokens.sql rename to synapse/storage/schema/main/delta/62/02create_registration_tokens.sql From c775dcee43ebf1974f5ca04f074b454e178f25df Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 19 Aug 2021 17:02:00 +0100 Subject: [PATCH 42/52] Add changelog entry Signed-off-by: Callum Brown --- changelog.d/10142.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10142.feature diff --git a/changelog.d/10142.feature b/changelog.d/10142.feature new file mode 100644 index 000000000000..5353f6269d01 --- /dev/null +++ b/changelog.d/10142.feature @@ -0,0 +1 @@ +Add support for [MSC3231 - Token authenticated registration](https://github.com/matrix-org/matrix-doc/pull/3231). Users can be required to submit a token during registration to authenticate themselves. Contributed by Callum Brown. From f327b293b8af0c6f07ae5a7ed44af03f8f85f87d Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Thu, 19 Aug 2021 17:11:07 +0100 Subject: [PATCH 43/52] Regenerate sample config Signed-off-by: Callum Brown --- docs/sample_config.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index faf4447014ea..935841dbfa37 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1176,7 +1176,8 @@ url_preview_accept_language: #enable_3pid_lookup: true # Require users to submit a token during registration. -# Tokens can be generated with the admin API (once it exists). +# Tokens can be managed using the admin API: +# https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/registration_tokens.html # Note that `enable_registration` must be set to `true`. # Disabling this option will not delete any tokens previously generated. # Defaults to false. Uncomment the following to require tokens: From c6bcae2e926be94cdad4eb40d22397dbf4e65179 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 12:40:18 +0100 Subject: [PATCH 44/52] Move table creation sql to actual newest delta Signed-off-by: Callum Brown --- .../01create_registration_tokens.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename synapse/storage/schema/main/delta/{62/02create_registration_tokens.sql => 63/01create_registration_tokens.sql} (100%) diff --git a/synapse/storage/schema/main/delta/62/02create_registration_tokens.sql b/synapse/storage/schema/main/delta/63/01create_registration_tokens.sql similarity index 100% rename from synapse/storage/schema/main/delta/62/02create_registration_tokens.sql rename to synapse/storage/schema/main/delta/63/01create_registration_tokens.sql From 01a74da6f7b42a1b146210c51aa0928602d4e85f Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 12:55:44 +0100 Subject: [PATCH 45/52] Avoid integrity error when creating tokens Check if token already exists before trying to create it Signed-off-by: Callum Brown --- .../storage/databases/main/registration.py | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 2849c68d56d9..4f09ba649cb2 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1343,8 +1343,23 @@ async def create_registration_token( Returns: Whether the row was inserted or not. """ - try: - await self.db_pool.simple_insert( + + def _create_registration_token_txn(txn): + row = self.db_pool.simple_select_one_txn( + txn, + "registration_tokens", + keyvalues={"token": token}, + retcols=["token"], + allow_none=True, + ) + + if row is not None: + raise SynapseError( + 400, f"Token already exists: {token}", Codes.INVALID_PARAM + ) + + self.db_pool.simple_insert_txn( + txn, "registration_tokens", values={ "token": token, @@ -1353,13 +1368,12 @@ async def create_registration_token( "completed": 0, "expiry_time": expiry_time, }, - desc="create_registration_token", - ) - except self.database_engine.module.IntegrityError: - raise SynapseError( - 400, f"Token already exists: {token}", Codes.INVALID_PARAM ) + return await self.db_pool.runInteraction( + "create_registration_token", _create_registration_token_txn + ) + async def update_registration_token( self, token: str, updatevalues: Dict[str, Optional[int]] ) -> Optional[Dict[str, Any]]: From 5bfc707da4e59f5d692b9fcd6c592cc82ecaf5b7 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 13:26:24 +0100 Subject: [PATCH 46/52] Fix docs, comments and variable names Signed-off-by: Callum Brown Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- .../admin_api/registration_tokens.md | 8 +++---- synapse/handlers/ui_auth/__init__.py | 2 +- synapse/handlers/ui_auth/checkers.py | 4 ++-- synapse/rest/admin/registration_tokens.py | 23 +++++++++++-------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/docs/usage/administration/admin_api/registration_tokens.md b/docs/usage/administration/admin_api/registration_tokens.md index 95bfb8bb9dd2..9a050ba1e781 100644 --- a/docs/usage/administration/admin_api/registration_tokens.md +++ b/docs/usage/administration/admin_api/registration_tokens.md @@ -5,6 +5,7 @@ registration requests, as proposed in [MSC3231](https://github.com/govynnus/matr To use it, you will need to enable the `registration_requires_token` config option, and authenticate by providing an `access_token` for a server admin: see [Admin API](../../usage/administration/admin_api). +Note that this API is still experimental; not all clients may support it yet. ## Registration token objects @@ -23,9 +24,6 @@ These objects have the following fields: complete a registration. - `expiry_time`: The latest time the token is valid. Given as the number of milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). - A more human friendly format will be provided at some point, but in the - meantime you can remove the milliseconds and use the `date` command. - For example, `date -d '@1625394937'`. ## List all tokens @@ -40,8 +38,8 @@ GET /_synapse/admin/v1/registration_tokens Optional query parameters: - `valid`: `true` or `false`. If `true`, only valid tokens are returned. - If `false`, only invalid tokens are returned. If omitted, all tokens are - returned regardless of validity. + If `false`, only tokens that have expired or have had all uses exhausted are + returned. If omitted, all tokens are returned regardless of validity. Example: diff --git a/synapse/handlers/ui_auth/__init__.py b/synapse/handlers/ui_auth/__init__.py index 54f4164a9a6f..13b0c61d2e20 100644 --- a/synapse/handlers/ui_auth/__init__.py +++ b/synapse/handlers/ui_auth/__init__.py @@ -35,7 +35,7 @@ class UIAuthSessionDataConstants: # for. REQUEST_USER_ID = "request_user_id" - # used during registration to store the registration used (if required) so that + # used during registration to store the registration token used (if required) so that: # - we can prevent a token being used twice by one session # - we can 'use up' the token after registration has successfully completed REGISTRATION_TOKEN = "org.matrix.msc3231.login.registration_token" diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 202dd31e816d..d3828dec6b3f 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -263,11 +263,11 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any: if "session" not in authdict: raise LoginError(400, "Missing UIA session", Codes.MISSING_PARAM) - # Import here to avoid a cyclic dependency + # Get these here to avoid cyclic dependencies from synapse.handlers.ui_auth import UIAuthSessionDataConstants - # Retrieve the auth handler here to avoid a cyclic dependency auth_handler = self.hs.get_auth_handler() + session = authdict["session"] token = authdict["token"] diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index ce4e1899753b..e3aff93ae17a 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -61,8 +61,10 @@ class ListRegistrationTokensRestServlet(RestServlet): ] } - The optional query parameter `valid` is used to the response to only - valid or invalid tokens. It can be `true` or `false`. + The optional query parameter `valid` can be used to filter the response. + If it is `true`, only valid tokens are returned. If it is `false`, only + tokens that have expired or have had all uses exhausted are returned. + If it is omitted, all tokens are returned regardless of validity. """ PATTERNS = admin_patterns("/registration_tokens$") @@ -262,26 +264,29 @@ async def on_PUT(self, request: SynapseRequest, token: str) -> Tuple[int, JsonDi # Only add uses_allowed to new_attributes if it is present and valid if "uses_allowed" in body: - ua = body["uses_allowed"] - if not (ua is None or (isinstance(ua, int) and ua >= 0)): + uses_allowed = body["uses_allowed"] + if not ( + uses_allowed is None + or (isinstance(uses_allowed, int) and uses_allowed >= 0) + ): raise SynapseError( 400, "uses_allowed must be a non-negative integer or null", Codes.INVALID_PARAM, ) - new_attributes["uses_allowed"] = ua + new_attributes["uses_allowed"] = uses_allowed if "expiry_time" in body: - et = body["expiry_time"] - if not isinstance(et, (int, type(None))): + expiry_time = body["expiry_time"] + if not isinstance(expiry_time, (int, type(None))): raise SynapseError( 400, "expiry_time must be an integer or null", Codes.INVALID_PARAM ) - if isinstance(et, int) and et < self.clock.time_msec(): + if isinstance(expiry_time, int) and expiry_time < self.clock.time_msec(): raise SynapseError( 400, "expiry_time must not be in the past", Codes.INVALID_PARAM ) - new_attributes["expiry_time"] = et + new_attributes["expiry_time"] = expiry_time if len(new_attributes) == 0: # Nothing to update, get token info to return From b5608c37944ef69d2a63288a97718b72880206a7 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 14:42:55 +0100 Subject: [PATCH 47/52] Try again if generated token already exists Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 29 ++++++++++++++++++- .../storage/databases/main/registration.py | 9 +++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index e3aff93ae17a..eabdcd466d3a 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -176,7 +176,34 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: 400, "expiry_time must not be in the past", Codes.INVALID_PARAM ) - await self.store.create_registration_token(token, uses_allowed, expiry_time) + created = await self.store.create_registration_token( + token, uses_allowed, expiry_time + ) + + if "token" not in body: + # The token was generated. If it could not be created because + # that token already exists, then try a few more times before + # reporting a failure. + i = 0 + while not created and i < 3: + token = "".join(random.choices(self.allowed_chars, k=length)) + created = await self.store.create_registration_token( + token, uses_allowed, expiry_time + ) + i += 1 + if not created: + raise SynapseError( + 500, + "The generated token already exists. Try again with a greater length.", + Codes.UNKNOWN, + ) + + elif not created: + # The token was specified in the request, but it already exists + # so could not be created. + raise SynapseError( + 400, f"Token already exists: {token}", Codes.INVALID_PARAM + ) resp = { "token": token, diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 4f09ba649cb2..b7df9bce22b8 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1328,7 +1328,7 @@ async def get_one_registration_token(self, token: str) -> Optional[Dict[str, Any async def create_registration_token( self, token: str, uses_allowed: Optional[int], expiry_time: Optional[int] - ) -> None: + ) -> bool: """Create a new registration token. Used by the admin API. Args: @@ -1354,9 +1354,8 @@ def _create_registration_token_txn(txn): ) if row is not None: - raise SynapseError( - 400, f"Token already exists: {token}", Codes.INVALID_PARAM - ) + # Token already exists + return False self.db_pool.simple_insert_txn( txn, @@ -1370,6 +1369,8 @@ def _create_registration_token_txn(txn): }, ) + return True + return await self.db_pool.runInteraction( "create_registration_token", _create_registration_token_txn ) From bf28876f6764e9ac820ae61cf0f7436b413cb19c Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 16:10:48 +0100 Subject: [PATCH 48/52] Let validity checking endpoint be used by workers Signed-off-by: Callum Brown --- docs/workers.md | 1 + synapse/app/generic_worker.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/workers.md b/docs/workers.md index 2e63f0345288..3121241894f2 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -236,6 +236,7 @@ expressions: # Registration/login requests ^/_matrix/client/(api/v1|r0|unstable)/login$ ^/_matrix/client/(r0|unstable)/register$ + ^/_matrix/client/unstable/org.matrix.msc3231/register/org.matrix.msc3231.login.registration_token/validity$ # Event sending requests ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/redact diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 845e6a822046..fd2626dbe1db 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -95,7 +95,10 @@ ProfileRestServlet, ) from synapse.rest.client.push_rule import PushRuleRestServlet -from synapse.rest.client.register import RegisterRestServlet +from synapse.rest.client.register import ( + RegisterRestServlet, + RegistrationTokenValidityRestServlet, +) from synapse.rest.client.sendtodevice import SendToDeviceRestServlet from synapse.rest.client.versions import VersionsRestServlet from synapse.rest.client.voip import VoipRestServlet @@ -279,6 +282,7 @@ def _listen_http(self, listener_config: ListenerConfig): resource = JsonResource(self, canonical_json=False) RegisterRestServlet(self).register(resource) + RegistrationTokenValidityRestServlet(self).register(resource) login.register_servlets(self, resource) ThreepidRestServlet(self).register(resource) DevicesRestServlet(self).register(resource) From 2e59dda0502cc8076af1cff406f1584379f45c81 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Fri, 20 Aug 2021 16:35:29 +0100 Subject: [PATCH 49/52] Document usage of `null` when updating tokens Signed-off-by: Callum Brown --- docs/usage/administration/admin_api/registration_tokens.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/usage/administration/admin_api/registration_tokens.md b/docs/usage/administration/admin_api/registration_tokens.md index 9a050ba1e781..74cb42ce6532 100644 --- a/docs/usage/administration/admin_api/registration_tokens.md +++ b/docs/usage/administration/admin_api/registration_tokens.md @@ -218,8 +218,10 @@ Path parameters: The request body must be a JSON object and can contain the following fields: - `uses_allowed`: The integer number of times the token can be used to complete a registration before it becomes invalid. + If `null` the token will have an unlimited number of uses. - `expiry_time`: The latest time the token is valid. Given as the number of milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). + If `null` the token will not expire. If a field is omitted its value is not modified. From 54867efb3a404042140f88bbba1c73055d4787b8 Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Sat, 21 Aug 2021 12:00:31 +0100 Subject: [PATCH 50/52] Simplify retrying of token generation Signed-off-by: Callum Brown --- synapse/rest/admin/registration_tokens.py | 28 ++----------- .../storage/databases/main/registration.py | 41 +++++++++++++++++++ tests/rest/admin/test_registration_tokens.py | 31 ++++++++++++++ 3 files changed, 76 insertions(+), 24 deletions(-) diff --git a/synapse/rest/admin/registration_tokens.py b/synapse/rest/admin/registration_tokens.py index eabdcd466d3a..5a1c929d85c4 100644 --- a/synapse/rest/admin/registration_tokens.py +++ b/synapse/rest/admin/registration_tokens.py @@ -13,7 +13,6 @@ # limitations under the License. import logging -import random import string from typing import TYPE_CHECKING, Tuple @@ -153,7 +152,9 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: ) # Generate token - token = "".join(random.choices(self.allowed_chars, k=length)) + token = await self.store.generate_registration_token( + length, self.allowed_chars + ) uses_allowed = body.get("uses_allowed", None) if not ( @@ -179,28 +180,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: created = await self.store.create_registration_token( token, uses_allowed, expiry_time ) - - if "token" not in body: - # The token was generated. If it could not be created because - # that token already exists, then try a few more times before - # reporting a failure. - i = 0 - while not created and i < 3: - token = "".join(random.choices(self.allowed_chars, k=length)) - created = await self.store.create_registration_token( - token, uses_allowed, expiry_time - ) - i += 1 - if not created: - raise SynapseError( - 500, - "The generated token already exists. Try again with a greater length.", - Codes.UNKNOWN, - ) - - elif not created: - # The token was specified in the request, but it already exists - # so could not be created. + if not created: raise SynapseError( 400, f"Token already exists: {token}", Codes.INVALID_PARAM ) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index b7df9bce22b8..e67bc83d6b17 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1326,6 +1326,47 @@ async def get_one_registration_token(self, token: str) -> Optional[Dict[str, Any desc="get_one_registration_token", ) + async def generate_registration_token( + self, length: int, chars: str + ) -> Optional[str]: + """Generate a random registration token. Used by the admin API. + + Args: + length: The length of the token to generate. + chars: A string of the characters allowed in the generated token. + + Returns: + The generated token. + + Raises: + SynapseError if a unique registration token could still not be + generated after a few tries. + """ + # Make a few attempts at generating a unique token of the required + # length before failing. + for _i in range(3): + # Generate token + token = "".join(random.choices(chars, k=length)) + + # Check if the token already exists + existing_token = await self.db_pool.simple_select_one_onecol( + "registration_tokens", + keyvalues={"token": token}, + retcol="token", + allow_none=True, + desc="check_if_registration_token_exists", + ) + + if existing_token is None: + # The generated token doesn't exist yet, return it + return token + + raise SynapseError( + 500, + "Unable to generate a registration token. Try again with a greater length", + Codes.UNKNOWN, + ) + async def create_registration_token( self, token: str, uses_allowed: Optional[int], expiry_time: Optional[int] ) -> bool: diff --git a/tests/rest/admin/test_registration_tokens.py b/tests/rest/admin/test_registration_tokens.py index 38ccfa0690c8..4927321e5a4f 100644 --- a/tests/rest/admin/test_registration_tokens.py +++ b/tests/rest/admin/test_registration_tokens.py @@ -189,6 +189,37 @@ def test_create_token_already_exists(self): self.assertEqual(400, int(channel2.result["code"]), msg=channel2.result["body"]) self.assertEqual(channel2.json_body["errcode"], Codes.INVALID_PARAM) + def test_create_unable_to_generate_token(self): + """Check right error is raised when server can't generate unique token.""" + # Create all possible single character tokens + tokens = [] + for c in string.ascii_letters + string.digits + "-_": + tokens.append( + { + "token": c, + "uses_allowed": None, + "pending": 0, + "completed": 0, + "expiry_time": None, + } + ) + self.get_success( + self.store.db_pool.simple_insert_many( + "registration_tokens", + tokens, + "create_all_registration_tokens", + ) + ) + + # Check creating a single character token fails with a 500 status code + channel = self.make_request( + "POST", + self.url + "/new", + {"length": 1}, + access_token=self.admin_user_tok, + ) + self.assertEqual(500, int(channel.result["code"]), msg=channel.result["body"]) + def test_create_uses_allowed(self): """Check you can only create a token with good values for uses_allowed.""" # Should work with 0 (token is invalid from the start) From 20b566cca63be707cad4383c2aa3af6f35cba9eb Mon Sep 17 00:00:00 2001 From: Callum Brown Date: Sat, 21 Aug 2021 12:06:46 +0100 Subject: [PATCH 51/52] Small additions to admin api documentation Signed-off-by: Callum Brown --- docs/usage/administration/admin_api/registration_tokens.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/usage/administration/admin_api/registration_tokens.md b/docs/usage/administration/admin_api/registration_tokens.md index 74cb42ce6532..828c0277d626 100644 --- a/docs/usage/administration/admin_api/registration_tokens.md +++ b/docs/usage/administration/admin_api/registration_tokens.md @@ -24,6 +24,8 @@ These objects have the following fields: complete a registration. - `expiry_time`: The latest time the token is valid. Given as the number of milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). + To convert this into a human-readable form you can remove the milliseconds + and use the `date` command. For example, `date -d '@1625394937'`. ## List all tokens @@ -217,7 +219,8 @@ Path parameters: The request body must be a JSON object and can contain the following fields: - `uses_allowed`: The integer number of times the token can be used to complete - a registration before it becomes invalid. + a registration before it becomes invalid. By setting `uses_allowed` to `0` + the token can be easily made invalid without deleting it. If `null` the token will have an unlimited number of uses. - `expiry_time`: The latest time the token is valid. Given as the number of milliseconds since 1970-01-01 00:00:00 UTC (the start of the Unix epoch). From 04b237aa42db6b1f7130b3b9e693f028fd8c5519 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Sat, 21 Aug 2021 21:45:08 +0100 Subject: [PATCH 52/52] Update synapse/storage/databases/main/registration.py --- synapse/storage/databases/main/registration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index e67bc83d6b17..a6517962f619 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -1363,7 +1363,7 @@ async def generate_registration_token( raise SynapseError( 500, - "Unable to generate a registration token. Try again with a greater length", + "Unable to generate a unique registration token. Try again with a greater length", Codes.UNKNOWN, )