Skip to content

[FIX] Removed fields from User Info for which the user doesn't have permissions.#20923

Merged
ggazzo merged 6 commits intoRocketChat:developfrom
Darshilp326:Wrong-UserInfo
May 14, 2021
Merged

[FIX] Removed fields from User Info for which the user doesn't have permissions.#20923
ggazzo merged 6 commits intoRocketChat:developfrom
Darshilp326:Wrong-UserInfo

Conversation

@Darshilp326
Copy link
Contributor

@Darshilp326 Darshilp326 commented Feb 27, 2021

Proposed changes (including videos or screenshots)

Removed LastLogin, CreatedAt and Roles for users who don't have permission.

Correct-info-pr.mp4

Issue(s)

Closes #20138
Closes #20394

Steps to test or reproduce

Further comments

In video second user is an Admin.

@Darshilp326
Copy link
Contributor Author

@dougfabris Please review 😄

@dougfabris dougfabris changed the title [FIX]Removed fields from User Info for which the user doesn't have permissions. [FIX] Removed fields from User Info for which the user doesn't have permissions. Mar 4, 2021
@lgtm-com
Copy link

lgtm-com bot commented May 11, 2021

This pull request introduces 1 alert when merging d1292cc into 82243ad - view on LGTM.com

new alerts:

  • 1 for Useless conditional

@dougfabris dougfabris added the stat: ready to merge PR tested and approved waiting for merge label May 13, 2021
@dougfabris dougfabris requested a review from ggazzo May 13, 2021 15:13
@ggazzo ggazzo merged commit d12ce70 into RocketChat:develop May 14, 2021
@sampaiodiego sampaiodiego mentioned this pull request May 28, 2021
@suninuni
Copy link

const user = Users.findOneByIdOrUsername(filterId || filterUsername, options);
user.canViewAllInfo = canViewAllInfo;

This caused a unexpected error when user is not found. @Darshilp326 @dougfabris

{
    "success": false,
    "error": "Cannot set property 'canViewAllInfo' of undefined"
}

@dougfabris
Copy link
Member

const user = Users.findOneByIdOrUsername(filterId || filterUsername, options);
user.canViewAllInfo = canViewAllInfo;

This caused a unexpected error when user is not found. @Darshilp326 @dougfabris

{
    "success": false,
    "error": "Cannot set property 'canViewAllInfo' of undefined"
}

@suninuni The issue you mentioned is already fixed: #23372
Thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to list/get roles for other members as channel owner [BUG] Wrong information in user info page for non-admin users.

4 participants

Comments