summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--changelog.d/5390.bugfix1
-rw-r--r--changelog.d/5394.bugfix1
-rw-r--r--changelog.d/5460.misc1
-rw-r--r--changelog.d/5465.misc2
-rwxr-xr-xdemo/start.sh4
-rw-r--r--synapse/handlers/account_validity.py3
-rw-r--r--synapse/handlers/deactivate_account.py6
-rw-r--r--synapse/http/client.py13
-rw-r--r--synapse/rest/media/v1/media_repository.py6
-rw-r--r--synapse/storage/_base.py4
-rw-r--r--synapse/storage/registration.py20
-rw-r--r--tests/rest/client/v2_alpha/test_register.py67
12 files changed, 91 insertions, 37 deletions
diff --git a/changelog.d/5390.bugfix b/changelog.d/5390.bugfix
new file mode 100644
index 0000000000..e7b7483cf2
--- /dev/null
+++ b/changelog.d/5390.bugfix
@@ -0,0 +1 @@
+Fix handling of failures fetching remote content to not log failures as exceptions.
diff --git a/changelog.d/5394.bugfix b/changelog.d/5394.bugfix
new file mode 100644
index 0000000000..2ad9fbe82c
--- /dev/null
+++ b/changelog.d/5394.bugfix
@@ -0,0 +1 @@
+Fix a bug where deactivated users could receive renewal emails if the account validity feature is on.
diff --git a/changelog.d/5460.misc b/changelog.d/5460.misc
new file mode 100644
index 0000000000..badc8bb79a
--- /dev/null
+++ b/changelog.d/5460.misc
@@ -0,0 +1 @@
+Demo script now uses python3.
diff --git a/changelog.d/5465.misc b/changelog.d/5465.misc
new file mode 100644
index 0000000000..af5f0f8f45
--- /dev/null
+++ b/changelog.d/5465.misc
@@ -0,0 +1,2 @@
+Track deactivated accounts in the database.
+
diff --git a/demo/start.sh b/demo/start.sh
index c4a1328a6f..5c3a8fe61f 100755
--- a/demo/start.sh
+++ b/demo/start.sh
@@ -21,7 +21,7 @@ for port in 8080 8081 8082; do
     pushd demo/$port
 
     #rm $DIR/etc/$port.config
-    python -m synapse.app.homeserver \
+    python3 -m synapse.app.homeserver \
         --generate-config \
         -H "localhost:$https_port" \
         --config-path "$DIR/etc/$port.config" \
@@ -55,7 +55,7 @@ for port in 8080 8081 8082; do
         echo "report_stats: false" >> $DIR/etc/$port.config
     fi
 
-    python -m synapse.app.homeserver \
+    python3 -m synapse.app.homeserver \
         --config-path "$DIR/etc/$port.config" \
         -D \
         -vv \
diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py
index 261446517d..5e0b92eb1c 100644
--- a/synapse/handlers/account_validity.py
+++ b/synapse/handlers/account_validity.py
@@ -110,6 +110,9 @@ class AccountValidityHandler(object):
         # Stop right here if the user doesn't have at least one email address.
         # In this case, they will have to ask their server admin to renew their
         # account manually.
+        # We don't need to do a specific check to make sure the account isn't
+        # deactivated, as a deactivated account isn't supposed to have any
+        # email address attached to it.
         if not addresses:
             return
 
diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py
index b29089d82c..7378b56c1d 100644
--- a/synapse/handlers/deactivate_account.py
+++ b/synapse/handlers/deactivate_account.py
@@ -43,6 +43,8 @@ class DeactivateAccountHandler(BaseHandler):
         # it left off (if it has work left to do).
         hs.get_reactor().callWhenRunning(self._start_user_parting)
 
+        self._account_validity_enabled = hs.config.account_validity.enabled
+
     @defer.inlineCallbacks
     def deactivate_account(self, user_id, erase_data, id_server=None):
         """Deactivate a user's account
@@ -115,6 +117,10 @@ class DeactivateAccountHandler(BaseHandler):
         # parts users from rooms (if it isn't already running)
         self._start_user_parting()
 
+        # Remove all information on the user from the account_validity table.
+        if self._account_validity_enabled:
+            yield self.store.delete_account_validity_for_user(user_id)
+
         # Mark the user as deactivated.
         yield self.store.set_user_deactivated_status(user_id, True)
 
diff --git a/synapse/http/client.py b/synapse/http/client.py
index 77fe68818b..5c073fff07 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -17,7 +17,7 @@
 import logging
 from io import BytesIO
 
-from six import text_type
+from six import raise_from, text_type
 from six.moves import urllib
 
 import treq
@@ -542,10 +542,15 @@ class SimpleHttpClient(object):
             length = yield make_deferred_yieldable(
                 _readBodyToFile(response, output_stream, max_size)
             )
+        except SynapseError:
+            # This can happen e.g. because the body is too large.
+            raise
         except Exception as e:
-            logger.exception("Failed to download body")
-            raise SynapseError(
-                502, ("Failed to download remote body: %s" % e), Codes.UNKNOWN
+            raise_from(
+                SynapseError(
+                    502, ("Failed to download remote body: %s" % e),
+                ),
+                e
             )
 
         defer.returnValue(
diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py
index 8569677355..a4929dd5db 100644
--- a/synapse/rest/media/v1/media_repository.py
+++ b/synapse/rest/media/v1/media_repository.py
@@ -386,8 +386,10 @@ class MediaRepository(object):
                 raise SynapseError(502, "Failed to fetch remote media")
 
             except SynapseError:
-                logger.exception("Failed to fetch remote media %s/%s",
-                                 server_name, media_id)
+                logger.warn(
+                    "Failed to fetch remote media %s/%s",
+                    server_name, media_id,
+                )
                 raise
             except NotRetryingDestination:
                 logger.warn("Not retrying destination %r", server_name)
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
index ae891aa332..941c07fce5 100644
--- a/synapse/storage/_base.py
+++ b/synapse/storage/_base.py
@@ -299,12 +299,12 @@ class SQLBaseStore(object):
 
         def select_users_with_no_expiration_date_txn(txn):
             """Retrieves the list of registered users with no expiration date from the
