From 6385c9c0dab8fe67bd3a8992a4777f243fdd1b68 Mon Sep 17 00:00:00 2001
From: David Baker <dbkr@users.noreply.github.com>
Date: Fri, 22 Sep 2023 16:31:17 +0100
Subject: [PATCH] Add membershipID to call memberships (#3745)

* Add membershipID to call memberships

This allows us to recognise easily when a membership is from some
previous sessions rather than our own and therefore ignore it
(see comment for more).

This was causing us to see existing, expired membership events and
bump the expiry on them rather than send a new membership. This might
have been okay if we bumped them enough to actually make them un-expired,
but it's a fresh session so semanticly we want to post a fresh membership
rather than resurrecting a previous, expired membership.

* Fix test types

* Fix tests

* Make test coverage happy

---------

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
---
 spec/unit/matrixrtc/CallMembership.spec.ts      |  1 +
 spec/unit/matrixrtc/MatrixRTCSession.spec.ts    |  7 +++++++
 .../matrixrtc/MatrixRTCSessionManager.spec.ts   |  1 +
 src/matrixrtc/CallMembership.ts                 |  5 +++++
 src/matrixrtc/MatrixRTCSession.ts               | 17 ++++++++++++++++-
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/spec/unit/matrixrtc/CallMembership.spec.ts b/spec/unit/matrixrtc/CallMembership.spec.ts
index a8ae0a822..6f7f8f5ca 100644
--- a/spec/unit/matrixrtc/CallMembership.spec.ts
+++ b/spec/unit/matrixrtc/CallMembership.spec.ts
@@ -23,6 +23,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 5000,
+    membershipID: "bloop",
 };
 
 function makeMockEvent(originTs = 0): MatrixEvent {
diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts
index 9572e1f8b..f8c229c9a 100644
--- a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts
+++ b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts
@@ -26,6 +26,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 60 * 60 * 1000,
+    membershipID: "bloop",
 };
 
 const mockFocus = { type: "mock" };
@@ -56,6 +57,7 @@ describe("MatrixRTCSession", () => {
         expect(sess?.memberships[0].scope).toEqual("m.room");
         expect(sess?.memberships[0].application).toEqual("m.call");
         expect(sess?.memberships[0].deviceId).toEqual("AAAAAAA");
+        expect(sess?.memberships[0].membershipID).toEqual("bloop");
         expect(sess?.memberships[0].isExpired()).toEqual(false);
     });
 
@@ -219,6 +221,7 @@ describe("MatrixRTCSession", () => {
                             device_id: "AAAAAAA",
                             expires: 3600000,
                             foci_active: [{ type: "mock" }],
+                            membershipID: expect.stringMatching(".*"),
                         },
                     ],
                 },
@@ -286,6 +289,7 @@ describe("MatrixRTCSession", () => {
                                 expires: 3600000 * 2,
                                 foci_active: [{ type: "mock" }],
                                 created_ts: 1000,
+                                membershipID: expect.stringMatching(".*"),
                             },
                         ],
                     },
@@ -357,6 +361,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "AAAAAAA",
                         expires: 3600000,
                         foci_active: [mockFocus],
+                        membershipID: expect.stringMatching(".*"),
                     },
                 ],
             },
@@ -388,6 +393,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "OTHERDEVICE",
                         expires: 3600000,
                         created_ts: 1000,
+                        membershipID: expect.stringMatching(".*"),
                     },
                     {
                         application: "m.call",
@@ -396,6 +402,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "AAAAAAA",
                         expires: 3600000,
                         foci_active: [mockFocus],
+                        membershipID: expect.stringMatching(".*"),
                     },
                 ],
             },
