Support MSC3814: Dehydrated devices v2 aka shrivelled sessions #13581

nico-famedly · 2022-08-22T16:00:38Z

Signed-off-by: Nicolas Werner n.werner@famedly.com

This implements MSC3814 with a few changes (as proposed on the MSC):

The /events endpoint is a GET endpoint using the somewhat standard pagination pattern (just with the additional return of an empty list once to ensure device messages get deleted).
The device doesn't get implicitly deleted when fetching events. Instead the user is supposed to manually replace it.
You can also use this endpoint to fetch the device messages for your own device. The limitation to only the dehydrated device seemed arbitrary.

This MSC conflicts with MSC2697, so I added a config option to disable the former and a config option to enable the latter. In theory you can use both at the same time, but it uses the same database table, which is why I would not recommend it.

Client side implementation: https://gitlab.com/famedly/company/frontend/famedlysdk/-/merge_requests/1111

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Pull request includes a sign off
Code style is correct
(run the linters)

Some differences: - We use GET instead of POST for the events endpoint. - We don't delete the device implicitly when fetching events. - We allow the fetch device messages endpoint for both dehydrated and the current requesters device. Signed-off-by: Nicolas Werner <n.werner@famedly.com>

reivilibre

Some comments for you; no particular complaints from me but I'm very out of the loop on dehydrated devices.

It looks like this MSC is quite early; are you expecting to have this merged or is this more a proof of concept at this point?

reivilibre · 2022-08-23T14:24:16Z

synapse/rest/client/devices.py

        super().__init__()
        self.hs = hs
        self.auth = hs.get_auth()
        self.device_handler = hs.get_device_handler()

+        self.PATTERNS = client_patterns(
+            "/org.matrix.msc2697.v2/dehydrated_device"


Suggested change

"/org.matrix.msc2697.v2/dehydrated_device"

"/org.matrix.msc2697.v2/dehydrated_device$"

presume that was accidentally dropped?

No, it just wasn't there originally and I didn't change it. It is only needed for MSC3814, which is why I added it there, should I add it here too?

I added it now, but there are a few other endpoints that could use it, which I didn't add it for.

reivilibre · 2022-08-23T14:26:20Z

synapse/handlers/devicemessage.py

+                )
+
+        since_stream_id = 0
+        if since_token and len(since_token) > 1 and since_token[0] == "d":


we seem to be mixing multiple conditions together here:

whether a since token was provided

whether it meets the expected format

It sounds suspicious to me that we'll just ignore invalid since tokens; should we complain with an error?

Well, the MSC doesn't really describe the behaviour there. What happens in that case on other endpoints? I guess if they return an error, I can just do that here too.

synapse/handlers/devicemessage.py

Return errors on invalid token formats and terminate regex in both cases

nico-famedly · 2022-08-25T13:14:05Z

It looks like this MSC is quite early; are you expecting to have this merged or is this more a proof of concept at this point?

Famedly has a customer who needs the feature. We implemented it for them and maintain a synapse fork with it. I don't really need it to be merged, the patch is fairly easy to maintain, but it might be interesting to others to play with so it is at least documented here for visibility. If this form is acceptable is probably on @uhoreg to decide :D

synapse/handlers/devicemessage.py

Co-authored-by: Hubert Chathi <hubert@uhoreg.ca>

clokep · 2022-10-19T20:02:58Z

According to @uhoreg:

IIRC, it doesn't follow the MSC exactly, so we need to synchronize it with the MSC (or synchronize the MSC with it).

Thus marking this as a draft for now.

H-Shay · 2023-07-12T19:33:50Z

@nico-famedly: we are implementing this in Synapse now, so I grabbed this PR brought it into alignment with the spec proposal. The PR for this is #15929. I credited you in the changelog, and in the PR message, please let me know if there's anything else you want done to acknowledge this work!

nico-famedly · 2023-07-31T10:20:24Z

I think this is fine. We are however using this with our changes in production for a year now, so we need to figure out how to change our feature flagging to make it not break other clients on our deployments. Thanks for picking this up and moving it forward!

