Skip to content
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.

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

Poll model - validate end events #3072

Merged
merged 38 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5ca533c
first cut poll model
Dec 30, 2022
984e5eb
process incoming poll relations
Dec 30, 2022
e578d84
allow alt event types in relations model
Jan 4, 2023
1123f1d
allow alt event types in relations model
Jan 4, 2023
ea71efe
remove unneccesary checks on remove relation
Jan 4, 2023
dfe4038
comment
Jan 4, 2023
515db7a
Revert "allow alt event types in relations model"
Jan 4, 2023
ae6ffb7
Revert "Revert "allow alt event types in relations model""
Jan 4, 2023
f189901
Merge branch 'psg-1014/relations-alt-event-types' into psg-1014/poll-…
Jan 4, 2023
cace5d4
basic handling for new poll relations
Jan 4, 2023
6b4f630
Merge branch 'develop' into psg-1014/poll-model
Jan 5, 2023
0f7829a
tests
Jan 9, 2023
32abfed
test room.processPollEvents
Jan 9, 2023
797123c
join processBeaconEvents and poll events in client
Jan 9, 2023
51896e1
Merge branch 'develop' into psg-1014/poll-model
Jan 9, 2023
ad49a00
Merge branch 'develop' into psg-1014/poll-model
Jan 10, 2023
b158dcc
tidy and set 23 copyrights
Jan 11, 2023
924a6c0
use rooms instance of matrixClient
Jan 11, 2023
d09700f
tidy
Jan 11, 2023
9e0d95e
more copyright
Jan 11, 2023
b173130
simplify processPollEvent code
Jan 12, 2023
15dd1c4
Merge branch 'develop' into psg-1014/poll-model
Jan 12, 2023
4a0494c
throw when poll start event has no roomId
Jan 12, 2023
07d154a
Merge branch 'develop' into psg-1014/poll-model
Jan 15, 2023
68ffea1
updates for events-sdk move
Jan 16, 2023
7d8f51c
more type changes for events-sdk changes
Jan 16, 2023
10117e7
validate poll end event senders
Jan 17, 2023
0a618a2
reformatted copyright
Jan 17, 2023
68291e8
undo more comment reformatting
Jan 17, 2023
9c239b2
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Jan 17, 2023
7588f07
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Jan 26, 2023
a2fc0b9
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Jan 29, 2023
2c457a6
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Jan 30, 2023
2dc4a81
fix poll end validation logic to allow poll creator to end poll regar…
Jan 31, 2023
3b3870b
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Jan 31, 2023
1d2edc2
Update src/models/poll.ts
Jan 31, 2023
ebbe6d4
correct creator == sender validationin poll end
Feb 1, 2023
1d164f7
Merge branch 'develop' into psg-1014/poll-model-only-valid-end-events
Feb 1, 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 spec/test-utils/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export const getMockClientWithEventEmitter = (
*/
export const mockClientMethodsUser = (userId = "@alice:domain") => ({
getUserId: jest.fn().mockReturnValue(userId),
getSafeUserId: jest.fn().mockReturnValue(userId),
getUser: jest.fn().mockReturnValue(new User(userId)),
isGuest: jest.fn().mockReturnValue(false),
mxcUrlToHttp: jest.fn().mockReturnValue("mock-mxcUrlToHttp"),
Expand Down
18 changes: 17 additions & 1 deletion spec/unit/models/poll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe("Poll", () => {
const basePollStartEvent = new MatrixEvent({
...PollStartEvent.from("What?", ["a", "b"], M_POLL_KIND_DISCLOSED.name).serialize(),
room_id: roomId,
sender: userId,
});
basePollStartEvent.event.event_id = "$12345";

Expand Down Expand Up @@ -155,7 +156,7 @@ describe("Poll", () => {
expect(poll.emit).toHaveBeenCalledWith(PollEvent.End);
});

it("does not set poll end event when sent by invalid user", async () => {
it("does not set poll end event when sent by a user without redaction rights", async () => {
const poll = new Poll(basePollStartEvent, mockClient, room);
maySendRedactionForEventSpy.mockReturnValue(false);
jest.spyOn(poll, "emit");
Expand All @@ -166,6 +167,21 @@ describe("Poll", () => {
expect(poll.emit).not.toHaveBeenCalledWith(PollEvent.End);
});

it("sets poll end event when current user created the poll, but does not have redaction rights", async () => {
const poll = new Poll(basePollStartEvent, mockClient, room);
const pollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable!, sender: userId });
mockClient.relations.mockResolvedValue({
events: [pollEndEvent],
});
maySendRedactionForEventSpy.mockReturnValue(false);
jest.spyOn(poll, "emit");
await poll.getResponses();

expect(maySendRedactionForEventSpy).not.toHaveBeenCalled();
expect(poll.isEnded).toBe(true);
expect(poll.emit).toHaveBeenCalledWith(PollEvent.End);
});

it("sets poll end event with unstable event type", async () => {
mockClient.relations.mockResolvedValue({
events: [unstablePollEndEvent],
Expand Down
8 changes: 6 additions & 2 deletions src/models/poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,14 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P
/**
* MSC3381
* 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.
* others' messages in the room, the event must be ignored by clients due to being invalid.
*/
const roomCurrentState = this.room.currentState;
const endEventSender = endEvent.getSender();
return !!endEventSender && roomCurrentState.maySendRedactionForEvent(this.rootEvent, endEventSender);
return (
!!endEventSender &&
(endEventSender === this.matrixClient.getSafeUserId() ||
Copy link
Member

Choose a reason for hiding this comment

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

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() ||

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

roomCurrentState.maySendRedactionForEvent(this.rootEvent, endEventSender))
);
}
}