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: Can't remove channel's password to join #31507

Merged

Conversation

Spiral-Memory
Copy link
Contributor

Proposed changes (including videos or screenshots)

The issue arises from a minor detail: when the password box is empty, the condition data.joinCode results in a falsy value. As a result, no key with the name joinCode is sent, preventing the password from unsetting. The backend, however, expects an empty string ('') to unset the password. To resolve this, a small fix was implemented. The getDirtyFields returns a key of joinCodeRequired when the password to access toggle changes. So, i used that property to create a new condition to check for the existence of this key. If it exists, an object named joinCode with an empty string value is created and sent to the backend to address the issue.

Issue(s)

Closes #31506

2024-01-21.21-28-26.mp4

Steps to test or reproduce

The steps to reproduce is mentioned in the issue I created, which is #31506.

Further comments

Just a very small but important fix.

@Spiral-Memory Spiral-Memory requested a review from a team as a code owner January 21, 2024 16:12
Copy link

changeset-bot bot commented Jan 21, 2024

🦋 Changeset detected

Latest commit: b1f03f7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/models Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dougfabris dougfabris changed the title chore: fixed channel password unset issue fix: Can't remove channel's password to join Jan 22, 2024
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

@Spiral-Memory Nice catch, thank you so much for helping us!

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Jan 22, 2024
@dougfabris dougfabris added this to the 6.6 milestone Jan 22, 2024
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Jan 22, 2024
Copy link

codecov bot commented Jan 22, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (4ff8bb9) 55.08% compared to head (b1f03f7) 53.24%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #31507      +/-   ##
===========================================
- Coverage    55.08%   53.24%   -1.84%     
===========================================
  Files         1537     1575      +38     
  Lines        30324    27063    -3261     
  Branches      6234     5769     -465     
===========================================
- Hits         16704    14411    -2293     
+ Misses       12332    11412     -920     
+ Partials      1288     1240      -48     
Flag Coverage Δ
e2e 53.22% <0.00%> (+7.92%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Jan 22, 2024
@kodiakhq kodiakhq bot merged commit 8aa1ac2 into RocketChat:develop Jan 22, 2024
42 of 43 checks passed
@Spiral-Memory
Copy link
Contributor Author

@dougfabris , Thank you so much for reviewing my changes and approving them. It's my very first PR in this repository to get merged, and I am really happy that I could be of some use in helping the application. I would like to request you to please review two more of my PRs: PR #1265 and PR #1262 in the Fuselage repository if possible. They were raised a week before, and I believe they might be helpful as well.

Additionally, I would like to report a security issue in the Rocket Chat application related with password protected channel only. However, I am uncertain whether to report it in the bug section or the security vulnerability section. Could you please guide me on this? Also, can I work on that issue?

gabriellsh added a commit that referenced this pull request Jan 23, 2024
* 'develop' of github.com:RocketChat/Rocket.Chat: (143 commits)
  fix: Marketplace apps installed as private showing as installed (#31514)
  test: fix messaging flaky test (#31512)
  fix: change the push sound sent when the push is from video conference (#30910)
  fix: ensure sessionId on Sessions documents (#31487)
  chore: handle rocket.cat creation and deletion (#31170)
  chore(agenda): Changes in Agenda API (#31427)
  refactor: Pay the debt of some TODO comments (#30338)
  feat: Room header keyboard navigability (#31516)
  fix: Cron job for clearing OEmbed cache isn't working (#31336)
  refactor(client): Handling of custom statuses (#31500)
  fix: Prevent `Fallback forward departments` feature to go on loop when configured as circular chain (#31328)
  feat: Add `push.test` endpoint (#31289)
  chore: split UserProvider into two: User and Authentication (#31513)
  fix: Can't remove channel's password to join (#31507)
  chore: convert CAS integration code to typescript (#31492)
  feat: Composer keyboard navigability (#31510)
  fix: Render warning in Logs - (MessageType.render is deprecated. Use MessageType.message instead) (#31426)
  feat: Convert emoji shortname to unicode on notification emails. (#31225)
  chore: Normalize `ButtonGroup` (#31499)
  chore: improve type definitions for login service configurations (#31491)
  ...
@dougfabris
Copy link
Member

@Spiral-Memory Sure, I will take a look as soon as possible.
For vulnerability and security issues I would recommend you use our Hackerone page

@Spiral-Memory
Copy link
Contributor Author

Spiral-Memory commented Jan 26, 2024

@Spiral-Memory Sure, I will take a look as soon as possible.
For vulnerability and security issues I would recommend you use our Hackerone page

Thank you for your reply. Sir, I have reported the security issue on GitHub in the security vulnerability section. You can have a look; it's still in the triage phase. Also, HackerOne is not accepting submissions, I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Unable to remove the channel password once it has been set.
2 participants