imterah/element-portable
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Only report undecryptable events once (#12501)

Browse source 
* persist previously-reported event IDs as a bloom filter

* Pin to older `@types/seedrandom`

... to work around https://github.com/Callidon/bloom-filters/issues/72

* Inline `DecryptionFailureTracker.addDecryptionFailure`

* Remove redundant TRACK_INTERVAL

There really doesn't seem to be much point to this batching up of decryption
failure reports. We still call the analytics callback the same number of times.

* Rename `trackedEvents` to `reportedEvents`

* Fix incorrect documentation on `visibleEvents`

This *does* overlap with `failures`.

* Combine `addFailure` and `reportFailure`

* Calculate client properties before starting reporting

* Clear localstorage after each test

... otherwise they interfere

* Remove redundant comment

* Ensure that reports are cleared on a logout/login cycle

* make private const private and const

---------

Co-authored-by: Richard van der Hoff <richard@matrix.org>
This commit is contained in:
Hubert Chathi 2024-05-20 10:53:50 -04:00 • committed by GitHub
parent 3e103941d6
commit 1bb70c5b3b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 197 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

124
test/DecryptionFailureTracker-test.ts
View file

@ -14,14 +14,15 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { mocked, Mocked } from "jest-mock";
import { CryptoEvent, HttpApiEvent, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix";
import { mocked, Mocked, MockedObject } from "jest-mock";
import { CryptoEvent, HttpApiEvent, MatrixClient, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix";
import { decryptExistingEvent, mkDecryptionFailureMatrixEvent } from "matrix-js-sdk/src/testing";
import { CryptoApi, DecryptionFailureCode, UserVerificationStatus } from "matrix-js-sdk/src/crypto-api";
import { sleep } from "matrix-js-sdk/src/utils";
import { DecryptionFailureTracker, ErrorProperties } from "../src/DecryptionFailureTracker";
import { stubClient } from "./test-utils";
import * as Lifecycle from "../src/Lifecycle";
async function createFailedDecryptionEvent(opts: { sender?: string; code?: DecryptionFailureCode } = {}) {
return await mkDecryptionFailureMatrixEvent({
@ -39,6 +40,10 @@ function eventDecrypted(tracker: DecryptionFailureTracker, e: MatrixEvent, nowTs
}
describe("DecryptionFailureTracker", function () {
afterEach(() => {
localStorage.clear();
});
it("tracks a failed decryption for a visible event", async function () {
const failedDecryptionEvent = await createFailedDecryptionEvent();
@ -247,6 +252,7 @@ describe("DecryptionFailureTracker", function () {
() => count++,
() => "UnknownError",
);
await tracker.start(mockClient());
tracker.addVisibleEvent(decryptedEvent);
@ -264,10 +270,7 @@ describe("DecryptionFailureTracker", function () {
expect(count).toBe(1);
});
it.skip("should not track a failure for an event that was tracked in a previous session", async () => {
// This test uses localStorage, clear it beforehand
localStorage.clear();
it("should not report a failure for an event that was reported in a previous session", async () => {
const decryptedEvent = await createFailedDecryptionEvent();
let count = 0;
@ -276,6 +279,7 @@ describe("DecryptionFailureTracker", function () {
() => count++,
() => "UnknownError",
);
await tracker.start(mockClient());
tracker.addVisibleEvent(decryptedEvent);
@ -289,14 +293,13 @@ describe("DecryptionFailureTracker", function () {
// Simulate the browser refreshing by destroying tracker and creating a new tracker
// @ts-ignore access to private constructor
const secondTracker = new DecryptionFailureTracker(
(total: number) => (count += total),
() => count++,
() => "UnknownError",
);
await secondTracker.start(mockClient());
secondTracker.addVisibleEvent(decryptedEvent);
//secondTracker.loadTrackedEvents();
eventDecrypted(secondTracker, decryptedEvent, Date.now());
secondTracker.checkFailures(Infinity);
@ -304,6 +307,70 @@ describe("DecryptionFailureTracker", function () {
expect(count).toBe(1);
});
it("should report a failure for an event that was tracked but not reported in a previous session", async () => {
const decryptedEvent = await createFailedDecryptionEvent();
let count = 0;
// @ts-ignore access to private constructor
const tracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
);
await tracker.start(mockClient());
tracker.addVisibleEvent(decryptedEvent);
// Indicate decryption
eventDecrypted(tracker, decryptedEvent, Date.now());
// we do *not* call `checkFailures` here
expect(count).toBe(0);
// Simulate the browser refreshing by destroying tracker and creating a new tracker
// @ts-ignore access to private constructor
const secondTracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
);
await secondTracker.start(mockClient());
secondTracker.addVisibleEvent(decryptedEvent);
eventDecrypted(secondTracker, decryptedEvent, Date.now());
secondTracker.checkFailures(Infinity);
expect(count).toBe(1);
});
it("should report a failure for an event that was reported before a logout/login cycle", async () => {
const decryptedEvent = await createFailedDecryptionEvent();
let count = 0;
// @ts-ignore access to private constructor
const tracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
);
await tracker.start(mockClient());
tracker.addVisibleEvent(decryptedEvent);
// Indicate decryption
eventDecrypted(tracker, decryptedEvent, Date.now());
tracker.checkFailures(Infinity);
expect(count).toBe(1);
// Simulate a logout/login cycle
await Lifecycle.onLoggedOut();
await tracker.start(mockClient());
tracker.addVisibleEvent(decryptedEvent);
eventDecrypted(tracker, decryptedEvent, Date.now());
tracker.checkFailures(Infinity);
expect(count).toBe(2);
});
it("should count different error codes separately for multiple failures with different error codes", async () => {
const counts: Record<string, number> = {};
@ -521,12 +588,7 @@ describe("DecryptionFailureTracker", function () {
it("listens for client events", async () => {
// Test that the decryption failure tracker registers the right event
// handlers on start, and unregisters them when the client logs out.
const client = mocked(stubClient());
const mockCrypto = {
getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"),
getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)),
} as unknown as Mocked<CryptoApi>;
client.getCrypto.mockReturnValue(mockCrypto);
const client = mockClient();
let errorCount: number = 0;
// @ts-ignore access to private constructor
@ -568,13 +630,7 @@ describe("DecryptionFailureTracker", function () {
});
it("tracks client information", async () => {
const client = mocked(stubClient());
const mockCrypto = {
getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"),
getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)),
} as unknown as Mocked<CryptoApi>;
client.getCrypto.mockReturnValue(mockCrypto);
const client = mockClient();
const propertiesByErrorCode: Record<string, ErrorProperties> = {};
// @ts-ignore access to private constructor
const tracker = new DecryptionFailureTracker(
@ -610,7 +666,9 @@ describe("DecryptionFailureTracker", function () {
const now = Date.now();
eventDecrypted(tracker, federatedDecryption, now);
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, true, false));
mocked(client.getCrypto()!.getUserVerificationStatus).mockResolvedValue(
new UserVerificationStatus(true, true, false),
);
client.emit(CryptoEvent.KeysChanged, {});
await sleep(100);
eventDecrypted(tracker, localDecryption, now);
@ -628,7 +686,7 @@ describe("DecryptionFailureTracker", function () {
// change client params, and make sure the reports the right values
client.getDomain.mockReturnValue("example.com");
mockCrypto.getVersion.mockReturnValue("Olm 0.0.0");
mocked(client.getCrypto()!.getVersion).mockReturnValue("Olm 0.0.0");
// @ts-ignore access to private method
await tracker.calculateClientProperties(client);
@ -673,3 +731,21 @@ describe("DecryptionFailureTracker", function () {
expect(failure?.timeToDecryptMillis).toEqual(50000);
});
});
function mockClient(): MockedObject<MatrixClient> {
const client = mocked(stubClient());
const mockCrypto = {
getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"),
getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)),
} as unknown as Mocked<CryptoApi>;
client.getCrypto.mockReturnValue(mockCrypto);
// @ts-ignore
client.stopClient = jest.fn(() => {});
// @ts-ignore
client.removeAllListeners = jest.fn(() => {});
client.store = { destroy: jest.fn(() => {}) } as any;
return client;
}