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 36 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
159 changes: 138 additions & 21 deletions spec/unit/models/poll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,31 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { IEvent, MatrixEvent, PollEvent } from "../../../src";
import { IEvent, MatrixEvent, PollEvent, Room } from "../../../src";
import { REFERENCE_RELATION } from "../../../src/@types/extensible_events";
import { M_POLL_END, M_POLL_KIND_DISCLOSED, M_POLL_RESPONSE } from "../../../src/@types/polls";
import { PollStartEvent } from "../../../src/extensible_events_v1/PollStartEvent";
import { Poll } from "../../../src/models/poll";
import { getMockClientWithEventEmitter } from "../../test-utils/client";
import { getMockClientWithEventEmitter, mockClientMethodsUser } from "../../test-utils/client";

jest.useFakeTimers();

describe("Poll", () => {
const userId = "@alice:server.org";
const mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
relations: jest.fn(),
});
const roomId = "!room:server";
const room = new Room(roomId, mockClient, userId);
const maySendRedactionForEventSpy = jest.spyOn(room.currentState, "maySendRedactionForEvent");
// 14.03.2022 16:15
const now = 1647270879403;

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 All @@ -42,6 +47,8 @@ describe("Poll", () => {
jest.setSystemTime(now);

mockClient.relations.mockResolvedValue({ events: [] });

maySendRedactionForEventSpy.mockClear().mockReturnValue(true);
});

let eventId = 1;
Expand All @@ -62,7 +69,7 @@ describe("Poll", () => {
};

it("initialises with root event", () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
expect(poll.roomId).toEqual(roomId);
expect(poll.pollId).toEqual(basePollStartEvent.getId());
expect(poll.pollEvent).toEqual(basePollStartEvent.unstableExtensibleEvent);
Expand All @@ -73,28 +80,28 @@ describe("Poll", () => {
const pollStartEvent = new MatrixEvent(
PollStartEvent.from("What?", ["a", "b"], M_POLL_KIND_DISCLOSED.name).serialize(),
);
expect(() => new Poll(pollStartEvent, mockClient)).toThrowError("Invalid poll start event.");
expect(() => new Poll(pollStartEvent, mockClient, room)).toThrowError("Invalid poll start event.");
});

it("throws when poll start has no event id", () => {
const pollStartEvent = new MatrixEvent({
...PollStartEvent.from("What?", ["a", "b"], M_POLL_KIND_DISCLOSED.name).serialize(),
room_id: roomId,
});
expect(() => new Poll(pollStartEvent, mockClient)).toThrowError("Invalid poll start event.");
expect(() => new Poll(pollStartEvent, mockClient, room)).toThrowError("Invalid poll start event.");
});

describe("fetching responses", () => {
it("calls relations api and emits", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const emitSpy = jest.spyOn(poll, "emit");
const responses = await poll.getResponses();
expect(mockClient.relations).toHaveBeenCalledWith(roomId, basePollStartEvent.getId(), "m.reference");
expect(emitSpy).toHaveBeenCalledWith(PollEvent.Responses, responses);
});

it("returns existing responses object after initial fetch", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const responses = await poll.getResponses();
const responses2 = await poll.getResponses();
// only fetched relations once
Expand All @@ -104,7 +111,7 @@ describe("Poll", () => {
});

it("waits for existing relations request to finish when getting responses", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const firstResponsePromise = poll.getResponses();
const secondResponsePromise = poll.getResponses();
await firstResponsePromise;
Expand All @@ -121,14 +128,14 @@ describe("Poll", () => {
mockClient.relations.mockResolvedValue({
events: [replyEvent, stableResponseEvent, unstableResponseEvent],
});
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const responses = await poll.getResponses();
expect(responses.getRelations()).toEqual([stableResponseEvent, unstableResponseEvent]);
});

describe("with poll end event", () => {
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable! });
const unstablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.unstable! });
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable!, sender: "@bob@server.org" });
const unstablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.unstable!, sender: "@bob@server.org" });
const responseEventBeforeEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now - 1000);
const responseEventAtEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now);
const responseEventAfterEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now + 1000);
Expand All @@ -140,10 +147,37 @@ describe("Poll", () => {
});

