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

We don't send push notifications for invites from remote servers (for rooms the local server isn't in). #8626

Closed
erikjohnston opened this issue Oct 22, 2020 · 5 comments · Fixed by #13719
Labels
A-Invite Inviting users to rooms and accepting invites S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@erikjohnston
Copy link
Member

erikjohnston commented Oct 22, 2020

The invite comes in via federation /invite and gets persisted as an outlier. This flow bypasses the handle_push_actions_for_event code.

    def test_remote_invite_sends_email(self):
        fed_handler = self.hs.get_federation_handler()

        fake_invite = {
            "auth_events": ["$dfdjslfdkf", "$sdfdjflsjoiy"],
            "prev_events": ["$sdfdjflsjoiy"],
            "type": "m.room.member",
            "state_key": self.user_id,
            "content": {"membership": "invite"},
            "sender": "@test:remote.example.com",
            "room_id": "!soom_room:remote.example.com",
            "depth": 3872,
        }

        testkey = signedjson.key.generate_signing_key("ver1")

        invite_event = create_local_event_from_event_dict(
            self.clock,
            hostname="remote.example.com",
            signing_key=testkey,
            room_version=RoomVersions.V6,
            event_dict=fake_invite,
        )

        self.get_success(
            fed_handler.on_invite_request(
                "remote.example.com", invite_event, RoomVersions.V6,
            )
        )

        # We should get emailed about the invite
        self._check_for_mail()

Is a unit test failing, if added to tests/push/test_email.py

@clokep clokep added z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Oct 22, 2020
@richvdh
Copy link
Member

richvdh commented Oct 22, 2020

is this the same as #8531?

@erikjohnston
Copy link
Member Author

Oh, yes I think so (.. how did that not come up in my search 😕 )

@richvdh
Copy link
Member

richvdh commented Oct 22, 2020

related: #8605 ?

@erikjohnston
Copy link
Member Author

Ish, though it's a different bug with probably a different fix

@Bubu
Copy link
Contributor

Bubu commented Jun 18, 2021

I've worked on a solution to this a while back but wasn't happy enough with it to start the upstreaming process.

My totally naïve approach to this was bypassing the database persistence for this narrow case of push notifications and triggering pushers directly as synapse receives the event over federation. I'm now wondering if this approach has any chance of being applicable for merging.

The current iteration also is restricted to http pushers, but I'm currently working on making it more general.

@erikjohnston erikjohnston added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. and removed z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Jul 23, 2021
@MadLittleMods MadLittleMods added the A-Invite Inviting users to rooms and accepting invites label May 16, 2022
@anoadragon453 anoadragon453 changed the title We don't push invites from remote servers (for rooms the local server isn't in). We don't send push notifications for invites from remote servers (for rooms the local server isn't in). Aug 22, 2022
@squahtx squahtx linked a pull request Sep 9, 2022 that will close this issue
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Invite Inviting users to rooms and accepting invites S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
5 participants