Allow a user who could join a restricted room to see it in spaces summary. #9922

clokep · 2021-05-03T16:19:23Z

Allows discovery of restricted rooms via MSC3083.

This is based on ~~#9974~~ #10009.

See matrix-org/complement#109 for complement tests.

…mary. Allows discovery of restricted rooms via the intersection of MSC2946 and MSC3083.

…tion.

…oom when queried over federation.

erikjohnston

I think this mostly looks sane?

synapse/handlers/space_summary.py

synapse/handlers/event_auth.py

Co-authored-by: Sumner Evans <me@sumnerevans.com>

clokep · 2021-05-17T18:10:07Z

This pretty much implements the logic in https://github.com/matrix-org/matrix-doc/blob/clokep/restricted-rooms/proposals/3083-restricted-rooms.md#discovery-of-restricted-rooms

The goal is that "restricted rooms" are able to show up in the space summary, but only for those who would be allowed to join them. This gets a bit hairy over federation.

synapse/handlers/event_auth.py

…icted-rooms-in-space-summary

erikjohnston

I think this is broadly looking good. I've made a couple of suggestions for things that might make this clearer, but feel free to push back if it just feels like rearranging deck chairs.

synapse/handlers/event_auth.py

erikjohnston · 2021-05-19T09:07:06Z

synapse/handlers/space_summary.py

@@ -408,6 +521,7 @@ async def _build_room_entry(self, room_id: str) -> JsonDict:
            "guest_can_join": stats["guest_access"] == "can_join",
            "creation_ts": create_event.origin_server_ts,
            "room_type": room_type,
+            "allowed_spaces": allowed_spaces if allowed_spaces else None,


I wonder if we should only be returning the list of spaces that the origin is in? I don't know to what extent we care about leaking the existence of spaces?

Oh, interesting. The issue with that is we don't necessarily know if the origin is in them (if we're not in them ourselves), so I think that would make the edge-case at the bottom of https://github.com/matrix-org/matrix-doc/blob/clokep/restricted-rooms/proposals/3083-restricted-rooms.md#discovery-of-restricted-rooms worse:

Note that there are still potential situations where each server individually doesn't have enough information to properly return the full summary, but these do not seem reasonable in what is considered a normal structure of spaces. (E.g. in the above example, if the remote server is not in the space and does not know whether the server is in the space or not it cannot return the room.)

So if we further filter this list down we might be making more situations where the servers cannot agree who is in the room. I'll need to think about it though...

synapse/handlers/space_summary.py

discarding stale review

Co-authored-by: Erik Johnston <erik@matrix.org>

synapse/handlers/event_auth.py

erikjohnston · 2021-05-19T16:13:22Z

synapse/handlers/event_auth.py

+        # If there's no join rule, then it defaults to invite (so this doesn't apply).
+        join_rules_event_id = state_ids.get((EventTypes.JoinRules, ""), None)
+        if not join_rules_event_id:
+            return ()


I'd be tempted to also do a check of has_restricted_join_rules here as well

Then we'll do the check twice for every caller? 😕

Ugh, true. IDK.

As of now we end up re-fetching the join rules event twice too which is unfortunate.

synapse/handlers/space_summary.py

Co-authored-by: Erik Johnston <erik@matrix.org>

clokep · 2021-05-19T17:56:45Z

@erikjohnston I think this is back to you. There's a couple of open threads above still, but I've taken into account most of your feedback (or replied to it).

erikjohnston

I think this now makes sense!

@junquera

