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

MSC3790: Register Clients #3790

Closed
wants to merge 7 commits into from
Closed

MSC3790: Register Clients #3790

wants to merge 7 commits into from

Conversation

xarvic
Copy link

@xarvic xarvic commented May 9, 2022

Rendered

Signed-off-by: Christoph König jan.christoph.koenig@posteo.net

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Please also sign off on your changes when you get a chance.

proposals/3788-register-clients.md Outdated Show resolved Hide resolved
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API 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. labels May 9, 2022
@xarvic
Copy link
Author

xarvic commented May 9, 2022

Wow that was quick, thanks for the reply :)

Please also sign off on your changes when you get a chance.

Just by editing the comment?

@turt2live turt2live changed the title MSC3788: Register Clients MSC3790: Register Clients May 9, 2022
@turt2live
Copy link
Member

Sign off can also be a comment in the PR description using the template:

Signed-off-by: Your Name <email@example.com>

@xarvic
Copy link
Author

xarvic commented May 9, 2022

After reading the MSC again i noticed one problem, that it is not that easy to detect whether two clients are one the same device, since the device_id is actually a client_id. Maybe querying the device name could help?


`client_name` is the app name.
`client_device_id` is the id the device this client is located on.
`launch_command` is the command to run when the user wants to inspect one of the associated_event_types.
Copy link
Member

Choose a reason for hiding this comment

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

This will only work if the command exists on the computer that your current client is running on. Similarly, if you try to use launch_url on a different computer/browser, then it will create a new session instead of using the existing session, which is presumably what you would want it to do.

Copy link
Author

Choose a reason for hiding this comment

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

You are right. Is there a way to detect if two clients are on the same device? If so the chat client should probably filter out the rest.

Copy link
Member

Choose a reason for hiding this comment

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

Strictly speaking, no. Browser sandboxes are actually supposed to prevent web applications from doing that, and application confinement mechanisms like Flatpak normally deny most of insight into the local environment to the application as well.


Using enviorment varaibles has the advantage that this functionality can be implemented as a plugin for an existing app.

To highlight certain events a client can use "m.event_description" event.
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear why a client would want to "highlight" an event, what it's supposed to do, and how it's related to the rest of this proposal.

Copy link
Author

@xarvic xarvic May 9, 2022

Choose a reason for hiding this comment

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

The idea was that by default events just get a small infotext combined with a link to click on like this:
image

Where as events which have a highlight get a bigger area with icon, header, description and a button. Something like this:
image

The aim is to distinguish between important and unimportant events so that the user doesn't get spammed.

Do you think this could be useful?

Copy link
Member

Choose a reason for hiding this comment

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

Still too many loose ends. Who decides which events are important and which are not? Where does this m.event_description event originate from and where is it placed?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see why something like that needs to be sent as a separate event.


## Proposal

The spec should reserve the special room "client_registrations".
Copy link
Member

Choose a reason for hiding this comment

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

That's not a room ID or alias. How does the client find that room?

Copy link
Author

Choose a reason for hiding this comment

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

Having a fixed room id is probably best, should i change it to #client_registration:yourhomeserver.com?

Copy link
Member

Choose a reason for hiding this comment

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

Why a room and not account data, if it's supposed to be private to the user?

Copy link
Member

Choose a reason for hiding this comment

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

That's not an ID, that's an alias. Also, that would mean that any homeservers that already have a room with such an alias would get confused clients.

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

So far the MSC is very sketchy and has quite a few gaps. The note in "Potential issues" is not completely accurate (the room is private to the user after all) but still provides ample opportunity for lateral propagation of an attack once one device has been compromised, making the suggestion a non-starter without proper security mitigations.

What you're trying to devise is a mechahism for experience hand-over really. Whether the client is running on the same device or not doesn't really matter - I might want to open some event on a desktop while running through the feed on a mobile phone, e.g. You really shouldn't deal with application invocation - it's a big security can of worms (have you heard of Windows OLE? You're half-way reinventing it for Matrix) and there are plenty of things in operating systems - think XDG specifications on Linux, e.g. - that are aimed at doing exactly that, with proper safeguards and local context.

That said, I think that advertising support of certain event types across devices of a user has some promise. I'd be interested to see an MSC focusing specifically on this - e.g., with a client at a certain device id claiming support of certain event types and other clients of the user unhiding these events even if they are unknown locally and hinting at the user's device(s) that can show them. This is a more compact and less controversial function.

Comment on lines +91 to +94
- When launch_command is excuted the enviorment varaible MATRIX_CLIENT_STARTUP_EVENT is set to `<room_id>/<message_id>`.
- When launch_url is used the url is extended by `?matrix_client_startup_event=<room_id>/<message_id>`.
- When an action is started the enviorment varaible MATRIX_CLIENT_ACTION_LOCATION is set to `<room_id>`.
- When an action_url is used the url is extended by `?matrix_client_action_location=<room_id>`.
Copy link
Member

Choose a reason for hiding this comment

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

You should have used matrix: URIs here instead of inventing yet another URI scheme.


## Proposal

The spec should reserve the special room "client_registrations".
Copy link
Member

Choose a reason for hiding this comment

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

Why a room and not account data, if it's supposed to be private to the user?

- When an action is started the enviorment varaible MATRIX_CLIENT_ACTION_LOCATION is set to `<room_id>`.
- When an action_url is used the url is extended by `?matrix_client_action_location=<room_id>`.

Using enviorment varaibles has the advantage that this functionality can be implemented as a plugin for an existing app.
Copy link
Member

Choose a reason for hiding this comment

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

This needs explanation - I don't see how using environment variables helps the case you mentioned. Besides, environment variables are used to pass external configuration at the start of the application. You can't really pass environment variables to an already running application unless you have some inter-process communication mechanism around - and if you have one, why use environment variables...


Using enviorment varaibles has the advantage that this functionality can be implemented as a plugin for an existing app.

To highlight certain events a client can use "m.event_description" event.
Copy link
Member

Choose a reason for hiding this comment

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

Still too many loose ends. Who decides which events are important and which are not? Where does this m.event_description event originate from and where is it placed?

@xarvic
Copy link
Author

xarvic commented May 11, 2022

So far the MSC is very sketchy and has quite a few gaps. The note in "Potential issues" is not completely accurate (the room is private to the user after all) but still provides ample opportunity for lateral propagation of an attack once one device has been compromised, making the suggestion a non-starter without proper security mitigations.

What you're trying to devise is a mechahism for experience hand-over really. Whether the client is running on the same device or not doesn't really matter - I might want to open some event on a desktop while running through the feed on a mobile phone, e.g. You really shouldn't deal with application invocation - it's a big security can of worms (have you heard of Windows OLE? You're half-way reinventing it for Matrix) and there are plenty of things in operating systems - think XDG specifications on Linux, e.g. - that are aimed at doing exactly that, with proper safeguards and local context.

Thanks, for your input. I took a look at OLE and XDG. You are right. This is what I wanted to have. But yes, i see the problem.

That said, I think that advertising support of certain event types across devices of a user has some promise. I'd be interested to see an MSC focusing specifically on this - e.g., with a client at a certain device id claiming support of certain event types and other clients of the user unhiding these events even if they are unknown locally and hinting at the user's device(s) that can show them. This is a more compact and less controversial function.

Maybe i will try that.

@xarvic xarvic closed this May 11, 2022
@turt2live turt2live added the abandoned A proposal where the author/shepherd is not responsive label May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned A proposal where the author/shepherd is not responsive client-server Client-Server API 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants