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

MSC2192: Inline widgets #2192

Open
wants to merge 10 commits into
base: old_master
Choose a base branch
from
Open

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Jul 28, 2019

Rendered

Disclaimer: This is currently a set of ideas and may not be complete or ideal.

Early implementation: matrix-org/matrix-react-sdk#3252
For https://github.com/vector-im/riot-web/issues/10060 and related issues.

@turt2live turt2live added the proposal A matrix spec change proposal label Jul 28, 2019
@turt2live turt2live changed the title Inline widgets (including polls and buttons) MSC2192: Inline widgets (including polls and buttons) Jul 28, 2019
proposals/2192-inline-widgets.md Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
@BillCarsonFr

This comment has been minimized.

@BillCarsonFr

This comment has been minimized.

@BillCarsonFr

This comment has been minimized.

@turt2live
Copy link
Member Author

@BillCarsonFr please always use threads so things can be replied to. I've moved your comments.

MSC1849 is used to define the relationship of a button press to an event, similar to how
MSC1485 did the relationship but instead using MSC1849's topology for the linking.

#### Inline Widgets (use case 1)

Choose a reason for hiding this comment

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

I think this concept is really flawed and this msc should not be accepted with it in place, for a few reasons:

  • Embedding arbitrary, interactive, user-defined content directly into the timeline is an absolute dream for phishers. Since clients like Riot already often ask users for passwords, a correctly styled "Enter your password to view message" widget would likely trick most users.
  • Embedding widgets is also a dream for tracking and de-anonymisation. Get people's IP addresses, fingerprint their browsers, and connect it to their mxid.
  • The above applies to room widgets too but: the nature of them being embedded in the timeline makes the required level of education infeasible. Since they are ephemeral, a confirmation would be required every time the widget is viewed. Big scary popups will quickly lead to dialog fatigue.
  • It makes non-web clients second class citizens and more or less forces all clients which want to provide a full-featured room timeline view to more or less embed chromium inside their app. Widgets made specifically for matrix will be styled to match a specific client, and look out of place in everything else.
  • Even if native clients solve this, it still prevents them from offering better experiences for actions. For example, a client might want to show a native video player, or a CLI client might want to provide a playback bar for audio.
  • For all of the issues it causes, it's not actually providing a lot of benefit over other solutions. I assume the main use of these embeds would be media, and prompting for response to some kind of event. But clients can already provide embeds for Youtube videos and other content by looking for links and embedding them via https://en.wikipedia.org/wiki/OEmbed or similar. And responses to events are better handled using the buttons described below. If all widgets need to be confirmed before loading anyway, is it really that much better than a link?

In general, I think this is a case of excessive generality. It's nice to say "this can theoretically do everything!" but it's worth remembering the cost that puts on everyone else in an ecosystem. I think that it would be better to look at specific problems to solve and find solutions for those.

Choose a reason for hiding this comment

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

While these aren't great for public rooms I'd agree, I think for enterprises and businesses running Matrix these are a killer feature. There could be some kind of access limit per room, you can say disable these "embedded widgets" in this room, may mitigate this issue.

For some examples of how these bots are used in a business, see slacks article: https://slackhq.com/slack-on-slack-unlocking-productivity-with-custom-slack-bots

Choose a reason for hiding this comment

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

@the0d0re9 I think you're misunderstanding. Slack does not allow you to insert arbitrary interactive html/js content. They allow you to add a very limited set of pre-defined widgets to a message. This is also what the rest of this MSC does and I do not object to it.

@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020

Allowing arbitrary embeds opens up a spam vector for auto-playing videos, scare content,
and similar spam. Clients should do their best to avoid these kinds of attacks, such as
by blocking the widget from loading until the user accepts the widget.

Choose a reason for hiding this comment

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

Add ability for rooms and users to control how to present the widget.

for example, room admin may config:

  • some people is trusted to send poll and present as widget,
  • some people is not trusted enough so whether present as widget is determined by the end user,
  • other people is prohibit so always show as text.

A widget present as widget only if room and user bother allowed.

proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
proposals/2192-inline-widgets.md Outdated Show resolved Hide resolved
@erkinalp
Copy link

Allowed HTML tags can be extended instead.

@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
@turt2live turt2live changed the title MSC2192: Inline widgets (including polls and buttons) MSC2192: Inline widgets Oct 29, 2021
@@ -0,0 +1,240 @@
# Proposal for inline widgets
Copy link
Member Author

Choose a reason for hiding this comment

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

we should use extensible events here

"content": {
"m.text": "https://www.youtube.com/watch?v=a4L94Rsg_nM",
"m.widget": {
"url": "https://www.youtube.com/embed/a4L94Rsg_nM",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think only allowing HTTP URLs here could be a potential privacy hazard, what about allowing MXC URIs as well, as an alternative to included HTML?

Copy link

@tcpipuk tcpipuk left a comment

Choose a reason for hiding this comment

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

Accidentally created #4214 because I didn't know this existed, but have closed as duplicate - great work! I've just got a few questions that feel relevant for 2024 clients.

opened up the client to several XSS and similar security vulnerabilities due to the complex
nature of untrusted user-provided HTML being rendered in the client.

## Security considerations
Copy link

@tcpipuk tcpipuk Oct 11, 2024

Choose a reason for hiding this comment

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

Would it be feasible to add a power_level event for sending m.widget content or m.embed events, so rooms can restrict who can embed widgets?

Comment on lines +52 to +53
* **Required** - An `m.text` block to act as a fallback for clients which can't process
inline widgets.
Copy link

Choose a reason for hiding this comment

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

Potentially this could also be displayed as a caption so users can explain why someone would want to approve the widget opening in their client? I'd definitely feel more comfortable being prompted before loading external content that may identify my location/machine/etc.

opened up the client to several XSS and similar security vulnerabilities due to the complex
nature of untrusted user-provided HTML being rendered in the client.

## Security considerations
Copy link

@tcpipuk tcpipuk Oct 11, 2024

Choose a reason for hiding this comment

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

Might be worth mentioning the sandbox attribute for iframes, and CSP enforcement, e.g. a mechanism for either users or servers to define an allowlist of domains to avoid random arbitrary widgets being loaded?

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 needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. 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.