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

Commit

Permalink
Remove legacy code related to deprecated `trust_identity_server_for_p…
Browse files Browse the repository at this point in the history
…assword_resets` config flag (#11333)

* remove code legacy code related to deprecated config flag "trust_identity_server_for_password_resets" from synapse/config/emailconfig.py

* remove legacy code supporting depreciated config flag "trust_identity_server_for_password_resets" from synapse/config/registration.py

* remove legacy code supporting depreciated config flag "trust_identity_server_for_password_resets" from synapse/handlers/identity.py

* add tests to ensure config error is thrown and synapse refuses to start when depreciated config flag is found

* add changelog

* slightly change behavior to only check for deprecated flag if set to 'true'

* Update changelog.d/11333.misc

Co-authored-by: reivilibre <[email protected]>

Co-authored-by: reivilibre <[email protected]>
  • Loading branch information
H-Shay and reivilibre authored Nov 18, 2021
1 parent 81b18fe commit 92b7538
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 47 deletions.
1 change: 1 addition & 0 deletions changelog.d/11333.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove deprecated `trust_identity_server_for_password_resets` configuration flag.
33 changes: 7 additions & 26 deletions synapse/config/emailconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,33 +137,14 @@ def read_config(self, config, **kwargs):
if self.root.registration.account_threepid_delegate_email
else ThreepidBehaviour.LOCAL
)
# Prior to Synapse v1.4.0, there was another option that defined whether Synapse would
# use an identity server to password reset tokens on its behalf. We now warn the user
# if they have this set and tell them to use the updated option, while using a default
# identity server in the process.
self.using_identity_server_from_trusted_list = False
if (
not self.root.registration.account_threepid_delegate_email
and config.get("trust_identity_server_for_password_resets", False) is True
):
# Use the first entry in self.trusted_third_party_id_servers instead
if self.trusted_third_party_id_servers:
# XXX: It's a little confusing that account_threepid_delegate_email is modified
# both in RegistrationConfig and here. We should factor this bit out

first_trusted_identity_server = self.trusted_third_party_id_servers[0]

# trusted_third_party_id_servers does not contain a scheme whereas
# account_threepid_delegate_email is expected to. Presume https
self.root.registration.account_threepid_delegate_email = (
"https://" + first_trusted_identity_server
)
self.using_identity_server_from_trusted_list = True
else:
raise ConfigError(
"Attempted to use an identity server from"
'"trusted_third_party_id_servers" but it is empty.'
)
if config.get("trust_identity_server_for_password_resets"):
raise ConfigError(
'The config option "trust_identity_server_for_password_resets" '
'has been replaced by "account_threepid_delegate". '
"Please consult the sample config at docs/sample_config.yaml for "
"details and update your config file."
)

self.local_threepid_handling_disabled_due_to_email_config = False
if (
Expand Down
4 changes: 1 addition & 3 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ def read_config(self, config, **kwargs):
self.registration_shared_secret = config.get("registration_shared_secret")

self.bcrypt_rounds = config.get("bcrypt_rounds", 12)
self.trusted_third_party_id_servers = config.get(
"trusted_third_party_id_servers", ["matrix.org", "vector.im"]
)

account_threepid_delegates = config.get("account_threepid_delegates") or {}
self.account_threepid_delegate_email = account_threepid_delegates.get("email")
self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn")
Expand Down
18 changes: 0 additions & 18 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,15 +464,6 @@ async def requestEmailToken(
if next_link:
params["next_link"] = next_link

if self.hs.config.email.using_identity_server_from_trusted_list:
# Warn that a deprecated config option is in use
logger.warning(
'The config option "trust_identity_server_for_password_resets" '
'has been replaced by "account_threepid_delegate". '
"Please consult the sample config at docs/sample_config.yaml for "
"details and update your config file."
)

try:
data = await self.http_client.post_json_get_json(
id_server + "/_matrix/identity/api/v1/validate/email/requestToken",
Expand Down Expand Up @@ -517,15 +508,6 @@ async def requestMsisdnToken(
if next_link:
params["next_link"] = next_link

if self.hs.config.email.using_identity_server_from_trusted_list:
# Warn that a deprecated config option is in use
logger.warning(
'The config option "trust_identity_server_for_password_resets" '
'has been replaced by "account_threepid_delegate". '
"Please consult the sample config at docs/sample_config.yaml for "
"details and update your config file."
)

try:
data = await self.http_client.post_json_get_json(
id_server + "/_matrix/identity/api/v1/validate/msisdn/requestToken",
Expand Down
9 changes: 9 additions & 0 deletions tests/config/test_load.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,12 @@ def test_stats_enabled(self):
# The default Metrics Flags are off by default.
config = HomeServerConfig.load_config("", ["-c", self.config_file])
self.assertFalse(config.metrics.metrics_flags.known_servers)

def test_depreciated_identity_server_flag_throws_error(self):
self.generate_config()
# Needed to ensure that actual key/value pair added below don't end up on a line with a comment
self.add_lines_to_config([" "])
# Check that presence of "trust_identity_server_for_password" throws config error
self.add_lines_to_config(["trust_identity_server_for_password_resets: true"])
with self.assertRaises(ConfigError):
HomeServerConfig.load_config("", ["-c", self.config_file])

0 comments on commit 92b7538

Please sign in to comment.