Implement MSC3952: Intentional mentions (#14823)

MSC3952 defines push rules which searches for mentions in a list of
Matrix IDs in the event body, instead of searching the entire event
body for display name / local part.

This is implemented behind an experimental configuration flag and
does not yet implement the backwards compatibility pieces of the MSC.
This commit is contained in:
Patrick Cloke 2023-01-27 10:16:21 -05:00 committed by GitHub
parent faecc6c083
commit 2a51f3ec36
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 263 additions and 11 deletions

View File

@ -0,0 +1 @@
Experimental support for [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952): intentional mentions.

View File

@ -131,6 +131,14 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default: true, default: true,
default_enabled: true, default_enabled: true,
}, },
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mentioned"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::IsUserMention)]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]),
default: true,
default_enabled: true,
},
PushRule { PushRule {
rule_id: Cow::Borrowed("global/override/.m.rule.contains_display_name"), rule_id: Cow::Borrowed("global/override/.m.rule.contains_display_name"),
priority_class: 5, priority_class: 5,
@ -139,6 +147,19 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default: true, default: true,
default_enabled: true, default_enabled: true,
}, },
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mentioned"),
priority_class: 5,
conditions: Cow::Borrowed(&[
Condition::Known(KnownCondition::IsRoomMention),
Condition::Known(KnownCondition::SenderNotificationPermission {
key: Cow::Borrowed("room"),
}),
]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]),
default: true,
default_enabled: true,
},
PushRule { PushRule {
rule_id: Cow::Borrowed("global/override/.m.rule.roomnotif"), rule_id: Cow::Borrowed("global/override/.m.rule.roomnotif"),
priority_class: 5, priority_class: 5,

View File

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
use std::collections::BTreeMap; use std::collections::{BTreeMap, BTreeSet};
use anyhow::{Context, Error}; use anyhow::{Context, Error};
use lazy_static::lazy_static; use lazy_static::lazy_static;
@ -68,6 +68,11 @@ pub struct PushRuleEvaluator {
/// The "content.body", if any. /// The "content.body", if any.
body: String, body: String,
/// The user mentions that were part of the message.
user_mentions: BTreeSet<String>,
/// True if the message is a room message.
room_mention: bool,
/// The number of users in the room. /// The number of users in the room.
room_member_count: u64, room_member_count: u64,
@ -100,6 +105,8 @@ impl PushRuleEvaluator {
#[new] #[new]
pub fn py_new( pub fn py_new(
flattened_keys: BTreeMap<String, String>, flattened_keys: BTreeMap<String, String>,
user_mentions: BTreeSet<String>,
room_mention: bool,
room_member_count: u64, room_member_count: u64,
sender_power_level: Option<i64>, sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>, notification_power_levels: BTreeMap<String, i64>,
@ -116,6 +123,8 @@ impl PushRuleEvaluator {
Ok(PushRuleEvaluator { Ok(PushRuleEvaluator {
flattened_keys, flattened_keys,
body, body,
user_mentions,
room_mention,
room_member_count, room_member_count,
notification_power_levels, notification_power_levels,
sender_power_level, sender_power_level,
@ -229,6 +238,14 @@ impl PushRuleEvaluator {
KnownCondition::RelatedEventMatch(event_match) => { KnownCondition::RelatedEventMatch(event_match) => {
self.match_related_event_match(event_match, user_id)? self.match_related_event_match(event_match, user_id)?
} }
KnownCondition::IsUserMention => {
if let Some(uid) = user_id {
self.user_mentions.contains(uid)
} else {
false
}
}
KnownCondition::IsRoomMention => self.room_mention,
KnownCondition::ContainsDisplayName => { KnownCondition::ContainsDisplayName => {
if let Some(dn) = display_name { if let Some(dn) = display_name {
if !dn.is_empty() { if !dn.is_empty() {
@ -424,6 +441,8 @@ fn push_rule_evaluator() {
flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string()); flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string());
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
BTreeMap::new(), BTreeMap::new(),
@ -449,6 +468,8 @@ fn test_requires_room_version_supports_condition() {
let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()]; let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()];
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
BTreeMap::new(), BTreeMap::new(),
@ -483,7 +504,7 @@ fn test_requires_room_version_supports_condition() {
}; };
let rules = PushRules::new(vec![custom_rule]); let rules = PushRules::new(vec![custom_rule]);
result = evaluator.run( result = evaluator.run(
&FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true), &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
None, None,
None, None,
); );

View File

@ -269,6 +269,10 @@ pub enum KnownCondition {
EventMatch(EventMatchCondition), EventMatch(EventMatchCondition),
#[serde(rename = "im.nheko.msc3664.related_event_match")] #[serde(rename = "im.nheko.msc3664.related_event_match")]
RelatedEventMatch(RelatedEventMatchCondition), RelatedEventMatch(RelatedEventMatchCondition),
#[serde(rename = "org.matrix.msc3952.is_user_mention")]
IsUserMention,
#[serde(rename = "org.matrix.msc3952.is_room_mention")]
IsRoomMention,
ContainsDisplayName, ContainsDisplayName,
RoomMemberCount { RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
@ -414,6 +418,7 @@ pub struct FilteredPushRules {
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
} }
#[pymethods] #[pymethods]
@ -425,6 +430,7 @@ impl FilteredPushRules {
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
) -> Self { ) -> Self {
Self { Self {
push_rules, push_rules,
@ -432,6 +438,7 @@ impl FilteredPushRules {
msc1767_enabled, msc1767_enabled,
msc3381_polls_enabled, msc3381_polls_enabled,
msc3664_enabled, msc3664_enabled,
msc3952_intentional_mentions,
} }
} }
@ -465,6 +472,11 @@ impl FilteredPushRules {
return false; return false;
} }
if !self.msc3952_intentional_mentions && rule.rule_id.contains("org.matrix.msc3952")
{
return false;
}
true true
}) })
.map(|r| { .map(|r| {
@ -522,6 +534,28 @@ fn test_deserialize_unstable_msc3931_condition() {
)); ));
} }
#[test]
fn test_deserialize_unstable_msc3952_user_condition() {
let json = r#"{"kind":"org.matrix.msc3952.is_user_mention"}"#;
let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::IsUserMention)
));
}
#[test]
fn test_deserialize_unstable_msc3952_room_condition() {
let json = r#"{"kind":"org.matrix.msc3952.is_room_mention"}"#;
let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::IsRoomMention)
));
}
#[test] #[test]
fn test_deserialize_custom_condition() { fn test_deserialize_custom_condition() {
let json = r#"{"kind":"custom_tag"}"#; let json = r#"{"kind":"custom_tag"}"#;

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Tuple, Union from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Set, Tuple, Union
from synapse.types import JsonDict from synapse.types import JsonDict
@ -46,6 +46,7 @@ class FilteredPushRules:
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
): ... ): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
@ -55,6 +56,8 @@ class PushRuleEvaluator:
def __init__( def __init__(
self, self,
flattened_keys: Mapping[str, str], flattened_keys: Mapping[str, str],
user_mentions: Set[str],
room_mention: bool,
room_member_count: int, room_member_count: int,
sender_power_level: Optional[int], sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int], notification_power_levels: Mapping[str, int],

View File

@ -233,6 +233,9 @@ class EventContentFields:
# The authorising user for joining a restricted room. # The authorising user for joining a restricted room.
AUTHORISING_USER: Final = "join_authorised_via_users_server" AUTHORISING_USER: Final = "join_authorised_via_users_server"
# Use for mentioning users.
MSC3952_MENTIONS: Final = "org.matrix.msc3952.mentions"
# an unspecced field added to to-device messages to identify them uniquely-ish # an unspecced field added to to-device messages to identify them uniquely-ish
TO_DEVICE_MSGID: Final = "org.matrix.msgid" TO_DEVICE_MSGID: Final = "org.matrix.msgid"

View File

@ -168,3 +168,8 @@ class ExperimentalConfig(Config):
# MSC3925: do not replace events with their edits # MSC3925: do not replace events with their edits
self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False) self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False)
# MSC3952: Intentional mentions
self.msc3952_intentional_mentions = experimental.get(
"msc3952_intentional_mentions", False
)

View File

@ -22,13 +22,20 @@ from typing import (
List, List,
Mapping, Mapping,
Optional, Optional,
Set,
Tuple, Tuple,
Union, Union,
) )
from prometheus_client import Counter from prometheus_client import Counter
from synapse.api.constants import MAIN_TIMELINE, EventTypes, Membership, RelationTypes from synapse.api.constants import (
MAIN_TIMELINE,
EventContentFields,
EventTypes,
Membership,
RelationTypes,
)
from synapse.api.room_versions import PushRuleRoomFlag, RoomVersion from synapse.api.room_versions import PushRuleRoomFlag, RoomVersion
from synapse.event_auth import auth_types_for_event, get_user_power_level from synapse.event_auth import auth_types_for_event, get_user_power_level
from synapse.events import EventBase, relation_from_event from synapse.events import EventBase, relation_from_event
@ -342,8 +349,24 @@ class BulkPushRuleEvaluator:
for user_id, level in notification_levels.items(): for user_id, level in notification_levels.items():
notification_levels[user_id] = int(level) notification_levels[user_id] = int(level)
# Pull out any user and room mentions.
mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
user_mentions: Set[str] = set()
room_mention = False
if isinstance(mentions, dict):
# Remove out any non-string items and convert to a set.
user_mentions_raw = mentions.get("user_ids")
if isinstance(user_mentions_raw, list):
user_mentions = set(
filter(lambda item: isinstance(item, str), user_mentions_raw)
)
# Room mention is only true if the value is exactly true.
room_mention = mentions.get("room") is True
evaluator = PushRuleEvaluator( evaluator = PushRuleEvaluator(
_flatten_dict(event, room_version=event.room_version), _flatten_dict(event, room_version=event.room_version),
user_mentions,
room_mention,
room_member_count, room_member_count,
sender_power_level, sender_power_level,
notification_levels, notification_levels,

View File

@ -89,6 +89,7 @@ def _load_rules(
msc1767_enabled=experimental_config.msc1767_enabled, msc1767_enabled=experimental_config.msc1767_enabled,
msc3664_enabled=experimental_config.msc3664_enabled, msc3664_enabled=experimental_config.msc3664_enabled,
msc3381_polls_enabled=experimental_config.msc3381_polls_enabled, msc3381_polls_enabled=experimental_config.msc3381_polls_enabled,
msc3952_intentional_mentions=experimental_config.msc3952_intentional_mentions,
) )
return filtered_rules return filtered_rules

View File

@ -12,10 +12,12 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Any
from unittest.mock import patch from unittest.mock import patch
from twisted.test.proto_helpers import MemoryReactor from twisted.test.proto_helpers import MemoryReactor
from synapse.api.constants import EventContentFields
from synapse.api.room_versions import RoomVersions from synapse.api.room_versions import RoomVersions
from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator
from synapse.rest import admin from synapse.rest import admin
@ -126,3 +128,89 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
# Ensure no actions are generated! # Ensure no actions are generated!
self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)])) self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)]))
bulk_evaluator._action_for_event_by_user.assert_not_called() bulk_evaluator._action_for_event_by_user.assert_not_called()
@override_config({"experimental_features": {"msc3952_intentional_mentions": True}})
def test_mentions(self) -> None:
"""Test the behavior of an event which includes invalid mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs)
sentinel = object()
def create_and_process(mentions: Any = sentinel) -> bool:
"""Returns true iff the `mentions` trigger an event push action."""
content = {}
if mentions is not sentinel:
content[EventContentFields.MSC3952_MENTIONS] = mentions
# Create a new message event which should cause a notification.
event, context = self.get_success(
self.event_creation_handler.create_event(
self.requester,
{
"type": "test",
"room_id": self.room_id,
"content": content,
"sender": f"@bob:{self.hs.hostname}",
},
)
)
# Ensure no actions are generated!
self.get_success(
bulk_evaluator.action_for_events_by_user([(event, context)])
)
# If any actions are generated for this event, return true.
result = self.get_success(
self.hs.get_datastores().main.db_pool.simple_select_list(
table="event_push_actions_staging",
keyvalues={"event_id": event.event_id},
retcols=("*",),
desc="get_event_push_actions_staging",
)
)
return len(result) > 0
# Not including the mentions field should not notify.
self.assertFalse(create_and_process())
# An empty mentions field should not notify.
self.assertFalse(create_and_process({}))
# Non-dict mentions should be ignored.
mentions: Any
for mentions in (None, True, False, 1, "foo", []):
self.assertFalse(create_and_process(mentions))
# A non-list should be ignored.
for mentions in (None, True, False, 1, "foo", {}):
self.assertFalse(create_and_process({"user_ids": mentions}))
# The Matrix ID appearing anywhere in the list should notify.
self.assertTrue(create_and_process({"user_ids": [self.alice]}))
self.assertTrue(create_and_process({"user_ids": ["@another:test", self.alice]}))
# Duplicate user IDs should notify.
self.assertTrue(create_and_process({"user_ids": [self.alice, self.alice]}))
# Invalid entries in the list are ignored.
self.assertFalse(create_and_process({"user_ids": [None, True, False, {}, []]}))
self.assertTrue(
create_and_process({"user_ids": [None, True, False, {}, [], self.alice]})
)
# Room mentions from those without power should not notify.
self.assertFalse(create_and_process({"room": True}))
# Room mentions from those with power should notify.
self.helper.send_state(
self.room_id,
"m.room.power_levels",
{"notifications": {"room": 0}},
self.token,
state_key="",
)
self.assertTrue(create_and_process({"room": True}))
# Invalid data should not notify.
for mentions in (None, False, 1, "foo", [], {}):
self.assertFalse(create_and_process({"room": mentions}))

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Dict, List, Optional, Union, cast from typing import Dict, List, Optional, Set, Union, cast
import frozendict import frozendict
@ -39,7 +39,12 @@ from tests.test_utils.event_injection import create_event, inject_member_event
class PushRuleEvaluatorTestCase(unittest.TestCase): class PushRuleEvaluatorTestCase(unittest.TestCase):
def _get_evaluator( def _get_evaluator(
self, content: JsonMapping, related_events: Optional[JsonDict] = None self,
content: JsonMapping,
*,
user_mentions: Optional[Set[str]] = None,
room_mention: bool = False,
related_events: Optional[JsonDict] = None,
) -> PushRuleEvaluator: ) -> PushRuleEvaluator:
event = FrozenEvent( event = FrozenEvent(
{ {
@ -57,13 +62,15 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
power_levels: Dict[str, Union[int, Dict[str, int]]] = {} power_levels: Dict[str, Union[int, Dict[str, int]]] = {}
return PushRuleEvaluator( return PushRuleEvaluator(
_flatten_dict(event), _flatten_dict(event),
user_mentions or set(),
room_mention,
room_member_count, room_member_count,
sender_power_level, sender_power_level,
cast(Dict[str, int], power_levels.get("notifications", {})), cast(Dict[str, int], power_levels.get("notifications", {})),
{} if related_events is None else related_events, {} if related_events is None else related_events,
True, related_event_match_enabled=True,
event.room_version.msc3931_push_features, room_version_feature_flags=event.room_version.msc3931_push_features,
True, msc3931_enabled=True,
) )
def test_display_name(self) -> None: def test_display_name(self) -> None:
@ -90,6 +97,51 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
# A display name with spaces should work fine. # A display name with spaces should work fine.
self.assertTrue(evaluator.matches(condition, "@user:test", "foo bar")) self.assertTrue(evaluator.matches(condition, "@user:test", "foo bar"))
def test_user_mentions(self) -> None:
"""Check for user mentions."""
condition = {"kind": "org.matrix.msc3952.is_user_mention"}
# No mentions shouldn't match.
evaluator = self._get_evaluator({})
self.assertFalse(evaluator.matches(condition, "@user:test", None))
# An empty set shouldn't match
evaluator = self._get_evaluator({}, user_mentions=set())
self.assertFalse(evaluator.matches(condition, "@user:test", None))
# The Matrix ID appearing anywhere in the mentions list should match
evaluator = self._get_evaluator({}, user_mentions={"@user:test"})
self.assertTrue(evaluator.matches(condition, "@user:test", None))
evaluator = self._get_evaluator(
{}, user_mentions={"@another:test", "@user:test"}
)
self.assertTrue(evaluator.matches(condition, "@user:test", None))
# Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
# since the BulkPushRuleEvaluator is what handles data sanitisation.
def test_room_mentions(self) -> None:
"""Check for room mentions."""
condition = {"kind": "org.matrix.msc3952.is_room_mention"}
# No room mention shouldn't match.
evaluator = self._get_evaluator({})
self.assertFalse(evaluator.matches(condition, None, None))
# Room mention should match.
evaluator = self._get_evaluator({}, room_mention=True)
self.assertTrue(evaluator.matches(condition, None, None))
# A room mention and user mention is valid.
evaluator = self._get_evaluator(
{}, user_mentions={"@another:test"}, room_mention=True
)
self.assertTrue(evaluator.matches(condition, None, None))
# Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
# since the BulkPushRuleEvaluator is what handles data sanitisation.
def _assert_matches( def _assert_matches(
self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None
) -> None: ) -> None:
@ -308,7 +360,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
}, },
} }
}, },
{ related_events={
"m.in_reply_to": { "m.in_reply_to": {
"event_id": "$parent_event_id", "event_id": "$parent_event_id",
"type": "m.room.message", "type": "m.room.message",
@ -408,7 +460,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
}, },
} }
}, },
{ related_events={
"m.in_reply_to": { "m.in_reply_to": {
"event_id": "$parent_event_id", "event_id": "$parent_event_id",
"type": "m.room.message", "type": "m.room.message",