-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add restrictions by default to open registration in Synapse #12091
Changes from 19 commits
be96970
46f86b6
8cddb87
cb55db5
25aa51b
97b1c95
0e328cb
3b21267
1d66611
dabf316
338c435
2f8fcc0
a0582ed
8346255
9b89835
b14b653
717af3c
e1a9098
0cd6a60
366da79
39ffa66
d99a1f1
11fa645
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 @@ | ||
Refuse to start if registration is enabled without email or captcha verification unless new config flag `enable_registration_without_verification` is set. | ||
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -348,6 +348,23 @@ def setup(config_options: List[str]) -> SynapseHomeServer: | |||||||||
if config.server.gc_seconds: | ||||||||||
synapse.metrics.MIN_TIME_BETWEEN_GCS = config.server.gc_seconds | ||||||||||
|
||||||||||
if ( | ||||||||||
config.registration.enable_registration | ||||||||||
and not config.registration.enable_registration_without_verification | ||||||||||
): | ||||||||||
if ( | ||||||||||
not config.captcha.enable_registration_captcha | ||||||||||
and not config.registration.registrations_require_3pid | ||||||||||
and not config.registration.registration_requires_token | ||||||||||
): | ||||||||||
|
||||||||||
raise ConfigError( | ||||||||||
"You have enabled open registration without any verification. This is a known vector for " | ||||||||||
"spam and abuse. If you have enabled registration, please add email, capcha, or token-based verification, or " | ||||||||||
"use the config option `enable_registration_without_verification` in conjunction with `enable_verification." | ||||||||||
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.
Suggested change
may be a bit clearer. |
||||||||||
) | ||||||||||
sys.exit(1) | ||||||||||
H-Shay marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
hs = SynapseHomeServer( | ||||||||||
config.server.server_name, | ||||||||||
config=config, | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,14 +11,16 @@ | |
# 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 synapse.app.homeserver | ||
from synapse.config import ConfigError | ||
from synapse.config.homeserver import HomeServerConfig | ||
|
||
from tests.unittest import TestCase | ||
from tests.config.utils import ConfigFileTestCase | ||
from tests.utils import default_config | ||
|
||
|
||
class RegistrationConfigTestCase(TestCase): | ||
class RegistrationConfigTestCase(ConfigFileTestCase): | ||
def test_session_lifetime_must_not_be_exceeded_by_smaller_lifetimes(self): | ||
""" | ||
session_lifetime should logically be larger than, or at least as large as, | ||
|
@@ -76,3 +78,19 @@ def test_session_lifetime_must_not_be_exceeded_by_smaller_lifetimes(self): | |
HomeServerConfig().parse_config_dict( | ||
{"session_lifetime": "31m", "refresh_token_lifetime": "31m", **config_dict} | ||
) | ||
|
||
def test_refuse_to_start_if_open_registration_and_no_verification(self): | ||
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. It's only config and straight-forward code so it's tempting not to bother, but this is only testing the negative. You may also be tempted to test that startup is able to proceed if both 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. Hmm I tested this condition locally and it works. Looking at the test though it feels like it's not necessary to add permanently to the tests. If you feel strongly about it I will, though. |
||
self.generate_config() | ||
self.add_lines_to_config( | ||
[ | ||
" ", | ||
"enable_registration: true", | ||
"registrations_require_3pid: []", | ||
"enable_registration_captcha: false", | ||
"registration_requires_token: false", | ||
] | ||
) | ||
|
||
# Test that allowing open registration without verification raises an error | ||
with self.assertRaises(ConfigError): | ||
synapse.app.homeserver.setup(["-c", self.config_file]) |
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.
We should add a line to https://matrix-org.github.io/synapse/latest/upgrade, as this change will require a subset of administrators to fiddle with their config before their homeserver can start up again.