Use the v2 Identity Service API for lookups (MSC2134 + MSC2140) #5976
Conversation
…s' desired lookup algos
Co-Authored-By: Erik Johnston <erik@matrix.org>
synapse/handlers/identity.py
Outdated
| ) | ||
| except (HttpResponseException, ValueError) as e: | ||
| # Catch HttpResponseExcept for a non-200 response code | ||
| # Catch ValueError for non-JSON response body |
There was a problem hiding this comment.
if we're doing this, should we check for other exceptions like "failed to connect"?
There was a problem hiding this comment.
Hm, I think ValueError doesn't have a code attribute either, which would cause this code to fail.
Should we drop ValueError?
There was a problem hiding this comment.
Mm, yeah I'm leaning towards dropping ValueError since that's considered a different error from an old IS.
|
damn, wrong button |
There was a problem hiding this comment.
so this generally looks good, modulo my constant refrain of "the world is simpler with smaller PRs". I'm not going to insist on breaking them up, but I honestly think that it would be easier to keep track of what's done and what's not if PRs are more granular, so if you can stomach the extra work without stabbing me in the eye I think it would be worthwhile.
Otherwise, just a few nitpicks really.
synapse/handlers/identity.py
Outdated
| Args: | ||
| id_server (str): The server name (including protocol and port, if required) | ||
| of the identity server to use. | ||
| id_access_token (str): The access token to authenticate to the identity server with |
There was a problem hiding this comment.
do we allow this to be None, or not? currently we can pass in None sometimes...
There was a problem hiding this comment.
Theoretically _lookup_3pid_v2 should never be called if id_access_token is None, as hash_details is an authenticated endpoint.
anoadragon453
changed the title
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Yeah, I experienced this backporting some things, but didn't realize that it would be a review issue as well. Might be time for a new sticky note... |
synapse/handlers/room_member.py
Outdated
| # Extract information from hash_details | ||
| supported_lookup_algorithms = hash_details.get("algorithms") | ||
| lookup_pepper = hash_details.get("lookup_pepper") | ||
| if not supported_lookup_algorithms or not lookup_pepper: |
There was a problem hiding this comment.
isn't pepper optional if the algorithm is NONE ?
There was a problem hiding this comment.
Nope, it's always required. It's to make implementation easier.
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Yeah, tests will fail until matrix-org/sytest#697 is merged, which in turn relies on #6013 |
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
3PID invites require making a request to an identity server to check that the invited 3PID has an Matrix ID linked, and if so, what it is. These requests are being made on behalf of a user. The user will supply an identity server and an access token for that identity server. The homeserver will then forward this request with the access token (using an `Authorization` header) and, if the given identity server doesn't support v2 endpoints, will fall back to v1 (which doesn't require any access tokens). Requires: ~~#5976~~
This is a redo of #5897 but with
id_access_tokenaccepted.Implements MSC2134 plus Identity Service v2 authentication ala MSC2140.
Identity lookup-related functions were also moved from
RoomMemberHandlertoIdentityHandler.