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

Email notifications for new users when creating via the Admin API. #7267

Merged
merged 3 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7267.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix email notifications not being enabled for new users when created via the Admin API.
16 changes: 16 additions & 0 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def __init__(self, hs):
self.set_password_handler = hs.get_set_password_handler()
self.deactivate_account_handler = hs.get_deactivate_account_handler()
self.registration_handler = hs.get_registration_handler()
self.pusher_pool = hs.get_pusherpool()

async def on_GET(self, request, user_id):
await assert_requester_is_admin(self.auth, request)
Expand Down Expand Up @@ -275,6 +276,21 @@ async def on_PUT(self, request, user_id):
await self.auth_handler.add_threepid(
user_id, threepid["medium"], threepid["address"], current_time
)
if (
self.hs.config.email_enable_notifs
and self.hs.config.email_notif_for_new_users
):
await self.pusher_pool.add_pusher(
user_id=user_id,
access_token=None,
Copy link
Member

@clokep clokep Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the access token be None? I'm skeptical this will work as expected due to the care that is taken with it in the standard API:

https://github.com/matrix-org/synapse/blob/v1.12.0/synapse/handlers/register.py#L633-L638

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The admin user has no token of the affected user. What will be the alternativ? Is it not possible for admin users to create the pusher? The access_token is needed for removing the pusher.

https://github.com/matrix-org/synapse/blob/v1.12.0/synapse/push/pusherpool.py#L170-L172

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the ramifications of doing this would be. I'm going to set the review flag on this and see if others have thoughts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The database table is fine with it being null, and I think the only thing we use it for is to delete the pusher when the associated device is logged out. I'd be slightly surprised if that also happened to email push, but might explain why some people are reporting email push as flakey?

kind="email",
app_id="m.email",
app_display_name="Email Notifications",
device_display_name=threepid["address"],
pushkey=threepid["address"],
lang=None, # We don't know a user's language here
data={},
)

if "avatar_url" in body:
await self.profile_handler.set_avatar_url(
Expand Down
75 changes: 75 additions & 0 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,81 @@ def test_create_user(self):
self.assertEqual(False, channel.json_body["is_guest"])
self.assertEqual(False, channel.json_body["deactivated"])

def test_create_user_email_notif_for_new_users(self):
"""
Check that a new regular user is created successfully and
got an email pusher.
"""
self.hs.config.registration_shared_secret = None
self.hs.config.email_enable_notifs = True
self.hs.config.email_notif_for_new_users = True
url = "/_synapse/admin/v2/users/@bob:test"

# Create user
body = json.dumps(
{
"password": "abc123",
"threepids": [{"medium": "email", "address": "bob@bob.bob"}],
}
)

request, channel = self.make_request(
"PUT",
url,
access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"),
)
self.render(request)

self.assertEqual(201, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@bob:test", channel.json_body["name"])
self.assertEqual("email", channel.json_body["threepids"][0]["medium"])
self.assertEqual("bob@bob.bob", channel.json_body["threepids"][0]["address"])

pushers = self.get_success(
self.store.get_pushers_by({"user_name": "@bob:test"})
)
pushers = list(pushers)
self.assertEqual(len(pushers), 1)
self.assertEqual("@bob:test", pushers[0]["user_name"])

def test_create_user_email_no_notif_for_new_users(self):
"""
Check that a new regular user is created successfully and
got not an email pusher.
"""
self.hs.config.registration_shared_secret = None
self.hs.config.email_enable_notifs = False
self.hs.config.email_notif_for_new_users = False
url = "/_synapse/admin/v2/users/@bob:test"

# Create user
body = json.dumps(
{
"password": "abc123",
"threepids": [{"medium": "email", "address": "bob@bob.bob"}],
}
)

request, channel = self.make_request(
"PUT",
url,
access_token=self.admin_user_tok,
content=body.encode(encoding="utf_8"),
)
self.render(request)

self.assertEqual(201, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("@bob:test", channel.json_body["name"])
self.assertEqual("email", channel.json_body["threepids"][0]["medium"])
self.assertEqual("bob@bob.bob", channel.json_body["threepids"][0]["address"])

pushers = self.get_success(
self.store.get_pushers_by({"user_name": "@bob:test"})
)
pushers = list(pushers)
self.assertEqual(len(pushers), 0)

def test_set_password(self):
"""
Test setting a new password for another user.
Expand Down