Stop returning `unsigned.invite_room_state` in `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}` responses by anoadragon453 · Pull Request #14064 · matrix-org/synapse

anoadragon453 · 2022-10-05T14:54:14Z

We were adding an 'invite_room_state' field in the 'unsigned' dict of invite events in response to a
PUT /_matrix/federation/v2/invite/{roomId}/{eventId} request.

Internally, Synapse expects this to field to exist (as it did in the v1 version of this API), so we plonked it on before reusing the old code.

Unfortunately, we then forgot to remove it before sending the event out again.

Confirmed the problem and the fix using PyCharm's debugger. I thought about writing a unit/complement test but it's a bit specific...

More information about stripped state events.

I assume this is backwards-compatible, and that nobody is relying on it. It appears in an example in the S2S spec (likely by accident), but not the actual schema.

We were accidentally returning an 'invite_room_state' field in the 'unsigned' dict of invite events in response to a PUT /_matrix/federation/v2/invite/{roomId}/{eventId} request.

DMRobertson · 2022-10-05T17:43:51Z

Related: #10800?

squahtx · 2022-10-07T12:40:07Z

synapse/federation/transport/server/federation.py

+
+        # We only store invite_room_state for internal use, so remove it before
+        # returning the event to the remote homeserver.
+        result["event"].get("unsigned", {}).pop("invite_room_state", None)
+


Will we expose unsigned.invite_room_state in other endpoints? (and does that matter?)

The field is purposely stripped when serialising an event for clients:

synapse/synapse/events/utils.py

Lines 371 to 377 in 06df5d4

# invite_room_state and knock_room_state are a list of stripped room state events

# that are meant to provide metadata about a room to an invitee/knocker. They are

# intended to only be included in specific circumstances, such as down sync, and

# should not be included in any other case.

if not config.include_stripped_room_state:

d["unsigned"].pop("invite_room_state", None)

d["unsigned"].pop("knock_room_state", None)

which covers the areas where clients could get access to this field. As for federation, it may be included during a backfill... perhaps it's something we should additionally add to filter_events_for_server:

synapse/synapse/visibility.py

Lines 534 to 623 in 535f8c8

async def filter_events_for_server(

storage: StorageControllers,

server_name: str,

events: List[EventBase],

redact: bool = True,

check_history_visibility_only: bool = False,

) -> List[EventBase]:

"""Filter a list of events based on whether given server is allowed to

see them.

Args:

storage

server_name

events

redact: Whether to return a redacted version of the event, or

to filter them out entirely.

check_history_visibility_only: Whether to only check the

history visibility, rather than things like if the sender has been

erased. This is used e.g. during pagination to decide whether to

backfill or not.

Returns

The filtered events.

"""

def is_sender_erased(event: EventBase, erased_senders: Dict[str, bool]) -> bool:

if erased_senders and erased_senders[event.sender]:

logger.info("Sender of %s has been erased, redacting", event.event_id)

return True

return False

def check_event_is_visible(

visibility: str, memberships: StateMap[EventBase]

) -> bool:

if visibility not in (HistoryVisibility.INVITED, HistoryVisibility.JOINED):

return True

# We now loop through all membership events looking for

# membership states for the requesting server to determine

# if the server is either in the room or has been invited

# into the room.

for ev in memberships.values():

assert get_domain_from_id(ev.state_key) == server_name

memtype = ev.membership

if memtype == Membership.JOIN:

return True

elif memtype == Membership.INVITE:

if visibility == HistoryVisibility.INVITED:

return True

# server has no users in the room: redact

return False

if not check_history_visibility_only:

erased_senders = await storage.main.are_users_erased(e.sender for e in events)

else:

# We don't want to check whether users are erased, which is equivalent

# to no users having been erased.

erased_senders = {}

# Let's check to see if all the events have a history visibility

# of "shared" or "world_readable". If that's the case then we don't

# need to check membership (as we know the server is in the room).

event_to_history_vis = await _event_to_history_vis(storage, events)

