From 4104058e830f2e99d09820014decf246d9ebc2aa Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 3 Jul 2026 13:09:16 +0100 Subject: [PATCH] Make MSC4388 (sign-in with QR code) PUT requests idempotent (#19808) --- changelog.d/19808.feature | 1 + rust/src/msc4388_rendezvous/mod.rs | 8 +++ rust/src/msc4388_rendezvous/session.rs | 22 ++++++ tests/rest/client/test_msc4388_rendezvous.py | 71 ++++++++++++++++++++ 4 files changed, 102 insertions(+) create mode 100644 changelog.d/19808.feature diff --git a/changelog.d/19808.feature b/changelog.d/19808.feature new file mode 100644 index 0000000000..69eca83681 --- /dev/null +++ b/changelog.d/19808.feature @@ -0,0 +1 @@ +Updated experimental support for [MSC4388: Secure out-of-band channel for sign in with QR](https://github.com/matrix-org/matrix-spec-proposals/pull/4388). diff --git a/rust/src/msc4388_rendezvous/mod.rs b/rust/src/msc4388_rendezvous/mod.rs index bc9463639f..2f1b004ec1 100644 --- a/rust/src/msc4388_rendezvous/mod.rs +++ b/rust/src/msc4388_rendezvous/mod.rs @@ -331,6 +331,14 @@ impl MSC4388RendezvousHandler { .ok_or_else(NotFoundError::new)?; if !session.sequence_token().eq(&sequence_token) { + // Allow clients to safely retry a PUT (e.g. after a network error) + // by accepting the previous sequence_token as long as the data + // being submitted matches what is currently stored. This makes + // PUTs idempotent without weakening the concurrent-write check. + if session.is_idempotent_retry(&sequence_token, &data) { + return Ok((200, session.put_response())); + } + return Err(SynapseError::new( StatusCode::CONFLICT, "sequence_token does not match".to_owned(), diff --git a/rust/src/msc4388_rendezvous/session.rs b/rust/src/msc4388_rendezvous/session.rs index 467d1b5baf..a4959a4f7f 100644 --- a/rust/src/msc4388_rendezvous/session.rs +++ b/rust/src/msc4388_rendezvous/session.rs @@ -25,6 +25,12 @@ use ulid::Ulid; pub struct Session { id: Ulid, hash: [u8; 32], + /// The hash from before the last `update`, if any. Used so that clients can + /// safely retry a PUT request (e.g. after a network error) without getting + /// a spurious 409 conflict: a PUT whose `sequence_token` matches this + /// previous hash and whose `data` matches the currently-stored data is + /// treated as an idempotent no-op. + previous_hash: Option<[u8; 32]>, data: String, last_modified: SystemTime, expires: SystemTime, @@ -86,6 +92,7 @@ impl Session { Self { id, hash, + previous_hash: None, data, expires: now + ttl, last_modified: now, @@ -99,11 +106,26 @@ impl Session { /// Update the session with new data and last modified time. pub fn update(&mut self, data: String, now: SystemTime) { + self.previous_hash = Some(self.hash); self.hash = Self::compute_hash(&data, now); self.data = data; self.last_modified = now; } + /// Returns true if a PUT with the given `sequence_token` and `data` should + /// be treated as an idempotent retry of the most recent update (i.e. the + /// token matches the hash from before the last update, and the data + /// already matches the currently-stored data). + pub fn is_idempotent_retry(&self, sequence_token: &str, data: &str) -> bool { + let Some(previous_hash) = self.previous_hash else { + return false; + }; + if data != self.data { + return false; + } + URL_SAFE_NO_PAD.encode(previous_hash) == sequence_token + } + /// Compute the hash of the data and timestamp. fn compute_hash(data: &str, now: SystemTime) -> [u8; 32] { let mut hasher = Sha256::new(); diff --git a/tests/rest/client/test_msc4388_rendezvous.py b/tests/rest/client/test_msc4388_rendezvous.py index 913a41dedb..e99aeee7a0 100644 --- a/tests/rest/client/test_msc4388_rendezvous.py +++ b/tests/rest/client/test_msc4388_rendezvous.py @@ -252,6 +252,77 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase): self.assertEqual(channel.code, 404) self.assertEqual(channel.json_body["errcode"], "M_NOT_FOUND") + @override_config( + { + "disable_registration": True, + "matrix_authentication_service": { + "enabled": True, + "secret": "secret_value", + "endpoint": "https://issuer", + }, + "experimental_features": { + "msc4388_mode": "open", + }, + } + ) + def test_rendezvous_put_is_idempotent(self) -> None: + """ + A PUT using the previous sequence_token but with data that already + matches what is currently stored should be treated as an idempotent + retry and succeed (rather than returning 409). This lets clients + safely retry a PUT after a network error without losing the session. + """ + channel = self.make_request( + "POST", + rz_endpoint, + {"data": "foo=bar"}, + access_token=None, + ) + self.assertEqual(channel.code, 200) + rendezvous_id = channel.json_body["id"] + initial_sequence_token = channel.json_body["sequence_token"] + session_endpoint = rz_endpoint + f"/{rendezvous_id}" + + # Perform an update. + channel = self.make_request( + "PUT", + session_endpoint, + {"sequence_token": initial_sequence_token, "data": "foo=baz"}, + access_token=None, + ) + self.assertEqual(channel.code, 200) + updated_sequence_token = channel.json_body["sequence_token"] + + # Replaying the same PUT with the previous (now-stale) sequence_token + # and matching data should succeed and return the current token. + channel = self.make_request( + "PUT", + session_endpoint, + {"sequence_token": initial_sequence_token, "data": "foo=baz"}, + access_token=None, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["sequence_token"], updated_sequence_token) + + # But replaying with the previous token and *different* data must + # still be rejected as a concurrent write. + channel = self.make_request( + "PUT", + session_endpoint, + {"sequence_token": initial_sequence_token, "data": "something=else"}, + access_token=None, + ) + self.assertEqual(channel.code, 409) + self.assertEqual( + channel.json_body["errcode"], "IO_ELEMENT_MSC4388_CONCURRENT_WRITE" + ) + + # The stored data should be unchanged. + channel = self.make_request("GET", session_endpoint, access_token=None) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body["data"], "foo=baz") + self.assertEqual(channel.json_body["sequence_token"], updated_sequence_token) + @override_config( { "disable_registration": True,