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

fix bug where preserved threepid user comes to sign up and server is … #3777

Merged
merged 7 commits into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/3777.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where preserved threepid user comes to sign up and server is mau blocked
18 changes: 16 additions & 2 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from synapse import event_auth
from synapse.api.constants import EventTypes, JoinRules, Membership
from synapse.api.errors import AuthError, Codes, ResourceLimitError
from synapse.config.server import is_threepid_reserved
from synapse.types import UserID
from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache
from synapse.util.caches.lrucache import LruCache
Expand Down Expand Up @@ -775,13 +776,18 @@ def check_in_room_or_world_readable(self, room_id, user_id):
)

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

Choose a reason for hiding this comment

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

Please doc threepid arg

"""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|None): If present, checks for presence against existing
MAU cohort
threepid(dict|None): If present, checks for presence against configured
reserved threepid. Used in cases where the user is trying register
with a MAU blocked server, normally they would be rejected but their
threepid is on the reserved list. user_id and
threepid should never be set at the same time.
"""

# Never fail an auth check for the server notices users
Expand All @@ -797,6 +803,10 @@ def check_auth_blocking(self, user_id=None):
limit_type=self.hs.config.hs_disabled_limit_type
)
if self.hs.config.limit_usage_by_mau is True:

if user_id and threepid:
logger.warn("Called with both user_id and threepid, this shoudn't happen")
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'd assert not (user_id and threepid) or something so that if someone does do it it gets picked up in tests.


# If the user is already part of the MAU cohort or a trial user
if user_id:
timestamp = yield self.store.user_last_seen_monthly_active(user_id)
Expand All @@ -806,12 +816,16 @@ def check_auth_blocking(self, user_id=None):
is_trial = yield self.store.is_trial_user(user_id)
if is_trial:
return
elif threepid:
# If the user does not exist yet, but is signing up with a
# reserved threepid then pass auth check
if is_threepid_reserved(self.hs.config, threepid):
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 ResourceLimitError(
403, "Monthly Active User Limit Exceeded",

admin_contact=self.hs.config.admin_contact,
errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
limit_type="monthly_active_user"
Expand Down
17 changes: 17 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,23 @@ def add_arguments(self, parser):
" service on the given port.")


def is_threepid_reserved(config, threepid):
"""Check the threepid against the reserved threepid config
Args:
config(ServerConfig) - to access server config attributes
threepid(dict) - The threepid to test for

Returns:
boolean Is the threepid undertest reserved_user
"""

for tp in config.mau_limits_reserved_threepids:
if (threepid['medium'] == tp['medium']
and threepid['address'] == tp['address']):
return True
return False


def read_gc_thresholds(thresholds):
"""Reads the three integer thresholds for garbage collection. Ensures that
the thresholds are integers if thresholds are supplied.
Expand Down
3 changes: 2 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def register(
guest_access_token=None,
make_guest=False,
admin=False,
threepid=None,
):
"""Registers a new client on the server.

Expand All @@ -145,7 +146,7 @@ def register(
RegistrationError if there was a problem registering.
"""

yield self.auth.check_auth_blocking()
yield self.auth.check_auth_blocking(threepid=threepid)
password_hash = None
if password:
password_hash = yield self.auth_handler().hash(password)
Expand Down
11 changes: 10 additions & 1 deletion synapse/rest/client/v1_only/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import synapse.util.stringutils as stringutils
from synapse.api.constants import LoginType
from synapse.api.errors import Codes, SynapseError
from synapse.config.server import is_threepid_reserved
from synapse.http.servlet import assert_params_in_dict, parse_json_object_from_request
from synapse.rest.client.v1.base import ClientV1RestServlet
from synapse.types import create_requester
Expand Down Expand Up @@ -281,12 +282,20 @@ def _do_password(self, request, register_json, session):
register_json["user"].encode("utf-8")
if "user" in register_json else None
)
threepid = None
if session.get(LoginType.EMAIL_IDENTITY):
threepid = session["threepidCreds"]

handler = self.handlers.registration_handler
(user_id, token) = yield handler.register(
localpart=desired_user_id,
password=password
password=password,
threepid=threepid,
)
# Necessary due to auth checks prior to the threepid being
# written to the db
if is_threepid_reserved(self.hs.config, threepid):
yield self.store.upsert_monthly_active_user(user_id)

if session[LoginType.EMAIL_IDENTITY]:
logger.debug("Binding emails %s to %s" % (
Expand Down
10 changes: 10 additions & 0 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import synapse.types
from synapse.api.constants import LoginType
from synapse.api.errors import Codes, SynapseError, UnrecognizedRequestError
from synapse.config.server import is_threepid_reserved
from synapse.http.servlet import (
RestServlet,
assert_params_in_dict,
Expand Down Expand Up @@ -395,12 +396,21 @@ def on_POST(self, request):
if desired_username is not None:
desired_username = desired_username.lower()

threepid = None
if auth_result:
threepid = auth_result.get(LoginType.EMAIL_IDENTITY)

(registered_user_id, _) = yield self.registration_handler.register(
localpart=desired_username,
password=new_password,
guest_access_token=guest_access_token,
generate_token=False,
threepid=threepid,
)
# Necessary due to auth checks prior to the threepid being
# written to the db
if is_threepid_reserved(self.hs.config, threepid):
yield self.store.upsert_monthly_active_user(registered_user_id)

# remember that we've now registered that user account, and with
# what user ID (since the user may not have specified)
Expand Down
1 change: 0 additions & 1 deletion synapse/storage/monthly_active_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ def __init__(self, dbconn, hs):

@defer.inlineCallbacks
def initialise_reserved_users(self, threepids):
# TODO Why can't I do this in init?
store = self.hs.get_datastore()
reserved_user_list = []

Expand Down
17 changes: 17 additions & 0 deletions tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,23 @@ def test_blocking_mau(self):
)
yield self.auth.check_auth_blocking()

@defer.inlineCallbacks
def test_reserved_threepid(self):
self.hs.config.limit_usage_by_mau = True
self.hs.config.max_mau_value = 1
threepid = {'medium': 'email', 'address': 'reserved@server.com'}
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)
with self.assertRaises(ResourceLimitError):
yield self.auth.check_auth_blocking()

with self.assertRaises(ResourceLimitError):
yield self.auth.check_auth_blocking(threepid=unknown_threepid)

yield self.auth.check_auth_blocking(threepid=threepid)

@defer.inlineCallbacks
def test_hs_disabled(self):
self.hs.config.hs_disabled = True
Expand Down
6 changes: 6 additions & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

from synapse.api.constants import EventTypes
from synapse.api.errors import CodeMessageException, cs_error
from synapse.config.server import ServerConfig
from synapse.federation.transport import server
from synapse.http.server import HttpServer
from synapse.server import HomeServer
Expand Down Expand Up @@ -158,6 +159,11 @@ def setup_test_homeserver(
# background, which upsets the test runner.
config.update_user_directory = False

def is_threepid_reserved(threepid):
return ServerConfig.is_threepid_reserved(config, threepid)

config.is_threepid_reserved.side_effect = is_threepid_reserved

config.use_frozen_dicts = True
config.ldap_enabled = False

Expand Down