Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use the hashed v2 lookup API for 3PIDs #10556

Closed
lampholder opened this issue Aug 14, 2019 · 5 comments · Fixed by matrix-org/matrix-js-sdk#1021
Closed

Use the hashed v2 lookup API for 3PIDs #10556

lampholder opened this issue Aug 14, 2019 · 5 comments · Fixed by matrix-org/matrix-js-sdk#1021
Assignees
@turt2live
Labels
A-Identity-Server Privacy T-Enhancement

Comments

@lampholder
Copy link
Member

No description provided.

@lampholder
Copy link
Member Author

lampholder commented Aug 14, 2019 •
edited by jryans
Loading

This is being specced in MSC2134.

There's a GET /_matrix/identity/v2/hash_details endpoint on the IS that riot can call to see which hashing algorithms the server supports - Riot should use sha256 if the server supports it, otherwise it should use none if that's all the server supports (we should think about whether none requires any sort of special warning, though we only expect it to be used in niche cases).

Edited by @jryans to clarify that only the IS is involved.

@jryans jryans changed the title Actually use the hashed lookup API for threepids Use the hashed lookup API for 3PIDs Aug 15, 2019
@jryans
Copy link
Collaborator

jryans commented Aug 15, 2019

Riot Web recently added a usage of v1/bulk_lookup to support many lookups in Settings, but this can be replaced by v2/lookup (defined by this hashing MSC) which supports many lookups in one request.

@jryans jryans changed the title Use the hashed lookup API for 3PIDs Use the hashed v2 lookup API for 3PIDs Aug 15, 2019
This was referenced Aug 15, 2019
Closed
Closed
Closed
@dbkr dbkr self-assigned this Aug 20, 2019
@turt2live turt2live assigned turt2live and unassigned dbkr Aug 20, 2019
@turt2live
Copy link
Member

turt2live commented Aug 21, 2019 •
edited
Loading

Conclusions from the syncs today:

  • Theoretically we can drop lookup support in Riot, however we won't do that yet. We can possibly drop it because the HS does the things we want, and the other usages can be replaced with better APIs.
  • In future we can consider a 'bind status' API to help us in the areas where we currently abuse lookups (https://github.com/matrix-org/matrix-doc/issues/2239 | disconnecting IS, checking bindings, verifying a third party invite belonging to us)
  • We will keep using a lookup in AddressPicker for UX. We want display names and avatars if possible.

@turt2live turt2live mentioned this issue Aug 21, 2019
Closed
turt2live added a commit to matrix-org/matrix-js-sdk that referenced this issue Aug 21, 2019
@turt2live
Use the v2 (hashed) lookup for identity server queries
8a0ddc4 
Fixes element-hq/element-web#10556
Implements [MSC2134](matrix-org/matrix-spec-proposals#2134) with assistance from [MSC2140](matrix-org/matrix-spec-proposals#2140) for auth.

Note: this also changes all identity server requests to use JSON as a request body. URL encoded forms were allowed in v1 but deprecated in favour of JSON. v2 APIs do not allow URL encoded forms.
@turt2live turt2live mentioned this issue Aug 21, 2019
Merged
@turt2live
Copy link
Member

Implementation is done solely in the js-sdk: matrix-org/matrix-js-sdk#1021

Currently blocked on having an identity server to test against that doesn't 500 the request.

@turt2live
Copy link
Member

unblocked: we fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@turt2live turt2live

Labels
A-Identity-Server Privacy T-Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use the v2 (hashed) lookup for identity server queries matrix-org/matrix-js-sdk
4 participants
@jryans @dbkr @turt2live @lampholder