Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Block ability to read via sync if mau limit exceeded #3670

Merged
merged 10 commits into from
Aug 14, 2018
1 change: 1 addition & 0 deletions changelog.d/3670.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Where server is disabled, block ability for locked out users to read new messages
11 changes: 10 additions & 1 deletion synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,15 +775,24 @@ def check_in_room_or_world_readable(self, room_id, user_id):
)

@defer.inlineCallbacks
def check_auth_blocking(self):
def check_auth_blocking(self, user_id=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should call this check_server_limits or something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree, since it will be a change in a lot of places I'll submit a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err given I have a few outstanding, I'll wait until a few more of them land

"""Checks if the user should be rejected for some external reason,
such as monthly active user limiting or global disable flag

Args:
user_id(str): If present, checks for presence against existing MAU cohort
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be clearer to say something like:

user_id(str|None): The user ID of the requester, or None if registering.

So that the docstring doesn't require readers to understand what the MAU cohort is to know roughly whats going on

"""
if self.hs.config.hs_disabled:
raise AuthError(
403, self.hs.config.hs_disabled_message, errcode=Codes.HS_DISABLED
)
if self.hs.config.limit_usage_by_mau is True:
# If the user is already part of the MAU cohort
if user_id:
timestamp = yield self.store.user_last_seen_monthly_active(user_id)
if timestamp:
return
# Else if there is no room in the MAU bucket, bail
current_mau = yield self.store.get_monthly_active_count()
if current_mau >= self.hs.config.max_mau_value:
raise AuthError(
Expand Down
13 changes: 11 additions & 2 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,26 +191,35 @@ def __init__(self, hs):
self.clock = hs.get_clock()
self.response_cache = ResponseCache(hs, "sync")
self.state = hs.get_state_handler()
self.auth = hs.get_auth()

# ExpiringCache((User, Device)) -> LruCache(state_key => event_id)
self.lazy_loaded_members_cache = ExpiringCache(
"lazy_loaded_members_cache", self.clock,
max_len=0, expiry_ms=LAZY_LOADED_MEMBERS_CACHE_MAX_AGE,
)

@defer.inlineCallbacks
def wait_for_sync_for_user(self, sync_config, since_token=None, timeout=0,
full_state=False):
"""Get the sync for a client if we have new data for it now. Otherwise
wait for new data to arrive on the server. If the timeout expires, then
return an empty sync result.
Returns:
A Deferred SyncResult.
Deferred[SyncResult]
"""
return self.response_cache.wrap(
# If the user is not part of the mau group, then check that limits have
# not been exceeded (if not part of the group by this point, almost certain
# auth_blocking will occur)
user_id = sync_config.user.to_string()
yield self.auth.check_auth_blocking(user_id)

res = yield self.response_cache.wrap(
sync_config.request_key,
self._wait_for_sync_for_user,
sync_config, since_token, timeout, full_state,
)
defer.returnValue(res)

@defer.inlineCallbacks
def _wait_for_sync_for_user(self, sync_config, since_token, timeout,
Expand Down
10 changes: 5 additions & 5 deletions synapse/storage/monthly_active_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def _reap_users(txn):
# is racy.
# Have resolved to invalidate the whole cache for now and do
# something about it if and when the perf becomes significant
self._user_last_seen_monthly_active.invalidate_all()
self.user_last_seen_monthly_active.invalidate_all()
self.get_monthly_active_count.invalidate_all()

@cached(num_args=0)
Expand Down Expand Up @@ -152,11 +152,11 @@ def upsert_monthly_active_user(self, user_id):
lock=False,
)
if is_insert:
self._user_last_seen_monthly_active.invalidate((user_id,))
self.user_last_seen_monthly_active.invalidate((user_id,))
self.get_monthly_active_count.invalidate(())

@cached(num_args=1)
def _user_last_seen_monthly_active(self, user_id):
def user_last_seen_monthly_active(self, user_id):
"""
Checks if a given user is part of the monthly active user group
Arguments:
Expand All @@ -173,7 +173,7 @@ def _user_last_seen_monthly_active(self, user_id):
},
retcol="timestamp",
allow_none=True,
desc="_user_last_seen_monthly_active",
desc="user_last_seen_monthly_active",
))

@defer.inlineCallbacks
Expand All @@ -185,7 +185,7 @@ def populate_monthly_active_users(self, user_id):
user_id(str): the user_id to query
"""
if self.hs.config.limit_usage_by_mau:
last_seen_timestamp = yield self._user_last_seen_monthly_active(user_id)
last_seen_timestamp = yield self.user_last_seen_monthly_active(user_id)
now = self.hs.get_clock().time_msec()

# We want to reduce to the total number of db writes, and are happy
Expand Down
71 changes: 71 additions & 0 deletions tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# -*- coding: utf-8 -*-
# Copyright 2018 New Vector Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from twisted.internet import defer

from synapse.api.errors import AuthError, Codes
from synapse.api.filtering import DEFAULT_FILTER_COLLECTION
from synapse.handlers.sync import SyncConfig, SyncHandler
from synapse.types import UserID

import tests.unittest
import tests.utils
from tests.utils import setup_test_homeserver


class SyncTestCase(tests.unittest.TestCase):
""" Tests Sync Handler. """

@defer.inlineCallbacks
def setUp(self):
self.hs = yield setup_test_homeserver()
self.sync_handler = SyncHandler(self.hs)
self.store = self.hs.get_datastore()

@defer.inlineCallbacks
def test_wait_for_sync_for_user_auth_blocking(self):

user_id1 = "@user1:server"
user_id2 = "@user2:server"
sync_config = self._generate_sync_config(user_id1)

self.hs.config.limit_usage_by_mau = True
self.hs.config.max_mau_value = 1

# Check that the happy case does not throw errors
yield self.store.upsert_monthly_active_user(user_id1)
yield self.sync_handler.wait_for_sync_for_user(sync_config)

# Test that global lock works
self.hs.config.hs_disabled = True
with self.assertRaises(AuthError) as e:
yield self.sync_handler.wait_for_sync_for_user(sync_config)
self.assertEquals(e.exception.errcode, Codes.HS_DISABLED)

self.hs.config.hs_disabled = False

sync_config = self._generate_sync_config(user_id2)

with self.assertRaises(AuthError) as e:
yield self.sync_handler.wait_for_sync_for_user(sync_config)
self.assertEquals(e.exception.errcode, Codes.MAU_LIMIT_EXCEEDED)

def _generate_sync_config(self, user_id):
return SyncConfig(
user=UserID(user_id.split(":")[0][1:], user_id.split(":")[1]),
filter_collection=DEFAULT_FILTER_COLLECTION,
is_guest=False,
request_key="request_key",
device_id="device_id",
)
12 changes: 6 additions & 6 deletions tests/storage/test_client_ips.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def test_disabled_monthly_active_user(self):
yield self.store.insert_client_ip(
user_id, "access_token", "ip", "user_agent", "device_id",
)
active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertFalse(active)

@defer.inlineCallbacks
Expand All @@ -80,21 +80,21 @@ def test_adding_monthly_active_user_when_full(self):
yield self.store.insert_client_ip(
user_id, "access_token", "ip", "user_agent", "device_id",
)
active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertFalse(active)

@defer.inlineCallbacks
def test_adding_monthly_active_user_when_space(self):
self.hs.config.limit_usage_by_mau = True
self.hs.config.max_mau_value = 50
user_id = "@user:server"
active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertFalse(active)

yield self.store.insert_client_ip(
user_id, "access_token", "ip", "user_agent", "device_id",
)
active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertTrue(active)

@defer.inlineCallbacks
Expand All @@ -103,7 +103,7 @@ def test_updating_monthly_active_user_when_space(self):
self.hs.config.max_mau_value = 50
user_id = "@user:server"

active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertFalse(active)

yield self.store.insert_client_ip(
Expand All @@ -112,5 +112,5 @@ def test_updating_monthly_active_user_when_space(self):
yield self.store.insert_client_ip(
user_id, "access_token", "ip", "user_agent", "device_id",
)
active = yield self.store._user_last_seen_monthly_active(user_id)
active = yield self.store.user_last_seen_monthly_active(user_id)
self.assertTrue(active)
13 changes: 7 additions & 6 deletions tests/storage/test_monthly_active_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ def test_initialise_reserved_users(self):

# Test user is marked as active

timestamp = yield self.store._user_last_seen_monthly_active(user1)
timestamp = yield self.store.user_last_seen_monthly_active(user1)
self.assertTrue(timestamp)
timestamp = yield self.store._user_last_seen_monthly_active(user2)
timestamp = yield self.store.user_last_seen_monthly_active(user2)
self.assertTrue(timestamp)

# Test that users are never removed from the db.
Expand All @@ -92,17 +92,18 @@ def test_can_insert_and_count_mau(self):
self.assertEqual(1, count)

@defer.inlineCallbacks
def test__user_last_seen_monthly_active(self):
def test_user_last_seen_monthly_active(self):
user_id1 = "@user1:server"
user_id2 = "@user2:server"
user_id3 = "@user3:server"
result = yield self.store._user_last_seen_monthly_active(user_id1)

result = yield self.store.user_last_seen_monthly_active(user_id1)
self.assertFalse(result == 0)
yield self.store.upsert_monthly_active_user(user_id1)
yield self.store.upsert_monthly_active_user(user_id2)
result = yield self.store._user_last_seen_monthly_active(user_id1)
result = yield self.store.user_last_seen_monthly_active(user_id1)
self.assertTrue(result > 0)
result = yield self.store._user_last_seen_monthly_active(user_id3)
result = yield self.store.user_last_seen_monthly_active(user_id3)
self.assertFalse(result == 0)

@defer.inlineCallbacks
Expand Down