This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Implement MSC2290 #6043
Merged
Merged
Implement MSC2290 #6043
Changes from 42 commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
d00435b
Allow HS to send emails when adding an email to the HS
anoadragon453 c04b2c0
Correct some small issues and fix bug
anoadragon453 292c007
Address review comments
anoadragon453 da11cc6
Remove blacklist
anoadragon453 f5e4c1b
Fix add_threepid template default values in emailconfig
anoadragon453 bb13515
Re-blacklist tests
anoadragon453 9718ad6
Factor out removing id_server from msisdn
anoadragon453 b7cd985
Ensure REMOTE vs LOCAL ThreepidBehaviour is handled
anoadragon453 1f17307
Move jinja failure template loading into servlet constructor
anoadragon453 d7ff1cd
Set email
anoadragon453 7a678a6
Make sure templates only get loaded when necessary
anoadragon453 102b608
Pull if validation_session out of helper method
anoadragon453 37281e3
this confused me for so long
anoadragon453 29fc7bb
Ensure we catch HttpResponseException when calling to id servers
anoadragon453 f4e93ae
Unpack response from identity server to check for errors
anoadragon453 b840aff
Factor out password_reset trailing slash change
anoadragon453 3713c2c
Address review comments
anoadragon453 8dcb79c
validation_session cannot be None
anoadragon453 3336902
Just added the endpoints, pulling in infra
anoadragon453 a7cd54e
Fill out ThreepidAddRestServlet
anoadragon453 088d6e4
Finish the bind endpoint servlet and remove _extract_items_from_creds…
anoadragon453 f3fbe5f
Add changelog
anoadragon453 8463b7d
Make user account deactivation remove bound 3pids not on the user acc…
anoadragon453 21ea59b
Just added the endpoints, pulling in infra
anoadragon453 b0a2c2e
Fill out ThreepidAddRestServlet
anoadragon453 113ebcf
Finish the bind endpoint servlet and remove _extract_items_from_creds…
anoadragon453 3a0b7f2
Remove id_server from POST /account/3pid/msisdn/requestToken
anoadragon453 b13db4a
Make sure these new endpoints aren't also on r0
anoadragon453 4206cfc
Fix wrong config option and delete double servlets
anoadragon453 8683acc
Correct pulling variables out of validation_session
anoadragon453 9066368
Make sure to yield
anoadragon453 c637f74
english
anoadragon453 d89152d
/account/3pid/add is email only for now
anoadragon453 4275980
Temporarily c/p /account/3pid to /account/3pid/add
anoadragon453 c1a676e
Address review comments
anoadragon453 8fd3581
Consolidate threepid adding functionality in /account/3pid, account/3…
anoadragon453 66944d7
lint
anoadragon453 3daf0de
typo
anoadragon453 e4bb5ab
submit_token got its trailing slash again
anoadragon453 abc6f20
remove cyclic dep
anoadragon453 ff9eca5
Forgot my darn yields
anoadragon453 b61b73a
Re-add error checking on threepid_from_creds
anoadragon453 4c784e9
address my own review comments
richvdh 93de39b
Update synapse/handlers/identity.py
richvdh e5f4041
Merge branch 'develop' into anoa/msc2290
richvdh 6b2f8d1
fix bad merge
richvdh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Implement new Client Server API endpoints `/account/3pid/add` and `/account/3pid/bind` as per [MSC2290](https://github.com/matrix-org/matrix-doc/pull/2290). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
HttpResponseException, | ||
SynapseError, | ||
) | ||
from synapse.config.emailconfig import ThreepidBehaviour | ||
from synapse.util.stringutils import random_string | ||
|
||
from ._base import BaseHandler | ||
|
@@ -44,36 +45,6 @@ def __init__(self, hs): | |
self.federation_http_client = hs.get_http_client() | ||
self.hs = hs | ||
|
||
def _extract_items_from_creds_dict(self, creds): | ||
""" | ||
Retrieve entries from a "credentials" dictionary | ||
|
||
Args: | ||
creds (dict[str, str]): Dictionary of credentials that contain the following keys: | ||
* client_secret|clientSecret: A unique secret str provided by the client | ||
* id_server|idServer: the domain of the identity server to query | ||
* id_access_token: The access token to authenticate to the identity | ||
server with. | ||
|
||
Returns: | ||
tuple(str, str, str|None): A tuple containing the client_secret, the id_server, | ||
and the id_access_token value if available. | ||
""" | ||
client_secret = creds.get("client_secret") or creds.get("clientSecret") | ||
if not client_secret: | ||
raise SynapseError( | ||
400, "No client_secret in creds", errcode=Codes.MISSING_PARAM | ||
) | ||
|
||
id_server = creds.get("id_server") or creds.get("idServer") | ||
if not id_server: | ||
raise SynapseError( | ||
400, "No id_server in creds", errcode=Codes.MISSING_PARAM | ||
) | ||
|
||
id_access_token = creds.get("id_access_token") | ||
return client_secret, id_server, id_access_token | ||
|
||
@defer.inlineCallbacks | ||
def threepid_from_creds(self, id_server, creds): | ||
""" | ||
|
@@ -108,36 +79,41 @@ def threepid_from_creds(self, id_server, creds): | |
|
||
url = id_server + "/_matrix/identity/api/v1/3pid/getValidated3pid" | ||
|
||
data = yield self.http_client.get_json(url, query_params) | ||
return data if "medium" in data else None | ||
try: | ||
data = yield self.http_client.get_json(url, query_params) | ||
return data if "medium" in data else None | ||
except HttpResponseException: | ||
logger.debug( | ||
"%s reported non-validated threepid: %s", | ||
self.hs.config.account_threepid_delegate_email, | ||
creds, | ||
) | ||
return None | ||
|
||
@defer.inlineCallbacks | ||
def bind_threepid(self, creds, mxid, use_v2=True): | ||
def bind_threepid( | ||
self, client_secret, sid, mxid, id_server, id_access_token=None, use_v2=True | ||
): | ||
"""Bind a 3PID to an identity server | ||
|
||
Args: | ||
creds (dict[str, str]): Dictionary of credentials that contain the following keys: | ||
* client_secret|clientSecret: A unique secret str provided by the client | ||
* id_server|idServer: the domain of the identity server to query | ||
* id_access_token: The access token to authenticate to the identity | ||
server with. Required if use_v2 is true | ||
client_secret (str): A unique secret provided by the client | ||
|
||
sid (str): The ID of the validation session | ||
|
||
mxid (str): The MXID to bind the 3PID to | ||
use_v2 (bool): Whether to use v2 Identity Service API endpoints | ||
|
||
id_server (str): The domain of the identity server to query | ||
|
||
id_access_token (str): The access token to authenticate to the identity | ||
server with, if necessary. Required if use_v2 is true | ||
|
||
use_v2 (bool): Whether to use v2 Identity Service API endpoints. Defaults to True | ||
|
||
Returns: | ||
Deferred[dict]: The response from the identity server | ||
""" | ||
logger.debug("binding threepid %r to %s", creds, mxid) | ||
|
||
client_secret, id_server, id_access_token = self._extract_items_from_creds_dict( | ||
creds | ||
) | ||
|
||
sid = creds.get("sid") | ||
if not sid: | ||
raise SynapseError( | ||
400, "No sid in three_pid_creds", errcode=Codes.MISSING_PARAM | ||
) | ||
logger.debug("Proxying threepid bind request for %s to %s", mxid, id_server) | ||
|
||
# If an id_access_token is not supplied, force usage of v1 | ||
if id_access_token is None: | ||
|
@@ -156,7 +132,6 @@ def bind_threepid(self, creds, mxid, use_v2=True): | |
data = yield self.http_client.post_json_get_json( | ||
bind_url, bind_data, headers=headers | ||
) | ||
logger.debug("bound threepid %r to %s", creds, mxid) | ||
|
||
# Remember where we bound the threepid | ||
yield self.store.add_user_bound_threepid( | ||
|
@@ -176,7 +151,10 @@ def bind_threepid(self, creds, mxid, use_v2=True): | |
return data | ||
|
||
logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", bind_url) | ||
return (yield self.bind_threepid(creds, mxid, use_v2=False)) | ||
res = yield self.bind_threepid( | ||
client_secret, sid, mxid, id_server, id_access_token, use_v2=False | ||
) | ||
return res | ||
|
||
@defer.inlineCallbacks | ||
def try_unbind_threepid(self, mxid, threepid): | ||
|
@@ -447,6 +425,64 @@ def requestMsisdnToken( | |
logger.info("Proxied requestToken failed: %r", e) | ||
raise e.to_synapse_error() | ||
|
||
@defer.inlineCallbacks | ||
def validate_threepid_session(self, client_secret, sid): | ||
"""Validates a threepid session with only the client secret and session ID | ||
Tries validating against any configured account_threepid_delegates as well as locally. | ||
|
||
Args: | ||
client_secret (str): A secret provided by the client | ||
|
||
sid (str): The ID of the session | ||
|
||
Returns: | ||
Dict[str, str|int] if validation was successful, otherwise None | ||
""" | ||
# We don't actually know which medium this 3PID is. Thus we first assume it's email, | ||
# and if validation fails we try msisdn | ||
validation_session = None | ||
|
||
# XXX: We shouldn't need to keep wrapping and unwrapping this value | ||
threepid_creds = {"client_secret": client_secret, "sid": sid} | ||
|
||
# We don't actually know which medium this 3PID is. Thus we first assume it's email, | ||
# and if validation fails we try msisdn | ||
validation_session = None | ||
|
||
# Try to validate as email | ||
if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE: | ||
# Ask our delegated email identity server | ||
validation_session = yield self.threepid_from_creds( | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.hs.config.account_threepid_delegate_email, threepid_creds | ||
) | ||
elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL: | ||
# Get a validated session matching these details | ||
validation_session = yield self.store.get_threepid_validation_session( | ||
"email", client_secret, sid=sid, validated=True | ||
) | ||
|
||
# Old versions of Sydent return a 200 http code even on a failed validation check. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this comment should be in threepid_from_creds now |
||
# Thus, in addition to the HttpResponseException check above (which checks for | ||
# non-200 errors), we need to make sure validation_session isn't actually an error, | ||
# identified by containing an "error" key | ||
# See https://github.com/matrix-org/sydent/issues/215 for details | ||
if validation_session and "error" not in validation_session: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and afaict the check on |
||
return validation_session | ||
|
||
# Try to validate as msisdn | ||
if self.hs.config.account_threepid_delegate_msisdn: | ||
# Ask our delegated msisdn identity server | ||
validation_session = yield self.threepid_from_creds( | ||
self.hs.config.account_threepid_delegate_msisdn, threepid_creds | ||
) | ||
|
||
# Check that validation_session isn't actually an error due to old Sydent instances | ||
# See explanatory comment above | ||
if validation_session and "error" not in validation_session: | ||
return validation_session | ||
|
||
return None | ||
|
||
|
||
def create_id_access_token_header(id_access_token): | ||
"""Create an Authorization header for passing to SimpleHttpClient as the header value | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll address this in a future PR