it("sets poll end event with stable event type", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
jest.spyOn(poll, "emit");
await poll.getResponses();

expect(maySendRedactionForEventSpy).toHaveBeenCalledWith(basePollStartEvent, "@bob@server.org");
expect(poll.isEnded).toBe(true);
expect(poll.emit).toHaveBeenCalledWith(PollEvent.End);
});

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");
await poll.getResponses();

expect(maySendRedactionForEventSpy).toHaveBeenCalledWith(basePollStartEvent, "@bob@server.org");
expect(poll.isEnded).toBe(false);
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);
});
Expand All @@ -152,7 +186,7 @@ describe("Poll", () => {
mockClient.relations.mockResolvedValue({
events: [unstablePollEndEvent],
});
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
jest.spyOn(poll, "emit");
await poll.getResponses();

Expand All @@ -161,7 +195,7 @@ describe("Poll", () => {
});

it("filters out responses that were sent after poll end", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const responses = await poll.getResponses();

// just response type events
Expand All @@ -173,7 +207,7 @@ describe("Poll", () => {

describe("onNewRelation()", () => {
it("discards response if poll responses have not been initialised", () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
jest.spyOn(poll, "emit");
const responseEvent = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now);

Expand All @@ -184,24 +218,107 @@ describe("Poll", () => {
});

it("sets poll end event when responses are not initialised", () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
jest.spyOn(poll, "emit");
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable!, sender: userId });

poll.onNewRelation(stablePollEndEvent);

expect(poll.emit).toHaveBeenCalledWith(PollEvent.End);
});

it("does not set poll end event when sent by invalid user", async () => {
maySendRedactionForEventSpy.mockReturnValue(false);
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable!, sender: "@charlie:server.org" });
const responseEventAfterEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now + 1000);
mockClient.relations.mockResolvedValue({
events: [responseEventAfterEnd],
});
const poll = new Poll(basePollStartEvent, mockClient, room);
await poll.getResponses();
jest.spyOn(poll, "emit");
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable! });

poll.onNewRelation(stablePollEndEvent);

// didn't end, didn't refilter responses
expect(poll.emit).not.toHaveBeenCalled();
expect(poll.isEnded).toBeFalsy();
expect(maySendRedactionForEventSpy).toHaveBeenCalledWith(basePollStartEvent, "@charlie:server.org");
});

it("does not set poll end event when an earlier end event already exists", async () => {
const earlierPollEndEvent = makeRelatedEvent(
{ type: M_POLL_END.stable!, sender: "@valid:server.org" },
now,
);
const laterPollEndEvent = makeRelatedEvent(
{ type: M_POLL_END.stable!, sender: "@valid:server.org" },
now + 2000,
);

const poll = new Poll(basePollStartEvent, mockClient, room);
await poll.getResponses();

poll.onNewRelation(earlierPollEndEvent);

// first end event set correctly
expect(poll.isEnded).toBeTruthy();

// reset spy count
jest.spyOn(poll, "emit").mockClear();

poll.onNewRelation(laterPollEndEvent);
// didn't set new end event, didn't refilter responses
expect(poll.emit).not.toHaveBeenCalled();
expect(poll.isEnded).toBeTruthy();
});

it("replaces poll end event and refilters when an older end event already exists", async () => {
const earlierPollEndEvent = makeRelatedEvent(
{ type: M_POLL_END.stable!, sender: "@valid:server.org" },
now,
);
const laterPollEndEvent = makeRelatedEvent(
{ type: M_POLL_END.stable!, sender: "@valid:server.org" },
now + 2000,
);
const responseEventBeforeEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now - 1000);
const responseEventAtEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now);
const responseEventAfterEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now + 1000);
mockClient.relations.mockResolvedValue({
events: [responseEventAfterEnd, responseEventAtEnd, responseEventBeforeEnd, laterPollEndEvent],
});

const poll = new Poll(basePollStartEvent, mockClient, room);
const responses = await poll.getResponses();

// all responses have a timestamp < laterPollEndEvent
expect(responses.getRelations().length).toEqual(3);
// first end event set correctly
expect(poll.isEnded).toBeTruthy();

// reset spy count
jest.spyOn(poll, "emit").mockClear();

// add a valid end event with earlier timestamp
poll.onNewRelation(earlierPollEndEvent);

// emitted new end event
expect(poll.emit).toHaveBeenCalledWith(PollEvent.End);
// filtered responses and emitted
expect(poll.emit).toHaveBeenCalledWith(PollEvent.Responses, responses);
expect(responses.getRelations()).toEqual([responseEventAtEnd, responseEventBeforeEnd]);
});