diff --git a/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts b/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts
index fb96c3e53..6a240831e 100644
--- a/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts
+++ b/spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts
@@ -26,6 +26,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 60 * 60 * 1000,
+    membershipID: "bloop",
 };
 
 describe("MatrixRTCSessionManager", () => {
diff --git a/src/matrixrtc/CallMembership.ts b/src/matrixrtc/CallMembership.ts
index e67909d1e..b73a937b9 100644
--- a/src/matrixrtc/CallMembership.ts
+++ b/src/matrixrtc/CallMembership.ts
@@ -29,6 +29,7 @@ export interface CallMembershipData {
     created_ts?: number;
     expires: number;
     foci_active?: Focus[];
+    membershipID: string;
 }
 
 export class CallMembership {
@@ -64,6 +65,10 @@ export class CallMembership {
         return this.data.scope;
     }
 
+    public get membershipID(): string {
+        return this.data.membershipID;
+    }
+
     public createdTs(): number {
         return this.data.created_ts ?? this.parentEvent.getTs();
     }
diff --git a/src/matrixrtc/MatrixRTCSession.ts b/src/matrixrtc/MatrixRTCSession.ts
index 5f9ce7cc5..fc3d63cc6 100644
--- a/src/matrixrtc/MatrixRTCSession.ts
+++ b/src/matrixrtc/MatrixRTCSession.ts
@@ -23,6 +23,7 @@ import { EventType } from "../@types/event";
 import { CallMembership, CallMembershipData } from "./CallMembership";
 import { Focus } from "./focus";
 import { MatrixEvent } from "../matrix";
+import { randomString } from "../randomstring";
 
 const MEMBERSHIP_EXPIRY_TIME = 60 * 60 * 1000;
 const MEMBER_EVENT_CHECK_PERIOD = 2 * 60 * 1000; // How often we check to see if we need to re-send our member event
@@ -54,6 +55,14 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
     // if we're not yet joined
     private relativeExpiry: number | undefined;
 
+    // An identifier for our membership of the call. This will allow us to easily recognise
+    // whether a membership was sent by this session or is stale from some other time.
+    // It also forces our membership events to be unique, because otherwise we could try
+    // to overwrite a membership from a previous session but it would do nothing because the
+    // event content would be identical. We need the origin_server_ts to update though, so
+    // forcing unique content fixes this.
+    private membershipId: string | undefined;
+
     private memberEventTimeout?: ReturnType<typeof setTimeout>;
     private expiryTimeout?: ReturnType<typeof setTimeout>;
 
@@ -174,6 +183,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         logger.info(`Joining call session in room ${this.room.roomId}`);
         this.activeFoci = activeFoci;
         this.relativeExpiry = MEMBERSHIP_EXPIRY_TIME;
+        this.membershipId = randomString(5);
         this.emit(MatrixRTCSessionEvent.JoinStateChanged, true);
         // We don't wait for this, mostly because it may fail and schedule a retry, so this
         // function returning doesn't really mean anything at all.
@@ -195,6 +205,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         logger.info(`Leaving call session in room ${this.room.roomId}`);
         this.relativeExpiry = undefined;
         this.activeFoci = undefined;
+        this.membershipId = undefined;
         this.emit(MatrixRTCSessionEvent.JoinStateChanged, false);
         this.triggerCallMembershipEventUpdate();
     }
@@ -249,6 +260,9 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         if (this.relativeExpiry === undefined) {
             throw new Error("Tried to create our own membership event when we're not joined!");
         }
+        if (this.membershipId === undefined) {
+            throw new Error("Tried to create our own membership event when we have no membership ID!");
+        }
 
         const m: CallMembershipData = {
             call_id: "",
@@ -257,6 +271,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
             device_id: this.client.getDeviceId()!,
             expires: this.relativeExpiry,
             foci_active: this.activeFoci,
+            membershipID: this.membershipId,
         };
 
         if (prevMembership) m.created_ts = prevMembership.createdTs();
@@ -373,7 +388,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         const myPrevMembershipData = memberships.find((m) => m.device_id === localDeviceId);
         let myPrevMembership;
         try {
-            if (myCallMemberEvent && myPrevMembershipData) {
+            if (myCallMemberEvent && myPrevMembershipData && myPrevMembershipData.membershipID === this.membershipId) {
                 myPrevMembership = new CallMembership(myCallMemberEvent, myPrevMembershipData);
             }
         } catch (e) {