Skip to content

Remove ICE server list from config response when unauthenticated#3054

Merged
gantoine merged 1 commit intomasterfrom
ise-server-config-response
Mar 1, 2026
Merged

Remove ICE server list from config response when unauthenticated#3054
gantoine merged 1 commit intomasterfrom
ise-server-config-response

Conversation

@gantoine
Copy link
Member

@gantoine gantoine commented Mar 1, 2026

Description
Explain the changes or enhancements you are proposing with this pull request.

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

Screenshots (if applicable)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

Test Results

804 tests   803 ✅  2m 5s ⏱️
  1 suites    1 💤
  1 files      0 ❌

Results for commit 8d4b03b.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
13005 8516 65% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
backend/endpoints/configs.py 39% 🟢
TOTAL 39% 🟢

updated for commit: 8d4b03b by action🐍

@greptile-apps
Copy link

greptile-apps bot commented Mar 1, 2026

Greptile Summary

This PR adds an authentication check to the /api/config endpoint to prevent exposing ICE server credentials (username and credential fields) to unauthenticated users. The implementation correctly leverages Starlette's AuthenticationMiddleware to check request.user.is_authenticated and returns an empty list for unauthenticated requests while maintaining full functionality for authenticated users.

  • Added Request parameter to get_config() function to access authentication state
  • Conditionally returns EJS_NETPLAY_ICE_SERVERS only when user is authenticated, otherwise returns empty list
  • Protects sensitive TURN/STUN server credentials from unauthorized access

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - it's a straightforward security fix with correct implementation
  • The change properly uses established authentication patterns in the codebase and fixes a real security issue. Score is 4/5 (not 5) due to missing test coverage for this security-sensitive feature
  • No files require special attention - the single changed file has a simple, well-implemented security fix

Important Files Changed

Filename Overview
backend/endpoints/configs.py Added authentication check to prevent exposing ICE server credentials to unauthenticated users; implementation is correct but lacks test coverage

Last reviewed commit: 8d4b03b

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@gantoine gantoine merged commit 8a892d9 into master Mar 1, 2026
9 checks passed
@gantoine gantoine deleted the ise-server-config-response branch March 1, 2026 00:45
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.

1 participant