Handle rejections of invites from local users locally #615

richvdh · Mar 1, 2016

Slightly hacky fix to SYN-642, which avoids the federation codepath when trying
to reject invites from local users.

richvdh · Mar 1, 2016

I'm a bit confused about this. The canonical check for if we received the invite over federation is if inviter.domain != self.server_name?

True, but I guess here we're actually less interested in whether the event arrived over federation, and more in whether we already have it in context.current_state. If the event is in context.current_state despite being received over federation, there's no point dropping the context.

I guess the comment is unclear. If you're happy otherwise I could update the comment.

Aha, right, yes, it was mainly the comment that confused me.

Call this context_event_ids

done, and comment rewritten.

richvdh · Mar 2, 2016

maybe @illicitonion should also take a look at this since he's been in the area recently.

erikjohnston · Mar 2, 2016

Looks ok to me, though I think @illicitonion should have a quick peek since I don't really know how any of this works anymore :)

self.hs.is_mine(inviter)?

I just lifted the code from do_remotely_reject_invite. I guess your way is better

if anybody had bothered to document what self.hs actually is, this would have been somewhat easier to figure out.

Being really picky, I could argue that this should probably be an append not an overwrite, because the caller may have specified some useful hints.

I know that none of the callers currently do, but...

agreed; thanks.

illicitonion · Mar 2, 2016

LGTM, small handful of comments

richvdh · Mar 2, 2016

@illicitonion ptal?

illicitonion · Mar 2, 2016

