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

Fix fallback value for account_threepid_delegates.email #7316

Merged
merged 5 commits into from
Apr 29, 2020
Merged

Fix fallback value for account_threepid_delegates.email #7316

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c005181
Ensure account_threepid_delegate_email has a protocol scheme
anoadragon453 Apr 21, 2020
f63f76c
Changelog and lint
anoadragon453 Apr 21, 2020
4ffe175
mypy commands it
anoadragon453 Apr 21, 2020
ddf39b8
Allow trusted_third_party_id_server to be an empty list
anoadragon453 Apr 22, 2020
0cf3e7f
Fix type hinting and comment
anoadragon453 Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7316.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed backwards compatibility logic of the first value of `trusted_third_party_id_servers` being used for `account_threepid_delegates.email`, which occurs when the former, deprecated option is set and the latter is not.
12 changes: 8 additions & 4 deletions synapse/config/emailconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import email.utils
import os
from enum import Enum
from typing import Optional

import pkg_resources

Expand Down Expand Up @@ -108,9 +107,14 @@ def read_config(self, config, **kwargs):
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
self.account_threepid_delegate_email = self.trusted_third_party_id_servers[
0
] # type: Optional[str]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed Optional from this as self.account_threepid_delegate_email won't be assigned here if self.trusted_third_party_id_servers is falsey. But would mypy take this hint and apply it even if the code won't run?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. the type information is static, and isn't affected by which code runs. In other words, the annotation here says "the type of Config.trusted_third_party_id_servers is str", and that is a thing that should hold true throughout the entire codebase. In this case, it's possible for trusted_third_party_id_servers to be None, so its type needs to stay as Optional[str].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant account_threepid_delegate_email, but TIL, thanks!


first_trusted_identity_server = self.trusted_third_party_id_servers[0]

# This config option does not contain a scheme whereas
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
# account_threepid_delegate_email is expected to. Presume https
self.account_threepid_delegate_email = (
"https://" + first_trusted_identity_server
) # type: str
self.using_identity_server_from_trusted_list = True
else:
raise ConfigError(
Expand Down
4 changes: 2 additions & 2 deletions synapse/config/registration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ def read_config(self, config, **kwargs):

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"]
)
"trusted_third_party_id_servers"
) or ["matrix.org", "vector.im"]
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
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