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

MSC3672: Sharing ephemeral streams of location data #3672

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

Conversation

stefanceriu
Copy link

@stefanceriu stefanceriu commented Jan 25, 2022

In which we build on top of MSC3489: Sharing streams of location data and MSC2477: User-defined ephemeral events to share streams of location data in a non-persistent way.

Rendered

Shepherd: @anoadragon453

@stefanceriu stefanceriu changed the title Sharing ephemeral streams of location data MSC3672: Sharing ephemeral streams of location data Jan 25, 2022
@stefanceriu stefanceriu force-pushed the stefan/ephemeral-location-streaming branch from 1cf81b9 to 12066dd Compare January 25, 2022 14:29
@turt2live turt2live added 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 proposal-in-review labels Jan 25, 2022
Comment on lines 9 to 10
We also need the ability to share this data in a non-persistent way for cases in
which privacy is a concern, like user locations.
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 this needs a bit of a longer justification of why this is needed and why EDUs are the way to go. Some ideas:

  • EDUs are not persisted in the timeline -> A user can only access the ones they received originally.
  • EDUs can be stored indefinitely on the server, but because they are E2EE, this should not cause issues.
  • What are the semantics for the last EDU? When do they expire? Do you always get the last sent one on initial sync? Does a client need to clobber that with a dummy?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you. I've added more details for the first two points but perhaps the last one should be taken up in MSC2477?
I don't think there should be any special behavior for location sharing.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to pick that up in the justification for a bit, i.e. "You can make EDUs expire after some time, so they never get delivered to devices, which are offline for some time" or so, if that is actually possible with 2477. It's been a while since I read it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added some wording about how long the server should keep the EDUs, and I think the beginning part of the Proposal section clarify the justification for EDUs. What do you think @deepbluev7 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Took me a while to find, but yes, that looks reasonable. Does that work when the EDU is encrypted? Because then the event type is m.room.encrypted, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I should have provided a proper link.

I expect that if we later add the ability to specify delivery guarantees for EDUs, we will include that in the unencrypted part. Before we have that, servers will have to apply the same policy to all user-defined encrypted EDUs.

@stefanceriu stefanceriu force-pushed the stefan/ephemeral-location-streaming branch from dbf1694 to 657ce76 Compare January 26, 2022 10:31
@stefanceriu stefanceriu force-pushed the stefan/ephemeral-location-streaming branch from 657ce76 to 2a26b20 Compare January 26, 2022 10:33
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
@anoadragon453
Copy link
Member

Heads up that @stefanceriu has moved on to other work. In their place I'll be acting as a shepherd and will be updating this MSC as new feedback comes in.

proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
proposals/3672-ephemeral-location-streaming.md Outdated Show resolved Hide resolved
@andybalaam andybalaam marked this pull request as ready for review May 6, 2022 14:12
@andybalaam andybalaam self-requested a review May 6, 2022 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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