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

Accept access_token instead of id_access_token on the IS, accept Authorization header #697

Merged
merged 2 commits into from
Sep 11, 2019

Conversation

anoadragon453
Copy link
Member

Synapse PR: matrix-org/synapse#6013 which has context

We now accept access_token instead of id_access_token and do so using the Authorization header instead of in the JSON body, as MSC2140 states.

@anoadragon453 anoadragon453 requested a review from a team September 10, 2019 17:59
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm otherwise, modulo the tests failing

lib/SyTest/Identity/Server.pm Show resolved Hide resolved
@anoadragon453
Copy link
Member Author

The tests will fail until we merge matrix-org/synapse#6013 unfortunately.

anoadragon453 added a commit to matrix-org/synapse that referenced this pull request Sep 11, 2019
Two things I missed while implementing [MSC2140](https://github.com/matrix-org/matrix-doc/pull/2140/files#diff-c03a26de5ac40fb532de19cb7fc2aaf7R80).

1. Access tokens should be provided to the identity server as `access_token`, not `id_access_token`, even though the homeserver may accept the tokens as `id_access_token`.
2. Access tokens must be sent to the identity server in a query parameter, the JSON body is not allowed.

We now send the access token as part of an `Authorization: ...` header, which fixes both things.

The breaking code was added in #5892

Sytest PR: matrix-org/sytest#697
@anoadragon453
Copy link
Member Author

And it's been merged!

@anoadragon453 anoadragon453 merged commit e242f69 into develop Sep 11, 2019
@anoadragon453 anoadragon453 deleted the anoa/id_access_token branch September 11, 2019 11:09
anoadragon453 added a commit to matrix-org/synapse that referenced this pull request Sep 13, 2019
Two things I missed while implementing [MSC2140](https://github.com/matrix-org/matrix-doc/pull/2140/files#diff-c03a26de5ac40fb532de19cb7fc2aaf7R80).

1. Access tokens should be provided to the identity server as `access_token`, not `id_access_token`, even though the homeserver may accept the tokens as `id_access_token`.
2. Access tokens must be sent to the identity server in a query parameter, the JSON body is not allowed.

We now send the access token as part of an `Authorization: ...` header, which fixes both things.

The breaking code was added in #5892

Sytest PR: matrix-org/sytest#697
anoadragon453 added a commit that referenced this pull request Mar 18, 2020
…ase-v1.4.0

* origin/release-v1.4.0: (36 commits)
  Improve logging for a failing test (#709)
  Make 3PID binding tests use /account/3pid/bind ala MSC2290 (#703)
  Use unstable prefix for 3PID unbind API
  Add support for handling email validation challenges (#707)
  do requestToken before adding a 3pid (#706)
  Configure synapse to use the test mail server (#705)
  Implement a mail server to help with 3pid testing (#704)
  federated_rooms_fixture (#701)
  Use Sytest develop for Dendrite's master branch (#700)
  Don't assume atomicity
  Fix flakiness due to create_room_synced (#702)
  Don't require an avatar_url
  Replace HOMESERVER_INFO incantations (#699)
  add some logging for flaky sync test (#698)
  Fix problems when using hash
  Accept access_token instead of id_access_token on the IS, accept Authorization header (#697)
  Use hash and handle variable sized final chunk
  Have the in-built identity server support v2 (#689)
  Add tests for 3PID /unbind API (#691)
  Fix typo
  ...
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