From 29ddb41fc54c6768178bd9d77ab32c5028ad68dc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 13 Jul 2023 07:15:29 -0400 Subject: [PATCH 1/4] Stabilize knock restricted tests (room v10). (#643) Use room version 10 instead of the unstable room version (and remove references to the MSC since it is now part of the spec). --- ...c3787_test.go => knock_restricted_test.go} | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) rename tests/{msc3787_test.go => knock_restricted_test.go} (70%) diff --git a/tests/msc3787_test.go b/tests/knock_restricted_test.go similarity index 70% rename from tests/msc3787_test.go rename to tests/knock_restricted_test.go index 6c8c7272..bdcc0dd7 100644 --- a/tests/msc3787_test.go +++ b/tests/knock_restricted_test.go @@ -2,8 +2,7 @@ // +build msc3787 // This file contains tests for a join rule which mixes concepts of restricted joins -// and knocking. This is currently experimental and defined by MSC3787, found here: -// https://github.com/matrix-org/matrix-spec-proposals/pull/3787 +// and knocking. This is implemented in room version 10. // // Generally, this is a combination of knocking_test and restricted_rooms_test. @@ -16,18 +15,18 @@ import ( ) var ( - msc3787RoomVersion = "org.matrix.msc3787" - msc3787JoinRule = "knock_restricted" + roomVersion = "10" + joinRule = "knock_restricted" ) // See TestKnocking func TestKnockingInMSC3787Room(t *testing.T) { - doTestKnocking(t, msc3787RoomVersion, msc3787JoinRule) + doTestKnocking(t, roomVersion, joinRule) } // See TestKnockRoomsInPublicRoomsDirectory func TestKnockRoomsInPublicRoomsDirectoryInMSC3787Room(t *testing.T) { - doTestKnockRoomsInPublicRoomsDirectory(t, msc3787RoomVersion, msc3787JoinRule) + doTestKnockRoomsInPublicRoomsDirectory(t, roomVersion, joinRule) } // See TestCannotSendKnockViaSendKnock @@ -35,7 +34,7 @@ func TestCannotSendKnockViaSendKnockInMSC3787Room(t *testing.T) { testValidationForSendMembershipEndpoint(t, "/_matrix/federation/v1/send_knock", "knock", map[string]interface{}{ "preset": "public_chat", - "room_version": msc3787RoomVersion, + "room_version": roomVersion, }, ) } @@ -46,13 +45,13 @@ func TestRestrictedRoomsLocalJoinInMSC3787Room(t *testing.T) { defer deployment.Destroy(t) // Setup the user, allowed room, and restricted room. - alice, allowed_room, room := setupRestrictedRoom(t, deployment, msc3787RoomVersion, msc3787JoinRule) + alice, allowed_room, room := setupRestrictedRoom(t, deployment, roomVersion, joinRule) // Create a second user on the same homeserver. bob := deployment.Client(t, "hs1", "@bob:hs1") // Execute the checks. - checkRestrictedRoom(t, alice, bob, allowed_room, room, msc3787JoinRule) + checkRestrictedRoom(t, alice, bob, allowed_room, room, joinRule) } // See TestRestrictedRoomsRemoteJoin @@ -61,21 +60,21 @@ func TestRestrictedRoomsRemoteJoinInMSC3787Room(t *testing.T) { defer deployment.Destroy(t) // Setup the user, allowed room, and restricted room. - alice, allowed_room, room := setupRestrictedRoom(t, deployment, msc3787RoomVersion, msc3787JoinRule) + alice, allowed_room, room := setupRestrictedRoom(t, deployment, roomVersion, joinRule) // Create a second user on a different homeserver. bob := deployment.Client(t, "hs2", "@bob:hs2") // Execute the checks. - checkRestrictedRoom(t, alice, bob, allowed_room, room, msc3787JoinRule) + checkRestrictedRoom(t, alice, bob, allowed_room, room, joinRule) } // See TestRestrictedRoomsRemoteJoinLocalUser func TestRestrictedRoomsRemoteJoinLocalUserInMSC3787Room(t *testing.T) { - doTestRestrictedRoomsRemoteJoinLocalUser(t, msc3787RoomVersion, msc3787JoinRule) + doTestRestrictedRoomsRemoteJoinLocalUser(t, roomVersion, joinRule) } // See TestRestrictedRoomsRemoteJoinFailOver func TestRestrictedRoomsRemoteJoinFailOverInMSC3787Room(t *testing.T) { - doTestRestrictedRoomsRemoteJoinFailOver(t, msc3787RoomVersion, msc3787JoinRule) + doTestRestrictedRoomsRemoteJoinFailOver(t, roomVersion, joinRule) } From f032b6cef0e47d4222eb9028459c036ab8257d14 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 13 Jul 2023 15:08:38 -0400 Subject: [PATCH 2/4] Move knock restricted tests to Dendrite blacklist. (#644) --- tests/knock_restricted_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/knock_restricted_test.go b/tests/knock_restricted_test.go index bdcc0dd7..91d4d897 100644 --- a/tests/knock_restricted_test.go +++ b/tests/knock_restricted_test.go @@ -1,5 +1,5 @@ -//go:build msc3787 -// +build msc3787 +//go:build !dendrite_blacklist +// +build !dendrite_blacklist // This file contains tests for a join rule which mixes concepts of restricted joins // and knocking. This is implemented in room version 10. From b986a3034cf83aaede57b0978362e7901cbc0ed3 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 7 Aug 2023 06:04:13 -0400 Subject: [PATCH 3/4] Test for new transaction ID scope after MSC3970 was merged in Matrix 1.7 (#637) Co-authored-by: Patrick Cloke --- internal/client/client.go | 47 +++++++++++++++++++++ tests/csapi/txnid_test.go | 86 +++++++++++++++++++++++++++------------ 2 files changed, 106 insertions(+), 27 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 9dde9516..2accca90 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -459,6 +459,53 @@ func (c *CSAPI) LoginUser(t *testing.T, localpart, password string, opts ...Logi return userID, accessToken, deviceID } +// LoginUserWithRefreshToken will log in to a homeserver, with refresh token enabled, +// and create a new device on an existing user. +func (c *CSAPI) LoginUserWithRefreshToken(t *testing.T, localpart, password string) (userID, accessToken, refreshToken, deviceID string, expiresInMs int64) { + t.Helper() + reqBody := map[string]interface{}{ + "identifier": map[string]interface{}{ + "type": "m.id.user", + "user": localpart, + }, + "password": password, + "type": "m.login.password", + "refresh_token": true, + } + res := c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "login"}, WithJSONBody(t, reqBody)) + + body, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatalf("unable to read response body: %v", err) + } + + userID = gjson.GetBytes(body, "user_id").Str + accessToken = gjson.GetBytes(body, "access_token").Str + deviceID = gjson.GetBytes(body, "device_id").Str + refreshToken = gjson.GetBytes(body, "refresh_token").Str + expiresInMs = gjson.GetBytes(body, "expires_in_ms").Int() + return userID, accessToken, refreshToken, deviceID, expiresInMs +} + +// RefreshToken will consume a refresh token and return a new access token and refresh token. +func (c *CSAPI) ConsumeRefreshToken(t *testing.T, refreshToken string) (newAccessToken, newRefreshToken string, expiresInMs int64) { + t.Helper() + reqBody := map[string]interface{}{ + "refresh_token": refreshToken, + } + res := c.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "refresh"}, WithJSONBody(t, reqBody)) + + body, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatalf("unable to read response body: %v", err) + } + + newAccessToken = gjson.GetBytes(body, "access_token").Str + newRefreshToken = gjson.GetBytes(body, "refresh_token").Str + expiresInMs = gjson.GetBytes(body, "expires_in_ms").Int() + return newAccessToken, newRefreshToken, expiresInMs +} + // RegisterUser will register the user with given parameters and // return user ID, access token and device ID. It fails the test on network error. func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, accessToken, deviceID string) { diff --git a/tests/csapi/txnid_test.go b/tests/csapi/txnid_test.go index 01c055dd..5ff7f512 100644 --- a/tests/csapi/txnid_test.go +++ b/tests/csapi/txnid_test.go @@ -65,25 +65,9 @@ func mustHaveTransactionIDForEvent(t *testing.T, roomID, eventID, expectedTxnId }) } -func mustNotHaveTransactionIDForEvent(t *testing.T, roomID, eventID string) client.SyncCheckOpt { - return client.SyncTimelineHas(roomID, func(r gjson.Result) bool { - if r.Get("event_id").Str == eventID { - unsignedTxnId := r.Get("unsigned.transaction_id") - if unsignedTxnId.Exists() { - t.Fatalf("Event %s in room %s should NOT have a 'unsigned.transaction_id', but it did (%s)", eventID, roomID, unsignedTxnId.Str) - } - - return true - } - - return false - }) -} - -// TestTxnScopeOnLocalEcho tests that transaction IDs in the sync response are scoped to the "client session", not the device +// TestTxnScopeOnLocalEcho tests that transaction IDs in the sync response are scoped to the device func TestTxnScopeOnLocalEcho(t *testing.T) { - // Conduit scope transaction IDs to the device ID, not the access token. - runtime.SkipIf(t, runtime.Conduit) + runtime.SkipIf(t, runtime.Dendrite) deployment := Deploy(t, b.BlueprintCleanHS) defer deployment.Destroy(t) @@ -115,15 +99,14 @@ func TestTxnScopeOnLocalEcho(t *testing.T) { c2.UserID, c2.AccessToken, c2.DeviceID = c2.LoginUser(t, "alice", "password", client.WithDeviceID(c1.DeviceID)) must.EqualStr(t, c1.DeviceID, c2.DeviceID, "Device ID should be the same") - // When syncing, we should find the event and it should *not* have a transaction ID on the second client. - c2.MustSyncUntil(t, client.SyncReq{}, mustNotHaveTransactionIDForEvent(t, roomID, eventID)) + // When syncing, we should find the event and it should have the same transaction ID on the second client. + c2.MustSyncUntil(t, client.SyncReq{}, mustHaveTransactionIDForEvent(t, roomID, eventID, txnId)) } -// TestTxnIdempotencyScopedToClientSession tests that transaction IDs are scoped to a "client session" -// and behave as expected across multiple clients even if they use the same device ID -func TestTxnIdempotencyScopedToClientSession(t *testing.T) { - // Conduit scope transaction IDs to the device ID, not the client session. - runtime.SkipIf(t, runtime.Conduit) +// TestTxnIdempotencyScopedToDevice tests that transaction IDs are scoped to a device +// and behave as expected across multiple clients if they use the same device ID +func TestTxnIdempotencyScopedToDevice(t *testing.T) { + runtime.SkipIf(t, runtime.Dendrite) deployment := Deploy(t, b.BlueprintCleanHS) defer deployment.Destroy(t) @@ -156,8 +139,8 @@ func TestTxnIdempotencyScopedToClientSession(t *testing.T) { // send another event with the same txnId via the second client eventID2 := c2.SendEventUnsyncedWithTxnID(t, roomID, event, txnId) - // the two events should have different event IDs as they came from different clients - must.NotEqualStr(t, eventID2, eventID1, "Expected eventID1 and eventID2 to be different from two clients sharing the same device ID") + // the two events should have the same event IDs as they came from the same device + must.EqualStr(t, eventID2, eventID1, "Expected eventID1 and eventID2 to be the same from two clients sharing the same device ID") } // TestTxnIdempotency tests that PUT requests idempotency follows required semantics @@ -213,3 +196,52 @@ func TestTxnIdempotency(t *testing.T) { must.NotEqualStr(t, eventID4, eventID3, "Expected eventID4 and eventID3 to be different, but they were not") } + +// TestTxnIdWithRefreshToken tests that when a client refreshes its access token, +// it still gets back a transaction ID in the sync response and idempotency is respected. +func TestTxnIdWithRefreshToken(t *testing.T) { + // Dendrite and Conduit don't support refresh tokens yet. + runtime.SkipIf(t, runtime.Dendrite, runtime.Conduit) + + deployment := Deploy(t, b.BlueprintCleanHS) + defer deployment.Destroy(t) + + deployment.RegisterUser(t, "hs1", "alice", "password", false) + + c := deployment.Client(t, "hs1", "") + + var refreshToken string + c.UserID, c.AccessToken, refreshToken, c.DeviceID, _ = c.LoginUserWithRefreshToken(t, "alice", "password") + + // Create a room where we can send events. + roomID := c.CreateRoom(t, map[string]interface{}{}) + + txnId := "abcdef" + // We send an event + eventID1 := c.SendEventUnsyncedWithTxnID(t, roomID, b.Event{ + Type: "m.room.message", + Content: map[string]interface{}{ + "msgtype": "m.text", + "body": "first", + }, + }, txnId) + + // Use the refresh token to get a new access token. + c.AccessToken, refreshToken, _ = c.ConsumeRefreshToken(t, refreshToken) + + // When syncing, we should find the event and it should also have the correct transaction ID even + // though the access token is different. + c.MustSyncUntil(t, client.SyncReq{}, mustHaveTransactionIDForEvent(t, roomID, eventID1, txnId)) + + // We try sending the event again with the same transaction ID + eventID2 := c.SendEventUnsyncedWithTxnID(t, roomID, b.Event{ + Type: "m.room.message", + Content: map[string]interface{}{ + "msgtype": "m.text", + "body": "first", + }, + }, txnId) + + // The event should have been deduplicated and we should get back the same event ID + must.EqualStr(t, eventID2, eventID1, "Expected eventID1 and eventID2 to be the same from a client using a refresh token") +} From 93ccbc85c03d21ad26ed1051a534f277c738c30b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 18 Aug 2023 15:10:00 +0100 Subject: [PATCH 4/4] Fix flakey test due to incorrectly running in parallel (#647) --- tests/federation_upload_keys_test.go | 127 +++++++++++++-------------- 1 file changed, 61 insertions(+), 66 deletions(-) diff --git a/tests/federation_upload_keys_test.go b/tests/federation_upload_keys_test.go index f89471dd..b87220fc 100644 --- a/tests/federation_upload_keys_test.go +++ b/tests/federation_upload_keys_test.go @@ -48,77 +48,72 @@ func TestFederationKeyUploadQuery(t *testing.T) { }, }) - t.Run("Parallel", func(t *testing.T) { - // sytest: Can claim remote one time key using POST - t.Run("Can claim remote one time key using POST", func(t *testing.T) { - t.Parallel() - // check keys on remote server - reqBody = client.WithJSONBody(t, map[string]interface{}{ - "one_time_keys": map[string]interface{}{ - alice.UserID: map[string]string{ - alice.DeviceID: "signed_curve25519", - }, + // sytest: Can claim remote one time key using POST + t.Run("Can claim remote one time key using POST", func(t *testing.T) { + // check keys on remote server + reqBody = client.WithJSONBody(t, map[string]interface{}{ + "one_time_keys": map[string]interface{}{ + alice.UserID: map[string]string{ + alice.DeviceID: "signed_curve25519", }, - }) - resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "claim"}, reqBody) - otksField := "one_time_keys." + client.GjsonEscape(alice.UserID) + "." + client.GjsonEscape(alice.DeviceID) - must.MatchResponse(t, resp, match.HTTPResponse{ - StatusCode: http.StatusOK, - JSON: []match.JSON{ - match.JSONKeyTypeEqual(otksField, gjson.JSON), - match.JSONKeyEqual(otksField, oneTimeKeys), - }, - }) - - // there should be no OTK left now - resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "claim"}, reqBody) - must.MatchResponse(t, resp, match.HTTPResponse{ - StatusCode: http.StatusOK, - JSON: []match.JSON{ - match.JSONKeyMissing("one_time_keys." + client.GjsonEscape(alice.UserID)), - }, - }) + }, + }) + resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "claim"}, reqBody) + otksField := "one_time_keys." + client.GjsonEscape(alice.UserID) + "." + client.GjsonEscape(alice.DeviceID) + must.MatchResponse(t, resp, match.HTTPResponse{ + StatusCode: http.StatusOK, + JSON: []match.JSON{ + match.JSONKeyTypeEqual(otksField, gjson.JSON), + match.JSONKeyEqual(otksField, oneTimeKeys), + }, + }) + + // there should be no OTK left now + resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "claim"}, reqBody) + must.MatchResponse(t, resp, match.HTTPResponse{ + StatusCode: http.StatusOK, + JSON: []match.JSON{ + match.JSONKeyMissing("one_time_keys." + client.GjsonEscape(alice.UserID)), + }, }) + }) - // sytest: Can query remote device keys using POST - t.Run("Can query remote device keys using POST", func(t *testing.T) { - t.Parallel() - - displayName := "My new displayname" - body := client.WithJSONBody(t, map[string]interface{}{ - "display_name": displayName, - }) - alice.MustDoFunc(t, http.MethodPut, []string{"_matrix", "client", "v3", "devices", alice.DeviceID}, body) - // wait for bob to receive the displayname change - bob.MustSyncUntil(t, client.SyncReq{}, func(clientUserID string, topLevelSyncJSON gjson.Result) error { - devicesChanged := topLevelSyncJSON.Get("device_lists.changed") - if devicesChanged.Exists() { - for _, userID := range devicesChanged.Array() { - if userID.Str == alice.UserID { - return nil - } + // sytest: Can query remote device keys using POST + t.Run("Can query remote device keys using POST", func(t *testing.T) { + displayName := "My new displayname" + body := client.WithJSONBody(t, map[string]interface{}{ + "display_name": displayName, + }) + alice.MustDoFunc(t, http.MethodPut, []string{"_matrix", "client", "v3", "devices", alice.DeviceID}, body) + // wait for bob to receive the displayname change + bob.MustSyncUntil(t, client.SyncReq{}, func(clientUserID string, topLevelSyncJSON gjson.Result) error { + devicesChanged := topLevelSyncJSON.Get("device_lists.changed") + if devicesChanged.Exists() { + for _, userID := range devicesChanged.Array() { + if userID.Str == alice.UserID { + return nil } } - return fmt.Errorf("no device_lists found") - }) - reqBody = client.WithJSONBody(t, map[string]interface{}{ - "device_keys": map[string]interface{}{ - alice.UserID: []string{}, - }, - }) - resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "query"}, reqBody) - deviceKeysField := "device_keys." + client.GjsonEscape(alice.UserID) + "." + client.GjsonEscape(alice.DeviceID) - - must.MatchResponse(t, resp, match.HTTPResponse{ - StatusCode: http.StatusOK, - JSON: []match.JSON{ - match.JSONKeyTypeEqual(deviceKeysField, gjson.JSON), - match.JSONKeyEqual(deviceKeysField+".algorithms", deviceKeys["algorithms"]), - match.JSONKeyEqual(deviceKeysField+".keys", deviceKeys["keys"]), - match.JSONKeyEqual(deviceKeysField+".signatures", deviceKeys["signatures"]), - match.JSONKeyEqual(deviceKeysField+".unsigned.device_display_name", displayName), - }, - }) + } + return fmt.Errorf("no device_lists found") + }) + reqBody = client.WithJSONBody(t, map[string]interface{}{ + "device_keys": map[string]interface{}{ + alice.UserID: []string{}, + }, + }) + resp = bob.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "keys", "query"}, reqBody) + deviceKeysField := "device_keys." + client.GjsonEscape(alice.UserID) + "." + client.GjsonEscape(alice.DeviceID) + + must.MatchResponse(t, resp, match.HTTPResponse{ + StatusCode: http.StatusOK, + JSON: []match.JSON{ + match.JSONKeyTypeEqual(deviceKeysField, gjson.JSON), + match.JSONKeyEqual(deviceKeysField+".algorithms", deviceKeys["algorithms"]), + match.JSONKeyEqual(deviceKeysField+".keys", deviceKeys["keys"]), + match.JSONKeyEqual(deviceKeysField+".signatures", deviceKeys["signatures"]), + match.JSONKeyEqual(deviceKeysField+".unsigned.device_display_name", displayName), + }, }) }) }