Skip to content

Reapply push rules on the decrypted event source (PSG-1146) #8170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 7, 2023
Merged

Reapply push rules on the decrypted event source (PSG-1146) #8170

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b6ac915
Apply push rules on encrypted events
Feb 23, 2023
9d7758c
Reuse event.supportsNotification extension
Feb 24, 2023
b20d74a
Add changelog file
Feb 24, 2023
ccf971a
Update encrypted raw json with decrypted event content for event matc…
Mar 1, 2023
75559fb
Ensure the event is decrypted before applying push rules
Mar 1, 2023
891897f
Add additional tests
Mar 1, 2023
245ca13
Improve encryption check condition
Mar 1, 2023
084bd44
Update decryption result on the events from the sync response
Mar 2, 2023
4146b55
Merge develop into feature/fre/apply_push_rules_after_decryption
Mar 2, 2023
21c7fc3
Merge remote-tracking branch 'origin/develop' into feature/fre/apply_…
Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8170.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reapply local push rules after event decryption
4 changes: 2 additions & 2 deletions ...android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class EventMatchCondition(
override fun technicalDescription() = "'$key' matches '$pattern'"

fun isSatisfied(event: Event): Boolean {
// TODO encrypted events?
val rawJson = MoshiProvider.providesMoshi().adapter(Event::class.java).toJsonValue(event) as? Map<*, *>
val rawJson: Map<*, *> = event.mxDecryptionResult?.payload
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't looks correct to me. The key "path" is based on the full event content.
If you only take the payload it won't work as expected.
Eg the key content.body won't work because your raw json will be

{ "body": "xxxx" }

Instead of

{ "content" : {{ "body": "xxxx" }} "

What you would need to do is to take the rawJson, then in case of encrypted event (with a decryption result) you replace the typeand content with what is in event.mxDecryptionResult.

There is an existing test class for this condition you can add test to check that it's working as expected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated with your suggestion, I replace all the entries from the encrypted raw json with the decrypted one. Also, I had to ensure the event is decrypted before applying the push rules. It was not necessary when I tested last time but it seems that the event is not always decrypted at this moment (it was not the case currently).

?: MoshiProvider.providesMoshi().adapter(Event::class.java).toJsonValue(event) as? Map<*, *>
?: return false
val value = extractField(rawJson, key) ?: return false

Expand Down
12 changes: 4 additions & 8 deletions vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,15 @@ class NotifiableEventResolver @Inject constructor(
private val buildMeta: BuildMeta,
) {

private val nonEncryptedNotifiableEventTypes: List<String> =
listOf(EventType.MESSAGE) + EventType.POLL_START.values + EventType.POLL_END.values + EventType.STATE_ROOM_BEACON_INFO.values

suspend fun resolveEvent(event: Event, session: Session, isNoisy: Boolean): NotifiableEvent? {
val roomID = event.roomId ?: return null
val eventId = event.eventId ?: return null
if (event.getClearType() == EventType.STATE_ROOM_MEMBER) {
return resolveStateRoomEvent(event, session, canBeReplaced = false, isNoisy = isNoisy)
}
val timelineEvent = session.getRoom(roomID)?.getTimelineEvent(eventId) ?: return null
return when (event.getClearType()) {
in nonEncryptedNotifiableEventTypes,
EventType.ENCRYPTED -> {
return when {
event.supportsNotification() || event.type == EventType.ENCRYPTED -> {
resolveMessageEvent(timelineEvent, session, canBeReplaced = false, isNoisy = isNoisy)
}
else -> {
Expand Down Expand Up @@ -163,8 +159,8 @@ class NotifiableEventResolver @Inject constructor(
} else {
event.attemptToDecryptIfNeeded(session)
// only convert encrypted messages to NotifiableMessageEvents
when (event.root.getClearType()) {
in nonEncryptedNotifiableEventTypes -> {
when {
event.root.supportsNotification() -> {
val body = displayableEventFormatter.format(event, isDm = room.roomSummary()?.isDirect.orFalse(), appendAuthor = false).toString()
val roomName = room.roomSummary()?.displayName ?: ""
val senderDisplayName = event.senderInfo.disambiguatedDisplayName
Expand Down