Synapse 1.35.1 (2021-06-03) =========================== Bugfixes -------- - Fix a bug introduced in v1.35.0 where invite-only rooms would be shown to all users in a space, regardless of if the user had access to it. ([\#10109](matrix-org/synapse#10109)) Synapse 1.35.0 (2021-06-01) =========================== Note that [the tag](https://github.com/matrix-org/synapse/releases/tag/v1.35.0rc3) and [docker images](https://hub.docker.com/layers/matrixdotorg/synapse/v1.35.0rc3/images/sha256-34ccc87bd99a17e2cbc0902e678b5937d16bdc1991ead097eee6096481ecf2c4?context=explore) for `v1.35.0rc3` were incorrectly built. If you are experiencing issues with either, it is recommended to upgrade to the equivalent tag or docker image for the `v1.35.0` release. Deprecations and Removals ------------------------- - The core Synapse development team plan to drop support for the [unstable API of MSC2858](https://github.com/matrix-org/matrix-doc/blob/master/proposals/2858-Multiple-SSO-Identity-Providers.md#unstable-prefix), including the undocumented `experimental.msc2858_enabled` config option, in August 2021. Client authors should ensure that their clients are updated to use the stable API (which has been supported since Synapse 1.30) well before that time, to give their users time to upgrade. ([\#10101](matrix-org/synapse#10101)) Bugfixes -------- - Fixed a bug causing replication requests to fail when receiving a lot of events via federation. Introduced in v1.33.0. ([\#10082](matrix-org/synapse#10082)) - Fix HTTP response size limit to allow joining very large rooms over federation. Introduced in v1.33.0. ([\#10093](matrix-org/synapse#10093)) Internal Changes ---------------- - Log method and path when dropping request due to size limit. ([\#10091](matrix-org/synapse#10091)) Synapse 1.35.0rc2 (2021-05-27) ============================== Bugfixes -------- - Fix a bug introduced in v1.35.0rc1 when calling the spaces summary API via a GET request. ([\#10079](matrix-org/synapse#10079)) Synapse 1.35.0rc1 (2021-05-25) ============================== Features -------- - Add experimental support to allow a user who could join a restricted room to view it in the spaces summary. ([\#9922](matrix-org/synapse#9922), [\#10007](matrix-org/synapse#10007), [\#10038](matrix-org/synapse#10038)) - Reduce memory usage when joining very large rooms over federation. ([\#9958](matrix-org/synapse#9958)) - Add a configuration option which allows enabling opentracing by user id. ([\#9978](matrix-org/synapse#9978)) - Enable experimental support for [MSC2946](matrix-org/matrix-spec-proposals#2946) (spaces summary API) and [MSC3083](matrix-org/matrix-spec-proposals#3083) (restricted join rules) by default. ([\#10011](matrix-org/synapse#10011)) Bugfixes -------- - Fix a bug introduced in v1.26.0 which meant that `synapse_port_db` would not correctly initialise some postgres sequences, requiring manual updates afterwards. ([\#9991](matrix-org/synapse#9991)) - Fix `synctl`'s `--no-daemonize` parameter to work correctly with worker processes. ([\#9995](matrix-org/synapse#9995)) - Fix a validation bug introduced in v1.34.0 in the ordering of spaces in the space summary API. ([\#10002](matrix-org/synapse#10002)) - Fixed deletion of new presence stream states from database. ([\#10014](matrix-org/synapse#10014), [\#10033](matrix-org/synapse#10033)) - Fixed a bug with very high resolution image uploads throwing internal server errors. ([\#10029](matrix-org/synapse#10029)) Updates to the Docker image --------------------------- - Fix bug introduced in Synapse 1.33.0 which caused a `Permission denied: '/homeserver.log'` error when starting Synapse with the generated log configuration. Contributed by Sergio Miguéns Iglesias. ([\#10045](matrix-org/synapse#10045)) Improved Documentation ---------------------- - Add hardened systemd files as proposed in [#9760](matrix-org/synapse#9760) and added them to `contrib/`. Change the docs to reflect the presence of these files. ([\#9803](matrix-org/synapse#9803)) - Clarify documentation around SSO mapping providers generating unique IDs and localparts. ([\#9980](matrix-org/synapse#9980)) - Updates to the PostgreSQL documentation (`postgres.md`). ([\#9988](matrix-org/synapse#9988), [\#9989](matrix-org/synapse#9989)) - Fix broken link in user directory documentation. Contributed by @junquera. ([\#10016](matrix-org/synapse#10016)) - Add missing room state entry to the table of contents of room admin API. ([\#10043](matrix-org/synapse#10043)) Deprecations and Removals ------------------------- - Removed support for the deprecated `tls_fingerprints` configuration setting. Contributed by Jerin J Titus. ([\#9280](matrix-org/synapse#9280)) Internal Changes ---------------- - Allow sending full presence to users via workers other than the one that called `ModuleApi.send_local_online_presence_to`. ([\#9823](matrix-org/synapse#9823)) - Update comments in the space summary handler. ([\#9974](matrix-org/synapse#9974)) - Minor enhancements to the `@cachedList` descriptor. ([\#9975](matrix-org/synapse#9975)) - Split multipart email sending into a dedicated handler. ([\#9977](matrix-org/synapse#9977)) - Run `black` on files in the `scripts` directory. ([\#9981](matrix-org/synapse#9981)) - Add missing type hints to `synapse.util` module. ([\#9982](matrix-org/synapse#9982)) - Simplify a few helper functions. ([\#9984](matrix-org/synapse#9984), [\#9985](matrix-org/synapse#9985), [\#9986](matrix-org/synapse#9986)) - Remove unnecessary property from SQLBaseStore. ([\#9987](matrix-org/synapse#9987)) - Remove `keylen` param on `LruCache`. ([\#9993](matrix-org/synapse#9993)) - Update the Grafana dashboard in `contrib/`. ([\#10001](matrix-org/synapse#10001)) - Add a batching queue implementation. ([\#10017](matrix-org/synapse#10017)) - Reduce memory usage when verifying signatures on large numbers of events at once. ([\#10018](matrix-org/synapse#10018)) - Properly invalidate caches for destination retry timings every (instead of expiring entries every 5 minutes). ([\#10036](matrix-org/synapse#10036)) - Fix running complement tests with Synapse workers. ([\#10039](matrix-org/synapse#10039)) - Fix typo in `get_state_ids_for_event` docstring where the return type was incorrect. ([\#10050](matrix-org/synapse#10050))

@junquera

Synapse 1.35.0 (2021-06-01) =========================== Note that [the tag](https://github.com/matrix-org/synapse/releases/tag/v1.35.0rc3) and [docker images](https://hub.docker.com/layers/matrixdotorg/synapse/v1.35.0rc3/images/sha256-34ccc87bd99a17e2cbc0902e678b5937d16bdc1991ead097eee6096481ecf2c4?context=explore) for `v1.35.0rc3` were incorrectly built. If you are experiencing issues with either, it is recommended to upgrade to the equivalent tag or docker image for the `v1.35.0` release. Deprecations and Removals ------------------------- - The core Synapse development team plan to drop support for the [unstable API of MSC2858](https://github.com/matrix-org/matrix-doc/blob/master/proposals/2858-Multiple-SSO-Identity-Providers.md#unstable-prefix), including the undocumented `experimental.msc2858_enabled` config option, in August 2021. Client authors should ensure that their clients are updated to use the stable API (which has been supported since Synapse 1.30) well before that time, to give their users time to upgrade. ([\#10101](matrix-org/synapse#10101)) Bugfixes -------- - Fixed a bug causing replication requests to fail when receiving a lot of events via federation. Introduced in v1.33.0. ([\#10082](matrix-org/synapse#10082)) - Fix HTTP response size limit to allow joining very large rooms over federation. Introduced in v1.33.0. ([\#10093](matrix-org/synapse#10093)) Internal Changes ---------------- - Log method and path when dropping request due to size limit. ([\#10091](matrix-org/synapse#10091)) Synapse 1.35.0rc2 (2021-05-27) ============================== Bugfixes -------- - Fix a bug introduced in v1.35.0rc1 when calling the spaces summary API via a GET request. ([\#10079](matrix-org/synapse#10079)) Synapse 1.35.0rc1 (2021-05-25) ============================== Features -------- - Add experimental support to allow a user who could join a restricted room to view it in the spaces summary. ([\#9922](matrix-org/synapse#9922), [\#10007](matrix-org/synapse#10007), [\#10038](matrix-org/synapse#10038)) - Reduce memory usage when joining very large rooms over federation. ([\#9958](matrix-org/synapse#9958)) - Add a configuration option which allows enabling opentracing by user id. ([\#9978](matrix-org/synapse#9978)) - Enable experimental support for [MSC2946](matrix-org/matrix-spec-proposals#2946) (spaces summary API) and [MSC3083](matrix-org/matrix-spec-proposals#3083) (restricted join rules) by default. ([\#10011](matrix-org/synapse#10011)) Bugfixes -------- - Fix a bug introduced in v1.26.0 which meant that `synapse_port_db` would not correctly initialise some postgres sequences, requiring manual updates afterwards. ([\#9991](matrix-org/synapse#9991)) - Fix `synctl`'s `--no-daemonize` parameter to work correctly with worker processes. ([\#9995](matrix-org/synapse#9995)) - Fix a validation bug introduced in v1.34.0 in the ordering of spaces in the space summary API. ([\#10002](matrix-org/synapse#10002)) - Fixed deletion of new presence stream states from database. ([\#10014](matrix-org/synapse#10014), [\#10033](matrix-org/synapse#10033)) - Fixed a bug with very high resolution image uploads throwing internal server errors. ([\#10029](matrix-org/synapse#10029)) Updates to the Docker image --------------------------- - Fix bug introduced in Synapse 1.33.0 which caused a `Permission denied: '/homeserver.log'` error when starting Synapse with the generated log configuration. Contributed by Sergio Miguéns Iglesias. ([\#10045](matrix-org/synapse#10045)) Improved Documentation ---------------------- - Add hardened systemd files as proposed in [#9760](matrix-org/synapse#9760) and added them to `contrib/`. Change the docs to reflect the presence of these files. ([\#9803](matrix-org/synapse#9803)) - Clarify documentation around SSO mapping providers generating unique IDs and localparts. ([\#9980](matrix-org/synapse#9980)) - Updates to the PostgreSQL documentation (`postgres.md`). ([\#9988](matrix-org/synapse#9988), [\#9989](matrix-org/synapse#9989)) - Fix broken link in user directory documentation. Contributed by @junquera. ([\#10016](matrix-org/synapse#10016)) - Add missing room state entry to the table of contents of room admin API. ([\#10043](matrix-org/synapse#10043)) Deprecations and Removals ------------------------- - Removed support for the deprecated `tls_fingerprints` configuration setting. Contributed by Jerin J Titus. ([\#9280](matrix-org/synapse#9280)) Internal Changes ---------------- - Allow sending full presence to users via workers other than the one that called `ModuleApi.send_local_online_presence_to`. ([\#9823](matrix-org/synapse#9823)) - Update comments in the space summary handler. ([\#9974](matrix-org/synapse#9974)) - Minor enhancements to the `@cachedList` descriptor. ([\#9975](matrix-org/synapse#9975)) - Split multipart email sending into a dedicated handler. ([\#9977](matrix-org/synapse#9977)) - Run `black` on files in the `scripts` directory. ([\#9981](matrix-org/synapse#9981)) - Add missing type hints to `synapse.util` module. ([\#9982](matrix-org/synapse#9982)) - Simplify a few helper functions. ([\#9984](matrix-org/synapse#9984), [\#9985](matrix-org/synapse#9985), [\#9986](matrix-org/synapse#9986)) - Remove unnecessary property from SQLBaseStore. ([\#9987](matrix-org/synapse#9987)) - Remove `keylen` param on `LruCache`. ([\#9993](matrix-org/synapse#9993)) - Update the Grafana dashboard in `contrib/`. ([\#10001](matrix-org/synapse#10001)) - Add a batching queue implementation. ([\#10017](matrix-org/synapse#10017)) - Reduce memory usage when verifying signatures on large numbers of events at once. ([\#10018](matrix-org/synapse#10018)) - Properly invalidate caches for destination retry timings every (instead of expiring entries every 5 minutes). ([\#10036](matrix-org/synapse#10036)) - Fix running complement tests with Synapse workers. ([\#10039](matrix-org/synapse#10039)) - Fix typo in `get_state_ids_for_event` docstring where the return type was incorrect. ([\#10050](matrix-org/synapse#10050))

clokep mentioned this pull request May 12, 2021

Additional comments for space summary handler #9974

Merged

clokep force-pushed the clokep/restricted-rooms-in-space-summary branch from 913cdab to 9e562e3 Compare May 13, 2021 13:51

clokep mentioned this pull request May 13, 2021

Add a test for restricted rooms appearing in the spaces summary (MSC3083). matrix-org/complement#109

Merged

clokep marked this pull request as ready for review May 13, 2021 16:39

clokep requested a review from a team May 13, 2021 17:20

clokep added 7 commits May 17, 2021 10:21

Allow a user who could join a restricted room to see it in spaces sum…

bce96c2

…mary. Allows discovery of restricted rooms via the intersection of MSC2946 and MSC3083.

Check if the origin server can access a room when queried over federa…

0a881ac

…tion.

Check if an origin server has access to any spaces for a restricted r…

45ad823

…oom when queried over federation.

Do not revisit any of the rooms received over federation.

5225ef1

Fix filtering for world readable rooms.

31bb2ad

Abstract out a helper for checking if a space is in a list.

417aa6c

Return the allowed spaces over federation and filter on it.

36d3932

clokep force-pushed the clokep/restricted-rooms-in-space-summary branch from 9e562e3 to 36d3932 Compare May 17, 2021 14:21

erikjohnston reviewed May 17, 2021

View reviewed changes

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

Remove unused variable.

9157200

sumnerevans suggested changes May 17, 2021

View reviewed changes

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

Fix typo.

c97917b

Co-authored-by: Sumner Evans <me@sumnerevans.com>

clokep requested a review from erikjohnston May 17, 2021 18:09

erikjohnston previously approved these changes May 18, 2021

View reviewed changes

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

clokep added 3 commits May 18, 2021 12:17

Split the method in two.

ce8efa9

Further split the methods.

83974e8

Merge branch 'clokep/check-restricted-join-rules-2' into clokep/restr…

3bd3fe0

…icted-rooms-in-space-summary

clokep mentioned this pull request May 18, 2021

Refactor the checking restricted join rules #10009

Closed

erikjohnston reviewed May 19, 2021

View reviewed changes

clokep and others added 4 commits May 19, 2021 09:36

Clarify comment.

225e06d

Co-authored-by: Erik Johnston <erik@matrix.org>

Clarify comments about when a room can be included.

7c28513

Fix-up some type hints (and fix a bug about processed rooms).

7c0ec1a

Further split the join rules checks.

4888f02

clokep force-pushed the clokep/restricted-rooms-in-space-summary branch from d720615 to 4888f02 Compare May 19, 2021 15:47

erikjohnston reviewed May 19, 2021

View reviewed changes

Fix typo.

9ba347b

Co-authored-by: Erik Johnston <erik@matrix.org>

clokep requested a review from erikjohnston May 19, 2021 17:56

Remove some duplicated code.

fe1eb18

erikjohnston approved these changes May 20, 2021

View reviewed changes

clokep merged commit 551d2c3 into develop May 20, 2021

clokep deleted the clokep/restricted-rooms-in-space-summary branch May 20, 2021 15:10

This was referenced May 21, 2021

Remove unused properties on SpaceSummaryHandler #10038

Merged

error 500 if room in space destroyed, but still in space, and then viewed over federation #10032

Closed

Pass the origin when calculating the spaces summary over GET. #10079

Merged

This was referenced Jun 2, 2021

all space members are able to see invite only rooms in spaces #10104

Closed

Do not show invite-only rooms in spaces summary (unless joined/invited). #10109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow a user who could join a restricted room to see it in spaces summary. #9922

Allow a user who could join a restricted room to see it in spaces summary. #9922

clokep commented May 3, 2021 •

edited

Loading

erikjohnston left a comment

clokep commented May 17, 2021

erikjohnston left a comment

erikjohnston May 19, 2021

clokep May 19, 2021

erikjohnston May 19, 2021

clokep May 19, 2021

erikjohnston May 19, 2021

clokep May 19, 2021

clokep commented May 19, 2021

erikjohnston left a comment

Allow a user who could join a restricted room to see it in spaces summary. #9922

Allow a user who could join a restricted room to see it in spaces summary. #9922

Conversation

clokep commented May 3, 2021 • edited Loading

erikjohnston left a comment

Choose a reason for hiding this comment

clokep commented May 17, 2021

erikjohnston left a comment

Choose a reason for hiding this comment

erikjohnston May 19, 2021

Choose a reason for hiding this comment

clokep May 19, 2021

Choose a reason for hiding this comment

erikjohnston May 19, 2021

Choose a reason for hiding this comment

clokep May 19, 2021

Choose a reason for hiding this comment

erikjohnston May 19, 2021

Choose a reason for hiding this comment

clokep May 19, 2021

Choose a reason for hiding this comment

clokep commented May 19, 2021

erikjohnston left a comment

Choose a reason for hiding this comment

clokep commented May 3, 2021 •

edited

Loading