-            database.
+            database, filtering out deactivated users.
             """
             sql = (
                 "SELECT users.name FROM users"
                 " LEFT JOIN account_validity ON (users.name = account_validity.user_id)"
-                " WHERE account_validity.user_id is NULL;"
+                " WHERE account_validity.user_id is NULL AND users.deactivated = 0;"
             )
             txn.execute(sql, [])
 
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index 4c5751b57f..d36917e4d6 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -252,6 +252,20 @@ class RegistrationWorkerStore(SQLBaseStore):
         )
 
     @defer.inlineCallbacks
+    def delete_account_validity_for_user(self, user_id):
+        """Deletes the entry for the given user in the account validity table, removing
+        their expiration date and renewal token.
+
+        Args:
+            user_id (str): ID of the user to remove from the account validity table.
+        """
+        yield self._simple_delete_one(
+            table="account_validity",
+            keyvalues={"user_id": user_id},
+            desc="delete_account_validity_for_user",
+        )
+
+    @defer.inlineCallbacks
     def is_server_admin(self, user):
         res = yield self._simple_select_one_onecol(
             table="users",
@@ -628,7 +642,9 @@ class RegistrationStore(
                 FROM users
                     LEFT JOIN access_tokens ON (access_tokens.user_id = users.name)
                     LEFT JOIN user_threepids ON (user_threepids.user_id = users.name)
-                WHERE password_hash IS NULL OR password_hash = ''
+                WHERE (users.password_hash IS NULL OR users.password_hash = '')
+                AND (users.appservice_id IS NULL OR users.appservice_id = '')
+                AND users.is_guest = 0
                 AND users.name > ?
                 GROUP BY users.name
                 ORDER BY users.name ASC
@@ -652,7 +668,7 @@ class RegistrationStore(
             logger.info("Marked %d rows as deactivated", rows_processed_nb)
 
             self._background_update_progress_txn(
-                txn, "users_set_deactivated_flag", {"user_id": rows[-1]["user_id"]}
+                txn, "users_set_deactivated_flag", {"user_id": rows[-1]["name"]}
             )
 
             if batch_size > len(rows):
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index 8536e6777a..b35b215446 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -26,7 +26,7 @@ from synapse.api.constants import LoginType
 from synapse.api.errors import Codes
 from synapse.appservice import ApplicationService
 from synapse.rest.client.v1 import login
-from synapse.rest.client.v2_alpha import account_validity, register, sync
+from synapse.rest.client.v2_alpha import account, account_validity, register, sync
 
 from tests import unittest
 
@@ -308,6 +308,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
         login.register_servlets,
         sync.register_servlets,
         account_validity.register_servlets,
+        account.register_servlets,
     ]
 
     def make_homeserver(self, reactor, clock):
@@ -358,20 +359,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
     def test_renewal_email(self):
         self.email_attempts = []
 
-        user_id = self.register_user("kermit", "monkey")
-        tok = self.login("kermit", "monkey")
-        # We need to manually add an email address otherwise the handler will do
-        # nothing.
-        now = self.hs.clock.time_msec()
-        self.get_success(
-            self.store.user_add_threepid(
-                user_id=user_id,
-                medium="email",
-                address="kermit@example.com",
-                validated_at=now,
-                added_at=now,
-            )
-        )
+        (user_id, tok) = self.create_user()
 
         # Move 6 days forward. This should trigger a renewal email to be sent.
         self.reactor.advance(datetime.timedelta(days=6).total_seconds())
@@ -396,6 +384,44 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
     def test_manual_email_send(self):
         self.email_attempts = []
 
+        (user_id, tok) = self.create_user()
+        request, channel = self.make_request(
+            b"POST",
+            "/_matrix/client/unstable/account_validity/send_mail",
+            access_token=tok,
+        )
+        self.render(request)
+        self.assertEquals(channel.result["code"], b"200", channel.result)
+
+        self.assertEqual(len(self.email_attempts), 1)
+
+    def test_deactivated_user(self):
+        self.email_attempts = []
+
+        (user_id, tok) = self.create_user()
+
+        request_data = json.dumps({
+            "auth": {
+                "type": "m.login.password",
+                "user": user_id,
+                "password": "monkey",
+            },
+            "erase": False,
+        })
+        request, channel = self.make_request(
+            "POST",
+            "account/deactivate",
+            request_data,
+            access_token=tok,
+        )
+        self.render(request)
+        self.assertEqual(request.code, 200)
+
+        self.reactor.advance(datetime.timedelta(days=8).total_seconds())
+
+        self.assertEqual(len(self.email_attempts), 0)
+
+    def create_user(self):
         user_id = self.register_user("kermit", "monkey")
         tok = self.login("kermit", "monkey")
         # We need to manually add an email address otherwise the handler will do
@@ -410,16 +436,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
                 added_at=now,
             )
         )
-
-        request, channel = self.make_request(
-            b"POST",
-            "/_matrix/client/unstable/account_validity/send_mail",
-            access_token=tok,
-        )
-        self.render(request)
-        self.assertEquals(channel.result["code"], b"200", channel.result)
-
-        self.assertEqual(len(self.email_attempts), 1)
+        return (user_id, tok)
 
     def test_manual_email_send_expired_account(self):
         user_id = self.register_user("kermit", "monkey")