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

Conversation

@turt2live
Copy link
Member

turt2live commented Aug 28, 2019

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 mentioned this pull request Aug 28, 2019
10 of 10 tasks complete
@turt2live turt2live force-pushed the travis/spec/is-auth branch from 62d22bb to fad9974 Aug 28, 2019
@turt2live turt2live requested a review from matrix-org/spec-core-team Aug 28, 2019
@richvdh

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member Author

turt2live commented Sep 4, 2019

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member Author

turt2live commented Sep 4, 2019

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

This comment has been minimized.

Copy link
Member

richvdh commented Sep 4, 2019

That sounds like a terrible idea :/

@turt2live

This comment has been minimized.

Copy link
Member Author

turt2live commented Sep 4, 2019

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member Author

turt2live commented Sep 5, 2019

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

@dbkr
dbkr approved these changes Sep 6, 2019
@jryans
jryans approved these changes Sep 6, 2019
Copy link
Member

jryans left a comment

This also looks reasonable to me. 😁

@turt2live turt2live merged commit f7e00b1 into master Sep 6, 2019
8 checks passed
8 checks passed
buildkite/matrix-doc Build #832 passed (1 minute, 20 seconds)
Details
ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details
@turt2live turt2live deleted the travis/spec/is-auth branch Sep 6, 2019
@jplatte jplatte mentioned this pull request Nov 14, 2019
1 of 21 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.