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

Element-R: Implement VerificationRequest.accept #3526

Merged
merged 4 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 44 additions & 12 deletions spec/integ/crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -541,24 +541,18 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
});

describe("Incoming verification from another device", () => {
beforeEach(() => {
beforeEach(async () => {
e2eKeyResponder.addDeviceKeys(TEST_USER_ID, TEST_DEVICE_ID, SIGNED_TEST_DEVICE_DATA);
});

oldBackendOnly("Incoming verification: can accept", async () => {
aliceClient = await startTestClient();
await waitForDeviceList();
});

it("Incoming verification: can accept", async () => {
const TRANSACTION_ID = "abcd";

// Initiate the request by sending a to-device message
returnToDeviceMessageFromSync({
type: "m.key.verification.request",
content: {
from_device: TEST_DEVICE_ID,
methods: ["m.sas.v1"],
transaction_id: TRANSACTION_ID,
timestamp: Date.now() - 1000,
},
});
returnToDeviceMessageFromSync(buildRequestMessage(TRANSACTION_ID));
const request: VerificationRequest = await emitPromise(
aliceClient,
CryptoEvent.VerificationRequestReceived,
Expand All @@ -585,6 +579,31 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
expect(toDeviceMessage.from_device).toEqual(aliceClient.deviceId);
expect(toDeviceMessage.transaction_id).toEqual(TRANSACTION_ID);
});

it("Incoming verification: can refuse", async () => {
const TRANSACTION_ID = "abcd";

// Initiate the request by sending a to-device message
returnToDeviceMessageFromSync(buildRequestMessage(TRANSACTION_ID));
const request: VerificationRequest = await emitPromise(
aliceClient,
CryptoEvent.VerificationRequestReceived,
);
expect(request.transactionId).toEqual(TRANSACTION_ID);

// Alice declines, by sending a cancellation
const sendToDevicePromise = expectSendToDeviceMessage("m.key.verification.cancel");
const cancelPromise = request.cancel();
expect(canAcceptVerificationRequest(request)).toBe(false);
expect(request.accepting).toBe(false);
expect(request.declining).toBe(true);
await cancelPromise;
const requestBody = await sendToDevicePromise;
expect(request.phase).toEqual(VerificationPhase.Cancelled);

const toDeviceMessage = requestBody.messages[TEST_USER_ID][TEST_DEVICE_ID];
expect(toDeviceMessage.transaction_id).toEqual(TRANSACTION_ID);
});
});

async function startTestClient(opts: Partial<ICreateClientOpts> = {}): Promise<MatrixClient> {
Expand Down Expand Up @@ -668,6 +687,19 @@ function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string {
return Buffer.from(uint8Array).toString("base64").replace(/=+$/g, "");
}

/** build an m.key.verification.request to-device message originating from the dummy device */
function buildRequestMessage(transactionId: string): { type: string; content: object } {
return {
type: "m.key.verification.request",
content: {
from_device: TEST_DEVICE_ID,
methods: ["m.sas.v1"],
transaction_id: transactionId,
timestamp: Date.now() - 1000,
},
};
}

/** build an m.key.verification.ready to-device message originating from the dummy device */
function buildReadyMessage(transactionId: string, methods: string[]): { type: string; content: object } {
return {
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/rust-crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe("VerificationRequest", () => {
startSas: jest.fn(),
} as unknown as Mocked<RustSdkCryptoJs.VerificationRequest>;
mockedOutgoingRequestProcessor = {} as Mocked<OutgoingRequestProcessor>;
request = new RustVerificationRequest(mockedInner, mockedOutgoingRequestProcessor);
request = new RustVerificationRequest(mockedInner, mockedOutgoingRequestProcessor, undefined);
});

it("does not permit methods other than SAS", async () => {
Expand Down
15 changes: 11 additions & 4 deletions src/rust-crypto/rust-crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,14 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
);
return requests
.filter((request) => request.roomId === undefined)
.map((request) => new RustVerificationRequest(request, this.outgoingRequestProcessor));
.map(
(request) =>
new RustVerificationRequest(
request,
this.outgoingRequestProcessor,
this.supportedVerificationMethods,
),
);
}

/**
Expand Down Expand Up @@ -600,7 +607,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
this.supportedVerificationMethods?.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor);
return new RustVerificationRequest(request, this.outgoingRequestProcessor, this.supportedVerificationMethods);
}

/**
Expand Down Expand Up @@ -630,7 +637,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
this.supportedVerificationMethods?.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor);
return new RustVerificationRequest(request, this.outgoingRequestProcessor, this.supportedVerificationMethods);
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -784,7 +791,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
if (request) {
this.emit(
CryptoEvent.VerificationRequestReceived,
new RustVerificationRequest(request, this.outgoingRequestProcessor),
new RustVerificationRequest(request, this.outgoingRequestProcessor, this.supportedVerificationMethods),
);
}
}
Expand Down
62 changes: 54 additions & 8 deletions src/rust-crypto/verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,25 @@ export class RustVerificationRequest
extends TypedEventEmitter<VerificationRequestEvent, VerificationRequestEventHandlerMap>
implements VerificationRequest
{
/** Are we in the process of sending an `m.key.verification.ready` event? */
private _accepting = false;

/** Are we in the process of sending an `m.key.verification.cancellation` event? */
private _cancelling = false;

private _verifier: Verifier | undefined;

/**
* Construct a new RustVerificationRequest to wrap the rust-level `VerificationRequest`.
*
* @param inner - VerificationRequest from the Rust SDK
* @param outgoingRequestProcessor - `OutgoingRequestProcessor` to use for making outgoing HTTP requests
* @param supportedVerificationMethods - Verification methods to use when `accept()` is called
*/
public constructor(
private readonly inner: RustSdkCryptoJs.VerificationRequest,
private readonly outgoingRequestProcessor: OutgoingRequestProcessor,
private readonly supportedVerificationMethods: string[] | undefined,
) {
super();

Expand Down Expand Up @@ -113,7 +127,9 @@ export class RustVerificationRequest
case RustSdkCryptoJs.VerificationRequestPhase.Requested:
return VerificationPhase.Requested;
case RustSdkCryptoJs.VerificationRequestPhase.Ready:
return VerificationPhase.Ready;
// if we're still sending the `m.key.verification.ready`, that counts as "Requested" in the js-sdk's
// parlance.
return this._accepting ? VerificationPhase.Requested : VerificationPhase.Ready;
case RustSdkCryptoJs.VerificationRequestPhase.Transitioned:
return VerificationPhase.Started;
case RustSdkCryptoJs.VerificationRequestPhase.Done:
Expand All @@ -137,15 +153,15 @@ export class RustVerificationRequest
* the remote echo which causes a transition to {@link VerificationPhase.Ready}.
*/
public get accepting(): boolean {
throw new Error("not implemented");
return this._accepting;
}

/**
* True if we have started the process of sending an `m.key.verification.cancel` (but have not necessarily received
* the remote echo which causes a transition to {@link VerificationPhase.Cancelled}).
*/
public get declining(): boolean {
throw new Error("not implemented");
return this._cancelling;
}

/**
Expand Down Expand Up @@ -198,8 +214,28 @@ export class RustVerificationRequest
*
* @returns Promise which resolves when the event has been sent.
*/
public accept(): Promise<void> {
throw new Error("not implemented");
public async accept(): Promise<void> {
if (this.inner.phase() !== RustSdkCryptoJs.VerificationRequestPhase.Requested || this._accepting) {
throw new Error(`Cannot accept a verification request in phase ${this.phase}`);
}

this._accepting = true;
try {
const req: undefined | OutgoingRequest =
this.supportedVerificationMethods === undefined
? this.inner.accept()
: this.inner.acceptWithMethods(
this.supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
);
if (req) {
await this.outgoingRequestProcessor.makeOutgoingRequest(req);
}
} finally {
this._accepting = false;
}

// phase may have changed, so emit a 'change' event
this.emit(VerificationRequestEvent.Change);
}

/**
Expand All @@ -211,9 +247,19 @@ export class RustVerificationRequest
* @returns Promise which resolves when the event has been sent.
*/
public async cancel(params?: { reason?: string; code?: string }): Promise<void> {
const req: undefined | OutgoingRequest = this.inner.cancel();
if (req) {
await this.outgoingRequestProcessor.makeOutgoingRequest(req);
if (this._cancelling) {
// already cancelling; do nothing
return;
}

this._cancelling = true;
try {
const req: undefined | OutgoingRequest = this.inner.cancel();
if (req) {
await this.outgoingRequestProcessor.makeOutgoingRequest(req);
}
} finally {
this._cancelling = false;
}
}

Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1427,9 +1427,9 @@
lodash "^4.17.21"

"@matrix-org/matrix-sdk-crypto-js@^0.1.0-alpha.11":
version "0.1.0-alpha.11"
resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-js/-/matrix-sdk-crypto-js-0.1.0-alpha.11.tgz#24d705318c3159ef7dbe43bca464ac2bdd11e45d"
integrity sha512-HD3rskPkqrUUSaKzGLg97k/bN+OZrkcX7ODB/pNBs/jqq+/A0wDKqsszJotzFwsQcDPpWn78BmMyvBo4tLxKjw==
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==
Comment on lines +1430 to +1432
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.1.0 fixes a bug in cancellation which you'll likely never notice unless you happen to be an integration test


"@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.14.tgz":
version "3.2.14"
Expand Down