Skip to content
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

Merged
merged 1 commit into from Mar 3, 2018

Conversation

Projects
None yet
4 participants
@rodrigok
Copy link
Member

commented Mar 2, 2018

Meteor 1.6.1 introduced a new default setting to MongoDB driver ignoreUndefined: true which can remove fields with undefined 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:

{
  t: 'd'
}

rather than

{
  t: 'd',
  usernames: undefined
}

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.

@rodrigok rodrigok added this to the 0.62.1 milestone Mar 2, 2018

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-9986 Mar 2, 2018 Inactive

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({

This comment has been minimized.

Copy link
@graywolf336

graywolf336 Mar 2, 2018

Member

May I ask why this is being done in the CORS package? That's my only conundrum with this..

This comment has been minimized.

Copy link
@sampaiodiego

sampaiodiego Mar 3, 2018

Member

My only guess is because it is the first one to be loaded

This comment has been minimized.

Copy link
@sampaiodiego
packages/rocketchat-lib/server/functions/deleteUser.js Outdated
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

This comment has been minimized.

Copy link
@graywolf336

graywolf336 Mar 2, 2018

Member

Grammar correction. "..so there is nothing to remove" 😉

@sampaiodiego sampaiodiego force-pushed the hotfix/user-delete-without-username branch Mar 3, 2018

@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-9986 Mar 3, 2018 Inactive

@sampaiodiego

This comment has been minimized.

Copy link
Member

commented Mar 3, 2018

based on @graywolf336 's comment my suggestion is renaming the CORS package to something like rocketchat:init, so it will make more sense to keep it as first and also because it already does not do only CORS related stuff .. what do you guys think??

@sampaiodiego sampaiodiego merged commit 784436d into develop Mar 3, 2018

4 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test-with-oplog Your tests passed on CircleCI!
Details
ci/circleci: test-without-oplog Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@sampaiodiego sampaiodiego deleted the hotfix/user-delete-without-username branch Mar 3, 2018

sampaiodiego added a commit that referenced this pull request Mar 3, 2018

Merge pull request #9986 from RocketChat/hotfix/user-delete-without-u…
…sername

[FIX] Delete user without username was removing direct rooms of all users

trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Apr 9, 2018

Merge branch 'master' into goalify
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.