From eb31335b9ba710aa841a52b6847d5a91513f03ba Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 3 May 2023 16:13:19 -0400 Subject: [PATCH 1/9] Clarify how m.push_rules works with account_data. --- proposals/4010-push-rules-and-account-data.md | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 proposals/4010-push-rules-and-account-data.md diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md new file mode 100644 index 00000000000..2a29122ae18 --- /dev/null +++ b/proposals/4010-push-rules-and-account-data.md @@ -0,0 +1,70 @@ +# MSC4010: Push rules and account data + +Push rules have a [bespoke API](https://spec.matrix.org/v1.6/client-server-api/#push-rules-api) +which clients use to retrieve, add, modify, and remove push rules. Any modifications +made using this API are sent to all clients via a `m.push_rules` event in the +`account_data` section in the +[the next `/sync` response](https://spec.matrix.org/v1.6/client-server-api/#push-rules-events). + +The client-server API does not have any special behavior around using the +[account data APIs](https://spec.matrix.org/v1.6/client-server-api/#client-behaviour-17) +with the `m.push_rules` type leading to different behavior on different homeservers: + +* Synapse will accept the data and shadow the real push rules in `/sync`, but + *won't use the data when evaluating push rules*. +* Dendrite will return an HTTP 403 if you attempt to set `m.push_rules` via + `/account_data`. + +The [fully read marker](https://spec.matrix.org/v1.6/client-server-api/#fully-read-markers) +operates in a similar way and +[servers must reject updating `m.fully_read` via `/account_data`](https://spec.matrix.org/v1.6/client-server-api/#server-behaviour-10). + +Note that when attempting to set `m.fully_read` via push rules the following +behavior is observed: + +* Synapse will reject it with a 405 error (only for room account data). +* Dendrite will reject it with an HTTP 403 error. + +## Proposal + +To make push rules data consistent with fully read markers, the following +clarifications are offered: + +* The `m.push_rules` account data type becomes protected and cannot be set using + the `/account_data` API, similarly to `m.fully_read`. +* "Rejected" means to use the 405 error response as + [documented](https://spec.matrix.org/v1.6/client-server-api/#put_matrixclientv3useruseridaccount_datatype): + + > This `type` of account data is controlled by the server; it cannot be modified + > by clients. Errcode: `M_BAD_JSON`. +* `m.push_rules` and `m.fully_read` should be rejected for both global and room + account data. +* Reading `m.push_rules` and `m.fully_read` should be allowed (although note that + currently `m.push_rules` only makes sense for global account data nd `m.fully_read` + only makes sense for room account data). + +## Potential issues + +None, hopefully. + +## Alternatives + +An alternative would be to remove the current push rules API and have clients +store all push rules in bulk. This would be subject to race conditions between +clients. + +A sleight variation of the above would be to *additionally* define the `/account_data/m.push_rules` +endpoint as bulk modifying the push rules data. This could be seen as an alternative +to [MSC3934](https://github.com/matrix-org/matrix-spec-proposals/pull/3934). + +## Security considerations + +None foreseen. + +## Unstable prefix + +This is mostly clarifications and does not add any event types or new endpoints. + +## Dependencies + +N/A From 15faf6b379f3d190319bf38655fb3ad140ee645e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 3 May 2023 16:16:28 -0400 Subject: [PATCH 2/9] Typo fix. --- proposals/4010-push-rules-and-account-data.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index 2a29122ae18..0c2223bb34c 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -40,7 +40,7 @@ clarifications are offered: * `m.push_rules` and `m.fully_read` should be rejected for both global and room account data. * Reading `m.push_rules` and `m.fully_read` should be allowed (although note that - currently `m.push_rules` only makes sense for global account data nd `m.fully_read` + currently `m.push_rules` only makes sense for global account data and `m.fully_read` only makes sense for room account data). ## Potential issues From 65c0a50958542101de86e273daafa18da0ddb98d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 4 May 2023 07:13:39 -0400 Subject: [PATCH 3/9] Fix typo. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/4010-push-rules-and-account-data.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index 0c2223bb34c..dde08cfe805 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -19,7 +19,7 @@ The [fully read marker](https://spec.matrix.org/v1.6/client-server-api/#fully-re operates in a similar way and [servers must reject updating `m.fully_read` via `/account_data`](https://spec.matrix.org/v1.6/client-server-api/#server-behaviour-10). -Note that when attempting to set `m.fully_read` via push rules the following +Note that when attempting to set `m.fully_read` via `/account_data` the following behavior is observed: * Synapse will reject it with a 405 error (only for room account data). From 05d168475afadb56f3acb018fdf036808b27cfa1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 May 2023 09:45:23 -0400 Subject: [PATCH 4/9] Add info on MSC3391. --- proposals/4010-push-rules-and-account-data.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index dde08cfe805..35f2ba23915 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -43,6 +43,9 @@ clarifications are offered: currently `m.push_rules` only makes sense for global account data and `m.fully_read` only makes sense for room account data). +The above rules shall also apply when deleting account data if [MSC3391](https://github.com/matrix-org/matrix-spec-proposals/pull/3391) +is merged before this MSC. + ## Potential issues None, hopefully. From 497db9a4ab77a85611e4f3515fa09a669e650f52 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 May 2023 10:11:11 -0400 Subject: [PATCH 5/9] Clarify potential issues. --- proposals/4010-push-rules-and-account-data.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index 35f2ba23915..0dafa23f762 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -48,7 +48,11 @@ is merged before this MSC. ## Potential issues -None, hopefully. +It is possible that a client is currently storing data in the `m.push_rules` +(or global `m.fully_read`) account data. After this change it could no longer +be updated, deleted, or retrieved. It seems unlikely that the data stored here +is done purposefully (and is likely the result of undefined behavior that this +MSC is attempting to fix). ## Alternatives From 66ed00620e3ba84dec75177004362d2483b72401 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 May 2023 10:18:52 -0400 Subject: [PATCH 6/9] Clarify returned value --- proposals/4010-push-rules-and-account-data.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index 0dafa23f762..c3137841a9e 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -41,7 +41,8 @@ clarifications are offered: account data. * Reading `m.push_rules` and `m.fully_read` should be allowed (although note that currently `m.push_rules` only makes sense for global account data and `m.fully_read` - only makes sense for room account data). + only makes sense for room account data). The format should match what is currently + [returned via `/sync`](https://spec.matrix.org/v1.6/client-server-api/#push-rules-events). The above rules shall also apply when deleting account data if [MSC3391](https://github.com/matrix-org/matrix-spec-proposals/pull/3391) is merged before this MSC. From cdd8552844466c811934b4905bf3b7db77872953 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 30 Nov 2023 07:29:59 -0500 Subject: [PATCH 7/9] Fix typo. Co-authored-by: David Vo --- proposals/4010-push-rules-and-account-data.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index c3137841a9e..bfca2902981 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -61,7 +61,7 @@ An alternative would be to remove the current push rules API and have clients store all push rules in bulk. This would be subject to race conditions between clients. -A sleight variation of the above would be to *additionally* define the `/account_data/m.push_rules` +A slight variation of the above would be to *additionally* define the `/account_data/m.push_rules` endpoint as bulk modifying the push rules data. This could be seen as an alternative to [MSC3934](https://github.com/matrix-org/matrix-spec-proposals/pull/3934). From 030858bffd806beb5913e3f77781c5927820e1e5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 20 Feb 2024 12:50:12 -0500 Subject: [PATCH 8/9] Add information on Conduit. Co-authored-by: Alexey Rusakov --- proposals/4010-push-rules-and-account-data.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index bfca2902981..99748b598b3 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -14,6 +14,8 @@ with the `m.push_rules` type leading to different behavior on different homeserv *won't use the data when evaluating push rules*. * Dendrite will return an HTTP 403 if you attempt to set `m.push_rules` via `/account_data`. +* Conduit has no protections for special account data yet, will accept the data _and_ use those when + evaluating push rules. The [fully read marker](https://spec.matrix.org/v1.6/client-server-api/#fully-read-markers) operates in a similar way and From 275a20179aa53d7e814a48aaa39c8b62f1ec769f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 20 Feb 2024 12:53:12 -0500 Subject: [PATCH 9/9] More consistent wording. --- proposals/4010-push-rules-and-account-data.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/4010-push-rules-and-account-data.md b/proposals/4010-push-rules-and-account-data.md index 99748b598b3..cd59b575f30 100644 --- a/proposals/4010-push-rules-and-account-data.md +++ b/proposals/4010-push-rules-and-account-data.md @@ -14,8 +14,8 @@ with the `m.push_rules` type leading to different behavior on different homeserv *won't use the data when evaluating push rules*. * Dendrite will return an HTTP 403 if you attempt to set `m.push_rules` via `/account_data`. -* Conduit has no protections for special account data yet, will accept the data _and_ use those when - evaluating push rules. +* Conduit has no protections for special account data. It will accept `m.push_rules` via + `/account_data` *and* use those when evaluating push rules. The [fully read marker](https://spec.matrix.org/v1.6/client-server-api/#fully-read-markers) operates in a similar way and