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

Commit 2e3b9a0

Browse files
committed
Revert "Revert "Merge pull request #7315 from matrix-org/babolivier/request_token""
This reverts commit 1adf6a5.
1 parent fb82575 commit 2e3b9a0

File tree

7 files changed

+121
-3
lines changed

7 files changed

+121
-3
lines changed

changelog.d/7315.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Allow `/requestToken` endpoints to hide the existence (or lack thereof) of 3PID associations on the homeserver.

docs/sample_config.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,16 @@ retention:
414414
# longest_max_lifetime: 1y
415415
# interval: 1d
416416

417+
# Inhibits the /requestToken endpoints from returning an error that might leak
418+
# information about whether an e-mail address is in use or not on this
419+
# homeserver.
420+
# Note that for some endpoints the error situation is the e-mail already being
421+
# used, and for others the error is entering the e-mail being unused.
422+
# If this option is enabled, instead of returning an error, these endpoints will
423+
# act as if no error happened and return a fake session ID ('sid') to clients.
424+
#
425+
#request_token_inhibit_3pid_errors: true
426+
417427

418428
## TLS ##
419429

synapse/config/server.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,17 @@ class LimitRemoteRoomsConfig(object):
507507

508508
self.enable_ephemeral_messages = config.get("enable_ephemeral_messages", False)
509509

510+
# Inhibits the /requestToken endpoints from returning an error that might leak
511+
# information about whether an e-mail address is in use or not on this
512+
# homeserver, and instead return a 200 with a fake sid if this kind of error is
513+
# met, without sending anything.
514+
# This is a compromise between sending an email, which could be a spam vector,
515+
# and letting the client know which email address is bound to an account and
516+
# which one isn't.
517+
self.request_token_inhibit_3pid_errors = config.get(
518+
"request_token_inhibit_3pid_errors", False,
519+
)
520+
510521
def has_tls_listener(self) -> bool:
511522
return any(l["tls"] for l in self.listeners)
512523

