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

MSC3819: Allowing widgets to send/receive to-device messages #3819

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

@turt2live turt2live changed the title Allowing widgets to send/receive to-device messages MSC3819: Allowing widgets to send/receive to-device messages May 19, 2022
@turt2live turt2live added proposal A matrix spec change proposal kind:feature MSC for not-core and not-maintenance stuff widgets anything to do with widgets needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels May 19, 2022
@turt2live turt2live marked this pull request as ready for review May 19, 2022 20:43
Comment on lines +78 to +85
"data": {
// Same structure as the `/sendToDevice` HTTP API request body
"@target:example.org": {
"DEVICEID": { // can also be a `*` to denote "all of the user's devices"
"example_content": "put your real message here"
}
}
}
Copy link
Member

@robintown robintown Jun 22, 2022

Choose a reason for hiding this comment

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

We need to include the event type somewhere in here. I'd suggest an interface of

{
  type: string;
  messages: { [userId: string]: { [deviceId: string]: unknown } };
}

Copy link
Member

@robintown robintown Jul 15, 2022

Choose a reason for hiding this comment

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

We also need to control whether the event is encrypted or not

{
  type: string;
  encrypted: boolean;
  messages: { [userId: string]: { [deviceId: string]: unknown } };
}

Copy link

Choose a reason for hiding this comment

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

I noticed an issue with the current implementation. The payload (unknown above) varies between encrypted: true and encrypted: false in Element Web right now. For unencrypted messages it is directly the content of the message, like {"key": "value"}, but for encrypted events it has to be wrapped like {"type": "event type…", "content": {"key": "value"} }. If that is really desired and not just a mistake, we should make sure to document the behavior.

I created an issue for it: element-hq/element-web#24470

}
```

## Potential issues
Copy link

Choose a reason for hiding this comment

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

While working with the current support of MSC3819 in Element we noticed a problem:

The send endpoint of the toDevice message API allows to target specific device
ids, but there is currently no way to know a specific device id for a widget.
Neither is it possible to embed a device id into the URL,
nor does a received toDevice message indicate the sender device to reply to it.
Sure, it is possible to target all devices via *, but using a specific device
id makes sending much more efficient.

Element Call, which this MSC initially was created for, is cheating a little bit.
As Element Call is a special widget anyway, it is created with
custom code instead of the normal widget API setup code.
This allows to embed additional parameters into the widget URL, including the device id.
From the perspective of bringing the product to the market quickly, I think it
is fine, but we should focus on getting this MSC into a complete state.

Therefore I suggest to amend small section here:

**Passing device id to widgets**

A new template variable is added to the available options for a widget URL:

* `device_id` - The device id of the client instance which is rendering the widget.

And extend the unstable prefix section with:

* `org.matrix.msc3819.device_id` in place of the proposed variable.

If this extension is desired, I'm open to add the implementation to the matrix-react-sdk and matrix-widget-api.

Copy link

Choose a reason for hiding this comment

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

I started working on implementing this addition over at matrix-org/matrix-widget-api#78

Choose a reason for hiding this comment

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

In matrix-org/matrix-widget-api#78, we decided to slightly rename the parameter. Thus the amendment should be as follows:

**Passing device id to widgets**

A new template variable is added to the available options for a widget URL:

* `matrix_device_id` - The device id of the client instance which is rendering the widget.

And extend the unstable prefix section with:

* `org.matrix.msc3819.matrix_device_id` in place of the proposed variable.

Fox32 added a commit to nordeck/matrix-widget-api that referenced this pull request Feb 21, 2023
I mentioned the need for this parameter in the related [MSC 3891 ](matrix-org/matrix-spec-proposals#3819) which implements to device messages. They are relying on the device id to be available, but there is currently no way to access a device id from within a widget (Element Call does this but is cheating :P). Therefore [I propose to include the device id in the available parameters](matrix-org/matrix-spec-proposals#3819 (comment)).

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
Fox32 added a commit to nordeck/matrix-react-sdk that referenced this pull request Feb 21, 2023
Implement the [comment in MSC 3819](matrix-org/matrix-spec-proposals#3819 (comment)) which requests passing a device id to a widget.

This is based on the previous work in the matrix-widget-api: matrix-org/matrix-widget-api#78

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
Fox32 added a commit to nordeck/matrix-react-sdk that referenced this pull request Feb 21, 2023
Implement the [comment in MSC 3819](matrix-org/matrix-spec-proposals#3819 (comment)) which requests passing a device id to a widget.

This is based on the previous work in the matrix-widget-api: matrix-org/matrix-widget-api#78

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
dbkr pushed a commit to matrix-org/matrix-widget-api that referenced this pull request Mar 23, 2023
* Extend url template variables with device_id

I mentioned the need for this parameter in the related [MSC 3891 ](matrix-org/matrix-spec-proposals#3819) which implements to device messages. They are relying on the device id to be available, but there is currently no way to access a device id from within a widget (Element Call does this but is cheating :P). Therefore [I propose to include the device id in the available parameters](matrix-org/matrix-spec-proposals#3819 (comment)).

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

* Add the `matrix_` prefix to the parameter

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

---------

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Co-authored-by: Dominik Henneke <dominik.henneke@nordeck.net>
dhenneke pushed a commit to nordeck/matrix-react-sdk that referenced this pull request May 8, 2023
Implement the [comment in MSC 3819](matrix-org/matrix-spec-proposals#3819 (comment)) which requests passing a device id to a widget.

This is based on the previous work in the matrix-widget-api: matrix-org/matrix-widget-api#78

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this pull request May 23, 2023
* Pass device id to widget

Implement the [comment in MSC 3819](matrix-org/matrix-spec-proposals#3819 (comment)) which requests passing a device id to a widget.

This is based on the previous work in the matrix-widget-api: matrix-org/matrix-widget-api#78

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

* Include all data that is shared in the permissions screen

* Update matrix-widget-api to version 1.4.0

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

* Fix type and test

Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>

---------

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Co-authored-by: Dominik Henneke <dominik.henneke@nordeck.net>
Co-authored-by: Robin <robin@robin.town>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal widgets anything to do with widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants