Skip to content

✨(backend) force ProConnect official name in trusted rooms#1155

Open
mmaudet wants to merge 1 commit intosuitenumerique:mainfrom
mmaudet:feat/784-force-proconnect-name-trusted-rooms
Open

✨(backend) force ProConnect official name in trusted rooms#1155
mmaudet wants to merge 1 commit intosuitenumerique:mainfrom
mmaudet:feat/784-force-proconnect-name-trusted-rooms

Conversation

@mmaudet
Copy link
Copy Markdown
Collaborator

@mmaudet mmaudet commented Mar 17, 2026

Summary

  • When a room has trusted access level and the user is authenticated via ProConnect/OIDC, the display name is now forced to the user's official full_name from the OIDC claims
  • Custom usernames passed as query parameters are ignored for authenticated users in trusted rooms, preventing identity fraud
  • Falls back to str(user) (email) if full_name is not available
  • Applied consistently in both the room serializer (direct room access) and the lobby service (entry request flow)

Closes #784

Test plan

  • Create a room with trusted access level
  • Join as an authenticated ProConnect user with a custom username → verify the display name shows the official ProConnect name, not the custom one
  • Join as an authenticated user with no full_name set → verify fallback to email
  • Join a public room as an authenticated user with a custom username → verify custom username is still used (no override)
  • Run backend tests: pytest src/backend/core/tests/rooms/test_api_rooms_retrieve.py src/backend/core/tests/services/test_lobby.py

@lebaudantoine
Copy link
Copy Markdown
Collaborator

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

Walkthrough

This pull request implements username override logic for authenticated users accessing trusted rooms. The changes add functionality to set the LiveKit token username to the user's official full_name (or string representation as fallback) in trusted room scenarios. Modifications are made to the serializer's to_representation method and the lobby service's request_entry method, with corresponding test updates across four files to validate the new behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: forcing the official ProConnect name in trusted rooms, which is the primary objective of this PR.
Description check ✅ Passed The description is well-related to the changeset, explaining the feature, its implementation, fallback behavior, scope, and test plan.
Linked Issues check ✅ Passed The code changes fully implement issue #784's requirements: forcing official ProConnect full_name for trusted rooms, preventing custom usernames, and falling back to email.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the security feature for trusted rooms. No unrelated modifications or scope creep detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/backend/core/tests/services/test_lobby.py (1)

374-409: 🧹 Nitpick | 🔵 Trivial

Consider adding a test for the fallback case when full_name is empty.

The test test_request_entry_accepted_participant uses a RESTRICTED room, so it doesn't exercise the trusted room code path for already-accepted participants. Additionally, there's no test covering the fallback to str(user) when full_name is empty in the lobby service (the serializers tests have this coverage but the lobby tests don't).

Would you like me to help draft additional test cases for:

  1. An accepted participant in a trusted room (exercising lines 186-191 in lobby.py)
  2. The fallback to str(user) when full_name is empty?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/services/test_lobby.py` around lines 374 - 409, Add
tests covering two missing cases: (1) an accepted participant in a trusted room
so request_entry follows the trusted-room branch (create a RoomFactory with
access_level=RoomAccessLevel.TRUSTED and assert lobby_service.request_entry
returns the participant and calls generate_livekit_config with
is_admin_or_owner=False and the participant's color/username), and (2) the
fallback to str(user) when full_name is empty by mocking request.user.full_name
= "" (or ensuring the user object has no full_name) and asserting that
generate_livekit_config is called with username equal to str(request.user); use
the same patterns as test_request_entry_accepted_participant to mock
lobby_service._get_or_create_participant_id, lobby_service._get_participant, and
generate_livekit_config and assert calls and returned values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/backend/core/tests/services/test_lobby.py`:
- Around line 374-409: Add tests covering two missing cases: (1) an accepted
participant in a trusted room so request_entry follows the trusted-room branch
(create a RoomFactory with access_level=RoomAccessLevel.TRUSTED and assert
lobby_service.request_entry returns the participant and calls
generate_livekit_config with is_admin_or_owner=False and the participant's
color/username), and (2) the fallback to str(user) when full_name is empty by
mocking request.user.full_name = "" (or ensuring the user object has no
full_name) and asserting that generate_livekit_config is called with username
equal to str(request.user); use the same patterns as
test_request_entry_accepted_participant to mock
lobby_service._get_or_create_participant_id, lobby_service._get_participant, and
generate_livekit_config and assert calls and returned values.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ecd07b4-cded-4d15-9cc9-0ba5dc834807

📥 Commits

Reviewing files that changed from the base of the PR and between 0cb3fb8 and b73ff99.

📒 Files selected for processing (4)
  • src/backend/core/api/serializers.py
  • src/backend/core/services/lobby.py
  • src/backend/core/tests/rooms/test_api_rooms_retrieve.py
  • src/backend/core/tests/services/test_lobby.py

When a room has trusted access level and the user is authenticated,
override the username with the user's full_name from ProConnect
claims to prevent identity fraud. Falls back to email if full_name
is empty.

Closes suitenumerique#784
@mmaudet mmaudet force-pushed the feat/784-force-proconnect-name-trusted-rooms branch from b73ff99 to 4b84bd9 Compare March 20, 2026 18:12
@sonarqubecloud
Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security link : Display the ProConnect offical name of the participants ( for the option "ouvrir aux personnes de confiance"

2 participants