Merge pull request #23304 from drodil/signals_auth

feat(signals): take new auth in use for signals

.changeset/tender-apes-doubt.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-signals-backend': patch
+---
+
+Improved error logging and fixed authentication

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<express.Router>;
 
 // @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

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,
           }),
         );
       },

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(

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<string>(),
@@ -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();

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<EventBroker> = {
   subscribe: jest.fn(),
@@ -38,6 +39,10 @@ const discovery: jest.Mocked<PluginEndpointDiscovery> = {
   getExternalBaseUrl: jest.fn(),
 };
 
+const userInfo: jest.Mocked<UserInfoService> = {
+  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);
   });

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<express.Router> {
-  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<any, any, any, any, any>,
+    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();

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<BackstageUserInfo> {
+      return {
+        userEntityRef: 'user:default/guest',
+        ownershipEntityRefs: ['user:default/guest'],
+      };
+    },
+  };
+
   const router = await createRouter({
     logger,
     identity,
     eventBroker,
     discovery,
+    userInfo,
   });
 
   let service = createServiceBuilder(module)