diff --git a/CHANGELOG.md b/CHANGELOG.md index 901e1ce4d..4803f6bd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ attached to tracing spans. Remove the `tracing` feature: tracing support is now always included. +- BugFix: `ToDeviceRequest` returned by `shareRoomKey(..)` always had an `undefined` `id` field. + # matrix-sdk-crypto-wasm v2.0.0 - Updated rust sdk version to revision [c2bb76029ae6d99c741727e0f87abcd734377016](https://github.com/matrix-org/matrix-rust-sdk/commit/c2bb76029ae6d99c741727e0f87abcd734377016), including: diff --git a/src/requests.rs b/src/requests.rs index ec7568689..cb5640ded 100644 --- a/src/requests.rs +++ b/src/requests.rs @@ -34,7 +34,7 @@ use wasm_bindgen::prelude::*; pub struct KeysUploadRequest { /// The request ID. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A JSON-encoded string containing the rest of the payload: `device_keys`, /// `one_time_keys`, `fallback_keys`. @@ -49,7 +49,7 @@ impl KeysUploadRequest { /// Create a new `KeysUploadRequest`. #[wasm_bindgen(constructor)] pub fn new(id: JsString, body: JsString) -> KeysUploadRequest { - Self { id: Some(id), body } + Self { id, body } } /// Get its request type. @@ -70,7 +70,7 @@ impl KeysUploadRequest { pub struct KeysQueryRequest { /// The request ID. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A JSON-encoded string containing the rest of the payload: `timeout`, /// `device_keys`, `token`. @@ -85,7 +85,7 @@ impl KeysQueryRequest { /// Create a new `KeysQueryRequest`. #[wasm_bindgen(constructor)] pub fn new(id: JsString, body: JsString) -> KeysQueryRequest { - Self { id: Some(id), body } + Self { id, body } } /// Get its request type. @@ -107,7 +107,7 @@ impl KeysQueryRequest { pub struct KeysClaimRequest { /// The request ID. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A JSON-encoded string containing the rest of the payload: `timeout`, /// `one_time_keys`. @@ -122,7 +122,7 @@ impl KeysClaimRequest { /// Create a new `KeysClaimRequest`. #[wasm_bindgen(constructor)] pub fn new(id: JsString, body: JsString) -> KeysClaimRequest { - Self { id: Some(id), body } + Self { id, body } } /// Get its request type. @@ -142,8 +142,11 @@ impl KeysClaimRequest { #[wasm_bindgen(getter_with_clone)] pub struct ToDeviceRequest { /// The request ID. + /// For to-device request this would be the same value as `txn_id`. It is + /// exposed also as `id` so that the js bindings are consistent with the + /// other request types by using this field to mark as sent. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A string representing the type of event being sent to each devices. #[wasm_bindgen(readonly)] @@ -171,7 +174,7 @@ impl ToDeviceRequest { txn_id: JsString, body: JsString, ) -> ToDeviceRequest { - Self { id: Some(id), event_type, txn_id, body } + Self { id, event_type, txn_id, body } } /// Get its request type. @@ -191,6 +194,9 @@ impl ToDeviceRequest { #[wasm_bindgen(getter_with_clone)] pub struct SignatureUploadRequest { /// The request ID. + /// Some signature upload will have to an `id` field, some won't. + /// They have one when they are created automatically during an interactive + /// verification, otherwise they don't. #[wasm_bindgen(readonly)] pub id: Option, @@ -225,7 +231,7 @@ impl SignatureUploadRequest { pub struct RoomMessageRequest { /// The request ID. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A string representing the room to send the event to. #[wasm_bindgen(readonly)] @@ -259,7 +265,7 @@ impl RoomMessageRequest { event_type: JsString, content: JsString, ) -> RoomMessageRequest { - Self { id: Some(id), room_id, txn_id, event_type, content } + Self { id, room_id, txn_id, event_type, content } } /// Get its request type. @@ -278,7 +284,7 @@ impl RoomMessageRequest { pub struct KeysBackupRequest { /// The request ID. #[wasm_bindgen(readonly)] - pub id: Option, + pub id: JsString, /// A JSON-encoded string containing the rest of the payload: `rooms`. /// @@ -296,7 +302,7 @@ impl KeysBackupRequest { /// Create a new `KeysBackupRequest`. #[wasm_bindgen(constructor)] pub fn new(id: JsString, body: JsString, version: JsString) -> KeysBackupRequest { - Self { id: Some(id), body, version } + Self { id, body, version } } /// Get its request type. @@ -306,59 +312,12 @@ impl KeysBackupRequest { } } -/** Other Requests * */ - -/// Request that will publish a cross signing identity. -/// -/// This uploads the public cross signing key triplet. -#[wasm_bindgen(getter_with_clone)] -#[derive(Debug)] -pub struct SigningKeysUploadRequest { - /// The request ID. - #[wasm_bindgen(readonly)] - pub id: Option, - - /// A JSON-encoded string containing the rest of the payload: `master_key`, - /// `self_signing_key`, `user_signing_key`. - /// - /// It represents the body of the HTTP request. - #[wasm_bindgen(readonly)] - pub body: JsString, -} - -#[wasm_bindgen] -impl SigningKeysUploadRequest { - /// Create a new `SigningKeysUploadRequest`. - #[wasm_bindgen(constructor)] - pub fn new(id: JsString, body: JsString) -> SigningKeysUploadRequest { - Self { id: Some(id), body } - } - - /// Get its request type. - #[wasm_bindgen(getter, js_name = "type")] - pub fn request_type(&self) -> RequestType { - RequestType::SigningKeysUpload - } -} - macro_rules! request { ( $destination_request:ident from $source_request:ident $( extracts $( $field_name:ident : $field_type:tt ),+ $(,)? )? $( $( and )? groups $( $grouped_field_name:ident ),+ $(,)? )? ) => { - impl TryFrom<&$source_request> for $destination_request { - type Error = serde_json::Error; - - fn try_from(request: &$source_request) -> Result { - request!( - @__try_from $destination_request from $source_request - (request_id = None, request = request) - $( extracts [ $( $field_name : $field_type, )+ ] )? - $( groups [ $( $grouped_field_name, )+ ] )? - ) - } - } impl TryFrom<(String, &$source_request)> for $destination_request { type Error = serde_json::Error; @@ -368,7 +327,7 @@ macro_rules! request { ) -> Result { request!( @__try_from $destination_request from $source_request - (request_id = Some(request_id.into()), request = request) + (request_id = request_id.into(), request = request) $( extracts [ $( $field_name : $field_type, )+ ] )? $( groups [ $( $grouped_field_name, )+ ] )? ) @@ -422,17 +381,80 @@ macro_rules! request { }; } -// Outgoing Requests +// Generate the methods needed to convert rust `OutgoingRequests` into the js +// counterpart. Technically it's converting tuples `(String, &Original)`, where +// the first member is the request ID, into js requests. Used by +// `TryFrom for JsValue`. request!(KeysUploadRequest from OriginalKeysUploadRequest groups device_keys, one_time_keys, fallback_keys); request!(KeysQueryRequest from OriginalKeysQueryRequest groups timeout, device_keys); request!(KeysClaimRequest from OriginalKeysClaimRequest groups timeout, one_time_keys); request!(ToDeviceRequest from OriginalToDeviceRequest extracts event_type: string, txn_id: string and groups messages); -request!(SignatureUploadRequest from OriginalSignatureUploadRequest extracts signed_keys: json); request!(RoomMessageRequest from OriginalRoomMessageRequest extracts room_id: string, txn_id: string, event_type: event_type, content: json); request!(KeysBackupRequest from OriginalKeysBackupRequest extracts version: string and groups rooms); -// Other Requests -request!(SigningKeysUploadRequest from OriginalUploadSigningKeysRequest groups master_key, self_signing_key, user_signing_key); +// Specific conversion for signature upload as they can be returned directly by +// some verification API and not via `outgoing_requests()`. If returned by +// `outgoing_requests()` they would need to be marked as sent using `id`, +// otherwise no need to mark them as sent. This is why `SignatureUploadRequest` +// has an optional `id` field. It is set to some when converting from +// `OutgoingRequest`, and to None if not. +impl TryFrom<&OriginalSignatureUploadRequest> for SignatureUploadRequest { + type Error = serde_json::Error; + fn try_from(request: &OriginalSignatureUploadRequest) -> Result { + { + Ok(SignatureUploadRequest { + id: None, + signed_keys: serde_json::to_string(&request.signed_keys)?.into(), + }) + } + } +} + +// Manual into/from for signature upload requests as the id is optional. +impl TryFrom<(String, &OriginalSignatureUploadRequest)> for SignatureUploadRequest { + type Error = serde_json::Error; + fn try_from( + (request_id, request): (String, &OriginalSignatureUploadRequest), + ) -> Result { + { + Ok(SignatureUploadRequest { + id: Some(request_id.into()), + signed_keys: serde_json::to_string(&request.signed_keys)?.into(), + }) + } + } +} + +// ToDeviceRequests can be returned directly from `OlmMachine::share_room_key`, +// instead of being wrapped in a `matrix_sdk_crypto::OutgoingRequest` via +// `OlmMachine::outgoing_requests`. +// +// We therefore need a custom conversion implementation to deal with that case. +impl TryFrom<&OriginalToDeviceRequest> for ToDeviceRequest { + type Error = serde_json::Error; + fn try_from(request: &OriginalToDeviceRequest) -> Result { + { + Ok(ToDeviceRequest { + // When matrix_sdk_crypto returns requests via `share_room_key`, they must be sent + // out to the server and the responses need to be passed back to the + // olmMachine using [`mark_request_as_sent`], using the `txn_id` as + // `request_id` + id: request.txn_id.to_string().into(), + event_type: request.event_type.to_string().into(), + txn_id: request.txn_id.to_string().into(), + body: { + let mut map = serde_json::Map::new(); + map.insert( + "messages".to_owned(), + serde_json::to_value(&request.messages).unwrap(), + ); + let object = serde_json::Value::Object(map); + serde_json::to_string(&object)?.into() + }, + }) + } + } +} // JavaScript has no complex enums like Rust. To return structs of // different types, we have no choice that hiding everything behind a @@ -501,7 +523,56 @@ pub enum RequestType { /// Represents a `KeysBackupRequest`. KeysBackup, +} + +/** Other Requests * */ + +/// Request that will publish a cross signing identity. +/// +/// This uploads the public cross signing key triplet. +#[wasm_bindgen(getter_with_clone)] +#[derive(Debug)] +pub struct SigningKeysUploadRequest { + /// A JSON-encoded string containing the rest of the payload: `master_key`, + /// `self_signing_key`, `user_signing_key`. + /// + /// It represents the body of the HTTP request. + #[wasm_bindgen(readonly)] + pub body: JsString, +} - /// Represents a `SigningKeysUploadRequest` - SigningKeysUpload, +#[wasm_bindgen] +impl SigningKeysUploadRequest { + /// Create a new `SigningKeysUploadRequest`. + #[wasm_bindgen(constructor)] + pub fn new(body: JsString) -> SigningKeysUploadRequest { + Self { body } + } +} + +impl TryFrom<&OriginalUploadSigningKeysRequest> for SigningKeysUploadRequest { + type Error = serde_json::Error; + fn try_from(request: &OriginalUploadSigningKeysRequest) -> Result { + { + Ok(SigningKeysUploadRequest { + body: { + let mut map = serde_json::Map::new(); + map.insert( + "master_key".to_owned(), + serde_json::to_value(&request.master_key).unwrap(), + ); + map.insert( + "self_signing_key".to_owned(), + serde_json::to_value(&request.self_signing_key).unwrap(), + ); + map.insert( + "user_signing_key".to_owned(), + serde_json::to_value(&request.user_signing_key).unwrap(), + ); + let object = serde_json::Value::Object(map); + serde_json::to_string(&object)?.into() + }, + }) + } + } } diff --git a/src/responses.rs b/src/responses.rs index 8f287c843..58db388df 100644 --- a/src/responses.rs +++ b/src/responses.rs @@ -7,7 +7,6 @@ pub(crate) use matrix_sdk_common::ruma::api::client::{ claim_keys::v3::Response as KeysClaimResponse, get_keys::v3::Response as KeysQueryResponse, upload_keys::v3::Response as KeysUploadResponse, upload_signatures::v3::Response as SignatureUploadResponse, - upload_signing_keys::v3::Response as SigningKeysUploadResponse, }, message::send_message_event::v3::Response as RoomMessageResponse, to_device::send_event_to_device::v3::Response as ToDeviceResponse, @@ -35,7 +34,6 @@ pub(crate) enum OwnedResponse { SignatureUpload(SignatureUploadResponse), RoomMessage(RoomMessageResponse), KeysBackup(KeysBackupResponse), - SigningKeysUpload(SigningKeysUploadResponse), } impl From for OwnedResponse { @@ -80,12 +78,6 @@ impl From for OwnedResponse { } } -impl From for OwnedResponse { - fn from(response: SigningKeysUploadResponse) -> OwnedResponse { - Self::SigningKeysUpload(response) - } -} - impl TryFrom<(RequestType, http::Response>)> for OwnedResponse { type Error = JsError; @@ -130,18 +122,6 @@ impl TryFrom<(RequestType, http::Response>)> for OwnedResponse { RequestType::KeysBackup => { KeysBackupResponse::try_from_http_response(response).map(Into::into) } - - RequestType::SigningKeysUpload => { - // SigningKeysUploadResponse::try_from_http_response returns a - // FromHttpResponseError instead of a - // ruma_client_api::error::Error, so we have to handle it - // separately. In practice, the application is supposed to - // have dealt with UIA before we get here, so we just map it straight into a - // regular `JsError` anyway. - return SigningKeysUploadResponse::try_from_http_response(response) - .map(Into::into) - .map_err(JsError::from); - } } .map_err(JsError::from) } @@ -161,9 +141,6 @@ impl<'a> From<&'a OwnedResponse> for IncomingResponse<'a> { OwnedResponse::SignatureUpload(response) => IncomingResponse::SignatureUpload(response), OwnedResponse::RoomMessage(response) => IncomingResponse::RoomMessage(response), OwnedResponse::KeysBackup(response) => IncomingResponse::KeysBackup(response), - OwnedResponse::SigningKeysUpload(response) => { - IncomingResponse::SigningKeysUpload(response) - } } } } diff --git a/tests/machine.test.js b/tests/machine.test.js index 93e2d4636..68347f0c1 100644 --- a/tests/machine.test.js +++ b/tests/machine.test.js @@ -127,7 +127,8 @@ describe(OlmMachine.name, () => { const room = new RoomId("!baz:matrix.org"); function machine(new_user, new_device) { - new RustSdkCryptoJs.Tracing(RustSdkCryptoJs.LoggerLevel.Trace).turnOn(); + // Uncomment to enable debug logging for tests + // new RustSdkCryptoJs.Tracing(RustSdkCryptoJs.LoggerLevel.Trace).turnOn(); return OlmMachine.initialize(new_user || user, new_device || device); } @@ -484,8 +485,13 @@ describe(OlmMachine.name, () => { expect(requests[0]).toBeInstanceOf(ToDeviceRequest); expect(requests[0].event_type).toEqual("m.room.encrypted"); expect(requests[0].txn_id).toBeDefined(); + expect(requests[0].id).toBeDefined(); const content = JSON.parse(requests[0].body); expect(Object.keys(content.messages)).toEqual(["@example:localhost"]); + + await m.markRequestAsSent(requests[0].id, RequestType.ToDevice, "{}"); + const requestsAfterMarkedAsSent = await m.shareRoomKey(room, other_users, new EncryptionSettings()); + expect(requestsAfterMarkedAsSent).toHaveLength(0); }); let encrypted;