I generally favour this being the very first line of a function, so that it's obvious what's going on (because it's really compensating for the fact that default params are mutably stateful across invocations), and so that it doesn't matter what codepath you try to access/modify from

illicitonion · Mar 2, 2016

LGTM, one minor comment

richvdh · Mar 2, 2016

retest this please

richvdh · Mar 2, 2016

retest this please

richvdh · Mar 3, 2016

retest this please

pleeeease

richvdh · Mar 3, 2016

retest this please

erikjohnston · Mar 3, 2016

retest this please

richvdh · Mar 3, 2016

retest this please

richvdh assigned erikjohnston Mar 1, 2016

erikjohnston assigned illicitonion and unassigned erikjohnston Mar 2, 2016

Address review comments

9ff940a

fix pyflakes quibble

All checks have failed
1 errored and 1 failing checks

Synapse — Build #6263 origin/pr/615/head failed in 5 min 33 sec

Details

default — Build finished. 372 tests run, 0 skipped, 1 failed.

Details

863d3f2

Move arg default to the start of the function
Also don't overwrite the list that gets passed in.

fc1f932

richvdh assigned richvdh and unassigned illicitonion Mar 2, 2016

Fix typo

74cd80e

richvdh deleted the rav/SYN-642 branch Mar 4, 2016

@@ -208,7 +207,18 @@ def _create_new_client_event(self, builder):
                  prev_member_event = yield self.store.get_room_member(
                      builder.sender, builder.room_id
                  )
-    -            if prev_member_event:
+    +
+    +            # if we have the prev_member_event in context, we didn't receive
+    +            # the invite over federation. (More likely is that the inviting
+    +            # user, and all other local users, have left, making
+    +            # is_host_in_room return false).
+    +            #
+    +            context_events = (e.event_id for e in context.current_state.values())

                  if not is_host_in_room:
-    -                action = "remote_reject"
+    +                # perhaps we've been invited
+    +                inviter = self.get_inviter(target_user.to_string(), context.current_state)
+    +                if not inviter:
+    +                    raise SynapseError(404, "Not a known room")
+    +
+    +                if inviter.domain == self.server_name:

-    -                action = "remote_reject"
+    +                # perhaps we've been invited
+    +                inviter = self.get_inviter(target_user.to_string(), context.current_state)
+    +                if not inviter:
+    +                    raise SynapseError(404, "Not a known room")
+    +
+    +                if inviter.domain == self.server_name:
+    +                    # the inviter was on our server, but has now left. Carry on
+    +                    # with the normal rejection codepath.
+    +                    #
+    +                    # This is a bit of a hack, because the room might still be
+    +                    # active on other servers.
+    +                    pass
+    +                else:
+    +                    # send the rejection to the inviter's HS.
+    +                    remote_room_hosts = [inviter.domain]

matrix-org/synapse

Handle rejections of invites from local users locally #615

richvdh commented Mar 1, 2016

richvdh added some commits Mar 1, 2016

richvdh assigned erikjohnston Mar 1, 2016

richvdh commented Mar 1, 2016

erikjohnston and 2 others commented on an outdated diff Mar 2, 2016

richvdh commented Mar 2, 2016

erikjohnston commented Mar 2, 2016

erikjohnston assigned illicitonion and unassigned erikjohnston Mar 2, 2016

illicitonion and 1 other commented on an outdated diff Mar 2, 2016

illicitonion and 1 other commented on an outdated diff Mar 2, 2016

illicitonion commented Mar 2, 2016

richvdh commented Mar 2, 2016

illicitonion commented on `synapse/handlers/room.py` in `9ff940a` Mar 2, 2016

illicitonion commented Mar 2, 2016

richvdh assigned richvdh and unassigned illicitonion Mar 2, 2016

richvdh commented Mar 2, 2016

richvdh commented Mar 2, 2016

richvdh commented Mar 3, 2016

richvdh commented Mar 3, 2016

erikjohnston commented Mar 3, 2016

richvdh added some commits Mar 3, 2016

richvdh commented Mar 3, 2016

richvdh added a commit that referenced this pull request Mar 4, 2016

richvdh merged commit `fa6d6bb` into develop Mar 4, 2016
7 of 8 checks passed

7 of 8 checks passed

richvdh deleted the rav/SYN-642 branch Mar 4, 2016

matrix-org/synapse

Join GitHub today

Handle rejections of invites from local users locally #615

Conversation

richvdh commented Mar 1, 2016

richvdh added some commits Mar 1, 2016

All checks have failed

All checks have passed

richvdh assigned erikjohnston Mar 1, 2016

richvdh commented Mar 1, 2016

Show 6 comments Hide comments erikjohnston and 2 others commented on an outdated diff Mar 2, 2016

richvdh commented Mar 2, 2016

erikjohnston commented Mar 2, 2016

erikjohnston assigned illicitonion and unassigned erikjohnston Mar 2, 2016

Show 2 comments Hide comments illicitonion and 1 other commented on an outdated diff Mar 2, 2016

Show 2 comments Hide comments illicitonion and 1 other commented on an outdated diff Mar 2, 2016

illicitonion commented Mar 2, 2016

richvdh commented Mar 2, 2016

All checks have failed

illicitonion commented on synapse/handlers/room.py in 9ff940a Mar 2, 2016

illicitonion commented Mar 2, 2016

richvdh assigned richvdh and unassigned illicitonion Mar 2, 2016

richvdh commented Mar 2, 2016

richvdh commented Mar 2, 2016

richvdh commented Mar 3, 2016

Some checks were not successful

richvdh commented Mar 3, 2016

erikjohnston commented Mar 3, 2016

richvdh added some commits Mar 3, 2016

Some checks were not successful

Some checks were not successful

Some checks were not successful

richvdh commented Mar 3, 2016

richvdh added a commit that referenced this pull request Mar 4, 2016

Some checks were not successful

Hide details View details richvdh merged commit fa6d6bb into develop Mar 4, 2016 7 of 8 checks passed

7 of 8 checks passed

richvdh deleted the rav/SYN-642 branch Mar 4, 2016

erikjohnston and 2 others commented on an outdated diff Mar 2, 2016

illicitonion and 1 other commented on an outdated diff Mar 2, 2016

illicitonion and 1 other commented on an outdated diff Mar 2, 2016

illicitonion commented on `synapse/handlers/room.py` in `9ff940a` Mar 2, 2016

richvdh merged commit `fa6d6bb` into develop Mar 4, 2016
7 of 8 checks passed