Add admin API to send a server_notice to one/all users by krombel · Pull Request #3531 · matrix-org/synapse

krombel · 2018-07-13T14:49:46Z

In #3525 it got requested to have an easier way to send a server notice
to all users on the server.

Besides the query that got mentioned there this adds another command
which can be called via manhole to send those messages.

This PR adds admin API as follows:

http://localhost:8008/_matrix/client/api/v1/admin/send_server_notice/@dest:user?access_token=admin_access_token
JsonBodyToSend:
    {
        "event": {'msgtype':'m.text', 'body':'This is my message'}
    }
    or simply
    {
        'event_body': 'This is my message'
    }

@dest:user is optional

This would fix #3532

Signed-Off-By: Matthias Kesler krombel@krombel.de

matrixbot · 2018-07-13T14:49:48Z

Can one of the admins verify this patch?

matrixbot · 2018-07-13T14:49:48Z

Can one of the admins verify this patch?

richvdh · 2018-07-13T15:09:11Z

let's not add too much magic which is only meant to be used via the manhole. better to add admin apis.

t3chguy · 2018-07-13T18:18:54Z

Seems like all the Admin_APIs in https://github.com/matrix-org/synapse/tree/master/docs/admin_api reference r0, your example references v1

t3chguy · 2018-07-13T18:19:19Z

you seem to switch between v1 and r0 interchangeably...

t3chguy · 2018-07-13T18:19:56Z

so is it r0 or v1?

it was a copy/paste error. I got confused by the doc strings of the other methods.
It should be r0 AFAIK. I have changed it now

t3chguy · 2018-07-13T20:07:19Z

it seems like it is actually v1

It is that class but it adds the /client/r0/ endpoints as well.

richvdh · 2018-08-13T17:14:56Z

could you stick this under docs/admin_api, with a link here to say "there is an API which is documented at [...]"?

richvdh · 2018-08-13T17:17:16Z

richvdh · 2018-08-13T17:17:33Z

"a lot of users". Or better: "a large number of registered users."

richvdh · 2018-08-13T17:17:51Z

richvdh · 2018-08-13T17:19:44Z

if you drop the access_token (which is kinda obvious), does this fit on one line?

If not, can you indent this line more?

also, I suggest @user:host rather than @dest:user

richvdh · 2018-08-13T17:22:43Z

I feel like sending a message to all users is a dangerous thing that might happen by accident. Would it be better to have separate APIs

/admin/send_server_notice_to_user/<user_id>

/admin/send_server_notice_to_all_users

?

It would cause some code duplication as I would have to create a new servlet for that.
Personally I think there is a reason why we require an admin token so that the admin should take care that (s)he does it right.
But I can change it if you like.

well, fair enough. I won't insist.

richvdh · 2018-08-13T17:26:03Z

surely we should yield here to wait for the request to complete?

In matrix-org#3525 it got requested to have an easier way to send a server notice to all users on the server. Besides the query that got mentioned there this adds another command which can be called via manhole to send those messages. Signed-Off-By: Matthias Kesler <krombel@krombel.de>

richvdh · 2018-08-13T20:03:16Z



+class ServerNoticeRestServlet(ClientV1RestServlet):
+    PATTERNS = client_path_patterns("/admin/send_server_notice(/(?P<user_id>[^/]+))?")


I'm trying to figure out whether there is a difference in behaviour between /admin/send_server_notice/ (with a trailing slash and an empty user_id) and /admin/send_server_notice (with no trailing slash). AFAICT the former will fail, but the docs say it is valid?

I'd suggest that you should not accept /admin/send_server_notice/, and the docs should be updated to clarify.

richvdh · 2019-05-02T09:49:36Z

I have taken this PR as a basis for another implementation, over in #5121. It doesn't currently implement the "send-to-all" functionality, so that is left for a future PR. My feeling is that should be a separate endpoint anyway.

In the meantime, I'm going to close this PR. Thank you for your work on this, @krombel!

eMPee584 · 2021-05-29T07:54:15Z

[…]
So is it currently possible to use a wildcard for the /admin/send_server_notice_to_user/ endpoint to send everyone a notice, or would it need to be scripted? @krombel's well-looking draft implementation of an endpoint seems to have been /dev/nulled?

krombel force-pushed the extend_server_notices_sendToAll branch from 994826b to 5ed5a60 Compare July 13, 2018 14:51

krombel force-pushed the extend_server_notices_sendToAll branch from 692dd05 to f4c79ea Compare July 13, 2018 16:37

krombel changed the title ~~Adds method to send a server_notice to all rooms~~ Adds admin API to send a server_notice to one/all users Jul 13, 2018

krombel force-pushed the extend_server_notices_sendToAll branch 2 times, most recently from 692dd05 to f4c79ea Compare July 13, 2018 17:34

t3chguy reviewed Jul 13, 2018

View reviewed changes

Comment thread synapse/rest/client/v1/admin.py Outdated

Copy link
Copy Markdown

Member

t3chguy Jul 13, 2018

Choose a reason for hiding this comment

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

you seem to switch between v1 and r0 interchangeably...

t3chguy reviewed Jul 13, 2018

View reviewed changes

krombel force-pushed the extend_server_notices_sendToAll branch 3 times, most recently from 37b755d to d1141bc Compare July 25, 2018 14:55

richvdh requested a review from a team August 10, 2018 22:06

krombel force-pushed the extend_server_notices_sendToAll branch from d1141bc to e9c2acd Compare August 10, 2018 22:53

richvdh changed the title ~~Adds admin API to send a server_notice to one/all users~~ Add admin API to send a server_notice to one/all users Aug 13, 2018

richvdh suggested changes Aug 13, 2018

View reviewed changes

krombel added 2 commits August 13, 2018 19:53

add admin_api

29ab2d7

krombel force-pushed the extend_server_notices_sendToAll branch 2 times, most recently from a926483 to 240c23e Compare August 13, 2018 19:12

update doc

a5dd1e9

krombel force-pushed the extend_server_notices_sendToAll branch from 240c23e to a5dd1e9 Compare August 13, 2018 19:16

richvdh reviewed Aug 13, 2018

View reviewed changes

richvdh closed this May 2, 2019

krombel deleted the extend_server_notices_sendToAll branch July 26, 2019 09:00



		class ServerNoticeRestServlet(ClientV1RestServlet):
		PATTERNS = client_path_patterns("/admin/send_server_notice(/(?P<user_id>[^/]+))?")

Uh oh!

Conversation

krombel commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matrixbot commented Jul 13, 2018

Uh oh!

matrixbot commented Jul 13, 2018

Uh oh!

richvdh commented Jul 13, 2018

Uh oh!

t3chguy commented Jul 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krombel Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh commented May 2, 2019

Uh oh!

eMPee584 commented May 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

krombel commented Jul 13, 2018 •

edited

Loading

krombel Jul 13, 2018 •

edited

Loading