clokep · 2023-07-31T11:58:06Z

@nico-famedly Can this PR be closed then?

nico-famedly · 2023-07-31T12:23:51Z

Since the commit was merged, yes :)

clokep · 2023-07-31T12:29:14Z

Since the commit was merged, yes :)

Woo, thanks for your initial work on this. 👍

nico-famedly requested a review from a team as a code owner August 22, 2022 16:00

nico-famedly added 3 commits August 22, 2022 18:05

Move the older dehydrated devices behind a config flag

6baeda9

Add changelog

49f892d

nico-famedly force-pushed the nico/dehydrated-devices branch from d397077 to 49f892d Compare August 22, 2022 16:05

nico-famedly mentioned this pull request Aug 22, 2022

[WIP] MSC3814: Dehydrated devices with SSSS matrix-org/matrix-spec-proposals#3814

Draft

reivilibre reviewed Aug 23, 2022

View reviewed changes

clokep changed the title ~~MSC3814: Dehydrated devices v2 aka shrivelled sessions~~ Support MSC3814: Dehydrated devices v2 aka shrivelled sessions Aug 24, 2022

Address review feedback

6c183c5

Return errors on invalid token formats and terminate regex in both cases

uhoreg reviewed Sep 5, 2022

View reviewed changes

synapse/handlers/devicemessage.py Outdated Show resolved Hide resolved

nico-famedly and others added 2 commits September 6, 2022 10:59

Update synapse/handlers/devicemessage.py

6d0ce6f

Co-authored-by: Hubert Chathi <hubert@uhoreg.ca>

Merge branch 'develop' into nico/dehydrated-devices

b95364e

clokep marked this pull request as draft October 19, 2022 20:02

DMRobertson added the Z-Spec-Blocked This change is blocked on specification (e.g. an MSC). label Feb 16, 2023

H-Shay mentioned this pull request Jul 12, 2023

Support MSC3814: Dehydrated Devices #15929

Merged

nico-famedly closed this Jul 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support MSC3814: Dehydrated devices v2 aka shrivelled sessions #13581

Support MSC3814: Dehydrated devices v2 aka shrivelled sessions #13581

nico-famedly commented Aug 22, 2022 •

edited

Loading

reivilibre left a comment

reivilibre Aug 23, 2022

nico-famedly Aug 25, 2022

nico-famedly Aug 25, 2022

reivilibre Aug 23, 2022

nico-famedly Aug 25, 2022

nico-famedly commented Aug 25, 2022

clokep commented Oct 19, 2022

H-Shay commented Jul 12, 2023 •

edited

Loading

nico-famedly commented Jul 31, 2023

clokep commented Jul 31, 2023

nico-famedly commented Jul 31, 2023

clokep commented Jul 31, 2023

	"/org.matrix.msc2697.v2/dehydrated_device"
	"/org.matrix.msc2697.v2/dehydrated_device$"

Support MSC3814: Dehydrated devices v2 aka shrivelled sessions #13581

Support MSC3814: Dehydrated devices v2 aka shrivelled sessions #13581

Conversation

nico-famedly commented Aug 22, 2022 • edited Loading

Pull Request Checklist

reivilibre left a comment

Choose a reason for hiding this comment

reivilibre Aug 23, 2022

Choose a reason for hiding this comment

nico-famedly Aug 25, 2022

Choose a reason for hiding this comment

nico-famedly Aug 25, 2022

Choose a reason for hiding this comment

reivilibre Aug 23, 2022

Choose a reason for hiding this comment

nico-famedly Aug 25, 2022

Choose a reason for hiding this comment

nico-famedly commented Aug 25, 2022

clokep commented Oct 19, 2022

H-Shay commented Jul 12, 2023 • edited Loading

nico-famedly commented Jul 31, 2023

clokep commented Jul 31, 2023

nico-famedly commented Jul 31, 2023

clokep commented Jul 31, 2023

nico-famedly commented Aug 22, 2022 •

edited

Loading

H-Shay commented Jul 12, 2023 •

edited

Loading