Merge pull request #5462 from matrix-org/babolivier/account_validity_deactivated_accounts_2

Don't send renewal emails to deactivated users (second attempt)
pull/5478/head
Brendan Abolivier 2019-06-14 15:35:31 +01:00 committed by GitHub
commit 9b14a810d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 68 additions and 27 deletions

1
changelog.d/5394.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug where deactivated users could receive renewal emails if the account validity feature is on.

View File

@ -110,6 +110,9 @@ class AccountValidityHandler(object):
# Stop right here if the user doesn't have at least one email address. # 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 # In this case, they will have to ask their server admin to renew their
# account manually. # 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: if not addresses:
return return

View File

@ -43,6 +43,8 @@ class DeactivateAccountHandler(BaseHandler):
# it left off (if it has work left to do). # it left off (if it has work left to do).
hs.get_reactor().callWhenRunning(self._start_user_parting) hs.get_reactor().callWhenRunning(self._start_user_parting)
self._account_validity_enabled = hs.config.account_validity.enabled
@defer.inlineCallbacks @defer.inlineCallbacks
def deactivate_account(self, user_id, erase_data, id_server=None): def deactivate_account(self, user_id, erase_data, id_server=None):
"""Deactivate a user's account """Deactivate a user's account
@ -115,6 +117,10 @@ class DeactivateAccountHandler(BaseHandler):
# parts users from rooms (if it isn't already running) # parts users from rooms (if it isn't already running)
self._start_user_parting() 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. # Mark the user as deactivated.
yield self.store.set_user_deactivated_status(user_id, True) yield self.store.set_user_deactivated_status(user_id, True)

View File

@ -299,12 +299,12 @@ class SQLBaseStore(object):
def select_users_with_no_expiration_date_txn(txn): def select_users_with_no_expiration_date_txn(txn):
"""Retrieves the list of registered users with no expiration date from the """Retrieves the list of registered users with no expiration date from the
database. database, filtering out deactivated users.
""" """
sql = ( sql = (
"SELECT users.name FROM users" "SELECT users.name FROM users"
" LEFT JOIN account_validity ON (users.name = account_validity.user_id)" " 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, []) txn.execute(sql, [])

View File

@ -251,6 +251,20 @@ class RegistrationWorkerStore(SQLBaseStore):
desc="set_renewal_mail_status", desc="set_renewal_mail_status",
) )
@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 @defer.inlineCallbacks
def is_server_admin(self, user): def is_server_admin(self, user):
res = yield self._simple_select_one_onecol( res = yield self._simple_select_one_onecol(

View File

@ -26,7 +26,7 @@ from synapse.api.constants import LoginType
from synapse.api.errors import Codes from synapse.api.errors import Codes
from synapse.appservice import ApplicationService from synapse.appservice import ApplicationService
from synapse.rest.client.v1 import login 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 from tests import unittest
@ -308,6 +308,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
login.register_servlets, login.register_servlets,
sync.register_servlets, sync.register_servlets,
account_validity.register_servlets, account_validity.register_servlets,
account.register_servlets,
] ]
def make_homeserver(self, reactor, clock): def make_homeserver(self, reactor, clock):
@ -358,20 +359,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
def test_renewal_email(self): def test_renewal_email(self):
self.email_attempts = [] self.email_attempts = []
user_id = self.register_user("kermit", "monkey") (user_id, tok) = self.create_user()
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,
)
)
# Move 6 days forward. This should trigger a renewal email to be sent. # Move 6 days forward. This should trigger a renewal email to be sent.
self.reactor.advance(datetime.timedelta(days=6).total_seconds()) self.reactor.advance(datetime.timedelta(days=6).total_seconds())
@ -396,6 +384,44 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
def test_manual_email_send(self): def test_manual_email_send(self):
self.email_attempts = [] 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") user_id = self.register_user("kermit", "monkey")
tok = self.login("kermit", "monkey") tok = self.login("kermit", "monkey")
# We need to manually add an email address otherwise the handler will do # We need to manually add an email address otherwise the handler will do
@ -410,16 +436,7 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
added_at=now, added_at=now,
) )
) )
return (user_id, tok)
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_manual_email_send_expired_account(self): def test_manual_email_send_expired_account(self):
user_id = self.register_user("kermit", "monkey") user_id = self.register_user("kermit", "monkey")