mirror of
https://git.anonymousland.org/anonymousland/synapse-product.git
synced 2024-10-01 08:25:44 -04:00
Don't fail /submit_token requests on incorrect session ID if request_token_inhibit_3pid_errors is turned on (#7991)
* Don't raise session_id errors on submit_token if request_token_inhibit_3pid_errors is set * Changelog * Also wait some time before responding to /requestToken * Incorporate review * Update synapse/storage/databases/main/registration.py Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> * Incorporate review Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
This commit is contained in:
parent
cbbf9126cb
commit
3f49f74610
1
changelog.d/7991.misc
Normal file
1
changelog.d/7991.misc
Normal file
@ -0,0 +1 @@
|
|||||||
|
Don't fail `/submit_token` requests on incorrect session ID if `request_token_inhibit_3pid_errors` is turned on.
|
@ -15,6 +15,7 @@
|
|||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
import logging
|
import logging
|
||||||
|
import random
|
||||||
from http import HTTPStatus
|
from http import HTTPStatus
|
||||||
|
|
||||||
from synapse.api.constants import LoginType
|
from synapse.api.constants import LoginType
|
||||||
@ -109,6 +110,9 @@ class EmailPasswordRequestTokenRestServlet(RestServlet):
|
|||||||
if self.config.request_token_inhibit_3pid_errors:
|
if self.config.request_token_inhibit_3pid_errors:
|
||||||
# Make the client think the operation succeeded. See the rationale in the
|
# Make the client think the operation succeeded. See the rationale in the
|
||||||
# comments for request_token_inhibit_3pid_errors.
|
# comments for request_token_inhibit_3pid_errors.
|
||||||
|
# Also wait for some random amount of time between 100ms and 1s to make it
|
||||||
|
# look like we did something.
|
||||||
|
await self.hs.clock.sleep(random.randint(1, 10) / 10)
|
||||||
return 200, {"sid": random_string(16)}
|
return 200, {"sid": random_string(16)}
|
||||||
|
|
||||||
raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
|
raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
|
||||||
@ -448,6 +452,9 @@ class EmailThreepidRequestTokenRestServlet(RestServlet):
|
|||||||
if self.config.request_token_inhibit_3pid_errors:
|
if self.config.request_token_inhibit_3pid_errors:
|
||||||
# Make the client think the operation succeeded. See the rationale in the
|
# Make the client think the operation succeeded. See the rationale in the
|
||||||
# comments for request_token_inhibit_3pid_errors.
|
# comments for request_token_inhibit_3pid_errors.
|
||||||
|
# Also wait for some random amount of time between 100ms and 1s to make it
|
||||||
|
# look like we did something.
|
||||||
|
await self.hs.clock.sleep(random.randint(1, 10) / 10)
|
||||||
return 200, {"sid": random_string(16)}
|
return 200, {"sid": random_string(16)}
|
||||||
|
|
||||||
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
|
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
|
||||||
@ -516,6 +523,9 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet):
|
|||||||
if self.hs.config.request_token_inhibit_3pid_errors:
|
if self.hs.config.request_token_inhibit_3pid_errors:
|
||||||
# Make the client think the operation succeeded. See the rationale in the
|
# Make the client think the operation succeeded. See the rationale in the
|
||||||
# comments for request_token_inhibit_3pid_errors.
|
# comments for request_token_inhibit_3pid_errors.
|
||||||
|
# Also wait for some random amount of time between 100ms and 1s to make it
|
||||||
|
# look like we did something.
|
||||||
|
await self.hs.clock.sleep(random.randint(1, 10) / 10)
|
||||||
return 200, {"sid": random_string(16)}
|
return 200, {"sid": random_string(16)}
|
||||||
|
|
||||||
raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)
|
raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)
|
||||||
|
@ -16,6 +16,7 @@
|
|||||||
|
|
||||||
import hmac
|
import hmac
|
||||||
import logging
|
import logging
|
||||||
|
import random
|
||||||
from typing import List, Union
|
from typing import List, Union
|
||||||
|
|
||||||
import synapse
|
import synapse
|
||||||
@ -131,6 +132,9 @@ class EmailRegisterRequestTokenRestServlet(RestServlet):
|
|||||||
if self.hs.config.request_token_inhibit_3pid_errors:
|
if self.hs.config.request_token_inhibit_3pid_errors:
|
||||||
# Make the client think the operation succeeded. See the rationale in the
|
# Make the client think the operation succeeded. See the rationale in the
|
||||||
# comments for request_token_inhibit_3pid_errors.
|
# comments for request_token_inhibit_3pid_errors.
|
||||||
|
# Also wait for some random amount of time between 100ms and 1s to make it
|
||||||
|
# look like we did something.
|
||||||
|
await self.hs.clock.sleep(random.randint(1, 10) / 10)
|
||||||
return 200, {"sid": random_string(16)}
|
return 200, {"sid": random_string(16)}
|
||||||
|
|
||||||
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
|
raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
|
||||||
@ -203,6 +207,9 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet):
|
|||||||
if self.hs.config.request_token_inhibit_3pid_errors:
|
if self.hs.config.request_token_inhibit_3pid_errors:
|
||||||
# Make the client think the operation succeeded. See the rationale in the
|
# Make the client think the operation succeeded. See the rationale in the
|
||||||
# comments for request_token_inhibit_3pid_errors.
|
# comments for request_token_inhibit_3pid_errors.
|
||||||
|
# Also wait for some random amount of time between 100ms and 1s to make it
|
||||||
|
# look like we did something.
|
||||||
|
await self.hs.clock.sleep(random.randint(1, 10) / 10)
|
||||||
return 200, {"sid": random_string(16)}
|
return 200, {"sid": random_string(16)}
|
||||||
|
|
||||||
raise SynapseError(
|
raise SynapseError(
|
||||||
|
@ -889,6 +889,7 @@ class RegistrationStore(RegistrationBackgroundUpdateStore):
|
|||||||
super(RegistrationStore, self).__init__(database, db_conn, hs)
|
super(RegistrationStore, self).__init__(database, db_conn, hs)
|
||||||
|
|
||||||
self._account_validity = hs.config.account_validity
|
self._account_validity = hs.config.account_validity
|
||||||
|
self._ignore_unknown_session_error = hs.config.request_token_inhibit_3pid_errors
|
||||||
|
|
||||||
if self._account_validity.enabled:
|
if self._account_validity.enabled:
|
||||||
self._clock.call_later(
|
self._clock.call_later(
|
||||||
@ -1302,15 +1303,22 @@ class RegistrationStore(RegistrationBackgroundUpdateStore):
|
|||||||
)
|
)
|
||||||
|
|
||||||
if not row:
|
if not row:
|
||||||
raise ThreepidValidationError(400, "Unknown session_id")
|
if self._ignore_unknown_session_error:
|
||||||
|
# If we need to inhibit the error caused by an incorrect session ID,
|
||||||
|
# use None as placeholder values for the client secret and the
|
||||||
|
# validation timestamp.
|
||||||
|
# It shouldn't be an issue because they're both only checked after
|
||||||
|
# the token check, which should fail. And if it doesn't for some
|
||||||
|
# reason, the next check is on the client secret, which is NOT NULL,
|
||||||
|
# so we don't have to worry about the client secret matching by
|
||||||
|
# accident.
|
||||||
|
row = {"client_secret": None, "validated_at": None}
|
||||||
|
else:
|
||||||
|
raise ThreepidValidationError(400, "Unknown session_id")
|
||||||
|
|
||||||
retrieved_client_secret = row["client_secret"]
|
retrieved_client_secret = row["client_secret"]
|
||||||
validated_at = row["validated_at"]
|
validated_at = row["validated_at"]
|
||||||
|
|
||||||
if retrieved_client_secret != client_secret:
|
|
||||||
raise ThreepidValidationError(
|
|
||||||
400, "This client_secret does not match the provided session_id"
|
|
||||||
)
|
|
||||||
|
|
||||||
row = self.db_pool.simple_select_one_txn(
|
row = self.db_pool.simple_select_one_txn(
|
||||||
txn,
|
txn,
|
||||||
table="threepid_validation_token",
|
table="threepid_validation_token",
|
||||||
@ -1326,6 +1334,11 @@ class RegistrationStore(RegistrationBackgroundUpdateStore):
|
|||||||
expires = row["expires"]
|
expires = row["expires"]
|
||||||
next_link = row["next_link"]
|
next_link = row["next_link"]
|
||||||
|
|
||||||
|
if retrieved_client_secret != client_secret:
|
||||||
|
raise ThreepidValidationError(
|
||||||
|
400, "This client_secret does not match the provided session_id"
|
||||||
|
)
|
||||||
|
|
||||||
# If the session is already validated, no need to revalidate
|
# If the session is already validated, no need to revalidate
|
||||||
if validated_at:
|
if validated_at:
|
||||||
return next_link
|
return next_link
|
||||||
|
@ -17,6 +17,7 @@
|
|||||||
from twisted.internet import defer
|
from twisted.internet import defer
|
||||||
|
|
||||||
from synapse.api.constants import UserTypes
|
from synapse.api.constants import UserTypes
|
||||||
|
from synapse.api.errors import ThreepidValidationError
|
||||||
|
|
||||||
from tests import unittest
|
from tests import unittest
|
||||||
from tests.utils import setup_test_homeserver
|
from tests.utils import setup_test_homeserver
|
||||||
@ -122,3 +123,33 @@ class RegistrationStoreTestCase(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
res = yield self.store.is_support_user(SUPPORT_USER)
|
res = yield self.store.is_support_user(SUPPORT_USER)
|
||||||
self.assertTrue(res)
|
self.assertTrue(res)
|
||||||
|
|
||||||
|
@defer.inlineCallbacks
|
||||||
|
def test_3pid_inhibit_invalid_validation_session_error(self):
|
||||||
|
"""Tests that enabling the configuration option to inhibit 3PID errors on
|
||||||
|
/requestToken also inhibits validation errors caused by an unknown session ID.
|
||||||
|
"""
|
||||||
|
|
||||||
|
# Check that, with the config setting set to false (the default value), a
|
||||||
|
# validation error is caused by the unknown session ID.
|
||||||
|
try:
|
||||||
|
yield defer.ensureDeferred(
|
||||||
|
self.store.validate_threepid_session(
|
||||||
|
"fake_sid", "fake_client_secret", "fake_token", 0,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
except ThreepidValidationError as e:
|
||||||
|
self.assertEquals(e.msg, "Unknown session_id", e)
|
||||||
|
|
||||||
|
# Set the config setting to true.
|
||||||
|
self.store._ignore_unknown_session_error = True
|
||||||
|
|
||||||
|
# Check that now the validation error is caused by the token not matching.
|
||||||
|
try:
|
||||||
|
yield defer.ensureDeferred(
|
||||||
|
self.store.validate_threepid_session(
|
||||||
|
"fake_sid", "fake_client_secret", "fake_token", 0,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
except ThreepidValidationError as e:
|
||||||
|
self.assertEquals(e.msg, "Validation token not found or has expired", e)
|
||||||
|
Loading…
Reference in New Issue
Block a user