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

support admin_email config and pass through into blocking errors, #3687

Merged
merged 13 commits into from
Aug 15, 2018
Merged
Show file tree
Hide file tree
Changes from 12 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/3687.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
set admin uri via config, to be used in error messages where the user should contact the administrator
1 change: 1 addition & 0 deletions changelog.d/3697.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
update resource limit error codes
Copy link
Member

Choose a reason for hiding this comment

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

We should probably not have two changelogs?

8 changes: 6 additions & 2 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,9 @@ def check_auth_blocking(self, user_id=None):
"""
if self.hs.config.hs_disabled:
raise AuthError(
403, self.hs.config.hs_disabled_message, errcode=Codes.HS_DISABLED
403, self.hs.config.hs_disabled_message,
errcode=Codes.RESOURCE_LIMIT_EXCEED,
admin_uri=self.hs.config.admin_uri,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you've manged to pull this in from the PR to change the error codes. I'm raising an eyebrow slightly that we're not able to differentiate between the mau/resource stuff and the generic disable HS option.

Copy link
Contributor Author

@neilisfragile neilisfragile Aug 15, 2018

Choose a reason for hiding this comment

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

I'm using https://docs.google.com/document/d/1_CgwkfLznU56fwLUFZXFnKivzVUPjjNsTO0DpCB9ncM/edit#heading=h.g9t44e4usbqd as the base - the proposal is not ratified so could change further (in fact please add comments) - the idea is to be flexible for future limiting, with a human readable explainer in the body - this approach doesn't lend itself to i18n though

Copy link
Member

Choose a reason for hiding this comment

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

I guess we can leave it as is for now and add an option for setting the error code in the config when we disable, if it proves necessary.

)
if self.hs.config.limit_usage_by_mau is True:
# If the user is already part of the MAU cohort
Expand All @@ -797,5 +799,7 @@ def check_auth_blocking(self, user_id=None):
current_mau = yield self.store.get_monthly_active_count()
if current_mau >= self.hs.config.max_mau_value:
raise AuthError(
403, "MAU Limit Exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED
403, "Monthly Active User Limits AU Limit Exceeded",
admin_uri=self.hs.config.admin_uri,
errcode=Codes.RESOURCE_LIMIT_EXCEED
)
16 changes: 10 additions & 6 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ class Codes(object):
SERVER_NOT_TRUSTED = "M_SERVER_NOT_TRUSTED"
CONSENT_NOT_GIVEN = "M_CONSENT_NOT_GIVEN"
CANNOT_LEAVE_SERVER_NOTICE_ROOM = "M_CANNOT_LEAVE_SERVER_NOTICE_ROOM"
MAU_LIMIT_EXCEEDED = "M_MAU_LIMIT_EXCEEDED"
HS_DISABLED = "M_HS_DISABLED"
RESOURCE_LIMIT_EXCEED = "M_RESOURCE_LIMIT_EXCEED"
UNSUPPORTED_ROOM_VERSION = "M_UNSUPPORTED_ROOM_VERSION"
INCOMPATIBLE_ROOM_VERSION = "M_INCOMPATIBLE_ROOM_VERSION"

Expand Down Expand Up @@ -225,11 +224,16 @@ def __init__(self, msg="Not found", errcode=Codes.NOT_FOUND):

class AuthError(SynapseError):
"""An error raised when there was a problem authorising an event."""
def __init__(self, code, msg, errcode=Codes.FORBIDDEN, admin_uri=None):
self.admin_uri = admin_uri
super(AuthError, self).__init__(code, msg, errcode=errcode)

def __init__(self, *args, **kwargs):
if "errcode" not in kwargs:
kwargs["errcode"] = Codes.FORBIDDEN
super(AuthError, self).__init__(*args, **kwargs)
def error_dict(self):
return cs_error(
self.msg,
self.errcode,
admin_uri=self.admin_uri,
)


class EventSizeError(SynapseError):
Expand Down
4 changes: 4 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ def read_config(self, config):
self.hs_disabled = config.get("hs_disabled", False)
self.hs_disabled_message = config.get("hs_disabled_message", "")

# Admin uri to direct users at should their instance become blocked
# due to resource constraints
self.admin_uri = config.get("admin_uri", None)
Copy link
Member

Choose a reason for hiding this comment

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

Admin email or admin URI?


# FIXME: federation_domain_whitelist needs sytests
self.federation_domain_whitelist = None
federation_domain_whitelist = config.get(
Expand Down
18 changes: 4 additions & 14 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ def register(
Raises:
RegistrationError if there was a problem registering.
"""
yield self._check_mau_limits()

