From 251e6c1210087069a6133140519de80a4ddf218a Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 30 Jul 2018 15:55:57 +0100 Subject: limit register and sign in on number of monthly users --- tests/handlers/test_auth.py | 49 ++++++++++++++++++++++++++++++++++++++++- tests/handlers/test_register.py | 49 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) (limited to 'tests/handlers') diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py index 2e5e8e4dec..57f78a6bec 100644 --- a/tests/handlers/test_auth.py +++ b/tests/handlers/test_auth.py @@ -12,15 +12,17 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +from mock import Mock import pymacaroons from twisted.internet import defer import synapse +from synapse.api.errors import AuthError import synapse.api.errors from synapse.handlers.auth import AuthHandler + from tests import unittest from tests.utils import setup_test_homeserver @@ -37,6 +39,10 @@ class AuthTestCase(unittest.TestCase): self.hs.handlers = AuthHandlers(self.hs) self.auth_handler = self.hs.handlers.auth_handler self.macaroon_generator = self.hs.get_macaroon_generator() + # MAU tests + self.hs.config.max_mau_value = 50 + self.small_number_of_users = 1 + self.large_number_of_users = 100 def test_token_is_a_macaroon(self): token = self.macaroon_generator.generate_access_token("some_user") @@ -113,3 +119,44 @@ class AuthTestCase(unittest.TestCase): self.auth_handler.validate_short_term_login_token_and_get_user_id( macaroon.serialize() ) + + @defer.inlineCallbacks + 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') + + self.auth_handler.validate_short_term_login_token_and_get_user_id( + self._get_macaroon().serialize() + ) + + @defer.inlineCallbacks + def test_mau_limits_exceeded(self): + self.hs.config.limit_usage_by_mau = True + self.hs.get_datastore().count_monthly_users = Mock( + return_value=self.large_number_of_users + ) + with self.assertRaises(AuthError): + yield self.auth_handler.get_access_token_for_user_id('user_a') + with self.assertRaises(AuthError): + self.auth_handler.validate_short_term_login_token_and_get_user_id( + self._get_macaroon().serialize() + ) + + @defer.inlineCallbacks + def test_mau_limits_not_exceeded(self): + self.hs.config.limit_usage_by_mau = True + self.hs.get_datastore().count_monthly_users = Mock( + return_value=self.small_number_of_users + ) + # Ensure does not raise exception + yield self.auth_handler.get_access_token_for_user_id('user_a') + self.auth_handler.validate_short_term_login_token_and_get_user_id( + self._get_macaroon().serialize() + ) + + def _get_macaroon(self): + token = self.macaroon_generator.generate_short_term_login_token( + "user_a", 5000 + ) + return pymacaroons.Macaroon.deserialize(token) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 025fa1be81..a5a8e7c954 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -17,6 +17,7 @@ from mock import Mock from twisted.internet import defer +from synapse.api.errors import RegistrationError from synapse.handlers.register import RegistrationHandler from synapse.types import UserID, create_requester @@ -77,3 +78,51 @@ class RegistrationTestCase(unittest.TestCase): requester, local_part, display_name) self.assertEquals(result_user_id, user_id) self.assertEquals(result_token, 'secret') + + @defer.inlineCallbacks + def test_cannot_register_when_mau_limits_exceeded(self): + local_part = "someone" + display_name = "someone" + requester = create_requester("@as:test") + store = self.hs.get_datastore() + self.hs.config.limit_usage_by_mau = False + self.hs.config.max_mau_value = 50 + lots_of_users = 100 + small_number_users = 1 + + store.count_monthly_users = Mock(return_value=lots_of_users) + + # Ensure does not throw exception + yield self.handler.get_or_create_user(requester, 'a', display_name) + + self.hs.config.limit_usage_by_mau = True + + with self.assertRaises(RegistrationError): + yield self.handler.get_or_create_user(requester, 'b', display_name) + + store.count_monthly_users = Mock(return_value=small_number_users) + + self._macaroon_mock_generator("another_secret") + + # Ensure does not throw exception + yield self.handler.get_or_create_user("@neil:matrix.org", 'c', "Neil") + + self._macaroon_mock_generator("another another secret") + store.count_monthly_users = Mock(return_value=lots_of_users) + with self.assertRaises(RegistrationError): + yield self.handler.register(localpart=local_part) + + self._macaroon_mock_generator("another another secret") + store.count_monthly_users = Mock(return_value=lots_of_users) + with self.assertRaises(RegistrationError): + yield self.handler.register_saml2(local_part) + + def _macaroon_mock_generator(self, secret): + """ + Reset macaroon generator in the case where the test creates multiple users + """ + macaroon_generator = Mock( + generate_access_token=Mock(return_value=secret)) + self.hs.get_macaroon_generator = Mock(return_value=macaroon_generator) + self.hs.handlers = RegistrationHandlers(self.hs) + self.handler = self.hs.get_handlers().registration_handler -- cgit 1.5.1 From e908b8683248328dd92479fc81be350336b9c8f4 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 30 Jul 2018 16:24:02 -0600 Subject: Remove pdu_failures from transactions The field is never read from, and all the opportunities given to populate it are not utilized. It should be very safe to remove this. --- changelog.d/3628.misc | 1 + synapse/federation/federation_server.py | 4 --- synapse/federation/send_queue.py | 63 +-------------------------------- synapse/federation/transaction_queue.py | 32 +++-------------- synapse/federation/transport/server.py | 3 +- synapse/federation/units.py | 1 - tests/handlers/test_typing.py | 1 - 7 files changed, 8 insertions(+), 97 deletions(-) create mode 100644 changelog.d/3628.misc (limited to 'tests/handlers') diff --git a/changelog.d/3628.misc b/changelog.d/3628.misc new file mode 100644 index 0000000000..1aebefbe18 --- /dev/null +++ b/changelog.d/3628.misc @@ -0,0 +1 @@ +Remove unused field "pdu_failures" from transactions. diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index e501251b6e..657935d1ac 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -207,10 +207,6 @@ class FederationServer(FederationBase): edu.content ) - pdu_failures = getattr(transaction, "pdu_failures", []) - for fail in pdu_failures: - logger.info("Got failure %r", fail) - response = { "pdus": pdu_results, } diff --git a/synapse/federation/send_queue.py b/synapse/federation/send_queue.py index 5157c3860d..0bb468385d 100644 --- a/synapse/federation/send_queue.py +++ b/synapse/federation/send_queue.py @@ -62,8 +62,6 @@ class FederationRemoteSendQueue(object): self.edus = SortedDict() # stream position -> Edu - self.failures = SortedDict() # stream position -> (destination, Failure) - self.device_messages = SortedDict() # stream position -> destination self.pos = 1 @@ -79,7 +77,7 @@ class FederationRemoteSendQueue(object): for queue_name in [ "presence_map", "presence_changed", "keyed_edu", "keyed_edu_changed", - "edus", "failures", "device_messages", "pos_time", + "edus", "device_messages", "pos_time", ]: register(queue_name, getattr(self, queue_name)) @@ -149,12 +147,6 @@ class FederationRemoteSendQueue(object): for key in keys[:i]: del self.edus[key] - # Delete things out of failure map - keys = self.failures.keys() - i = self.failures.bisect_left(position_to_delete) - for key in keys[:i]: - del self.failures[key] - # Delete things out of device map keys = self.device_messages.keys() i = self.device_messages.bisect_left(position_to_delete) @@ -204,13 +196,6 @@ class FederationRemoteSendQueue(object): self.notifier.on_new_replication_data() - def send_failure(self, failure, destination): - """As per TransactionQueue""" - pos = self._next_pos() - - self.failures[pos] = (destination, str(failure)) - self.notifier.on_new_replication_data() - def send_device_messages(self, destination): """As per TransactionQueue""" pos = self._next_pos() @@ -285,17 +270,6 @@ class FederationRemoteSendQueue(object): for (pos, edu) in edus: rows.append((pos, EduRow(edu))) - # Fetch changed failures - i = self.failures.bisect_right(from_token) - j = self.failures.bisect_right(to_token) + 1 - failures = self.failures.items()[i:j] - - for (pos, (destination, failure)) in failures: - rows.append((pos, FailureRow( - destination=destination, - failure=failure, - ))) - # Fetch changed device messages i = self.device_messages.bisect_right(from_token) j = self.device_messages.bisect_right(to_token) + 1 @@ -417,34 +391,6 @@ class EduRow(BaseFederationRow, namedtuple("EduRow", ( buff.edus.setdefault(self.edu.destination, []).append(self.edu) -class FailureRow(BaseFederationRow, namedtuple("FailureRow", ( - "destination", # str - "failure", -))): - """Streams failures to a remote server. Failures are issued when there was - something wrong with a transaction the remote sent us, e.g. it included - an event that was invalid. - """ - - TypeId = "f" - - @staticmethod - def from_data(data): - return FailureRow( - destination=data["destination"], - failure=data["failure"], - ) - - def to_data(self): - return { - "destination": self.destination, - "failure": self.failure, - } - - def add_to_buffer(self, buff): - buff.failures.setdefault(self.destination, []).append(self.failure) - - class DeviceRow(BaseFederationRow, namedtuple("DeviceRow", ( "destination", # str ))): @@ -471,7 +417,6 @@ TypeToRow = { PresenceRow, KeyedEduRow, EduRow, - FailureRow, DeviceRow, ) } @@ -481,7 +426,6 @@ ParsedFederationStreamData = namedtuple("ParsedFederationStreamData", ( "presence", # list(UserPresenceState) "keyed_edus", # dict of destination -> { key -> Edu } "edus", # dict of destination -> [Edu] - "failures", # dict of destination -> [failures] "device_destinations", # set of destinations )) @@ -503,7 +447,6 @@ def process_rows_for_federation(transaction_queue, rows): presence=[], keyed_edus={}, edus={}, - failures={}, device_destinations=set(), ) @@ -532,9 +475,5 @@ def process_rows_for_federation(transaction_queue, rows): edu.destination, edu.edu_type, edu.content, key=None, ) - for destination, failure_list in iteritems(buff.failures): - for failure in failure_list: - transaction_queue.send_failure(destination, failure) - for destination in buff.device_destinations: transaction_queue.send_device_messages(destination) diff --git a/synapse/federation/transaction_queue.py b/synapse/federation/transaction_queue.py index 6996d6b695..78f9d40a3a 100644 --- a/synapse/federation/transaction_queue.py +++ b/synapse/federation/transaction_queue.py @@ -116,9 +116,6 @@ class TransactionQueue(object): ), ) - # destination -> list of tuple(failure, deferred) - self.pending_failures_by_dest = {} - # destination -> stream_id of last successfully sent to-device message. # NB: may be a long or an int. self.last_device_stream_id_by_dest = {} @@ -382,19 +379,6 @@ class TransactionQueue(object): self._attempt_new_transaction(destination) - def send_failure(self, failure, destination): - if destination == self.server_name or destination == "localhost": - return - - if not self.can_send_to(destination): - return - - self.pending_failures_by_dest.setdefault( - destination, [] - ).append(failure) - - self._attempt_new_transaction(destination) - def send_device_messages(self, destination): if destination == self.server_name or destination == "localhost": return @@ -469,7 +453,6 @@ class TransactionQueue(object): pending_pdus = self.pending_pdus_by_dest.pop(destination, []) pending_edus = self.pending_edus_by_dest.pop(destination, []) pending_presence = self.pending_presence_by_dest.pop(destination, {}) - pending_failures = self.pending_failures_by_dest.pop(destination, []) pending_edus.extend( self.pending_edus_keyed_by_dest.pop(destination, {}).values() @@ -497,7 +480,7 @@ class TransactionQueue(object): logger.debug("TX [%s] len(pending_pdus_by_dest[dest]) = %d", destination, len(pending_pdus)) - if not pending_pdus and not pending_edus and not pending_failures: + if not pending_pdus and not pending_edus: logger.debug("TX [%s] Nothing to send", destination) self.last_device_stream_id_by_dest[destination] = ( device_stream_id @@ -507,7 +490,7 @@ class TransactionQueue(object): # END CRITICAL SECTION success = yield self._send_new_transaction( - destination, pending_pdus, pending_edus, pending_failures, + destination, pending_pdus, pending_edus, ) if success: sent_transactions_counter.inc() @@ -584,14 +567,12 @@ class TransactionQueue(object): @measure_func("_send_new_transaction") @defer.inlineCallbacks - def _send_new_transaction(self, destination, pending_pdus, pending_edus, - pending_failures): + def _send_new_transaction(self, destination, pending_pdus, pending_edus): # Sort based on the order field pending_pdus.sort(key=lambda t: t[1]) pdus = [x[0] for x in pending_pdus] edus = pending_edus - failures = [x.get_dict() for x in pending_failures] success = True @@ -601,11 +582,10 @@ class TransactionQueue(object): logger.debug( "TX [%s] {%s} Attempting new transaction" - " (pdus: %d, edus: %d, failures: %d)", + " (pdus: %d, edus: %d)", destination, txn_id, len(pdus), len(edus), - len(failures) ) logger.debug("TX [%s] Persisting transaction...", destination) @@ -617,7 +597,6 @@ class TransactionQueue(object): destination=destination, pdus=pdus, edus=edus, - pdu_failures=failures, ) self._next_txn_id += 1 @@ -627,12 +606,11 @@ class TransactionQueue(object): logger.debug("TX [%s] Persisted transaction", destination) logger.info( "TX [%s] {%s} Sending transaction [%s]," - " (PDUs: %d, EDUs: %d, failures: %d)", + " (PDUs: %d, EDUs: %d)", destination, txn_id, transaction.transaction_id, len(pdus), len(edus), - len(failures), ) # Actually send the transaction diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 8574898f0c..3b5ea9515a 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -283,11 +283,10 @@ class FederationSendServlet(BaseFederationServlet): ) logger.info( - "Received txn %s from %s. (PDUs: %d, EDUs: %d, failures: %d)", + "Received txn %s from %s. (PDUs: %d, EDUs: %d)", transaction_id, origin, len(transaction_data.get("pdus", [])), len(transaction_data.get("edus", [])), - len(transaction_data.get("failures", [])), ) # We should ideally be getting this from the security layer. diff --git a/synapse/federation/units.py b/synapse/federation/units.py index bb1b3b13f7..c5ab14314e 100644 --- a/synapse/federation/units.py +++ b/synapse/federation/units.py @@ -73,7 +73,6 @@ class Transaction(JsonEncodedObject): "previous_ids", "pdus", "edus", - "pdu_failures", ] internal_keys = [ diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index b08856f763..2c263af1a3 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -44,7 +44,6 @@ def _expect_edu(destination, edu_type, content, origin="test"): "content": content, } ], - "pdu_failures": [], } -- cgit 1.5.1 From df2235e7fab44a5155134a336a4c27424398c1be Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Tue, 31 Jul 2018 13:16:20 +0100 Subject: coding style --- synapse/app/homeserver.py | 6 +++++- synapse/config/server.py | 2 +- synapse/handlers/auth.py | 3 ++- synapse/storage/__init__.py | 3 +-- synapse/storage/schema/delta/50/make_event_content_nullable.py | 2 +- tests/handlers/test_auth.py | 4 ++-- tests/storage/test__init__.py | 1 - 7 files changed, 12 insertions(+), 9 deletions(-) (limited to 'tests/handlers') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 96c45b7209..82979e7d1b 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -18,9 +18,10 @@ import logging import os import sys -from prometheus_client import Gauge from six import iteritems +from prometheus_client import Gauge + from twisted.application import service from twisted.internet import defer, reactor from twisted.web.resource import EncodingResourceWrapper, NoResource @@ -300,12 +301,15 @@ class SynapseHomeServer(HomeServer): except IncorrectDatabaseSetup as e: quit_with_error(e.message) + # Gauges to expose monthly active user control metrics current_mau_gauge = Gauge("synapse_admin_current_mau", "Current MAU") max_mau_value_gauge = Gauge("synapse_admin_max_mau_value", "MAU Limit") limit_usage_by_mau_gauge = Gauge( "synapse_admin_limit_usage_by_mau", "MAU Limiting enabled" ) + + def setup(config_options): """ Args: diff --git a/synapse/config/server.py b/synapse/config/server.py index 8b335bff3f..9af42a93ad 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -70,7 +70,7 @@ class ServerConfig(Config): # Options to control access by tracking MAU self.limit_usage_by_mau = config.get("limit_usage_by_mau", False) self.max_mau_value = config.get( - "max_mau_value", 0, + "max_mau_value", 0, ) # FIXME: federation_domain_whitelist needs sytests self.federation_domain_whitelist = None diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index f3734f11bd..28f1c1afbb 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -903,9 +903,10 @@ class AuthHandler(BaseHandler): current_mau = self.store.count_monthly_users() if current_mau >= self.hs.config.max_mau_value: raise AuthError( - 403, "MAU Limit Exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED + 403, "MAU Limit Exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED ) + @attr.s class MacaroonGenerator(object): diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 044e988e92..4747118ed7 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -60,6 +60,7 @@ from .util.id_generators import ChainedIdGenerator, IdGenerator, StreamIdGenerat logger = logging.getLogger(__name__) + class DataStore(RoomMemberStore, RoomStore, RegistrationStore, StreamStore, ProfileStore, PresenceStore, TransactionStore, @@ -291,8 +292,6 @@ class DataStore(RoomMemberStore, RoomStore, finally: txn.close() - - def count_r30_users(self): """ Counts the number of 30 day retained users, defined as:- diff --git a/synapse/storage/schema/delta/50/make_event_content_nullable.py b/synapse/storage/schema/delta/50/make_event_content_nullable.py index 7d27342e39..6dd467b6c5 100644 --- a/synapse/storage/schema/delta/50/make_event_content_nullable.py +++ b/synapse/storage/schema/delta/50/make_event_content_nullable.py @@ -88,5 +88,5 @@ def run_upgrade(cur, database_engine, *args, **kwargs): "UPDATE sqlite_master SET sql=? WHERE tbl_name='events' AND type='table'", (sql, ), ) - cur.execute("PRAGMA schema_version=%i" % (oldver+1,)) + cur.execute("PRAGMA schema_version=%i" % (oldver + 1,)) cur.execute("PRAGMA writable_schema=OFF") diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py index 57f78a6bec..e01f14a10a 100644 --- a/tests/handlers/test_auth.py +++ b/tests/handlers/test_auth.py @@ -13,16 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. from mock import Mock + import pymacaroons from twisted.internet import defer import synapse -from synapse.api.errors import AuthError import synapse.api.errors +from synapse.api.errors import AuthError from synapse.handlers.auth import AuthHandler - from tests import unittest from tests.utils import setup_test_homeserver diff --git a/tests/storage/test__init__.py b/tests/storage/test__init__.py index c9ae349871..fe6eeeaf10 100644 --- a/tests/storage/test__init__.py +++ b/tests/storage/test__init__.py @@ -12,7 +12,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import sys from twisted.internet import defer -- cgit 1.5.1 From 7931393495c76eef0af9b91c7904c88943197054 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 1 Aug 2018 10:21:56 +0100 Subject: make count_monthly_users async synapse/handlers/auth.py --- synapse/handlers/register.py | 9 +++++---- synapse/storage/__init__.py | 26 +++++++++++++------------- tests/handlers/test_auth.py | 39 ++++++++++++++++++++++----------------- tests/handlers/test_register.py | 10 ++++++---- 4 files changed, 46 insertions(+), 38 deletions(-) (limited to 'tests/handlers') diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index f46b8355c0..cc935a5e84 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -144,7 +144,7 @@ class RegistrationHandler(BaseHandler): Raises: RegistrationError if there was a problem registering. """ - self._check_mau_limits() + yield self._check_mau_limits() password_hash = None if password: password_hash = yield self.auth_handler().hash(password) @@ -289,7 +289,7 @@ class RegistrationHandler(BaseHandler): 400, "User ID can only contain characters a-z, 0-9, or '=_-./'", ) - self._check_mau_limits() + yield self._check_mau_limits() user = UserID(localpart, self.hs.hostname) user_id = user.to_string() @@ -439,7 +439,7 @@ class RegistrationHandler(BaseHandler): """ if localpart is None: raise SynapseError(400, "Request must include user id") - self._check_mau_limits() + yield self._check_mau_limits() need_register = True try: @@ -534,13 +534,14 @@ class RegistrationHandler(BaseHandler): action="join", ) + @defer.inlineCallbacks def _check_mau_limits(self): """ Do not accept registrations if monthly active user limits exceeded and limiting is enabled """ if self.hs.config.limit_usage_by_mau is True: - current_mau = self.store.count_monthly_users() + current_mau = yield self.store.count_monthly_users() if current_mau >= self.hs.config.max_mau_value: raise RegistrationError( 403, "MAU Limit Exceeded", Codes.MAU_LIMIT_EXCEEDED diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 4747118ed7..f9682832ca 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -273,24 +273,24 @@ class DataStore(RoomMemberStore, RoomStore, This method should be refactored with count_daily_users - the only reason not to is waiting on definition of mau returns: - int: count of current monthly active users + defered: resolves to int """ + def _count_monthly_users(txn): + thirty_days_ago = int(self._clock.time_msec()) - (1000 * 60 * 60 * 24 * 30) + sql = """ + SELECT COALESCE(count(*), 0) FROM ( + SELECT user_id FROM user_ips + WHERE last_seen > ? + GROUP BY user_id + ) u + """ - thirty_days_ago = int(self._clock.time_msec()) - (1000 * 60 * 60 * 24 * 30) - sql = """ - SELECT COALESCE(count(*), 0) FROM ( - SELECT user_id FROM user_ips - WHERE last_seen > ? - GROUP BY user_id - ) u - """ - try: - txn = self.db_conn.cursor() txn.execute(sql, (thirty_days_ago,)) count, = txn.fetchone() + print "Count is %d" % (count,) return count - finally: - txn.close() + + return self.runInteraction("count_monthly_users", _count_monthly_users) def count_r30_users(self): """ diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py index e01f14a10a..440a453082 100644 --- a/tests/handlers/test_auth.py +++ b/tests/handlers/test_auth.py @@ -77,38 +77,37 @@ class AuthTestCase(unittest.TestCase): v.satisfy_general(verify_nonce) v.verify(macaroon, self.hs.config.macaroon_secret_key) + @defer.inlineCallbacks def test_short_term_login_token_gives_user_id(self): self.hs.clock.now = 1000 token = self.macaroon_generator.generate_short_term_login_token( "a_user", 5000 ) - - self.assertEqual( - "a_user", - self.auth_handler.validate_short_term_login_token_and_get_user_id( - token - ) + user_id = yield self.auth_handler.validate_short_term_login_token_and_get_user_id( + token ) + self.assertEqual("a_user", user_id) # when we advance the clock, the token should be rejected self.hs.clock.now = 6000 with self.assertRaises(synapse.api.errors.AuthError): - self.auth_handler.validate_short_term_login_token_and_get_user_id( + yield self.auth_handler.validate_short_term_login_token_and_get_user_id( token ) + @defer.inlineCallbacks def test_short_term_login_token_cannot_replace_user_id(self): token = self.macaroon_generator.generate_short_term_login_token( "a_user", 5000 ) macaroon = pymacaroons.Macaroon.deserialize(token) + user_id = yield self.auth_handler.validate_short_term_login_token_and_get_user_id( + macaroon.serialize() + ) self.assertEqual( - "a_user", - self.auth_handler.validate_short_term_login_token_and_get_user_id( - macaroon.serialize() - ) + "a_user", user_id ) # add another "user_id" caveat, which might allow us to override the @@ -116,7 +115,7 @@ class AuthTestCase(unittest.TestCase): macaroon.add_first_party_caveat("user_id = b_user") with self.assertRaises(synapse.api.errors.AuthError): - self.auth_handler.validate_short_term_login_token_and_get_user_id( + yield self.auth_handler.validate_short_term_login_token_and_get_user_id( macaroon.serialize() ) @@ -126,7 +125,7 @@ class AuthTestCase(unittest.TestCase): # Ensure does not throw exception yield self.auth_handler.get_access_token_for_user_id('user_a') - self.auth_handler.validate_short_term_login_token_and_get_user_id( + yield self.auth_handler.validate_short_term_login_token_and_get_user_id( self._get_macaroon().serialize() ) @@ -134,24 +133,30 @@ class AuthTestCase(unittest.TestCase): def test_mau_limits_exceeded(self): self.hs.config.limit_usage_by_mau = True self.hs.get_datastore().count_monthly_users = Mock( - return_value=self.large_number_of_users + return_value=defer.succeed(self.large_number_of_users) ) + with self.assertRaises(AuthError): yield self.auth_handler.get_access_token_for_user_id('user_a') + + self.hs.get_datastore().count_monthly_users = Mock( + return_value=defer.succeed(self.large_number_of_users) + ) with self.assertRaises(AuthError): - self.auth_handler.validate_short_term_login_token_and_get_user_id( + yield self.auth_handler.validate_short_term_login_token_and_get_user_id( self._get_macaroon().serialize() ) @defer.inlineCallbacks def test_mau_limits_not_exceeded(self): self.hs.config.limit_usage_by_mau = True + self.hs.get_datastore().count_monthly_users = Mock( - return_value=self.small_number_of_users + 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') - self.auth_handler.validate_short_term_login_token_and_get_user_id( + yield self.auth_handler.validate_short_term_login_token_and_get_user_id( self._get_macaroon().serialize() ) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index a5a8e7c954..0937d71cf6 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -90,7 +90,7 @@ class RegistrationTestCase(unittest.TestCase): lots_of_users = 100 small_number_users = 1 - store.count_monthly_users = Mock(return_value=lots_of_users) + store.count_monthly_users = Mock(return_value=defer.succeed(lots_of_users)) # Ensure does not throw exception yield self.handler.get_or_create_user(requester, 'a', display_name) @@ -100,7 +100,7 @@ class RegistrationTestCase(unittest.TestCase): with self.assertRaises(RegistrationError): yield self.handler.get_or_create_user(requester, 'b', display_name) - store.count_monthly_users = Mock(return_value=small_number_users) + store.count_monthly_users = Mock(return_value=defer.succeed(small_number_users)) self._macaroon_mock_generator("another_secret") @@ -108,12 +108,14 @@ class RegistrationTestCase(unittest.TestCase): yield self.handler.get_or_create_user("@neil:matrix.org", 'c', "Neil") self._macaroon_mock_generator("another another secret") - store.count_monthly_users = Mock(return_value=lots_of_users) + store.count_monthly_users = Mock(return_value=defer.succeed(lots_of_users)) + with self.assertRaises(RegistrationError): yield self.handler.register(localpart=local_part) self._macaroon_mock_generator("another another secret") - store.count_monthly_users = Mock(return_value=lots_of_users) + store.count_monthly_users = Mock(return_value=defer.succeed(lots_of_users)) + with self.assertRaises(RegistrationError): yield self.handler.register_saml2(local_part) -- cgit 1.5.1 From 6eed16d8a2335c97675b6b8661869848f397ea29 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 1 Aug 2018 14:02:10 +0100 Subject: fix test for py3 --- tests/handlers/test_auth.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'tests/handlers') diff --git a/tests/handlers/test_auth.py b/tests/handlers/test_auth.py index 440a453082..55eab9e9cf 100644 --- a/tests/handlers/test_auth.py +++ b/tests/handlers/test_auth.py @@ -156,6 +156,10 @@ class AuthTestCase(unittest.TestCase): ) # Ensure does not raise exception yield self.auth_handler.get_access_token_for_user_id('user_a') + + self.hs.get_datastore().count_monthly_users = Mock( + return_value=defer.succeed(self.small_number_of_users) + ) yield self.auth_handler.validate_short_term_login_token_and_get_user_id( self._get_macaroon().serialize() ) -- cgit 1.5.1