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.

Sign up for GitHub

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

Jump to bottom

Clarify that refreshed access tokens don't invalidate the scope of txnid #1236

Merged
merged 1 commit into from
Sep 26, 2022
Merged

Clarify that refreshed access tokens don't invalidate the scope of txnid #1236

merged 1 commit into from
Sep 26, 2022

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Sep 12, 2022 •
edited by github-actions bot
Loading

@turt2live turt2live added the release-blocker Blocks the next release from happening label Sep 12, 2022
@turt2live turt2live marked this pull request as ready for review September 12, 2022 22:15
@turt2live turt2live requested a review from a team as a code owner September 12, 2022 22:15
@turt2live turt2live merged commit 6c6c602 into main Sep 26, 2022
@turt2live turt2live deleted the travis/fix/txnid branch September 26, 2022 20:39
This was referenced Sep 30, 2022
Closed
Closed
Merged
@uhoreg uhoreg mentioned this pull request Oct 4, 2022
Closed
@sandhose sandhose mentioned this pull request Feb 15, 2023
Closed
sandhose added a commit to sandhose/complement that referenced this pull request Feb 15, 2023
@sandhose
Test the scope of a transaction IDs
50b038d 
This adds two tests, which check the current spec behaviour of
transaction IDs, which are that they are scoped to a series of access
tokens, and not the device ID.

The first test highlight this behaviour, by logging in with refresh
token enabled, sending an event, using the refresh token and syncing
with the new access token. On the sync, the transaction ID should be
there, but currently in Synapse it is not.

The second test highlight that the transaction ID is not scoped to the
device ID, by logging in twice with the same device ID, sending an event
with the first access token, and syncing with the second access token.
In that case, the sync should not contain the transaction ID, but
I think it's the case in HS implementations which use the device ID to
scope the transaction IDs, like Conduit.

Related: matrix-org/matrix-spec#1133, matrix-org/matrix-spec#1236, matrix-org/synapse#13064 and matrix-org/synapse#13083
sandhose added a commit to sandhose/complement that referenced this pull request Feb 15, 2023
@sandhose
Test the scope of a transaction IDs
9f153c0 
This adds two tests, which check the current spec behaviour of
transaction IDs, which are that they are scoped to a series of access
tokens, and not the device ID.

The first test highlight this behaviour, by logging in with refresh
token enabled, sending an event, using the refresh token and syncing
with the new access token. On the sync, the transaction ID should be
there, but currently in Synapse it is not.

The second test highlight that the transaction ID is not scoped to the
device ID, by logging in twice with the same device ID, sending an event
with the first access token, and syncing with the second access token.
In that case, the sync should not contain the transaction ID, but
I think it's the case in HS implementations which use the device ID to
scope the transaction IDs, like Conduit.

Related: matrix-org/matrix-spec#1133, matrix-org/matrix-spec#1236, matrix-org/synapse#13064 and matrix-org/synapse#13083
sandhose added a commit to sandhose/complement that referenced this pull request Feb 16, 2023
@sandhose
Test the scope of a transaction IDs
d776cd1 
This adds two tests, which check the current spec behaviour of
transaction IDs, which are that they are scoped to a series of access
tokens, and not the device ID.

The first test highlight this behaviour, by logging in with refresh
token enabled, sending an event, using the refresh token and syncing
with the new access token. On the sync, the transaction ID should be
there, but currently in Synapse it is not.

The second test highlight that the transaction ID is not scoped to the
device ID, by logging in twice with the same device ID, sending an event
with the first access token, and syncing with the second access token.
In that case, the sync should not contain the transaction ID, but
I think it's the case in HS implementations which use the device ID to
scope the transaction IDs, like Conduit.

Related: matrix-org/matrix-spec#1133, matrix-org/matrix-spec#1236, matrix-org/synapse#13064 and matrix-org/synapse#13083
sandhose added a commit to sandhose/complement that referenced this pull request Feb 16, 2023
@sandhose
Test the scope of a transaction IDs
56838e8 
This adds two tests, which check the current spec behaviour of
transaction IDs, which are that they are scoped to a series of access
tokens, and not the device ID.

The first test highlight this behaviour, by logging in with refresh
token enabled, sending an event, using the refresh token and syncing
with the new access token. On the sync, the transaction ID should be
there, but currently in Synapse it is not.

The second test highlight that the transaction ID is not scoped to the
device ID, by logging in twice with the same device ID, sending an event
with the first access token, and syncing with the second access token.
In that case, the sync should not contain the transaction ID, but
I think it's the case in HS implementations which use the device ID to
scope the transaction IDs, like Conduit.

Related: matrix-org/matrix-spec#1133, matrix-org/matrix-spec#1236, matrix-org/synapse#13064 and matrix-org/synapse#13083
hughns pushed a commit to sandhose/complement that referenced this pull request Apr 18, 2023
@sandhose @hughns
Test the scope of a transaction IDs
fa67145 
This adds two tests, which check the current spec behaviour of
transaction IDs, which are that they are scoped to a series of access
tokens, and not the device ID.

The first test highlight this behaviour, by logging in with refresh
token enabled, sending an event, using the refresh token and syncing
with the new access token. On the sync, the transaction ID should be
there, but currently in Synapse it is not.

The second test highlight that the transaction ID is not scoped to the
device ID, by logging in twice with the same device ID, sending an event
with the first access token, and syncing with the second access token.
In that case, the sync should not contain the transaction ID, but
I think it's the case in HS implementations which use the device ID to
scope the transaction IDs, like Conduit.

Related: matrix-org/matrix-spec#1133, matrix-org/matrix-spec#1236, matrix-org/synapse#13064 and matrix-org/synapse#13083
clokep pushed a commit to clokep/matrix-spec that referenced this pull request May 3, 2023
@turt2live @clokep
Clarify that refreshed access tokens don't invalidate the scope of tx…
8e28f16 
…nid (matrix-org#1236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikjohnston erikjohnston erikjohnston approved these changes

Assignees
No one assigned
Labels
release-blocker Blocks the next release from happening
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scope transaction IDs to the (user, device_id) instead of the access token
2 participants
@turt2live @erikjohnston