-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
support admin_email config and pass through into blocking errors, #3687
Changes from 6 commits
f4b4915
1f24d86
99ebaed
c74c711
19b433e
ab035bd
b8429c7
596ce63
75c663c
55afba0
c4eb975
87a824b
39176f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
set admin email via config, to be used in error messages where the user should contact the administrator |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -225,11 +225,20 @@ 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, *args, **kwargs): | ||
if "errcode" not in kwargs: | ||
kwargs["errcode"] = Codes.FORBIDDEN | ||
super(AuthError, self).__init__(*args, **kwargs) | ||
self.admin_uri = kwargs.get('admin_uri') | ||
self.msg = kwargs.get('msg') | ||
self.errcode = kwargs.get('errcode') | ||
super(AuthError, self).__init__(*args, errcode=kwargs["errcode"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this constructor now looks odd. We should either pass through Its also worth noting that I'd probably write this as something like: def __init__(self, code, msg, errcode=Codes.FORBIDDEN, admin_url=None):
self.admin_uri = admin_url
super(AuthError, self).__init__(code, msg, errcode=errcode) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, that's brain dead on my part, thanks for the clean up |
||
|
||
def error_dict(self): | ||
return cs_error( | ||
self.msg, | ||
self.errcode, | ||
admin_uri=self.admin_uri, | ||
) | ||
|
||
|
||
class EventSizeError(SynapseError): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 email to direct users at should their instance become blocked | ||
# due to resource constraints | ||
self.admin_uri = config.get("admin_uri", None) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.