# for any with restricted vis, we also need the memberships

event_to_memberships = await _event_to_memberships(

storage,

[

e

for e in events

if event_to_history_vis[e.event_id]

not in (HistoryVisibility.SHARED, HistoryVisibility.WORLD_READABLE)

],

server_name,

)

to_return = []

for e in events:

erased = is_sender_erased(e, erased_senders)

visible = check_event_is_visible(

event_to_history_vis[e.event_id], event_to_memberships.get(e.event_id, {})

)

if visible and not erased:

to_return.append(e)

elif redact:

to_return.append(prune_event(e))

return to_return

Actually, perhaps it's best if we just stop storing these fields altogether. I've filed #14160 to discuss it.

DMRobertson · 2022-10-19T19:06:40Z

Looks like tests fell foul of #14079. I'll re-run. (Edit: err, merge in develop).

DMRobertson · 2022-10-20T10:52:18Z

@anoadragon453 looks like this is good to go now. (I assume you still want to land this?)

anoadragon453 · 2022-10-20T14:17:20Z

Yep, thank you!

@asymmetric

Synapse 1.71.0 (2022-11-08) =========================== Please note that, as announced in the release notes for Synapse 1.69.0, legacy Prometheus metric names are now disabled by default. They will be removed altogether in Synapse 1.73.0. If not already done, server administrators should update their dashboards and alerting rules to avoid using the deprecated metric names. See the [upgrade notes](https://matrix-org.github.io/synapse/v1.71/upgrade.html#upgrading-to-v1710) for more details. **Note:** in line with our [deprecation policy](https://matrix-org.github.io/synapse/latest/deprecation_policy.html) for platform dependencies, this will be the last release to support PostgreSQL 10, which reaches upstream end-of-life on November 10th, 2022. Future releases of Synapse will require PostgreSQL 11+. No significant changes since 1.71.0rc2. Synapse 1.71.0rc2 (2022-11-04) ============================== Improved Documentation ---------------------- - Document the changes to monthly active user metrics due to deprecation of legacy Prometheus metric names. ([\matrix-org#14358](matrix-org#14358), [\matrix-org#14360](matrix-org#14360)) Deprecations and Removals ------------------------- - Disable legacy Prometheus metric names by default. They can still be re-enabled for now, but they will be removed altogether in Synapse 1.73.0. ([\matrix-org#14353](matrix-org#14353)) Internal Changes ---------------- - Run unit tests against Python 3.11. ([\matrix-org#13812](matrix-org#13812)) Synapse 1.71.0rc1 (2022-11-01) ============================== Features -------- - Support back-channel logouts from OpenID Connect providers. ([\matrix-org#11414](matrix-org#11414)) - Allow use of Postgres and SQLlite full-text search operators in search queries. ([\matrix-org#11635](matrix-org#11635), [\matrix-org#14310](matrix-org#14310), [\matrix-org#14311](matrix-org#14311)) - Implement [MSC3664](matrix-org/matrix-spec-proposals#3664), Pushrules for relations. Contributed by Nico. ([\matrix-org#11804](matrix-org#11804)) - Improve aesthetics of HTML templates. Note that these changes do not retroactively apply to templates which have been [customised](https://matrix-org.github.io/synapse/latest/templates.html#templates) by server admins. ([\matrix-org#13652](matrix-org#13652)) - Enable write-ahead logging for SQLite installations. Contributed by [@asymmetric](https://github.com/asymmetric). ([\matrix-org#13897](matrix-org#13897)) - Show erasure status when [listing users](https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html#query-user-account) in the Admin API. ([\matrix-org#14205](matrix-org#14205)) - Provide a specific error code when a `/sync` request provides a filter which doesn't represent a JSON object. ([\matrix-org#14262](matrix-org#14262)) Bugfixes -------- - Fix a long-standing bug where the `update_synapse_database` script could not be run with multiple databases. Contributed by @thefinn93 @ Beeper. ([\matrix-org#13422](matrix-org#13422)) - Fix a bug which prevented setting an avatar on homeservers which have an explicit port in their `server_name` and have `max_avatar_size` and/or `allowed_avatar_mimetypes` configuration. Contributed by @ashfame. ([\matrix-org#13927](matrix-org#13927)) - Check appservice user interest against the local users instead of all users in the room to align with [MSC3905](matrix-org/matrix-spec-proposals#3905). ([\matrix-org#13958](matrix-org#13958)) - Fix a long-standing bug where Synapse would accidentally include extra information in the response to [`PUT /_matrix/federation/v2/invite/{roomId}/{eventId}`](https://spec.matrix.org/v1.4/server-server-api/#put_matrixfederationv2inviteroomideventid). ([\matrix-org#14064](matrix-org#14064)) - Fix a bug introduced in Synapse 1.64.0 where presence updates could be missing from `/sync` responses. ([\matrix-org#14243](matrix-org#14243)) - Fix a bug introduced in Synapse 1.60.0 which caused an error to be logged when Synapse received a SIGHUP signal if debug logging was enabled. ([\matrix-org#14258](matrix-org#14258)) - Prevent history insertion ([MSC2716](matrix-org/matrix-spec-proposals#2716)) during an partial join ([MSC3706](matrix-org/matrix-spec-proposals#3706)). ([\matrix-org#14291](matrix-org#14291)) - Fix a bug introduced in Synapse 1.34.0 where device names would be returned via a federation user key query request when `allow_device_name_lookup_over_federation` was set to `false`. ([\matrix-org#14304](matrix-org#14304)) - Fix a bug introduced in Synapse 0.34.0 where logs could include error spam when background processes are measured as taking a negative amount of time. ([\matrix-org#14323](matrix-org#14323)) - Fix a bug introduced in Synapse 1.70.0 where clients were unable to PUT new [dehydrated devices](matrix-org/matrix-spec-proposals#2697). ([\matrix-org#14336](matrix-org#14336)) Improved Documentation ---------------------- - Explain how to disable the use of [`trusted_key_servers`](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#trusted_key_servers). ([\matrix-org#13999](matrix-org#13999)) - Add workers settings to [configuration manual](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#individual-worker-configuration). ([\matrix-org#14086](matrix-org#14086)) - Correct the name of the config option [`encryption_enabled_by_default_for_room_type`](https://matrix-org.github.io/synapse/latest/usage/configuration/config_documentation.html#encryption_enabled_by_default_for_room_type). ([\matrix-org#14110](matrix-org#14110)) - Update docstrings of `SynapseError` and `FederationError` to bettter describe what they are used for and the effects of using them are. ([\matrix-org#14191](matrix-org#14191)) Internal Changes ---------------- - Remove unused `@lru_cache` decorator. ([\matrix-org#13595](matrix-org#13595)) - Save login tokens in database and prevent login token reuse. ([\matrix-org#13844](matrix-org#13844)) - Refactor OIDC tests to better mimic an actual OIDC provider. ([\matrix-org#13910](matrix-org#13910)) - Fix type annotation causing import time error in the Complement forking launcher. ([\matrix-org#14084](matrix-org#14084)) - Refactor [MSC3030](matrix-org/matrix-spec-proposals#3030) `/timestamp_to_event` endpoint to loop over federation destinations with standard pattern and error handling. ([\matrix-org#14096](matrix-org#14096)) - Add initial power level event to batch of bulk persisted events when creating a new room. ([\matrix-org#14228](matrix-org#14228)) - Refactor `/key/` endpoints to use `RestServlet` classes. ([\matrix-org#14229](matrix-org#14229)) - Switch to using the `matrix-org/backend-meta` version of `triage-incoming` for new issues in CI. ([\matrix-org#14230](matrix-org#14230)) - Build wheels on macos 11, not 10.15. ([\matrix-org#14249](matrix-org#14249)) - Add debugging to help diagnose lost device list updates. ([\matrix-org#14268](matrix-org#14268)) - Add Rust cache to CI for `trial` runs. ([\matrix-org#14287](matrix-org#14287)) - Improve type hinting of `RawHeaders`. ([\matrix-org#14303](matrix-org#14303)) - Use Poetry 1.2.0 in the Twisted Trunk CI job. ([\matrix-org#14305](matrix-org#14305)) <details> <summary>Dependency updates</summary> Runtime: - Bump anyhow from 1.0.65 to 1.0.66. ([\matrix-org#14278](matrix-org#14278)) - Bump jinja2 from 3.0.3 to 3.1.2. ([\matrix-org#14271](matrix-org#14271)) - Bump prometheus-client from 0.14.0 to 0.15.0. ([\matrix-org#14274](matrix-org#14274)) - Bump psycopg2 from 2.9.4 to 2.9.5. ([\matrix-org#14331](matrix-org#14331)) - Bump pysaml2 from 7.1.2 to 7.2.1. ([\matrix-org#14270](matrix-org#14270)) - Bump sentry-sdk from 1.5.11 to 1.10.1. ([\matrix-org#14330](matrix-org#14330)) - Bump serde from 1.0.145 to 1.0.147. ([\matrix-org#14277](matrix-org#14277)) - Bump serde_json from 1.0.86 to 1.0.87. ([\matrix-org#14279](matrix-org#14279)) Tooling and CI: - Bump black from 22.3.0 to 22.10.0. ([\matrix-org#14328](matrix-org#14328)) - Bump flake8-bugbear from 21.3.2 to 22.9.23. ([\matrix-org#14042](matrix-org#14042)) - Bump peaceiris/actions-gh-pages from 3.8.0 to 3.9.0. ([\matrix-org#14276](matrix-org#14276)) - Bump peaceiris/actions-mdbook from 1.1.14 to 1.2.0. ([\matrix-org#14275](matrix-org#14275)) - Bump setuptools-rust from 1.5.1 to 1.5.2. ([\matrix-org#14273](matrix-org#14273)) - Bump twine from 3.8.0 to 4.0.1. ([\matrix-org#14332](matrix-org#14332)) - Bump types-opentracing from 2.4.7 to 2.4.10. ([\matrix-org#14133](matrix-org#14133)) - Bump types-requests from 2.28.11 to 2.28.11.2. ([\matrix-org#14272](matrix-org#14272)) </details>

anoadragon453 added 2 commits October 5, 2022 15:49

Stop returning unsigned.invite_room_state in v2 invite fed responses

e899e0b

We were accidentally returning an 'invite_room_state' field in the 'unsigned' dict of invite events in response to a PUT /_matrix/federation/v2/invite/{roomId}/{eventId} request.

changelog

ceef1fc

anoadragon453 marked this pull request as ready for review October 5, 2022 15:30

anoadragon453 requested a review from a team as a code owner October 5, 2022 15:30

anoadragon453 mentioned this pull request Oct 5, 2022

Remove invite_room_state, knock_room_state from the CS API. Fix examples in SS API matrix-org/matrix-spec#1273

Closed

kick CI

72fe1f7

squahtx approved these changes Oct 7, 2022

View reviewed changes

anoadragon453 mentioned this pull request Oct 12, 2022

We should stop putting invite_room_state and knock_room_state into the unsigned field of events #14160

Open

Merge branch 'develop' into anoa/invite_room_state_response

472fd9b

anoadragon453 merged commit da2c93d into develop Oct 20, 2022

anoadragon453 deleted the anoa/invite_room_state_response branch October 20, 2022 14:17

anoadragon453 mentioned this pull request Oct 21, 2022

MSC3912: Redaction of related events matrix-org/matrix-spec-proposals#3912

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Stop returning `unsigned.invite_room_state` in `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}` responses#14064

Stop returning `unsigned.invite_room_state` in `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}` responses#14064
anoadragon453 merged 4 commits intodevelopfrom
anoa/invite_room_state_response

anoadragon453 commented Oct 5, 2022 •

edited

Loading

Uh oh!

DMRobertson commented Oct 5, 2022

Uh oh!

squahtx Oct 7, 2022

Uh oh!

anoadragon453 Oct 12, 2022

Uh oh!

anoadragon453 Oct 12, 2022

Uh oh!

DMRobertson commented Oct 19, 2022 •

edited

Loading

Uh oh!

DMRobertson commented Oct 20, 2022

Uh oh!

anoadragon453 commented Oct 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# invite_room_state and knock_room_state are a list of stripped room state events
	# that are meant to provide metadata about a room to an invitee/knocker. They are
	# intended to only be included in specific circumstances, such as down sync, and
	# should not be included in any other case.
	if not config.include_stripped_room_state:
	d["unsigned"].pop("invite_room_state", None)
	d["unsigned"].pop("knock_room_state", None)

	async def filter_events_for_server(
	storage: StorageControllers,
	server_name: str,
	events: List[EventBase],
	redact: bool = True,
	check_history_visibility_only: bool = False,
	) -> List[EventBase]:
	"""Filter a list of events based on whether given server is allowed to
	see them.

	Args:
	storage
	server_name
	events
	redact: Whether to return a redacted version of the event, or
	to filter them out entirely.
	check_history_visibility_only: Whether to only check the
	history visibility, rather than things like if the sender has been
	erased. This is used e.g. during pagination to decide whether to
	backfill or not.

	Returns
	The filtered events.
	"""

	def is_sender_erased(event: EventBase, erased_senders: Dict[str, bool]) -> bool:
	if erased_senders and erased_senders[event.sender]:
	logger.info("Sender of %s has been erased, redacting", event.event_id)
	return True
	return False

	def check_event_is_visible(
	visibility: str, memberships: StateMap[EventBase]
	) -> bool:
	if visibility not in (HistoryVisibility.INVITED, HistoryVisibility.JOINED):
	return True

	# We now loop through all membership events looking for
	# membership states for the requesting server to determine
	# if the server is either in the room or has been invited
	# into the room.
	for ev in memberships.values():
	assert get_domain_from_id(ev.state_key) == server_name

	memtype = ev.membership
	if memtype == Membership.JOIN:
	return True
	elif memtype == Membership.INVITE:
	if visibility == HistoryVisibility.INVITED:
	return True

	# server has no users in the room: redact
	return False

	if not check_history_visibility_only:
	erased_senders = await storage.main.are_users_erased(e.sender for e in events)
	else:
	# We don't want to check whether users are erased, which is equivalent
	# to no users having been erased.
	erased_senders = {}

	# Let's check to see if all the events have a history visibility
	# of "shared" or "world_readable". If that's the case then we don't
	# need to check membership (as we know the server is in the room).
	event_to_history_vis = await _event_to_history_vis(storage, events)

	# for any with restricted vis, we also need the memberships
	event_to_memberships = await _event_to_memberships(
	storage,
	[
	e
	for e in events
	if event_to_history_vis[e.event_id]
	not in (HistoryVisibility.SHARED, HistoryVisibility.WORLD_READABLE)
	],
	server_name,
	)

	to_return = []
	for e in events:
	erased = is_sender_erased(e, erased_senders)
	visible = check_event_is_visible(
	event_to_history_vis[e.event_id], event_to_memberships.get(e.event_id, {})
	)
	if visible and not erased:
	to_return.append(e)
	elif redact:
	to_return.append(prune_event(e))

	return to_return

Uh oh!

Conversation

anoadragon453 commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DMRobertson commented Oct 5, 2022

Uh oh!

squahtx Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

anoadragon453 Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

anoadragon453 Oct 12, 2022

Choose a reason for hiding this comment

Uh oh!

DMRobertson commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DMRobertson commented Oct 20, 2022

Uh oh!

anoadragon453 commented Oct 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anoadragon453 commented Oct 5, 2022 •

edited

Loading

DMRobertson commented Oct 19, 2022 •

edited

Loading