Poll model - validate end events by kerryarchibald · Pull Request #3072 · matrix-org/matrix-js-sdk

kerryarchibald · 2023-01-17T03:19:29Z

~~Includes #3036~~

Repeated end events are ignored - only the first (valid) closure event by origin_server_ts is counted.

If a m.poll.end event is received from someone other than the poll creator or user with permission to redact other's messages in the room, the event must be ignored by clients due to being invalid.

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

Poll model - validate end events (#3072). Contributed by @kerryarchibald.

This reverts commit e578d84.

This reverts commit 515db7a.

…model

SimonBrandner · 2023-01-17T07:25:36Z

Any chance the base of this could be changed for easier review?

richvdh

this seems to be mixing in a bunch of changes to threads - is that deliberate?

src/models/poll.ts

kerryarchibald · 2023-01-26T02:26:48Z

@richvdh @SimonBrandner This is ready for another look!

src/models/poll.ts

richvdh · 2023-01-31T12:23:17Z

src/models/poll.ts

+        const endEventSender = endEvent.getSender();
+        return !!endEventSender && roomCurrentState.maySendRedactionForEvent(this.rootEvent, endEventSender);


this doesn't seem to be quite the same thing as the comment above. It's possible that endEventSender is the same as the sender of the root event, and yet they lack permission to redact that create event (for example, because sending m.room.redaction events has been disabled).

good catch, have added a sender === creator check here

…dless of redaction

Co-authored-by: Richard van der Hoff <[email protected]>

richvdh

lgtm otherwise

richvdh · 2023-02-01T11:43:31Z

src/models/poll.ts

-        return !!endEventSender && roomCurrentState.maySendRedactionForEvent(this.rootEvent, endEventSender);
+        return (
+            !!endEventSender &&
+            (endEventSender === this.matrixClient.getSafeUserId() ||


this is checking if the sender of the end event is the current user? Don't we want to check if it was the sender of the rootEvent?

Suggested change

(endEventSender === this.matrixClient.getSafeUserId() ||

(endEventSender === this.rootEvent.getSender() ||

🤦‍♀️ fixed and improved the tests to catch this. Thanks!

@kerryarchibald

* Element-R: implement encryption of outgoing events ([\matrix-org#3122](matrix-org#3122)). * Poll model - page /relations results ([\matrix-org#3073](matrix-org#3073)). Contributed by @kerryarchibald. * Poll model - validate end events ([\matrix-org#3072](matrix-org#3072)). Contributed by @kerryarchibald. * Handle optional last_known_event_id property in m.predecessor ([\matrix-org#3119](matrix-org#3119)). Contributed by @andybalaam. * Add support for stable identifier for fixed MAC in SAS verification ([\matrix-org#3101](matrix-org#3101)). * Provide eventId as well as roomId from Room.findPredecessor ([\matrix-org#3095](matrix-org#3095)). Contributed by @andybalaam. * MSC3946 Dynamic room predecessors ([\matrix-org#3042](matrix-org#3042)). Contributed by @andybalaam. * Poll model ([\matrix-org#3036](matrix-org#3036)). Contributed by @kerryarchibald. * Remove video tracks on video mute without renegotiating ([\matrix-org#3091](matrix-org#3091)). * Introduces a backwards-compatible API change. `MegolmEncrypter#prepareToEncrypt`'s return type has changed from `void` to `() => void`. ([\matrix-org#3035](matrix-org#3035)). Contributed by @clarkf. * Stop the ICE disconnected timer on call terminate ([\matrix-org#3147](matrix-org#3147)). * Clear notifications when we can infer read status from receipts ([\matrix-org#3139](matrix-org#3139)). Fixes element-hq/element-web#23991. * Messages sent out of order after one message fails ([\matrix-org#3131](matrix-org#3131)). Fixes element-hq/element-web#22885 and element-hq/element-web#18942. Contributed by @justjanne. * Element-R: fix a bug which prevented encryption working after a reload ([\matrix-org#3126](matrix-org#3126)). * Element-R: Fix invite processing ([\matrix-org#3121](matrix-org#3121)). * Don't throw with no `opponentDeviceInfo` ([\matrix-org#3107](matrix-org#3107)). * Remove flaky megolm test ([\matrix-org#3098](matrix-org#3098)). Contributed by @clarkf. * Fix "verifyLinks" functionality of getRoomUpgradeHistory ([\matrix-org#3089](matrix-org#3089)). Contributed by @andybalaam.

Kerry Archibald added 29 commits December 30, 2022 15:45

first cut poll model

5ca533c

process incoming poll relations

984e5eb

allow alt event types in relations model

e578d84

allow alt event types in relations model

1123f1d

remove unneccesary checks on remove relation

ea71efe

comment

dfe4038

Revert "allow alt event types in relations model"

515db7a

This reverts commit e578d84.

Revert "Revert "allow alt event types in relations model""

ae6ffb7

This reverts commit 515db7a.

Merge branch 'psg-1014/relations-alt-event-types' into psg-1014/poll-…

f189901

…model

basic handling for new poll relations

cace5d4

Merge branch 'develop' into psg-1014/poll-model

6b4f630

tests

0f7829a

test room.processPollEvents

32abfed

join processBeaconEvents and poll events in client

797123c

Merge branch 'develop' into psg-1014/poll-model

51896e1

Merge branch 'develop' into psg-1014/poll-model

ad49a00

tidy and set 23 copyrights

b158dcc

use rooms instance of matrixClient

924a6c0

tidy

d09700f

more copyright

9e0d95e

simplify processPollEvent code

b173130

Merge branch 'develop' into psg-1014/poll-model

15dd1c4

throw when poll start event has no roomId

4a0494c

Merge branch 'develop' into psg-1014/poll-model

07d154a

updates for events-sdk move

68ffea1

more type changes for events-sdk changes

7d8f51c

validate poll end event senders

10117e7

reformatted copyright

0a618a2

undo more comment reformatting

68291e8

kerryarchibald added the T-Enhancement label Jan 17, 2023

kerryarchibald requested review from SimonBrandner and richvdh January 17, 2023 03:19

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

9c239b2

github-actions bot deployed to PR Documentation Preview January 17, 2023 03:23 View deployment

kerryarchibald changed the base branch from develop to psg-1014/poll-model January 17, 2023 22:47

richvdh reviewed Jan 18, 2023

View reviewed changes

src/models/poll.ts Outdated Show resolved Hide resolved

Base automatically changed from psg-1014/poll-model to develop January 26, 2023 02:07

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

7588f07

kerryarchibald requested a review from richvdh January 26, 2023 02:25

github-actions bot deployed to PR Documentation Preview January 26, 2023 02:26 View deployment

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

a2fc0b9

github-actions bot deployed to PR Documentation Preview January 29, 2023 19:34 View deployment

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

2c457a6

github-actions bot deployed to PR Documentation Preview January 30, 2023 21:28 View deployment

richvdh requested changes Jan 31, 2023

View reviewed changes

Kerry Archibald added 2 commits February 1, 2023 10:34

fix poll end validation logic to allow poll creator to end poll regar…

2dc4a81

…dless of redaction

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

3b3870b

kerryarchibald requested a review from richvdh January 31, 2023 21:35

Update src/models/poll.ts

1d2edc2

Co-authored-by: Richard van der Hoff <[email protected]>

github-actions bot deployed to PR Documentation Preview January 31, 2023 21:38 View deployment

richvdh approved these changes Feb 1, 2023

View reviewed changes

Kerry Archibald added 2 commits February 2, 2023 09:24

correct creator == sender validationin poll end

ebbe6d4

Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events

1d164f7

kerryarchibald enabled auto-merge (squash) February 1, 2023 20:25

github-actions bot deployed to PR Documentation Preview February 1, 2023 20:28 View deployment

kerryarchibald merged commit 2800681 into develop Feb 1, 2023

kerryarchibald deleted the psg-1014/poll-model-only-valid-end-events branch February 1, 2023 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Poll model - validate end events#3072

Poll model - validate end events#3072
kerryarchibald merged 38 commits intodevelopfrom
psg-1014/poll-model-only-valid-end-events

kerryarchibald commented Jan 17, 2023 •

edited by github-actions bot

Loading

Uh oh!

SimonBrandner commented Jan 17, 2023

Uh oh!

richvdh left a comment

Uh oh!

Uh oh!

kerryarchibald commented Jan 26, 2023

Uh oh!

Uh oh!

richvdh Jan 31, 2023

Uh oh!

kerryarchibald Jan 31, 2023

Uh oh!

richvdh left a comment

Uh oh!

richvdh Feb 1, 2023

Uh oh!

kerryarchibald Feb 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const endEventSender = endEvent.getSender();
		return !!endEventSender && roomCurrentState.maySendRedactionForEvent(this.rootEvent, endEventSender);

	(endEventSender === this.matrixClient.getSafeUserId() \|\|
	(endEventSender === this.rootEvent.getSender() \|\|

Uh oh!

Conversation

kerryarchibald commented Jan 17, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

✨ Features

Uh oh!

SimonBrandner commented Jan 17, 2023

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kerryarchibald commented Jan 26, 2023

Uh oh!

Uh oh!

richvdh Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

kerryarchibald Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

richvdh Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

kerryarchibald Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kerryarchibald commented Jan 17, 2023 •

edited by github-actions bot

Loading