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.

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

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

Comments

@lampholder
Copy link
Member

commented Aug 14, 2019

No description provided.

@lampholder

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

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

This comment has been minimized.

Copy link
Member

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
@dbkr dbkr self-assigned this Aug 20, 2019
@dbkr dbkr added this to In Progress in Workflow Aug 20, 2019
@turt2live turt2live assigned turt2live and unassigned dbkr Aug 20, 2019
@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

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 (matrix-org/matrix-doc#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 added a commit to matrix-org/matrix-js-sdk that referenced this issue Aug 21, 2019
Fixes vector-im/riot-web#10556
Implements [MSC2134](matrix-org/matrix-doc#2134) with assistance from [MSC2140](matrix-org/matrix-doc#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

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

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 turt2live added blocked and removed blocked labels Aug 21, 2019
@turt2live

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

unblocked: we fixed it

@turt2live turt2live moved this from In Progress to In Review in Workflow Aug 21, 2019
Workflow automation moved this from In Review to In Test Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Workflow
In Test
4 participants
You can’t perform that action at this time.