From 8ee69f299cb3360de5c88f0c6b07525d35247fbd Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Mon, 1 Jul 2019 17:55:11 +0100
Subject: Add basic function to get all data for a user out of synapse

---
 synapse/handlers/admin.py | 247 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 247 insertions(+)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index 941ebfa107..e424fc46bd 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -14,9 +14,17 @@
 # limitations under the License.
 
 import logging
+import os
+import tempfile
+
+from canonicaljson import json
 
 from twisted.internet import defer
 
+from synapse.api.constants import Membership
+from synapse.types import RoomStreamToken
+from synapse.visibility import filter_events_for_client
+
 from ._base import BaseHandler
 
 logger = logging.getLogger(__name__)
@@ -89,3 +97,242 @@ class AdminHandler(BaseHandler):
         ret = yield self.store.search_users(term)
 
         defer.returnValue(ret)
+
+    @defer.inlineCallbacks
+    def exfiltrate_user_data(self, user_id, writer):
+        """Write all data we have of the user to the specified directory.
+
+        Args:
+            user_id (str)
+            writer (ExfiltrationWriter)
+
+        Returns:
+            defer.Deferred
+        """
+        # Get all rooms the user is in or has been in
+        rooms = yield self.store.get_rooms_for_user_where_membership_is(
+            user_id,
+            membership_list=(
+                Membership.JOIN,
+                Membership.LEAVE,
+                Membership.BAN,
+                Membership.INVITE,
+            ),
+        )
+
+        # We only try and fetch events for rooms the user has been in. If
+        # they've been e.g. invited to a room without joining then we handle
+        # those seperately.
+        rooms_user_has_been_in = yield self.store.get_rooms_user_has_been_in(user_id)
+
+        for index, room in enumerate(rooms):
+            room_id = room.room_id
+
+            logger.info(
+                "[%s] Handling room %s, %d/%d", user_id, room_id, index + 1, len(rooms)
+            )
+
+            forgotten = yield self.store.did_forget(user_id, room_id)
+            if forgotten:
+                logger.info("[%s] User forgot room %d, ignoring", room_id)
+                continue
+
+            if room_id not in rooms_user_has_been_in:
+                # If we haven't been in the rooms then the filtering code below
+                # won't return anything, so we need to handle these cases
+                # explicitly.
+
+                if room.membership == Membership.INVITE:
+                    event_id = room.event_id
+                    invite = yield self.store.get_event(event_id, allow_none=True)
+                    if invite:
+                        invited_state = invite.unsigned["invite_room_state"]
+                        writer.write_invite(room_id, invite, invited_state)
+
+                continue
+
+            # We only want to bother fetching events up to the last time they
+            # were joined. We estimate that point by looking at the
+            # stream_ordering of the last membership if it wasn't a join.
+            if room.membership == Membership.JOIN:
+                stream_ordering = yield self.store.get_room_max_stream_ordering()
+            else:
+                stream_ordering = room.stream_ordering
+
+            from_key = str(RoomStreamToken(0, 0))
+            to_key = str(RoomStreamToken(None, stream_ordering))
+
+            written_events = set()  # Events that we've processed in this room
+
+            # We need to track gaps in the events stream so that we can then
+            # write out the state at those events. We do this by keeping track
+            # of events whose prev events we haven't seen.
+
+            # Map from event ID to prev events that haven't been processed,
+            # dict[str, set[str]].
+            event_to_unseen_prevs = {}
+
+            # The reverse mapping to above, i.e. map from unseen event to parent
+            # events. dict[str, set[str]]
+            unseen_event_to_parents = {}
+
+            # We fetch events in the room the user could see by fetching *all*
+            # events that we have and then filtering, this isn't the most
+            # efficient method perhaps but it does guarentee we get everything.
+            while True:
+                events, _ = yield self.store.paginate_room_events(
+                    room_id, from_key, to_key, limit=100, direction="f"
+                )
+                if not events:
+                    break
+
+                from_key = events[-1].internal_metadata.after
+
+                events = yield filter_events_for_client(self.store, user_id, events)
+
+                writer.write_events(room_id, events)
+
+                # Update the extremity tracking dicts
+                for event in events:
+                    # Check if we have any prev events that haven't been
+                    # processed yet, and add those to the appropriate dicts.
+                    unseen_events = set(event.prev_event_ids()) - written_events
+                    if unseen_events:
+                        event_to_unseen_prevs[event.event_id] = unseen_events
+                        for unseen in unseen_events:
+                            unseen_event_to_parents.setdefault(unseen, set()).add(
+                                event.event_id
+                            )
+
+                    # Now check if this event is an unseen prev event, if so
+                    # then we remove this event from the appropriate dicts.
+                    for event_id in unseen_event_to_parents.pop(event.event_id, []):
+                        event_to_unseen_prevs.get(event_id, set()).discard(
+                            event.event_id
+                        )
+
+                    written_events.add(event.event_id)
+
+                logger.info(
+                    "Written %d events in room %s", len(written_events), room_id
+                )
+
+            # Extremities are the events who have at least one unseen prev event.
+            extremities = (
+                event_id
+                for event_id, unseen_prevs in event_to_unseen_prevs.items()
+                if unseen_prevs
+            )
+            for event_id in extremities:
+                if not event_to_unseen_prevs[event_id]:
+                    continue
+                state = yield self.store.get_state_for_event(event_id)
+                writer.write_state(room_id, event_id, state)
+
+        defer.returnValue(writer.finished())
+
+
+class ExfiltrationWriter(object):
+    """Interfaced used to specify how to write exfilrated data.
+    """
+
+    def write_events(self, room_id, events):
+        """Write a batch of events for a room.
+
+        Args:
+            room_id (str)
+            events (list[FrozenEvent])
+        """
+        pass
+
+    def write_state(self, room_id, event_id, state):
+        """Write the state at the given event in the room.
+
+        This only gets called for backward extremities rather than for each
+        event.
+
+        Args:
+            room_id (str)
+            event_id (str)
+            state (list[FrozenEvent])
+        """
+        pass
+
+    def write_invite(self, room_id, event, state):
+        """Write an invite for the room, with associated invite state.
+
+        Args:
+            room_id (str)
+            invite (FrozenEvent)
+            state (list[dict]): A subset of the state at the invite, with a
+                subset of the event keys (type, state_key, content and sender)
+        """
+
+    def finished(self):
+        """Called when exfiltration is complete, and the return valus is passed
+        to the requester.
+        """
+        pass
+
+
+class FileExfiltrationWriter(ExfiltrationWriter):
+    """An ExfiltrationWriter that writes the users data to a directory.
+
+    Returns the directory location on completion.
+
+    Args:
+        user_id (str): The user whose data is being exfiltrated.
+        directory (str|None): The directory to write the data to, if None then
+            will write to a temporary directory.
+    """
+
+    def __init__(self, user_id, directory=None):
+        self.user_id = user_id
+
+        if directory:
+            self.base_directory = directory
+        else:
+            self.base_directory = tempfile.mkdtemp(
+                prefix="synapse-exfiltrate__%s__" % (user_id,)
+            )
+
+        os.makedirs(self.base_directory, exist_ok=True)
+        if list(os.listdir(self.base_directory)):
+            raise Exception("Directory must be empty")
+
+    def write_events(self, room_id, events):
+        room_directory = os.path.join(self.base_directory, "rooms", room_id)
+        os.makedirs(room_directory, exist_ok=True)
+        events_file = os.path.join(room_directory, "events")
+
+        with open(events_file, "a") as f:
+            for event in events:
+                print(json.dumps(event.get_pdu_json()), file=f)
+
+    def write_state(self, room_id, event_id, state):
+        room_directory = os.path.join(self.base_directory, "rooms", room_id)
+        state_directory = os.path.join(room_directory, "state")
+        os.makedirs(state_directory, exist_ok=True)
+
+        event_file = os.path.join(state_directory, event_id)
+
+        with open(event_file, "a") as f:
+            for event in state.values():
+                print(json.dumps(event.get_pdu_json()), file=f)
+
+    def write_invite(self, room_id, event, state):
+        self.write_events(room_id, [event])
+
+        # We write the invite state somewhere else as they aren't full events
+        # and are only a subset of the state at the event.
+        room_directory = os.path.join(self.base_directory, "rooms", room_id)
+        os.makedirs(room_directory, exist_ok=True)
+
+        invite_state = os.path.join(room_directory, "invite_state")
+
+        with open(invite_state, "a") as f:
+            for event in state.values():
+                print(json.dumps(event), file=f)
+
+    def finished(self):
+        return self.base_directory
-- 
cgit 1.5.1


From d0b849c86d93ace21bdf7f73e1411f33a9e1b2fe Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 3 Jul 2019 15:03:38 +0100
Subject: Apply comment fixups from code review

Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 synapse/handlers/admin.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index e424fc46bd..6c905e97a7 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -100,7 +100,7 @@ class AdminHandler(BaseHandler):
 
     @defer.inlineCallbacks
     def exfiltrate_user_data(self, user_id, writer):
-        """Write all data we have of the user to the specified directory.
+        """Write all data we have on the user to the given writer.
 
         Args:
             user_id (str)
@@ -178,7 +178,7 @@ class AdminHandler(BaseHandler):
 
             # We fetch events in the room the user could see by fetching *all*
             # events that we have and then filtering, this isn't the most
