FRE-603: Fix code review blockers (memory leak + auth security)
- Fixed memory leak in PresenceManager: event handlers now use bound methods so they can be properly removed in shutdown() - Removed auth token from URL query parameters (security: prevents token leakage to server logs and browser history) - Fixed TypeScript errors: corrected WebsocketProvider import, removed unsupported send() calls, fixed type mismatches in presence callbacks - All collaboration module files now type-check successfully Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -64,18 +64,14 @@ export class WebSocketConnection implements WebSocketConnectionWithPresence {
|
||||
// Create or use provided Yjs doc
|
||||
const ydoc = this.options.doc || new Y.Doc();
|
||||
|
||||
// Prepare auth params (y-websocket uses query params for auth)
|
||||
const params: Record<string, string> = {
|
||||
token: this.options.authToken,
|
||||
};
|
||||
|
||||
// Create provider without auth token in URL (security: tokens in query params leak to logs)
|
||||
this.provider = new WebsocketProvider(
|
||||
this.options.serverUrl,
|
||||
this.options.documentName,
|
||||
ydoc,
|
||||
{
|
||||
connect: true,
|
||||
params,
|
||||
// Don't pass token in URL - send via awareness after connection
|
||||
maxBackoffTime: this.options.maxReconnectInterval || 30000,
|
||||
}
|
||||
);
|
||||
@@ -88,6 +84,8 @@ export class WebSocketConnection implements WebSocketConnectionWithPresence {
|
||||
if (newStatus === 'connected') {
|
||||
this.reconnectAttempts = 0;
|
||||
this.currentReconnectInterval = this.options.reconnectInterval || 1000;
|
||||
// Send auth token via awareness state after connection (secure: not in URL logs)
|
||||
this.sendAuthToken();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -189,6 +187,25 @@ export class WebSocketConnection implements WebSocketConnectionWithPresence {
|
||||
}, delay);
|
||||
}
|
||||
|
||||
/**
|
||||
* Send auth token via awareness state after connection
|
||||
* Security: Token not exposed in URL/logs, only sent over secure WebSocket
|
||||
*/
|
||||
private sendAuthToken(): void {
|
||||
if (!this.provider || !this.provider.awareness) {
|
||||
console.error('[WebSocketConnection] Cannot send auth token: provider not ready');
|
||||
return;
|
||||
}
|
||||
|
||||
// Store token in awareness state (sent to server, not in URL)
|
||||
this.provider.awareness.setLocalStateField('auth', {
|
||||
token: this.options.authToken,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
|
||||
console.log('[WebSocketConnection] Auth token sent via awareness state');
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the presence manager instance
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user