Skip to content

Commit

Permalink
Revert "Merge pull request matrix-org#7315 from matrix-org/babolivier…
Browse files Browse the repository at this point in the history
…/request_token"

This reverts commit 6f43193, reversing
changes made to 0d775fc.
  • Loading branch information
babolivier committed Apr 23, 2020
1 parent 6f43193 commit 1adf6a5
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 121 deletions.
1 change: 0 additions & 1 deletion changelog.d/7315.feature

This file was deleted.

10 changes: 0 additions & 10 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -409,16 +409,6 @@ retention:
# longest_max_lifetime: 1y
# interval: 1d

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


## TLS ##

Expand Down
21 changes: 0 additions & 21 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,17 +507,6 @@ class LimitRemoteRoomsConfig(object):

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

# Inhibits the /requestToken endpoints from returning an error that might leak
# information about whether an e-mail address is in use or not on this
# homeserver, and instead return a 200 with a fake sid if this kind of error is
# met, without sending anything.
# This is a compromise between sending an email, which could be a spam vector,
# and letting the client know which email address is bound to an account and
# which one isn't.
self.request_token_inhibit_3pid_errors = config.get(
"request_token_inhibit_3pid_errors", False,
)

def has_tls_listener(self) -> bool:
return any(l["tls"] for l in self.listeners)

Expand Down Expand Up @@ -978,16 +967,6 @@ def generate_config_section(
# - shortest_max_lifetime: 3d
# longest_max_lifetime: 1y
# interval: 1d
# Inhibits the /requestToken endpoints from returning an error that might leak
# information about whether an e-mail address is in use or not on this
# homeserver.
# Note that for some endpoints the error situation is the e-mail already being
# used, and for others the error is entering the e-mail being unused.
# If this option is enabled, instead of returning an error, these endpoints will
# act as if no error happened and return a fake session ID ('sid') to clients.
#
#request_token_inhibit_3pid_errors: true
"""
% locals()
)
Expand Down
17 changes: 1 addition & 16 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)
from synapse.push.mailer import Mailer, load_jinja2_templates
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.stringutils import assert_valid_client_secret
from synapse.util.threepids import check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler
Expand Down Expand Up @@ -100,11 +100,6 @@ async def on_POST(self, request):
)

if existing_user_id is None:
if self.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -383,11 +378,6 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -451,11 +441,6 @@ async def on_POST(self, request):
existing_user_id = await self.store.get_user_id_by_threepid("msisdn", msisdn)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)

if not self.hs.config.account_threepid_delegate_msisdn:
Expand Down
12 changes: 1 addition & 11 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
from synapse.push.mailer import load_jinja2_templates
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.ratelimitutils import FederationRateLimiter
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.stringutils import assert_valid_client_secret
from synapse.util.threepids import check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler
Expand Down Expand Up @@ -135,11 +135,6 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -207,11 +202,6 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(
400, "Phone number is already in use", Codes.THREEPID_IN_USE
)
Expand Down
16 changes: 0 additions & 16 deletions tests/rest/client/v2_alpha/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,22 +178,6 @@ def test_no_valid_token(self):
# Assert we can't log in with the new password
self.attempt_wrong_password_login("kermit", new_password)

@unittest.override_config({"request_token_inhibit_3pid_errors": True})
def test_password_reset_bad_email_inhibit_error(self):
"""Test that triggering a password reset with an email address that isn't bound
to an account doesn't leak the lack of binding for that address if configured
that way.
"""
self.register_user("kermit", "monkey")
self.login("kermit", "monkey")

email = "test@example.com"

client_secret = "foobar"
session_id = self._request_token(email, client_secret)

self.assertIsNotNone(session_id)

def _request_token(self, email, client_secret):
request, channel = self.make_request(
"POST",
Expand Down
47 changes: 1 addition & 46 deletions tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,7 @@

class RegisterRestServletTestCase(unittest.HomeserverTestCase):

servlets = [
login.register_servlets,
register.register_servlets,
synapse.rest.admin.register_servlets,
]
servlets = [register.register_servlets]
url = b"/_matrix/client/r0/register"

def default_config(self, name="test"):
Expand Down Expand Up @@ -264,47 +260,6 @@ def test_advertised_flows_no_msisdn_email_required(self):
[["m.login.email.identity"]], (f["stages"] for f in flows)
)

@unittest.override_config(
{
"request_token_inhibit_3pid_errors": True,
"public_baseurl": "https://test_server",
"email": {
"smtp_host": "mail_server",
"smtp_port": 2525,
"notif_from": "sender@host",
},
}
)
def test_request_token_existing_email_inhibit_error(self):
"""Test that requesting a token via this endpoint doesn't leak existing
associations if configured that way.
"""
user_id = self.register_user("kermit", "monkey")
self.login("kermit", "monkey")

email = "test@example.com"

# Add a threepid
self.get_success(
self.hs.get_datastore().user_add_threepid(
user_id=user_id,
medium="email",
address=email,
validated_at=0,
added_at=0,
)
)

request, channel = self.make_request(
"POST",
b"register/email/requestToken",
{"client_secret": "foobar", "email": email, "send_attempt": 1},
)
self.render(request)
self.assertEquals(200, channel.code, channel.result)

self.assertIsNotNone(channel.json_body.get("sid"))


class AccountValidityTestCase(unittest.HomeserverTestCase):

Expand Down

0 comments on commit 1adf6a5

Please sign in to comment.