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

Spec the v2 IS auth APIs #2255

Merged
merged 11 commits into from
Sep 6, 2019
Merged

Spec the v2 IS auth APIs #2255

merged 11 commits into from
Sep 6, 2019

Conversation

turt2live
Copy link
Member

This is part 2 of many, as shown by #2253

It may be easier to review this commit by commit. See 'docs' status check for what this looks like in the spec.

This PR intentionally does not include terms of service handling to try and keep the PR reviewable.

Specs part of MSC2140.

@turt2live turt2live force-pushed the travis/spec/is-auth branch from 62d22bb to fad9974 Compare August 28, 2019 21:47
@turt2live turt2live requested a review from a team August 28, 2019 21:48
@richvdh
Copy link
Member

richvdh commented Sep 4, 2019

I've not had time to read this, but please can it specify the expected behaviour for the various endpoints when no id access token is given, with respect to fallback to v1 apis?

@turt2live
Copy link
Member Author

@richvdh upon reflection, the behaviour is a 400 bad request because the field is required. At least that's how I'm reading the MSC's requirement - @dbkr might want to fight me on that (please do). This is why it's listed as a breaking change.

@richvdh
Copy link
Member

richvdh commented Sep 4, 2019

ok, but how is a homeserver that supports both cs api 0.5 and cs api 0.6 supposed to behave when it gets a call to /invite (or whatever) without an id_access_token?

@turt2live
Copy link
Member Author

I'm not saying its the right answer, but the answer I have is that the homeserver can't support r0.5 and r0.6

@richvdh
Copy link
Member

richvdh commented Sep 4, 2019

That sounds like a terrible idea :/

@turt2live
Copy link
Member Author

The right answer is probably amending the MSC to clarify that homeservers can perform calls without it, if they want to. They'd just have to use the (now-deprecated) v1 IS API.

@dbkr
Copy link
Member

dbkr commented Sep 5, 2019

The MSC does say that, although I would argue here that if a client supplies id_server but not id_access_token, it is respecting r0.5 and not r0.6. Just because the spec mandates that a parameter must be supplied, doesn't mean a server must also mandate it: the server can allow it to be optional in order to be compatible with both r0.5 and r0.6.

@turt2live
Copy link
Member Author

I'll add a note to the spec to clarify that, just to avoid the future question.

Copy link
Contributor

@jryans jryans left a comment

Choose a reason for hiding this comment

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

This also looks reasonable to me. 😁

@turt2live turt2live merged commit f7e00b1 into master Sep 6, 2019
@turt2live turt2live deleted the travis/spec/is-auth branch September 6, 2019 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants