Skip to content

Commit

Permalink
Cache non-encrypted rooms and skip fetching encrypted rooms on startup (
Browse files Browse the repository at this point in the history
#39)

We need this for specific situations where a bot is joined to a LOT of
rooms, but the encrypted rooms are in the minority.

This should:
 - Cache rooms that explicitly do not have a encryption state event.
 - Recheck every time we see an encryption state event.

## Checklist

* [ ] Tests written for all new code
* [ ] Linter has been satisfied
* [ ] Sign-off given on the changes (see CONTRIBUTING.md)
  • Loading branch information
Half-Shot authored Dec 4, 2023
2 parents 669408d + 7e050c7 commit da2792e
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 63 deletions.
4 changes: 2 additions & 2 deletions examples/encryption_bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ const worksImage = fs.readFileSync("./examples/static/it-works.png");
const client = new MatrixClient(homeserverUrl, accessToken, storage, crypto);

(async function() {
let encryptedRoomId: string;
let encryptedRoomId: string|undefined = undefined;
const joinedRooms = await client.getJoinedRooms();
await client.crypto.prepare(joinedRooms); // init crypto because we're doing things before the client is started
await client.crypto.prepare(); // init crypto because we're doing things before the client is started
for (const roomId of joinedRooms) {
if (await client.crypto.isRoomEncrypted(roomId)) {
encryptedRoomId = roomId;
Expand Down
2 changes: 1 addition & 1 deletion src/MatrixClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ export class MatrixClient extends EventEmitter {

if (this.crypto) {
LogService.debug("MatrixClientLite", "Preparing end-to-end encryption");
await this.crypto.prepare(this.lastJoinedRoomIds);
await this.crypto.prepare();
LogService.info("MatrixClientLite", "End-to-end encryption enabled");
}

Expand Down
2 changes: 1 addition & 1 deletion src/appservice/Intent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export class Intent {
}

// Now set up crypto
await this.client.crypto.prepare(await this.getJoinedRooms());
await this.client.crypto.prepare();

this.appservice.on("room.event", (roomId, event) => {
this.client.crypto.onRoomEvent(roomId, event);
Expand Down
4 changes: 1 addition & 3 deletions src/e2ee/CryptoClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ export class CryptoClient {
* Prepares the crypto client for usage.
* @param {string[]} roomIds The room IDs the MatrixClient is joined to.
*/
public async prepare(roomIds: string[]) {
await this.roomTracker.prepare(roomIds);

public async prepare() {
if (this.ready) return; // stop re-preparing here

const storedDeviceId = await this.client.cryptoStore.getDeviceId();
Expand Down
17 changes: 6 additions & 11 deletions src/e2ee/RoomTracker.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { MatrixClient } from "../MatrixClient";
import { MatrixError } from "../models/MatrixError";
import { EncryptionEventContent } from "../models/events/EncryptionEvent";
import { ICryptoRoomInformation } from "./ICryptoRoomInformation";

Expand Down Expand Up @@ -33,16 +34,6 @@ export class RoomTracker {
}
}

/**
* Prepares the room tracker to track the given rooms.
* @param {string[]} roomIds The room IDs to track. This should be the joined rooms set.
*/
public async prepare(roomIds: string[]) {
for (const roomId of roomIds) {
await this.queueRoomCheck(roomId);
}
}

/**
* Queues a room check for the tracker. If the room needs an update to the store, an
* update will be made.
Expand All @@ -61,7 +52,11 @@ export class RoomTracker {
encEvent = await this.client.getRoomStateEvent(roomId, "m.room.encryption", "");
encEvent.algorithm = encEvent.algorithm ?? 'UNKNOWN';
} catch (e) {
return; // failure == no encryption
if (e instanceof MatrixError && e.errcode === "M_NOT_FOUND") {
encEvent = {};
} else {
return; // Other failures should not be cached.
}
}

// Pick out the history visibility setting too
Expand Down
41 changes: 17 additions & 24 deletions test/encryption/CryptoClientTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -28,24 +28,17 @@ describe('CryptoClient', () => {
describe('prepare', () => {
it('should prepare the room tracker', () => testCryptoStores(async (cryptoStoreType) => {
const userId = "@alice:example.org";
const roomIds = ["!a:example.org", "!b:example.org"];
const { client, http } = createTestClient(null, userId, cryptoStoreType);

client.getWhoAmI = () => Promise.resolve({ user_id: userId, device_id: TEST_DEVICE_ID });

const prepareSpy = simple.stub().callFn((rids: string[]) => {
expect(rids).toBe(roomIds);
return Promise.resolve();
});

(<any>client.crypto).roomTracker.prepare = prepareSpy; // private member access

bindNullEngine(http);
// Prepare first
await Promise.all([
client.crypto.prepare(roomIds),
client.crypto.prepare(),
http.flushAllExpected(),
]);
expect(prepareSpy.callCount).toEqual(1);
expect(client.crypto.isReady).toBe(true);
}));

it('should use a stored device ID', () => testCryptoStores(async (cryptoStoreType) => {
Expand All @@ -59,7 +52,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);
expect(whoamiSpy.callCount).toEqual(0);
Expand Down Expand Up @@ -118,7 +111,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -138,7 +131,7 @@ describe('CryptoClient', () => {
const { client } = createTestClient(null, userId, cryptoStoreType);

await client.cryptoStore.setDeviceId(TEST_DEVICE_ID);
// await client.crypto.prepare([]); // deliberately commented
// await client.crypto.prepare(); // deliberately commented

try {
await client.crypto.isRoomEncrypted("!new:example.org");
Expand All @@ -159,7 +152,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -176,7 +169,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -193,7 +186,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -210,7 +203,7 @@ describe('CryptoClient', () => {

bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -248,7 +241,7 @@ describe('CryptoClient', () => {
it('should sign the object while retaining signatures without mutation', async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -305,7 +298,7 @@ describe('CryptoClient', () => {
it('should fail in unencrypted rooms', async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -381,7 +374,7 @@ describe('CryptoClient', () => {
it('should encrypt media', async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -467,7 +460,7 @@ describe('CryptoClient', () => {
it('should be symmetrical', async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand All @@ -492,7 +485,7 @@ describe('CryptoClient', () => {
it('should decrypt', async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -522,7 +515,7 @@ describe('CryptoClient', () => {
await client.cryptoStore.setDeviceId(TEST_DEVICE_ID);
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);
}));
Expand Down
2 changes: 1 addition & 1 deletion test/encryption/KeyBackupTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('KeyBackups', () => {
const prepareCrypto = async () => {
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);
};
Expand Down
22 changes: 2 additions & 20 deletions test/encryption/RoomTrackerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('RoomTracker', () => {
await client.cryptoStore.setDeviceId(TEST_DEVICE_ID);
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);
(client.crypto as any).engine.addTrackedUsers = () => Promise.resolve();
Expand Down Expand Up @@ -72,7 +72,7 @@ describe('RoomTracker', () => {
await client.cryptoStore.setDeviceId(TEST_DEVICE_ID);
bindNullEngine(http);
await Promise.all([
client.crypto.prepare([]),
client.crypto.prepare(),
http.flushAllExpected(),
]);

Expand Down Expand Up @@ -103,24 +103,6 @@ describe('RoomTracker', () => {
expect(queueSpy.callCount).toEqual(1);
}));

describe('prepare', () => {
it('should queue updates for rooms', async () => {
const roomIds = ["!a:example.org", "!b:example.org"];

const { client } = createTestClient();

const queueSpy = simple.stub().callFn((rid: string) => {
expect(rid).toEqual(roomIds[queueSpy.callCount - 1]);
return Promise.resolve();
});

const tracker = new RoomTracker(client);
tracker.queueRoomCheck = queueSpy;
await tracker.prepare(roomIds);
expect(queueSpy.callCount).toEqual(2);
});
});

describe('queueRoomCheck', () => {
it('should store unknown rooms', () => testCryptoStores(async (cryptoStoreType) => {
const roomId = "!b:example.org";
Expand Down

0 comments on commit da2792e

Please sign in to comment.