From 351cc35342cc1edbb567b929da05c47d59baa2d1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Oct 2017 10:28:41 +0100 Subject: code_style.rst: a couple of tidyups --- docs/code_style.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'docs') diff --git a/docs/code_style.rst b/docs/code_style.rst index 8d73d17beb..38d52abd47 100644 --- a/docs/code_style.rst +++ b/docs/code_style.rst @@ -1,5 +1,5 @@ -Basically, PEP8 - +- Everything should comply with PEP8. Code should pass + ``pep8 --max-line-length=100`` without any warnings. - NEVER tabs. 4 spaces to indent. - Max line width: 79 chars (with flexibility to overflow by a "few chars" if the overflowing content is not semantically significant and avoids an @@ -43,10 +43,10 @@ Basically, PEP8 together, or want to deliberately extend or preserve vertical/horizontal space) -Comments should follow the `google code style `_. -This is so that we can generate documentation with -`sphinx `_. See the -`examples `_ -in the sphinx documentation. - -Code should pass pep8 --max-line-length=100 without any warnings. +- Comments should follow the `google code style + `_. + This is so that we can generate documentation with `sphinx + `_. See the + `examples + `_ + in the sphinx documentation. -- cgit 1.5.1 From f7f6bfaae45c0ac01132ea99b15008d70a7cd52f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Oct 2017 10:42:06 +0100 Subject: code_style: more formatting --- docs/code_style.rst | 91 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 34 deletions(-) (limited to 'docs') diff --git a/docs/code_style.rst b/docs/code_style.rst index 38d52abd47..a7a71686ba 100644 --- a/docs/code_style.rst +++ b/docs/code_style.rst @@ -1,49 +1,72 @@ - Everything should comply with PEP8. Code should pass ``pep8 --max-line-length=100`` without any warnings. -- NEVER tabs. 4 spaces to indent. -- Max line width: 79 chars (with flexibility to overflow by a "few chars" if + +- **Indenting**: + + - NEVER tabs. 4 spaces to indent. + + - follow PEP8; either hanging indent or multiline-visual indent depending + on the size and shape of the arguments and what makes more sense to the + author. In other words, both this:: + + print("I am a fish %s" % "moo") + + and this:: + + print("I am a fish %s" % + "moo") + + and this:: + + print( + "I am a fish %s" % + "moo", + ) + + ...are valid, although given each one takes up 2x more vertical space than + the previous, it's up to the author's discretion as to which layout makes + most sense for their function invocation. (e.g. if they want to add + comments per-argument, or put expressions in the arguments, or group + related arguments together, or want to deliberately extend or preserve + vertical/horizontal space) + +- **Line length**: + + Max line length is 79 chars (with flexibility to overflow by a "few chars" if the overflowing content is not semantically significant and avoids an explosion of vertical whitespace). -- Use camel case for class and type names -- Use underscores for functions and variables. -- Use double quotes. -- Use parentheses instead of '\\' for line continuation where ever possible - (which is pretty much everywhere) -- There should be max a single new line between: + + Use parentheses instead of ``\`` for line continuation where ever possible + (which is pretty much everywhere). + +- **Naming**: + + - Use camel case for class and type names + - Use underscores for functions and variables. + +- Use double quotes ``"foo"`` rather than single quotes ``'foo'``. + +- **Blank lines**: + + - There should be max a single new line between: + - statements - functions in a class -- There should be two new lines between: - - definitions in a module (e.g., between different classes) -- There should be spaces where spaces should be and not where there shouldn't be: - - a single space after a comma - - a single space before and after for '=' when used as assignment - - no spaces before and after for '=' for default values and keyword arguments. -- Indenting must follow PEP8; either hanging indent or multiline-visual indent - depending on the size and shape of the arguments and what makes more sense to - the author. In other words, both this:: - print("I am a fish %s" % "moo") + - There should be two new lines between: - and this:: - - print("I am a fish %s" % - "moo") + - definitions in a module (e.g., between different classes) - and this:: +- **Whitespace**: - print( - "I am a fish %s" % - "moo" - ) + There should be spaces where spaces should be and not where there shouldn't + be: - ...are valid, although given each one takes up 2x more vertical space than - the previous, it's up to the author's discretion as to which layout makes most - sense for their function invocation. (e.g. if they want to add comments - per-argument, or put expressions in the arguments, or group related arguments - together, or want to deliberately extend or preserve vertical/horizontal - space) + - a single space after a comma + - a single space before and after for '=' when used as assignment + - no spaces before and after for '=' for default values and keyword arguments. -- Comments should follow the `google code style +- **Comments**: should follow the `google code style `_. This is so that we can generate documentation with `sphinx `_. See the -- cgit 1.5.1 From 1eb300e1fcc2ec05c33420033f1d2acdf46d7e20 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Oct 2017 10:58:34 +0100 Subject: Document import rules --- docs/code_style.rst | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'docs') diff --git a/docs/code_style.rst b/docs/code_style.rst index a7a71686ba..9c52cb3182 100644 --- a/docs/code_style.rst +++ b/docs/code_style.rst @@ -73,3 +73,47 @@ `examples `_ in the sphinx documentation. + +- **Imports**: + + - Prefer to import classes and functions than packages or modules. + + Example:: + + from synapse.types import UserID + ... + user_id = UserID(local, server) + + is preferred over:: + + from synapse import types + ... + user_id = types.UserID(local, server) + + (or any other variant). + + This goes against the advice in the Google style guide, but it means that + errors in the name are caught early (at import time). + + - Multiple imports from the same package can be combined onto one line:: + + from synapse.types import GroupID, RoomID, UserID + + An effort should be made to keep the individual imports in alphabetical + order. + + If the list becomes long, wrap it with parentheses and split it over + multiple lines. + + - As per `PEP-8 `_, + imports should be grouped in the following order, with a blank line between + each group: + + 1. standard library imports + 2. related third party imports + 3. local application/library specific imports + + - Imports within each group should be sorted alphabetically by module name. + + - Avoid wildcard imports (``from synapse.types import *``) and relative + imports (``from .types import UserID``). -- cgit 1.5.1 From e51c2bcaef4b15a1e24a31b7edbfefbf93b7c425 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Oct 2017 20:47:06 +0000 Subject: move url_previews to MD as RST does my head in --- docs/url_previews.md | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++ docs/url_previews.rst | 74 ------------------------------------------------- 2 files changed, 76 insertions(+), 74 deletions(-) create mode 100644 docs/url_previews.md delete mode 100644 docs/url_previews.rst (limited to 'docs') diff --git a/docs/url_previews.md b/docs/url_previews.md new file mode 100644 index 0000000000..665554e165 --- /dev/null +++ b/docs/url_previews.md @@ -0,0 +1,76 @@ +URL Previews +============ + +Design notes on a URL previewing service for Matrix: + +Options are: + + 1. Have an AS which listens for URLs, downloads them, and inserts an event that describes their metadata. + * Pros: + * Decouples the implementation entirely from Synapse. + * Uses existing Matrix events & content repo to store the metadata. + * Cons: + * Which AS should provide this service for a room, and why should you trust it? + * Doesn't work well with E2E; you'd have to cut the AS into every room + * the AS would end up subscribing to every room anyway. + + 2. Have a generic preview API (nothing to do with Matrix) that provides a previewing service: + * Pros: + * Simple and flexible; can be used by any clients at any point + * Cons: + * If each HS provides one of these independently, all the HSes in a room may needlessly DoS the target URI + * We need somewhere to store the URL metadata rather than just using Matrix itself + * We can't piggyback on matrix to distribute the metadata between HSes. + + 3. Make the synapse of the sending user responsible for spidering the URL and inserting an event asynchronously which describes the metadata. + * Pros: + * Works transparently for all clients + * Piggy-backs nicely on using Matrix for distributing the metadata. + * No confusion as to which AS + * Cons: + * Doesn't work with E2E + * We might want to decouple the implementation of the spider from the HS, given spider behaviour can be quite complicated and evolve much more rapidly than the HS. It's more like a bot than a core part of the server. + + 4. Make the sending client use the preview API and insert the event itself when successful. + * Pros: + * Works well with E2E + * No custom server functionality + * Lets the client customise the preview that they send (like on FB) + * Cons: + * Entirely specific to the sending client, whereas it'd be nice if /any/ URL was correctly previewed if clients support it. + + 5. Have the option of specifying a shared (centralised) previewing service used by a room, to avoid all the different HSes in the room DoSing the target. + +Best solution is probably a combination of both 2 and 4. + * Sending clients do their best to create and send a preview at the point of sending the message, perhaps delaying the message until the preview is computed? (This also lets the user validate the preview before sending) + * Receiving clients have the option of going and creating their own preview if one doesn't arrive soon enough (or if the original sender didn't create one) + +This is a bit magical though in that the preview could come from two entirely different sources - the sending HS or your local one. However, this can always be exposed to users: "Generate your own URL previews if none are available?" + +This is tantamount also to senders calculating their own thumbnails for sending in advance of the main content - we are trusting the sender not to lie about the content in the thumbnail. Whereas currently thumbnails are calculated by the receiving homeserver to avoid this attack. + +However, this kind of phishing attack does exist whether we let senders pick their thumbnails or not, in that a malicious sender can send normal text messages around the attachment claiming it to be legitimate. We could rely on (future) reputation/abuse management to punish users who phish (be it with bogus metadata or bogus descriptions). Bogus metadata is particularly bad though, especially if it's avoidable. + +As a first cut, let's do #2 and have the receiver hit the API to calculate its own previews (as it does currently for image thumbnails). We can then extend/optimise this to option 4 as a special extra if needed. + +API +--- + +``` +GET /_matrix/media/r0/preview_url?url=http://wherever.com +200 OK +{ + "og:type" : "article" + "og:url" : "https://twitter.com/matrixdotorg/status/684074366691356672" + "og:title" : "Matrix on Twitter" + "og:image" : "https://pbs.twimg.com/profile_images/500400952029888512/yI0qtFi7_400x400.png" + "og:description" : "“Synapse 0.12 is out! Lots of polishing, performance & bugfixes: /sync API, /r0 prefix, fulltext search, 3PID invites https://t.co/5alhXLLEGP”" + "og:site_name" : "Twitter" +} +``` + +* Downloads the URL + * If HTML, just stores it in RAM and parses it for OG meta tags + * Download any media OG meta tags to the media repo, and refer to them in the OG via mxc:// URIs. + * If a media filetype we know we can thumbnail: store it on disk, and hand it to the thumbnailer. Generate OG meta tags from the thumbnailer contents. + * Otherwise, don't bother downloading further. diff --git a/docs/url_previews.rst b/docs/url_previews.rst deleted file mode 100644 index 634d9d907f..0000000000 --- a/docs/url_previews.rst +++ /dev/null @@ -1,74 +0,0 @@ -URL Previews -============ - -Design notes on a URL previewing service for Matrix: - -Options are: - - 1. Have an AS which listens for URLs, downloads them, and inserts an event that describes their metadata. - * Pros: - * Decouples the implementation entirely from Synapse. - * Uses existing Matrix events & content repo to store the metadata. - * Cons: - * Which AS should provide this service for a room, and why should you trust it? - * Doesn't work well with E2E; you'd have to cut the AS into every room - * the AS would end up subscribing to every room anyway. - - 2. Have a generic preview API (nothing to do with Matrix) that provides a previewing service: - * Pros: - * Simple and flexible; can be used by any clients at any point - * Cons: - * If each HS provides one of these independently, all the HSes in a room may needlessly DoS the target URI - * We need somewhere to store the URL metadata rather than just using Matrix itself - * We can't piggyback on matrix to distribute the metadata between HSes. - - 3. Make the synapse of the sending user responsible for spidering the URL and inserting an event asynchronously which describes the metadata. - * Pros: - * Works transparently for all clients - * Piggy-backs nicely on using Matrix for distributing the metadata. - * No confusion as to which AS - * Cons: - * Doesn't work with E2E - * We might want to decouple the implementation of the spider from the HS, given spider behaviour can be quite complicated and evolve much more rapidly than the HS. It's more like a bot than a core part of the server. - - 4. Make the sending client use the preview API and insert the event itself when successful. - * Pros: - * Works well with E2E - * No custom server functionality - * Lets the client customise the preview that they send (like on FB) - * Cons: - * Entirely specific to the sending client, whereas it'd be nice if /any/ URL was correctly previewed if clients support it. - - 5. Have the option of specifying a shared (centralised) previewing service used by a room, to avoid all the different HSes in the room DoSing the target. - -Best solution is probably a combination of both 2 and 4. - * Sending clients do their best to create and send a preview at the point of sending the message, perhaps delaying the message until the preview is computed? (This also lets the user validate the preview before sending) - * Receiving clients have the option of going and creating their own preview if one doesn't arrive soon enough (or if the original sender didn't create one) - -This is a bit magical though in that the preview could come from two entirely different sources - the sending HS or your local one. However, this can always be exposed to users: "Generate your own URL previews if none are available?" - -This is tantamount also to senders calculating their own thumbnails for sending in advance of the main content - we are trusting the sender not to lie about the content in the thumbnail. Whereas currently thumbnails are calculated by the receiving homeserver to avoid this attack. - -However, this kind of phishing attack does exist whether we let senders pick their thumbnails or not, in that a malicious sender can send normal text messages around the attachment claiming it to be legitimate. We could rely on (future) reputation/abuse management to punish users who phish (be it with bogus metadata or bogus descriptions). Bogus metadata is particularly bad though, especially if it's avoidable. - -As a first cut, let's do #2 and have the receiver hit the API to calculate its own previews (as it does currently for image thumbnails). We can then extend/optimise this to option 4 as a special extra if needed. - -API ---- - -GET /_matrix/media/r0/preview_url?url=http://wherever.com -200 OK -{ - "og:type" : "article" - "og:url" : "https://twitter.com/matrixdotorg/status/684074366691356672" - "og:title" : "Matrix on Twitter" - "og:image" : "https://pbs.twimg.com/profile_images/500400952029888512/yI0qtFi7_400x400.png" - "og:description" : "“Synapse 0.12 is out! Lots of polishing, performance & bugfixes: /sync API, /r0 prefix, fulltext search, 3PID invites https://t.co/5alhXLLEGP”" - "og:site_name" : "Twitter" -} - -* Downloads the URL - * If HTML, just stores it in RAM and parses it for OG meta tags - * Download any media OG meta tags to the media repo, and refer to them in the OG via mxc:// URIs. - * If a media filetype we know we can thumbnail: store it on disk, and hand it to the thumbnailer. Generate OG meta tags from the thumbnailer contents. - * Otherwise, don't bother downloading further. -- cgit 1.5.1 From ebda45de4c04d4a3d569e4f2969b798699bd5b16 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 30 Oct 2017 14:41:35 +0000 Subject: Start some documentation on password providers Document the existing interface, before I start adding new stuff. --- docs/password_auth_providers.rst | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 docs/password_auth_providers.rst (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst new file mode 100644 index 0000000000..3da1a67844 --- /dev/null +++ b/docs/password_auth_providers.rst @@ -0,0 +1,39 @@ +Password auth provider modules +============================== + +Password auth providers offer a way for server administrators to integrate +their Synapse installation with an existing authentication system. + +A password auth provider is a Python class which is dynamically loaded into +Synapse, and provides a number of methods by which it can integrate with the +authentication system. + +This document serves as a reference for those looking to implement their own +password auth providers. + +Required methods +---------------- + +Password auth provider classes must provide the following methods: + +*class* ``SomeProvider.parse_config``\(*config*) + + This method is passed the ``config`` object for this module from the + homeserver configuration file. + + It should perform any appropriate sanity checks on the provided + configuration, and return an object which is then passed into ``__init__``. + +*class* ``SomeProvider``\(*config*, *account_handler*) + + The constructor is passed the config object returned by ``parse_config``, + and a ``synapse.handlers.auth._AccountHandler`` object which allows the + password provider to check if accounts exist and/or create new ones. + +``someprovider.check_password``\(*user_id*, *password*) + + This is the method that actually does the work. It is passed a qualified + ``@localpart:domain`` user id, and the password provided by the user. + + The method should return a Twisted ``Deferred`` object, which resolves to + ``True`` if authentication is successful, and ``False`` if not. -- cgit 1.5.1 From 1650eb584772dbad61d74c2b3c9c932a52fe1979 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 30 Oct 2017 15:16:21 +0000 Subject: DB schema interface for password auth providers Provide an interface by which password auth providers can register db schema files to be run at startup --- docs/password_auth_providers.rst | 12 ++++++ synapse/storage/prepare_database.py | 70 +++++++++++++++++++++++++++++++ synapse/storage/schema/schema_version.sql | 7 ++++ 3 files changed, 89 insertions(+) (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst index 3da1a67844..ca05a76617 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst @@ -37,3 +37,15 @@ Password auth provider classes must provide the following methods: The method should return a Twisted ``Deferred`` object, which resolves to ``True`` if authentication is successful, and ``False`` if not. + +Optional methods +---------------- + +Password provider classes may optionally provide the following methods. + +*class* ``SomeProvider.get_db_schema_files()`` + + This method, if implemented, should return an Iterable of ``(name, + stream)`` pairs of database schema files. Each file is applied in turn at + initialisation, and a record is then made in the database so that it is + not re-applied on the next start. diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index a4e08e6757..d1691bbac2 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -44,6 +44,13 @@ def prepare_database(db_conn, database_engine, config): If `config` is None then prepare_database will assert that no upgrade is necessary, *or* will create a fresh database if the database is empty. + + Args: + db_conn: + database_engine: + config (synapse.config.homeserver.HomeServerConfig|None): + application config, or None if we are connecting to an existing + database which we expect to be configured already """ try: cur = db_conn.cursor() @@ -64,6 +71,10 @@ def prepare_database(db_conn, database_engine, config): else: _setup_new_database(cur, database_engine) + # check if any of our configured dynamic modules want a database + if config is not None: + _apply_module_schemas(cur, database_engine, config) + cur.close() db_conn.commit() except Exception: @@ -283,6 +294,65 @@ def _upgrade_existing_database(cur, current_version, applied_delta_files, ) +def _apply_module_schemas(txn, database_engine, config): + """Apply the module schemas for the dynamic modules, if any + + Args: + cur: database cursor + database_engine: synapse database engine class + config (synapse.config.homeserver.HomeServerConfig): + application config + """ + for (mod, _config) in config.password_providers: + if not hasattr(mod, 'get_db_schema_files'): + continue + modname = ".".join((mod.__module__, mod.__name__)) + _apply_module_schema_files( + txn, database_engine, modname, mod.get_db_schema_files(), + ) + + +def _apply_module_schema_files(cur, database_engine, modname, names_and_streams): + """Apply the module schemas for a single module + + Args: + cur: database cursor + database_engine: synapse database engine class + modname (str): fully qualified name of the module + names_and_streams (Iterable[(str, file)]): the names and streams of + schemas to be applied + """ + cur.execute( + database_engine.convert_param_style( + "SELECT file FROM applied_module_schemas WHERE module_name = ?" + ), + (modname,) + ) + applied_deltas = set(d for d, in cur) + for (name, stream) in names_and_streams: + if name in applied_deltas: + continue + + root_name, ext = os.path.splitext(name) + if ext != '.sql': + raise PrepareDatabaseException( + "only .sql files are currently supported for module schemas", + ) + + logger.info("applying schema %s for %s", name, modname) + for statement in get_statements(stream): + cur.execute(statement) + + # Mark as done. + cur.execute( + database_engine.convert_param_style( + "INSERT INTO applied_module_schemas (module_name, file)" + " VALUES (?,?)", + ), + (modname, name) + ) + + def get_statements(f): statement_buffer = "" in_comment = False # If we're in a /* ... */ style comment diff --git a/synapse/storage/schema/schema_version.sql b/synapse/storage/schema/schema_version.sql index a7ade69986..42e5cb6df5 100644 --- a/synapse/storage/schema/schema_version.sql +++ b/synapse/storage/schema/schema_version.sql @@ -25,3 +25,10 @@ CREATE TABLE IF NOT EXISTS applied_schema_deltas( file TEXT NOT NULL, UNIQUE(version, file) ); + +-- a list of schema files we have loaded on behalf of dynamic modules +CREATE TABLE IF NOT EXISTS applied_module_schemas( + module_name TEXT NOT NULL, + file TEXT NOT NULL, + UNIQUE(module_name, file) +); -- cgit 1.5.1 From 3cd6b22c7bf0aa0108535bad5656a0d2d9e85634 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 31 Oct 2017 10:43:57 +0000 Subject: Let password auth providers handle arbitrary login types Provide a hook where password auth providers can say they know about other login types, and get passed the relevant parameters --- docs/password_auth_providers.rst | 53 ++++++++++++++---- synapse/handlers/auth.py | 118 +++++++++++++++++++++++++++++++-------- 2 files changed, 139 insertions(+), 32 deletions(-) (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst index ca05a76617..2dbebcd72c 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst @@ -30,22 +30,55 @@ Password auth provider classes must provide the following methods: and a ``synapse.handlers.auth._AccountHandler`` object which allows the password provider to check if accounts exist and/or create new ones. -``someprovider.check_password``\(*user_id*, *password*) - - This is the method that actually does the work. It is passed a qualified - ``@localpart:domain`` user id, and the password provided by the user. - - The method should return a Twisted ``Deferred`` object, which resolves to - ``True`` if authentication is successful, and ``False`` if not. - Optional methods ---------------- -Password provider classes may optionally provide the following methods. +Password auth provider classes may optionally provide the following methods. -*class* ``SomeProvider.get_db_schema_files()`` +*class* ``SomeProvider.get_db_schema_files``\() This method, if implemented, should return an Iterable of ``(name, stream)`` pairs of database schema files. Each file is applied in turn at initialisation, and a record is then made in the database so that it is not re-applied on the next start. + +``someprovider.get_supported_login_types``\() + + This method, if implemented, should return a ``dict`` mapping from a login + type identifier (such as ``m.login.password``) to an iterable giving the + fields which must be provided by the user in the submission to the + ``/login`` api. These fields are passed in the ``login_dict`` dictionary + to ``check_auth``. + + For example, if a password auth provider wants to implement a custom login + type of ``com.example.custom_login``, where the client is expected to pass + the fields ``secret1`` and ``secret2``, the provider should implement this + method and return the following dict:: + + {"com.example.custom_login": ("secret1", "secret2")} + +``someprovider.check_auth``\(*username*, *login_type*, *login_dict*) + + This method is the one that does the real work. If implemented, it will be + called for each login attempt where the login type matches one of the keys + returned by ``get_supported_login_types``. + + It is passed the (possibly UNqualified) ``user`` provided by the client, + the login type, and a dictionary of login secrets passed by the client. + + The method should return a Twisted ``Deferred`` object, which resolves to + the canonical ``@localpart:domain`` user id if authentication is successful, + and ``None`` if not. + +``someprovider.check_password``\(*user_id*, *password*) + + This method provides a simpler interface than ``get_supported_login_types`` + and ``check_auth`` for password auth providers that just want to provide a + mechanism for validating ``m.login.password`` logins. + + Iif implemented, it will be called to check logins with an + ``m.login.password`` login type. It is passed a qualified + ``@localpart:domain`` user id, and the password provided by the user. + + The method should return a Twisted ``Deferred`` object, which resolves to + ``True`` if authentication is successful, and ``False`` if not. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 93d8ac0e04..d5da27a3c3 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -82,6 +82,11 @@ class AuthHandler(BaseHandler): login_types = set() if self._password_enabled: login_types.add(LoginType.PASSWORD) + for provider in self.password_providers: + if hasattr(provider, "get_supported_login_types"): + login_types.update( + provider.get_supported_login_types().keys() + ) self._supported_login_types = frozenset(login_types) @defer.inlineCallbacks @@ -504,14 +509,14 @@ class AuthHandler(BaseHandler): return self._supported_login_types @defer.inlineCallbacks - def validate_login(self, user_id, login_submission): + def validate_login(self, username, login_submission): """Authenticates the user for the /login API Also used by the user-interactive auth flow to validate m.login.password auth types. Args: - user_id (str): user_id supplied by the user + username (str): username supplied by the user login_submission (dict): the whole of the login submission (including 'type' and other relevant fields) Returns: @@ -522,32 +527,81 @@ class AuthHandler(BaseHandler): LoginError if there was an authentication problem. """ - if not user_id.startswith('@'): - user_id = UserID( - user_id, self.hs.hostname + if username.startswith('@'): + qualified_user_id = username + else: + qualified_user_id = UserID( + username, self.hs.hostname ).to_string() login_type = login_submission.get("type") + known_login_type = False - if login_type != LoginType.PASSWORD: - raise SynapseError(400, "Bad login type.") - if not self._password_enabled: - raise SynapseError(400, "Password login has been disabled.") - if "password" not in login_submission: - raise SynapseError(400, "Missing parameter: password") + # special case to check for "password" for the check_password interface + # for the auth providers + password = login_submission.get("password") + if login_type == LoginType.PASSWORD: + if not self._password_enabled: + raise SynapseError(400, "Password login has been disabled.") + if not password: + raise SynapseError(400, "Missing parameter: password") - password = login_submission["password"] for provider in self.password_providers: - is_valid = yield provider.check_password(user_id, password) - if is_valid: - defer.returnValue(user_id) + if (hasattr(provider, "check_password") + and login_type == LoginType.PASSWORD): + known_login_type = True + is_valid = yield provider.check_password( + qualified_user_id, password, + ) + if is_valid: + defer.returnValue(qualified_user_id) + + if (not hasattr(provider, "get_supported_login_types") + or not hasattr(provider, "check_auth")): + # this password provider doesn't understand custom login types + continue + + supported_login_types = provider.get_supported_login_types() + if login_type not in supported_login_types: + # this password provider doesn't understand this login type + continue + + known_login_type = True + login_fields = supported_login_types[login_type] + + missing_fields = [] + login_dict = {} + for f in login_fields: + if f not in login_submission: + missing_fields.append(f) + else: + login_dict[f] = login_submission[f] + if missing_fields: + raise SynapseError( + 400, "Missing parameters for login type %s: %s" % ( + login_type, + missing_fields, + ), + ) - canonical_user_id = yield self._check_local_password( - user_id, password, - ) + returned_user_id = yield provider.check_auth( + username, login_type, login_dict, + ) + if returned_user_id: + defer.returnValue(returned_user_id) + + if login_type == LoginType.PASSWORD: + known_login_type = True + + canonical_user_id = yield self._check_local_password( + qualified_user_id, password, + ) - if canonical_user_id: - defer.returnValue(canonical_user_id) + if canonical_user_id: + defer.returnValue(canonical_user_id) + + if not known_login_type: + raise SynapseError(400, "Unknown login type %s" % login_type) # unknown username or invalid password. We raise a 403 here, but note # that if we're doing user-interactive login, it turns all LoginErrors @@ -731,11 +785,31 @@ class _AccountHandler(object): self._check_user_exists = check_user_exists + def get_qualified_user_id(self, username): + """Qualify a user id, if necessary + + Takes a user id provided by the user and adds the @ and :domain to + qualify it, if necessary + + Args: + username (str): provided user id + + Returns: + str: qualified @user:id + """ + if username.startswith('@'): + return username + return UserID(username, self.hs.hostname).to_string() + def check_user_exists(self, user_id): - """Check if user exissts. + """Check if user exists. + + Args: + user_id (str): Complete @user:id Returns: - Deferred(bool) + Deferred[str|None]: Canonical (case-corrected) user_id, or None + if the user is not registered. """ return self._check_user_exists(user_id) -- cgit 1.5.1 From 4c8f94ac9433753464c4d8379aae650c3129500d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 31 Oct 2017 15:15:51 +0000 Subject: Allow password_auth_providers to return a callback ... so that they have a way to record access tokens. --- docs/password_auth_providers.rst | 5 +++++ synapse/handlers/auth.py | 13 ++++++++----- synapse/rest/client/v1/login.py | 5 ++++- 3 files changed, 17 insertions(+), 6 deletions(-) (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst index 2dbebcd72c..4ae4aeb53f 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst @@ -70,6 +70,11 @@ Password auth provider classes may optionally provide the following methods. the canonical ``@localpart:domain`` user id if authentication is successful, and ``None`` if not. + Alternatively, the ``Deferred`` can resolve to a ``(str, func)`` tuple, in + which case the second field is a callback which will be called with the + result from the ``/login`` call (including ``access_token``, ``device_id``, + etc.) + ``someprovider.check_password``\(*user_id*, *password*) This method provides a simpler interface than ``get_supported_login_types`` diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 9799461d26..5c89768c14 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -517,7 +517,8 @@ class AuthHandler(BaseHandler): login_submission (dict): the whole of the login submission (including 'type' and other relevant fields) Returns: - Deferred[str]: canonical user id + Deferred[str, func]: canonical user id, and optional callback + to be called once the access token and device id are issued Raises: StoreError if there was a problem accessing the database SynapseError if there was a problem with the request @@ -581,11 +582,13 @@ class AuthHandler(BaseHandler): ), ) - returned_user_id = yield provider.check_auth( + result = yield provider.check_auth( username, login_type, login_dict, ) - if returned_user_id: - defer.returnValue(returned_user_id) + if result: + if isinstance(result, str): + result = (result, None) + defer.returnValue(result) if login_type == LoginType.PASSWORD: known_login_type = True @@ -595,7 +598,7 @@ class AuthHandler(BaseHandler): ) if canonical_user_id: - defer.returnValue(canonical_user_id) + defer.returnValue((canonical_user_id, None)) if not known_login_type: raise SynapseError(400, "Unknown login type %s" % login_type) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index d25a68e753..5669ecb724 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -219,7 +219,7 @@ class LoginRestServlet(ClientV1RestServlet): raise SynapseError(400, "User identifier is missing 'user' key") auth_handler = self.auth_handler - canonical_user_id = yield auth_handler.validate_login( + canonical_user_id, callback = yield auth_handler.validate_login( identifier["user"], login_submission, ) @@ -238,6 +238,9 @@ class LoginRestServlet(ClientV1RestServlet): "device_id": device_id, } + if callback is not None: + yield callback(result) + defer.returnValue((200, result)) @defer.inlineCallbacks -- cgit 1.5.1 From bc8a5c033097f719d6b2971660ad833ab8cb3838 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Nov 2017 15:42:38 +0000 Subject: Notify auth providers on logout Provide a hook by which auth providers can be notified of logouts. --- docs/password_auth_providers.rst | 10 ++++++++++ synapse/handlers/auth.py | 26 ++++++++++++++++++++++++-- synapse/storage/registration.py | 13 ++++++++----- 3 files changed, 42 insertions(+), 7 deletions(-) (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst index 2dbebcd72c..98019cea01 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst @@ -82,3 +82,13 @@ Password auth provider classes may optionally provide the following methods. The method should return a Twisted ``Deferred`` object, which resolves to ``True`` if authentication is successful, and ``False`` if not. + +``someprovider.on_logged_out``\(*user_id*, *device_id*, *access_token*) + + This method, if implemented, is called when a user logs out. It is passed + the qualified user ID, the ID of the deactivated device (if any: access + tokens are occasionally created without an associated device ID), and the + (now deactivated) access token. + + It may return a Twisted ``Deferred`` object; the logout request will wait + for the deferred to complete but the result is ignored. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 9799461d26..cc667b6d8b 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -682,6 +682,7 @@ class AuthHandler(BaseHandler): yield self.store.user_delete_threepids(user_id) yield self.store.user_set_password_hash(user_id, None) + @defer.inlineCallbacks def delete_access_token(self, access_token): """Invalidate a single access token @@ -691,8 +692,19 @@ class AuthHandler(BaseHandler): Returns: Deferred """ - return self.store.delete_access_token(access_token) + user_info = yield self.auth.get_user_by_access_token(access_token) + yield self.store.delete_access_token(access_token) + + # see if any of our auth providers want to know about this + for provider in self.password_providers: + if hasattr(provider, "on_logged_out"): + yield provider.on_logged_out( + user_id=str(user_info["user"]), + device_id=user_info["device_id"], + access_token=access_token, + ) + @defer.inlineCallbacks def delete_access_tokens_for_user(self, user_id, except_token_id=None, device_id=None): """Invalidate access tokens belonging to a user @@ -707,10 +719,20 @@ class AuthHandler(BaseHandler): Returns: Deferred """ - return self.store.user_delete_access_tokens( + tokens_and_devices = yield self.store.user_delete_access_tokens( user_id, except_token_id=except_token_id, device_id=device_id, ) + # see if any of our auth providers want to know about this + for provider in self.password_providers: + if hasattr(provider, "on_logged_out"): + for token, device_id in tokens_and_devices: + yield provider.on_logged_out( + user_id=user_id, + device_id=device_id, + access_token=token, + ) + @defer.inlineCallbacks def add_threepid(self, user_id, medium, address, validated_at): # 'Canonicalise' email addresses down to lower case. diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 65ddefda92..9c4f61da76 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -255,7 +255,8 @@ class RegistrationStore(background_updates.BackgroundUpdateStore): If None, tokens associated with any device (or no device) will be deleted Returns: - defer.Deferred: + defer.Deferred[list[str, str|None]]: a list of the deleted tokens + and device IDs """ def f(txn): keyvalues = { @@ -272,14 +273,14 @@ class RegistrationStore(background_updates.BackgroundUpdateStore): values.append(except_token_id) txn.execute( - "SELECT token FROM access_tokens WHERE %s" % where_clause, + "SELECT token, device_id FROM access_tokens WHERE %s" % where_clause, values ) - rows = self.cursor_to_dict(txn) + tokens_and_devices = [(r[0], r[1]) for r in txn] - for row in rows: + for token, _ in tokens_and_devices: self._invalidate_cache_and_stream( - txn, self.get_user_by_access_token, (row["token"],) + txn, self.get_user_by_access_token, (token,) ) txn.execute( @@ -287,6 +288,8 @@ class RegistrationStore(background_updates.BackgroundUpdateStore): values ) + return tokens_and_devices + yield self.runInteraction( "user_delete_access_tokens", f, ) -- cgit 1.5.1 From 1189be43a2479f5adf034613e8d10e3f4f452eb9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Nov 2017 14:13:25 +0000 Subject: Factor _AccountHandler proxy out to ModuleApi We're going to need to use this from places that aren't password auth, so let's move it to a proper class. --- docs/password_auth_providers.rst | 2 +- synapse/handlers/auth.py | 72 ++---------------------------------- synapse/module_api/__init__.py | 79 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 70 deletions(-) create mode 100644 synapse/module_api/__init__.py (limited to 'docs') diff --git a/docs/password_auth_providers.rst b/docs/password_auth_providers.rst index 2842d187e5..d8a7b61cdc 100644 --- a/docs/password_auth_providers.rst +++ b/docs/password_auth_providers.rst @@ -27,7 +27,7 @@ Password auth provider classes must provide the following methods: *class* ``SomeProvider``\(*config*, *account_handler*) The constructor is passed the config object returned by ``parse_config``, - and a ``synapse.handlers.auth._AccountHandler`` object which allows the + and a ``synapse.module_api.ModuleApi`` object which allows the password provider to check if accounts exist and/or create new ones. Optional methods diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 0337be36c2..7a0ba6ef35 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -13,13 +13,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - from twisted.internet import defer from ._base import BaseHandler from synapse.api.constants import LoginType -from synapse.types import UserID from synapse.api.errors import AuthError, LoginError, Codes, StoreError, SynapseError +from synapse.module_api import ModuleApi +from synapse.types import UserID from synapse.util.async import run_on_reactor from synapse.util.caches.expiringcache import ExpiringCache @@ -63,10 +63,7 @@ class AuthHandler(BaseHandler): reset_expiry_on_get=True, ) - account_handler = _AccountHandler( - hs, check_user_exists=self.check_user_exists - ) - + account_handler = ModuleApi(hs, self) self.password_providers = [ module(config=config, account_handler=account_handler) for module, config in hs.config.password_providers @@ -843,66 +840,3 @@ class MacaroonGeneartor(object): macaroon.add_first_party_caveat("gen = 1") macaroon.add_first_party_caveat("user_id = %s" % (user_id,)) return macaroon - - -class _AccountHandler(object): - """A proxy object that gets passed to password auth providers so they - can register new users etc if necessary. - """ - def __init__(self, hs, check_user_exists): - self.hs = hs - - self._check_user_exists = check_user_exists - self._store = hs.get_datastore() - - def get_qualified_user_id(self, username): - """Qualify a user id, if necessary - - Takes a user id provided by the user and adds the @ and :domain to - qualify it, if necessary - - Args: - username (str): provided user id - - Returns: - str: qualified @user:id - """ - if username.startswith('@'): - return username - return UserID(username, self.hs.hostname).to_string() - - def check_user_exists(self, user_id): - """Check if user exists. - - Args: - user_id (str): Complete @user:id - - Returns: - Deferred[str|None]: Canonical (case-corrected) user_id, or None - if the user is not registered. - """ - return self._check_user_exists(user_id) - - def register(self, localpart): - """Registers a new user with given localpart - - Returns: - Deferred: a 2-tuple of (user_id, access_token) - """ - reg = self.hs.get_handlers().registration_handler - return reg.register(localpart=localpart) - - def run_db_interaction(self, desc, func, *args, **kwargs): - """Run a function with a database connection - - Args: - desc (str): description for the transaction, for metrics etc - func (func): function to be run. Passed a database cursor object - as well as *args and **kwargs - *args: positional args to be passed to func - **kwargs: named args to be passed to func - - Returns: - Deferred[object]: result of func - """ - return self._store.runInteraction(desc, func, *args, **kwargs) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py new file mode 100644 index 0000000000..9ccf6dfcd6 --- /dev/null +++ b/synapse/module_api/__init__.py @@ -0,0 +1,79 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 New Vector 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. + +from synapse.types import UserID + + +class ModuleApi(object): + """A proxy object that gets passed to password auth providers so they + can register new users etc if necessary. + """ + def __init__(self, hs, auth_handler): + self.hs = hs + + self._store = hs.get_datastore() + self._auth_handler = auth_handler + + def get_qualified_user_id(self, username): + """Qualify a user id, if necessary + + Takes a user id provided by the user and adds the @ and :domain to + qualify it, if necessary + + Args: + username (str): provided user id + + Returns: + str: qualified @user:id + """ + if username.startswith('@'): + return username + return UserID(username, self.hs.hostname).to_string() + + def check_user_exists(self, user_id): + """Check if user exists. + + Args: + user_id (str): Complete @user:id + + Returns: + Deferred[str|None]: Canonical (case-corrected) user_id, or None + if the user is not registered. + """ + return self._auth_handler.check_user_exists(user_id) + + def register(self, localpart): + """Registers a new user with given localpart + + Returns: + Deferred: a 2-tuple of (user_id, access_token) + """ + reg = self.hs.get_handlers().registration_handler + return reg.register(localpart=localpart) + + def run_db_interaction(self, desc, func, *args, **kwargs): + """Run a function with a database connection + + Args: + desc (str): description for the transaction, for metrics etc + func (func): function to be run. Passed a database cursor object + as well as *args and **kwargs + *args: positional args to be passed to func + **kwargs: named args to be passed to func + + Returns: + Deferred[object]: result of func + """ + return self._store.runInteraction(desc, func, *args, **kwargs) -- cgit 1.5.1 From 2ac6deafb7a2f00579092693e0392730a08a6b82 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 4 Nov 2017 19:34:59 +0000 Subject: simplify instructions for regenerating user_dir --- docs/user_directory.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 docs/user_directory.md (limited to 'docs') diff --git a/docs/user_directory.md b/docs/user_directory.md new file mode 100644 index 0000000000..4c8ee44f37 --- /dev/null +++ b/docs/user_directory.md @@ -0,0 +1,17 @@ +User Directory API Implementation +================================= + +The user directory is currently maintained based on the 'visible' users +on this particular server - i.e. ones which your account shares a room with, or +who are present in a publicly viewable room present on the server. + +The directory info is stored in various tables, which can (typically after +DB corruption) get stale or out of sync. If this happens, for now the +quickest solution to fix it is: + +``` +UPDATE user_directory_stream_pos SET stream_id = NULL; +``` + +and restart the synapse, which should then start a background task to +flush the current tables and regenerate the directory. -- cgit 1.5.1