-
Notifications
You must be signed in to change notification settings - Fork 383
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
MSC4069: Inhibit profile propagation #4069
Open
turt2live
wants to merge
3
commits into
main
Choose a base branch
from
travis/msc/inhibit-profile
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| # MSC4069: Inhibit profile propagation | ||
|
|
||
| The current [`PUT /avatar_url`](https://spec.matrix.org/v1.8/client-server-api/#put_matrixclientv3profileuseridavatar_url) | ||
| and [`PUT /displayname`](https://spec.matrix.org/v1.8/client-server-api/#put_matrixclientv3profileuseriddisplayname) | ||
| endpoints modify the "global" profile for the user. This profile is typically exposed through the | ||
| user directory and other similar room-less places. Servers are additionally | ||
| [required to](https://spec.matrix.org/v1.8/client-server-api/#events-on-change-of-profile-information) | ||
| update the `m.room.member` (and similar) events for the user with the new profile information. | ||
|
|
||
| Some applications prefer to manually manage the profile updates in-room and over presence, as they | ||
| may need to enable per-space, per-room, or other sufficiently limited profile updates. Instead of | ||
| waiting for the server to update the user's profile everywhere before "fixing" the events, it could | ||
| ask the server to not update the events in the first place. This proposal introduces an optional | ||
| flag to enable that exact functionality. | ||
|
|
||
| A somewhat more niche usage of this MSC might be a project like [matrix-media-repo](https://github.com/turt2live/matrix-media-repo) | ||
| aiming to implement [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911): when | ||
| the user's avatar URL is changed, the media repo can intercept the request, proxy it forward to the | ||
| homeserver as inhibited, then manually copy the media enough times to update all the user's joined | ||
| rooms. This approach would reduce the number of custom endpoints required by a homeserver for MMR | ||
| integration. | ||
|
|
||
| ## Proposal | ||
|
|
||
| The following two endpoints gain an *optional* new query parameter: `propagate`. The value MUST be a | ||
| boolean, and defaults to `true` if not provided. If the value is *not* a boolean, a normal 400 Bad | ||
| Request error code is returned. | ||
|
|
||
| * [`PUT /avatar_url`](https://spec.matrix.org/v1.8/client-server-api/#put_matrixclientv3profileuseridavatar_url) | ||
| * [`PUT /displayname`](https://spec.matrix.org/v1.8/client-server-api/#put_matrixclientv3profileuseriddisplayname) | ||
|
|
||
| When `false`, `m.room.member` and `m.presence` events are *not* emitted automatically by the server | ||
| during execution of the above two endpoints. Servers should still perform this bit of the spec, | ||
| however: | ||
|
|
||
| > Additionally, when homeservers emit room membership events for their own users, they should | ||
| > include the display name and avatar URL fields in these events so that clients already have these | ||
| > details to hand, and do not have to perform extra round trips to query it. | ||
|
|
||
| With respect to [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911), when the | ||
| user updates their profile's avatar with `?propagate=false`, the media they use is *not* cloned nor | ||
| used in event updates. The client can manually clone the media if they prefer using MSC3911's copy | ||
| API. | ||
|
|
||
| Some servers use different rate limits depending on whether the client is using the profile APIs or | ||
| state APIs. This proposal suggests no changes to the rate limits, though servers *may* wish to apply | ||
| intelligent rate limits to state event changes. For example, detecting that the user is updating their | ||
| profile information in a subset of rooms shortly after updating their global profile. | ||
|
|
||
| ## Potential issues | ||
|
|
||
| This adds a bit of API bloat for a relatively uncommon feature, though may enable other features to | ||
| be more readily possible. For example, a client *introducing* per-space memberships might use this | ||
| new query parameter to make it easier to manually apply `m.room.member` event changes. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| Currently Discord bridges will update the global user profile, wait for that to complete (which can | ||
| take minutes), then manually fix all of the `m.room.member` events for the user to accomplish proper | ||
| platform bridging. While this does work, it is often slow and unreliable when the server takes an | ||
| eternity to complete the profile change request due to a large number of rooms needing updates. Some | ||
| servers, like t2bot.io, instead decide to [patch](https://github.com/t2bot/synapse/commit/049cacfc2d9e98fc602b85978c70363ce3c4f52f) | ||
| their server software to enforce this MSC's behaviour with a more blunt instrument. | ||
|
|
||
| ## Security considerations | ||
|
|
||
| No major security considerations required. This MSC effectively reduces spam, though may affect a | ||
| user's perceivable privacy. Clients should be careful to explain *where* a user's profile updates | ||
| will be visible, regardless of this MSC. | ||
|
|
||
| ## Unstable prefix | ||
|
|
||
| While this MSC is not considered stable, clients can use `?org.matrix.msc4069.propagate` instead of | ||
| `?propagate`. To ensure the server supports the functionality before a spec release, clients should | ||
| look for the unstable feature `org.matrix.msc4069`. | ||
|
|
||
| Once this MSC has successfully passed a merge FCP, clients can (and should) send *both* `?propagate` | ||
| and `?org.matrix.msc4069.propagate` to ensure backwards compatibility with slightly old servers. | ||
|
|
||
| Once released in the specification, clients should be checking for server support through advertised | ||
| spec versions instead. | ||
|
|
||
| ## Dependencies | ||
|
|
||
| This MSC has no dependencies. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of propagating at
/avatar_urland/displayname, it could be part ofm.room.member. A"locked": true(or similar) in the content would cause the server to not update that member event.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, I've been thinking about this a lot. There's some potential privacy concerns with revealing the information. Specifically, it tells other members of the room that the user is definitely using a customized, non-default, profile in the room. On the other hand, any malicious party wanting to make use of that information could just GET the user's default profile to find their "real" profile anyways. A separate MSC would be good to add protections/guarantees around the user's [default] profile (who can access it, etc).
Eventually it'd also be good to move the profile out of the
m.room.memberevent, to give at minimum separation of concerns. Currently the membership event's primary purpose is to denote the user's, well, membership. Profile information is added on as a convenience. While scalable, it feels a bit overloaded to use the event for two things.The third concern is mostly one of backwards compatibility. Clients may already have UX which says the user's profile changes everywhere. If we add
locked: trueto the membership event, that client UX is now a lie. This is relatively minor, but worth noting as an incompatibility. By preventing propagation at the endpoint level, existing clients can maintain their UX and new clients can more obviously add a toggle or similar to "overwrite" the profile in all rooms, or otherwise tailor their UI for their specific use case.Overall, I think I'm still in favour of using the endpoint-level control, though a counter MSC to discuss the
m.room.memberflagging would be interesting.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A possibly third option:
m.profile_lockedas a per-room account data event. This state would not be visible to other users. I would prefer this to alockedproperty in the membership event, but would prefer endpoint-level control over both.