-
Notifications
You must be signed in to change notification settings - Fork 61
Test that invites can be rescinded over federation #797
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
Conversation
|
Very sensible, I'm surprised we didn't! |
Co-authored-by: Eric Eastwood <[email protected]>
Co-authored-by: Eric Eastwood <[email protected]>
| "invite": []string{bob.UserID}, | ||
| }) | ||
| bob.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(bob.UserID, roomID)) | ||
| alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, |
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.
It would be good to have a test where someone else rescinds the invite (someone different than the inviter)
Also a test for someone else from another server in the room
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.
Test was added for the first case but not the second for someone else from another server in the room
| bob.MustSyncUntil(t, client.SyncReq{Filter: includeLeaveSyncFilter}, client.SyncLeftFrom(bob.UserID, roomID)) | ||
| }) | ||
|
|
||
| t.Run("Non-invitee user cannot rescind invite over federation", func(t *testing.T) { |
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.
| t.Run("Non-invitee user cannot rescind invite over federation", func(t *testing.T) { | |
| t.Run("Non-inviter user cannot rescind invite over federation", func(t *testing.T) { |
| // Alice, not the original inviter, kicks bob. This does not result | ||
| // in bob seeing the rescission. |
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.
We should comment that it would be fine for bob to see the rescission but they have no way to auth it because of X so they can't take it into account
| "invite": []string{bob.UserID}, | ||
| }) | ||
| bob.MustSyncUntil(t, client.SyncReq{}, client.SyncInvitedTo(bob.UserID, roomID)) | ||
| alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "kick"}, |
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.
Test was added for the first case but not the second for someone else from another server in the room
We should send events that rescind invites over federation. Similarly, we should handle receiving such events. Unfortunately, the protocol doesn't make it possible to fully auth such events, and so we can only handle the case where the original inviter rescinded the invite (rather than a room admin). Complement test: matrix-org/complement#797
|
Oh blast I thought I had dealt with all the reviews. I'll do them in another PR! |
We should send events that rescind invites over federation. Similarly, we should handle receiving such events. Unfortunately, the protocol doesn't make it possible to fully auth such events, and so we can only handle the case where the original inviter rescinded the invite (rather than a room admin). Complement test: matrix-org/complement#797
c.f. element-hq/synapse#18823