From eb6da20375804468c3eb278b73b72b98161ee9f4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Jan 2023 18:21:49 +0000 Subject: [PATCH 1/7] Add GetAllPushRules, GetPushRule, SetPushRule client helper methods Some helper methods to get and set push rules. --- internal/client/client.go | 71 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/internal/client/client.go b/internal/client/client.go index cd5a5b80..2a3ac5b2 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -188,6 +188,77 @@ func (c *CSAPI) SetRoomAccountData(t *testing.T, roomID string, eventType string return c.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "user", c.UserID, "rooms", roomID, "account_data", eventType}, WithJSONBody(t, content)) } +// GetAllPushRules fetches all configured push rules for a user from the homeserver. +// Push rules are returned as a parsed gjson result +// +// Example of printing the IDs of all underride rules of the current user: +// +// allPushRules := c.GetAllPushRules(t) +// globalUnderridePushRules := allPushRules.Get("global").Get("underride").Array() +// +// for index, rule := range globalUnderridePushRules { +// fmt.Printf("This rule's ID is: %s\n", rule.Get("rule_id").Str) +// } +// +// Push rules are returned in the same order received from the homeserver. +func (c *CSAPI) GetAllPushRules(t *testing.T) gjson.Result { + t.Helper() + + // We have to supply an empty string to the end of this path in order to generate a trailing slash. + // See https://github.com/matrix-org/matrix-spec/issues/457 + res := c.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "pushrules", ""}) + pushRulesBytes := ParseJSON(t, res) + return gjson.ParseBytes(pushRulesBytes) +} + +// GetPushRule queries the contents of a client's push rule by scope, kind and rule ID. +// A parsed gjson result is returned. Fails the test if the query to server returns a non-2xx status code. +// +// Example of checking that a global underride rule contains the expected actions: +// +// containsDisplayNameRule := c.GetPushRule(t, "global", "underride", ".m.rule.contains_display_name") +// must.MatchGJSON( +// t, +// containsDisplayNameRule, +// match.JSONKeyEqual("actions", []interface{}{ +// "notify", +// map[string]interface{}{"set_tweak": "sound", "value": "default"}, +// map[string]interface{}{"set_tweak": "highlight"}, +// }), +// ) +func (c *CSAPI) GetPushRule(t *testing.T, scope string, kind string, ruleID string) gjson.Result { + t.Helper() + + res := c.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "pushrules", scope, kind, ruleID}) + pushRuleBytes := ParseJSON(t, res) + return gjson.ParseBytes(pushRuleBytes) +} + +// SetPushRule creates a new push rule on the user, or modifies an existing one. +// `before` and `after` parameters can be provided, which if set will map to the +// `before` and `after` query parameters on the set push rules endpoint: +// https://spec.matrix.org/v1.5/client-server-api/#get_matrixclientv3pushrules. +// +// Example of setting a push rule with ID 'com.example.rule2' that must come after 'com.example.rule1': +// +// c.SetPushRule(t, "global", "underride", "com.example.rule2", map[string]interface{}{ +// "actions": []string{"dont_notify"}, +// }, nil, "com.example.rule1") +func (c *CSAPI) SetPushRule(t *testing.T, scope string, kind string, ruleID string, body map[string]interface{}, before *string, after *string) *http.Response { + t.Helper() + + // If the `before` or `after` arguments have been provided, construct same-named query parameters + queryParams := url.Values{} + if before != nil { + queryParams.Add("before", *before) + } + if after != nil { + queryParams.Add("after", *after) + } + + return c.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", scope, kind, ruleID}, WithJSONBody(t, body), WithQueries(queryParams)) +} + // SendEventSynced sends `e` into the room and waits for its event ID to come down /sync. // Returns the event ID of the sent event. func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string { From 9c590e015f8a1dbef95d01a3fd6039fd54fad4ca Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Jan 2023 18:22:17 +0000 Subject: [PATCH 2/7] Update push_test.go to use the new helpers --- tests/csapi/push_test.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index efcb6bcc..81157294 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -20,26 +20,16 @@ func TestPushRuleCacheHealth(t *testing.T) { alice := deployment.Client(t, "hs1", "@alice:hs1") - alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "sender", alice.UserID}, client.WithJSONBody(t, map[string]interface{}{ + // Set a global push rule + alice.SetPushRule(t, "global", "sender", alice.UserID, map[string]interface{}{ "actions": []string{"dont_notify"}, - })) + }, nil, nil) - // the extra "" is to make sure the submitted URL ends with a trailing slash - res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "pushrules", ""}) + // Fetch the rule once and check its contents + must.MatchGJSON(t, alice.GetAllPushRules(t), match.JSONKeyEqual("global.sender.0.actions.0", "dont_notify")) - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONKeyEqual("global.sender.0.actions.0", "dont_notify"), - }, - }) - - res = alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "v3", "pushrules", ""}) - - must.MatchResponse(t, res, match.HTTPResponse{ - JSON: []match.JSON{ - match.JSONKeyEqual("global.sender.0.actions.0", "dont_notify"), - }, - }) + // Fetch the rule and check its contents again. It should not have changed. + must.MatchGJSON(t, alice.GetAllPushRules(t), match.JSONKeyEqual("global.sender.0.actions.0", "dont_notify")) } func TestPushSync(t *testing.T) { From 93dd7ebdc600f5bb338951d345a853db03b0e797 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Jan 2023 18:22:38 +0000 Subject: [PATCH 3/7] Fix a small typo Found while traversing the codebase. --- tests/csapi/account_change_password_pushers_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/csapi/account_change_password_pushers_test.go b/tests/csapi/account_change_password_pushers_test.go index ab660ea5..3291fc88 100644 --- a/tests/csapi/account_change_password_pushers_test.go +++ b/tests/csapi/account_change_password_pushers_test.go @@ -59,7 +59,7 @@ func TestChangePasswordPushers(t *testing.T) { }) // sytest: Pushers created with a the same access token are not deleted on password change - t.Run("Pushers created with a the same access token are not deleted on password change", func(t *testing.T) { + t.Run("Pushers created with the same access token are not deleted on password change", func(t *testing.T) { reqBody := client.WithJSONBody(t, map[string]interface{}{ "data": map[string]interface{}{ "url": "https://dummy.url/_matrix/push/v1/notify", From eae51ad313185d5fefbe96fa545ec847632c2980 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 6 Jan 2023 18:33:07 +0000 Subject: [PATCH 4/7] Add tests for MSC3930 --- tests/msc3930_test.go | 182 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 tests/msc3930_test.go diff --git a/tests/msc3930_test.go b/tests/msc3930_test.go new file mode 100644 index 00000000..11395de4 --- /dev/null +++ b/tests/msc3930_test.go @@ -0,0 +1,182 @@ +//go:build msc3930 +// +build msc3930 + +// This file contains tests for "push rules of polls" as defined by MSC3930. +// The MSC that defines the design of the polls system is MSC3381. +// +// Note that implementation of MSC3381 is not required by the homeserver to +// pass the tests in this file. +// +// You can read either MSC using at the links below. +// Polls: https://github.com/matrix-org/matrix-doc/pull/3381 +// Push rules for polls: https://github.com/matrix-org/matrix-doc/pull/3930 + +package tests + +import ( + "math" + "testing" + + "github.com/matrix-org/complement/internal/b" + "github.com/matrix-org/complement/internal/match" + "github.com/matrix-org/complement/internal/must" +) + +const pollResponsePushRuleID = ".org.matrix.msc3930.rule.poll_response" +const pollStartOneToOneRuleID = ".org.matrix.msc3930.rule.poll_start_one_to_one" +const pollEndOneToOneRuleID = ".org.matrix.msc3930.rule.poll_end_one_to_one" +const pollStartRuleID = ".org.matrix.msc3930.rule.poll_start" +const pollEndRuleID = ".org.matrix.msc3930.rule.poll_end" + +func TestPollsLocalPushRules(t *testing.T) { + deployment := Deploy(t, b.BlueprintAlice) + defer deployment.Destroy(t) + + // Create a user to poll the push rules of. + aliceUserID := "@alice:hs1" + alice := deployment.Client(t, "hs1", aliceUserID) + + // Test for the presence of the expected push rules. Clients are expected + // to implement local matching of events based on the presented rules. + t.Run("Polls push rules are correctly presented to the client", func(t *testing.T) { + // Request each of the push rule IDs defined by MSC3930 and verify their structure. + + // This push rule silences all poll responses. + pollResponseRule := alice.GetPushRule(t, "global", "override", pollResponsePushRuleID) + must.MatchGJSON( + t, + pollResponseRule, + match.JSONKeyEqual("actions", []string{}), + match.JSONKeyEqual("default", true), + match.JSONKeyEqual("enabled", true), + // There should only be one condition defined for this type + match.JSONKeyEqual("conditions.#", 1), + // Check the contents of the first (and only) condition + match.JSONKeyEqual("conditions.0.kind", "event_match"), + match.JSONKeyEqual("conditions.0.key", "type"), + match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.response"), + ) + + // This push rule creates a sound when a poll is started in a one-to-one room. + pollStartOneToOneRule := alice.GetPushRule(t, "global", "underride", pollStartOneToOneRuleID) + must.MatchGJSON( + t, + pollStartOneToOneRule, + // Check that the appropriate actions are set for this rule + match.JSONKeyEqual("actions.#", 2), + match.JSONKeyEqual("actions", []interface{}{ + "notify", + map[string]interface{}{"set_tweak": "sound", "value": "default"}, + }, + ), + match.JSONKeyEqual("default", true), + match.JSONKeyEqual("enabled", true), + // There should two conditions defined for this type + match.JSONKeyEqual("conditions.#", 2), + // Check the condition that requires a room between two users + match.JSONKeyEqual("conditions.#(kind==\"room_member_count\").is", "2"), + // Check the condition that requires a poll start event + match.JSONKeyEqual("conditions.#(kind==\"event_match\").key", "type"), + match.JSONKeyEqual("conditions.#(kind==\"event_match\").pattern", "org.matrix.msc3381.poll.start"), + ) + + // This push rule creates a sound when a poll is ended in a one-to-one room. + pollEndOneToOneRule := alice.GetPushRule(t, "global", "underride", pollEndOneToOneRuleID) + must.MatchGJSON( + t, + pollEndOneToOneRule, + // Check that the appropriate actions are set for this rule + match.JSONKeyEqual("actions.#", 2), + match.JSONKeyEqual("actions", []interface{}{ + "notify", + map[string]interface{}{"set_tweak": "sound", "value": "default"}, + }, + ), + match.JSONKeyEqual("default", true), + match.JSONKeyEqual("enabled", true), + // There should two conditions defined for this type + match.JSONKeyEqual("conditions.#", 2), + // Check the condition that requires a room between two users + match.JSONKeyEqual("conditions.#(kind==\"room_member_count\").is", "2"), + // Check the condition that requires a poll start event + match.JSONKeyEqual("conditions.#(kind==\"event_match\").key", "type"), + match.JSONKeyEqual("conditions.#(kind==\"event_match\").pattern", "org.matrix.msc3381.poll.end"), + ) + + // This push rule creates a sound when a poll is started in any room. + pollStartRule := alice.GetPushRule(t, "global", "underride", pollStartRuleID) + must.MatchGJSON( + t, + pollStartRule, + // Check that the appropriate actions are set for this rule + match.JSONKeyEqual("actions", []string{"notify"}), + match.JSONKeyEqual("default", true), + match.JSONKeyEqual("enabled", true), + // There should only be one condition defined for this type + match.JSONKeyEqual("conditions.#", 1), + // Check the contents of the first (and only) condition + match.JSONKeyEqual("conditions.0.kind", "event_match"), + match.JSONKeyEqual("conditions.0.key", "type"), + match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.start"), + ) + + // This push rule creates a sound when a poll is ended in any room. + pollEndRule := alice.GetPushRule(t, "global", "underride", pollEndRuleID) + must.MatchGJSON( + t, + pollEndRule, + // Check that the appropriate actions are set for this rule + match.JSONKeyEqual("actions", []string{"notify"}), + match.JSONKeyEqual("default", true), + match.JSONKeyEqual("enabled", true), + // There should only be one condition defined for this type + match.JSONKeyEqual("conditions.#", 1), + // Check the contents of the first (and only) condition + match.JSONKeyEqual("conditions.0.kind", "event_match"), + match.JSONKeyEqual("conditions.0.key", "type"), + match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.end"), + ) + + // The DM-specific rules for poll start and poll end should come before the rules that + // define behaviour for any room. We verify this by ensuring that the DM-specific rules + // have a lower index when requesting all push rules. + allPushRules := alice.GetAllPushRules(t) + globalUnderridePushRules := allPushRules.Get("global").Get("underride").Array() + + pollStartOneToOneRuleIndex := math.MaxInt64 + pollEndOneToOneRuleIndex := math.MaxInt64 + for index, rule := range globalUnderridePushRules { + // Iterate over the user's global underride rules as a client would. + // If we come across a one-to-one room rule ID, we note down its index. + // When we come across the rule ID of its generic counterpart, we ensure + // its counterpart is at a higher index, and thus would be considered + // lower priority. + if rule.Get("rule_id").Str == pollStartOneToOneRuleID { + pollStartOneToOneRuleIndex = index + } else if rule.Get("rule_id").Str == pollEndOneToOneRuleID { + pollEndOneToOneRuleIndex = index + } else if rule.Get("rule_id").Str == pollStartRuleID { + if pollStartOneToOneRuleIndex > index { + t.Fatalf( + "Expected rule '%s' to come after '%s' in '%s's global underride rules", + pollStartRuleID, + pollStartOneToOneRuleID, + alice.UserID, + ) + } + } else if rule.Get("rule_id").Str == pollEndRuleID { + if pollEndOneToOneRuleIndex > index { + t.Fatalf( + "Expected rule '%s' to come after '%s' in '%s's global underride rules", + pollEndRuleID, + pollEndOneToOneRuleID, + alice.UserID, + ) + } + } + } + }) + + // TODO: Test whether the homeserver correctly calls POST /_matrix/push/v1/notify on the push gateway + // in accordance with the push rules. Blocked by Complement not having a push gateway implementation. +} From 8de70ccdbf4df70e7fbca63933e0ee0560fee3f8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 12 Jan 2023 18:52:18 +0000 Subject: [PATCH 5/7] Apply suggestions from code review --- tests/msc3930_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/msc3930_test.go b/tests/msc3930_test.go index 11395de4..eb9d309d 100644 --- a/tests/msc3930_test.go +++ b/tests/msc3930_test.go @@ -7,7 +7,7 @@ // Note that implementation of MSC3381 is not required by the homeserver to // pass the tests in this file. // -// You can read either MSC using at the links below. +// You can read either MSC using the links below. // Polls: https://github.com/matrix-org/matrix-doc/pull/3381 // Push rules for polls: https://github.com/matrix-org/matrix-doc/pull/3930 @@ -22,7 +22,7 @@ import ( "github.com/matrix-org/complement/internal/must" ) -const pollResponsePushRuleID = ".org.matrix.msc3930.rule.poll_response" +const pollResponseRuleID = ".org.matrix.msc3930.rule.poll_response" const pollStartOneToOneRuleID = ".org.matrix.msc3930.rule.poll_start_one_to_one" const pollEndOneToOneRuleID = ".org.matrix.msc3930.rule.poll_end_one_to_one" const pollStartRuleID = ".org.matrix.msc3930.rule.poll_start" @@ -42,7 +42,7 @@ func TestPollsLocalPushRules(t *testing.T) { // Request each of the push rule IDs defined by MSC3930 and verify their structure. // This push rule silences all poll responses. - pollResponseRule := alice.GetPushRule(t, "global", "override", pollResponsePushRuleID) + pollResponseRule := alice.GetPushRule(t, "global", "override", pollResponseRuleID) must.MatchGJSON( t, pollResponseRule, From d682dee581e035f09635b486baf74dac50f13a10 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 12 Jan 2023 19:01:26 +0000 Subject: [PATCH 6/7] Correct descriptions of push rules --- tests/msc3930_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/msc3930_test.go b/tests/msc3930_test.go index eb9d309d..a5c2a738 100644 --- a/tests/msc3930_test.go +++ b/tests/msc3930_test.go @@ -57,7 +57,7 @@ func TestPollsLocalPushRules(t *testing.T) { match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.response"), ) - // This push rule creates a sound when a poll is started in a one-to-one room. + // This push rule creates a sound and notifies the user when a poll is start in a one-to-one room. pollStartOneToOneRule := alice.GetPushRule(t, "global", "underride", pollStartOneToOneRuleID) must.MatchGJSON( t, @@ -80,7 +80,7 @@ func TestPollsLocalPushRules(t *testing.T) { match.JSONKeyEqual("conditions.#(kind==\"event_match\").pattern", "org.matrix.msc3381.poll.start"), ) - // This push rule creates a sound when a poll is ended in a one-to-one room. + // This push rule creates a sound and notifies the user when a poll is ended in a one-to-one room. pollEndOneToOneRule := alice.GetPushRule(t, "global", "underride", pollEndOneToOneRuleID) must.MatchGJSON( t, @@ -103,7 +103,7 @@ func TestPollsLocalPushRules(t *testing.T) { match.JSONKeyEqual("conditions.#(kind==\"event_match\").pattern", "org.matrix.msc3381.poll.end"), ) - // This push rule creates a sound when a poll is started in any room. + // This push rule notifies the user when a poll is started in any room. pollStartRule := alice.GetPushRule(t, "global", "underride", pollStartRuleID) must.MatchGJSON( t, @@ -120,7 +120,7 @@ func TestPollsLocalPushRules(t *testing.T) { match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.start"), ) - // This push rule creates a sound when a poll is ended in any room. + // This push rule notifies the user when a poll is ended in any room. pollEndRule := alice.GetPushRule(t, "global", "underride", pollEndRuleID) must.MatchGJSON( t, From d91030caac692aa81b34ebd1664119088cdda96a Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 18 Jan 2023 16:39:37 +0000 Subject: [PATCH 7/7] Update tests/msc3930_test.go Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- tests/msc3930_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/msc3930_test.go b/tests/msc3930_test.go index a5c2a738..04d6bb7f 100644 --- a/tests/msc3930_test.go +++ b/tests/msc3930_test.go @@ -57,7 +57,7 @@ func TestPollsLocalPushRules(t *testing.T) { match.JSONKeyEqual("conditions.0.pattern", "org.matrix.msc3381.poll.response"), ) - // This push rule creates a sound and notifies the user when a poll is start in a one-to-one room. + // This push rule creates a sound and notifies the user when a poll is started in a one-to-one room. pollStartOneToOneRule := alice.GetPushRule(t, "global", "underride", pollStartOneToOneRuleID) must.MatchGJSON( t,