it("sets poll end event and refilters responses based on timestamp", async () => {
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable! });
const stablePollEndEvent = makeRelatedEvent({ type: M_POLL_END.stable!, sender: userId });
const responseEventBeforeEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now - 1000);
const responseEventAtEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now);
const responseEventAfterEnd = makeRelatedEvent({ type: M_POLL_RESPONSE.name }, now + 1000);
mockClient.relations.mockResolvedValue({
events: [responseEventAfterEnd, responseEventAtEnd, responseEventBeforeEnd],
});
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
const responses = await poll.getResponses();
jest.spyOn(poll, "emit");

Expand All @@ -216,7 +333,7 @@ describe("Poll", () => {
});

it("filters out irrelevant relations", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
// init responses
const responses = await poll.getResponses();
jest.spyOn(poll, "emit");
Expand All @@ -230,7 +347,7 @@ describe("Poll", () => {
});

it("adds poll response relations to responses", async () => {
const poll = new Poll(basePollStartEvent, mockClient);
const poll = new Poll(basePollStartEvent, mockClient, room);
// init responses
const responses = await poll.getResponses();
jest.spyOn(poll, "emit");
Expand Down
35 changes: 31 additions & 4 deletions src/models/poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { M_POLL_END, M_POLL_RESPONSE, PollStartEvent } from "../@types/polls";
import { MatrixClient } from "../client";
import { MatrixEvent } from "./event";
import { Relations } from "./relations";
import { Room } from "./room";
import { TypedEventEmitter } from "./typed-event-emitter";

export enum PollEvent {
Expand Down Expand Up @@ -64,13 +65,12 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P
private responses: null | Relations = null;
private endEvent: MatrixEvent | undefined;

public constructor(private rootEvent: MatrixEvent, private matrixClient: MatrixClient) {
public constructor(private rootEvent: MatrixEvent, private matrixClient: MatrixClient, private room: Room) {
super();
if (!this.rootEvent.getRoomId() || !this.rootEvent.getId()) {
throw new Error("Invalid poll start event.");
}
this.roomId = this.rootEvent.getRoomId()!;
// @TODO(kerrya) proper way to do this?
this.pollEvent = this.rootEvent.unstableExtensibleEvent as unknown as PollStartEvent;
}

Expand Down Expand Up @@ -101,7 +101,7 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P
* @returns void
*/
public onNewRelation(event: MatrixEvent): void {
if (M_POLL_END.matches(event.getType())) {
if (M_POLL_END.matches(event.getType()) && this.validateEndEvent(event)) {
this.endEvent = event;
this.refilterResponsesOnEnd();
this.emit(PollEvent.End);
Expand Down Expand Up @@ -136,7 +136,8 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P
M_POLL_RESPONSE.altName!,
]);

const pollEndEvent = allRelations.events.find((event) => M_POLL_END.matches(event.getType()));
const potentialEndEvent = allRelations.events.find((event) => M_POLL_END.matches(event.getType()));
const pollEndEvent = this.validateEndEvent(potentialEndEvent) ? potentialEndEvent : undefined;
const pollCloseTimestamp = pollEndEvent?.getTs() || Number.MAX_SAFE_INTEGER;

const { responseEvents } = filterResponseRelations(allRelations.events, pollCloseTimestamp);
Expand Down Expand Up @@ -172,4 +173,30 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P

this.emit(PollEvent.Responses, this.responses);
}

private validateEndEvent(endEvent?: MatrixEvent): boolean {
if (!endEvent) {
return false;
}
/**
* Repeated end events are ignored -
* only the first (valid) closure event by origin_server_ts is counted.
*/
if (this.endEvent && this.endEvent.getTs() < endEvent.getTs()) {
return false;
}

/**
* MSC3381
* If a m.poll.end event is received from someone other than the poll creator or user with permission to redact
* 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 &&
(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))
);
}
}
2 changes: 1 addition & 1 deletion src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1897,7 +1897,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
const processPollStartEvent = (event: MatrixEvent): void => {
if (!M_POLL_START.matches(event.getType())) return;
try {
const poll = new Poll(event, this.client);
const poll = new Poll(event, this.client, this);
this.polls.set(event.getId()!, poll);
this.emit(PollEvent.New, poll);
} catch {}
Expand Down