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

Return an appropriate response when getting joined_members after leaving #2851

Closed
turt2live opened this issue Feb 6, 2018 · 5 comments · Fixed by #13374
Closed

Return an appropriate response when getting joined_members after leaving #2851

turt2live opened this issue Feb 6, 2018 · 5 comments · Fixed by #13374
Labels
good first issue Good for newcomers P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@turt2live
Copy link
Member

2018-02-05 19:59:38,357 - synapse.http.server - 183 - ERROR - GET-132067- Failed handle request synapse.http.server._async_render on <synapse.rest.ClientRest
Resource object at 0x7f9dae55b590>: <XForwardedForRequest at 0x7f9d7856c2d8 method=GET uri=/_matrix/client/r0/rooms/!vfFxDRtZSSdspfTSEr:matrix.org/joined_mem
bers?access_token=<redacted> clientproto=HTTP/1.1 site=8008>: Traceback (most recent call last):
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1532, in unwindGenerator
    return _inlineCallbacks(None, gen, Deferred())
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1386, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/synapse/rest/client/v1/room.py", line 408, in on_GET
    requester, room_id,
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1532, in unwindGenerator
    return _inlineCallbacks(None, gen, Deferred())
--- <exception caught here> ---
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1386, in _inlineCallbacks
    result = g.send(result)
  File "/home/travis/.synapse/local/lib/python2.7/site-packages/synapse/handlers/message.py", line 449, in get_joined_members
    "Getting joined members after leaving is not implemented"
exceptions.NotImplementedError: Getting joined members after leaving is not implemented
@ara4n ara4n added z-bug (Deprecated Label) z-p3 (Deprecated Label) z-minor (Deprecated Label) labels Feb 6, 2018
@DMRobertson
Copy link
Contributor

Shows up smiregularly in Sentry here: https://sentry.tools.element.io/organizations/element/issues/55/?project=2

The spec allows us to return 403 in this situation.

@DMRobertson DMRobertson added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches good first issue Good for newcomers and removed z-p3 (Deprecated Label) z-minor (Deprecated Label) z-bug (Deprecated Label) labels Jun 14, 2022
@DMRobertson
Copy link
Contributor

If anyone wants to take this on:

  1. Change this to raise a SynapseError with status code 403, an appropriate error message and and appropriate matrix error code.
  2. Add a test case which asserts that Synapse responses with status code 403 if you try to get the joined members of a room you've left.
  3. Optional, harder: for bonus points, write a complement test case instead of a Synapse unit test.

@andrewdoh
Copy link
Contributor

Hi @DMRobertson id like to work on this. I've created the two PRs below. please check them out, thanks.
#13374
matrix-org/complement#424

@akstron
Copy link
Contributor

akstron commented Apr 21, 2023

Hi @DMRobertson, seems like this issue is already resolved but not closed. Is there anything else which needs to be done?

@DMRobertson DMRobertson linked a pull request Apr 21, 2023 that will close this issue
4 tasks
@DMRobertson DMRobertson changed the title Return 404 or member list when getting joined_members after leaving Return an appropriate response when getting joined_members after leaving Apr 21, 2023
@DMRobertson
Copy link
Contributor

This is done, but wasn't marked as closed by the corresponding PR. Note that we ended up returning 403, contrary to the original title.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants