From 889fa22e79032927a0763dd92b4f40ae79d35e9b Mon Sep 17 00:00:00 2001 From: Brainslug Date: Tue, 8 Oct 2024 19:32:55 +0200 Subject: [PATCH] Fixed websocket authentication handling between websocket controllers (#23691) * Fixed authentication handling between websocket controllers * catch the error for a token upgrade * Create wicked-ligers-lay.md --- .changeset/wicked-ligers-lay.md | 5 +++ api/src/websocket/controllers/base.ts | 45 ++++++++++----------------- api/src/websocket/controllers/logs.ts | 12 +++++-- api/src/websocket/types.ts | 1 - 4 files changed, 30 insertions(+), 33 deletions(-) create mode 100644 .changeset/wicked-ligers-lay.md diff --git a/.changeset/wicked-ligers-lay.md b/.changeset/wicked-ligers-lay.md new file mode 100644 index 0000000000..3e4969b3e2 --- /dev/null +++ b/.changeset/wicked-ligers-lay.md @@ -0,0 +1,5 @@ +--- +"@directus/api": patch +--- + +Fixed authentication handling between logs streaming and regular websocket connections diff --git a/api/src/websocket/controllers/base.ts b/api/src/websocket/controllers/base.ts index 4061565d7c..9445036d5f 100644 --- a/api/src/websocket/controllers/base.ts +++ b/api/src/websocket/controllers/base.ts @@ -83,7 +83,6 @@ export default abstract class SocketController { authentication: { mode: authMode.data, timeout: authTimeout, - requireAdmin: false, }, }; } @@ -176,7 +175,14 @@ export default abstract class SocketController { return; } - if (!this.meetsAdminRequirement({ socket, accountability })) return; + try { + this.checkUserRequirements(accountability); + } catch { + logger.debug('WebSocket upgrade denied - ' + JSON.stringify(accountability || 'invalid')); + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); + return; + } this.server.handleUpgrade(request, socket, head, async (ws) => { this.catchInvalidMessages(ws); @@ -195,9 +201,10 @@ export default abstract class SocketController { const state = await authenticateConnection(WebSocketAuthMessage.parse(payload)); - if (this.meetsAdminRequirement({ client: ws, accountability: state.accountability })) return; + this.checkUserRequirements(state.accountability); ws.send(authenticationSuccess(payload['uid'], state.refresh_token)); + this.server.emit('connection', ws, state); } catch { logger.debug('WebSocket authentication handshake failed'); @@ -306,8 +313,7 @@ export default abstract class SocketController { protected async handleAuthRequest(client: WebSocketClient, message: WebSocketAuthMessage) { try { const { accountability, expires_at, refresh_token } = await authenticateConnection(message); - - if (!this.meetsAdminRequirement({ client, accountability })) return; + this.checkUserRequirements(accountability); client.accountability = accountability; client.expires_at = expires_at; @@ -335,6 +341,11 @@ export default abstract class SocketController { } } + protected checkUserRequirements(_accountability: Accountability | null) { + // there are no requirements in the abstract class + return; + } + setTokenExpireTimer(client: WebSocketClient) { if (client.auth_timer !== null) { // clear up old timeouts if needed @@ -375,30 +386,6 @@ export default abstract class SocketController { }, TOKEN_CHECK_INTERVAL); } - meetsAdminRequirement({ - socket, - client, - accountability, - }: { - socket?: UpgradeContext['socket']; - client?: WebSocketClient | WebSocket; - accountability: Accountability | null; - }) { - if (!this.authentication.requireAdmin || accountability?.admin) return true; - - logger.debug('WebSocket connection denied - ' + JSON.stringify(accountability || 'invalid')); - - if (socket) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.destroy(); - } else if (client) { - handleWebSocketError(client, new WebSocketError('auth', 'UNAUTHORIZED', 'Unauthorized.'), 'auth'); - client.close(); - } - - return false; - } - terminate() { if (this.authInterval) clearInterval(this.authInterval); diff --git a/api/src/websocket/controllers/logs.ts b/api/src/websocket/controllers/logs.ts index 8286307363..81a9b2e444 100644 --- a/api/src/websocket/controllers/logs.ts +++ b/api/src/websocket/controllers/logs.ts @@ -3,10 +3,11 @@ import type { Server as httpServer } from 'http'; import type WebSocket from 'ws'; import emitter from '../../emitter.js'; import { useLogger } from '../../logger/index.js'; -import { handleWebSocketError } from '../errors.js'; +import { handleWebSocketError, WebSocketError } from '../errors.js'; import { AuthMode, WebSocketMessage } from '../messages.js'; import type { AuthenticationState, WebSocketClient } from '../types.js'; import SocketController from './base.js'; +import type { Accountability } from '@directus/types'; const logger = useLogger(); @@ -34,11 +35,9 @@ export class LogsController extends SocketController { return { endpoint, maxConnections, - // require strict auth authentication: { mode: 'strict' as AuthMode, timeout: 0, - requireAdmin: true, }, }; } @@ -63,4 +62,11 @@ export class LogsController extends SocketController { emitter.emitAction('websocket.connect', { client }); } + + protected override checkUserRequirements(accountability: Accountability | null) { + // enforce admin only access for the logs streaming websocket + if (!accountability?.admin) { + throw new WebSocketError('auth', 'AUTH_FAILED', 'Unauthorized access.'); + } + } } diff --git a/api/src/websocket/types.ts b/api/src/websocket/types.ts index 98a484f0c5..440963b1bd 100644 --- a/api/src/websocket/types.ts +++ b/api/src/websocket/types.ts @@ -16,7 +16,6 @@ export type UpgradeRequest = IncomingMessage & AuthenticationState; export type WebSocketAuthentication = { mode: AuthMode; timeout: number; - requireAdmin: boolean; }; export type SubscriptionEvent = 'create' | 'update' | 'delete';