From 84af36198715c71c1d7febdbcdf9cd7d82f0f967 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 14 Feb 2024 12:18:01 +0100 Subject: [PATCH] notifications-backend: migrated to use new auth services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Carl-Erik Bergström Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .changeset/rude-masks-tan.md | 6 ++ plugins/notifications-backend/src/plugin.ts | 7 ++ .../src/service/router.ts | 39 +++++------ plugins/notifications-node/api-report.md | 12 ++-- plugins/notifications-node/package.json | 1 + plugins/notifications-node/src/lib.ts | 6 +- .../DefaultNotificationService.test.ts | 64 +++++++++++-------- .../src/service/DefaultNotificationService.ts | 27 ++++---- yarn.lock | 1 + 9 files changed, 94 insertions(+), 69 deletions(-) create mode 100644 .changeset/rude-masks-tan.md diff --git a/.changeset/rude-masks-tan.md b/.changeset/rude-masks-tan.md new file mode 100644 index 0000000000..8c8d52da6b --- /dev/null +++ b/.changeset/rude-masks-tan.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-notifications-node': minor +'@backstage/plugin-notifications-backend': patch +--- + +Migrated to using the new auth services. diff --git a/plugins/notifications-backend/src/plugin.ts b/plugins/notifications-backend/src/plugin.ts index 1d8806cf49..1d81818e30 100644 --- a/plugins/notifications-backend/src/plugin.ts +++ b/plugins/notifications-backend/src/plugin.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { coreServices, createBackendPlugin, @@ -58,6 +59,8 @@ export const notificationsPlugin = createBackendPlugin({ env.registerInit({ deps: { + auth: coreServices.auth, + httpAuth: coreServices.httpAuth, httpRouter: coreServices.httpRouter, logger: coreServices.logger, identity: coreServices.identity, @@ -67,6 +70,8 @@ export const notificationsPlugin = createBackendPlugin({ signals: signalService, }, async init({ + auth, + httpAuth, httpRouter, logger, identity, @@ -77,6 +82,8 @@ export const notificationsPlugin = createBackendPlugin({ }) { httpRouter.use( await createRouter({ + auth, + httpAuth, logger, identity, database, diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 6c745e158e..aa03726dfb 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -14,16 +14,14 @@ * limitations under the License. */ import { + createLegacyAuthAdapters, errorHandler, PluginDatabaseManager, TokenManager, } from '@backstage/backend-common'; import express, { Request } from 'express'; import Router from 'express-promise-router'; -import { - getBearerTokenFromAuthorizationHeader, - IdentityApi, -} from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { DatabaseNotificationsStore, NotificationGetOptions, @@ -39,7 +37,12 @@ import { } from '@backstage/catalog-model'; import { NotificationProcessor } from '@backstage/plugin-notifications-node'; import { AuthenticationError, InputError } from '@backstage/errors'; -import { DiscoveryService, LoggerService } from '@backstage/backend-plugin-api'; +import { + AuthService, + DiscoveryService, + HttpAuthService, + LoggerService, +} from '@backstage/backend-plugin-api'; import { SignalService } from '@backstage/plugin-signals-node'; import { NewNotificationSignal, @@ -58,6 +61,8 @@ export interface RouterOptions { signalService?: SignalService; catalog?: CatalogApi; processors?: NotificationProcessor[]; + auth?: AuthService; + httpAuth?: HttpAuthService; } /** @internal */ @@ -70,7 +75,6 @@ export async function createRouter( identity, discovery, catalog, - tokenManager, processors, signalService, } = options; @@ -79,6 +83,8 @@ export async function createRouter( catalog ?? new CatalogClient({ discoveryApi: discovery }); const store = await DatabaseNotificationsStore.create({ database }); + const { auth, httpAuth } = createLegacyAuthAdapters(options); + const getUser = async (req: Request) => { const user = await identity.getIdentity({ request: req }); if (!user) { @@ -87,20 +93,13 @@ export async function createRouter( return user.identity.userEntityRef; }; - const authenticateService = async (req: Request) => { - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); - if (!token) { - throw new AuthenticationError(); - } - await tokenManager.authenticate(token); - }; - const getUsersForEntityRef = async ( entityRef: string | string[] | null, ): Promise => { - const { token } = await tokenManager.getToken(); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await auth.getOwnServiceCredentials(), + targetPluginId: 'catalog', + }); // TODO: Support for broadcast if (entityRef === null) { @@ -276,11 +275,7 @@ export async function createRouter( const notifications = []; let users = []; - try { - await authenticateService(req); - } catch (e) { - throw new AuthenticationError(); - } + await httpAuth.credentials(req, { allow: ['service'] }); const { title, link, scope } = payload; diff --git a/plugins/notifications-node/api-report.md b/plugins/notifications-node/api-report.md index 75a0fb68fd..24ae31904c 100644 --- a/plugins/notifications-node/api-report.md +++ b/plugins/notifications-node/api-report.md @@ -3,21 +3,19 @@ > 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 { DiscoveryService } from '@backstage/backend-plugin-api'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; import { Notification as Notification_2 } from '@backstage/plugin-notifications-common'; import { NotificationPayload } from '@backstage/plugin-notifications-common'; import { ServiceRef } from '@backstage/backend-plugin-api'; -import { TokenManager } from '@backstage/backend-common'; // @public (undocumented) export class DefaultNotificationService implements NotificationService { // (undocumented) - static create({ - tokenManager, - discovery, - pluginId, - }: NotificationServiceOptions): DefaultNotificationService; + static create( + options: NotificationServiceOptions, + ): DefaultNotificationService; // (undocumented) send(notification: NotificationSendOptions): Promise; } @@ -51,8 +49,8 @@ export const notificationService: ServiceRef; // @public (undocumented) export type NotificationServiceOptions = { + auth: AuthService; discovery: DiscoveryService; - tokenManager: TokenManager; pluginId: string; }; diff --git a/plugins/notifications-node/package.json b/plugins/notifications-node/package.json index ef3bc62ad2..9d2094beaf 100644 --- a/plugins/notifications-node/package.json +++ b/plugins/notifications-node/package.json @@ -27,6 +27,7 @@ "postpack": "backstage-cli package postpack" }, "devDependencies": { + "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "@backstage/test-utils": "workspace:^", "msw": "^1.0.0" diff --git a/plugins/notifications-node/src/lib.ts b/plugins/notifications-node/src/lib.ts index c79d871ff8..2ba33608b4 100644 --- a/plugins/notifications-node/src/lib.ts +++ b/plugins/notifications-node/src/lib.ts @@ -29,14 +29,14 @@ export const notificationService = createServiceRef({ createServiceFactory({ service, deps: { + auth: coreServices.auth, discovery: coreServices.discovery, - tokenManager: coreServices.tokenManager, pluginMetadata: coreServices.pluginMetadata, }, - factory({ discovery, tokenManager, pluginMetadata }) { + factory({ auth, discovery, pluginMetadata }) { return DefaultNotificationService.create({ + auth, discovery, - tokenManager, pluginId: pluginMetadata.getId(), }); }, diff --git a/plugins/notifications-node/src/service/DefaultNotificationService.test.ts b/plugins/notifications-node/src/service/DefaultNotificationService.test.ts index a9b204ca7f..9d8f5b82d9 100644 --- a/plugins/notifications-node/src/service/DefaultNotificationService.test.ts +++ b/plugins/notifications-node/src/service/DefaultNotificationService.test.ts @@ -22,6 +22,7 @@ import { DefaultNotificationService, NotificationSendOptions, } from './DefaultNotificationService'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; const server = setupServer(); @@ -33,21 +34,14 @@ const testNotification: NotificationPayload = { describe('DefaultNotificationService', () => { setupRequestMockHandlers(server); - const mockBaseUrl = 'http://backstage/api/notifications'; - const discoveryApi = { - getBaseUrl: async () => mockBaseUrl, - getExternalBaseUrl: async () => mockBaseUrl, - }; - const tokenManager = { - getToken: async () => ({ token: '1234' }), - authenticate: jest.fn(), - }; + const discovery = mockServices.discovery(); + const auth = mockServices.auth(); let service: DefaultNotificationService; - beforeEach(() => { + beforeEach(async () => { service = DefaultNotificationService.create({ - discovery: discoveryApi, - tokenManager, + auth, + discovery, pluginId: 'test', }); }); @@ -60,14 +54,22 @@ describe('DefaultNotificationService', () => { }; server.use( - rest.post(`${mockBaseUrl}/`, async (req, res, ctx) => { - const json = await req.json(); - expect(json).toEqual({ ...body, origin: 'plugin-test' }); - expect(req.headers.get('Authorization')).toEqual('Bearer 1234'); - return res(ctx.status(200)); - }), + rest.post( + `${await discovery.getBaseUrl('notifications')}/`, + async (req, res, ctx) => { + const json = await req.json(); + expect(json).toEqual({ ...body, origin: 'plugin-test' }); + expect(req.headers.get('Authorization')).toBe( + mockCredentials.service.header({ + onBehalfOf: await auth.getOwnServiceCredentials(), + targetPluginId: 'notifications', + }), + ); + return res(ctx.status(200)); + }, + ), ); - await expect(service.send(body)).resolves.not.toThrow(); + await expect(service.send(body)).resolves.toBeUndefined(); }); it('should throw error if failing', async () => { @@ -77,14 +79,24 @@ describe('DefaultNotificationService', () => { }; server.use( - rest.post(`${mockBaseUrl}/`, async (req, res, ctx) => { - const json = await req.json(); - expect(json).toEqual({ ...body, origin: 'plugin-test' }); - expect(req.headers.get('Authorization')).toEqual('Bearer 1234'); - return res(ctx.status(400)); - }), + rest.post( + `${await discovery.getBaseUrl('notifications')}/`, + async (req, res, ctx) => { + const json = await req.json(); + expect(json).toEqual({ ...body, origin: 'plugin-test' }); + expect(req.headers.get('Authorization')).toBe( + mockCredentials.service.header({ + onBehalfOf: await auth.getOwnServiceCredentials(), + targetPluginId: 'notifications', + }), + ); + return res(ctx.status(400)); + }, + ), + ); + await expect(service.send(body)).rejects.toThrow( + 'Request failed with status 400', ); - await expect(service.send(body)).rejects.toThrow(); }); }); }); diff --git a/plugins/notifications-node/src/service/DefaultNotificationService.ts b/plugins/notifications-node/src/service/DefaultNotificationService.ts index 7e5cf58f53..ef06133d36 100644 --- a/plugins/notifications-node/src/service/DefaultNotificationService.ts +++ b/plugins/notifications-node/src/service/DefaultNotificationService.ts @@ -13,15 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { TokenManager } from '@backstage/backend-common'; + import { NotificationService } from './NotificationService'; -import { DiscoveryService } from '@backstage/backend-plugin-api'; +import { AuthService, DiscoveryService } from '@backstage/backend-plugin-api'; import { NotificationPayload } from '@backstage/plugin-notifications-common'; /** @public */ export type NotificationServiceOptions = { + auth: AuthService; discovery: DiscoveryService; - tokenManager: TokenManager; pluginId: string; }; @@ -44,22 +44,27 @@ export type NotificationSendOptions = { export class DefaultNotificationService implements NotificationService { private constructor( private readonly discovery: DiscoveryService, - private readonly tokenManager: TokenManager, + private readonly auth: AuthService, private readonly pluginId: string, ) {} - static create({ - tokenManager, - discovery, - pluginId, - }: NotificationServiceOptions): DefaultNotificationService { - return new DefaultNotificationService(discovery, tokenManager, pluginId); + static create( + options: NotificationServiceOptions, + ): DefaultNotificationService { + return new DefaultNotificationService( + options.discovery, + options.auth, + options.pluginId, + ); } async send(notification: NotificationSendOptions): Promise { try { const baseUrl = await this.discovery.getBaseUrl('notifications'); - const { token } = await this.tokenManager.getToken(); + const { token } = await this.auth.getPluginRequestToken({ + onBehalfOf: await this.auth.getOwnServiceCredentials(), + targetPluginId: 'notifications', + }); const response = await fetch(`${baseUrl}/`, { method: 'POST', body: JSON.stringify({ diff --git a/yarn.lock b/yarn.lock index 53c220af6d..19415ada96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7664,6 +7664,7 @@ __metadata: dependencies: "@backstage/backend-common": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" + "@backstage/backend-test-utils": "workspace:^" "@backstage/catalog-client": "workspace:^" "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^"