-
Notifications
You must be signed in to change notification settings - Fork 353
Description
Description
Every authenticated API method in packages/api/src/EmbeddedChatApi.ts silently fails when getCurrentUser() returns null or undefined, sending HTTP requests with the literal string "undefined" as credentials.
Affected File
packages/api/src/EmbeddedChatApi.ts (all authenticated methods)
Root Cause
Every authenticated method follows this pattern:
const { userId, authToken } = (await this.auth.getCurrentUser()) || {};When getCurrentUser() returns null or undefined, JavaScript destructures it to:
{ userId: undefined, authToken: undefined }The HTTP request is then dispatched with:
X-Auth-Token: undefined
X-User-Id: undefined
These are literal string values - not missing headers - causing three distinct problems:
Impact
- Wasted Network Round-Trip
When getCurrentUser() returns null, the old code still sends the full HTTP request to the Rocket.Chat server - just with "undefined" as the credential strings:
X-Auth-Token: undefined ← literal string, not a missing header
X-User-Id: undefined ← literal string, not a missing header
The server receives these, validates them, finds no matching session, and returns a 401 Unauthorized response. This means:
-
CPU, memory, and bandwidth are consumed on both client and server for a request that was always going to fail.
-
In high-frequency scenarios (e.g., auto-retry loops, polling), this could generate significant unnecessary load.
-
The fix eliminates this entirely - the error is thrown before any network call is made.
- Silent FailureThis is the most dangerous impact. The request doesn't crash visibly - it just quietly returns a failed API response. Here's why that's a problem:
// Old pattern - callers might do this:
const result = await this.sendMessage(msg);
// result is a 401 error object, not a thrown exception!
// If the caller doesn't check result.success, the failure is invisible.
-
Methods like sendMessage, getChannelInfo, etc. would return an error object rather than throwing, so callers that don't explicitly check result.success or result.error would have no idea the operation failed.
-
This masks the real root cause (unauthenticated state) behind what looks like a generic server response.
-
With the fix, a proper throw new Error('Not authenticated') ensures callers are forced to handle the failure - you can't silently ignore a thrown exception.
- Information Leakage
Even though the request fails with a 401, the request itself still goes out and reveals useful information to anyone observing network traffic:
-
API endpoint structure - which routes exist (e.g.,
/api/v1/channels.info, /api/v1/chat.sendMessage) -
HTTP methods in use - GET, POST, etc.
-
Parameter shapes - query params, body structure
-
Timing information - which features the app is trying to use
In an unauthenticated context (e.g., before login, after session expiry, or in a public embed), this information shouldn't be leaving the client at all. The fix ensures no request is dispatched unless auth is confirmed valid, so the API surface remains opaque to unauthenticated observers.
Steps to Reproduce
- Initialize
EmbeddedChatApiwithout completing authentication (or with an expired session). - Call any authenticated method (e.g.,
sendMessage,getChannelInfo, etc.). - Observe the outgoing HTTP request -
X-Auth-Token: undefinedandX-User-Id: undefinedare sent. - No early error is thrown; the caller receives a failed API response.
Expected Behavior
An error should be thrown before the HTTP request is dispatched, preventing the network call entirely and giving callers a clear, actionable error message.