yield self.auth.check_auth_blocking()
password_hash = None
if password:
password_hash = yield self.auth_handler().hash(password)
Expand Down Expand Up @@ -289,7 +290,7 @@ def register_saml2(self, localpart):
400,
"User ID can only contain characters a-z, 0-9, or '=_-./'",
)
yield self._check_mau_limits()
yield self.auth.check_auth_blocking()
user = UserID(localpart, self.hs.hostname)
user_id = user.to_string()

Expand Down Expand Up @@ -439,7 +440,7 @@ def get_or_create_user(self, requester, localpart, displayname,
"""
if localpart is None:
raise SynapseError(400, "Request must include user id")
yield self._check_mau_limits()
yield self.auth.check_auth_blocking()
need_register = True

try:
Expand Down Expand Up @@ -533,14 +534,3 @@ def _join_user_to_room(self, requester, room_identifier):
remote_room_hosts=remote_room_hosts,
action="join",
)

@defer.inlineCallbacks
def _check_mau_limits(self):
"""
Do not accept registrations if monthly active user limits exceeded
and limiting is enabled
"""
try:
yield self.auth.check_auth_blocking()
except AuthError as e:
raise RegistrationError(e.code, str(e), e.errcode)
8 changes: 6 additions & 2 deletions tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,11 @@ def test_blocking_mau(self):
return_value=defer.succeed(lots_of_users)
)

with self.assertRaises(AuthError):
with self.assertRaises(AuthError) as e:
yield self.auth.check_auth_blocking()
self.assertEquals(e.exception.admin_uri, self.hs.config.admin_uri)
self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
self.assertEquals(e.exception.code, 403)

# Ensure does not throw an error
self.store.get_monthly_active_count = Mock(
Expand All @@ -470,5 +473,6 @@ def test_hs_disabled(self):
self.hs.config.hs_disabled_message = "Reason for being disabled"
with self.assertRaises(AuthError) as e:
yield self.auth.check_auth_blocking()
self.assertEquals(e.exception.errcode, Codes.HS_DISABLED)
self.assertEquals(e.exception.admin_uri, self.hs.config.admin_uri)
self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
self.assertEquals(e.exception.code, 403)
8 changes: 4 additions & 4 deletions tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from twisted.internet import defer

from synapse.api.errors import RegistrationError
from synapse.api.errors import AuthError
from synapse.handlers.register import RegistrationHandler
from synapse.types import UserID, create_requester

Expand Down Expand Up @@ -109,7 +109,7 @@ def test_get_or_create_user_mau_blocked(self):
self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.lots_of_users)
)
with self.assertRaises(RegistrationError):
with self.assertRaises(AuthError):
yield self.handler.get_or_create_user("requester", 'b', "display_name")

@defer.inlineCallbacks
Expand All @@ -118,7 +118,7 @@ def test_register_mau_blocked(self):
self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.lots_of_users)
)
with self.assertRaises(RegistrationError):
with self.assertRaises(AuthError):
yield self.handler.register(localpart="local_part")

@defer.inlineCallbacks
Expand All @@ -127,5 +127,5 @@ def test_register_saml2_mau_blocked(self):
self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.lots_of_users)
)
with self.assertRaises(RegistrationError):
with self.assertRaises(AuthError):
yield self.handler.register_saml2(localpart="local_part")
4 changes: 2 additions & 2 deletions tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ def test_wait_for_sync_for_user_auth_blocking(self):
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.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)

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)
self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)

def _generate_sync_config(self, user_id):
return SyncConfig(
Expand Down
1 change: 1 addition & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def setup_test_homeserver(
config.hs_disabled_message = ""
config.max_mau_value = 50
config.mau_limits_reserved_threepids = []
config.admin_uri = None

# we need a sane default_room_version, otherwise attempts to create rooms will
# fail.
Expand Down