-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] Delete user without username was removing direct rooms of all users #9986
Conversation
packages/rocketchat-cors/cors.js
Outdated
import tls from 'tls'; | ||
// FIX For TLS error see more here https://github.com/RocketChat/Rocket.Chat/issues/9316 | ||
// TODO: Remove after NodeJS fix it, more information https://github.com/nodejs/node/issues/16196 https://github.com/nodejs/node/pull/16853 | ||
tls.DEFAULT_ECDH_CURVE = 'auto'; | ||
|
||
// Revert change from Meteor 1.6.1 who set ignoreUndefined: true | ||
// more informatin https://github.com/meteor/meteor/pull/9444 | ||
Mongo.setConnectionOptions({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I ask why this is being done in the CORS package? That's my only conundrum with this..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only guess is because it is the first one to be loaded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (room) { | ||
if (room.t !== 'c' && room.usernames.length === 1) { | ||
RocketChat.models.Rooms.removeById(subscription.rid); // Remove non-channel rooms with only 1 user (the one being deleted) | ||
// Users without username can't do anything, so there are nothing to remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar correction. "..so there is nothing to remove" 😉
a560e31
to
1ac5052
Compare
1ac5052
to
ca487a4
Compare
based on @graywolf336 's comment my suggestion is renaming the CORS package to something like |
…sername [FIX] Delete user without username was removing direct rooms of all users
* master: Revise test job, fix cache path, fix artifact path, update README Trying to fix memory issue during meteor build Add S3 upload script Remove root-priviledged command switch gitlab ci config to use shell executor Try another build Fix another stupid docker image reference try to fix a build error with katex Fix root permission for meteor command everywhere Try to fix build with gitlab-ci Add --allow-superuser to fix meteor build error on root permission First try with a gitlab-ci build Bump version to 0.62.1 Merge pull request RocketChat#9986 from RocketChat/hotfix/user-delete-without-username Merge pull request RocketChat#9988 from RocketChat/fix-new-channel Merge pull request RocketChat#9960 from RocketChat/hotfix-subscription-without-room-ui Merge pull request RocketChat#9982 from RocketChat/fix-two-factor-login
Meteor 1.6.1 introduced a new default setting to MongoDB driver
ignoreUndefined: true
which can remove fields withundefined
values from the query, that execute a different behavior than expected when deleting users without usernames executing the deletion agains all direct rooms with the query:rather than
This PR rollback that MongoDB option and does not tries to remove data for users without username cuz they can't have message, subscriptions, rooms, etc before set their username.