Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Make room alias lists peekable #6949

Merged
merged 4 commits into from
Feb 19, 2020
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Feb 18, 2020

As per matrix-org/matrix-spec-proposals#2432 (review), make room alias lists accessible to users outside world_readable rooms.

This also involves a bit of a refactor in Auth, in particular trying to avoid yet another method for checking user room membership, by combining check_joined_room and check_user_was_in_room. The commits should stand alone and be independently reviewable.

@richvdh richvdh force-pushed the rav/list_room_aliases_peekable branch from e0ba252 to bdeb189 Compare February 18, 2020 22:52
@@ -433,25 +433,3 @@ def snapshot_all_rooms(
ret["membership"] = membership

return ret

async def _check_in_room_or_world_readable(self, room_id, user_id):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was an exact cut-and-paste of Auth.check_in_room_or_world_readable; removed for sanity.

@richvdh richvdh force-pushed the rav/list_room_aliases_peekable branch from bdeb189 to f7c6bec Compare February 18, 2020 23:16
@@ -662,7 +664,9 @@ def check_in_room_or_world_readable(self, room_id, user_id):
):
return Membership.JOIN, None
raise AuthError(
403, "Guest access not allowed", errcode=Codes.GUEST_ACCESS_FORBIDDEN
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was never anything to do with guest access, so the error message was wrong and confusing; I've fixed it up while I'm here.

@@ -633,10 +625,18 @@ def get_access_token_from_request(request):
return query_params[0].decode("ascii")

@defer.inlineCallbacks
def check_in_room_or_world_readable(self, room_id, user_id):
def check_user_in_room_or_world_readable(
self, room_id: str, user_id: str, allow_departed_users: bool = False
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this defaults to False for consistency with check_user_in_room, and for safety (if you forget to set it, it's safer if we deny access to users that should have it than grant access to users that should).

@richvdh richvdh requested a review from a team February 18, 2020 23:19
these were getting a bit unwieldy, so let's combine `check_joined_room` and
`check_user_was_in_room` into a single `check_user_in_room`.
... and set it everywhere it's called.

while we're here, rename it for consistency with `check_user_in_room` (and to
help check that I haven't missed any instances)
As per
matrix-org/matrix-spec-proposals#2432 (review),
make room alias lists accessible to users outside world_readable rooms.
@richvdh richvdh force-pushed the rav/list_room_aliases_peekable branch from 31e9e9e to 603618c Compare February 19, 2020 08:53
@richvdh richvdh merged commit 2fb7794 into develop Feb 19, 2020
@richvdh richvdh deleted the rav/list_room_aliases_peekable branch February 19, 2020 11:19
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '2fb7794e6':
  changelog
  Make room alias lists peekable
  Add `allow_departed_users` param to `check_in_room_or_world_readable`
  Refactor the membership check methods in Auth
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants