From 33a9026cdf19dc13a3492e838f0893755d380981 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 25 Oct 2017 10:26:06 +0100
Subject: Add logging and fix log contexts for publicRooms

---
 synapse/handlers/room_list.py | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py
index 41e1781df7..ae7c611170 100644
--- a/synapse/handlers/room_list.py
+++ b/synapse/handlers/room_list.py
@@ -70,6 +70,7 @@ class RoomListHandler(BaseHandler):
         if search_filter:
             # We explicitly don't bother caching searches or requests for
             # appservice specific lists.
+            logger.info("Bypassing cache as search request.")
             return self._get_public_room_list(
                 limit, since_token, search_filter, network_tuple=network_tuple,
             )
@@ -77,6 +78,7 @@ class RoomListHandler(BaseHandler):
         key = (limit, since_token, network_tuple)
         result = self.response_cache.get(key)
         if not result:
+            logger.info("No cached result, calculating one.")
             result = self.response_cache.set(
                 key,
                 self._get_public_room_list(
-- 
cgit 1.5.1


From 2a7e9faeec9af8723b9613fef3b3059b1fe777f5 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 25 Oct 2017 15:21:08 +0100
Subject: Do logcontexts outside ResponseCache

---
 synapse/appservice/api.py               | 10 +++++++---
 synapse/federation/federation_server.py |  8 +++++---
 synapse/handlers/room_list.py           |  7 +++++--
 synapse/handlers/sync.py                |  6 +++---
 synapse/util/caches/response_cache.py   |  3 +--
 5 files changed, 21 insertions(+), 13 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py
index 6893610e71..40c433d7ae 100644
--- a/synapse/appservice/api.py
+++ b/synapse/appservice/api.py
@@ -18,6 +18,7 @@ from synapse.api.constants import ThirdPartyEntityKind
 from synapse.api.errors import CodeMessageException
 from synapse.http.client import SimpleHttpClient
 from synapse.events.utils import serialize_event
+from synapse.util.logcontext import preserve_fn, make_deferred_yieldable
 from synapse.util.caches.response_cache import ResponseCache
 from synapse.types import ThirdPartyInstanceID
 
@@ -192,9 +193,12 @@ class ApplicationServiceApi(SimpleHttpClient):
                 defer.returnValue(None)
 
         key = (service.id, protocol)
-        return self.protocol_meta_cache.get(key) or (
-            self.protocol_meta_cache.set(key, _get())
-        )
+        result = self.protocol_meta_cache.get(key)
+        if not result:
+            result = self.protocol_meta_cache.set(
+                key, preserve_fn(_get)()
+            )
+        return make_deferred_yieldable(result)
 
     @defer.inlineCallbacks
     def push_bulk(self, service, events, txn_id=None):
diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py
index e15228e70b..a2327f24b6 100644
--- a/synapse/federation/federation_server.py
+++ b/synapse/federation/federation_server.py
@@ -18,6 +18,7 @@ from .federation_base import FederationBase
 from .units import Transaction, Edu
 
 from synapse.util import async
+from synapse.util.logcontext import make_deferred_yieldable, preserve_fn
 from synapse.util.logutils import log_function
 from synapse.util.caches.response_cache import ResponseCache
 from synapse.events import FrozenEvent
@@ -253,12 +254,13 @@ class FederationServer(FederationBase):
         result = self._state_resp_cache.get((room_id, event_id))
         if not result:
             with (yield self._server_linearizer.queue((origin, room_id))):
-                resp = yield self._state_resp_cache.set(
+                d = self._state_resp_cache.set(
                     (room_id, event_id),
-                    self._on_context_state_request_compute(room_id, event_id)
+                    preserve_fn(self._on_context_state_request_compute)(room_id, event_id)
                 )
+                resp = yield make_deferred_yieldable(d)
         else:
-            resp = yield result
+            resp = yield make_deferred_yieldable(result)
 
         defer.returnValue((200, resp))
 
diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py
index ae7c611170..5a47254b56 100644
--- a/synapse/handlers/room_list.py
+++ b/synapse/handlers/room_list.py
@@ -20,6 +20,7 @@ from ._base import BaseHandler
 from synapse.api.constants import (
     EventTypes, JoinRules,
 )
+from synapse.util.logcontext import make_deferred_yieldable, preserve_fn
 from synapse.util.async import concurrently_execute
 from synapse.util.caches.descriptors import cachedInlineCallbacks
 from synapse.util.caches.response_cache import ResponseCache
@@ -81,11 +82,13 @@ class RoomListHandler(BaseHandler):
             logger.info("No cached result, calculating one.")
             result = self.response_cache.set(
                 key,
-                self._get_public_room_list(
+                preserve_fn(self._get_public_room_list)(
                     limit, since_token, network_tuple=network_tuple
                 )
             )
-        return result
+        else:
+            logger.info("Using cached result.")
+        return make_deferred_yieldable(result)
 
     @defer.inlineCallbacks
     def _get_public_room_list(self, limit=None, since_token=None,
diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py
index 219529936f..b12988f3c9 100644
--- a/synapse/handlers/sync.py
+++ b/synapse/handlers/sync.py
@@ -15,7 +15,7 @@
 
 from synapse.api.constants import Membership, EventTypes
 from synapse.util.async import concurrently_execute
-from synapse.util.logcontext import LoggingContext
+from synapse.util.logcontext import LoggingContext, make_deferred_yieldable, preserve_fn
 from synapse.util.metrics import Measure, measure_func
 from synapse.util.caches.response_cache import ResponseCache
 from synapse.push.clientformat import format_push_rules_for_user
@@ -184,11 +184,11 @@ class SyncHandler(object):
         if not result:
             result = self.response_cache.set(
                 sync_config.request_key,
-                self._wait_for_sync_for_user(
+                preserve_fn(self._wait_for_sync_for_user)(
                     sync_config, since_token, timeout, full_state
                 )
             )
-        return result
+        return make_deferred_yieldable(result)
 
     @defer.inlineCallbacks
     def _wait_for_sync_for_user(self, sync_config, since_token, timeout,
diff --git a/synapse/util/caches/response_cache.py b/synapse/util/caches/response_cache.py
index df0ae099c8..00af539880 100644
--- a/synapse/util/caches/response_cache.py
+++ b/synapse/util/caches/response_cache.py
@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from synapse.util import logcontext
 from synapse.util.async import ObservableDeferred
 
 
@@ -53,4 +52,4 @@ class ResponseCache(object):
             return r
 
         result.addBoth(remove)
-        return logcontext.make_deferred_yieldable(result.observe())
+        return result.observe()
-- 
cgit 1.5.1


From 566e21eac80f1bf921da5876a0fa03ecc412d551 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erikj@jki.re>
Date: Thu, 26 Oct 2017 11:39:54 +0100
Subject: Update room_list.py

---
 synapse/handlers/room_list.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py
index 5a47254b56..2cf34e51cb 100644
--- a/synapse/handlers/room_list.py
+++ b/synapse/handlers/room_list.py
@@ -87,7 +87,7 @@ class RoomListHandler(BaseHandler):
                 )
             )
         else:
-            logger.info("Using cached result.")
+            logger.info("Using cached deferred result.")
         return make_deferred_yieldable(result)
 
     @defer.inlineCallbacks
-- 
cgit 1.5.1


From 785bd7fd75ffd944f6257185fe3129495e5fa6e7 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Fri, 27 Oct 2017 00:01:00 +0100
Subject: Allow ASes to deactivate their own users

---
 synapse/handlers/auth.py                |  2 +-
 synapse/rest/client/v2_alpha/account.py | 48 ++++++++++++++++++++++-----------
 2 files changed, 33 insertions(+), 17 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 9cef9d184b..acae4d9e0d 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -82,7 +82,7 @@ class AuthHandler(BaseHandler):
     def check_auth(self, flows, clientdict, clientip):
         """
         Takes a dictionary sent by the client in the login / registration
-        protocol and handles the login flow.
+        protocol and handles the User-Interactive Auth flow.
 
         As a side effect, this function fills in the 'creds' key on the user's
         session with a map, which maps each auth-type (str) to the relevant
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 4990b22b9f..1a0d57a04a 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -13,22 +13,21 @@
 # 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
 
 from twisted.internet import defer
 
+from synapse.api.auth import has_access_token
 from synapse.api.constants import LoginType
-from synapse.api.errors import LoginError, SynapseError, Codes
+from synapse.api.errors import Codes, LoginError, SynapseError
 from synapse.http.servlet import (
-    RestServlet, parse_json_object_from_request, assert_params_in_request
+    RestServlet, assert_params_in_request,
+    parse_json_object_from_request,
 )
 from synapse.util.async import run_on_reactor
 from synapse.util.msisdn import phone_number_to_msisdn
-
 from ._base import client_v2_patterns
 
-import logging
-
-
 logger = logging.getLogger(__name__)
 
 
@@ -172,6 +171,18 @@ class DeactivateAccountRestServlet(RestServlet):
     def on_POST(self, request):
         body = parse_json_object_from_request(request)
 
+        # if the caller provides an access token, it ought to be valid.
+        requester = None
+        if has_access_token(request):
+            requester = yield self.auth.get_user_by_req(
+                request,
+            )  # type: synapse.types.Requester
+
+        # allow ASes to dectivate their own users
+        if requester and requester.app_service:
+            yield self._deactivate_account(requester.user.to_string())
+            defer.returnValue((200, {}))
+
         authed, result, params, _ = yield self.auth_handler.check_auth([
             [LoginType.PASSWORD],
         ], body, self.hs.get_ip_from_request(request))
@@ -179,27 +190,32 @@ class DeactivateAccountRestServlet(RestServlet):
         if not authed:
             defer.returnValue((401, result))
 
-        user_id = None
-        requester = None
-
         if LoginType.PASSWORD in result:
+            user_id = result[LoginType.PASSWORD]
             # if using password, they should also be logged in
-            requester = yield self.auth.get_user_by_req(request)
-            user_id = requester.user.to_string()
-            if user_id != result[LoginType.PASSWORD]:
+            if requester is None:
+                raise SynapseError(
+                    400,
+                    "Deactivate account requires an access_token",
+                    errcode=Codes.MISSING_TOKEN
+                )
+            if requester.user.to_string() != user_id:
                 raise LoginError(400, "", Codes.UNKNOWN)
         else:
             logger.error("Auth succeeded but no known type!", result.keys())
             raise SynapseError(500, "", Codes.UNKNOWN)
 
-        # FIXME: Theoretically there is a race here wherein user resets password
-        # using threepid.
+        yield self._deactivate_account(user_id)
+        defer.returnValue((200, {}))
+
+    @defer.inlineCallbacks
+    def _deactivate_account(self, user_id):
+        # FIXME: Theoretically there is a race here wherein user resets
+        # password using threepid.
         yield self.store.user_delete_access_tokens(user_id)
         yield self.store.user_delete_threepids(user_id)
         yield self.store.user_set_password_hash(user_id, None)
 
-        defer.returnValue((200, {}))
-
 
 class EmailThreepidRequestTokenRestServlet(RestServlet):
     PATTERNS = client_v2_patterns("/account/3pid/email/requestToken$")
-- 
cgit 1.5.1


From 1b65ae00ac7e15767c2e61036ba349170d0e91b7 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 31 Oct 2017 10:38:40 +0000
Subject: Refactor some logic from LoginRestServlet into AuthHandler

I'm going to need some more flexibility in handling login types in password
auth providers, so as a first step, move some stuff from LoginRestServlet into
AuthHandler.

In particular, we pass everything other than SAML, JWT and token logins down to
the AuthHandler, which now has responsibility for checking the login type and
fishing the password out of the login dictionary, as well as qualifying the
user_id if need be. Ideally SAML, JWT and token would go that way too, but
there's no real need for it right now and I'm trying to minimise impact.

This commit *should* be non-functional.
---
 synapse/handlers/auth.py        | 80 ++++++++++++++++++++++++++---------------
 synapse/rest/client/v1/login.py | 55 ++++++++++++++--------------
 2 files changed, 78 insertions(+), 57 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index acae4d9e0d..93d8ac0e04 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -77,6 +77,12 @@ class AuthHandler(BaseHandler):
         self.hs = hs  # FIXME better possibility to access registrationHandler later?
         self.device_handler = hs.get_device_handler()
         self.macaroon_gen = hs.get_macaroon_generator()
+        self._password_enabled = hs.config.password_enabled
+
+        login_types = set()
+        if self._password_enabled:
+            login_types.add(LoginType.PASSWORD)
+        self._supported_login_types = frozenset(login_types)
 
     @defer.inlineCallbacks
     def check_auth(self, flows, clientdict, clientip):
@@ -266,10 +272,11 @@ class AuthHandler(BaseHandler):
 
         user_id = authdict["user"]
         password = authdict["password"]
-        if not user_id.startswith('@'):
-            user_id = UserID(user_id, self.hs.hostname).to_string()
 
-        return self._check_password(user_id, password)
+        return self.validate_login(user_id, {
+            "type": LoginType.PASSWORD,
+            "password": password,
+        })
 
     @defer.inlineCallbacks
     def _check_recaptcha(self, authdict, clientip):
@@ -398,23 +405,6 @@ class AuthHandler(BaseHandler):
 
         return self.sessions[session_id]
 
-    def validate_password_login(self, user_id, password):
-        """
-        Authenticates the user with their username and password.
-
-        Used only by the v1 login API.
-
-        Args:
-            user_id (str): complete @user:id
-            password (str): Password
-        Returns:
-            defer.Deferred: (str) canonical user id
-        Raises:
-            StoreError if there was a problem accessing the database
-            LoginError if there was an authentication problem.
-        """
-        return self._check_password(user_id, password)
-
     @defer.inlineCallbacks
     def get_access_token_for_user_id(self, user_id, device_id=None,
                                      initial_display_name=None):
@@ -501,26 +491,60 @@ class AuthHandler(BaseHandler):
             )
         defer.returnValue(result)
 
+    def get_supported_login_types(self):
+        """Get a the login types supported for the /login API
+
+        By default this is just 'm.login.password' (unless password_enabled is
+        False in the config file), but password auth providers can provide
+        other login types.
+
+        Returns:
+            Iterable[str]: login types
+        """
+        return self._supported_login_types
+
     @defer.inlineCallbacks
-    def _check_password(self, user_id, password):
-        """Authenticate a user against the LDAP and local databases.
+    def validate_login(self, user_id, login_submission):
+        """Authenticates the user for the /login API
 
-        user_id is checked case insensitively against the local database, but
-        will throw if there are multiple inexact matches.
+        Also used by the user-interactive auth flow to validate
+        m.login.password auth types.
 
         Args:
-            user_id (str): complete @user:id
+            user_id (str): user_id supplied by the user
+            login_submission (dict): the whole of the login submission
+                (including 'type' and other relevant fields)
         Returns:
-            (str) the canonical_user_id
+            Deferred[str]: canonical user id
         Raises:
-            LoginError if login fails
+            StoreError if there was a problem accessing the database
+            SynapseError if there was a problem with the request
+            LoginError if there was an authentication problem.
         """
+
+        if not user_id.startswith('@'):
+            user_id = UserID(
+                user_id, self.hs.hostname
+            ).to_string()
+
+        login_type = login_submission.get("type")
+
+        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")
+
+        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)
 
-        canonical_user_id = yield self._check_local_password(user_id, password)
+        canonical_user_id = yield self._check_local_password(
+            user_id, password,
+        )
 
         if canonical_user_id:
             defer.returnValue(canonical_user_id)
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 9536e8ade6..d24590011b 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -85,7 +85,6 @@ def login_id_thirdparty_from_phone(identifier):
 
 class LoginRestServlet(ClientV1RestServlet):
     PATTERNS = client_path_patterns("/login$")
-    PASS_TYPE = "m.login.password"
     SAML2_TYPE = "m.login.saml2"
     CAS_TYPE = "m.login.cas"
     TOKEN_TYPE = "m.login.token"
@@ -94,7 +93,6 @@ class LoginRestServlet(ClientV1RestServlet):
     def __init__(self, hs):
         super(LoginRestServlet, self).__init__(hs)
         self.idp_redirect_url = hs.config.saml2_idp_redirect_url
-        self.password_enabled = hs.config.password_enabled
         self.saml2_enabled = hs.config.saml2_enabled
         self.jwt_enabled = hs.config.jwt_enabled
         self.jwt_secret = hs.config.jwt_secret
@@ -121,8 +119,10 @@ class LoginRestServlet(ClientV1RestServlet):
             # fall back to the fallback API if they don't understand one of the
             # login flow types returned.
             flows.append({"type": LoginRestServlet.TOKEN_TYPE})
-        if self.password_enabled:
-            flows.append({"type": LoginRestServlet.PASS_TYPE})
+
+        flows.extend((
+            {"type": t} for t in self.auth_handler.get_supported_login_types()
+        ))
 
         return (200, {"flows": flows})
 
@@ -133,14 +133,8 @@ class LoginRestServlet(ClientV1RestServlet):
     def on_POST(self, request):
         login_submission = parse_json_object_from_request(request)
         try:
-            if login_submission["type"] == LoginRestServlet.PASS_TYPE:
-                if not self.password_enabled:
-                    raise SynapseError(400, "Password login has been disabled.")
-
-                result = yield self.do_password_login(login_submission)
-                defer.returnValue(result)
-            elif self.saml2_enabled and (login_submission["type"] ==
-                                         LoginRestServlet.SAML2_TYPE):
+            if self.saml2_enabled and (login_submission["type"] ==
+                                       LoginRestServlet.SAML2_TYPE):
                 relay_state = ""
                 if "relay_state" in login_submission:
                     relay_state = "&RelayState=" + urllib.quote(
@@ -157,15 +151,21 @@ class LoginRestServlet(ClientV1RestServlet):
                 result = yield self.do_token_login(login_submission)
                 defer.returnValue(result)
             else:
-                raise SynapseError(400, "Bad login type.")
+                result = yield self._do_other_login(login_submission)
+                defer.returnValue(result)
         except KeyError:
             raise SynapseError(400, "Missing JSON keys.")
 
     @defer.inlineCallbacks
-    def do_password_login(self, login_submission):
-        if "password" not in login_submission:
-            raise SynapseError(400, "Missing parameter: password")
+    def _do_other_login(self, login_submission):
+        """Handle non-token/saml/jwt logins
 
+        Args:
+            login_submission:
+
+        Returns:
+            (int, object): HTTP code/response
+        """
         login_submission_legacy_convert(login_submission)
 
         if "identifier" not in login_submission:
@@ -208,25 +208,22 @@ class LoginRestServlet(ClientV1RestServlet):
         if "user" not in identifier:
             raise SynapseError(400, "User identifier is missing 'user' key")
 
-        user_id = identifier["user"]
-
-        if not user_id.startswith('@'):
-            user_id = UserID(
-                user_id, self.hs.hostname
-            ).to_string()
-
         auth_handler = self.auth_handler
-        user_id = yield auth_handler.validate_password_login(
-            user_id=user_id,
-            password=login_submission["password"],
+        canonical_user_id = yield auth_handler.validate_login(
+            identifier["user"],
+            login_submission,
+        )
+
+        device_id = yield self._register_device(
+            canonical_user_id, login_submission,
         )
-        device_id = yield self._register_device(user_id, login_submission)
         access_token = yield auth_handler.get_access_token_for_user_id(
-            user_id, device_id,
+            canonical_user_id, device_id,
             login_submission.get("initial_device_display_name"),
         )
+
         result = {
-            "user_id": user_id,  # may have changed
+            "user_id": canonical_user_id,
             "access_token": access_token,
             "home_server": self.hs.hostname,
             "device_id": device_id,
-- 
cgit 1.5.1


From 20fe347906355e6eec2b890f5d9dc8e2c4534ce3 Mon Sep 17 00:00:00 2001
From: Luke Barnard <lukeb@openmarket.com>
Date: Tue, 31 Oct 2017 17:04:28 +0000
Subject: Modify group room association API to allow modification of is_public

also includes renamings to make things more consistent.
---
 synapse/federation/transport/client.py |  4 ++--
 synapse/federation/transport/server.py |  4 ++--
 synapse/groups/groups_server.py        |  8 ++++----
 synapse/handlers/groups_local.py       |  4 ++--
 synapse/rest/client/v2_alpha/groups.py |  4 ++--
 synapse/storage/group_server.py        | 20 +++++++++++++-------
 6 files changed, 25 insertions(+), 19 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index d25ae1b282..2fcbb7069b 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -531,7 +531,7 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
-    def add_room_to_group(self, destination, group_id, requester_user_id, room_id,
+    def update_room_group_association(self, destination, group_id, requester_user_id, room_id,
                           content):
         """Add a room to a group
         """
@@ -545,7 +545,7 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
-    def remove_room_from_group(self, destination, group_id, requester_user_id, room_id):
+    def delete_room_group_association(self, destination, group_id, requester_user_id, room_id):
         """Remove a room from a group
         """
         path = PREFIX + "/groups/%s/room/%s" % (group_id, room_id,)
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 8f3c14c303..ded6d4edc9 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -684,7 +684,7 @@ class FederationGroupsAddRoomsServlet(BaseFederationServlet):
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
 
-        new_content = yield self.handler.add_room_to_group(
+        new_content = yield self.handler.update_room_group_association(
             group_id, requester_user_id, room_id, content
         )
 
@@ -696,7 +696,7 @@ class FederationGroupsAddRoomsServlet(BaseFederationServlet):
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
 
-        new_content = yield self.handler.remove_room_from_group(
+        new_content = yield self.handler.delete_room_group_association(
             group_id, requester_user_id, room_id,
         )
 
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index dedc9cc7fd..c91d8e624f 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -531,7 +531,7 @@ class GroupsServerHandler(object):
         })
 
     @defer.inlineCallbacks
-    def add_room_to_group(self, group_id, requester_user_id, room_id, content):
+    def update_room_group_association(self, group_id, requester_user_id, room_id, content):
         """Add room to group
         """
         RoomID.from_string(room_id)  # Ensure valid room id
@@ -542,19 +542,19 @@ class GroupsServerHandler(object):
 
         is_public = _parse_visibility_from_contents(content)
 
-        yield self.store.add_room_to_group(group_id, room_id, is_public=is_public)
+        yield self.store.update_room_group_association(group_id, room_id, is_public=is_public)
 
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def remove_room_from_group(self, group_id, requester_user_id, room_id):
+    def delete_room_group_association(self, group_id, requester_user_id, room_id):
         """Remove room from group
         """
         yield self.check_group_is_ours(
             group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
-        yield self.store.remove_room_from_group(group_id, room_id)
+        yield self.store.delete_room_group_association(group_id, room_id)
 
         defer.returnValue({})
 
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index 6699d0888f..dabc2a3fbb 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -70,8 +70,8 @@ class GroupsLocalHandler(object):
 
     get_invited_users_in_group = _create_rerouter("get_invited_users_in_group")
 
-    add_room_to_group = _create_rerouter("add_room_to_group")
-    remove_room_from_group = _create_rerouter("remove_room_from_group")
+    update_room_group_association = _create_rerouter("update_room_group_association")
+    delete_room_group_association = _create_rerouter("delete_room_group_association")
 
     update_group_summary_room = _create_rerouter("update_group_summary_room")
     delete_group_summary_room = _create_rerouter("delete_group_summary_room")
diff --git a/synapse/rest/client/v2_alpha/groups.py b/synapse/rest/client/v2_alpha/groups.py
index c97885cfc7..792608cd48 100644
--- a/synapse/rest/client/v2_alpha/groups.py
+++ b/synapse/rest/client/v2_alpha/groups.py
@@ -451,7 +451,7 @@ class GroupAdminRoomsServlet(RestServlet):
         requester_user_id = requester.user.to_string()
 
         content = parse_json_object_from_request(request)
-        result = yield self.groups_handler.add_room_to_group(
+        result = yield self.groups_handler.update_room_group_association(
             group_id, requester_user_id, room_id, content,
         )
 
@@ -462,7 +462,7 @@ class GroupAdminRoomsServlet(RestServlet):
         requester = yield self.auth.get_user_by_req(request)
         requester_user_id = requester.user.to_string()
 
-        result = yield self.groups_handler.remove_room_from_group(
+        result = yield self.groups_handler.delete_room_group_association(
             group_id, requester_user_id, room_id,
         )
 
diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py
index 8c4ad0a9a9..a7a43de279 100644
--- a/synapse/storage/group_server.py
+++ b/synapse/storage/group_server.py
@@ -846,19 +846,25 @@ class GroupServerStore(SQLBaseStore):
             )
         return self.runInteraction("remove_user_from_group", _remove_user_from_group_txn)
 
-    def add_room_to_group(self, group_id, room_id, is_public):
-        return self._simple_insert(
+    def update_room_group_association(self, group_id, room_id, is_public=True):
+        return self._simple_upsert(
             table="group_rooms",
-            values={
+            keyvalues={
                 "group_id": group_id,
                 "room_id": room_id,
+            },
+            values={
                 "is_public": is_public,
             },
-            desc="add_room_to_group",
+            insertion_values={
+                "group_id": group_id,
+                "room_id": room_id,
+            },
+            desc="update_room_group_association",
         )
 
-    def remove_room_from_group(self, group_id, room_id):
-        def _remove_room_from_group_txn(txn):
+    def delete_room_group_association(self, group_id, room_id):
+        def _delete_room_group_association_txn(txn):
             self._simple_delete_txn(
                 txn,
                 table="group_rooms",
@@ -877,7 +883,7 @@ class GroupServerStore(SQLBaseStore):
                 },
             )
         return self.runInteraction(
-            "remove_room_from_group", _remove_room_from_group_txn,
+            "delete_room_group_association", _delete_room_group_association_txn,
         )
 
     def get_publicised_groups_for_user(self, user_id):
-- 
cgit 1.5.1


From 3e0aaad1903cb942920b06ba5eeb345d0256af19 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 31 Oct 2017 16:20:11 +0000
Subject: Let auth providers get to the database

Somewhat open to abuse, but also somewhat unavoidable :/
---
 synapse/handlers/auth.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 93d8ac0e04..12c50f32f2 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -730,6 +730,7 @@ class _AccountHandler(object):
         self.hs = hs
 
         self._check_user_exists = check_user_exists
+        self._store = hs.get_datastore()
 
     def check_user_exists(self, user_id):
         """Check if user exissts.
@@ -747,3 +748,18 @@ class _AccountHandler(object):
         """
         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 356bcafc4452d1cf5deea61c5084fc25a54a5ead Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 31 Oct 2017 20:35:58 +0000
Subject: Remove the last vestiges of refresh_tokens

---
 synapse/handlers/device.py                         |  2 --
 synapse/storage/registration.py                    | 29 ++++++++--------------
 synapse/storage/schema/delta/23/refresh_tokens.sql | 21 ----------------
 .../schema/delta/33/refreshtoken_device.sql        | 16 ------------
 .../schema/delta/33/refreshtoken_device_index.sql  | 17 -------------
 .../schema/delta/46/drop_refresh_tokens.sql        | 17 +++++++++++++
 6 files changed, 28 insertions(+), 74 deletions(-)
 delete mode 100644 synapse/storage/schema/delta/23/refresh_tokens.sql
 delete mode 100644 synapse/storage/schema/delta/33/refreshtoken_device.sql
 delete mode 100644 synapse/storage/schema/delta/33/refreshtoken_device_index.sql
 create mode 100644 synapse/storage/schema/delta/46/drop_refresh_tokens.sql

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index dac4b3f4e0..94fc08446b 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -161,7 +161,6 @@ class DeviceHandler(BaseHandler):
 
         yield self.store.user_delete_access_tokens(
             user_id, device_id=device_id,
-            delete_refresh_tokens=True,
         )
 
         yield self.store.delete_e2e_keys_by_device(
@@ -196,7 +195,6 @@ class DeviceHandler(BaseHandler):
         for device_id in device_ids:
             yield self.store.user_delete_access_tokens(
                 user_id, device_id=device_id,
-                delete_refresh_tokens=True,
             )
             yield self.store.delete_e2e_keys_by_device(
                 user_id=user_id, device_id=device_id
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index 20acd58fcf..3d3bdba894 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -36,12 +36,15 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
             columns=["user_id", "device_id"],
         )
 
-        self.register_background_index_update(
-            "refresh_tokens_device_index",
-            index_name="refresh_tokens_device_id",
-            table="refresh_tokens",
-            columns=["user_id", "device_id"],
-        )
+        # we no longer use refresh tokens, but it's possible that some people
+        # might have a background update queued to build this index. Just
+        # clear the background update.
+        @defer.inlineCallbacks
+        def noop_update(progress, batch_size):
+            yield self._end_background_update("refresh_tokens_device_index")
+            defer.returnValue(1)
+        self.register_background_update_handler(
+            "refresh_tokens_device_index", noop_update)
 
     @defer.inlineCallbacks
     def add_access_token_to_user(self, user_id, token, device_id=None):
@@ -238,10 +241,9 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
 
     @defer.inlineCallbacks
     def user_delete_access_tokens(self, user_id, except_token_id=None,
-                                  device_id=None,
-                                  delete_refresh_tokens=False):
+                                  device_id=None):
         """
-        Invalidate access/refresh tokens belonging to a user
+        Invalidate access tokens belonging to a user
 
         Args:
             user_id (str):  ID of user the tokens belong to
@@ -250,8 +252,6 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
             device_id (str|None):  ID of device the tokens are associated with.
                 If None, tokens associated with any device (or no device) will
                 be deleted
-            delete_refresh_tokens (bool):  True to delete refresh tokens as
-                well as access tokens.
         Returns:
             defer.Deferred:
         """
@@ -262,13 +262,6 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
             if device_id is not None:
                 keyvalues["device_id"] = device_id
 
-            if delete_refresh_tokens:
-                self._simple_delete_txn(
-                    txn,
-                    table="refresh_tokens",
-                    keyvalues=keyvalues,
-                )
-
             items = keyvalues.items()
             where_clause = " AND ".join(k + " = ?" for k, _ in items)
             values = [v for _, v in items]
diff --git a/synapse/storage/schema/delta/23/refresh_tokens.sql b/synapse/storage/schema/delta/23/refresh_tokens.sql
deleted file mode 100644
index 34db0cf12b..0000000000
--- a/synapse/storage/schema/delta/23/refresh_tokens.sql
+++ /dev/null
@@ -1,21 +0,0 @@
-/* Copyright 2015, 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.
- */
-
-CREATE TABLE IF NOT EXISTS refresh_tokens(
-    id INTEGER PRIMARY KEY,
-    token TEXT NOT NULL,
-    user_id TEXT NOT NULL,
-    UNIQUE (token)
-);
diff --git a/synapse/storage/schema/delta/33/refreshtoken_device.sql b/synapse/storage/schema/delta/33/refreshtoken_device.sql
deleted file mode 100644
index 290bd6da86..0000000000
--- a/synapse/storage/schema/delta/33/refreshtoken_device.sql
+++ /dev/null
@@ -1,16 +0,0 @@
-/* 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.
- */
-
-ALTER TABLE refresh_tokens ADD COLUMN device_id TEXT;
diff --git a/synapse/storage/schema/delta/33/refreshtoken_device_index.sql b/synapse/storage/schema/delta/33/refreshtoken_device_index.sql
deleted file mode 100644
index bb225dafbf..0000000000
--- a/synapse/storage/schema/delta/33/refreshtoken_device_index.sql
+++ /dev/null
@@ -1,17 +0,0 @@
-/* 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.
- */
-
-INSERT INTO background_updates (update_name, progress_json) VALUES
-  ('refresh_tokens_device_index', '{}');
diff --git a/synapse/storage/schema/delta/46/drop_refresh_tokens.sql b/synapse/storage/schema/delta/46/drop_refresh_tokens.sql
new file mode 100644
index 0000000000..68c48a89a9
--- /dev/null
+++ b/synapse/storage/schema/delta/46/drop_refresh_tokens.sql
@@ -0,0 +1,17 @@
+/* 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.
+ */
+
+/* we no longer use (or create) the refresh_tokens table */
+DROP TABLE IF EXISTS refresh_tokens;
-- 
cgit 1.5.1


From 74c56f794cc33adb52746bf76c49ec5ca1edebed Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Wed, 1 Nov 2017 10:23:21 +0000
Subject: Break dependency of auth_handler on device_handler

I'm going to need to make the device_handler depend on the auth_handler, so I
need to break this dependency to avoid a cycle.

It turns out that the auth_handler was only using the device_handler in one
place which was an edge case which we can more elegantly handle by throwing an
error rather than fixing it up.
---
 synapse/handlers/auth.py                 | 15 ++++++---------
 synapse/rest/client/v1/login.py          |  3 ---
 synapse/rest/client/v2_alpha/register.py |  1 -
 3 files changed, 6 insertions(+), 13 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 93d8ac0e04..051995bcce 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -75,7 +75,6 @@ class AuthHandler(BaseHandler):
         logger.info("Extra password_providers: %r", self.password_providers)
 
         self.hs = hs  # FIXME better possibility to access registrationHandler later?
-        self.device_handler = hs.get_device_handler()
         self.macaroon_gen = hs.get_macaroon_generator()
         self._password_enabled = hs.config.password_enabled
 
@@ -406,8 +405,7 @@ class AuthHandler(BaseHandler):
         return self.sessions[session_id]
 
     @defer.inlineCallbacks
-    def get_access_token_for_user_id(self, user_id, device_id=None,
-                                     initial_display_name=None):
+    def get_access_token_for_user_id(self, user_id, device_id=None):
         """
         Creates a new access token for the user with the given user ID.
 
@@ -421,13 +419,10 @@ class AuthHandler(BaseHandler):
             device_id (str|None): the device ID to associate with the tokens.
                None to leave the tokens unassociated with a device (deprecated:
                we should always have a device ID)
-            initial_display_name (str): display name to associate with the
-               device if it needs re-registering
         Returns:
               The access token for the user's session.
         Raises:
             StoreError if there was a problem storing the token.
-            LoginError if there was an authentication problem.
         """
         logger.info("Logging in user %s on device %s", user_id, device_id)
         access_token = yield self.issue_access_token(user_id, device_id)
@@ -437,9 +432,11 @@ class AuthHandler(BaseHandler):
         # really don't want is active access_tokens without a record of the
         # device, so we double-check it here.
         if device_id is not None:
-            yield self.device_handler.check_device_registered(
-                user_id, device_id, initial_display_name
-            )
+            try:
+                yield self.store.get_device(user_id, device_id)
+            except StoreError:
+                yield self.store.delete_access_token(access_token)
+                raise StoreError(400, "Login raced against device deletion")
 
         defer.returnValue(access_token)
 
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index d24590011b..55d2fb056e 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -219,7 +219,6 @@ class LoginRestServlet(ClientV1RestServlet):
         )
         access_token = yield auth_handler.get_access_token_for_user_id(
             canonical_user_id, device_id,
-            login_submission.get("initial_device_display_name"),
         )
 
         result = {
@@ -241,7 +240,6 @@ class LoginRestServlet(ClientV1RestServlet):
         device_id = yield self._register_device(user_id, login_submission)
         access_token = yield auth_handler.get_access_token_for_user_id(
             user_id, device_id,
-            login_submission.get("initial_device_display_name"),
         )
         result = {
             "user_id": user_id,  # may have changed
@@ -284,7 +282,6 @@ class LoginRestServlet(ClientV1RestServlet):
             )
             access_token = yield auth_handler.get_access_token_for_user_id(
                 registered_user_id, device_id,
-                login_submission.get("initial_device_display_name"),
             )
 
             result = {
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index d9a8cdbbb5..a077146c89 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -566,7 +566,6 @@ class RegisterRestServlet(RestServlet):
         access_token = (
             yield self.auth_handler.get_access_token_for_user_id(
                 user_id, device_id=device_id,
-                initial_display_name=params.get("initial_device_display_name")
             )
         )
 
-- 
cgit 1.5.1


From dd13310fb8ca0cfce60e4fccdb93e90a16078609 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Wed, 1 Nov 2017 10:29:34 +0000
Subject: Move access token deletion into auth handler

Also move duplicated deactivation code into the auth handler.

I want to add some hooks when we deactivate an access token, so let's bring it
all in here so that there's somewhere to put it.
---
 synapse/handlers/auth.py                | 49 +++++++++++++++++++++++++++++++--
 synapse/handlers/device.py              |  5 ++--
 synapse/handlers/register.py            |  3 +-
 synapse/rest/client/v1/admin.py         |  9 ++----
 synapse/rest/client/v1/logout.py        |  8 +++---
 synapse/rest/client/v2_alpha/account.py | 15 +++-------
 6 files changed, 62 insertions(+), 27 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 93d8ac0e04..1a90c10b01 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -608,13 +608,58 @@ class AuthHandler(BaseHandler):
             if e.code == 404:
                 raise SynapseError(404, "Unknown user", Codes.NOT_FOUND)
             raise e
-        yield self.store.user_delete_access_tokens(
-            user_id, except_access_token_id
+        yield self.delete_access_tokens_for_user(
+            user_id, except_token_id=except_access_token_id,
         )
         yield self.hs.get_pusherpool().remove_pushers_by_user(
             user_id, except_access_token_id
         )
 
+    @defer.inlineCallbacks
+    def deactivate_account(self, user_id):
+        """Deactivate a user's account
+
+        Args:
+            user_id (str): ID of user to be deactivated
+
+        Returns:
+            Deferred
+        """
+        # FIXME: Theoretically there is a race here wherein user resets
+        # password using threepid.
+        yield self.delete_access_tokens_for_user(user_id)
+        yield self.store.user_delete_threepids(user_id)
+        yield self.store.user_set_password_hash(user_id, None)
+
+    def delete_access_token(self, access_token):
+        """Invalidate a single access token
+
+        Args:
+            access_token (str): access token to be deleted
+
+        Returns:
+            Deferred
+        """
+        return self.store.delete_access_token(access_token)
+
+    def delete_access_tokens_for_user(self, user_id, except_token_id=None,
+                                      device_id=None):
+        """Invalidate access tokens belonging to a user
+
+        Args:
+            user_id (str):  ID of user the tokens belong to
+            except_token_id (str|None): access_token ID which should *not* be
+                deleted
+            device_id (str|None):  ID of device the tokens are associated with.
+                If None, tokens associated with any device (or no device) will
+                be deleted
+        Returns:
+            Deferred
+        """
+        return self.store.user_delete_access_tokens(
+            user_id, except_token_id=except_token_id, device_id=device_id,
+        )
+
     @defer.inlineCallbacks
     def add_threepid(self, user_id, medium, address, validated_at):
         # 'Canonicalise' email addresses down to lower case.
diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index dac4b3f4e0..5201e8be16 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -34,6 +34,7 @@ class DeviceHandler(BaseHandler):
 
         self.hs = hs
         self.state = hs.get_state_handler()
+        self._auth_handler = hs.get_auth_handler()
         self.federation_sender = hs.get_federation_sender()
         self.federation = hs.get_replication_layer()
 
@@ -159,7 +160,7 @@ class DeviceHandler(BaseHandler):
             else:
                 raise
 
-        yield self.store.user_delete_access_tokens(
+        yield self._auth_handler.delete_access_tokens_for_user(
             user_id, device_id=device_id,
             delete_refresh_tokens=True,
         )
@@ -194,7 +195,7 @@ class DeviceHandler(BaseHandler):
         # Delete access tokens and e2e keys for each device. Not optimised as it is not
         # considered as part of a critical path.
         for device_id in device_ids:
-            yield self.store.user_delete_access_tokens(
+            yield self._auth_handler.delete_access_tokens_for_user(
                 user_id, device_id=device_id,
                 delete_refresh_tokens=True,
             )
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 49dc33c147..f6e7e58563 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -36,6 +36,7 @@ class RegistrationHandler(BaseHandler):
         super(RegistrationHandler, self).__init__(hs)
 
         self.auth = hs.get_auth()
+        self._auth_handler = hs.get_auth_handler()
         self.profile_handler = hs.get_profile_handler()
         self.captcha_client = CaptchaServerHttpClient(hs)
 
@@ -416,7 +417,7 @@ class RegistrationHandler(BaseHandler):
                 create_profile_with_localpart=user.localpart,
             )
         else:
-            yield self.store.user_delete_access_tokens(user_id=user_id)
+            yield self._auth_handler.delete_access_tokens_for_user(user_id)
             yield self.store.add_access_token_to_user(user_id=user_id, token=token)
 
         if displayname is not None:
diff --git a/synapse/rest/client/v1/admin.py b/synapse/rest/client/v1/admin.py
index 465b25033d..1197158fdc 100644
--- a/synapse/rest/client/v1/admin.py
+++ b/synapse/rest/client/v1/admin.py
@@ -137,7 +137,7 @@ class DeactivateAccountRestServlet(ClientV1RestServlet):
     PATTERNS = client_path_patterns("/admin/deactivate/(?P<target_user_id>[^/]*)")
 
     def __init__(self, hs):
-        self.store = hs.get_datastore()
+        self._auth_handler = hs.get_auth_handler()
         super(DeactivateAccountRestServlet, self).__init__(hs)
 
     @defer.inlineCallbacks
@@ -149,12 +149,7 @@ class DeactivateAccountRestServlet(ClientV1RestServlet):
         if not is_admin:
             raise AuthError(403, "You are not a server admin")
 
-        # FIXME: Theoretically there is a race here wherein user resets password
-        # using threepid.
-        yield self.store.user_delete_access_tokens(target_user_id)
-        yield self.store.user_delete_threepids(target_user_id)
-        yield self.store.user_set_password_hash(target_user_id, None)
-
+        yield self._auth_handler.deactivate_account(target_user_id)
         defer.returnValue((200, {}))
 
 
diff --git a/synapse/rest/client/v1/logout.py b/synapse/rest/client/v1/logout.py
index 1358d0acab..6add754782 100644
--- a/synapse/rest/client/v1/logout.py
+++ b/synapse/rest/client/v1/logout.py
@@ -30,7 +30,7 @@ class LogoutRestServlet(ClientV1RestServlet):
 
     def __init__(self, hs):
         super(LogoutRestServlet, self).__init__(hs)
-        self.store = hs.get_datastore()
+        self._auth_handler = hs.get_auth_handler()
 
     def on_OPTIONS(self, request):
         return (200, {})
@@ -38,7 +38,7 @@ class LogoutRestServlet(ClientV1RestServlet):
     @defer.inlineCallbacks
     def on_POST(self, request):
         access_token = get_access_token_from_request(request)
-        yield self.store.delete_access_token(access_token)
+        yield self._auth_handler.delete_access_token(access_token)
         defer.returnValue((200, {}))
 
 
@@ -47,8 +47,8 @@ class LogoutAllRestServlet(ClientV1RestServlet):
 
     def __init__(self, hs):
         super(LogoutAllRestServlet, self).__init__(hs)
-        self.store = hs.get_datastore()
         self.auth = hs.get_auth()
+        self._auth_handler = hs.get_auth_handler()
 
     def on_OPTIONS(self, request):
         return (200, {})
@@ -57,7 +57,7 @@ class LogoutAllRestServlet(ClientV1RestServlet):
     def on_POST(self, request):
         requester = yield self.auth.get_user_by_req(request)
         user_id = requester.user.to_string()
-        yield self.store.user_delete_access_tokens(user_id)
+        yield self._auth_handler.delete_access_tokens_for_user(user_id)
         defer.returnValue((200, {}))
 
 
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 1a0d57a04a..3062e04c59 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -162,7 +162,6 @@ class DeactivateAccountRestServlet(RestServlet):
 
     def __init__(self, hs):
         self.hs = hs
-        self.store = hs.get_datastore()
         self.auth = hs.get_auth()
         self.auth_handler = hs.get_auth_handler()
         super(DeactivateAccountRestServlet, self).__init__()
@@ -180,7 +179,9 @@ class DeactivateAccountRestServlet(RestServlet):
 
         # allow ASes to dectivate their own users
         if requester and requester.app_service:
-            yield self._deactivate_account(requester.user.to_string())
+            yield self.auth_handler.deactivate_account(
+                requester.user.to_string()
+            )
             defer.returnValue((200, {}))
 
         authed, result, params, _ = yield self.auth_handler.check_auth([
@@ -205,17 +206,9 @@ class DeactivateAccountRestServlet(RestServlet):
             logger.error("Auth succeeded but no known type!", result.keys())
             raise SynapseError(500, "", Codes.UNKNOWN)
 
-        yield self._deactivate_account(user_id)
+        yield self.auth_handler.deactivate_account(user_id)
         defer.returnValue((200, {}))
 
-    @defer.inlineCallbacks
-    def _deactivate_account(self, user_id):
-        # FIXME: Theoretically there is a race here wherein user resets
-        # password using threepid.
-        yield self.store.user_delete_access_tokens(user_id)
-        yield self.store.user_delete_threepids(user_id)
-        yield self.store.user_set_password_hash(user_id, None)
-
 
 class EmailThreepidRequestTokenRestServlet(RestServlet):
     PATTERNS = client_v2_patterns("/account/3pid/email/requestToken$")
-- 
cgit 1.5.1


From 3cd6b22c7bf0aa0108535bad5656a0d2d9e85634 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
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 'synapse/handlers')

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 <richard@matrix.org>
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 'synapse/handlers')

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 <richard@matrix.org>
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 'synapse/handlers')

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 979eed43627e3d13a8a09d4904c80d84b0b6e609 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Wed, 1 Nov 2017 17:03:20 +0000
Subject: Fix user-interactive password auth

this got broken in the previous commit
---
 synapse/handlers/auth.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 5c89768c14..11d2b804d4 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -270,6 +270,7 @@ class AuthHandler(BaseHandler):
         sess = self._get_session_info(session_id)
         return sess.setdefault('serverdict', {}).get(key, default)
 
+    @defer.inlineCallbacks
     def _check_password_auth(self, authdict, _):
         if "user" not in authdict or "password" not in authdict:
             raise LoginError(400, "", Codes.MISSING_PARAM)
@@ -277,10 +278,11 @@ class AuthHandler(BaseHandler):
         user_id = authdict["user"]
         password = authdict["password"]
 
-        return self.validate_login(user_id, {
+        (canonical_id, callback) = yield self.validate_login(user_id, {
             "type": LoginType.PASSWORD,
             "password": password,
         })
+        defer.returnValue(canonical_id)
 
     @defer.inlineCallbacks
     def _check_recaptcha(self, authdict, clientip):
-- 
cgit 1.5.1


From 1189be43a2479f5adf034613e8d10e3f4f452eb9 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
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 'synapse/handlers')

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 45fbe4ff67b7a7b51cbe474572947dc8e7b8c0da Mon Sep 17 00:00:00 2001
From: Ilya Zhuravlev <whatever@xyz.is>
Date: Thu, 2 Nov 2017 22:49:43 +0300
Subject: Fix appservices being backlogged and not receiving new events due to
 a bug in notify_interested_services

---
 synapse/handlers/appservice.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py
index 05af54d31b..5ce752a196 100644
--- a/synapse/handlers/appservice.py
+++ b/synapse/handlers/appservice.py
@@ -74,7 +74,7 @@ class ApplicationServicesHandler(object):
                 limit = 100
                 while True:
                     upper_bound, events = yield self.store.get_new_events_for_appservice(
-                        upper_bound, limit
+                        self.current_max, limit
                     )
 
                     if not events:
@@ -105,9 +105,6 @@ class ApplicationServicesHandler(object):
                             )
 
                     yield self.store.set_appservice_last_pos(upper_bound)
-
-                    if len(events) < limit:
-                        break
             finally:
                 self.is_processing = False
 
-- 
cgit 1.5.1


From fa4f337b49b9a417cbb3a555cd959b1be36cc666 Mon Sep 17 00:00:00 2001
From: Francois Granade <github.com@farialima.net>
Date: Fri, 3 Nov 2017 18:25:04 +0100
Subject: Fix for issue 2635: correctly update rooms avatar/display name when
 modified by admin

---
 synapse/handlers/profile.py | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index 62b9bd503e..d6646825d8 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -140,7 +140,7 @@ class ProfileHandler(BaseHandler):
             target_user.localpart, new_displayname
         )
 
-        yield self._update_join_states(requester)
+        yield self._update_join_states(requester, target_user)
 
     @defer.inlineCallbacks
     def get_avatar_url(self, target_user):
@@ -184,7 +184,7 @@ class ProfileHandler(BaseHandler):
             target_user.localpart, new_avatar_url
         )
 
-        yield self._update_join_states(requester)
+        yield self._update_join_states(requester, target_user)
 
     @defer.inlineCallbacks
     def on_profile_query(self, args):
@@ -209,28 +209,24 @@ class ProfileHandler(BaseHandler):
         defer.returnValue(response)
 
     @defer.inlineCallbacks
-    def _update_join_states(self, requester):
-        user = requester.user
-        if not self.hs.is_mine(user):
+    def _update_join_states(self, requester, target_user):
+        if not self.hs.is_mine(target_user):
             return
 
         yield self.ratelimit(requester)
 
         room_ids = yield self.store.get_rooms_for_user(
-            user.to_string(),
+            target_user.to_string(),
         )
 
         for room_id in room_ids:
             handler = self.hs.get_handlers().room_member_handler
             try:
-                # Assume the user isn't a guest because we don't let guests set
-                # profile or avatar data.
-                # XXX why are we recreating `requester` here for each room?
-                # what was wrong with the `requester` we were passed?
-                requester = synapse.types.create_requester(user)
+                # Assume the target_user isn't a guest,
+                # because we don't let guests set profile or avatar data.
                 yield handler.update_membership(
                     requester,
-                    user,
+                    target_user,
                     room_id,
                     "join",  # We treat a profile update like a join.
                     ratelimit=False,  # Try to hide that these events aren't atomic.
-- 
cgit 1.5.1


From f103b91ffa536d3d36697c159d6c13a7b952ba3a Mon Sep 17 00:00:00 2001
From: Francois Granade <github.com@farialima.net>
Date: Fri, 3 Nov 2017 18:45:49 +0100
Subject: removed unused import flagged by flake8a

---
 synapse/handlers/profile.py | 1 -
 1 file changed, 1 deletion(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index d6646825d8..5e5b1952dd 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -17,7 +17,6 @@ import logging
 
 from twisted.internet import defer
 
-import synapse.types
 from synapse.api.errors import SynapseError, AuthError, CodeMessageException
 from synapse.types import UserID, get_domain_from_id
 from ._base import BaseHandler
-- 
cgit 1.5.1


From d1622e080f6711ef1fef52560fc286212f2cbeb4 Mon Sep 17 00:00:00 2001
From: Matthew Hodgson <matthew@matrix.org>
Date: Sat, 4 Nov 2017 19:35:14 +0000
Subject: s/intial/initial/

---
 synapse/handlers/user_directory.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py
index 2a49456bfc..b5be5d9623 100644
--- a/synapse/handlers/user_directory.py
+++ b/synapse/handlers/user_directory.py
@@ -152,7 +152,7 @@ class UserDirectoyHandler(object):
 
         for room_id in room_ids:
             logger.info("Handling room %d/%d", num_processed_rooms, len(room_ids))
-            yield self._handle_intial_room(room_id)
+            yield self._handle_initial_room(room_id)
             num_processed_rooms += 1
             yield sleep(self.INITIAL_SLEEP_MS / 1000.)
 
@@ -166,7 +166,7 @@ class UserDirectoyHandler(object):
         yield self.store.update_user_directory_stream_pos(new_pos)
 
     @defer.inlineCallbacks
-    def _handle_intial_room(self, room_id):
+    def _handle_initial_room(self, room_id):
         """Called when we initially fill out user_directory one room at a time
         """
         is_in_room = yield self.store.is_host_joined(room_id, self.server_name)
-- 
cgit 1.5.1


From 1ca42881350b26f58172dd2c82fcfd8d45ddd5c0 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 7 Nov 2017 16:43:00 +0000
Subject: factor out _update_context_for_auth_events

This is duplicated, so let's factor it out before fixing it
---
 synapse/handlers/federation.py | 62 ++++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 20 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 8b1e606754..6cceb8998e 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1706,6 +1706,17 @@ class FederationHandler(BaseHandler):
     @defer.inlineCallbacks
     @log_function
     def do_auth(self, origin, event, context, auth_events):
+        """
+
+        Args:
+            origin (str):
+            event (synapse.events.FrozenEvent):
+            context (synapse.events.snapshot.EventContext):
+            auth_events (dict[(str, str)->str]):
+
+        Returns:
+            defer.Deferred[None]
+        """
         # Check if we have all the auth events.
         current_state = set(e.event_id for e in auth_events.values())
         event_auth_events = set(e_id for e_id, _ in event.auth_events)
@@ -1817,16 +1828,9 @@ class FederationHandler(BaseHandler):
                 current_state = set(e.event_id for e in auth_events.values())
                 different_auth = event_auth_events - current_state
 
-                context.current_state_ids = dict(context.current_state_ids)
-                context.current_state_ids.update({
-                    k: a.event_id for k, a in auth_events.items()
-                    if k != event_key
-                })
-                context.prev_state_ids = dict(context.prev_state_ids)
-                context.prev_state_ids.update({
-                    k: a.event_id for k, a in auth_events.items()
-                })
-                context.state_group = self.store.get_next_state_group()
+                self._update_context_for_auth_events(
+                    context, auth_events, event_key,
+                )
 
         if different_auth and not event.internal_metadata.is_outlier():
             logger.info("Different auth after resolution: %s", different_auth)
@@ -1906,16 +1910,9 @@ class FederationHandler(BaseHandler):
                 # 4. Look at rejects and their proofs.
                 # TODO.
 
-                context.current_state_ids = dict(context.current_state_ids)
-                context.current_state_ids.update({
-                    k: a.event_id for k, a in auth_events.items()
-                    if k != event_key
-                })
-                context.prev_state_ids = dict(context.prev_state_ids)
-                context.prev_state_ids.update({
-                    k: a.event_id for k, a in auth_events.items()
-                })
-                context.state_group = self.store.get_next_state_group()
+                self._update_context_for_auth_events(
+                    context, auth_events, event_key,
+                )
 
         try:
             self.auth.check(event, auth_events=auth_events)
@@ -1923,6 +1920,31 @@ class FederationHandler(BaseHandler):
             logger.warn("Failed auth resolution for %r because %s", event, e)
             raise e
 
+    def _update_context_for_auth_events(self, context, auth_events,
+                                        event_key):
+        """Update the state_ids in an event context after auth event resolution
+
+        Args:
+            context (synapse.events.snapshot.EventContext): event context
+                to be updated
+
+            auth_events (dict[(str, str)->str]): Events to update in the event
+                context.
+
+            event_key ((str, str)): (type, state_key) for the current event.
+                this will not be included in the current_state in the context.
+        """
+        context.current_state_ids = dict(context.current_state_ids)
+        context.current_state_ids.update({
+            k: a.event_id for k, a in auth_events.items()
+            if k != event_key
+        })
+        context.prev_state_ids = dict(context.prev_state_ids)
+        context.prev_state_ids.update({
+            k: a.event_id for k, a in auth_events.items()
+        })
+        context.state_group = self.store.get_next_state_group()
+
     @defer.inlineCallbacks
     def construct_auth_difference(self, local_auth, remote_auth):
         """ Given a local and remote auth chain, find the differences. This
-- 
cgit 1.5.1


From 780dbb378fba8c7fb2eb154c2ce9f640ab52128f Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 7 Nov 2017 16:43:00 +0000
Subject: Update deltas when doing auth resolution

Fixes a bug where the persisted state groups were different to those actually
being used after auth resolution.
---
 synapse/handlers/federation.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 6cceb8998e..b9e1b24dab 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1934,11 +1934,15 @@ class FederationHandler(BaseHandler):
             event_key ((str, str)): (type, state_key) for the current event.
                 this will not be included in the current_state in the context.
         """
-        context.current_state_ids = dict(context.current_state_ids)
-        context.current_state_ids.update({
+        state_updates = {
             k: a.event_id for k, a in auth_events.items()
             if k != event_key
-        })
+        }
+        context.current_state_ids = dict(context.current_state_ids)
+        context.current_state_ids.update(state_updates)
+        if context.delta_ids is not None:
+            context.delta_ids = dict(context.delta_ids)
+            context.delta_ids.update(state_updates)
         context.prev_state_ids = dict(context.prev_state_ids)
         context.prev_state_ids.update({
             k: a.event_id for k, a in auth_events.items()
-- 
cgit 1.5.1


From d46386d57e4756e3bd5ca6ed17337d54e73dbbbf Mon Sep 17 00:00:00 2001
From: Ilya Zhuravlev <whatever@xyz.is>
Date: Tue, 7 Nov 2017 22:23:22 +0300
Subject: Remove useless assignment in notify_interested_services

---
 synapse/handlers/appservice.py | 1 -
 1 file changed, 1 deletion(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/appservice.py b/synapse/handlers/appservice.py
index 5ce752a196..543bf28aec 100644
--- a/synapse/handlers/appservice.py
+++ b/synapse/handlers/appservice.py
@@ -70,7 +70,6 @@ class ApplicationServicesHandler(object):
         with Measure(self.clock, "notify_interested_services"):
             self.is_processing = True
             try:
-                upper_bound = self.current_max
                 limit = 100
                 while True:
                     upper_bound, events = yield self.store.get_new_events_for_appservice(
-- 
cgit 1.5.1


From e148438e97c0d4bdd0dffbc3c1626dd4553f7d88 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Wed, 8 Nov 2017 09:21:41 +0000
Subject: s/items/iteritems/

---
 synapse/handlers/federation.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index b9e1b24dab..ac70730885 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1935,7 +1935,7 @@ class FederationHandler(BaseHandler):
                 this will not be included in the current_state in the context.
         """
         state_updates = {
-            k: a.event_id for k, a in auth_events.items()
+            k: a.event_id for k, a in auth_events.iteritems()
             if k != event_key
         }
         context.current_state_ids = dict(context.current_state_ids)
@@ -1945,7 +1945,7 @@ class FederationHandler(BaseHandler):
             context.delta_ids.update(state_updates)
         context.prev_state_ids = dict(context.prev_state_ids)
         context.prev_state_ids.update({
-            k: a.event_id for k, a in auth_events.items()
+            k: a.event_id for k, a in auth_events.iteritems()
         })
         context.state_group = self.store.get_next_state_group()
 
-- 
cgit 1.5.1


From 94ff2cda73cbd1aa14b2dd07f9598733229abc00 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erikj@jki.re>
Date: Wed, 8 Nov 2017 15:43:34 +0000
Subject: Revert "Modify group room association API to allow modification of
 is_public"

---
 synapse/federation/transport/client.py |  9 ++++-----
 synapse/federation/transport/server.py |  4 ++--
 synapse/groups/groups_server.py        | 13 +++++--------
 synapse/handlers/groups_local.py       |  4 ++--
 synapse/rest/client/v2_alpha/groups.py |  4 ++--
 synapse/storage/group_server.py        | 20 +++++++-------------
 6 files changed, 22 insertions(+), 32 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index ed41dfc7ee..d25ae1b282 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -531,9 +531,9 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
-    def update_room_group_association(self, destination, group_id, requester_user_id,
-                                      room_id, content):
-        """Add or update an association between room and group
+    def add_room_to_group(self, destination, group_id, requester_user_id, room_id,
+                          content):
+        """Add a room to a group
         """
         path = PREFIX + "/groups/%s/room/%s" % (group_id, room_id,)
 
@@ -545,8 +545,7 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
-    def delete_room_group_association(self, destination, group_id, requester_user_id,
-                                      room_id):
+    def remove_room_from_group(self, destination, group_id, requester_user_id, room_id):
         """Remove a room from a group
         """
         path = PREFIX + "/groups/%s/room/%s" % (group_id, room_id,)
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index ded6d4edc9..8f3c14c303 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -684,7 +684,7 @@ class FederationGroupsAddRoomsServlet(BaseFederationServlet):
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
 
-        new_content = yield self.handler.update_room_group_association(
+        new_content = yield self.handler.add_room_to_group(
             group_id, requester_user_id, room_id, content
         )
 
@@ -696,7 +696,7 @@ class FederationGroupsAddRoomsServlet(BaseFederationServlet):
         if get_domain_from_id(requester_user_id) != origin:
             raise SynapseError(403, "requester_user_id doesn't match origin")
 
-        new_content = yield self.handler.delete_room_group_association(
+        new_content = yield self.handler.remove_room_from_group(
             group_id, requester_user_id, room_id,
         )
 
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 11199dd215..81f21a36f5 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -530,9 +530,8 @@ class GroupsServerHandler(object):
         })
 
     @defer.inlineCallbacks
-    def update_room_group_association(self, group_id, requester_user_id, room_id,
-                                      content):
-        """Add or update an association between room and group
+    def add_room_to_group(self, group_id, requester_user_id, room_id, content):
+        """Add room to group
         """
         RoomID.from_string(room_id)  # Ensure valid room id
 
@@ -542,21 +541,19 @@ class GroupsServerHandler(object):
 
         is_public = _parse_visibility_from_contents(content)
 
-        yield self.store.update_room_group_association(
-            group_id, room_id, is_public=is_public
-        )
+        yield self.store.add_room_to_group(group_id, room_id, is_public=is_public)
 
         defer.returnValue({})
 
     @defer.inlineCallbacks
-    def delete_room_group_association(self, group_id, requester_user_id, room_id):
+    def remove_room_from_group(self, group_id, requester_user_id, room_id):
         """Remove room from group
         """
         yield self.check_group_is_ours(
             group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
         )
 
-        yield self.store.delete_room_group_association(group_id, room_id)
+        yield self.store.remove_room_from_group(group_id, room_id)
 
         defer.returnValue({})
 
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index dabc2a3fbb..6699d0888f 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -70,8 +70,8 @@ class GroupsLocalHandler(object):
 
     get_invited_users_in_group = _create_rerouter("get_invited_users_in_group")
 
-    update_room_group_association = _create_rerouter("update_room_group_association")
-    delete_room_group_association = _create_rerouter("delete_room_group_association")
+    add_room_to_group = _create_rerouter("add_room_to_group")
+    remove_room_from_group = _create_rerouter("remove_room_from_group")
 
     update_group_summary_room = _create_rerouter("update_group_summary_room")
     delete_group_summary_room = _create_rerouter("delete_group_summary_room")
diff --git a/synapse/rest/client/v2_alpha/groups.py b/synapse/rest/client/v2_alpha/groups.py
index 792608cd48..c97885cfc7 100644
--- a/synapse/rest/client/v2_alpha/groups.py
+++ b/synapse/rest/client/v2_alpha/groups.py
@@ -451,7 +451,7 @@ class GroupAdminRoomsServlet(RestServlet):
         requester_user_id = requester.user.to_string()
 
         content = parse_json_object_from_request(request)
-        result = yield self.groups_handler.update_room_group_association(
+        result = yield self.groups_handler.add_room_to_group(
             group_id, requester_user_id, room_id, content,
         )
 
@@ -462,7 +462,7 @@ class GroupAdminRoomsServlet(RestServlet):
         requester = yield self.auth.get_user_by_req(request)
         requester_user_id = requester.user.to_string()
 
-        result = yield self.groups_handler.delete_room_group_association(
+        result = yield self.groups_handler.remove_room_from_group(
             group_id, requester_user_id, room_id,
         )
 
diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py
index 6b261dcc0f..ede6bdfaee 100644
--- a/synapse/storage/group_server.py
+++ b/synapse/storage/group_server.py
@@ -846,25 +846,19 @@ class GroupServerStore(SQLBaseStore):
             )
         return self.runInteraction("remove_user_from_group", _remove_user_from_group_txn)
 
-    def update_room_group_association(self, group_id, room_id, is_public):
-        return self._simple_upsert(
+    def add_room_to_group(self, group_id, room_id, is_public):
+        return self._simple_insert(
             table="group_rooms",
-            keyvalues={
-                "group_id": group_id,
-                "room_id": room_id,
-            },
             values={
-                "is_public": is_public,
-            },
-            insertion_values={
                 "group_id": group_id,
                 "room_id": room_id,
+                "is_public": is_public,
             },
-            desc="update_room_group_association",
+            desc="add_room_to_group",
         )
 
-    def delete_room_group_association(self, group_id, room_id):
-        def _delete_room_group_association_txn(txn):
+    def remove_room_from_group(self, group_id, room_id):
+        def _remove_room_from_group_txn(txn):
             self._simple_delete_txn(
                 txn,
                 table="group_rooms",
@@ -883,7 +877,7 @@ class GroupServerStore(SQLBaseStore):
                 },
             )
         return self.runInteraction(
-            "delete_room_group_association", _delete_room_group_association_txn,
+            "remove_room_from_group", _remove_room_from_group_txn,
         )
 
     def get_publicised_groups_for_user(self, user_id):
-- 
cgit 1.5.1


From e8814410ef682a1e277a1cfe6fda7268fd7a33d6 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 8 Nov 2017 16:13:27 +0000
Subject: Have an explicit API to update room config

---
 synapse/federation/transport/client.py | 14 ++++++++++++++
 synapse/federation/transport/server.py | 23 ++++++++++++++++++++++-
 synapse/groups/groups_server.py        | 23 +++++++++++++++++++++++
 synapse/handlers/groups_local.py       |  1 +
 synapse/rest/client/v2_alpha/groups.py | 27 +++++++++++++++++++++++++++
 synapse/storage/group_server.py        | 13 +++++++++++++
 6 files changed, 100 insertions(+), 1 deletion(-)

(limited to 'synapse/handlers')

diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py
index d25ae1b282..1f3ce238f6 100644
--- a/synapse/federation/transport/client.py
+++ b/synapse/federation/transport/client.py
@@ -545,6 +545,20 @@ class TransportLayerClient(object):
             ignore_backoff=True,
         )
 
+    def update_room_in_group(self, destination, group_id, requester_user_id, room_id,
+                             config_key, content):
+        """Update room in group
+        """
+        path = PREFIX + "/groups/%s/room/%s/config/%s" % (group_id, room_id, config_key,)
+
+        return self.client.post_json(
+            destination=destination,
+            path=path,
+            args={"requester_user_id": requester_user_id},
+            data=content,
+            ignore_backoff=True,
+        )
+
     def remove_room_from_group(self, destination, group_id, requester_user_id, room_id):
         """Remove a room from a group
         """
diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py
index 8f3c14c303..6ef6cce592 100644
--- a/synapse/federation/transport/server.py
+++ b/synapse/federation/transport/server.py
@@ -676,7 +676,7 @@ class FederationGroupsRoomsServlet(BaseFederationServlet):
 class FederationGroupsAddRoomsServlet(BaseFederationServlet):
     """Add/remove room from group
     """
-    PATH = "/groups/(?P<group_id>[^/]*)/room/(?<room_id>)$"
+    PATH = "/groups/(?P<group_id>[^/]*)/room/(?P<room_id>[^/]*)$"
 
     @defer.inlineCallbacks
     def on_POST(self, origin, content, query, group_id, room_id):
@@ -703,6 +703,25 @@ class FederationGroupsAddRoomsServlet(BaseFederationServlet):
         defer.returnValue((200, new_content))
 
 
+class FederationGroupsAddRoomsConfigServlet(BaseFederationServlet):
+    """Update room config in group
+    """
+    PATH = "/groups/(?P<group_id>[^/]*)/room/(?P<room_id>[^/]*)"
+    "/config/(?P<config_key>[^/]*)$"
+
+    @defer.inlineCallbacks
+    def on_POST(self, origin, content, query, group_id, room_id, config_key):
+        requester_user_id = parse_string_from_args(query, "requester_user_id")
+        if get_domain_from_id(requester_user_id) != origin:
+            raise SynapseError(403, "requester_user_id doesn't match origin")
+
+        result = yield self.groups_handler.update_room_in_group(
+            group_id, requester_user_id, room_id, config_key, content,
+        )
+
+        defer.returnValue((200, result))
+
+
 class FederationGroupsUsersServlet(BaseFederationServlet):
     """Get the users in a group on behalf of a user
     """
@@ -1142,6 +1161,8 @@ GROUP_SERVER_SERVLET_CLASSES = (
     FederationGroupsRolesServlet,
     FederationGroupsRoleServlet,
     FederationGroupsSummaryUsersServlet,
+    FederationGroupsAddRoomsServlet,
+    FederationGroupsAddRoomsConfigServlet,
 )
 
 
diff --git a/synapse/groups/groups_server.py b/synapse/groups/groups_server.py
index 81f21a36f5..a8039f4788 100644
--- a/synapse/groups/groups_server.py
+++ b/synapse/groups/groups_server.py
@@ -545,6 +545,29 @@ class GroupsServerHandler(object):
 
         defer.returnValue({})
 
+    @defer.inlineCallbacks
+    def update_room_in_group(self, group_id, requester_user_id, room_id, config_key,
+                             content):
+        """Update room in group
+        """
+        RoomID.from_string(room_id)  # Ensure valid room id
+
+        yield self.check_group_is_ours(
+            group_id, requester_user_id, and_exists=True, and_is_admin=requester_user_id
+        )
+
+        if config_key == "visibility":
+            is_public = _parse_visibility_from_contents(content)
+
+            yield self.store.update_room_in_group_visibility(
+                group_id, room_id,
+                is_public=is_public,
+            )
+        else:
+            raise SynapseError(400, "Uknown config option")
+
+        defer.returnValue({})
+
     @defer.inlineCallbacks
     def remove_room_from_group(self, group_id, requester_user_id, room_id):
         """Remove room from group
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index 6699d0888f..da00aeb0f4 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -71,6 +71,7 @@ class GroupsLocalHandler(object):
     get_invited_users_in_group = _create_rerouter("get_invited_users_in_group")
 
     add_room_to_group = _create_rerouter("add_room_to_group")
+    update_room_in_group = _create_rerouter("update_room_in_group")
     remove_room_from_group = _create_rerouter("remove_room_from_group")
 
     update_group_summary_room = _create_rerouter("update_group_summary_room")
diff --git a/synapse/rest/client/v2_alpha/groups.py b/synapse/rest/client/v2_alpha/groups.py
index c97885cfc7..67f163e812 100644
--- a/synapse/rest/client/v2_alpha/groups.py
+++ b/synapse/rest/client/v2_alpha/groups.py
@@ -469,6 +469,33 @@ class GroupAdminRoomsServlet(RestServlet):
         defer.returnValue((200, result))
 
 
+class GroupAdminRoomsConfigServlet(RestServlet):
+    """Update the config of a room in a group
+    """
+    PATTERNS = client_v2_patterns(
+        "/groups/(?P<group_id>[^/]*)/admin/rooms/(?P<room_id>[^/]*)"
+        "/config/(?P<config_key>[^/]*)$"
+    )
+
+    def __init__(self, hs):
+        super(GroupAdminRoomsConfigServlet, self).__init__()
+        self.auth = hs.get_auth()
+        self.clock = hs.get_clock()
+        self.groups_handler = hs.get_groups_local_handler()
+
+    @defer.inlineCallbacks
+    def on_PUT(self, request, group_id, room_id, config_key):
+        requester = yield self.auth.get_user_by_req(request)
+        requester_user_id = requester.user.to_string()
+
+        content = parse_json_object_from_request(request)
+        result = yield self.groups_handler.update_room_in_group(
+            group_id, requester_user_id, room_id, config_key, content,
+        )
+
+        defer.returnValue((200, result))
+
+
 class GroupAdminUsersInviteServlet(RestServlet):
     """Invite a user to the group
     """
diff --git a/synapse/storage/group_server.py b/synapse/storage/group_server.py
index ede6bdfaee..6cb4ac28be 100644
--- a/synapse/storage/group_server.py
+++ b/synapse/storage/group_server.py
@@ -857,6 +857,19 @@ class GroupServerStore(SQLBaseStore):
             desc="add_room_to_group",
         )
 
+    def update_room_in_group_visibility(self, group_id, room_id, is_public):
+        return self._simple_update(
+            table="group_rooms",
+            keyvalues={
+                "group_id": group_id,
+                "room_id": room_id,
+            },
+            values={
+                "is_public": is_public,
+            },
+            desc="update_room_in_group_visibility",
+        )
+
     def remove_room_from_group(self, group_id, room_id):
         def _remove_room_from_group_txn(txn):
             self._simple_delete_txn(
-- 
cgit 1.5.1


From b70b64690330c25cbd04c1b2cacf8276b566efc8 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 9 Nov 2017 19:36:13 +0000
Subject: Allow upper-case characters in mxids

Because we're never going to be able to fix this :'(
---
 synapse/handlers/register.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index f6e7e58563..bd5ba6e1d8 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -15,6 +15,7 @@
 
 """Contains functions for registering clients."""
 import logging
+import urllib
 
 from twisted.internet import defer
 
@@ -22,7 +23,6 @@ from synapse.api.errors import (
     AuthError, Codes, SynapseError, RegistrationError, InvalidCaptchaError
 )
 from synapse.http.client import CaptchaServerHttpClient
-from synapse import types
 from synapse.types import UserID
 from synapse.util.async import run_on_reactor
 from ._base import BaseHandler
@@ -47,7 +47,7 @@ class RegistrationHandler(BaseHandler):
     @defer.inlineCallbacks
     def check_username(self, localpart, guest_access_token=None,
                        assigned_user_id=None):
-        if types.contains_invalid_mxid_characters(localpart):
+        if urllib.quote(localpart.encode('utf-8')) != localpart:
             raise SynapseError(
                 400,
                 "User ID can only contain characters a-z, 0-9, or '=_-./'",
@@ -253,7 +253,7 @@ class RegistrationHandler(BaseHandler):
         """
         Registers email_id as SAML2 Based Auth.
         """
-        if types.contains_invalid_mxid_characters(localpart):
+        if urllib.quote(localpart) != localpart:
             raise SynapseError(
                 400,
                 "User ID can only contain characters a-z, 0-9, or '=_-./'",
-- 
cgit 1.5.1


From 9b803ccc9886d4f77a00d903eefa092b74f13dc3 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Thu, 9 Nov 2017 21:57:24 +0000
Subject: Revert "Allow upper-case characters in mxids"

This reverts commit b70b64690330c25cbd04c1b2cacf8276b566efc8.
---
 synapse/handlers/register.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index bd5ba6e1d8..f6e7e58563 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -15,7 +15,6 @@
 
 """Contains functions for registering clients."""
 import logging
-import urllib
 
 from twisted.internet import defer
 
@@ -23,6 +22,7 @@ from synapse.api.errors import (
     AuthError, Codes, SynapseError, RegistrationError, InvalidCaptchaError
 )
 from synapse.http.client import CaptchaServerHttpClient
+from synapse import types
 from synapse.types import UserID
 from synapse.util.async import run_on_reactor
 from ._base import BaseHandler
@@ -47,7 +47,7 @@ class RegistrationHandler(BaseHandler):
     @defer.inlineCallbacks
     def check_username(self, localpart, guest_access_token=None,
                        assigned_user_id=None):
-        if urllib.quote(localpart.encode('utf-8')) != localpart:
+        if types.contains_invalid_mxid_characters(localpart):
             raise SynapseError(
                 400,
                 "User ID can only contain characters a-z, 0-9, or '=_-./'",
@@ -253,7 +253,7 @@ class RegistrationHandler(BaseHandler):
         """
         Registers email_id as SAML2 Based Auth.
         """
-        if urllib.quote(localpart) != localpart:
+        if types.contains_invalid_mxid_characters(localpart):
             raise SynapseError(
                 400,
                 "User ID can only contain characters a-z, 0-9, or '=_-./'",
-- 
cgit 1.5.1