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

Update m.id.phone to use 'phone' instead of 'number' #7687

Merged
merged 6 commits into from
Jun 15, 2020
Merged

Update m.id.phone to use 'phone' instead of 'number' #7687

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
65ca5a7
Update m.id.phone to use 'phone' instead of 'number'
anoadragon453 Jun 12, 2020
90c714d
Changelog
anoadragon453 Jun 12, 2020
91c1b70
Allow for old and new key name
anoadragon453 Jun 12, 2020
a7c2c32
Update changelog.d/7687.bugfix
anoadragon453 Jun 15, 2020
1f07f0f
Update changelog.d/7687.bugfix
anoadragon453 Jun 15, 2020
cd9a005
Clean up backwards compatibility comment
anoadragon453 Jun 15, 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/7687.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correct Synapse's definition of `m.id.phone`, changing `number` to `phone`. Bug introduced in v0.20.0-rc1.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 10 additions & 2 deletions synapse/rest/client/v1/login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,18 @@ def login_id_thirdparty_from_phone(identifier):

Returns: Login identifier dict of type 'm.id.threepid'
"""
if "country" not in identifier or "number" not in identifier:
if "country" not in identifier or (
# XXX: We used to require `number` instead of `phone`. The spec
Copy link
Member

Choose a reason for hiding this comment

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

Why XXX? I usually reserve that for something that's an action, maybe something like:

The specification requires a phone field, but Synapse used to require a number field. Accept both for backwards compatibility.

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 guess it was more intended as a TODO, we should remove this later.

...but, we're probably never going to remove it, so I think your suggestion does work better.

# defines `phone`. So accept both
"phone" not in identifier
and "number" not in identifier
):
raise SynapseError(400, "Invalid phone-type identifier")

msisdn = phone_number_to_msisdn(identifier["country"], identifier["number"])
# Accept both "phone" and "number" as valid keys in m.id.phone
phone_number = identifier.get("phone", identifier["number"])

msisdn = phone_number_to_msisdn(identifier["country"], phone_number)

return {"type": "m.id.thirdparty", "medium": "msisdn", "address": msisdn}

Expand Down