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

feat: Un-encrypted messages not allowed in E2EE rooms #32040

Merged
merged 56 commits into from
May 24, 2024

Conversation

yash-rajpal
Copy link
Member

@yash-rajpal yash-rajpal commented Mar 20, 2024

Proposed changes (including videos or screenshots)

We allowed users to send un-encrypted messages in encrypted rooms if the users didn't had the E2EE setup or keys were missing.

Now we are introducing a setting which will not allow users to send un-encrypted messages in E2EE rooms. If the users don't have the E2EE setup or don't have room keys, it will show a new E2EE setup interface to users, using which users will be able to get a feedback regarding their E2EE status.

image

Also introduced the E2EE state to track in which state the E2EE functionality is in.

Issue(s)

Steps to test or reproduce

  • Enable the new setting under E2EE admin section, which won't allow users to end un-encrypted messages in encrypted rooms
  • Without your E2EE setup, go to any encrypted room to see the new flow.
  • The encrypted room will show all states feedback

Further comments

E2EE-3

Copy link

changeset-bot bot commented Mar 20, 2024

🦋 Changeset detected

Latest commit: eb685e0

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

This PR includes changesets to release 34 packages
Name Type
@rocket.chat/i18n Minor
@rocket.chat/meteor Minor
@rocket.chat/mock-providers Patch
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/ui-client Major
@rocket.chat/gazzodown Major
@rocket.chat/livechat Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-video-conf Major
@rocket.chat/uikit-playground Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/model-typings Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service 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/models 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

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 82.38994% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 56.27%. Comparing base (a565999) to head (eb685e0).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #32040      +/-   ##
===========================================
+ Coverage    56.23%   56.27%   +0.03%     
===========================================
  Files         2428     2434       +6     
  Lines        53560    53681     +121     
  Branches     11031    11045      +14     
===========================================
+ Hits         30122    30211      +89     
- Misses       20799    20832      +33     
+ Partials      2639     2638       -1     
Flag Coverage Δ
e2e 55.97% <82.38%> (+0.06%) ⬆️
unit 72.30% <ø> (-0.01%) ⬇️

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

Copy link
Contributor

dionisio-bot bot commented Apr 3, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is targeting the wrong base branch. It should target 6.10.0, but it targets 6.9.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@yash-rajpal yash-rajpal marked this pull request as ready for review April 19, 2024 16:47
@yash-rajpal yash-rajpal requested review from a team as code owners April 19, 2024 16:47
Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

I think we could improve a little bit the e2ee state hooks. I'm mostly thinking about not having both useE2EERoomState and useIsE2EEReady.

Also since e2e is base on emitters, we could also have a function that returns the subscribe and get functions to use with useSyncExternalStore.

Maybe let's have a chat sometime about all the E2EE feature/code?

apps/meteor/client/views/room/RoomBodyWithE2EESetup.tsx Outdated Show resolved Hide resolved
apps/meteor/client/views/room/RoomBodyWithE2EESetup.tsx Outdated Show resolved Hide resolved
apps/meteor/client/views/room/RoomE2EENotAllowed.tsx Outdated Show resolved Hide resolved
apps/meteor/client/views/room/hooks/useIsE2EEReady.ts Outdated Show resolved Hide resolved
apps/meteor/client/views/room/hooks/useE2EEState.ts Outdated Show resolved Hide resolved
apps/meteor/client/views/room/hooks/useE2EERoomState.ts Outdated Show resolved Hide resolved
apps/meteor/app/e2e/client/rocketchat.e2e.ts Show resolved Hide resolved
hugocostadev
hugocostadev previously approved these changes May 16, 2024
@yash-rajpal yash-rajpal dismissed stale reviews from hugocostadev and matheusbsilva137 via aa81aab May 17, 2024 14:11
@jessicaschelly jessicaschelly added the stat: QA assured Means it has been tested and approved by a company insider label May 20, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label May 20, 2024
hugocostadev
hugocostadev previously approved these changes May 21, 2024
@scuciatto scuciatto modified the milestones: 6.9, 6.10 May 21, 2024
Copy link
Member

@matheusbsilva137 matheusbsilva137 left a comment

Choose a reason for hiding this comment

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

Back-end LGTM

@kodiakhq kodiakhq bot merged commit 4fd9c4c into develop May 24, 2024
46 of 47 checks passed
@kodiakhq kodiakhq bot deleted the e2e-unencrypted-setting branch May 24, 2024 20:08
gabriellsh added a commit that referenced this pull request May 28, 2024
…retention

* 'develop' of github.com:RocketChat/Rocket.Chat: (36 commits)
  refactor: IntegrationHistory out of DB Watcher (#32502)
  fix: Message update being broadcasted without updated values (#32472)
  test: make api teams test fully independent (#31756)
  test: Fix test name (#32490)
  fix: streams being called with no logged user (#32489)
  feat: Un-encrypted messages not allowed in E2EE rooms (#32040)
  feat(UiKit): Users select (#31455)
  fix: Re-login same browser tab issues (#32479)
  chore: move all webclient code out of the COSS folders (#32273)
  chore(deps): bump thehanimo/pr-title-checker from 1.3.7 to 1.4.1 (#30619)
  fix: Don't show join default channels option for edit user form  (#31750)
  fix: CAS user merge not working (#32444)
  fix: Overriding Retention Policy not working (#32454)
  fix: `rooms.export` endpoint generates an empty export when given an invalid date (#32364)
  fix: "Allow Password Change for OAuth Users" setting is not honored in the "Forgot Password" flow (#32398)
  fix: Bypass trash when removing OTR system messages and read receipts (#32269)
  fix: Monitors dissapearing from Unit upon edit (#32393)
  fix: Link image preview not opening in gallery (#32391)
  feat: Allow visitors & integrations to access downloaded files after a room has closed (#32439)
  regression: Users tab misaligned (#32451)
  ...
matheusbsilva137 pushed a commit that referenced this pull request May 31, 2024
Co-authored-by: gabriellsh <40830821+gabriellsh@users.noreply.github.com>
Co-authored-by: Hugo Costa <20212776+hugocostadev@users.noreply.github.com>
This was referenced Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants