🔒️ (security) implement hash mode encryption#1337
Open
sneko wants to merge 6 commits into
Open
Conversation
Collaborator
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR implements a comprehensive end-to-end encryption system for meetings. On the backend, it introduces an EncryptionMode enum, adds encryption_mode and default_encryption_mode fields to Room and User models respectively, extends API serializers with encryption validation (preventing mode changes post-creation and enforcing RESTRICTED access), and modifies LiveKit token generation to include encrypted participant metadata and restrict email visibility. The lobby service tracks participant authentication state and constrains usernames in encrypted rooms to authenticated users' display names. On the frontend, it adds passphrase generation and validation utilities, implements E2EE UI components (DecryptionFailedTileOverlay, EncryptionMismatchScreen, FeaturePill, RoomStatusBanner), converts the join dialog to a two-step flow with passphrase entry, integrates LiveKit's ExternalE2EEKeyProvider into the Conference component with connection gating, disables room tools (recording, transcription) in encrypted rooms, adds a Security settings tab for default encryption preferences, and updates participant displays to show anonymous/authenticated badges. Comprehensive English and French localization covers all new UI strings. Infrastructure changes include Keycloak encryption service client configuration, vitest setup, and OIDC user info field mapping updates. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches⚔️ Resolve merge conflicts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/backend/core/tests/rooms/test_api_rooms_retrieve.py (1)
269-277:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid hard-coding
encryption_mode="none"in token call assertionsThese assertions currently depend on
RoomFactorydefaults rather than the room under test. Please assertencryption_mode=room.encryption_mode(or setencryption_mode="none"explicitly on eachRoomFactory) to keep tests stable and intent-driven.Suggested patch
- encryption_mode="none", + encryption_mode=room.encryption_mode,Also applies to: 322-331, 411-420, 508-517
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/core/tests/rooms/test_api_rooms_retrieve.py` around lines 269 - 277, The test currently hardcodes encryption_mode="none" in the mock_token.assert_called_once_with expectations, which ties the test to RoomFactory defaults; update each assertion (e.g. the mock_token.assert_called_once_with calls at the locations around the tests that reference mock_token) to use encryption_mode=room.encryption_mode (or alternatively create the RoomFactory with encryption_mode="none" explicitly) so the assertion reflects the actual room under test and keeps the test stable; change all occurrences referenced in the review (around lines 269-277, 322-331, 411-420, 508-517) to reference the room variable's encryption_mode instead of the hard-coded string.src/backend/core/tests/services/test_lobby.py (1)
322-347:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet an explicit auth state on
request.userin this test
request.useris not initialized here, sorequest.user.is_authenticatedis a dynamicMock, not a boolean. This can mask type/behavior regressions inLobbyService.request_entry. Setrequest.user.is_authenticatedexplicitly and assert againstTrue/False.Suggested patch
def test_request_entry_new_participant( mock_enter, lobby_service, participant_id, username ): """Test requesting entry for a new participant.""" request = mock.Mock() + request.user = mock.Mock() + request.user.is_authenticated = False request.COOKIES = {settings.LOBBY_COOKIE_NAME: participant_id} @@ mock_enter.assert_called_once_with( room.id, participant_id, username, - is_authenticated=request.user.is_authenticated, + is_authenticated=False, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/core/tests/services/test_lobby.py` around lines 322 - 347, The test leaves request.user.is_authenticated as a Mock which can mask regressions; set request.user explicitly to have a concrete boolean is_authenticated (e.g. request.user = mock.Mock(is_authenticated=True) or False) before calling lobby_service.request_entry and update the mock_enter assert to pass that explicit boolean (replace is_authenticated=request.user.is_authenticated with the concrete True/False) so the test verifies LobbyService.request_entry receives a real boolean; reference the request object, request.user.is_authenticated and lobby_service.request_entry/mock_enter in your change.src/backend/core/services/lobby.py (1)
193-205:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh the cached lobby identity before returning.
Lines 193-205 only touch the TTL or mint a token. If the same browser first queued anonymously and then retries authenticated, the waiting entry keeps the old
username/is_authenticatedwhile the accepted join uses the new server-forced name. That lets an admin approve one identity and admit another.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/core/services/lobby.py` around lines 193 - 205, The lobby allows a stale cached identity to persist when a participant moves from WAITING to ACCEPTED, so before returning the accepted join path (the branch using LobbyParticipantStatus.ACCEPTED and calling utils.generate_livekit_config), refresh the cached/waiting identity for this participant to pick up the server-forced username/is_authenticated; call the same refresh logic (e.g., refresh_waiting_status with room.id and participant_id) or update the cached participant record (the participant object used to build livekit_config) so the livekit token and returned identity reflect the latest server state rather than the old queued anonymous values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/auth/realm.json`:
- Around line 849-865: The public client "encryption" is missing PKCE
enforcement and allows full scope; update the client object for clientId
"encryption" to set "fullScopeAllowed": false, add a minimal explicit scope list
via "defaultClientScopes" (match the minimal scopes used by "account-console" or
list only required scopes), and enforce PKCE by adding the attributes that
require S256 PKCE (e.g. include "attributes":
{"pkce.code.challenge.method":"S256", "pkce.required":"true"}), keeping
"publicClient" and "standardFlowEnabled" as appropriate.
In `@README.md`:
- Line 120: The table row currently shows a mismatch between the link text and
the target URL: the markdown link "[mosacloud.cloud](https://mosa.cloud/)"
displays mosacloud.cloud but points to https://mosa.cloud/; update the markdown
so the display label and the href match (either change the link target to
https://mosacloud.cloud/ or change the display text to mosa.cloud) for the table
entry to keep the host and URL consistent.
- Line 63: Update the README's enum description for Room.encryption_mode to
match the API (replace the outdated `basic` value with `hash`), ensuring all
occurrences (the enum list, explanatory sentences, and any examples) now state
`none` / `hash`; keep the rest of the paragraph (behavior when encrypted, error
messages like "Recording is unavailable in encrypted rooms.", More-tools UI
text, SIP gateway behavior, and force-locked `restricted` access level)
unchanged so docs remain consistent with the implemented contract.
In `@src/backend/core/api/serializers.py`:
- Around line 33-41: The serializer must reject non-null/default_encryption_mode
when ENCRYPTION_ENABLED is False so the API cannot persist an encryption default
while the server disables encryption; add validation to the user serializer
(e.g., a validate_default_encryption_mode method or in validate()) that checks
django settings.ENCRYPTION_ENABLED and, if False and the incoming
default_encryption_mode is not None (or not "none"/empty), raise
serializers.ValidationError with a clear message; reference the
default_encryption_mode field in this serializer and keep
RoomViewSet.perform_create behaviour unchanged.
In `@src/backend/core/models.py`:
- Around line 411-419: The model must enforce the invariants currently only in
RoomSerializer: make Room.validate/clean (or override save to call full_clean)
to (1) prevent changing encryption_mode after creation (detect existing instance
by pk and raise ValidationError if encryption_mode differs from DB value), and
(2) enforce that when encryption_mode == EncryptionMode.RESTRICTED the
access_level is/always remains AccessLevel.RESTRICTED (and reject any change
that would set access_level to OPEN), and (3) block creating a RESTRICTED
encryption_mode with a non-RESTRICTED access_level; ensure these checks live in
Room.clean() (and call full_clean from save) so ORM/admin writes are covered,
using the model fields encryption_mode and access_level and raising
django.core.exceptions.ValidationError on violation.
In `@src/backend/core/services/lobby.py`:
- Around line 151-152: The branch that handles encrypted rooms still falls back
to the client-provided username (variable username) when request.user.full_name
and request.user.email are missing, reopening an impersonation vector; change
the logic in the room.is_encrypted && request.user.is_authenticated path to
never use the incoming client username and instead set a server-controlled
fallback (e.g., derive from the authenticated user id or a server-generated
token—use request.user.id or a deterministic "user-{id}" or "guest-{id}"
pattern) so that username is always trusted when set in this branch (update the
assignment where username = request.user.full_name or request.user.email or
username).
In `@src/backend/meet/settings.py`:
- Around line 563-570: The defaults for OIDC claim mappings were changed to
provider-specific names and must be restored to OIDC standard claims: update
OIDC_USERINFO_FULLNAME_FIELDS to default to ["given_name", "family_name"] and
OIDC_USERINFO_SHORTNAME_FIELD to default to "given_name" (keeping
environ_name/environ_prefix behavior unchanged) so existing deployments that
rely on standard OIDC userinfo claims will continue to populate full_name and
short_name correctly.
In `@src/frontend/src/features/rooms/components/Conference.tsx`:
- Around line 123-152: The E2EE Worker created by getWorker() (stored in
workerRef) is never terminated, leaking a Worker each mount; add cleanup to
terminate and clear workerRef when the component unmounts (and optionally when
encryption is disabled or room changes): implement a useEffect that watches
lifecycle (or room/isEncrypted) and in its cleanup calls
workerRef.current?.terminate() and sets workerRef.current = null to release the
worker; ensure the same pattern is used for keyProviderRef if needed.
- Around line 106-121: The effect that installs the hashchange listener is
currently gated by isEncrypted which prevents installation when the room data
exists but the fragment/hash is missing or invalid; change the guard to if
(!data) return so the listener is installed whenever room data is loaded, update
the effect dependency array to [data] (not [isEncrypted]) so it re-runs when
room payload arrives, and ensure the hashchange handler (the listener you
register in this effect) still validates the fragment, updates whatever signals
compute hasValidHash/encryptionMismatch, and is properly cleaned up on unmount.
- Around line 211-218: The warm-up call uses the raw apiConfig.livekit.url
instead of the normalized serverUrl used by LiveKitRoom, causing failures when
apiConfig.livekit.force_wss_protocol rewrites https:// to wss://; change the
warm-up invocation to use the computed serverUrl (from the useMemo with name
serverUrl) when calling room.prepareConnection so it receives the same
normalized URL that LiveKitRoom/connect will use, ensuring prepareConnection is
passed serverUrl (or falls back consistently) and keeping isConnectionWarmedUp
gating semantics intact.
In `@src/frontend/src/features/rooms/components/Join.tsx`:
- Around line 128-135: The Join component reads the URL fragment once via
getPassphraseFromHash() and computes passphrase, hasValidPassphrase and
unexpectedPassphrase during render, but never listens for hash updates; make the
passphrase reactive by moving the passphrase into component state (e.g., a
passphrase state variable) and subscribe to window "hashchange" in a useEffect
inside Join to update that state when the fragment changes; recompute
hasValidPassphrase and unexpectedPassphrase from that state so the UI updates
immediately when the user edits the URL fragment; remember to clean up the
hashchange listener in the effect cleanup.
In `@src/frontend/src/features/rooms/hooks/useLobby.ts`:
- Around line 59-62: The startWaiting callback is marked async but contains no
awaits, causing it to return Promise<void> instead of void and mismatching
callers (e.g., Join.tsx); remove the async keyword from the startWaiting
definition (which sets setStatus(ApiLobbyStatus.WAITING) and calls
startWaitingTimeout()) so it returns void and keep the
useCallback([...startWaitingTimeout]) wrapper unchanged.
In `@src/frontend/src/features/rooms/livekit/components/Admin.tsx`:
- Around line 170-261: The component currently derives isEncrypted from an
optional readOnlyData prop which may be absent on first render; change it to use
a guaranteed source (e.g., call/useRoomData() or enable the fetch that provides
room data) so isEncrypted is computed from always-present room data, and gate
the radio group and forced ApiAccessLevel.RESTRICTED value from that source
instead of readOnlyData; update the logic around isDisabled, value,
aria-disabled and the displayed banner to use the new hook (or enabled fetch) so
the banner and disabled state never briefly allow editing when the room is
encrypted (references: readOnlyData, isEncrypted, useRoomData(), Field,
ApiAccessLevel, patchRoom, queryClient, keys.room, roomId).
In
`@src/frontend/src/features/rooms/livekit/components/controls/Options/ScreenRecordingMenuItem.tsx`:
- Line 15: Replace the hard-coded string check that sets isEncrypted (const
isEncrypted = roomData?.encryption_mode === 'basic') with a robust, shared check
using the project's encryption enum or helper (e.g., EncryptionMode enum or an
isRoomEncrypted(roomData) utility); update all related gating logic in
ScreenRecordingMenuItem (and the block around lines 27-32) to call that helper
or compare against the enum constant instead of the literal 'basic' so changes
to API values or new modes (hash-mode, etc.) are handled centrally.
In
`@src/frontend/src/features/rooms/livekit/components/controls/Options/TranscriptMenuItem.tsx`:
- Line 14: The component currently uses a raw string check const isEncrypted =
roomData?.encryption_mode === 'basic' (and similar checks around lines 26-31);
replace these literal comparisons with the shared encryption-mode enum or helper
(e.g., use the existing EncryptionMode enum or an isEncryptedRoom(roomData)
helper) so the logic uses the central source of truth for encrypted modes;
update TranscriptMenuItem.tsx to import and call that enum/helper where
isEncrypted and any subsequent conditionals reference roomData.encryption_mode
so future mode additions still disable transcripts correctly.
In
`@src/frontend/src/features/rooms/livekit/components/controls/Participants/ParticipantListItem.tsx`:
- Around line 107-111: The component currently reads sensitive email from
participant.attributes and only hides it in the UI via isLoggedIn
(ParticipantListItem), but attributes are broadcast to all peers; remove
reliance on participant.attributes.email in ParticipantListItem and stop
displaying or reading it directly from LiveKit attributes. Instead, return
undefined for the email when constructing the UI and add a secure call to an
authenticated backend endpoint (e.g., a new getUserEmail/getParticipantEmail API
used by ParticipantListItem or a surrounding hook) to fetch the email only for
authenticated users; update ParticipantListItem to use that API/hook and keep
the isLoggedIn check before calling it so emails are fetched and displayed only
for authenticated sessions.
In `@src/frontend/src/features/rooms/utils/isRoomValid.ts`:
- Around line 10-16: isRoomValid currently allows URLs that start with a valid
room URL but have extra path segments (e.g., /room-id/extra); update the URL
test to anchor the roomIdPattern so only optional query/hash suffixes are
permitted. In isRoomValid, modify the RegExp that uses window.location.origin
and roomIdPattern to require end-of-path after the room id and allow only an
optional query or hash (e.g., append a non-capturing group like (?:[?#].*)?$ to
the pattern) so the test for full URLs rejects trailing path segments; keep the
other checks (roomRegex, roomWithoutHyphensRegex) unchanged.
In `@src/frontend/src/features/settings/components/EncryptionDefaultField.tsx`:
- Around line 24-43: The mutation call in handleToggle uses mutateAsync without
handling rejections; update the useMutation call that references
updateUserPreferences to provide an onError handler (e.g., onError: (err,
variables, context) => { /* revert UI or notify */ }) or change handleToggle to
call mutate instead of mutateAsync so errors are handled via the mutation's
callbacks; modify the useMutation options around mutateAsync/mutate (symbols:
useMutation, updateUserPreferences, mutateAsync, mutate, onError, handleToggle)
to ensure failures are caught and the UI/query state is reverted or an error is
surfaced.
In `@src/frontend/src/primitives/Checkbox.tsx`:
- Around line 138-140: The render is calling props.validate and calling setError
(using error) directly which can trigger infinite re-renders; move this
validation out of the render path by running it inside a useEffect that depends
on renderProps.isSelected and renderProps.isInvalid (and props.validate if
needed) or by deriving the error synchronously from props without calling
setError in render; update the Checkbox component to compute next =
props.validate(renderProps.isSelected) inside that effect and only call
setError(next) if next !== error (or eliminate local error state and use a
derivedError variable) to prevent state updates during render.
---
Outside diff comments:
In `@src/backend/core/services/lobby.py`:
- Around line 193-205: The lobby allows a stale cached identity to persist when
a participant moves from WAITING to ACCEPTED, so before returning the accepted
join path (the branch using LobbyParticipantStatus.ACCEPTED and calling
utils.generate_livekit_config), refresh the cached/waiting identity for this
participant to pick up the server-forced username/is_authenticated; call the
same refresh logic (e.g., refresh_waiting_status with room.id and
participant_id) or update the cached participant record (the participant object
used to build livekit_config) so the livekit token and returned identity reflect
the latest server state rather than the old queued anonymous values.
In `@src/backend/core/tests/rooms/test_api_rooms_retrieve.py`:
- Around line 269-277: The test currently hardcodes encryption_mode="none" in
the mock_token.assert_called_once_with expectations, which ties the test to
RoomFactory defaults; update each assertion (e.g. the
mock_token.assert_called_once_with calls at the locations around the tests that
reference mock_token) to use encryption_mode=room.encryption_mode (or
alternatively create the RoomFactory with encryption_mode="none" explicitly) so
the assertion reflects the actual room under test and keeps the test stable;
change all occurrences referenced in the review (around lines 269-277, 322-331,
411-420, 508-517) to reference the room variable's encryption_mode instead of
the hard-coded string.
In `@src/backend/core/tests/services/test_lobby.py`:
- Around line 322-347: The test leaves request.user.is_authenticated as a Mock
which can mask regressions; set request.user explicitly to have a concrete
boolean is_authenticated (e.g. request.user = mock.Mock(is_authenticated=True)
or False) before calling lobby_service.request_entry and update the mock_enter
assert to pass that explicit boolean (replace
is_authenticated=request.user.is_authenticated with the concrete True/False) so
the test verifies LobbyService.request_entry receives a real boolean; reference
the request object, request.user.is_authenticated and
lobby_service.request_entry/mock_enter in your change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c91a5f1-5d2c-4915-96d2-2df2f8e6ff07
⛔ Files ignored due to path filters (1)
src/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (78)
CHANGELOG.mdREADME.mddocker/auth/realm.jsonsrc/backend/core/api/__init__.pysrc/backend/core/api/serializers.pysrc/backend/core/api/viewsets.pysrc/backend/core/migrations/0019_room_encryption_mode_user_default_encryption_mode.pysrc/backend/core/models.pysrc/backend/core/services/livekit_events.pysrc/backend/core/services/lobby.pysrc/backend/core/tests/rooms/test_api_rooms_lobby.pysrc/backend/core/tests/rooms/test_api_rooms_retrieve.pysrc/backend/core/tests/services/test_lobby.pysrc/backend/core/tests/test_api_users.pysrc/backend/core/utils.pysrc/backend/meet/settings.pysrc/frontend/package.jsonsrc/frontend/src/api/useConfig.tssrc/frontend/src/components/Avatar.tsxsrc/frontend/src/features/auth/api/ApiUser.tssrc/frontend/src/features/auth/api/updateUserPreferences.tssrc/frontend/src/features/encryption/DecryptionFailedTileOverlay.tsxsrc/frontend/src/features/encryption/EncryptionMismatchScreen.tsxsrc/frontend/src/features/encryption/FeaturePill.tsxsrc/frontend/src/features/encryption/RoomStatusBanner.tsxsrc/frontend/src/features/encryption/index.tssrc/frontend/src/features/encryption/passphrase.tssrc/frontend/src/features/home/components/ConnectionDetailsDialog.tsxsrc/frontend/src/features/home/components/CreateEncryptedMeetingDialog.tsxsrc/frontend/src/features/home/components/JoinMeetingDialog.tsxsrc/frontend/src/features/home/components/LaterMeetingDialog.tsxsrc/frontend/src/features/home/routes/Home.tsxsrc/frontend/src/features/notifications/components/WaitingParticipantNotification.tsxsrc/frontend/src/features/recording/components/RecordingProvider.tsxsrc/frontend/src/features/rooms/api/ApiRoom.tssrc/frontend/src/features/rooms/api/createRoom.tssrc/frontend/src/features/rooms/api/listWaitingParticipants.tssrc/frontend/src/features/rooms/components/Conference.tsxsrc/frontend/src/features/rooms/components/InviteDialog.tsxsrc/frontend/src/features/rooms/components/Join.tsxsrc/frontend/src/features/rooms/hooks/useLobby.tssrc/frontend/src/features/rooms/hooks/useWaitingParticipants.tssrc/frontend/src/features/rooms/livekit/components/Admin.tsxsrc/frontend/src/features/rooms/livekit/components/Info.tsxsrc/frontend/src/features/rooms/livekit/components/ParticipantTile.tsxsrc/frontend/src/features/rooms/livekit/components/Tools.tsxsrc/frontend/src/features/rooms/livekit/components/controls/Options/ScreenRecordingMenuItem.tsxsrc/frontend/src/features/rooms/livekit/components/controls/Options/TranscriptMenuItem.tsxsrc/frontend/src/features/rooms/livekit/components/controls/Participants/ParticipantListItem.tsxsrc/frontend/src/features/rooms/livekit/components/controls/Participants/WaitingParticipantListItem.tsxsrc/frontend/src/features/rooms/livekit/hooks/useCopyRoomToClipboard.tssrc/frontend/src/features/rooms/livekit/prefabs/VideoConference.tsxsrc/frontend/src/features/rooms/utils/isRoomValid.tssrc/frontend/src/features/sdk/routes/CreateMeetingButton.tsxsrc/frontend/src/features/sdk/routes/CreatePopup.tsxsrc/frontend/src/features/sdk/utils/PopupManager.tssrc/frontend/src/features/sdk/utils/PopupWindow.tssrc/frontend/src/features/sdk/utils/types.tssrc/frontend/src/features/settings/components/EncryptionDefaultField.tsxsrc/frontend/src/features/settings/components/SettingsDialog.tsxsrc/frontend/src/features/settings/components/SettingsDialogExtended.tsxsrc/frontend/src/features/settings/components/tabs/SecurityTab.tsxsrc/frontend/src/features/settings/type.tssrc/frontend/src/layout/Header.tsxsrc/frontend/src/locales/en/home.jsonsrc/frontend/src/locales/en/rooms.jsonsrc/frontend/src/locales/en/sdk.jsonsrc/frontend/src/locales/en/settings.jsonsrc/frontend/src/locales/fr/home.jsonsrc/frontend/src/locales/fr/rooms.jsonsrc/frontend/src/locales/fr/sdk.jsonsrc/frontend/src/locales/fr/settings.jsonsrc/frontend/src/navigation/navigateTo.tssrc/frontend/src/primitives/Checkbox.tsxsrc/frontend/src/primitives/Field.tsxsrc/frontend/src/primitives/Radio.tsxsrc/frontend/src/primitives/TooltipWrapper.tsxsrc/frontend/vite.config.ts
Comment on lines
849
to
865
| { | ||
| "clientId": "encryption", | ||
| "name": "Encryption Service", | ||
| "enabled": true, | ||
| "publicClient": true, | ||
| "standardFlowEnabled": true, | ||
| "directAccessGrantsEnabled": false, | ||
| "redirectUris": [ | ||
| "http://encryption.localhost:7200/auth/callback" | ||
| ], | ||
| "webOrigins": [ | ||
| "http://encryption.localhost:7200", | ||
| "http://data.encryption.localhost:7200" | ||
| ], | ||
| "protocol": "openid-connect", | ||
| "fullScopeAllowed": true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
from pathlib import Path
realm = json.loads(Path("docker/auth/realm.json").read_text())
for client in realm["clients"]:
if client.get("clientId") in {"encryption", "account-console"}:
attrs = client.get("attributes") or {}
print(client["clientId"])
print(" publicClient:", client.get("publicClient"))
print(" standardFlowEnabled:", client.get("standardFlowEnabled"))
print(" fullScopeAllowed:", client.get("fullScopeAllowed"))
print(" pkce.code.challenge.method:", attrs.get("pkce.code.challenge.method"))
print()
PYRepository: suitenumerique/meet
Length of output: 311
Require PKCE and narrow scopes on the public encryption client.
This client is configured as publicClient: true with standard authorization-code flow enabled (standardFlowEnabled: true), but it lacks PKCE protection and leaves fullScopeAllowed enabled. Compared to the account-console client in the same realm, which requires S256 PKCE and disables full scope access, the encryption client accepts overly broad token scope without additional proof of authorization.
Suggested hardening
{
"clientId": "encryption",
"name": "Encryption Service",
"enabled": true,
"publicClient": true,
"standardFlowEnabled": true,
"directAccessGrantsEnabled": false,
+ "attributes": {
+ "pkce.code.challenge.method": "S256"
+ },
"redirectUris": [
"http://encryption.localhost:7200/auth/callback"
],
"webOrigins": [
"http://encryption.localhost:7200",
"http://data.encryption.localhost:7200"
],
"protocol": "openid-connect",
- "fullScopeAllowed": true
+ "fullScopeAllowed": false
}Set fullScopeAllowed to false and define explicit scopes as needed, matching the security posture of account-console in the same realm export.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "clientId": "encryption", | |
| "name": "Encryption Service", | |
| "enabled": true, | |
| "publicClient": true, | |
| "standardFlowEnabled": true, | |
| "directAccessGrantsEnabled": false, | |
| "redirectUris": [ | |
| "http://encryption.localhost:7200/auth/callback" | |
| ], | |
| "webOrigins": [ | |
| "http://encryption.localhost:7200", | |
| "http://data.encryption.localhost:7200" | |
| ], | |
| "protocol": "openid-connect", | |
| "fullScopeAllowed": true | |
| } | |
| { | |
| "clientId": "encryption", | |
| "name": "Encryption Service", | |
| "enabled": true, | |
| "publicClient": true, | |
| "standardFlowEnabled": true, | |
| "directAccessGrantsEnabled": false, | |
| "attributes": { | |
| "pkce.code.challenge.method": "S256" | |
| }, | |
| "redirectUris": [ | |
| "http://encryption.localhost:7200/auth/callback" | |
| ], | |
| "webOrigins": [ | |
| "http://encryption.localhost:7200", | |
| "http://data.encryption.localhost:7200" | |
| ], | |
| "protocol": "openid-connect", | |
| "fullScopeAllowed": false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/auth/realm.json` around lines 849 - 865, The public client
"encryption" is missing PKCE enforcement and allows full scope; update the
client object for clientId "encryption" to set "fullScopeAllowed": false, add a
minimal explicit scope list via "defaultClientScopes" (match the minimal scopes
used by "account-console" or list only required scopes), and enforce PKCE by
adding the attributes that require S256 PKCE (e.g. include "attributes":
{"pkce.code.challenge.method":"S256", "pkce.required":"true"}), keeping
"publicClient" and "standardFlowEnabled" as appropriate.
|
|
||
| #### Encryption mode is set at creation, immutable after | ||
|
|
||
| `Room.encryption_mode` is a string enum (`none` / `basic`) chosen when the room is created and never mutated afterwards — changing it would change the link's semantics, since the passphrase lives in the URL hash. There is no mid-call "pause encryption" mechanism: while a meeting is encrypted, **recording and transcription endpoints reject requests with a 400** (`Recording is unavailable in encrypted rooms.` / `Subtitles are unavailable in encrypted rooms.`), the More-tools panel renders those items disabled with an explanatory banner, and the SIP gateway never gets a dispatch rule for encrypted rooms (so dial-in numbers and PINs aren't allocated). Encrypted rooms are also force-locked to `restricted` access level (lobby admission), since basic E2EE only meaningfully protects against passive eavesdropping if the host vets joiners before they receive the in-URL key. |
There was a problem hiding this comment.
Enum value in docs appears outdated (basic vs hash).
The README documents Room.encryption_mode as none/basic, but this PR scope states the retained distinct mode is hash. Please align the documented enum values with the implemented API contract to avoid misconfiguration.
Suggested doc fix
-`Room.encryption_mode` is a string enum (`none` / `basic`) chosen when the room is created and never mutated afterwards
+`Room.encryption_mode` is a string enum (`none` / `hash`) chosen when the room is created and never mutated afterwards📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| `Room.encryption_mode` is a string enum (`none` / `basic`) chosen when the room is created and never mutated afterwards — changing it would change the link's semantics, since the passphrase lives in the URL hash. There is no mid-call "pause encryption" mechanism: while a meeting is encrypted, **recording and transcription endpoints reject requests with a 400** (`Recording is unavailable in encrypted rooms.` / `Subtitles are unavailable in encrypted rooms.`), the More-tools panel renders those items disabled with an explanatory banner, and the SIP gateway never gets a dispatch rule for encrypted rooms (so dial-in numbers and PINs aren't allocated). Encrypted rooms are also force-locked to `restricted` access level (lobby admission), since basic E2EE only meaningfully protects against passive eavesdropping if the host vets joiners before they receive the in-URL key. | |
| `Room.encryption_mode` is a string enum (`none` / `hash`) chosen when the room is created and never mutated afterwards — changing it would change the link's semantics, since the passphrase lives in the URL hash. There is no mid-call "pause encryption" mechanism: while a meeting is encrypted, **recording and transcription endpoints reject requests with a 400** (`Recording is unavailable in encrypted rooms.` / `Subtitles are unavailable in encrypted rooms.`), the More-tools panel renders those items disabled with an explanatory banner, and the SIP gateway never gets a dispatch rule for encrypted rooms (so dial-in numbers and PINs aren't allocated). Encrypted rooms are also force-locked to `restricted` access level (lobby admission), since basic E2EE only meaningfully protects against passive eavesdropping if the host vets joiners before they receive the in-URL key. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 63, Update the README's enum description for
Room.encryption_mode to match the API (replace the outdated `basic` value with
`hash`), ensuring all occurrences (the enum list, explanatory sentences, and any
examples) now state `none` / `hash`; keep the rest of the paragraph (behavior
when encrypted, error messages like "Recording is unavailable in encrypted
rooms.", More-tools UI text, SIP gateway behavior, and force-locked `restricted`
access level) unchanged so docs remain consistent with the implemented contract.
| | [visio.numerique.gouv.fr](https://visio.numerique.gouv.fr/) | DINUM | French public agents working for the central administration and the extended public sphere. ProConnect is required to login in or sign up | | ||
| | [visio.suite.anct.gouv.fr](https://visio.suite.anct.gouv.fr/) | ANCT | French public agents working for the territorial administration and the extended public sphere. ProConnect is required to login in or sign up | | ||
| | [visio.lasuite.coop](https://visio.lasuite.coop/) | lasuite.coop | Free and open demo to all. Content and accounts are reset after one month | | ||
| | [mosacloud.cloud](https://mosa.cloud/) | mosa.cloud | Demo instance of mosa.cloud, a dutch company providing services around La Suite apps. | |
There was a problem hiding this comment.
Known-instances label and URL host are inconsistent.
The displayed host is mosacloud.cloud while the actual link points to mosa.cloud, which is confusing for readers.
Suggested doc fix
-| [mosacloud.cloud](https://mosa.cloud/) | mosa.cloud | Demo instance of mosa.cloud, a dutch company providing services around La Suite apps. |
+| [mosa.cloud](https://mosa.cloud/) | mosa.cloud | Demo instance of mosa.cloud, a dutch company providing services around La Suite apps. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | [mosacloud.cloud](https://mosa.cloud/) | mosa.cloud | Demo instance of mosa.cloud, a dutch company providing services around La Suite apps. | | |
| | [mosa.cloud](https://mosa.cloud/) | mosa.cloud | Demo instance of mosa.cloud, a dutch company providing services around La Suite apps. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 120, The table row currently shows a mismatch between the
link text and the target URL: the markdown link
"[mosacloud.cloud](https://mosa.cloud/)" displays mosacloud.cloud but points to
https://mosa.cloud/; update the markdown so the display label and the href match
(either change the link target to https://mosacloud.cloud/ or change the display
text to mosa.cloud) for the table entry to keep the host and URL consistent.
Comment on lines
+33
to
+41
| fields = [ | ||
| "id", | ||
| "email", | ||
| "full_name", | ||
| "short_name", | ||
| "timezone", | ||
| "language", | ||
| "default_encryption_mode", | ||
| ] |
There was a problem hiding this comment.
Reject non-none user defaults while encryption is disabled.
RoomViewSet.perform_create() already blocks encrypted room creation when ENCRYPTION_ENABLED is false, but /users/me can still persist "basic" here. That leaves the API storing a default for a feature the same server says is unavailable.
Localized validation
class UserSerializer(serializers.ModelSerializer):
"""Serialize users."""
@@
class Meta:
model = models.User
fields = [
"id",
"email",
"full_name",
"short_name",
"timezone",
"language",
"default_encryption_mode",
]
read_only_fields = ["id", "email", "full_name", "short_name"]
+
+ def validate_default_encryption_mode(self, value):
+ if value != models.EncryptionMode.NONE and not settings.ENCRYPTION_ENABLED:
+ raise serializers.ValidationError(
+ "Encryption is not enabled on this server."
+ )
+ return value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fields = [ | |
| "id", | |
| "email", | |
| "full_name", | |
| "short_name", | |
| "timezone", | |
| "language", | |
| "default_encryption_mode", | |
| ] | |
| class UserSerializer(serializers.ModelSerializer): | |
| """Serialize users.""" | |
| class Meta: | |
| model = models.User | |
| fields = [ | |
| "id", | |
| "email", | |
| "full_name", | |
| "short_name", | |
| "timezone", | |
| "language", | |
| "default_encryption_mode", | |
| ] | |
| read_only_fields = ["id", "email", "full_name", "short_name"] | |
| def validate_default_encryption_mode(self, value): | |
| if value != models.EncryptionMode.NONE and not settings.ENCRYPTION_ENABLED: | |
| raise serializers.ValidationError( | |
| "Encryption is not enabled on this server." | |
| ) | |
| return value |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/core/api/serializers.py` around lines 33 - 41, The serializer
must reject non-null/default_encryption_mode when ENCRYPTION_ENABLED is False so
the API cannot persist an encryption default while the server disables
encryption; add validation to the user serializer (e.g., a
validate_default_encryption_mode method or in validate()) that checks django
settings.ENCRYPTION_ENABLED and, if False and the incoming
default_encryption_mode is not None (or not "none"/empty), raise
serializers.ValidationError with a clear message; reference the
default_encryption_mode field in this serializer and keep
RoomViewSet.perform_create behaviour unchanged.
Comment on lines
+411
to
+419
| # Set at creation, immutable after (the URL hash carries the passphrase, | ||
| # so changing the mode would break every previously-shared link). | ||
| encryption_mode = models.CharField( | ||
| max_length=20, | ||
| choices=EncryptionMode.choices, | ||
| default=EncryptionMode.NONE, | ||
| verbose_name=_("Encryption mode"), | ||
| help_text=_("End-to-end encryption mode for this room."), | ||
| ) |
There was a problem hiding this comment.
Enforce encrypted-room invariants in the model layer.
encryption_mode is documented as immutable here, and encrypted rooms are only safe when they stay RESTRICTED, but both guarantees currently live only in RoomSerializer. Any ORM/admin write can still flip encryption_mode or access_level, which either breaks every shared link or bypasses the waiting-room requirement.
Possible guard in Room.clean()
class Room(Resource):
"""Model for one room"""
@@
+ def clean(self):
+ super().clean()
+
+ if self.encryption_mode != EncryptionMode.NONE:
+ if self.access_level != RoomAccessLevel.RESTRICTED:
+ raise ValidationError(
+ {"access_level": _("Encrypted rooms require restricted access level.")}
+ )
+
+ if self.pk:
+ previous = type(self).objects.only(
+ "encryption_mode",
+ ).get(pk=self.pk)
+ if previous.encryption_mode != self.encryption_mode:
+ raise ValidationError(
+ {
+ "encryption_mode": _(
+ "Encryption mode cannot be changed after room creation."
+ )
+ }
+ )Also applies to: 444-461
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/core/models.py` around lines 411 - 419, The model must enforce
the invariants currently only in RoomSerializer: make Room.validate/clean (or
override save to call full_clean) to (1) prevent changing encryption_mode after
creation (detect existing instance by pk and raise ValidationError if
encryption_mode differs from DB value), and (2) enforce that when
encryption_mode == EncryptionMode.RESTRICTED the access_level is/always remains
AccessLevel.RESTRICTED (and reject any change that would set access_level to
OPEN), and (3) block creating a RESTRICTED encryption_mode with a non-RESTRICTED
access_level; ensure these checks live in Room.clean() (and call full_clean from
save) so ORM/admin writes are covered, using the model fields encryption_mode
and access_level and raising django.core.exceptions.ValidationError on
violation.
| const { t } = useTranslation('rooms', { keyPrefix: 'options.items' }) | ||
| const { isTranscriptOpen, openTranscript, toggleTools } = useSidePanel() | ||
| const roomData = useRoomData() | ||
| const isEncrypted = roomData?.encryption_mode === 'basic' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use enum/shared encrypted check here too (avoid 'basic' literal).
Line 14 repeats a raw mode string. Please align with enum/helper-based encryption detection so transcript disabling remains correct if mode values or additional encrypted modes change.
Proposed change
+import { ApiEncryptionMode } from '`@/features/rooms/api/ApiRoom`'
import { useRoomData } from '`@/features/rooms/livekit/hooks/useRoomData`'
...
- const isEncrypted = roomData?.encryption_mode === 'basic'
+ const isEncrypted =
+ roomData?.encryption_mode != null &&
+ roomData.encryption_mode !== ApiEncryptionMode.NONEAlso applies to: 26-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/frontend/src/features/rooms/livekit/components/controls/Options/TranscriptMenuItem.tsx`
at line 14, The component currently uses a raw string check const isEncrypted =
roomData?.encryption_mode === 'basic' (and similar checks around lines 26-31);
replace these literal comparisons with the shared encryption-mode enum or helper
(e.g., use the existing EncryptionMode enum or an isEncryptedRoom(roomData)
helper) so the logic uses the central source of truth for encrypted modes;
update TranscriptMenuItem.tsx to import and call that enum/helper where
isEncrypted and any subsequent conditionals reference roomData.encryption_mode
so future mode additions still disable transcripts correctly.
Comment on lines
+107
to
+111
| // Email is only displayed to authenticated viewers (defense-in-depth on | ||
| // top of the JWT-level guarantee that it's only emitted for authenticated | ||
| // participants). The LK signaling channel broadcasts attributes to every | ||
| // peer, so the UI is what protects anonymous viewers from seeing it. | ||
| const email = isLoggedIn ? participant.attributes?.email : undefined |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C3 "participant\.attributes\?\.email|attributes.*email|email.*attributes|is_authenticated" src/backend src/frontendRepository: suitenumerique/meet
Length of output: 28655
Email is broadcast to all peers; UI gating alone does not prevent access from anonymous participants.
The backend adds email to participant.attributes for encrypted rooms (see src/backend/core/utils.py lines 139–142). LiveKit broadcasts participant attributes to every connected peer. Line 111's isLoggedIn check only hides the email in React; the value remains accessible to any participant through client state inspection or devtools.
Move email disclosure to an authenticated API endpoint instead of peer-broadcast participant attributes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/frontend/src/features/rooms/livekit/components/controls/Participants/ParticipantListItem.tsx`
around lines 107 - 111, The component currently reads sensitive email from
participant.attributes and only hides it in the UI via isLoggedIn
(ParticipantListItem), but attributes are broadcast to all peers; remove
reliance on participant.attributes.email in ParticipantListItem and stop
displaying or reading it directly from LiveKit attributes. Instead, return
undefined for the email when constructing the UI and add a secure call to an
authenticated backend endpoint (e.g., a new getUserEmail/getParticipantEmail API
used by ParticipantListItem or a surrounding hook) to fetch the email only for
authenticated users; update ParticipantListItem to use that API/hook and keep
the isLoggedIn check before calling it so emails are fetched and displayed only
for authenticated sessions.
Comment on lines
+10
to
+16
| export const isRoomValid = (roomIdOrUrl: string) => { | ||
| const lower = roomIdOrUrl.toLowerCase() | ||
| return ( | ||
| roomRegex.test(lower) || | ||
| roomWithoutHyphensRegex.test(lower) || | ||
| new RegExp(`^${window.location.origin}/${roomIdPattern}`).test(roomIdOrUrl) | ||
| ) |
There was a problem hiding this comment.
Tighten room URL validation to reject invalid trailing paths.
The current URL regex accepts any value that merely starts with a valid room URL (e.g., /abc-defg-hij/extra). Please constrain the suffix to optional query/hash only, so encrypted links still pass but malformed paths do not.
Suggested fix
export const isRoomValid = (roomIdOrUrl: string) => {
const lower = roomIdOrUrl.toLowerCase()
+ const escapedOrigin = window.location.origin.replace(
+ /[.*+?^${}()|[\]\\]/g,
+ '\\$&'
+ )
+ const roomUrlRegex = new RegExp(
+ `^${escapedOrigin}/${roomIdPattern}(?:[?#].*)?$`
+ )
return (
roomRegex.test(lower) ||
roomWithoutHyphensRegex.test(lower) ||
- new RegExp(`^${window.location.origin}/${roomIdPattern}`).test(roomIdOrUrl)
+ roomUrlRegex.test(lower)
)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/src/features/rooms/utils/isRoomValid.ts` around lines 10 - 16,
isRoomValid currently allows URLs that start with a valid room URL but have
extra path segments (e.g., /room-id/extra); update the URL test to anchor the
roomIdPattern so only optional query/hash suffixes are permitted. In
isRoomValid, modify the RegExp that uses window.location.origin and
roomIdPattern to require end-of-path after the room id and allow only an
optional query or hash (e.g., append a non-capturing group like (?:[?#].*)?$ to
the pattern) so the test for full URLs rejects trailing path segments; keep the
other checks (roomRegex, roomWithoutHyphensRegex) unchanged.
Comment on lines
+24
to
+43
| const { mutateAsync, isPending } = useMutation({ | ||
| mutationFn: updateUserPreferences, | ||
| onSuccess: (updatedUser) => { | ||
| queryClient.setQueryData([keys.user], updatedUser) | ||
| }, | ||
| }) | ||
|
|
||
| const isOn = user?.default_encryption_mode === ApiEncryptionMode.BASIC | ||
|
|
||
| const handleToggle = (next: boolean) => { | ||
| if (!user) return | ||
| void mutateAsync({ | ||
| user: { | ||
| id: user.id, | ||
| default_encryption_mode: next | ||
| ? ApiEncryptionMode.BASIC | ||
| : ApiEncryptionMode.NONE, | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find package.json to check React Query version
find . -name "package.json" -type f | head -5Repository: suitenumerique/meet
Length of output: 221
🏁 Script executed:
# Look at the full component to see complete context
cat -n src/frontend/src/features/settings/components/EncryptionDefaultField.tsxRepository: suitenumerique/meet
Length of output: 3273
🏁 Script executed:
# Search for other useMutation patterns to see how mutations are typically handled
rg "useMutation" -A 5 -B 2 | head -100Repository: suitenumerique/meet
Length of output: 8314
🏁 Script executed:
# Check React Query version in package.json
cat src/frontend/package.json | grep -A 2 -B 2 "react-query\|`@tanstack/react-query`"Repository: suitenumerique/meet
Length of output: 443
🏁 Script executed:
# Search for any global error handlers or other error handling patterns in the app
rg "onError" src/frontend/src --type tsx --type ts | head -20Repository: suitenumerique/meet
Length of output: 91
🏁 Script executed:
# Search for onError handlers with correct syntax
rg "onError" src/frontend/src | head -30Repository: suitenumerique/meet
Length of output: 1924
🏁 Script executed:
# Check if updateUserPreferences has any built-in error handling
grep -r "updateUserPreferences" src/frontend/src --include="*.ts" --include="*.tsx" -A 10Repository: suitenumerique/meet
Length of output: 5860
Add error handling to the mutation or use mutate instead of mutateAsync.
Line 35 calls void mutateAsync(...) without error handling. With React Query v5, mutateAsync returns a Promise that rejects on failures. Without await/catch or an onError handler, errors silently surface as unhandled promise rejections.
Either add onError to the mutation options or switch to mutate (the callback variant):
Example fix
- const { mutateAsync, isPending } = useMutation({
+ const { mutate, isPending } = useMutation({
mutationFn: updateUserPreferences,
onSuccess: (updatedUser) => {
queryClient.setQueryData([keys.user], updatedUser)
},
})
@@
- void mutateAsync({
+ mutate({
user: {
id: user.id,
default_encryption_mode: next
? ApiEncryptionMode.BASIC
: ApiEncryptionMode.NONE,
},
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { mutateAsync, isPending } = useMutation({ | |
| mutationFn: updateUserPreferences, | |
| onSuccess: (updatedUser) => { | |
| queryClient.setQueryData([keys.user], updatedUser) | |
| }, | |
| }) | |
| const isOn = user?.default_encryption_mode === ApiEncryptionMode.BASIC | |
| const handleToggle = (next: boolean) => { | |
| if (!user) return | |
| void mutateAsync({ | |
| user: { | |
| id: user.id, | |
| default_encryption_mode: next | |
| ? ApiEncryptionMode.BASIC | |
| : ApiEncryptionMode.NONE, | |
| }, | |
| }) | |
| } | |
| const { mutate, isPending } = useMutation({ | |
| mutationFn: updateUserPreferences, | |
| onSuccess: (updatedUser) => { | |
| queryClient.setQueryData([keys.user], updatedUser) | |
| }, | |
| }) | |
| const isOn = user?.default_encryption_mode === ApiEncryptionMode.BASIC | |
| const handleToggle = (next: boolean) => { | |
| if (!user) return | |
| mutate({ | |
| user: { | |
| id: user.id, | |
| default_encryption_mode: next | |
| ? ApiEncryptionMode.BASIC | |
| : ApiEncryptionMode.NONE, | |
| }, | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/src/features/settings/components/EncryptionDefaultField.tsx`
around lines 24 - 43, The mutation call in handleToggle uses mutateAsync without
handling rejections; update the useMutation call that references
updateUserPreferences to provide an onError handler (e.g., onError: (err,
variables, context) => { /* revert UI or notify */ }) or change handleToggle to
call mutate instead of mutateAsync so errors are handled via the mutation's
callbacks; modify the useMutation options around mutateAsync/mutate (symbols:
useMutation, updateUserPreferences, mutateAsync, mutate, onError, handleToggle)
to ensure failures are caught and the UI/query state is reverted or an error is
surfaced.
Comment on lines
+138
to
+140
| const next = props.validate(renderProps.isSelected) | ||
| if (next !== error) setError(next) | ||
| } else if (error !== null) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "Checkbox.tsx" --path "*frontend*"Repository: suitenumerique/meet
Length of output: 297
🏁 Script executed:
# Read the Checkbox component to see the full context around lines 138-140
cat -n src/frontend/src/primitives/Checkbox.tsx | sed -n '130,150p'Repository: suitenumerique/meet
Length of output: 951
🏁 Script executed:
# Get the full file to understand the component structure
wc -l src/frontend/src/primitives/Checkbox.tsxRepository: suitenumerique/meet
Length of output: 108
🏁 Script executed:
cat -n src/frontend/src/primitives/Checkbox.tsx | head -80Repository: suitenumerique/meet
Length of output: 2913
🏁 Script executed:
cat -n src/frontend/src/primitives/Checkbox.tsx | tail -100Repository: suitenumerique/meet
Length of output: 3790
Move validation state updates out of render function
Lines 138–141 call setError during render, which is an anti-pattern in React. If props.validate returns non-stable ReactNode values (arrays, objects, or new elements), the comparison next !== error will always be true, causing infinite rerenders. Move the validation logic to a useEffect hook that synchronizes when renderProps.isSelected or renderProps.isInvalid changes, or derive the error state without direct state mutation in render.
Proposed direction
- {(renderProps) => {
- if (renderProps.isInvalid && !!props.validate) {
- const next = props.validate(renderProps.isSelected)
- if (next !== error) setError(next)
- } else if (error !== null) {
- setError(null)
- }
+ {(renderProps) => {
+ // Avoid setState during render; synchronize validation result in an effect-driven layer.
+ // Keep render pure here.
return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/src/primitives/Checkbox.tsx` around lines 138 - 140, The render
is calling props.validate and calling setError (using error) directly which can
trigger infinite re-renders; move this validation out of the render path by
running it inside a useEffect that depends on renderProps.isSelected and
renderProps.isInvalid (and props.validate if needed) or by deriving the error
synchronously from props without calling setError in render; update the Checkbox
component to compute next = props.validate(renderProps.isSelected) inside that
effect and only call setError(next) if next !== error (or eliminate local error
state and use a derivedError variable) to prevent state updates during render.
…advanced encryption client in keycloak
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
First step to bring more privacy to meetings without requiring a specific user onboarding or a third-party server managing keys distribution.
The shared secret is passed within the URL hash so it's never sent to the
meetserver. The meeting creator is responsible to provide the link having the hash over secured channels (Tchap, ...). Knowing those meetings require the waiting room, so even if the link leaked across an email, the meeting admin still accept or not new participants.Also:
EDIT:
Due to first thoughts I had "default_encryption_mode" in the database to also prepare for potential advanced encryption mode, but then we switched to an additional step on the home screen to do "per-meeting" choice. Maybe I should change this for now, thoughts?