search

matrix-org / complement

Matrix compliance test suite
Apache License 2.0
61 stars 52 forks source link

Test the scope of a transaction IDs #613

Closed sandhose closed 1 year ago

sandhose commented 1 year ago

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.

This test will fail in Synapse because the behaviour does not match the spec, and it will fail in Dendrite because it does not implement refreshable access tokens. Let me know if I should at it to their respective blacklist?

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 commented 1 year ago

I've added a further test that checks the idempotency of requests.

hughns commented 1 year ago

@clokep Okay, I've removed the test entirely in https://github.com/matrix-org/complement/pull/613/commits/f829a9445899b10df5bdcbe3877e3ed8f45bed3a and we can add it back in another PR later on 👍

hughns commented 1 year ago

As per discussion in #synapse-dev:matrix.org I've pulled the definitely good tests out in to https://github.com/matrix-org/complement/pull/622 for separate review.

erikjohnston commented 1 year ago

Sorry, is this still requiring review?

hughns commented 1 year ago

@erikjohnston I've given this a severe rebase. It doesn't require review at the moment as the test does not pass for any implementation. Please can it be made draft?

hughns commented 1 year ago

This PR should be closed as MSC3970 has been accepted and so new tests are in https://github.com/matrix-org/complement/pull/637