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

Add support for IS v2 API with authentication #1002

Merged
merged 5 commits into from
Jul 30, 2019
Merged

Conversation

jryans
Copy link
Collaborator

@jryans jryans commented Jul 29, 2019

This adds support for the IS v2 API in all existing IS APIs. It also adds new support for request IS account tokens (which are new in the v2 API version).

This will fall back to v1 APIs if v2 is not supported for the moment, but this will eventually be removed once servers are updated.

Implements part of MSC2140
Fixes part of element-hq/element-web#10408

jryans added 3 commits July 29, 2019 13:15
This only updates the `/lookup` API so far. It also doesn't handle falling back
to v1.
This adds v2 support with fallback to other IS APIs in the SDK.
@turt2live
Copy link
Member

This will fall back to v1 APIs if v2 is not supported.

for outside readers: the plan is to not continue this fallback once servers are reasonably updated.

@jryans
Copy link
Collaborator Author

jryans commented Jul 29, 2019

This will fall back to v1 APIs if v2 is not supported.

for outside readers: the plan is to not continue this fallback once servers are reasonably updated.

Thanks, wasn't quite sure how to state that at the JS SDK level. Updated original comment to match.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

otherwise lgtm, mostly naming concerns

src/base-apis.js Outdated Show resolved Hide resolved
undefined, "POST", "/validate/email/requestToken",
params, httpApi.PREFIX_IDENTITY_V2, isAccessToken,
);
// TODO: Fold callback into above call once v1 path below is removed
Copy link
Member

Choose a reason for hiding this comment

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

we should also make sure to have a maintenance issue on riot-web to clean this fallback up at a future date. Not a blocker for this PR to be merged, but should be done sometime around the merge at latest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, good call. I filed element-hq/element-web#10443 for this. I'll link to it in the code as well.

@jryans jryans requested a review from turt2live July 30, 2019 09:39
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

woo!

@jryans jryans merged commit 2450d46 into develop Jul 30, 2019
@t3chguy t3chguy deleted the jryans/is-v2-auth branch May 10, 2022 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants