Skip to content

Commit

Permalink
Element-R: Implement VerificationRequest.{timeout,pending} (#3532)
Browse files Browse the repository at this point in the history
* implement `VerificationRequest.pending`

* Implement `VerificationRequest.timeout`

* Rust crypto: allow using a memory store (#3536)

* Rust crypto: allow using a memory store

It turns out that, for some usecases (in particular, "bot users" for cypress
tests), we don't need persistent storage and an in-memory store will be fine.

* Rust crypto: use a memory store for the unit tests
  • Loading branch information
richvdh authored Jul 3, 2023
1 parent 3a8a138 commit 3a694f4
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 25 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
],
"dependencies": {
"@babel/runtime": "^7.12.5",
"@matrix-org/matrix-sdk-crypto-js": "^0.1.0-alpha.11",
"@matrix-org/matrix-sdk-crypto-js": "^0.1.0",
"another-json": "^0.2.0",
"bs58": "^5.0.0",
"content-type": "^1.0.4",
Expand Down
5 changes: 5 additions & 0 deletions spec/integ/crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
expect(request.chosenMethod).toBe(null); // nothing chosen yet
expect(request.initiatedByMe).toBe(true);
expect(request.otherUserId).toEqual(TEST_USER_ID);
expect(request.pending).toBe(true);
// we're using fake timers, so the timeout should have exactly 10 minutes left still.
expect(request.timeout).toEqual(600_000);

// and now the request should be visible via `getVerificationRequestsToDeviceInProgress`
{
Expand Down Expand Up @@ -248,6 +251,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
// ... and the whole thing should be done!
await verificationPromise;
expect(request.phase).toEqual(VerificationPhase.Done);
expect(request.pending).toBe(false);

// at this point, cancelling should do nothing.
await request.cancel();
Expand Down Expand Up @@ -564,6 +568,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
expect(request.otherUserId).toEqual(TEST_USER_ID);
expect(request.chosenMethod).toBe(null); // nothing chosen yet
expect(canAcceptVerificationRequest(request)).toBe(true);
expect(request.pending).toBe(true);

// Alice accepts, by sending a to-device message
const sendToDevicePromise = expectSendToDeviceMessage("m.key.verification.ready");
Expand Down
11 changes: 1 addition & 10 deletions spec/unit/rust-crypto/rust-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import "fake-indexeddb/auto";
import { IDBFactory } from "fake-indexeddb";
import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-js";
import { KeysQueryRequest, OlmMachine } from "@matrix-org/matrix-sdk-crypto-js";
import { Mocked } from "jest-mock";
Expand All @@ -41,13 +39,6 @@ import { ServerSideSecretStorage } from "../../../src/secret-storage";
import { CryptoCallbacks, ImportRoomKeysOpts, VerificationRequest } from "../../../src/crypto-api";
import * as testData from "../../test-utils/test-data";

afterEach(() => {
// reset fake-indexeddb after each test, to make sure we don't leak connections
// cf https://github.com/dumbmatter/fakeIndexedDB#wipingresetting-the-indexeddb-for-a-fresh-state
// eslint-disable-next-line no-global-assign
indexedDB = new IDBFactory();
});

const TEST_USER = "@alice:example.com";
const TEST_DEVICE_ID = "TEST_DEVICE";

Expand Down Expand Up @@ -542,5 +533,5 @@ async function makeTestRustCrypto(
secretStorage: ServerSideSecretStorage = {} as ServerSideSecretStorage,
cryptoCallbacks: CryptoCallbacks = {} as CryptoCallbacks,
): Promise<RustCrypto> {
return await initRustCrypto(http, userId, deviceId, secretStorage, cryptoCallbacks);
return await initRustCrypto(http, userId, deviceId, secretStorage, cryptoCallbacks, null);
}
68 changes: 60 additions & 8 deletions spec/unit/rust-crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,49 @@ import { RustVerificationRequest } from "../../../src/rust-crypto/verification";
import { OutgoingRequestProcessor } from "../../../src/rust-crypto/OutgoingRequestProcessor";

describe("VerificationRequest", () => {
describe("startVerification", () => {
describe("pending", () => {
let request: RustVerificationRequest;
let mockedInner: Mocked<RustSdkCryptoJs.VerificationRequest>;
let mockedOutgoingRequestProcessor: Mocked<OutgoingRequestProcessor>;

beforeEach(() => {
mockedInner = makeMockedInner();
request = makeTestRequest(mockedInner);
});

it("returns true for a created request", () => {
expect(request.pending).toBe(true);
});

it("returns false for passive requests", () => {
mockedInner.isPassive.mockReturnValue(true);
expect(request.pending).toBe(false);
});

it("returns false for completed requests", () => {
mockedInner.phase.mockReturnValue(RustSdkCryptoJs.VerificationRequestPhase.Done);
expect(request.pending).toBe(false);
});

it("returns false for cancelled requests", () => {
mockedInner.phase.mockReturnValue(RustSdkCryptoJs.VerificationRequestPhase.Cancelled);
expect(request.pending).toBe(false);
});
});

describe("timeout", () => {
it("passes through the result", () => {
const mockedInner = makeMockedInner();
const request = makeTestRequest(mockedInner);
mockedInner.timeRemainingMillis.mockReturnValue(10_000);
expect(request.timeout).toEqual(10_000);
});
});

describe("startVerification", () => {
let request: RustVerificationRequest;

beforeEach(() => {
mockedInner = {
registerChangesCallback: jest.fn(),
startSas: jest.fn(),
} as unknown as Mocked<RustSdkCryptoJs.VerificationRequest>;
mockedOutgoingRequestProcessor = {} as Mocked<OutgoingRequestProcessor>;
request = new RustVerificationRequest(mockedInner, mockedOutgoingRequestProcessor, undefined);
request = makeTestRequest();
});

it("does not permit methods other than SAS", async () => {
Expand All @@ -48,3 +79,24 @@ describe("VerificationRequest", () => {
});
});
});

/** build a RustVerificationRequest with default parameters */
function makeTestRequest(
inner?: RustSdkCryptoJs.VerificationRequest,
outgoingRequestProcessor?: OutgoingRequestProcessor,
): RustVerificationRequest {
inner ??= makeMockedInner();
outgoingRequestProcessor ??= {} as OutgoingRequestProcessor;
return new RustVerificationRequest(inner, outgoingRequestProcessor, undefined);
}

/** Mock up a rust-side VerificationRequest */
function makeMockedInner(): Mocked<RustSdkCryptoJs.VerificationRequest> {
return {
registerChangesCallback: jest.fn(),
startSas: jest.fn(),
phase: jest.fn().mockReturnValue(RustSdkCryptoJs.VerificationRequestPhase.Created),
isPassive: jest.fn().mockReturnValue(false),
timeRemainingMillis: jest.fn(),
} as unknown as Mocked<RustSdkCryptoJs.VerificationRequest>;
}
5 changes: 4 additions & 1 deletion src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2206,10 +2206,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
*
* @experimental
*
* @param useIndexedDB - True to use an indexeddb store, false to use an in-memory store. Defaults to 'true'.
*
* @returns a Promise which will resolve when the crypto layer has been
* successfully initialised.
*/
public async initRustCrypto(): Promise<void> {
public async initRustCrypto({ useIndexedDB = true }: { useIndexedDB?: boolean } = {}): Promise<void> {
if (this.cryptoBackend) {
logger.warn("Attempt to re-initialise e2e encryption on MatrixClient");
return;
Expand Down Expand Up @@ -2239,6 +2241,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
deviceId,
this.secretStorage,
this.cryptoCallbacks,
useIndexedDB ? RUST_SDK_STORE_PREFIX : null,
);
rustCrypto.supportedVerificationMethods = this.verificationMethods;

Expand Down
11 changes: 9 additions & 2 deletions src/rust-crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import * as RustSdkCryptoJs from "@matrix-org/matrix-sdk-crypto-js";

import { RustCrypto } from "./rust-crypto";
import { logger } from "../logger";
import { RUST_SDK_STORE_PREFIX } from "./constants";
import { IHttpOpts, MatrixHttpApi } from "../http-api";
import { ServerSideSecretStorage } from "../secret-storage";
import { ICryptoCallbacks } from "../crypto";
Expand All @@ -32,13 +31,16 @@ import { ICryptoCallbacks } from "../crypto";
* @param deviceId - The local user's Device ID.
* @param secretStorage - Interface to server-side secret storage.
* @param cryptoCallbacks - Crypto callbacks provided by the application
* @param storePrefix - the prefix to use on the indexeddbs created by rust-crypto.
* If unset, a memory store will be used.
*/
export async function initRustCrypto(
http: MatrixHttpApi<IHttpOpts & { onlyData: true }>,
userId: string,
deviceId: string,
secretStorage: ServerSideSecretStorage,
cryptoCallbacks: ICryptoCallbacks,
storePrefix: string | null,
): Promise<RustCrypto> {
// initialise the rust matrix-sdk-crypto-js, if it hasn't already been done
await RustSdkCryptoJs.initAsync();
Expand All @@ -51,7 +53,12 @@ export async function initRustCrypto(
logger.info("Init OlmMachine");

// TODO: use the pickle key for the passphrase
const olmMachine = await RustSdkCryptoJs.OlmMachine.initialize(u, d, RUST_SDK_STORE_PREFIX, "test pass");
const olmMachine = await RustSdkCryptoJs.OlmMachine.initialize(
u,
d,
storePrefix ?? undefined,
(storePrefix && "test pass") ?? undefined,
);
const rustCrypto = new RustCrypto(olmMachine, http, userId, deviceId, secretStorage, cryptoCallbacks);
await olmMachine.registerRoomKeyUpdatedCallback((sessions: RustSdkCryptoJs.RoomKeyInfo[]) =>
rustCrypto.onRoomKeysUpdated(sessions),
Expand Down
6 changes: 4 additions & 2 deletions src/rust-crypto/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ export class RustVerificationRequest
* (ie it is in phase `Requested`, `Ready` or `Started`).
*/
public get pending(): boolean {
throw new Error("not implemented");
if (this.inner.isPassive()) return false;
const phase = this.phase;
return phase !== VerificationPhase.Done && phase !== VerificationPhase.Cancelled;
}

/**
Expand All @@ -170,7 +172,7 @@ export class RustVerificationRequest
* `null` indicates that there is no timeout
*/
public get timeout(): number | null {
throw new Error("not implemented");
return this.inner.timeRemainingMillis();
}

/** once the phase is Started (and !initiatedByMe) or Ready: common methods supported by both sides */
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@
dependencies:
lodash "^4.17.21"

"@matrix-org/matrix-sdk-crypto-js@^0.1.0-alpha.11":
"@matrix-org/matrix-sdk-crypto-js@^0.1.0":
version "0.1.0"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0.tgz#766580036d4df12120ded223e13b5640e77db136"
integrity sha512-ra/bcFdleC1iRNms2I96UXA0NvQYWpMsHrV5EfJRS7qV1PtnQNvgsvMfjMbkx8QT2ErEmIhsvB5fPCpfp8BSuw==
Expand Down

0 comments on commit 3a694f4

Please sign in to comment.