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

Removing an email does not remove the notifications #10568

Closed
nirgal opened this issue Aug 10, 2021 · 1 comment · Fixed by #10581
Closed

Removing an email does not remove the notifications #10568

nirgal opened this issue Aug 10, 2021 · 1 comment · Fixed by #10581
Assignees
Labels
A-Email-Push Email notifications A-Push Issues related to push/notifications good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@nirgal
Copy link

nirgal commented Aug 10, 2021

Hello

When you add an email, then activate notifications by email at that address, and then remove the email from your profile, you still get notifications.

I believe removing the email (threepid) from your profile should remove the notification.
Alternatively, the email notification block should allow you to remove it.

The work around is to add the email back, disable the notification, then remove the email again.

However, if you lost access to an email account, you can find yourself in a position when you have absolutely not way of removing the notifications by email. This is really a problem.

Fixing that issue would probably fix bug #4958 too.

@anoadragon453
Copy link
Member

Email notifications are implemented as "pushers" in Synapse, and are listed in the pushers database table. The email is stored here as well - which explains why email notifications continue to work even after an email has been removed from a user's account.

The fix is fairly straightforward. First, one would need to add a storage method for removing a pusher based on a user's Matrix ID and email address. Probably right next to this method:

async def delete_all_pushers_for_user(self, user_id: str) -> None:
"""Delete all pushers associated with an account."""
# We want to generate a row in `deleted_pushers` for each pusher we're
# deleting, so we fetch the list now so we can generate the appropriate
# number of stream IDs.
#
# Note: technically there could be a race here between adding/deleting
# pushers, but a) the worst case if we don't stop a pusher until the
# next restart and b) this is only called when we're deactivating an
# account.
pushers = list(await self.get_pushers_by_user_id(user_id))

Then, they'd need to call that method when an email was removed from an account - i.e from the end of:

async def delete_threepid(
self, user_id: str, medium: str, address: str, id_server: Optional[str] = None
) -> bool:
"""Attempts to unbind the 3pid on the identity servers and deletes it
from the local database.
Args:
user_id: ID of user to remove the 3pid from.
medium: The medium of the 3pid being removed: "email" or "msisdn".
address: The 3pid address to remove.
id_server: Use the given identity server when unbinding
any threepids. If None then will attempt to unbind using the
identity server specified when binding (if known).
Returns:
Returns True if successfully unbound the 3pid on
the identity server, False if identity server doesn't support the
unbind API.
"""
# 'Canonicalise' email addresses as per above
if medium == "email":
address = canonicalise_email(address)
identity_handler = self.hs.get_identity_handler()
result = await identity_handler.try_unbind_threepid(
user_id, {"medium": medium, "address": address, "id_server": id_server}
)
await self.store.user_delete_threepid(user_id, medium, address)
return result

@anoadragon453 anoadragon453 added good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Aug 11, 2021
@MadLittleMods MadLittleMods added A-Email-Push Email notifications A-Push Issues related to push/notifications labels Aug 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Email-Push Email notifications A-Push Issues related to push/notifications good first issue Good for newcomers S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants