diff --git a/.changeset/tender-apes-doubt.md b/.changeset/tender-apes-doubt.md new file mode 100644 index 0000000000..f727bdeb25 --- /dev/null +++ b/.changeset/tender-apes-doubt.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-signals-backend': patch +--- + +Improved error logging and fixed authentication diff --git a/plugins/signals-backend/api-report.md b/plugins/signals-backend/api-report.md index 8bdaa581f3..5724c14aff 100644 --- a/plugins/signals-backend/api-report.md +++ b/plugins/signals-backend/api-report.md @@ -3,18 +3,22 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { AuthService } from '@backstage/backend-plugin-api'; import { BackendFeature } from '@backstage/backend-plugin-api'; import { EventBroker } from '@backstage/plugin-events-node'; import express from 'express'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { LoggerService } from '@backstage/backend-plugin-api'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { UserInfoService } from '@backstage/backend-plugin-api'; // @public (undocumented) export function createRouter(options: RouterOptions): Promise; // @public (undocumented) export interface RouterOptions { + // (undocumented) + auth?: AuthService; // (undocumented) discovery: PluginEndpointDiscovery; // (undocumented) @@ -23,6 +27,8 @@ export interface RouterOptions { identity: IdentityApi; // (undocumented) logger: LoggerService; + // (undocumented) + userInfo?: UserInfoService; } // @public diff --git a/plugins/signals-backend/src/plugin.ts b/plugins/signals-backend/src/plugin.ts index 11b63163d5..f80a8a2dbd 100644 --- a/plugins/signals-backend/src/plugin.ts +++ b/plugins/signals-backend/src/plugin.ts @@ -33,15 +33,19 @@ export const signalsPlugin = createBackendPlugin({ logger: coreServices.logger, identity: coreServices.identity, discovery: coreServices.discovery, + userInfo: coreServices.userInfo, + auth: coreServices.auth, // TODO: EventBroker. It is optional for now but it's actually required so waiting for the new backend system // for the events-backend for this to work. }, - async init({ httpRouter, logger, identity, discovery }) { + async init({ httpRouter, logger, identity, discovery, userInfo, auth }) { httpRouter.use( await createRouter({ logger, identity, discovery, + userInfo, + auth, }), ); }, diff --git a/plugins/signals-backend/src/service/SignalManager.test.ts b/plugins/signals-backend/src/service/SignalManager.test.ts index 5720d1ff52..cf19f36224 100644 --- a/plugins/signals-backend/src/service/SignalManager.test.ts +++ b/plugins/signals-backend/src/service/SignalManager.test.ts @@ -126,25 +126,15 @@ describe('SignalManager', () => { // Connection with identity and subscription const ws2 = new MockWebSocket(); manager.addConnection(ws2 as unknown as WebSocket, { - identity: { - type: 'user', - ownershipEntityRefs: ['user:default/john.doe'], - userEntityRef: 'user:default/john.doe', - }, - expiresInSeconds: 3600, - token: '1234', + ownershipEntityRefs: ['user:default/john.doe'], + userEntityRef: 'user:default/john.doe', }); // Connection without subscription const ws3 = new MockWebSocket(); manager.addConnection(ws3 as unknown as WebSocket, { - identity: { - type: 'user', - ownershipEntityRefs: ['user:default/john.doe'], - userEntityRef: 'user:default/john.doe', - }, - expiresInSeconds: 3600, - token: '1234', + ownershipEntityRefs: ['user:default/john.doe'], + userEntityRef: 'user:default/john.doe', }); ws1.trigger( diff --git a/plugins/signals-backend/src/service/SignalManager.ts b/plugins/signals-backend/src/service/SignalManager.ts index db374418e1..c31c7869bd 100644 --- a/plugins/signals-backend/src/service/SignalManager.ts +++ b/plugins/signals-backend/src/service/SignalManager.ts @@ -18,8 +18,10 @@ import { SignalPayload } from '@backstage/plugin-signals-node'; import { RawData, WebSocket } from 'ws'; import { v4 as uuid } from 'uuid'; import { JsonObject } from '@backstage/types'; -import { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; -import { LoggerService } from '@backstage/backend-plugin-api'; +import { + BackstageUserInfo, + LoggerService, +} from '@backstage/backend-plugin-api'; /** * @internal @@ -64,14 +66,14 @@ export class SignalManager { }); } - addConnection(ws: WebSocket, identity?: BackstageIdentityResponse) { + addConnection(ws: WebSocket, identity?: BackstageUserInfo) { const id = uuid(); const conn = { id, - user: identity?.identity.userEntityRef ?? 'user:default/guest', + user: identity?.userEntityRef ?? 'user:default/guest', ws, - ownershipEntityRefs: identity?.identity.ownershipEntityRefs ?? [ + ownershipEntityRefs: identity?.ownershipEntityRefs ?? [ 'user:default/guest', ], subscriptions: new Set(), @@ -80,7 +82,7 @@ export class SignalManager { this.connections.set(id, conn); ws.on('error', (err: Error) => { - this.logger.info( + this.logger.error( `Error occurred with connection ${id}: ${err}, closing connection`, ); ws.close(); diff --git a/plugins/signals-backend/src/service/router.test.ts b/plugins/signals-backend/src/service/router.test.ts index 807b45d53e..64367ee779 100644 --- a/plugins/signals-backend/src/service/router.test.ts +++ b/plugins/signals-backend/src/service/router.test.ts @@ -23,6 +23,7 @@ import request from 'supertest'; import { createRouter } from './router'; import { EventBroker } from '@backstage/plugin-events-node'; import { IdentityApi } from '@backstage/plugin-auth-node'; +import { UserInfoService } from '@backstage/backend-plugin-api'; const eventBrokerMock: jest.Mocked = { subscribe: jest.fn(), @@ -38,6 +39,10 @@ const discovery: jest.Mocked = { getExternalBaseUrl: jest.fn(), }; +const userInfo: jest.Mocked = { + getUserInfo: jest.fn(), +}; + describe('createRouter', () => { let app: express.Express; @@ -47,6 +52,7 @@ describe('createRouter', () => { identity: identityApiMock, eventBroker: eventBrokerMock, discovery, + userInfo, }); app = express().use(router); }); diff --git a/plugins/signals-backend/src/service/router.ts b/plugins/signals-backend/src/service/router.ts index cc8fe9b851..5a6f4469b2 100644 --- a/plugins/signals-backend/src/service/router.ts +++ b/plugins/signals-backend/src/service/router.ts @@ -14,22 +14,25 @@ * limitations under the License. */ import { + createLegacyAuthAdapters, errorHandler, PluginEndpointDiscovery, } from '@backstage/backend-common'; import express, { NextFunction, Request, Response } from 'express'; import Router from 'express-promise-router'; -import { LoggerService } from '@backstage/backend-plugin-api'; +import { + AuthService, + BackstageUserInfo, + LoggerService, + UserInfoService, +} from '@backstage/backend-plugin-api'; import * as https from 'https'; import http, { IncomingMessage } from 'http'; import { SignalManager } from './SignalManager'; -import { - BackstageIdentityResponse, - IdentityApi, - IdentityApiGetIdentityRequest, -} from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { EventBroker } from '@backstage/plugin-events-node'; import { WebSocket, WebSocketServer } from 'ws'; +import { Duplex } from 'stream'; /** @public */ export interface RouterOptions { @@ -37,21 +40,85 @@ export interface RouterOptions { eventBroker?: EventBroker; identity: IdentityApi; discovery: PluginEndpointDiscovery; + auth?: AuthService; + userInfo?: UserInfoService; } /** @public */ export async function createRouter( options: RouterOptions, ): Promise { - const { logger, identity, discovery } = options; + const { logger, discovery } = options; + const { auth, userInfo } = createLegacyAuthAdapters(options); + const manager = SignalManager.create(options); let subscribedToUpgradeRequests = false; + let apiUrl: string | undefined = undefined; const webSocketServer = new WebSocketServer({ noServer: true, clientTracking: false, }); + webSocketServer.on('error', (error: Error) => { + logger.error('WebSocket server error', error); + }); + + webSocketServer.on('close', () => { + logger.info('WebSocket server closed'); + }); + + const handleUpgrade = async ( + request: Request, + socket: Duplex, + head: Buffer, + ) => { + if (!apiUrl) { + apiUrl = await discovery.getBaseUrl('signals'); + } + + if (!request.url || !apiUrl || !apiUrl.endsWith(request.url)) { + return; + } + + let userIdentity: BackstageUserInfo | undefined = undefined; + + // Authentication token is passed in Sec-WebSocket-Protocol header as there + // is no other way to pass the token with plain websockets + try { + const token = request.headers['sec-websocket-protocol']; + if (token) { + const credentials = await auth.authenticate(token); + if (auth.isPrincipal(credentials, 'user')) { + userIdentity = await userInfo.getUserInfo(credentials); + } + } + } catch (e) { + logger.error('Failed to authenticate WebSocket connection', e); + socket.write( + 'HTTP/1.1 401 Web Socket Protocol Handshake\r\n' + + 'Upgrade: WebSocket\r\n' + + 'Connection: Upgrade\r\n' + + '\r\n', + ); + socket.destroy(); + return; + } + + try { + webSocketServer.handleUpgrade( + request, + socket, + head, + (ws: WebSocket, __: IncomingMessage) => { + manager.addConnection(ws, userIdentity); + }, + ); + } catch (e) { + logger.error('Failed to handle WebSocket upgrade', e); + } + }; + const upgradeMiddleware = async ( req: Request, _: Response, @@ -70,34 +137,7 @@ export async function createRouter( } subscribedToUpgradeRequests = true; - const apiUrl = await discovery.getBaseUrl('signals'); - server.on('upgrade', async (request, socket, head) => { - if (!request.url || !apiUrl.endsWith(request.url)) { - return; - } - - let userIdentity: BackstageIdentityResponse | undefined = undefined; - - // Authentication token is passed in Sec-WebSocket-Protocol header as there - // is no other way to pass the token with plain websockets - const token = req.headers['sec-websocket-protocol']; - if (token) { - userIdentity = await identity.getIdentity({ - request: { - headers: { authorization: token }, - }, - } as IdentityApiGetIdentityRequest); - } - - webSocketServer.handleUpgrade( - request, - socket, - head, - (ws: WebSocket, __: IncomingMessage) => { - manager.addConnection(ws, userIdentity); - }, - ); - }); + server.on('upgrade', handleUpgrade); }; const router = Router(); diff --git a/plugins/signals-backend/src/service/standaloneServer.ts b/plugins/signals-backend/src/service/standaloneServer.ts index 00ef7b5c77..f88848fffc 100644 --- a/plugins/signals-backend/src/service/standaloneServer.ts +++ b/plugins/signals-backend/src/service/standaloneServer.ts @@ -28,6 +28,11 @@ import { EventParams, EventSubscriber, } from '@backstage/plugin-events-node'; +import { + BackstageCredentials, + BackstageUserInfo, + UserInfoService, +} from '@backstage/backend-plugin-api'; export interface ServerOptions { port: number; @@ -64,11 +69,21 @@ export async function startStandaloneServer( eventBroker, }); + const userInfo: UserInfoService = { + async getUserInfo(_: BackstageCredentials): Promise { + return { + userEntityRef: 'user:default/guest', + ownershipEntityRefs: ['user:default/guest'], + }; + }, + }; + const router = await createRouter({ logger, identity, eventBroker, discovery, + userInfo, }); let service = createServiceBuilder(module)