From 77535a6dd213eabf7792c75dc9bb2ef5966d7c1e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 3 Jan 2023 18:52:05 +0000 Subject: [PATCH 1/8] Create a CSAPI.LoginUser interface method This method allows for creating a new device under the same user. This is a bit of a hacky step towards supporting multiple devices for the same user (which Complement doesn't do yet). --- internal/client/client.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/client/client.go b/internal/client/client.go index cd5a5b80..558e269e 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -333,6 +333,30 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck } } +// LoginUser will create a new device on the existing user. +func (c *CSAPI) LoginUser(t *testing.T, password string) (userID, accessToken, deviceID string) { + t.Helper() + reqBody := map[string]interface{}{ + "identifier": map[string]interface{}{ + "type": "m.id.user", + "user": c.UserID, + }, + "password": password, + "type": "m.login.password", + } + 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 + return userID, accessToken, deviceID +} + //RegisterUser will register the user with given parameters and // return user ID & access token, and fail the test on network error func (c *CSAPI) RegisterUser(t *testing.T, localpart, password string) (userID, accessToken, deviceID string) { From 8f80db5143755a4001bcc9aa3628e48312fc9a6e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 3 Jan 2023 18:52:52 +0000 Subject: [PATCH 2/8] Add tests for MSC3890 Again, a bit of a hack to support logging out another device. --- tests/msc3890_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/msc3890_test.go diff --git a/tests/msc3890_test.go b/tests/msc3890_test.go new file mode 100644 index 00000000..57b96009 --- /dev/null +++ b/tests/msc3890_test.go @@ -0,0 +1,87 @@ +//go:build msc3890 +// +build msc3890 + +// This file contains tests for local notification settings as +// defined by MSC3890, which you can read here: +// https://github.com/matrix-org/matrix-doc/pull/3890 + +package tests + +import ( + "testing" + "time" + + "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/client" + "github.com/matrix-org/complement/internal/match" + "github.com/matrix-org/complement/internal/must" + "github.com/tidwall/gjson" +) + +func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { + // Create a deployment with a single user + deployment := Deploy(t, b.BlueprintCleanHS) + defer deployment.Destroy(t) + + // Create a user which we can log in to multiple times + alicePassword := "hunter2" + alice := deployment.RegisterUser(t, "hs1", "alice", alicePassword, false) + + // Log in to another device on this user + _, secondDeviceAccessToken, secondDeviceID := alice.LoginUser(t, alicePassword) + secondDevice := &client.CSAPI{ + AccessToken: secondDeviceAccessToken, + BaseURL: deployment.HS["hs1"].BaseURL, + Client: client.NewLoggedClient(t, "hs1", nil), + SyncUntilTimeout: 5 * time.Second, + Debug: alice.Debug, + } + + accountDataType := "org.matrix.msc3890.local_notification_settings." + secondDeviceID + accountDataContent := map[string]interface{}{"is_silenced": true} + + // Test deleting global account data. + t.Run("Deleting a user's device should delete any local notification settings entries from their account data", func(t *testing.T) { + // Retrieve a sync token for this user + _, nextBatchToken := alice.MustSync( + t, + client.SyncReq{}, + ) + + // Using the first device, create some local notification settings in the user's account data for the second device. + alice.SetGlobalAccountData( + t, + accountDataType, + accountDataContent, + ) + + checkAccountDataContent := func(r gjson.Result) bool { + // Only listen for our test type + if r.Get("type").Str != accountDataType { + return false + } + content := r.Get("content") + + // Ensure the content of this account data type is as we expect + return match.JSONDeepEqual([]byte(content.Raw), accountDataContent) + } + + // Check that the content of the user account data for this type has been set successfully + alice.MustSyncUntil( + t, + client.SyncReq{ + Since: nextBatchToken, + }, + client.SyncGlobalAccountDataHas(checkAccountDataContent), + ) + + // Log out the second device + secondDevice.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "logout"}) + + // Using the first device, check that the local notification setting account data for the deleted device was removed. + res := alice.DoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) + must.MatchResponse(t, res, match.HTTPResponse{ + StatusCode: 404, + }) + }) +} \ No newline at end of file From 8b7e842825906dfc9a3b019ea42c75f9df7df449 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 3 Jan 2023 18:53:09 +0000 Subject: [PATCH 3/8] Small typo fix in msc3391_test.go --- tests/msc3391_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/msc3391_test.go b/tests/msc3391_test.go index c797bc50..e74f9703 100644 --- a/tests/msc3391_test.go +++ b/tests/msc3391_test.go @@ -84,7 +84,7 @@ func createUserAccountData(t *testing.T, c *client.CSAPI) { func(body []byte) error { if !match.JSONDeepEqual(body, testAccountDataContent) { return fmt.Errorf( - "Expected %s for room account data content when, got '%s'", + "Expected %s for room account data content, got '%s'", testAccountDataType, string(body), ) @@ -124,7 +124,7 @@ func createRoomAccountData(t *testing.T, c *client.CSAPI, roomID string) { func(body []byte) error { if !match.JSONDeepEqual(body, testAccountDataContent) { return fmt.Errorf( - "Expected %s for room account data content when, got '%s'", + "Expected %s for room account data content, got '%s'", testAccountDataType, string(body), ) From 04ab196a3d4ddd6d5c30f70583afbd8a3a4c94c2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 20 Dec 2022 13:00:49 +0000 Subject: [PATCH 4/8] Add flake.nix and flake.lock --- flake.lock | 40 ++++++++++++++++++++++++++++++++++++++++ flake.nix | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 flake.lock create mode 100644 flake.nix diff --git a/flake.lock b/flake.lock new file mode 100644 index 00000000..2aba659e --- /dev/null +++ b/flake.lock @@ -0,0 +1,40 @@ +{ + "nodes": { + "flake-utils": { + "locked": { + "lastModified": 1667395993, + "narHash": "sha256-nuEHfE/LcWyuSWnS8t12N1wc105Qtau+/OdUAjtQ0rA=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "5aed5285a952e0b949eb3ba02c12fa4fcfef535f", + "type": "github" + }, + "original": { + "id": "flake-utils", + "type": "indirect" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1671417167, + "narHash": "sha256-JkHam6WQOwZN1t2C2sbp1TqMv3TVRjzrdoejqfefwrM=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "bb31220cca6d044baa6dc2715b07497a2a7c4bc7", + "type": "github" + }, + "original": { + "id": "nixpkgs", + "type": "indirect" + } + }, + "root": { + "inputs": { + "flake-utils": "flake-utils", + "nixpkgs": "nixpkgs" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 00000000..d75b6cae --- /dev/null +++ b/flake.nix @@ -0,0 +1,37 @@ +{ + description = "A Golang development environment"; + + outputs = { + self, + nixpkgs, + flake-utils, + }: + flake-utils.lib.eachDefaultSystem + (system: let + pkgs = import nixpkgs { + inherit system; + }; + in { + devShells.default = pkgs.mkShell { + buildInputs = with pkgs; [ + # Native dependencies needed by Complement + olm + + go + + # Go language server + gopls + + # Go debugger + delve + + # Tools dependended on by VSCode Go Extension + gomodifytags + gotests + impl + go-outline + go-tools # includes `staticcheck` + ]; + }; + }); +} From ea2a0f73434ee84200c8e4cd2c198dbf61c4a4c7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 12 Jan 2023 17:32:03 +0000 Subject: [PATCH 5/8] Stricter checks for ensure the account data was deleted --- tests/msc3890_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/msc3890_test.go b/tests/msc3890_test.go index 57b96009..ca4ca6a4 100644 --- a/tests/msc3890_test.go +++ b/tests/msc3890_test.go @@ -74,14 +74,27 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { }, client.SyncGlobalAccountDataHas(checkAccountDataContent), ) + // Also check via the dedicated account data endpoint to ensure the similar check later is not 404'ing for some other reason. + // Using `MustDoFunc` ensures that the response code is 2xx. + res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) + must.MatchResponse(t, res, match.HTTPResponse{ + JSON: []match.JSON{ + match.JSONKeyEqual("is_silenced", true), + }, + }) // Log out the second device secondDevice.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "logout"}) // Using the first device, check that the local notification setting account data for the deleted device was removed. - res := alice.DoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) + res = alice.DoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: 404, + JSON: []match.JSON{ + // A 404 can be generated for missing endpoints as well (which would have an errcode of `M_UNRECOGNIZED`). + // Ensure we're getting the error we expect. + match.JSONKeyEqual("errcode", "M_NOT_FOUND"), + }, }) }) } \ No newline at end of file From cd70eb98d0ba71e3666a66358c1412a7c1ddb8d5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 12 Jan 2023 17:55:30 +0000 Subject: [PATCH 6/8] Create a deployment.LoginUser method than calls clients.LoginUser This is much cleaner - thanks for the suggestion Sean! --- internal/client/client.go | 6 +++--- internal/docker/deployment.go | 26 ++++++++++++++++++++++++++ tests/msc3890_test.go | 29 +++++++++++------------------ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 558e269e..0d9ce3c3 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -333,13 +333,13 @@ func (c *CSAPI) MustSyncUntil(t *testing.T, syncReq SyncReq, checks ...SyncCheck } } -// LoginUser will create a new device on the existing user. -func (c *CSAPI) LoginUser(t *testing.T, password string) (userID, accessToken, deviceID string) { +// LoginUser will log in to a homeserver and create a new device on an existing user. +func (c *CSAPI) LoginUser(t *testing.T, localpart, password string) (userID, accessToken, deviceID string) { t.Helper() reqBody := map[string]interface{}{ "identifier": map[string]interface{}{ "type": "m.id.user", - "user": c.UserID, + "user": localpart, }, "password": password, "type": "m.login.password", diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index cc0b53e1..b943f79a 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -118,6 +118,32 @@ func (d *Deployment) RegisterUser(t *testing.T, hsName, localpart, password stri return client } +// LoginUser within a homeserver and return an authenticatedClient. Fails the test if the hsName is not found. +func (d *Deployment) LoginUser(t *testing.T, hsName, localpart, password string) *client.CSAPI { + t.Helper() + dep, ok := d.HS[hsName] + if !ok { + t.Fatalf("Deployment.Client - HS name '%s' not found", hsName) + return nil + } + client := &client.CSAPI{ + BaseURL: dep.BaseURL, + Client: client.NewLoggedClient(t, hsName, nil), + SyncUntilTimeout: 5 * time.Second, + Debug: d.Deployer.debugLogging, + } + dep.CSAPIClients = append(dep.CSAPIClients, client) + userID, accessToken, deviceID := client.LoginUser(t, localpart, password) + + // remember the token so subsequent calls to deployment.Client return the user + dep.AccessTokens[userID] = accessToken + + client.UserID = userID + client.AccessToken = accessToken + client.DeviceID = deviceID + return client +} + // Restart a deployment. func (d *Deployment) Restart(t *testing.T) error { t.Helper() diff --git a/tests/msc3890_test.go b/tests/msc3890_test.go index ca4ca6a4..9cb4df4c 100644 --- a/tests/msc3890_test.go +++ b/tests/msc3890_test.go @@ -9,7 +9,6 @@ package tests import ( "testing" - "time" "github.com/matrix-org/complement/internal/b" "github.com/matrix-org/complement/internal/client" @@ -24,32 +23,26 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { defer deployment.Destroy(t) // Create a user which we can log in to multiple times + aliceLocalpart := "alice" alicePassword := "hunter2" - alice := deployment.RegisterUser(t, "hs1", "alice", alicePassword, false) + aliceDeviceOne := deployment.RegisterUser(t, "hs1", aliceLocalpart, alicePassword, false) // Log in to another device on this user - _, secondDeviceAccessToken, secondDeviceID := alice.LoginUser(t, alicePassword) - secondDevice := &client.CSAPI{ - AccessToken: secondDeviceAccessToken, - BaseURL: deployment.HS["hs1"].BaseURL, - Client: client.NewLoggedClient(t, "hs1", nil), - SyncUntilTimeout: 5 * time.Second, - Debug: alice.Debug, - } + aliceDeviceTwo := deployment.LoginUser(t, "hs1", aliceLocalpart, alicePassword) - accountDataType := "org.matrix.msc3890.local_notification_settings." + secondDeviceID + accountDataType := "org.matrix.msc3890.local_notification_settings." + aliceDeviceTwo.DeviceID accountDataContent := map[string]interface{}{"is_silenced": true} // Test deleting global account data. t.Run("Deleting a user's device should delete any local notification settings entries from their account data", func(t *testing.T) { // Retrieve a sync token for this user - _, nextBatchToken := alice.MustSync( + _, nextBatchToken := aliceDeviceOne.MustSync( t, client.SyncReq{}, ) // Using the first device, create some local notification settings in the user's account data for the second device. - alice.SetGlobalAccountData( + aliceDeviceOne.SetGlobalAccountData( t, accountDataType, accountDataContent, @@ -67,7 +60,7 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { } // Check that the content of the user account data for this type has been set successfully - alice.MustSyncUntil( + aliceDeviceOne.MustSyncUntil( t, client.SyncReq{ Since: nextBatchToken, @@ -76,7 +69,7 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { ) // Also check via the dedicated account data endpoint to ensure the similar check later is not 404'ing for some other reason. // Using `MustDoFunc` ensures that the response code is 2xx. - res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) + res := aliceDeviceOne.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", aliceDeviceOne.UserID, "account_data", accountDataType}) must.MatchResponse(t, res, match.HTTPResponse{ JSON: []match.JSON{ match.JSONKeyEqual("is_silenced", true), @@ -84,10 +77,10 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { }) // Log out the second device - secondDevice.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "logout"}) + aliceDeviceTwo.MustDoFunc(t, "POST", []string{"_matrix", "client", "v3", "logout"}) // Using the first device, check that the local notification setting account data for the deleted device was removed. - res = alice.DoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", alice.UserID, "account_data", accountDataType}) + res = aliceDeviceOne.DoFunc(t, "GET", []string{"_matrix", "client", "v3", "user", aliceDeviceOne.UserID, "account_data", accountDataType}) must.MatchResponse(t, res, match.HTTPResponse{ StatusCode: 404, JSON: []match.JSON{ @@ -97,4 +90,4 @@ func TestDeletingDeviceRemovesDeviceLocalNotificationSettings(t *testing.T) { }, }) }) -} \ No newline at end of file +} From 5042fb707bcd449d11c928fd4b4df9fafd64f321 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 13 Jan 2023 14:27:52 +0000 Subject: [PATCH 7/8] Do not overwrite deployment's record of a user in LoginUser --- internal/docker/deployment.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/docker/deployment.go b/internal/docker/deployment.go index b943f79a..effcd523 100644 --- a/internal/docker/deployment.go +++ b/internal/docker/deployment.go @@ -119,6 +119,7 @@ func (d *Deployment) RegisterUser(t *testing.T, hsName, localpart, password stri } // LoginUser within a homeserver and return an authenticatedClient. Fails the test if the hsName is not found. +// Note that this will not change the access token of the client that is returned by `deployment.Client`. func (d *Deployment) LoginUser(t *testing.T, hsName, localpart, password string) *client.CSAPI { t.Helper() dep, ok := d.HS[hsName] @@ -135,9 +136,6 @@ func (d *Deployment) LoginUser(t *testing.T, hsName, localpart, password string) dep.CSAPIClients = append(dep.CSAPIClients, client) userID, accessToken, deviceID := client.LoginUser(t, localpart, password) - // remember the token so subsequent calls to deployment.Client return the user - dep.AccessTokens[userID] = accessToken - client.UserID = userID client.AccessToken = accessToken client.DeviceID = deviceID From 71eae29edcfd40d6af84dc0d7262555c429f03ce Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 13 Jan 2023 14:29:09 +0000 Subject: [PATCH 8/8] Shoo flakes --- flake.lock | 40 ---------------------------------------- flake.nix | 37 ------------------------------------- 2 files changed, 77 deletions(-) delete mode 100644 flake.lock delete mode 100644 flake.nix diff --git a/flake.lock b/flake.lock deleted file mode 100644 index 2aba659e..00000000 --- a/flake.lock +++ /dev/null @@ -1,40 +0,0 @@ -{ - "nodes": { - "flake-utils": { - "locked": { - "lastModified": 1667395993, - "narHash": "sha256-nuEHfE/LcWyuSWnS8t12N1wc105Qtau+/OdUAjtQ0rA=", - "owner": "numtide", - "repo": "flake-utils", - "rev": "5aed5285a952e0b949eb3ba02c12fa4fcfef535f", - "type": "github" - }, - "original": { - "id": "flake-utils", - "type": "indirect" - } - }, - "nixpkgs": { - "locked": { - "lastModified": 1671417167, - "narHash": "sha256-JkHam6WQOwZN1t2C2sbp1TqMv3TVRjzrdoejqfefwrM=", - "owner": "NixOS", - "repo": "nixpkgs", - "rev": "bb31220cca6d044baa6dc2715b07497a2a7c4bc7", - "type": "github" - }, - "original": { - "id": "nixpkgs", - "type": "indirect" - } - }, - "root": { - "inputs": { - "flake-utils": "flake-utils", - "nixpkgs": "nixpkgs" - } - } - }, - "root": "root", - "version": 7 -} diff --git a/flake.nix b/flake.nix deleted file mode 100644 index d75b6cae..00000000 --- a/flake.nix +++ /dev/null @@ -1,37 +0,0 @@ -{ - description = "A Golang development environment"; - - outputs = { - self, - nixpkgs, - flake-utils, - }: - flake-utils.lib.eachDefaultSystem - (system: let - pkgs = import nixpkgs { - inherit system; - }; - in { - devShells.default = pkgs.mkShell { - buildInputs = with pkgs; [ - # Native dependencies needed by Complement - olm - - go - - # Go language server - gopls - - # Go debugger - delve - - # Tools dependended on by VSCode Go Extension - gomodifytags - gotests - impl - go-outline - go-tools # includes `staticcheck` - ]; - }; - }); -}