Skip to content

Commit e997b78

Browse files
committed
CLI: Return 401 for bad channel tokens and spy handleUpgrade
Token-only and other token failures now respond with 401 Unauthorized. Invalid Origin still returns 403 Forbidden. Tests spy on handleUpgrade instead of replacing the method.
1 parent 130f127 commit e997b78

2 files changed

Lines changed: 37 additions & 48 deletions

File tree

code/core/src/core-server/utils/__tests__/server-channel.test.ts

Lines changed: 31 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ describe('ServerChannelTransport', () => {
114114
server.listeners('upgrade')[0](request, socket, head);
115115

116116
expect(socket.write).toHaveBeenCalledWith(
117-
'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
117+
'HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n'
118118
);
119119
expect(destroySpy).toHaveBeenCalled();
120120
});
@@ -139,7 +139,7 @@ describe('ServerChannelTransport', () => {
139139
server.listeners('upgrade')[0](request, socket, head);
140140

141141
expect(socket.write).toHaveBeenCalledWith(
142-
'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
142+
'HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n'
143143
);
144144
expect(destroySpy).toHaveBeenCalled();
145145
});
@@ -150,12 +150,10 @@ describe('ServerChannelTransport', () => {
150150
socket.write = vi.fn();
151151
socket.destroy = vi.fn();
152152
const destroySpy = vi.spyOn(socket, 'destroy');
153-
const handleUpgradeSpy = vi.fn();
154153
const transport = new ServerChannelTransport(server, options);
155-
156-
// Mock handleUpgrade to track if it's called
157-
// @ts-expect-error (accessing private property)
158-
transport.socket.handleUpgrade = handleUpgradeSpy;
154+
const handleUpgradeSpy = vi
155+
.spyOn(transport.socket, 'handleUpgrade')
156+
.mockImplementation(() => {});
159157

160158
// Simulate upgrade request with correct token and valid origin
161159
const request = {
@@ -204,11 +202,10 @@ describe('ServerChannelTransport', () => {
204202
socket.write = vi.fn();
205203
socket.destroy = vi.fn();
206204
const destroySpy = vi.spyOn(socket, 'destroy');
207-
const handleUpgradeSpy = vi.fn();
208205
const transport = new ServerChannelTransport(server, options);
209-
210-
// @ts-expect-error (accessing private property)
211-
transport.socket.handleUpgrade = handleUpgradeSpy;
206+
const handleUpgradeSpy = vi
207+
.spyOn(transport.socket, 'handleUpgrade')
208+
.mockImplementation(() => {});
212209

213210
const request = {
214211
url: `/storybook-server-channel?token=${mockToken}`,
@@ -229,11 +226,10 @@ describe('ServerChannelTransport', () => {
229226
socket.write = vi.fn();
230227
socket.destroy = vi.fn();
231228
const destroySpy = vi.spyOn(socket, 'destroy');
232-
const handleUpgradeSpy = vi.fn();
233229
const transport = new ServerChannelTransport(server, options);
234-
235-
// @ts-expect-error (accessing private property)
236-
transport.socket.handleUpgrade = handleUpgradeSpy;
230+
const handleUpgradeSpy = vi
231+
.spyOn(transport.socket, 'handleUpgrade')
232+
.mockImplementation(() => {});
237233

238234
const request = {
239235
url: '/storybook-server-channel?token=wrong-token',
@@ -244,7 +240,7 @@ describe('ServerChannelTransport', () => {
244240
server.listeners('upgrade')[0](request, socket, head);
245241

246242
expect(socket.write).toHaveBeenCalledWith(
247-
'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
243+
'HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n'
248244
);
249245
expect(destroySpy).toHaveBeenCalled();
250246
expect(handleUpgradeSpy).not.toHaveBeenCalled();
@@ -256,11 +252,10 @@ describe('ServerChannelTransport', () => {
256252
socket.write = vi.fn();
257253
socket.destroy = vi.fn();
258254
const destroySpy = vi.spyOn(socket, 'destroy');
259-
const handleUpgradeSpy = vi.fn();
260255
const transport = new ServerChannelTransport(server, options);
261-
262-
// @ts-expect-error (accessing private property)
263-
transport.socket.handleUpgrade = handleUpgradeSpy;
256+
const handleUpgradeSpy = vi
257+
.spyOn(transport.socket, 'handleUpgrade')
258+
.mockImplementation(() => {});
264259

265260
const request = {
266261
url: '/storybook-server-channel',
@@ -271,7 +266,7 @@ describe('ServerChannelTransport', () => {
271266
server.listeners('upgrade')[0](request, socket, head);
272267

273268
expect(socket.write).toHaveBeenCalledWith(
274-
'HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n'
269+
'HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n'
275270
);
276271
expect(destroySpy).toHaveBeenCalled();
277272
expect(handleUpgradeSpy).not.toHaveBeenCalled();
@@ -283,12 +278,10 @@ describe('ServerChannelTransport', () => {
283278
socket.write = vi.fn();
284279
socket.destroy = vi.fn();
285280
const destroySpy = vi.spyOn(socket, 'destroy');
286-
const handleUpgradeSpy = vi.fn();
287281
const transport = new ServerChannelTransport(server, options);
288-
289-
// Mock handleUpgrade to track if it's called
290-
// @ts-expect-error (accessing private property)
291-
transport.socket.handleUpgrade = handleUpgradeSpy;
282+
const handleUpgradeSpy = vi
283+
.spyOn(transport.socket, 'handleUpgrade')
284+
.mockImplementation(() => {});
292285

293286
// Simulate upgrade request with network address origin
294287
const request = {
@@ -312,12 +305,10 @@ describe('ServerChannelTransport', () => {
312305
socket.write = vi.fn();
313306
socket.destroy = vi.fn();
314307
const destroySpy = vi.spyOn(socket, 'destroy');
315-
const handleUpgradeSpy = vi.fn();
316308
const transport = new ServerChannelTransport(server, options);
317-
318-
// Mock handleUpgrade to track if it's called
319-
// @ts-expect-error (accessing private property)
320-
transport.socket.handleUpgrade = handleUpgradeSpy;
309+
const handleUpgradeSpy = vi
310+
.spyOn(transport.socket, 'handleUpgrade')
311+
.mockImplementation(() => {});
321312

322313
// Simulate upgrade request with 127.0.0.1 origin
323314
const request = {
@@ -341,12 +332,10 @@ describe('ServerChannelTransport', () => {
341332
socket.write = vi.fn();
342333
socket.destroy = vi.fn();
343334
const destroySpy = vi.spyOn(socket, 'destroy');
344-
const handleUpgradeSpy = vi.fn();
345335
const transport = new ServerChannelTransport(server, options);
346-
347-
// Mock handleUpgrade to track if it's called
348-
// @ts-expect-error (accessing private property)
349-
transport.socket.handleUpgrade = handleUpgradeSpy;
336+
const handleUpgradeSpy = vi
337+
.spyOn(transport.socket, 'handleUpgrade')
338+
.mockImplementation(() => {});
350339

351340
// Simulate upgrade request to wrong path
352341
const request = {
@@ -371,12 +360,10 @@ describe('ServerChannelTransport', () => {
371360
socket.write = vi.fn();
372361
socket.destroy = vi.fn();
373362
const destroySpy = vi.spyOn(socket, 'destroy');
374-
const handleUpgradeSpy = vi.fn();
375363
const transport = new ServerChannelTransport(server, webContainerOptions);
376-
377-
// Mock handleUpgrade to track if it's called
378-
// @ts-expect-error (accessing private property)
379-
transport.socket.handleUpgrade = handleUpgradeSpy;
364+
const handleUpgradeSpy = vi
365+
.spyOn(transport.socket, 'handleUpgrade')
366+
.mockImplementation(() => {});
380367

381368
const request = {
382369
url: '/storybook-server-channel',
@@ -399,12 +386,10 @@ describe('ServerChannelTransport', () => {
399386
socket.write = vi.fn();
400387
socket.destroy = vi.fn();
401388
const destroySpy = vi.spyOn(socket, 'destroy');
402-
const handleUpgradeSpy = vi.fn();
403389
const transport = new ServerChannelTransport(server, webContainerOptions);
404-
405-
// Mock handleUpgrade to track if it's called
406-
// @ts-expect-error (accessing private property)
407-
transport.socket.handleUpgrade = handleUpgradeSpy;
390+
const handleUpgradeSpy = vi
391+
.spyOn(transport.socket, 'handleUpgrade')
392+
.mockImplementation(() => {});
408393

409394
const request = {
410395
url: '/storybook-server-channel?token=wrong-token',

code/core/src/core-server/utils/get-server-channel.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,16 @@ export class ServerChannelTransport {
4444
// which the token alone authenticates.
4545
const { origin } = request.headers;
4646
if (origin && !isValidHost(new URL(origin).host, options)) {
47-
throw new Error('Invalid websocket origin');
47+
socket.write('HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n');
48+
socket.destroy();
49+
return;
4850
}
4951

5052
const requestToken = url.searchParams.get('token');
5153
if (!isValidToken(requestToken, options.token)) {
52-
throw new Error('Invalid websocket token');
54+
socket.write('HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n');
55+
socket.destroy();
56+
return;
5357
}
5458
}
5559

0 commit comments

Comments
 (0)