REST API Add all users to room #3569
Conversation
Deepakkothandan
force-pushed
the
addAllUsersToRoom
branch
from
5242ee8
to
8d43ecf
Compare
|
@geekgonecrazy This PR adds a method which provides the functionality to add all existing users to a newly create Channel. At present, it is not possible to add all existing users to a newly created channel. Please review the PR and provide your suggestions |
|
@Deepakkothandan thanks for contributing! We'll take a look. At first glance though this makes me very nervous.... On our demo server we have 60k+ users. If we add all of those to a channel... That could potentially be a very costly operation. |
|
@geekgonecrazy Yeah I understand, that with a huge amount of users this could be expensive and that's the reason why only admins are allowed to do this. There are occasions, where one need to create a new channel with all existing user. Would be nice to have this feature |
|
@Deepakkothandan nvm. missed that it was REST-API. I would still suggest adding a group of Settings that limits (by default) the API call if the count of users are above a reasonable threshold. This will allow the admin (with GUI access) to explicitly override the system if necessary. And be somewhat proofed to DoS attacks via REST. |
|
@Sing-Li Thank you for your suggestions, Added a condition if the count exceeds 500, then the call would return a error. I believe its a reasonable amount. Please provide your thoughts. |
Deepakkothandan
force-pushed
the
addAllUsersToRoom
branch
from
4a48f6f
to
2123652
Compare
|
@RocketChat/core is this ready to merge? |
|
@Deepakkothandan really thank you for your contributions. LGTM |
|
@Deepakkothandan thanks for making those changes, and thanks again for the contribution. 😄 |
|
So, I see two problems here:
@Deepakkothandan can you please fix the issues above? Thank you again for the contribution =) |
|
Ah! I missed that. Yes for sure if you could address those. The method needs validation because it could potentially be called from the client by unauthorized user otherwise |
|
Thank you all for reviewing the PR. @sampaiodiego Updated the PR as per your suggestions. |
|
thanks @Deepakkothandan Maybe I wasn't clear enough, sorry about that 😞 But on the REST route you have a validation to see if the user is an admin.. this validation have to be done in the Meteor method instead of the REST endpoint, because the method can be called directly from the client and the REST endpoint also call it. |
|
@sampaiodiego Thank you for the clarification, I made the required changes. |
…nto develop * 'develop' of https://github.com/RocketChat/Rocket.Chat: (210 commits) Save room topic escaped Update references from ES2016 to ECMAScript 2015 REST API Add all users to room (RocketChat#3569) Use .eslintc file instead of .eslintrc.js Update settings.js Restore watermark Add eslint and ES2016 to contributing guide Fix eslint errors Fix Ubuntu link closes 3575 Update more links to current documentation Fixes RocketChat#3571: sort slash commands before filtering Rename settings group Lingohub (RocketChat#3562) Remove unused method User findOneByEmailAddress case insensitive fixes language_version on catalan translation fixes language_version on spanish translation version bump to 0.34.0 Add "How it all started" Fix issue with updateTimeout using rid out of context ... # Conflicts: # packages/rocketchat-theme/assets/stylesheets/base.less # packages/rocketchat-ui-login/login/form.coffee # packages/rocketchat-ui-sidenav/side-nav/sideNav.html
|
how is this progress going? Ability to add all existing users to a channel is a great feature, (many competitors are missing) and very needed for our group! |
@RocketChat/core
Rest API method to add all existing users to a Channel