From dc6da63e30a636133b8047c09e45d2c2716cbecb Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 11 Feb 2016 17:37:38 +0000 Subject: Enforce user_id exclusivity for AS registrations This whole set of checks is kind of an ugly mess, which I may clean up at some point, but for now let's be correct. --- synapse/handlers/register.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'synapse/handlers/register.py') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 24c850ae9b..f8959e5d82 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -60,7 +60,7 @@ class RegistrationHandler(BaseHandler): user = UserID(localpart, self.hs.hostname) user_id = user.to_string() - yield self.check_user_id_is_valid(user_id) + yield self.check_user_id_not_appservice_exclusive(user_id) users = yield self.store.get_users_by_id_case_insensitive(user_id) if users: @@ -145,7 +145,7 @@ class RegistrationHandler(BaseHandler): localpart = yield self._generate_user_id(attempts > 0) user = UserID(localpart, self.hs.hostname) user_id = user.to_string() - yield self.check_user_id_is_valid(user_id) + yield self.check_user_id_not_appservice_exclusive(user_id) if generate_token: token = self.auth_handler().generate_access_token(user_id) try: @@ -180,6 +180,11 @@ class RegistrationHandler(BaseHandler): 400, "Invalid user localpart for this application service.", errcode=Codes.EXCLUSIVE ) + + yield self.check_user_id_not_appservice_exclusive( + user_id, allowed_appservice=service + ) + token = self.auth_handler().generate_access_token(user_id) yield self.store.register( user_id=user_id, @@ -226,7 +231,7 @@ class RegistrationHandler(BaseHandler): user = UserID(localpart, self.hs.hostname) user_id = user.to_string() - yield self.check_user_id_is_valid(user_id) + yield self.check_user_id_not_appservice_exclusive(user_id) token = self.auth_handler().generate_access_token(user_id) try: yield self.store.register( @@ -278,12 +283,14 @@ class RegistrationHandler(BaseHandler): yield identity_handler.bind_threepid(c, user_id) @defer.inlineCallbacks - def check_user_id_is_valid(self, user_id): + def check_user_id_not_appservice_exclusive(self, user_id, allowed_appservice=None): # valid user IDs must not clash with any user ID namespaces claimed by # application services. services = yield self.store.get_app_services() interested_services = [ - s for s in services if s.is_interested_in_user(user_id) + s for s in services + if s.is_interested_in_user(user_id) + and s != allowed_appservice ] for service in interested_services: if service.is_exclusive_user(user_id): -- cgit 1.4.1 From 33300673b7a6f79802f691ac121e720cb44c0dfc Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 24 Feb 2016 14:41:25 +0000 Subject: Generate guest access token on 3pid invites This means that following the same link across multiple sessions or devices can re-use the same guest account. Note that this is somewhat of an abuse vector; we can't throw up captchas on this flow, so this is a way of registering ephemeral accounts for spam, whose sign-up we don't rate limit. --- synapse/handlers/register.py | 15 ++++++++ synapse/handlers/room.py | 8 ++++ synapse/storage/registration.py | 44 ++++++++++++++++++++++ .../delta/30/threepid_guest_access_tokens.sql | 24 ++++++++++++ 4 files changed, 91 insertions(+) create mode 100644 synapse/storage/schema/delta/30/threepid_guest_access_tokens.sql (limited to 'synapse/handlers/register.py') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index f8959e5d82..6d155d57e7 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -349,3 +349,18 @@ class RegistrationHandler(BaseHandler): def auth_handler(self): return self.hs.get_handlers().auth_handler + + @defer.inlineCallbacks + def guest_access_token_for(self, medium, address, inviter_user_id): + access_token = yield self.store.get_3pid_guest_access_token(medium, address) + if access_token: + defer.returnValue(access_token) + + _, access_token = yield self.register( + generate_token=True, + make_guest=True + ) + access_token = yield self.store.save_or_get_3pid_guest_access_token( + medium, address, access_token, inviter_user_id + ) + defer.returnValue(access_token) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index eb9700a35b..d2de23a6cc 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -848,6 +848,13 @@ class RoomMemberHandler(BaseHandler): user. """ + registration_handler = self.hs.get_handlers().registration_handler + guest_access_token = yield registration_handler.guest_access_token_for( + medium=medium, + address=address, + inviter_user_id=inviter_user_id, + ) + is_url = "%s%s/_matrix/identity/api/v1/store-invite" % ( id_server_scheme, id_server, ) @@ -864,6 +871,7 @@ class RoomMemberHandler(BaseHandler): "sender": inviter_user_id, "sender_display_name": inviter_display_name, "sender_avatar_url": inviter_avatar_url, + "guest_access_token": guest_access_token, } ) # TODO: Check for success diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 967c732bda..03a9b66e4a 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -387,3 +387,47 @@ class RegistrationStore(SQLBaseStore): "find_next_generated_user_id", _find_next_generated_user_id ))) + + @defer.inlineCallbacks + def get_3pid_guest_access_token(self, medium, address): + ret = yield self._simple_select_one( + "threepid_guest_access_tokens", + { + "medium": medium, + "address": address + }, + ["guest_access_token"], True, 'get_3pid_guest_access_token' + ) + if ret: + defer.returnValue(ret["guest_access_token"]) + defer.returnValue(None) + + @defer.inlineCallbacks + def save_or_get_3pid_guest_access_token( + self, medium, address, access_token, inviter_user_id + ): + """ + Gets the 3pid's guest access token if exists, else saves access_token. + + :param medium (str): Medium of the 3pid. Must be "email". + :param address (str): 3pid address. + :param access_token (str): The access token to persist if none is + already persisted. + :param inviter_user_id (str): User ID of the inviter. + :return (deferred str): Whichever access token is persisted at the end + of this function call. + """ + def insert(txn): + txn.execute( + "INSERT INTO threepid_guest_access_tokens " + "(medium, address, guest_access_token, first_inviter) " + "VALUES (?, ?, ?, ?)", + (medium, address, access_token, inviter_user_id) + ) + + try: + yield self.runInteraction("save_3pid_guest_access_token", insert) + defer.returnValue(access_token) + except self.database_engine.module.IntegrityError: + ret = yield self.get_3pid_guest_access_token(medium, address) + defer.returnValue(ret) diff --git a/synapse/storage/schema/delta/30/threepid_guest_access_tokens.sql b/synapse/storage/schema/delta/30/threepid_guest_access_tokens.sql new file mode 100644 index 0000000000..0dd2f1360c --- /dev/null +++ b/synapse/storage/schema/delta/30/threepid_guest_access_tokens.sql @@ -0,0 +1,24 @@ +/* Copyright 2016 OpenMarket Ltd + * + * 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. + */ + +-- Stores guest account access tokens generated for unbound 3pids. +CREATE TABLE threepid_guest_access_tokens( + medium TEXT, -- The medium of the 3pid. Must be "email". + address TEXT, -- The 3pid address. + guest_access_token TEXT, -- The access token for a guest user for this 3pid. + first_inviter TEXT -- User ID of the first user to invite this 3pid to a room. +); + +CREATE UNIQUE INDEX threepid_guest_access_tokens_index ON threepid_guest_access_tokens(medium, address); -- cgit 1.4.1 From 9c48f1ed224e02c30d73388c5e6a5839976bbc58 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 29 Feb 2016 23:11:06 +0100 Subject: handlers/register: make sure another user id is generated when a collision occurs Signed-off-by: Patrik Oldsberg --- synapse/handlers/register.py | 1 + 1 file changed, 1 insertion(+) (limited to 'synapse/handlers/register.py') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 6d155d57e7..c5e5b28811 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -157,6 +157,7 @@ class RegistrationHandler(BaseHandler): ) except SynapseError: # if user id is taken, just generate another + user = None user_id = None token = None attempts += 1 -- cgit 1.4.1 From 239badea9be1dd7857833408209ef22dd99773de Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 7 Mar 2016 20:13:10 +0000 Subject: Use syntax that works on both py2.7 and py3 --- synapse/app/homeserver.py | 2 +- synapse/app/synctl.py | 6 +++--- synapse/config/__main__.py | 2 +- synapse/config/_base.py | 2 +- synapse/handlers/federation.py | 2 +- synapse/handlers/register.py | 2 +- synapse/rest/client/v1/login.py | 2 +- synapse/util/caches/expiringcache.py | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) (limited to 'synapse/handlers/register.py') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 021dc1d610..fcdc8e6e10 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -722,7 +722,7 @@ def run(hs): if hs.config.daemonize: if hs.config.print_pidfile: - print hs.config.pid_file + print (hs.config.pid_file) daemon = Daemonize( app="synapse-homeserver", diff --git a/synapse/app/synctl.py b/synapse/app/synctl.py index 9249e36d82..ab3a31d7b7 100755 --- a/synapse/app/synctl.py +++ b/synapse/app/synctl.py @@ -29,13 +29,13 @@ NORMAL = "\x1b[m" def start(configfile): - print "Starting ...", + print ("Starting ...") args = SYNAPSE args.extend(["--daemonize", "-c", configfile]) try: subprocess.check_call(args) - print GREEN + "started" + NORMAL + print (GREEN + "started" + NORMAL) except subprocess.CalledProcessError as e: print ( RED + @@ -48,7 +48,7 @@ def stop(pidfile): if os.path.exists(pidfile): pid = int(open(pidfile).read()) os.kill(pid, signal.SIGTERM) - print GREEN + "stopped" + NORMAL + print (GREEN + "stopped" + NORMAL) def main(): diff --git a/synapse/config/__main__.py b/synapse/config/__main__.py index 0a3b70e11f..58c97a70af 100644 --- a/synapse/config/__main__.py +++ b/synapse/config/__main__.py @@ -28,7 +28,7 @@ if __name__ == "__main__": sys.stderr.write("\n" + e.message + "\n") sys.exit(1) - print getattr(config, key) + print (getattr(config, key)) sys.exit(0) else: sys.stderr.write("Unknown command %r\n" % (action,)) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 15d78ff33a..7449f36491 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -104,7 +104,7 @@ class Config(object): dir_path = cls.abspath(dir_path) try: os.makedirs(dir_path) - except OSError, e: + except OSError as e: if e.errno != errno.EEXIST: raise if not os.path.isdir(dir_path): diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 6e50b0963e..27f2b40bfe 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -472,7 +472,7 @@ class FederationHandler(BaseHandler): limit=100, extremities=[e for e in extremities.keys()] ) - except SynapseError: + except SynapseError as e: logger.info( "Failed to backfill from %s because %s", dom, e, diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index c5e5b28811..e2ace6a4e5 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -241,7 +241,7 @@ class RegistrationHandler(BaseHandler): password_hash=None ) yield registered_user(self.distributor, user) - except Exception, e: + except Exception as e: yield self.store.add_access_token_to_user(user_id, token) # Ignore Registration errors logger.exception(e) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index f13272da8e..c14e8af00e 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -252,7 +252,7 @@ class SAML2RestServlet(ClientV1RestServlet): SP = Saml2Client(conf) saml2_auth = SP.parse_authn_request_response( request.args['SAMLResponse'][0], BINDING_HTTP_POST) - except Exception, e: # Not authenticated + except Exception as e: # Not authenticated logger.exception(e) if saml2_auth and saml2_auth.status_ok() and not saml2_auth.not_signed: username = saml2_auth.name_id.text diff --git a/synapse/util/caches/expiringcache.py b/synapse/util/caches/expiringcache.py index e863a8f8a9..2b68c1ac93 100644 --- a/synapse/util/caches/expiringcache.py +++ b/synapse/util/caches/expiringcache.py @@ -69,7 +69,7 @@ class ExpiringCache(object): if self._max_len and len(self._cache.keys()) > self._max_len: sorted_entries = sorted( self._cache.items(), - key=lambda (k, v): v.time, + key=lambda item: item[1].time, ) for k, _ in sorted_entries[self._max_len:]: -- cgit 1.4.1 From 465605d616c991760ce021932f0453fc6bc477ef Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 10 Mar 2016 15:58:22 +0000 Subject: Store appservice ID on register --- synapse/handlers/register.py | 5 ++++- synapse/storage/registration.py | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 7 deletions(-) (limited to 'synapse/handlers/register.py') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index e2ace6a4e5..6ffb8c0da6 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -182,6 +182,8 @@ class RegistrationHandler(BaseHandler): errcode=Codes.EXCLUSIVE ) + service_id = service.id if service.is_exclusive_user(user_id) else None + yield self.check_user_id_not_appservice_exclusive( user_id, allowed_appservice=service ) @@ -190,7 +192,8 @@ class RegistrationHandler(BaseHandler): yield self.store.register( user_id=user_id, token=token, - password_hash="" + password_hash="", + appservice_id=service_id, ) yield registered_user(self.distributor, user) defer.returnValue((user_id, token)) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index ad1157f979..aa49f53458 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -76,7 +76,7 @@ class RegistrationStore(SQLBaseStore): @defer.inlineCallbacks def register(self, user_id, token, password_hash, - was_guest=False, make_guest=False): + was_guest=False, make_guest=False, appservice_id=None): """Attempts to register an account. Args: @@ -87,16 +87,32 @@ class RegistrationStore(SQLBaseStore): upgraded to a non-guest account. make_guest (boolean): True if the the new user should be guest, false to add a regular user account. + appservice_id (str): The ID of the appservice registering the user. Raises: StoreError if the user_id could not be registered. """ yield self.runInteraction( "register", - self._register, user_id, token, password_hash, was_guest, make_guest + self._register, + user_id, + token, + password_hash, + was_guest, + make_guest, + appservice_id ) self.is_guest.invalidate((user_id,)) - def _register(self, txn, user_id, token, password_hash, was_guest, make_guest): + def _register( + self, + txn, + user_id, + token, + password_hash, + was_guest, + make_guest, + appservice_id + ): now = int(self.clock.time()) next_id = self._access_tokens_id_gen.get_next() @@ -111,9 +127,21 @@ class RegistrationStore(SQLBaseStore): [password_hash, now, 1 if make_guest else 0, user_id]) else: txn.execute("INSERT INTO users " - "(name, password_hash, creation_ts, is_guest) " - "VALUES (?,?,?,?)", - [user_id, password_hash, now, 1 if make_guest else 0]) + "(" + " name," + " password_hash," + " creation_ts," + " is_guest," + " appservice_id" + ") " + "VALUES (?,?,?,?,?)", + [ + user_id, + password_hash, + now, + 1 if make_guest else 0, + appservice_id, + ]) except self.database_engine.module.IntegrityError: raise StoreError( 400, "User ID already taken.", errcode=Codes.USER_IN_USE -- cgit 1.4.1 From a7daa5ae131cc860769d859cf03b48cefdc0500a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 16 Mar 2016 19:36:57 +0000 Subject: Make registration idempotent, part 2: be idempotent if the client specifies a username. --- synapse/handlers/auth.py | 14 ++++++++++++++ synapse/handlers/register.py | 12 +++++++++++- synapse/rest/client/v2_alpha/register.py | 22 +++++++++++++++++----- 3 files changed, 42 insertions(+), 6 deletions(-) (limited to 'synapse/handlers/register.py') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index d7233cd0d6..82d458b424 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -160,6 +160,20 @@ class AuthHandler(BaseHandler): defer.returnValue(True) defer.returnValue(False) + def get_session_id(self, clientdict): + """ + Gets the session ID for a client given the client dictionary + :param clientdict: The dictionary sent by the client in the request + :return: The string session ID the client sent. If the client did not + send a session ID, returns None. + """ + sid = None + if clientdict and 'auth' in clientdict: + authdict = clientdict['auth'] + if 'session' in authdict: + sid = authdict['session'] + return sid + def set_session_data(self, session_id, key, value): """ Store a key-value pair into the sessions data associated with this diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 6ffb8c0da6..f287ee247b 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -47,7 +47,8 @@ class RegistrationHandler(BaseHandler): self._next_generated_user_id = None @defer.inlineCallbacks - def check_username(self, localpart, guest_access_token=None): + def check_username(self, localpart, guest_access_token=None, + assigned_user_id=None): yield run_on_reactor() if urllib.quote(localpart.encode('utf-8')) != localpart: @@ -60,6 +61,15 @@ class RegistrationHandler(BaseHandler): user = UserID(localpart, self.hs.hostname) user_id = user.to_string() + if assigned_user_id: + if user_id == assigned_user_id: + return + else: + raise SynapseError( + 400, + "A different user ID has already been registered for this session", + ) + yield self.check_user_id_not_appservice_exclusive(user_id) users = yield self.store.get_users_by_id_case_insensitive(user_id) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index c440430e25..b8590560d3 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -16,6 +16,7 @@ from twisted.internet import defer from synapse.api.constants import LoginType +from synapse.types import UserID from synapse.api.errors import SynapseError, Codes, UnrecognizedRequestError from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -122,10 +123,25 @@ class RegisterRestServlet(RestServlet): guest_access_token = body.get("guest_access_token", None) + session_id = self.auth_handler.get_session_id(body) + logger.error("session id: %r", session_id) + registered_user_id = None + if session_id: + # if we get a registered user id out of here, it means we previously + # registered a user for this session, so we could just return the + # user here. We carry on and go through the auth checks though, + # for paranoia. + registered_user_id = self.auth_handler.get_session_data( + session_id, "registered_user_id", None + ) + logger.error("already regged: %r", registered_user_id) + logger.error("check: %r", desired_username) + if desired_username is not None: yield self.registration_handler.check_username( desired_username, - guest_access_token=guest_access_token + guest_access_token=guest_access_token, + assigned_user_id=registered_user_id, ) if self.hs.config.enable_registration_captcha: @@ -147,10 +163,6 @@ class RegisterRestServlet(RestServlet): defer.returnValue((401, result)) return - # have we already registered a user for this session - registered_user_id = self.auth_handler.get_session_data( - session_id, "registered_user_id", None - ) if registered_user_id is not None: logger.info( "Already registered user ID %r for this session", -- cgit 1.4.1