-            # efficient method perhaps but it does guarentee we get everything.
+            # efficient method perhaps but it does guarantee we get everything.
             while True:
                 events, _ = yield self.store.paginate_room_events(
                     room_id, from_key, to_key, limit=100, direction="f"
@@ -233,7 +233,7 @@ class AdminHandler(BaseHandler):
 
 
 class ExfiltrationWriter(object):
-    """Interfaced used to specify how to write exfilrated data.
+    """Interface used to specify how to write exfiltrated data.
     """
 
     def write_events(self, room_id, events):
@@ -263,7 +263,7 @@ class ExfiltrationWriter(object):
 
         Args:
             room_id (str)
-            invite (FrozenEvent)
+            event (FrozenEvent)
             state (list[dict]): A subset of the state at the invite, with a
                 subset of the event keys (type, state_key, content and sender)
         """
@@ -276,13 +276,13 @@ class ExfiltrationWriter(object):
 
 
 class FileExfiltrationWriter(ExfiltrationWriter):
-    """An ExfiltrationWriter that writes the users data to a directory.
+    """An ExfiltrationWriter that writes the user's data to a directory.
 
     Returns the directory location on completion.
 
     Args:
         user_id (str): The user whose data is being exfiltrated.
-        directory (str|None): The directory to write the data to, if None then
+        directory (str|None): The directory to write the data to. If None then
             will write to a temporary directory.
     """
 
-- 
cgit 1.5.1


From c061d4f237273f3400dc8e62aa7421f02caec3dd Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Thu, 4 Jul 2019 11:07:09 +0100
Subject: Fixup from review comments.

---
 synapse/handlers/admin.py    | 39 ++++++++++++++++++++++-----------------
 tests/handlers/test_admin.py | 10 +++++-----
 2 files changed, 27 insertions(+), 22 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index 6c905e97a7..69d2c8c36f 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -99,7 +99,7 @@ class AdminHandler(BaseHandler):
         defer.returnValue(ret)
 
     @defer.inlineCallbacks
-    def exfiltrate_user_data(self, user_id, writer):
+    def export_user_data(self, user_id, writer):
         """Write all data we have on the user to the given writer.
 
         Args:
@@ -107,7 +107,8 @@ class AdminHandler(BaseHandler):
             writer (ExfiltrationWriter)
 
         Returns:
-            defer.Deferred
+            defer.Deferred: Resolves when all data for a user has been written.
+            The returned value is that returned by `writer.finished()`.
         """
         # Get all rooms the user is in or has been in
         rooms = yield self.store.get_rooms_for_user_where_membership_is(
@@ -134,7 +135,7 @@ class AdminHandler(BaseHandler):
 
             forgotten = yield self.store.did_forget(user_id, room_id)
             if forgotten:
-                logger.info("[%s] User forgot room %d, ignoring", room_id)
+                logger.info("[%s] User forgot room %d, ignoring", user_id, room_id)
                 continue
 
             if room_id not in rooms_user_has_been_in:
@@ -172,9 +173,10 @@ class AdminHandler(BaseHandler):
             # dict[str, set[str]].
             event_to_unseen_prevs = {}
 
-            # The reverse mapping to above, i.e. map from unseen event to parent
-            # events. dict[str, set[str]]
-            unseen_event_to_parents = {}
+            # The reverse mapping to above, i.e. map from unseen event to events
+            # that have the unseen event in their prev_events, i.e. the unseen
+            # events "children". dict[str, set[str]]
+            unseen_to_child_events = {}
 
             # We fetch events in the room the user could see by fetching *all*
             # events that we have and then filtering, this isn't the most
@@ -200,14 +202,14 @@ class AdminHandler(BaseHandler):
                     if unseen_events:
                         event_to_unseen_prevs[event.event_id] = unseen_events
                         for unseen in unseen_events:
-                            unseen_event_to_parents.setdefault(unseen, set()).add(
+                            unseen_to_child_events.setdefault(unseen, set()).add(
                                 event.event_id
                             )
 
                     # Now check if this event is an unseen prev event, if so
                     # then we remove this event from the appropriate dicts.
-                    for event_id in unseen_event_to_parents.pop(event.event_id, []):
-                        event_to_unseen_prevs.get(event_id, set()).discard(
+                    for child_id in unseen_to_child_events.pop(event.event_id, []):
+                        event_to_unseen_prevs.get(child_id, set()).discard(
                             event.event_id
                         )
 
@@ -233,7 +235,7 @@ class AdminHandler(BaseHandler):
 
 
 class ExfiltrationWriter(object):
-    """Interface used to specify how to write exfiltrated data.
+    """Interface used to specify how to write exported data.
     """
 
     def write_events(self, room_id, events):
@@ -254,7 +256,7 @@ class ExfiltrationWriter(object):
         Args:
             room_id (str)
             event_id (str)
-            state (list[FrozenEvent])
+            state (dict[tuple[str, str], FrozenEvent])
         """
         pass
 
@@ -264,13 +266,16 @@ class ExfiltrationWriter(object):
         Args:
             room_id (str)
             event (FrozenEvent)
-            state (list[dict]): A subset of the state at the invite, with a
-                subset of the event keys (type, state_key, content and sender)
+            state (dict[tuple[str, str], dict]): A subset of the state at the
+                invite, with a subset of the event keys (type, state_key
+                content and sender)
         """
 
     def finished(self):
-        """Called when exfiltration is complete, and the return valus is passed
-        to the requester.
+        """Called when all data has succesfully been exported and written.
+
+        This functions return value is passed to the caller of
+        `export_user_data`.
         """
         pass
 
@@ -281,7 +286,7 @@ class FileExfiltrationWriter(ExfiltrationWriter):
     Returns the directory location on completion.
 
     Args:
-        user_id (str): The user whose data is being exfiltrated.
+        user_id (str): The user whose data is being exported.
         directory (str|None): The directory to write the data to. If None then
             will write to a temporary directory.
     """
@@ -293,7 +298,7 @@ class FileExfiltrationWriter(ExfiltrationWriter):
             self.base_directory = directory
         else:
             self.base_directory = tempfile.mkdtemp(
-                prefix="synapse-exfiltrate__%s__" % (user_id,)
+                prefix="synapse-exported__%s__" % (user_id,)
             )
 
         os.makedirs(self.base_directory, exist_ok=True)
diff --git a/tests/handlers/test_admin.py b/tests/handlers/test_admin.py
index 5e7d2d3361..fc37c4328c 100644
--- a/tests/handlers/test_admin.py
+++ b/tests/handlers/test_admin.py
@@ -55,7 +55,7 @@ class ExfiltrateData(unittest.HomeserverTestCase):
 
         writer = Mock()
 
-        self.get_success(self.admin_handler.exfiltrate_user_data(self.user2, writer))
+        self.get_success(self.admin_handler.export_user_data(self.user2, writer))
 
         writer.write_events.assert_called()
 
@@ -94,7 +94,7 @@ class ExfiltrateData(unittest.HomeserverTestCase):
 
         writer = Mock()
 
-        self.get_success(self.admin_handler.exfiltrate_user_data(self.user2, writer))
+        self.get_success(self.admin_handler.export_user_data(self.user2, writer))
 
         writer.write_events.assert_called()
 
@@ -127,7 +127,7 @@ class ExfiltrateData(unittest.HomeserverTestCase):
 
         writer = Mock()
 
-        self.get_success(self.admin_handler.exfiltrate_user_data(self.user2, writer))
+        self.get_success(self.admin_handler.export_user_data(self.user2, writer))
 
         writer.write_events.assert_called()
 
@@ -169,7 +169,7 @@ class ExfiltrateData(unittest.HomeserverTestCase):
 
         writer = Mock()
 
-        self.get_success(self.admin_handler.exfiltrate_user_data(self.user2, writer))
+        self.get_success(self.admin_handler.export_user_data(self.user2, writer))
 
         writer.write_events.assert_called_once()
 
@@ -198,7 +198,7 @@ class ExfiltrateData(unittest.HomeserverTestCase):
 
         writer = Mock()
 
-        self.get_success(self.admin_handler.exfiltrate_user_data(self.user2, writer))
+        self.get_success(self.admin_handler.export_user_data(self.user2, writer))
 
         writer.write_events.assert_not_called()
         writer.write_state.assert_not_called()
-- 
cgit 1.5.1


From 9ccea16d45416397b37fa407709ff455bca415e3 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 5 Jul 2019 14:07:56 +0100
Subject: Assume key existence. Update docstrings

---
 synapse/handlers/admin.py |  4 +---
 synapse/storage/stream.py | 16 ++++++++++------
 2 files changed, 11 insertions(+), 9 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index 69d2c8c36f..f06914a378 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -209,9 +209,7 @@ class AdminHandler(BaseHandler):
                     # Now check if this event is an unseen prev event, if so
                     # then we remove this event from the appropriate dicts.
                     for child_id in unseen_to_child_events.pop(event.event_id, []):
-                        event_to_unseen_prevs.get(child_id, set()).discard(
-                            event.event_id
-                        )
+                        event_to_unseen_prevs[child_id].discard(event.event_id)
 
                     written_events.add(event.event_id)
 
diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py
index d9482a3843..7b5b3b8c8d 100644
--- a/synapse/storage/stream.py
+++ b/synapse/storage/stream.py
@@ -833,7 +833,9 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
         Returns:
             Deferred[tuple[list[_EventDictReturn], str]]: Returns the results
             as a list of _EventDictReturn and a token that points to the end
-            of the result set.
+            of the result set. If no events are returned then the end of the
+            stream has been reached (i.e. there are no events between
+            `from_token` and `to_token`).
         """
 
         assert int(limit) >= 0
@@ -905,15 +907,17 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
                 only those before
             direction(char): Either 'b' or 'f' to indicate whether we are
                 paginating forwards or backwards from `from_key`.
-            limit (int): The maximum number of events to return. Zero or less
-                means no limit.
+            limit (int): The maximum number of events to return.
             event_filter (Filter|None): If provided filters the events to
                 those that match the filter.
 
         Returns:
-            tuple[list[dict], str]: Returns the results as a list of dicts and
-            a token that points to the end of the result set. The dicts have
-            the keys "event_id", "topological_ordering" and "stream_orderign".
+            tuple[list[FrozenEvents], str]: Returns the results as a list of
+            dicts and a token that points to the end of the result set. The
+            dicts have the keys "event_id", "topological_ordering" and
+            "stream_ordering". If no events are returned then the end of the
+            stream has been reached (i.e. there are no events between
+            `from_key` and `to_key`).
         """
 
         from_key = RoomStreamToken.parse(from_key)
-- 
cgit 1.5.1


From eadb13d2e9caaa391f4efe2609a7d54d1723d311 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 5 Jul 2019 14:15:00 +0100
Subject: Remove FileExfiltrationWriter

---
 synapse/handlers/admin.py | 63 -----------------------------------------------
 1 file changed, 63 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index f06914a378..5ff02c12bf 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -276,66 +276,3 @@ class ExfiltrationWriter(object):
         `export_user_data`.
         """
         pass
-
-
-class FileExfiltrationWriter(ExfiltrationWriter):
-    """An ExfiltrationWriter that writes the user's data to a directory.
-
-    Returns the directory location on completion.
-
-    Args:
-        user_id (str): The user whose data is being exported.
-        directory (str|None): The directory to write the data to. If None then
-            will write to a temporary directory.
-    """
-
-    def __init__(self, user_id, directory=None):
-        self.user_id = user_id
-
-        if directory:
-            self.base_directory = directory
-        else:
-            self.base_directory = tempfile.mkdtemp(
-                prefix="synapse-exported__%s__" % (user_id,)
-            )
-
-        os.makedirs(self.base_directory, exist_ok=True)
-        if list(os.listdir(self.base_directory)):
-            raise Exception("Directory must be empty")
-
-    def write_events(self, room_id, events):
-        room_directory = os.path.join(self.base_directory, "rooms", room_id)
-        os.makedirs(room_directory, exist_ok=True)
-        events_file = os.path.join(room_directory, "events")
-
-        with open(events_file, "a") as f:
-            for event in events:
-                print(json.dumps(event.get_pdu_json()), file=f)
-
-    def write_state(self, room_id, event_id, state):
-        room_directory = os.path.join(self.base_directory, "rooms", room_id)
-        state_directory = os.path.join(room_directory, "state")
-        os.makedirs(state_directory, exist_ok=True)
-
-        event_file = os.path.join(state_directory, event_id)
-
-        with open(event_file, "a") as f:
-            for event in state.values():
-                print(json.dumps(event.get_pdu_json()), file=f)
-
-    def write_invite(self, room_id, event, state):
-        self.write_events(room_id, [event])
-
-        # We write the invite state somewhere else as they aren't full events
-        # and are only a subset of the state at the event.
-        room_directory = os.path.join(self.base_directory, "rooms", room_id)
-        os.makedirs(room_directory, exist_ok=True)
-
-        invite_state = os.path.join(room_directory, "invite_state")
-
-        with open(invite_state, "a") as f:
-            for event in state.values():
-                print(json.dumps(event), file=f)
-
-    def finished(self):
-        return self.base_directory
-- 
cgit 1.5.1


From b4f5416dd9bd7635a4b859e3d13eaee992096ef7 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Fri, 5 Jul 2019 14:41:29 +0100
Subject: pep8

---
 synapse/handlers/admin.py | 4 ----
 1 file changed, 4 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py
index 5ff02c12bf..e8a651e231 100644
--- a/synapse/handlers/admin.py
+++ b/synapse/handlers/admin.py
@@ -14,10 +14,6 @@
 # limitations under the License.
 
 import logging
-import os
-import tempfile
-
-from canonicaljson import json
 
 from twisted.internet import defer
 
-- 
cgit 1.5.1


From 1a807dfe6855b15d8574eb92541dd84b946f16bd Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Mon, 8 Jul 2019 14:19:39 +0100
Subject: Use application/json when querying the IS's /store-invite endpoint

---
 synapse/handlers/room_member.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 4d6e883802..b050967b8f 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -29,7 +29,7 @@ from twisted.internet import defer
 import synapse.server
 import synapse.types
 from synapse.api.constants import EventTypes, Membership
-from synapse.api.errors import AuthError, Codes, SynapseError
+from synapse.api.errors import AuthError, Codes, HttpResponseException, SynapseError
 from synapse.types import RoomID, UserID
 from synapse.util.async_helpers import Linearizer
 from synapse.util.distributor import user_joined_room, user_left_room
@@ -904,9 +904,22 @@ class RoomMemberHandler(object):
                 }
             )
 
-        data = yield self.simple_http_client.post_urlencoded_get_json(
-            is_url, invite_config
-        )
+        try:
+            data = yield self.simple_http_client.post_json_get_json(
+                is_url, invite_config
+            )
+        except HttpResponseException as e:
+            # Some identity servers may only support application/x-www-form-urlencoded
+            # types. This is especially true with old instances of Sydent, see
+            # https://github.com/matrix-org/sydent/pull/170
+            logger.info(
+                "Failed to POST %s with JSON, falling back to urlencoded form: %s",
+                is_url, e,
+            )
+            data = yield self.simple_http_client.post_urlencoded_get_json(
+                is_url, invite_config
+            )
+
         # TODO: Check for success
         token = data["token"]
         public_keys = data.get("public_keys", [])
-- 
cgit 1.5.1


From f05c7d62bc2bb504f0048adf6e68decdbe31f80d Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Mon, 8 Jul 2019 14:29:27 +0100
Subject: Lint

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

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index b050967b8f..c3420b4b22 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -914,7 +914,8 @@ class RoomMemberHandler(object):
             # https://github.com/matrix-org/sydent/pull/170
             logger.info(
                 "Failed to POST %s with JSON, falling back to urlencoded form: %s",
-                is_url, e,
+                is_url,
+                e,
             )
             data = yield self.simple_http_client.post_urlencoded_get_json(
                 is_url, invite_config
-- 
cgit 1.5.1


From 7556851665ce623ce49f6fd8eaf22c1b4f234b9d Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Mon, 8 Jul 2019 17:31:00 +0100
Subject: Allow newly-registered users to lookup their own profiles

When a user creates an account and the 'require_auth_for_profile_requests' config flag is set, and a client that performed the registration wants to lookup the newly-created profile, the request will be denied because the user doesn't share a room with themselves yet.
---
 synapse/handlers/profile.py | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py
index d8462b75ec..a2388a7091 100644
--- a/synapse/handlers/profile.py
+++ b/synapse/handlers/profile.py
@@ -303,6 +303,10 @@ class BaseProfileHandler(BaseHandler):
         if not self.hs.config.require_auth_for_profile_requests or not requester:
             return
 
+        # Always allow the user to query their own profile.
+        if target_user.to_string() == requester.to_string():
+            return
+
         try:
             requester_rooms = yield self.store.get_rooms_for_user(requester.to_string())
             target_user_rooms = yield self.store.get_rooms_for_user(
-- 
cgit 1.5.1


From 7b3bc755a34cf97138e614379234cfc47f91a5a9 Mon Sep 17 00:00:00 2001
From: Hubert Chathi <hubert@uhoreg.ca>
Date: Tue, 9 Jul 2019 13:37:39 -0400
Subject: remove unused and unnecessary check for FederationDeniedError (#5645)

FederationDeniedError is a subclass of SynapseError, which is a subclass of
CodeMessageException, so if e is a FederationDeniedError, then this check for
FederationDeniedError will never be reached since it will be caught by the
check for CodeMessageException above.  The check for CodeMessageException does
almost the same thing as this check (since FederationDeniedError initialises
with code=403 and msg="Federation denied with %s."), so may as well just keep
allowing it to handle this case.
---
 changelog.d/5645.misc        | 1 +
 synapse/handlers/e2e_keys.py | 5 +----
 2 files changed, 2 insertions(+), 4 deletions(-)
 create mode 100644 changelog.d/5645.misc

(limited to 'synapse/handlers')

diff --git a/changelog.d/5645.misc b/changelog.d/5645.misc
new file mode 100644
index 0000000000..4fa9699e4f
--- /dev/null
+++ b/changelog.d/5645.misc
@@ -0,0 +1 @@
+Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure.
\ No newline at end of file
diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py
index 55b4ab3a1a..fdfe8611b6 100644
--- a/synapse/handlers/e2e_keys.py
+++ b/synapse/handlers/e2e_keys.py
@@ -22,7 +22,7 @@ from canonicaljson import encode_canonical_json, json
 
 from twisted.internet import defer
 
-from synapse.api.errors import CodeMessageException, FederationDeniedError, SynapseError
+from synapse.api.errors import CodeMessageException, SynapseError
 from synapse.logging.context import make_deferred_yieldable, run_in_background
 from synapse.types import UserID, get_domain_from_id
 from synapse.util.retryutils import NotRetryingDestination
@@ -350,9 +350,6 @@ def _exception_to_failure(e):
     if isinstance(e, NotRetryingDestination):
         return {"status": 503, "message": "Not ready for retry"}
 
-    if isinstance(e, FederationDeniedError):
-        return {"status": 403, "message": "Federation Denied"}
-
     # include ConnectionRefused and other errors
     #
     # Note that some Exceptions (notably twisted's ResponseFailed etc) don't
-- 
cgit 1.5.1


From f77e99761919db671960aae4792cb563ad2b8e53 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 10 Jul 2019 15:46:42 +0100
Subject: Send 3PID bind requests as JSON data

---
 changelog.d/5656.bugfix      | 1 +
 synapse/handlers/identity.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 changelog.d/5656.bugfix

(limited to 'synapse/handlers')

diff --git a/changelog.d/5656.bugfix b/changelog.d/5656.bugfix
new file mode 100644
index 0000000000..f6ae906a9a
--- /dev/null
+++ b/changelog.d/5656.bugfix
@@ -0,0 +1 @@
+Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated.
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index c82b1933f2..ee6c2c4f8b 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -118,7 +118,7 @@ class IdentityHandler(BaseHandler):
             raise SynapseError(400, "No client_secret in creds")
 
         try:
-            data = yield self.http_client.post_urlencoded_get_json(
+            data = yield self.http_client.post_post_get_json(
                 "https://%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/bind"),
                 {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid},
             )
-- 
cgit 1.5.1


From b2a2e96ea6d6a2e4f47a9ad28919371227a5dd49 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 10 Jul 2019 15:56:21 +0100
Subject: Typo

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

(limited to 'synapse/handlers')

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index ee6c2c4f8b..546d6169e9 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -118,7 +118,7 @@ class IdentityHandler(BaseHandler):
             raise SynapseError(400, "No client_secret in creds")
 
         try:
-            data = yield self.http_client.post_post_get_json(
+            data = yield self.http_client.post_json_get_json(
                 "https://%s%s" % (id_server, "/_matrix/identity/api/v1/3pid/bind"),
                 {"sid": creds["sid"], "client_secret": client_secret, "mxid": mxid},
             )
-- 
cgit 1.5.1


From 953dbb79808c018fe34999a662f4c7cef8ea3721 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 10 Jul 2019 16:26:49 +0100
Subject: Remove access-token support from RegistrationStore.register (#5642)

The 'token' param is no longer used anywhere except the tests, so let's kill
that off too.
---
 changelog.d/5642.misc                      |  1 +
 synapse/handlers/register.py               |  2 +-
 synapse/storage/registration.py            | 24 ++++-------------------
 tests/api/test_auth.py                     |  2 +-
 tests/handlers/test_register.py            |  6 +-----
 tests/handlers/test_user_directory.py      | 16 +++++----------
 tests/storage/test_client_ips.py           |  4 +---
 tests/storage/test_monthly_active_users.py | 23 +++++++++-------------
 tests/storage/test_registration.py         | 31 +++++++-----------------------
 9 files changed, 30 insertions(+), 79 deletions(-)
 create mode 100644 changelog.d/5642.misc

(limited to 'synapse/handlers')

diff --git a/changelog.d/5642.misc b/changelog.d/5642.misc
new file mode 100644
index 0000000000..e7f8e214a4
--- /dev/null
+++ b/changelog.d/5642.misc
@@ -0,0 +1 @@
+Remove access-token support from `RegistrationStore.register`, and rename it.
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index a3e553d5f5..420c5cb5bc 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -584,7 +584,7 @@ class RegistrationHandler(BaseHandler):
                 address=address,
             )
         else:
-            return self.store.register(
+            return self.store.register_user(
                 user_id=user_id,
                 password_hash=password_hash,
                 was_guest=was_guest,
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index aea5b3276b..8e217c9408 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -698,10 +698,9 @@ class RegistrationStore(
             desc="add_access_token_to_user",
         )
 
-    def register(
+    def register_user(
         self,
         user_id,
-        token=None,
         password_hash=None,
         was_guest=False,
         make_guest=False,
@@ -714,9 +713,6 @@ class RegistrationStore(
 
         Args:
             user_id (str): The desired user ID to register.
-            token (str): The desired access token to use for this user. If this
-                is not None, the given access token is associated with the user
-                id.
             password_hash (str): Optional. The password hash for this user.
             was_guest (bool): Optional. Whether this is a guest account being
                 upgraded to a non-guest account.
@@ -733,10 +729,9 @@ class RegistrationStore(
             StoreError if the user_id could not be registered.
         """
         return self.runInteraction(
-            "register",
-            self._register,
+            "register_user",
+            self._register_user,
             user_id,
-            token,
             password_hash,
             was_guest,
             make_guest,
@@ -746,11 +741,10 @@ class RegistrationStore(
             user_type,
         )
 
-    def _register(
+    def _register_user(
         self,
         txn,
         user_id,
-        token,
         password_hash,
         was_guest,
         make_guest,
@@ -763,8 +757,6 @@ class RegistrationStore(
 
         now = int(self.clock.time())
 
-        next_id = self._access_tokens_id_gen.get_next()
-
         try:
             if was_guest:
                 # Ensure that the guest user actually exists
@@ -812,14 +804,6 @@ class RegistrationStore(
         if self._account_validity.enabled:
             self.set_expiration_date_for_user_txn(txn, user_id)
 
-        if token:
-            # it's possible for this to get a conflict, but only for a single user
-            # since tokens are namespaced based on their user ID
-            txn.execute(
-                "INSERT INTO access_tokens(id, user_id, token)" " VALUES (?,?,?)",
-                (next_id, user_id, token),
-            )
-
         if create_profile_with_displayname:
             # set a default displayname serverside to avoid ugly race
             # between auto-joins and clients trying to set displaynames
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index d4e75b5b2e..96b26f974b 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -325,7 +325,7 @@ class AuthTestCase(unittest.TestCase):
         unknown_threepid = {"medium": "email", "address": "unreserved@server.com"}
         self.hs.config.mau_limits_reserved_threepids = [threepid]
 
-        yield self.store.register(user_id="user1", token="123", password_hash=None)
+        yield self.store.register_user(user_id="user1", password_hash=None)
         with self.assertRaises(ResourceLimitError):
             yield self.auth.check_auth_blocking()
 
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index 8197f26d4f..1b7e1dacee 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -77,11 +77,7 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
         store = self.hs.get_datastore()
         frank = UserID.from_string("@frank:test")
         self.get_success(
-            store.register(
-                user_id=frank.to_string(),
-                token="jkv;g498752-43gj['eamb!-5",
-                password_hash=None,
-            )
+            store.register_user(user_id=frank.to_string(), password_hash=None)
         )
         local_part = frank.localpart
         user_id = frank.to_string()
diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py
index b135486c48..c5e91a8c41 100644
--- a/tests/handlers/test_user_directory.py
+++ b/tests/handlers/test_user_directory.py
@@ -47,11 +47,8 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
     def test_handle_local_profile_change_with_support_user(self):
         support_user_id = "@support:test"
         self.get_success(
-            self.store.register(
-                user_id=support_user_id,
-                token="123",
-                password_hash=None,
-                user_type=UserTypes.SUPPORT,
+            self.store.register_user(
+                user_id=support_user_id, password_hash=None, user_type=UserTypes.SUPPORT
             )
         )
 
@@ -73,11 +70,8 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
     def test_handle_user_deactivated_support_user(self):
         s_user_id = "@support:test"
         self.get_success(
-            self.store.register(
-                user_id=s_user_id,
-                token="123",
-                password_hash=None,
-                user_type=UserTypes.SUPPORT,
+            self.store.register_user(
+                user_id=s_user_id, password_hash=None, user_type=UserTypes.SUPPORT
             )
         )
 
@@ -90,7 +84,7 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
     def test_handle_user_deactivated_regular_user(self):
         r_user_id = "@regular:test"
         self.get_success(
-            self.store.register(user_id=r_user_id, token="123", password_hash=None)
+            self.store.register_user(user_id=r_user_id, password_hash=None)
         )
         self.store.remove_from_user_dir = Mock()
         self.get_success(self.handler.handle_user_deactivated(r_user_id))
diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py
index 59c6f8c227..09305c3bf1 100644
--- a/tests/storage/test_client_ips.py
+++ b/tests/storage/test_client_ips.py
@@ -185,9 +185,7 @@ class ClientIpStoreTestCase(unittest.HomeserverTestCase):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
         user_id = "@user:server"
-        self.get_success(
-            self.store.register(user_id=user_id, token="123", password_hash=None)
-        )
+        self.get_success(self.store.register_user(user_id=user_id, password_hash=None))
 
         active = self.get_success(self.store.user_last_seen_monthly_active(user_id))
         self.assertFalse(active)
diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py
index 0ce0b991f9..1494650d10 100644
--- a/tests/storage/test_monthly_active_users.py
+++ b/tests/storage/test_monthly_active_users.py
@@ -53,10 +53,10 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
         # -1 because user3 is a support user and does not count
         user_num = len(threepids) - 1
 
-        self.store.register(user_id=user1, token="123", password_hash=None)
-        self.store.register(user_id=user2, token="456", password_hash=None)
-        self.store.register(
-            user_id=user3, token="789", password_hash=None, user_type=UserTypes.SUPPORT
+        self.store.register_user(user_id=user1, password_hash=None)
+        self.store.register_user(user_id=user2, password_hash=None)
+        self.store.register_user(
+            user_id=user3, password_hash=None, user_type=UserTypes.SUPPORT
         )
         self.pump()
 
@@ -161,9 +161,7 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
     def test_populate_monthly_users_is_guest(self):
         # Test that guest users are not added to mau list
         user_id = "@user_id:host"
-        self.store.register(
-            user_id=user_id, token="123", password_hash=None, make_guest=True
-        )
+        self.store.register_user(user_id=user_id, password_hash=None, make_guest=True)
         self.store.upsert_monthly_active_user = Mock()
         self.store.populate_monthly_active_users(user_id)
         self.pump()
@@ -216,8 +214,8 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
         self.assertEquals(self.get_success(count), 0)
 
         # Test reserved registed users
-        self.store.register(user_id=user1, token="123", password_hash=None)
-        self.store.register(user_id=user2, token="456", password_hash=None)
+        self.store.register_user(user_id=user1, password_hash=None)
+        self.store.register_user(user_id=user2, password_hash=None)
         self.pump()
 
         now = int(self.hs.get_clock().time_msec())
@@ -232,11 +230,8 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
         self.pump()
         self.assertEqual(self.get_success(count), 0)
 
-        self.store.register(
-            user_id=support_user_id,
-            token="123",
-            password_hash=None,
-            user_type=UserTypes.SUPPORT,
+        self.store.register_user(
+            user_id=support_user_id, password_hash=None, user_type=UserTypes.SUPPORT
         )
 
         self.store.upsert_monthly_active_user(support_user_id)
diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py
index 625b651e91..9365c4622d 100644
--- a/tests/storage/test_registration.py
+++ b/tests/storage/test_registration.py
@@ -37,7 +37,7 @@ class RegistrationStoreTestCase(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_register(self):
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
 
         self.assertEquals(
             {
@@ -53,15 +53,9 @@ class RegistrationStoreTestCase(unittest.TestCase):
             (yield self.store.get_user_by_id(self.user_id)),
         )
 
-        result = yield self.store.get_user_by_access_token(self.tokens[0])
-
-        self.assertDictContainsSubset({"name": self.user_id}, result)
-
-        self.assertTrue("token_id" in result)
-
     @defer.inlineCallbacks
     def test_add_tokens(self):
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
         yield self.store.add_access_token_to_user(
             self.user_id, self.tokens[1], self.device_id
         )
@@ -77,7 +71,8 @@ class RegistrationStoreTestCase(unittest.TestCase):
     @defer.inlineCallbacks
     def test_user_delete_access_tokens(self):
         # add some tokens
-        yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
+        yield self.store.register_user(self.user_id, self.pwhash)
+        yield self.store.add_access_token_to_user(self.user_id, self.tokens[0])
         yield self.store.add_access_token_to_user(
             self.user_id, self.tokens[1], self.device_id
         )
@@ -108,24 +103,12 @@ class RegistrationStoreTestCase(unittest.TestCase):
 
         res = yield self.store.is_support_user(None)
         self.assertFalse(res)
-        yield self.store.register(user_id=TEST_USER, token="123", password_hash=None)
+        yield self.store.register_user(user_id=TEST_USER, password_hash=None)
         res = yield self.store.is_support_user(TEST_USER)
         self.assertFalse(res)
 
-        yield self.store.register(
-            user_id=SUPPORT_USER,
-            token="456",
-            password_hash=None,
-            user_type=UserTypes.SUPPORT,
+        yield self.store.register_user(
+            user_id=SUPPORT_USER, password_hash=None, user_type=UserTypes.SUPPORT
         )
         res = yield self.store.is_support_user(SUPPORT_USER)
         self.assertTrue(res)
-
-
-class TokenGenerator:
-    def __init__(self):
-        self._last_issued_token = 0
-
-    def generate(self, user_id):
-        self._last_issued_token += 1
-        return "%s-%d" % (user_id, self._last_issued_token)
-- 
cgit 1.5.1


From 1890cfcf82aa3e69530e97bf9ce783e390f22fbe Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 10 Jul 2019 19:10:07 +0100
Subject: Inline issue_access_token (#5659)

this is only used in one place, so it's clearer if we inline it and reduce the
API surface.

Also, fixes a buglet where we would create an access token even if we were
about to block the user (we would never return the AT, so the user could never
use it, but it was still created and added to the db.)
---
 changelog.d/5659.misc    |  1 +
 synapse/handlers/auth.py | 10 +++-------
 tests/api/test_auth.py   |  2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)
 create mode 100644 changelog.d/5659.misc

(limited to 'synapse/handlers')

diff --git a/changelog.d/5659.misc b/changelog.d/5659.misc
new file mode 100644
index 0000000000..686001295c
--- /dev/null
+++ b/changelog.d/5659.misc
@@ -0,0 +1 @@
+Inline issue_access_token.
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index ef5585aa99..da312b188e 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -578,9 +578,11 @@ class AuthHandler(BaseHandler):
             StoreError if there was a problem storing the token.
         """
         logger.info("Logging in user %s on device %s", user_id, device_id)
-        access_token = yield self.issue_access_token(user_id, device_id)
         yield self.auth.check_auth_blocking(user_id)
 
+        access_token = self.macaroon_gen.generate_access_token(user_id)
+        yield self.store.add_access_token_to_user(user_id, access_token, device_id)
+
         # the device *should* have been registered before we got here; however,
         # it's possible we raced against a DELETE operation. The thing we
         # really don't want is active access_tokens without a record of the
@@ -831,12 +833,6 @@ class AuthHandler(BaseHandler):
             defer.returnValue(None)
         defer.returnValue(user_id)
 
-    @defer.inlineCallbacks
-    def issue_access_token(self, user_id, device_id=None):
-        access_token = self.macaroon_gen.generate_access_token(user_id)
-        yield self.store.add_access_token_to_user(user_id, access_token, device_id)
-        defer.returnValue(access_token)
-
     @defer.inlineCallbacks
     def validate_short_term_login_token_and_get_user_id(self, login_token):
         auth_api = self.hs.get_auth()
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index 96b26f974b..ddf2b578b3 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -244,7 +244,7 @@ class AuthTestCase(unittest.TestCase):
         USER_ID = "@percy:matrix.org"
         self.store.add_access_token_to_user = Mock()
 
-        token = yield self.hs.handlers.auth_handler.issue_access_token(
+        token = yield self.hs.handlers.auth_handler.get_access_token_for_user_id(
             USER_ID, "DEVICE"
         )
         self.store.add_access_token_to_user.assert_called_with(USER_ID, token, "DEVICE")
-- 
cgit 1.5.1


From 5f158ec039e4753959aad9b8d288b3d8cb4959a1 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Fri, 12 Jul 2019 17:26:02 +0100
Subject: Implement access token expiry (#5660)

Record how long an access token is valid for, and raise a soft-logout once it
expires.
---
 changelog.d/5660.feature                           |   1 +
 docs/sample_config.yaml                            |  11 +++
 synapse/api/auth.py                                |  12 +++
 synapse/api/errors.py                              |   8 +-
 synapse/config/registration.py                     |  16 +++
 synapse/handlers/auth.py                           |  17 +++-
 synapse/handlers/register.py                       |  35 ++++---
 synapse/storage/registration.py                    |  19 +++-
 .../schema/delta/55/access_token_expiry.sql        |  18 ++++
 tests/api/test_auth.py                             |   6 +-
 tests/handlers/test_auth.py                        |  20 +++-
 tests/handlers/test_register.py                    |   5 +-
 tests/rest/client/v1/test_login.py                 | 108 +++++++++++++++++++++
 tests/storage/test_registration.py                 |   8 +-
 14 files changed, 253 insertions(+), 31 deletions(-)
 create mode 100644 changelog.d/5660.feature
 create mode 100644 synapse/storage/schema/delta/55/access_token_expiry.sql

(limited to 'synapse/handlers')

diff --git a/changelog.d/5660.feature b/changelog.d/5660.feature
new file mode 100644
index 0000000000..82889fdaf1
--- /dev/null
+++ b/changelog.d/5660.feature
@@ -0,0 +1 @@
+Implement `session_lifetime` configuration option, after which access tokens will expire.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 0462f0a17a..663ff31622 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -786,6 +786,17 @@ uploads_path: "DATADIR/uploads"
 #  renew_at: 1w
 #  renew_email_subject: "Renew your %(app)s account"
 
+# Time that a user's session remains valid for, after they log in.
+#
+# Note that this is not currently compatible with guest logins.
+#
+# Note also that this is calculated at login time: changes are not applied
+# retrospectively to users who have already logged in.
+#
+# By default, this is infinite.
+#
+#session_lifetime: 24h
+
 # The user must provide all of the below types of 3PID when registering.
 #
 #registrations_require_3pid:
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index afc6400948..d9e943c39c 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -319,6 +319,17 @@ class Auth(object):
             # first look in the database
             r = yield self._look_up_user_by_access_token(token)
             if r:
+                valid_until_ms = r["valid_until_ms"]
+                if (
+                    valid_until_ms is not None
+                    and valid_until_ms < self.clock.time_msec()
+                ):
+                    # there was a valid access token, but it has expired.
+                    # soft-logout the user.
+                    raise InvalidClientTokenError(
+                        msg="Access token has expired", soft_logout=True
+                    )
+
                 defer.returnValue(r)
 
         # otherwise it needs to be a valid macaroon
@@ -505,6 +516,7 @@ class Auth(object):
             "token_id": ret.get("token_id", None),
             "is_guest": False,
             "device_id": ret.get("device_id"),
+            "valid_until_ms": ret.get("valid_until_ms"),
         }
         defer.returnValue(user_info)
 
diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index 41fd04cd54..a6e753c30c 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -245,8 +245,14 @@ class MissingClientTokenError(InvalidClientCredentialsError):
 class InvalidClientTokenError(InvalidClientCredentialsError):
     """Raised when we didn't understand the access token in a request"""
 
-    def __init__(self, msg="Unrecognised access token"):
+    def __init__(self, msg="Unrecognised access token", soft_logout=False):
         super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN")
+        self._soft_logout = soft_logout
+
+    def error_dict(self):
+        d = super().error_dict()
+        d["soft_logout"] = self._soft_logout
+        return d
 
 
 class ResourceLimitError(SynapseError):
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index b895c4e9f4..34cb11468c 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -84,6 +84,11 @@ class RegistrationConfig(Config):
             "disable_msisdn_registration", False
         )
 
+        session_lifetime = config.get("session_lifetime")
+        if session_lifetime is not None:
+            session_lifetime = self.parse_duration(session_lifetime)
+        self.session_lifetime = session_lifetime
+
     def generate_config_section(self, generate_secrets=False, **kwargs):
         if generate_secrets:
             registration_shared_secret = 'registration_shared_secret: "%s"' % (
@@ -141,6 +146,17 @@ class RegistrationConfig(Config):
         #  renew_at: 1w
         #  renew_email_subject: "Renew your %%(app)s account"
 
+        # Time that a user's session remains valid for, after they log in.
+        #
+        # Note that this is not currently compatible with guest logins.
+        #
+        # Note also that this is calculated at login time: changes are not applied
+        # retrospectively to users who have already logged in.
+        #
+        # By default, this is infinite.
+        #
+        #session_lifetime: 24h
+
         # The user must provide all of the below types of 3PID when registering.
         #
         #registrations_require_3pid:
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index da312b188e..b74a6e9c62 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -15,6 +15,7 @@
 # limitations under the License.
 
 import logging
+import time
 import unicodedata
 
 import attr
@@ -558,7 +559,7 @@ class AuthHandler(BaseHandler):
         return self.sessions[session_id]
 
     @defer.inlineCallbacks
-    def get_access_token_for_user_id(self, user_id, device_id=None):
+    def get_access_token_for_user_id(self, user_id, device_id, valid_until_ms):
         """
         Creates a new access token for the user with the given user ID.
 
@@ -572,16 +573,26 @@ 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)
+            valid_until_ms (int|None): when the token is valid until. None for
+                no expiry.
         Returns:
               The access token for the user's session.
         Raises:
             StoreError if there was a problem storing the token.
         """
-        logger.info("Logging in user %s on device %s", user_id, device_id)
+        fmt_expiry = ""
+        if valid_until_ms is not None:
+            fmt_expiry = time.strftime(
+                " until %Y-%m-%d %H:%M:%S", time.localtime(valid_until_ms / 1000.0)
+            )
+        logger.info("Logging in user %s on device %s%s", user_id, device_id, fmt_expiry)
+
         yield self.auth.check_auth_blocking(user_id)
 
         access_token = self.macaroon_gen.generate_access_token(user_id)
-        yield self.store.add_access_token_to_user(user_id, access_token, device_id)
+        yield self.store.add_access_token_to_user(
+            user_id, access_token, device_id, valid_until_ms
+        )
 
         # the device *should* have been registered before we got here; however,
         # it's possible we raced against a DELETE operation. The thing we
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 420c5cb5bc..bb7cfd71b9 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -84,6 +84,8 @@ class RegistrationHandler(BaseHandler):
             self.device_handler = hs.get_device_handler()
             self.pusher_pool = hs.get_pusherpool()
 
+        self.session_lifetime = hs.config.session_lifetime
+
     @defer.inlineCallbacks
     def check_username(self, localpart, guest_access_token=None, assigned_user_id=None):
         if types.contains_invalid_mxid_characters(localpart):
@@ -599,6 +601,8 @@ class RegistrationHandler(BaseHandler):
     def register_device(self, user_id, device_id, initial_display_name, is_guest=False):
         """Register a device for a user and generate an access token.
 
+        The access token will be limited by the homeserver's session_lifetime config.
+
         Args:
             user_id (str): full canonical @user:id
             device_id (str|None): The device ID to check, or None to generate
@@ -619,20 +623,29 @@ class RegistrationHandler(BaseHandler):
                 is_guest=is_guest,
             )
             defer.returnValue((r["device_id"], r["access_token"]))
-        else:
-            device_id = yield self.device_handler.check_device_registered(
-                user_id, device_id, initial_display_name
-            )
+
+        valid_until_ms = None
+        if self.session_lifetime is not None:
             if is_guest:
-                access_token = self.macaroon_gen.generate_access_token(
-                    user_id, ["guest = true"]
-                )
-            else:
-                access_token = yield self._auth_handler.get_access_token_for_user_id(
-                    user_id, device_id=device_id
+                raise Exception(
+                    "session_lifetime is not currently implemented for guest access"
                 )
+            valid_until_ms = self.clock.time_msec() + self.session_lifetime
+
+        device_id = yield self.device_handler.check_device_registered(
+            user_id, device_id, initial_display_name
+        )
+        if is_guest:
+            assert valid_until_ms is None
+            access_token = self.macaroon_gen.generate_access_token(
+                user_id, ["guest = true"]
+            )
+        else:
+            access_token = yield self._auth_handler.get_access_token_for_user_id(
+                user_id, device_id=device_id, valid_until_ms=valid_until_ms
+            )
 
-            defer.returnValue((device_id, access_token))
+        defer.returnValue((device_id, access_token))
 
     @defer.inlineCallbacks
     def post_registration_actions(
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index 73580f1725..8b2c2a97ab 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -90,7 +90,8 @@ class RegistrationWorkerStore(SQLBaseStore):
             token (str): The access token of a user.
         Returns:
             defer.Deferred: None, if the token did not match, otherwise dict
-                including the keys `name`, `is_guest`, `device_id`, `token_id`.
+                including the keys `name`, `is_guest`, `device_id`, `token_id`,
+                `valid_until_ms`.
         """
         return self.runInteraction(
             "get_user_by_access_token", self._query_for_auth, token
@@ -284,7 +285,7 @@ class RegistrationWorkerStore(SQLBaseStore):
     def _query_for_auth(self, txn, token):
         sql = (
             "SELECT users.name, users.is_guest, access_tokens.id as token_id,"
-            " access_tokens.device_id"
+            " access_tokens.device_id, access_tokens.valid_until_ms"
             " FROM users"
             " INNER JOIN access_tokens on users.name = access_tokens.user_id"
             " WHERE token = ?"
@@ -679,14 +680,16 @@ class RegistrationStore(
         defer.returnValue(batch_size)
 
     @defer.inlineCallbacks
-    def add_access_token_to_user(self, user_id, token, device_id=None):
+    def add_access_token_to_user(self, user_id, token, device_id, valid_until_ms):
         """Adds an access token for the given user.
 
         Args:
             user_id (str): The user ID.
             token (str): The new access token to add.
             device_id (str): ID of the device to associate with the access
-               token
+                token
+            valid_until_ms (int|None): when the token is valid until. None for
+                no expiry.
         Raises:
             StoreError if there was a problem adding this.
         """
@@ -694,7 +697,13 @@ class RegistrationStore(
 
         yield self._simple_insert(
             "access_tokens",
-            {"id": next_id, "user_id": user_id, "token": token, "device_id": device_id},
+            {
+                "id": next_id,
+                "user_id": user_id,
+                "token": token,
+                "device_id": device_id,
+                "valid_until_ms": valid_until_ms,
+            },
             desc="add_access_token_to_user",
         )
 
diff --git a/synapse/storage/schema/delta/55/access_token_expiry.sql b/synapse/storage/schema/delta/55/access_token_expiry.sql
new file mode 100644
index 0000000000..4590604bfd
--- /dev/null
+++ b/synapse/storage/schema/delta/55/access_token_expiry.sql
@@ -0,0 +1,18 @@
+/* Copyright 2019 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+-- when this access token can be used until, in ms since the epoch. NULL means the token
+-- never expires.
+ALTER TABLE access_tokens ADD COLUMN valid_until_ms BIGINT;
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index ee92ceeb60..c0cb8ef296 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -262,9 +262,11 @@ class AuthTestCase(unittest.TestCase):
         self.store.add_access_token_to_user = Mock()
 
         token = yield self.hs.handlers.auth_handler.get_access_token_for_user_id(
-            USER_ID, "DEVICE"
+            USER_ID, "DEVICE", valid_until_ms=None
+        )
+        self.store.add_access_token_to_user.assert_called_with(
+            USER_ID, token, "DEVICE", None
         )
-        self.store.add_access_token_to_user.assert_called_with(USER_ID, token, "DEVICE")
 
         def get_user(tok):
             if token != tok:
diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py
index b204a0700d..b03103d96f 100644
--- a/tests/handlers/test_auth.py
+++ b/tests/handlers/test_auth.py
@@ -117,7 +117,9 @@ class AuthTestCase(unittest.TestCase):
     def test_mau_limits_disabled(self):
         self.hs.config.limit_usage_by_mau = False
         # Ensure does not throw exception
-        yield self.auth_handler.get_access_token_for_user_id("user_a")
+        yield self.auth_handler.get_access_token_for_user_id(
+            "user_a", device_id=None, valid_until_ms=None
+        )
 
         yield self.auth_handler.validate_short_term_login_token_and_get_user_id(
             self._get_macaroon().serialize()
@@ -131,7 +133,9 @@ class AuthTestCase(unittest.TestCase):
         )
 
         with self.assertRaises(ResourceLimitError):
-            yield self.auth_handler.get_access_token_for_user_id("user_a")
+            yield self.auth_handler.get_access_token_for_user_id(
+                "user_a", device_id=None, valid_until_ms=None
+            )
 
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.large_number_of_users)
@@ -150,7 +154,9 @@ class AuthTestCase(unittest.TestCase):
             return_value=defer.succeed(self.hs.config.max_mau_value)
         )
         with self.assertRaises(ResourceLimitError):
-            yield self.auth_handler.get_access_token_for_user_id("user_a")
+            yield self.auth_handler.get_access_token_for_user_id(
+                "user_a", device_id=None, valid_until_ms=None
+            )
 
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.hs.config.max_mau_value)
@@ -166,7 +172,9 @@ class AuthTestCase(unittest.TestCase):
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.hs.config.max_mau_value)
         )
-        yield self.auth_handler.get_access_token_for_user_id("user_a")
+        yield self.auth_handler.get_access_token_for_user_id(
+            "user_a", device_id=None, valid_until_ms=None
+        )
         self.hs.get_datastore().user_last_seen_monthly_active = Mock(
             return_value=defer.succeed(self.hs.get_clock().time_msec())
         )
@@ -185,7 +193,9 @@ class AuthTestCase(unittest.TestCase):
             return_value=defer.succeed(self.small_number_of_users)
         )
         # Ensure does not raise exception
-        yield self.auth_handler.get_access_token_for_user_id("user_a")
+        yield self.auth_handler.get_access_token_for_user_id(
+            "user_a", device_id=None, valid_until_ms=None
+        )
 
         self.hs.get_datastore().get_monthly_active_count = Mock(
             return_value=defer.succeed(self.small_number_of_users)
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index 1b7e1dacee..90d0129374 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -272,7 +272,10 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
             )
         else:
             yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id)
-        yield self.store.add_access_token_to_user(user_id=user_id, token=token)
+
+        yield self.store.add_access_token_to_user(
+            user_id=user_id, token=token, device_id=None, valid_until_ms=None
+        )
 
         if displayname is not None:
             # logger.info("setting user display name: %s -> %s", user_id, displayname)
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
index 0397f91a9e..eae5411325 100644
--- a/tests/rest/client/v1/test_login.py
+++ b/tests/rest/client/v1/test_login.py
@@ -2,10 +2,14 @@ import json
 
 import synapse.rest.admin
 from synapse.rest.client.v1 import login
+from synapse.rest.client.v2_alpha import devices
+from synapse.rest.client.v2_alpha.account import WhoamiRestServlet
 
 from tests import unittest
+from tests.unittest import override_config
 
 LOGIN_URL = b"/_matrix/client/r0/login"
+TEST_URL = b"/_matrix/client/r0/account/whoami"
 
 
 class LoginRestServletTestCase(unittest.HomeserverTestCase):
@@ -13,6 +17,8 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
     servlets = [
         synapse.rest.admin.register_servlets_for_client_rest_resource,
         login.register_servlets,
+        devices.register_servlets,
+        lambda hs, http_server: WhoamiRestServlet(hs).register(http_server),
     ]
 
     def make_homeserver(self, reactor, clock):
@@ -144,3 +150,105 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
         self.render(request)
 
         self.assertEquals(channel.result["code"], b"403", channel.result)
+
+    @override_config({"session_lifetime": "24h"})
+    def test_soft_logout(self):
+        self.register_user("kermit", "monkey")
+
+        # we shouldn't be able to make requests without an access token
+        request, channel = self.make_request(b"GET", TEST_URL)
+        self.render(request)
+        self.assertEquals(channel.result["code"], b"401", channel.result)
+        self.assertEquals(channel.json_body["errcode"], "M_MISSING_TOKEN")
+
+        # log in as normal
+        params = {
+            "type": "m.login.password",
+            "identifier": {"type": "m.id.user", "user": "kermit"},
+            "password": "monkey",
+        }
+        request, channel = self.make_request(b"POST", LOGIN_URL, params)
+        self.render(request)
+
+        self.assertEquals(channel.code, 200, channel.result)
+        access_token = channel.json_body["access_token"]
+        device_id = channel.json_body["device_id"]
+
+        # we should now be able to make requests with the access token
+        request, channel = self.make_request(
+            b"GET", TEST_URL, access_token=access_token
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 200, channel.result)
+
+        # time passes
+        self.reactor.advance(24 * 3600)
+
+        # ... and we should be soft-logouted
+        request, channel = self.make_request(
+            b"GET", TEST_URL, access_token=access_token
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 401, channel.result)
+        self.assertEquals(channel.json_body["errcode"], "M_UNKNOWN_TOKEN")
+        self.assertEquals(channel.json_body["soft_logout"], True)
+
+        #
+        # test behaviour after deleting the expired device
+        #
+
+        # we now log in as a different device
+        access_token_2 = self.login("kermit", "monkey")
+
+        # more requests with the expired token should still return a soft-logout
+        self.reactor.advance(3600)
+        request, channel = self.make_request(
+            b"GET", TEST_URL, access_token=access_token
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 401, channel.result)
+        self.assertEquals(channel.json_body["errcode"], "M_UNKNOWN_TOKEN")
+        self.assertEquals(channel.json_body["soft_logout"], True)
+
+        # ... but if we delete that device, it will be a proper logout
+        self._delete_device(access_token_2, "kermit", "monkey", device_id)
+
+        request, channel = self.make_request(
+            b"GET", TEST_URL, access_token=access_token
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 401, channel.result)
+        self.assertEquals(channel.json_body["errcode"], "M_UNKNOWN_TOKEN")
+        self.assertEquals(channel.json_body["soft_logout"], False)
+
+    def _delete_device(self, access_token, user_id, password, device_id):
+        """Perform the UI-Auth to delete a device"""
+        request, channel = self.make_request(
+            b"DELETE", "devices/" + device_id, access_token=access_token
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 401, channel.result)
+        # check it's a UI-Auth fail
+        self.assertEqual(
+            set(channel.json_body.keys()),
+            {"flows", "params", "session"},
+            channel.result,
+        )
+
+        auth = {
+            "type": "m.login.password",
+            # https://github.com/matrix-org/synapse/issues/5665
+            # "identifier": {"type": "m.id.user", "user": user_id},
+            "user": user_id,
+            "password": password,
+            "session": channel.json_body["session"],
+        }
+
+        request, channel = self.make_request(
+            b"DELETE",
+            "devices/" + device_id,
+            access_token=access_token,
+            content={"auth": auth},
+        )
+        self.render(request)
+        self.assertEquals(channel.code, 200, channel.result)
diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py
index 9365c4622d..0253c4ac05 100644
--- a/tests/storage/test_registration.py
+++ b/tests/storage/test_registration.py
@@ -57,7 +57,7 @@ class RegistrationStoreTestCase(unittest.TestCase):
     def test_add_tokens(self):
         yield self.store.register_user(self.user_id, self.pwhash)
         yield self.store.add_access_token_to_user(
-            self.user_id, self.tokens[1], self.device_id
+            self.user_id, self.tokens[1], self.device_id, valid_until_ms=None
         )
 
         result = yield self.store.get_user_by_access_token(self.tokens[1])
@@ -72,9 +72,11 @@ class RegistrationStoreTestCase(unittest.TestCase):
     def test_user_delete_access_tokens(self):
         # add some tokens
         yield self.store.register_user(self.user_id, self.pwhash)
-        yield self.store.add_access_token_to_user(self.user_id, self.tokens[0])
         yield self.store.add_access_token_to_user(
-            self.user_id, self.tokens[1], self.device_id
+            self.user_id, self.tokens[0], device_id=None, valid_until_ms=None
+        )
+        yield self.store.add_access_token_to_user(
+            self.user_id, self.tokens[1], self.device_id, valid_until_ms=None
         )
 
         # now delete some
-- 
cgit 1.5.1


From 18c516698e78004df39ec62c3fc08ff03313ba4e Mon Sep 17 00:00:00 2001
From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Date: Mon, 15 Jul 2019 11:45:29 +0100
Subject: Return a different error from Invalid Password when a user is
 deactivated (#5674)

Return `This account has been deactivated` instead of `Invalid password` when a user is deactivated.
---
 changelog.d/5674.feature |  1 +
 synapse/api/errors.py    | 16 ++++++++++++++++
 synapse/handlers/auth.py |  9 +++++++++
 3 files changed, 26 insertions(+)
 create mode 100644 changelog.d/5674.feature

(limited to 'synapse/handlers')

diff --git a/changelog.d/5674.feature b/changelog.d/5674.feature
new file mode 100644
index 0000000000..04bdfa4ad5
--- /dev/null
+++ b/changelog.d/5674.feature
@@ -0,0 +1 @@
+Return "This account has been deactivated" when a deactivated user tries to login.
diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index a6e753c30c..ad3e262041 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -139,6 +139,22 @@ class ConsentNotGivenError(SynapseError):
         return cs_error(self.msg, self.errcode, consent_uri=self._consent_uri)
 
 
+class UserDeactivatedError(SynapseError):
+    """The error returned to the client when the user attempted to access an
+    authenticated endpoint, but the account has been deactivated.
+    """
+
+    def __init__(self, msg):
+        """Constructs a UserDeactivatedError
+
+        Args:
+            msg (str): The human-readable error message
+        """
+        super(UserDeactivatedError, self).__init__(
+            code=http_client.FORBIDDEN, msg=msg, errcode=Codes.UNKNOWN
+        )
+
+
 class RegistrationError(SynapseError):
     """An error raised when a registration event fails."""
 
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index b74a6e9c62..d4d6574975 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -35,6 +35,7 @@ from synapse.api.errors import (
     LoginError,
     StoreError,
     SynapseError,
+    UserDeactivatedError,
 )
 from synapse.api.ratelimiting import Ratelimiter
 from synapse.logging.context import defer_to_thread
@@ -623,6 +624,7 @@ class AuthHandler(BaseHandler):
         Raises:
             LimitExceededError if the ratelimiter's login requests count for this
                 user is too high too proceed.
+            UserDeactivatedError if a user is found but is deactivated.
         """
         self.ratelimit_login_per_account(user_id)
         res = yield self._find_user_id_and_pwd_hash(user_id)
@@ -838,6 +840,13 @@ class AuthHandler(BaseHandler):
         if not lookupres:
             defer.returnValue(None)
         (user_id, password_hash) = lookupres
+
+        # If the password hash is None, the account has likely been deactivated
+        if not password_hash:
+            deactivated = yield self.store.get_user_deactivated_status(user_id)
+            if deactivated:
+                raise UserDeactivatedError("This account has been deactivated")
+
         result = yield self.validate_hash(password, password_hash)
         if not result:
             logger.warn("Failed password login for user %s", user_id)
-- 
cgit 1.5.1


From 9c70a02a9cddf36521c3d6ae6f72e3b46a5f4c2d Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 17 Jul 2019 19:08:02 +0100
Subject: Ignore redactions of m.room.create events (#5701)

---
 changelog.d/5701.bugfix              |  1 +
 synapse/api/auth.py                  | 15 ---------------
 synapse/handlers/message.py          | 33 ++++++++++++++++++++++++---------
 synapse/storage/events_worker.py     | 12 ++++++++++++
 tests/rest/client/test_redactions.py | 20 ++++++++++++++++++++
 5 files changed, 57 insertions(+), 24 deletions(-)
 create mode 100644 changelog.d/5701.bugfix

(limited to 'synapse/handlers')

diff --git a/changelog.d/5701.bugfix b/changelog.d/5701.bugfix
new file mode 100644
index 0000000000..fd2866e16a
--- /dev/null
+++ b/changelog.d/5701.bugfix
@@ -0,0 +1 @@
+Ignore redactions of m.room.create events.
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index d9e943c39c..7ce6540bdd 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -606,21 +606,6 @@ class Auth(object):
 
         defer.returnValue(auth_ids)
 
-    def check_redaction(self, room_version, event, auth_events):
-        """Check whether the event sender is allowed to redact the target event.
-
-        Returns:
-            True if the the sender is allowed to redact the target event if the
-                target event was created by them.
-            False if the sender is allowed to redact the target event with no
-                further checks.
-
-        Raises:
-            AuthError if the event sender is definitely not allowed to redact
-                the target event.
-        """
-        return event_auth.check_redaction(room_version, event, auth_events)
-
     @defer.inlineCallbacks
     def check_can_change_room_list(self, room_id, user):
         """Check if the user is allowed to edit the room's entry in the
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index eaeda7a5cb..6d7a987f13 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -23,6 +23,7 @@ from canonicaljson import encode_canonical_json, json
 from twisted.internet import defer
 from twisted.internet.defer import succeed
 
+from synapse import event_auth
 from synapse.api.constants import EventTypes, Membership, RelationTypes
 from synapse.api.errors import (
     AuthError,
@@ -784,6 +785,20 @@ class EventCreationHandler(object):
                     event.signatures.update(returned_invite.signatures)
 
         if event.type == EventTypes.Redaction:
+            original_event = yield self.store.get_event(
+                event.redacts,
+                check_redacted=False,
+                get_prev_content=False,
+                allow_rejected=False,
+                allow_none=True,
+                check_room_id=event.room_id,
+            )
+
+            # we can make some additional checks now if we have the original event.
+            if original_event:
+                if original_event.type == EventTypes.Create:
+                    raise AuthError(403, "Redacting create events is not permitted")
+
             prev_state_ids = yield context.get_prev_state_ids(self.store)
             auth_events_ids = yield self.auth.compute_auth_events(
                 event, prev_state_ids, for_verification=True
@@ -791,18 +806,18 @@ class EventCreationHandler(object):
             auth_events = yield self.store.get_events(auth_events_ids)
             auth_events = {(e.type, e.state_key): e for e in auth_events.values()}
             room_version = yield self.store.get_room_version(event.room_id)
-            if self.auth.check_redaction(room_version, event, auth_events=auth_events):
-                original_event = yield self.store.get_event(
-                    event.redacts,
-                    check_redacted=False,
-                    get_prev_content=False,
-                    allow_rejected=False,
-                    allow_none=False,
-                )
+
+            if event_auth.check_redaction(room_version, event, auth_events=auth_events):
+                # this user doesn't have 'redact' rights, so we need to do some more
+                # checks on the original event. Let's start by checking the original
+                # event exists.
+                if not original_event:
+                    raise NotFoundError("Could not find event %s" % (event.redacts,))
+
                 if event.user_id != original_event.user_id:
                     raise AuthError(403, "You don't have permission to redact events")
 
-                # We've already checked.
+                # all the checks are done.
                 event.internal_metadata.recheck_redaction = False
 
         if event.type == EventTypes.Create:
diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py
index 1d969d70be..858fc755a1 100644
--- a/synapse/storage/events_worker.py
+++ b/synapse/storage/events_worker.py
@@ -259,6 +259,14 @@ class EventsWorkerStore(SQLBaseStore):
                     continue
 
                 original_event = original_event_entry.event
+                if original_event.type == EventTypes.Create:
+                    # we never serve redactions of Creates to clients.
+                    logger.info(
+                        "Withholding redaction %s of create event %s",
+                        event_id,
+                        redacted_event_id,
+                    )
+                    continue
 
                 if entry.event.internal_metadata.need_to_check_redaction():
                     original_domain = get_domain_from_id(original_event.sender)
@@ -617,6 +625,10 @@ class EventsWorkerStore(SQLBaseStore):
             Deferred[EventBase|None]: if the event should be redacted, a pruned
                 event object. Otherwise, None.
         """
+        if original_ev.type == "m.room.create":
+            # we choose to ignore redactions of m.room.create events.
+            return None
+
         redaction_map = yield self._get_events_from_cache_or_db(redactions)
 
         for redaction_id in redactions:
diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py
index 7d5df95855..fe66e397c4 100644
--- a/tests/rest/client/test_redactions.py
+++ b/tests/rest/client/test_redactions.py
@@ -157,3 +157,23 @@ class RedactionsTestCase(HomeserverTestCase):
         self.assertEqual(timeline[-2]["event_id"], msg_id)
         self.assertEqual(timeline[-2]["unsigned"]["redacted_by"], redaction_id)
         self.assertEqual(timeline[-2]["content"], {})
+
+    def test_redact_create_event(self):
+        # control case: an existing event
+        b = self.helper.send(room_id=self.room_id, tok=self.mod_access_token)
+        msg_id = b["event_id"]
+        self._redact_event(self.mod_access_token, self.room_id, msg_id)
+
+        # sync the room, to get the id of the create event
+        timeline = self._sync_room_timeline(self.other_access_token, self.room_id)
+        create_event_id = timeline[0]["event_id"]
+
+        # room moderators cannot send redactions for create events
+        self._redact_event(
+            self.mod_access_token, self.room_id, create_event_id, expect_code=403
+        )
+
+        # and nor can normals
+        self._redact_event(
+            self.other_access_token, self.room_id, create_event_id, expect_code=403
+        )
-- 
cgit 1.5.1