MSC3391: API to delete account data

[ShadowJonathan]

Rendered

Implementations;

• Synapse
  • PR: https://github.com/matrix-org/synapse/pull/14714 (design: https://github.com/matrix-org/synapse/issues/14244) + https://github.com/matrix-org/synapse/pull/15562 (add an unstable features flag)
• Ruma
  • #TBD
• JS SDK
  • https://github.com/matrix-org/matrix-js-sdk/commit/b2a10e6
• Element iOS
  • https://github.com/vector-im/element-ios/pull/7164

Fixes https://github.com/matrix-org/matrix-spec/issues/190

Signed-off-by: Jonathan de Jong <[email protected]>

FCP tickyboxes

[ShadowJonathan]

Heads up that with 333264f (#3391), in response to feedback, I have changed the semantics of the MSC, the most important one is the removal of "removed_events" from sync, and redefining {} account data as "deleted" across the board. This simplifies the MSC and related changes going forward.

[anoadragon453]

This MSC has been implemented in Synapse: https://github.com/matrix-org/synapse/pull/14714.

I've updated the MSC's description to reflect this :slightly_smiling_face: cc @ShadowJonathan

[anoadragon453]

This MSC is still missing a client implementation before it can progress further through the Spec Process.

(I don't believe the linked Element iOS PR yet counts as it doesn't make use of the new endpoints.)

[Johennes]

This MSC is still missing a client implementation before it can progress further through the Spec Process.

(I don't believe the linked Element iOS PR yet counts as it doesn't make use of the new endpoints.)

It was implemented in the JS SDK with https://github.com/matrix-org/matrix-js-sdk/commit/b2a10e6 actually.

[anoadragon453]

@Johennes Oh I see! Thank you for updating the PR description as well to reflect that.

It was implemented in the JS SDK with https://github.com/matrix-org/matrix-js-sdk/commit/b2a10e6 actually.

Has this been tested against the Synapse implementation yet? As far as I know the Synapse implementation does not actually include a org.matrix.msc3391 field, which it appears the JS implementation requires to use the new endpoints (as it should). Otherwise it will fall back to using {} to override the account data object.

Of course PUTing {} will under this MSC still delete the account data entry, but it'd be good to test out the new endpoints as well.

[Johennes]

Has this been tested against the Synapse implementation yet? As far as I know the Synapse implementation does not actually include a org.matrix.msc3391 field, which it appears the JS implementation requires to use the new endpoints (as it should). Otherwise it will fall back to using {} to override the account data object.

Hm, you're right. We seem to use the endpoints without checking the feature flag on Element Android. I think we're falling back to doing the PUT override when that request fails there. @kerryarchibald could you comment on how this was tested during implementation?

[kerryarchibald]

This was tested against the ROSA deployment with the msc3391 flag enabled, and then against matrix.org to test the fallback.

As far as I know the Synapse implementation does not actually include a org.matrix.msc3391 field, which it appears the JS implementation requires to use the new endpoints (as it should). Otherwise it will fall back to using {} to override the account data object.

Does this mean there was never a working feature flag? I do seem to remember testing and seeing DELETE calls in the console, so possibly I tested it without the flag check, then added that later for completeness. I don't think the support check should be removed in js-sdk, will the flag be added to synapse?

[anoadragon453]

(I've yet to deploy a new version of the bot that supports commands in PR review bodies.)

@mscbot fcp merge

[mscbot]

This FCP proposal has been cancelled by https://github.com/matrix-org/matrix-spec-proposals/pull/3391#issuecomment-1619415111.

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

• [ ] @dbkr
• [ ] @uhoreg
• [x] @turt2live
• [ ] @ara4n
• [x] @anoadragon453
• [ ] @richvdh
• [x] @erikjohnston
• [x] @KitsuneRal

Concerns:

• Unclear semantics for sync
• Unspecified behaviour for when a client can expected deleted account data to be served
• New endpoints may require auth and rate limiting

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

[turt2live]

SCT context: this is primarily maintenance work we shouldn't let languish, but don't have a strong driving force to bump it to the top of the priorities list. The SCT will take a look whenever they have a chance (which may mean it'll be sitting in proposed-FCP for a while, sorry).

If folks have a use case which might affect the SCT's priority on this, please let us know in the SCT Office.

[turt2live]

@mscbot concern Unclear semantics for sync @mscbot concern Unspecified behaviour for when a client can expected deleted account data to be served @mscbot concern New endpoints may require auth and rate limiting

[turt2live]

It appears as though the author(s) aren't currently able to take this on - cancelling FCP pending the MSC being re-raised to the SCT

@mscbot fcp cancel

[turt2live]

This is currently in need of a shepherd to get the MSC to the next steps. It's been proposed for FCP at least once already and got quite a bit of feedback: working through that feedback and sending it back to the SCT to send through FCP is what's required.

As the branch is on a contributor's fork, the shepherd would either need to be a member of the matrix-org github to push to the PR branch, ask the original author for access, or failing all of that open a new MSC so we can mark this one obsolete.

Unfortunately there is no bandwidth within the SCT to do the shepherding required here.

If folks are interested in taking this on, please speak up and do so :)