@@ -972,6 +983,16 @@ def generate_config_section(
972983
# - shortest_max_lifetime: 3d
973984
# longest_max_lifetime: 1y
974985
# interval: 1d
986+
987+
# Inhibits the /requestToken endpoints from returning an error that might leak
988+
# information about whether an e-mail address is in use or not on this
989+
# homeserver.
990+
# Note that for some endpoints the error situation is the e-mail already being
991+
# used, and for others the error is entering the e-mail being unused.
992+
# If this option is enabled, instead of returning an error, these endpoints will
993+
# act as if no error happened and return a fake session ID ('sid') to clients.
994+
#
995+
#request_token_inhibit_3pid_errors: true
975996
"""
976997
% locals()
977998
)

synapse/rest/client/v2_alpha/account.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
)
3131
from synapse.push.mailer import Mailer, load_jinja2_templates
3232
from synapse.util.msisdn import phone_number_to_msisdn
33-
from synapse.util.stringutils import assert_valid_client_secret
33+
from synapse.util.stringutils import assert_valid_client_secret, random_string
3434
from synapse.util.threepids import check_3pid_allowed
3535

3636
from ._base import client_patterns, interactive_auth_handler
@@ -100,6 +100,11 @@ async def on_POST(self, request):
100100
)
101101

102102
if existing_user_id is None:
103+
if self.config.request_token_inhibit_3pid_errors:
104+
# Make the client think the operation succeeded. See the rationale in the
105+
# comments for request_token_inhibit_3pid_errors.
106+
return 200, {"sid": random_string(16)}
107+
103108
raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
104109

105110
if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
@@ -390,6 +395,11 @@ async def on_POST(self, request):
390395
)
391396

392397
if existing_user_id is not None:
398+
if self.config.request_token_inhibit_3pid_errors:
399+
# Make the client think the operation succeeded. See the rationale in the
400+
# comments for request_token_inhibit_3pid_errors.
401+
return 200, {"sid": random_string(16)}
402+
393403
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
394404

395405
if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
@@ -453,6 +463,11 @@ async def on_POST(self, request):
453463
existing_user_id = await self.store.get_user_id_by_threepid("msisdn", msisdn)
454464

455465
if existing_user_id is not None:
466+
if self.hs.config.request_token_inhibit_3pid_errors:
467+
# Make the client think the operation succeeded. See the rationale in the
468+
# comments for request_token_inhibit_3pid_errors.
469+
return 200, {"sid": random_string(16)}
470+
456471
raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)
457472

458473
if not self.hs.config.account_threepid_delegate_msisdn:

synapse/rest/client/v2_alpha/register.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
from synapse.push.mailer import load_jinja2_templates
5050
from synapse.util.msisdn import phone_number_to_msisdn
5151
from synapse.util.ratelimitutils import FederationRateLimiter
52-
from synapse.util.stringutils import assert_valid_client_secret
52+
from synapse.util.stringutils import assert_valid_client_secret, random_string
5353
from synapse.util.threepids import check_3pid_allowed
5454

5555
from ._base import client_patterns, interactive_auth_handler
@@ -135,6 +135,11 @@ async def on_POST(self, request):
135135
)
136136

137137
if existing_user_id is not None:
138+
if self.hs.config.request_token_inhibit_3pid_errors:
139+
# Make the client think the operation succeeded. See the rationale in the
140+
# comments for request_token_inhibit_3pid_errors.
141+
return 200, {"sid": random_string(16)}
142+
138143
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
139144

140145
if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
@@ -202,6 +207,11 @@ async def on_POST(self, request):
202207
)
203208

204209
if existing_user_id is not None:
210+
if self.hs.config.request_token_inhibit_3pid_errors:
211+
# Make the client think the operation succeeded. See the rationale in the
212+
# comments for request_token_inhibit_3pid_errors.
213+
return 200, {"sid": random_string(16)}
214+
205215
raise SynapseError(
206216
400, "Phone number is already in use", Codes.THREEPID_IN_USE
207217
)

tests/rest/client/v2_alpha/test_account.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,22 @@ def test_no_valid_token(self):
179179
# Assert we can't log in with the new password
180180
self.attempt_wrong_password_login("kermit", new_password)
181181

182+
@unittest.override_config({"request_token_inhibit_3pid_errors": True})
183+
def test_password_reset_bad_email_inhibit_error(self):
184+
"""Test that triggering a password reset with an email address that isn't bound
185+
to an account doesn't leak the lack of binding for that address if configured
186+
that way.
187+
"""
188+
self.register_user("kermit", "monkey")
189+
self.login("kermit", "monkey")
190+
191+
email = "test@example.com"
192+
193+
client_secret = "foobar"
194+
session_id = self._request_token(email, client_secret)
195+
196+
self.assertIsNotNone(session_id)
197+
182198
def _request_token(self, email, client_secret):
183199
request, channel = self.make_request(
184200
"POST",

tests/rest/client/v2_alpha/test_register.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@
3333

3434
class RegisterRestServletTestCase(unittest.HomeserverTestCase):
3535

36-
servlets = [register.register_servlets]
36+
servlets = [
37+
login.register_servlets,
38+
register.register_servlets,
39+
synapse.rest.admin.register_servlets,
40+
]
3741
url = b"/_matrix/client/r0/register"
3842

3943
def default_config(self):
@@ -260,6 +264,47 @@ def test_advertised_flows_no_msisdn_email_required(self):
260264
[["m.login.email.identity"]], (f["stages"] for f in flows)
261265
)
262266

267+
@unittest.override_config(
268+
{
269+
"request_token_inhibit_3pid_errors": True,
270+
"public_baseurl": "https://test_server",
271+
"email": {
272+
"smtp_host": "mail_server",
273+
"smtp_port": 2525,
274+
"notif_from": "sender@host",
275+
},
276+
}
277+
)
278+
def test_request_token_existing_email_inhibit_error(self):
279+
"""Test that requesting a token via this endpoint doesn't leak existing
280+
associations if configured that way.
281+
"""
282+
user_id = self.register_user("kermit", "monkey")
283+
self.login("kermit", "monkey")
284+
285+
email = "test@example.com"
286+
287+
# Add a threepid
288+
self.get_success(
289+
self.hs.get_datastore().user_add_threepid(
290+
user_id=user_id,
291+
medium="email",
292+
address=email,
293+
validated_at=0,
294+
added_at=0,
295+
)
296+
)
297+
298+
request, channel = self.make_request(
299+
"POST",
300+
b"register/email/requestToken",
301+
{"client_secret": "foobar", "email": email, "send_attempt": 1},
302+
)
303+
self.render(request)
304+
self.assertEquals(200, channel.code, channel.result)
305+
306+
self.assertIsNotNone(channel.json_body.get("sid"))
307+
263308

264309
class AccountValidityTestCase(unittest.HomeserverTestCase):
265310

0 commit comments

Comments
 (0)