diff --git a/.changeset/fair-cheetahs-suffer.md b/.changeset/fair-cheetahs-suffer.md new file mode 100644 index 0000000000..a41237acaa --- /dev/null +++ b/.changeset/fair-cheetahs-suffer.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-notifications-backend': patch +'@backstage/plugin-notifications-common': patch +'@backstage/plugin-notifications-node': patch +--- + +Move notification origin resolving to backend with new auth diff --git a/plugins/notifications-backend/src/plugin.ts b/plugins/notifications-backend/src/plugin.ts index 1d81818e30..29d444efd3 100644 --- a/plugins/notifications-backend/src/plugin.ts +++ b/plugins/notifications-backend/src/plugin.ts @@ -61,22 +61,20 @@ export const notificationsPlugin = createBackendPlugin({ deps: { auth: coreServices.auth, httpAuth: coreServices.httpAuth, + userInfo: coreServices.userInfo, httpRouter: coreServices.httpRouter, logger: coreServices.logger, - identity: coreServices.identity, database: coreServices.database, - tokenManager: coreServices.tokenManager, discovery: coreServices.discovery, signals: signalService, }, async init({ auth, httpAuth, + userInfo, httpRouter, logger, - identity, database, - tokenManager, discovery, signals, }) { @@ -84,10 +82,9 @@ export const notificationsPlugin = createBackendPlugin({ await createRouter({ auth, httpAuth, + userInfo, logger, - identity, database, - tokenManager, discovery, signalService: signals, processors: processingExtensions.processors, diff --git a/plugins/notifications-backend/src/service/router.test.ts b/plugins/notifications-backend/src/service/router.test.ts index 3152b1b07f..13cb5e2f54 100644 --- a/plugins/notifications-backend/src/service/router.test.ts +++ b/plugins/notifications-backend/src/service/router.test.ts @@ -17,16 +17,14 @@ import { DatabaseManager, getVoidLogger, PluginDatabaseManager, - PluginEndpointDiscovery, - TokenManager, } from '@backstage/backend-common'; import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; -import { IdentityApi } from '@backstage/plugin-auth-node'; import { ConfigReader } from '@backstage/config'; import { SignalService } from '@backstage/plugin-signals-node'; +import { mockServices } from '@backstage/backend-test-utils'; function createDatabase(): PluginDatabaseManager { return DatabaseManager.fromConfig( @@ -44,40 +42,24 @@ function createDatabase(): PluginDatabaseManager { describe('createRouter', () => { let app: express.Express; - const identityMock: IdentityApi = { - async getIdentity() { - return { - identity: { - type: 'user', - ownershipEntityRefs: [], - userEntityRef: 'user:default/guest', - }, - token: 'no-token', - }; - }, - }; - const mockedTokenManager: jest.Mocked = { - getToken: jest.fn(), - authenticate: jest.fn(), - }; - - const discovery: jest.Mocked = { - getBaseUrl: jest.fn(), - getExternalBaseUrl: jest.fn(), - }; - const signalService: jest.Mocked = { publish: jest.fn(), }; + const discovery = mockServices.discovery(); + const userInfo = mockServices.userInfo(); + const httpAuth = mockServices.httpAuth(); + const auth = mockServices.auth(); + beforeAll(async () => { const router = await createRouter({ logger: getVoidLogger(), - identity: identityMock, database: createDatabase(), - tokenManager: mockedTokenManager, discovery, signalService, + userInfo, + httpAuth, + auth, }); app = express().use(router); }); diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 78d44dcd44..221606e2da 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -13,15 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { - createLegacyAuthAdapters, - errorHandler, - PluginDatabaseManager, - TokenManager, -} from '@backstage/backend-common'; +import { errorHandler, PluginDatabaseManager } from '@backstage/backend-common'; import express, { Request } from 'express'; import Router from 'express-promise-router'; -import { IdentityApi } from '@backstage/plugin-auth-node'; import { DatabaseNotificationsStore, NotificationGetOptions, @@ -36,12 +30,13 @@ import { stringifyEntityRef, } from '@backstage/catalog-model'; import { NotificationProcessor } from '@backstage/plugin-notifications-node'; -import { AuthenticationError, InputError } from '@backstage/errors'; +import { InputError } from '@backstage/errors'; import { AuthService, DiscoveryService, HttpAuthService, LoggerService, + UserInfoService, } from '@backstage/backend-plugin-api'; import { SignalService } from '@backstage/plugin-signals-node'; import { @@ -53,15 +48,14 @@ import { /** @internal */ export interface RouterOptions { logger: LoggerService; - identity: IdentityApi; database: PluginDatabaseManager; - tokenManager: TokenManager; discovery: DiscoveryService; + auth: AuthService; + httpAuth: HttpAuthService; + userInfo: UserInfoService; signalService?: SignalService; catalog?: CatalogApi; processors?: NotificationProcessor[]; - auth?: AuthService; - httpAuth?: HttpAuthService; } /** @internal */ @@ -71,7 +65,9 @@ export async function createRouter( const { logger, database, - identity, + auth, + httpAuth, + userInfo, discovery, catalog, processors, @@ -82,14 +78,10 @@ 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) { - throw new AuthenticationError(); - } - return user.identity.userEntityRef; + const credentials = await httpAuth.credentials(req, { allow: ['user'] }); + const info = await userInfo.getUserInfo(credentials); + return info.userEntityRef; }; const getUsersForEntityRef = async ( @@ -277,18 +269,16 @@ export async function createRouter( }); // Add new notification - // Allowed only for service-to-service authentication, uses `getUsersForEntityRef` to retrieve recipients for - // specific entity reference router.post('/', async (req, res) => { - const { recipients, origin, payload } = req.body; + const { recipients, payload } = req.body; const notifications = []; let users = []; - await httpAuth.credentials(req, { allow: ['service'] }); + const credentials = await httpAuth.credentials(req, { allow: ['service'] }); - const { title, link, scope } = payload; + const { title, scope } = payload; - if (!recipients || !title || !origin || !link) { + if (!recipients || !title) { logger.error(`Invalid notification request received`); throw new InputError(); } @@ -305,6 +295,7 @@ export async function createRouter( throw new InputError(); } + const origin = credentials.principal.subject; const baseNotification: Omit = { payload: { ...payload, diff --git a/plugins/notifications-backend/src/service/standaloneServer.ts b/plugins/notifications-backend/src/service/standaloneServer.ts index c4927e8b38..817c35f739 100644 --- a/plugins/notifications-backend/src/service/standaloneServer.ts +++ b/plugins/notifications-backend/src/service/standaloneServer.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { + createLegacyAuthAdapters, createServiceBuilder, HostDiscovery, loadBackendConfig, @@ -37,6 +38,11 @@ import { EventsService, EventsServiceSubscribeOptions, } from '@backstage/plugin-events-node'; +import { + AuthService, + HttpAuthService, + UserInfoService, +} from '@backstage/backend-plugin-api'; export interface ServerOptions { port: number; @@ -107,15 +113,25 @@ export async function startStandaloneServer( }; const signalService = DefaultSignalService.create({ events }); + // TODO: Move to use services instead this hack + const { auth, httpAuth, userInfo } = createLegacyAuthAdapters< + any, + { auth: AuthService; httpAuth: HttpAuthService; userInfo: UserInfoService } + >({ + identity: identityMock, + tokenManager, + discovery, + }); const router = await createRouter({ logger, - identity: identityMock, database: dbMock, catalog: catalogApi, discovery, - tokenManager, signalService, + auth, + httpAuth, + userInfo, }); let service = createServiceBuilder(module) diff --git a/plugins/notifications-common/api-report.md b/plugins/notifications-common/api-report.md index 55c9b28489..bd01aa7f5a 100644 --- a/plugins/notifications-common/api-report.md +++ b/plugins/notifications-common/api-report.md @@ -27,7 +27,7 @@ export type NotificationPayload = { title: string; description?: string; link?: string; - severity: NotificationSeverity; + severity?: NotificationSeverity; topic?: string; scope?: string; icon?: string; diff --git a/plugins/notifications-common/src/types.ts b/plugins/notifications-common/src/types.ts index c338a772d7..2a5f803652 100644 --- a/plugins/notifications-common/src/types.ts +++ b/plugins/notifications-common/src/types.ts @@ -24,7 +24,7 @@ export type NotificationPayload = { link?: string; // TODO: Add support for additional links // additionalLinks?: string[]; - severity: NotificationSeverity; + severity?: NotificationSeverity; topic?: string; scope?: string; icon?: string; diff --git a/plugins/notifications-node/api-report.md b/plugins/notifications-node/api-report.md index 24ae31904c..1e0e5718d1 100644 --- a/plugins/notifications-node/api-report.md +++ b/plugins/notifications-node/api-report.md @@ -51,7 +51,6 @@ export const notificationService: ServiceRef; export type NotificationServiceOptions = { auth: AuthService; discovery: DiscoveryService; - pluginId: string; }; // @public (undocumented) diff --git a/plugins/notifications-node/src/lib.ts b/plugins/notifications-node/src/lib.ts index 2ba33608b4..3e0095f72c 100644 --- a/plugins/notifications-node/src/lib.ts +++ b/plugins/notifications-node/src/lib.ts @@ -31,13 +31,11 @@ export const notificationService = createServiceRef({ deps: { auth: coreServices.auth, discovery: coreServices.discovery, - pluginMetadata: coreServices.pluginMetadata, }, - factory({ auth, discovery, pluginMetadata }) { + factory({ auth, discovery }) { return DefaultNotificationService.create({ auth, discovery, - pluginId: pluginMetadata.getId(), }); }, }), diff --git a/plugins/notifications-node/src/service/DefaultNotificationService.test.ts b/plugins/notifications-node/src/service/DefaultNotificationService.test.ts index 9d8f5b82d9..4711f3de18 100644 --- a/plugins/notifications-node/src/service/DefaultNotificationService.test.ts +++ b/plugins/notifications-node/src/service/DefaultNotificationService.test.ts @@ -42,7 +42,6 @@ describe('DefaultNotificationService', () => { service = DefaultNotificationService.create({ auth, discovery, - pluginId: 'test', }); }); @@ -58,7 +57,7 @@ describe('DefaultNotificationService', () => { `${await discovery.getBaseUrl('notifications')}/`, async (req, res, ctx) => { const json = await req.json(); - expect(json).toEqual({ ...body, origin: 'plugin-test' }); + expect(json).toEqual(body); expect(req.headers.get('Authorization')).toBe( mockCredentials.service.header({ onBehalfOf: await auth.getOwnServiceCredentials(), @@ -83,7 +82,7 @@ describe('DefaultNotificationService', () => { `${await discovery.getBaseUrl('notifications')}/`, async (req, res, ctx) => { const json = await req.json(); - expect(json).toEqual({ ...body, origin: 'plugin-test' }); + expect(json).toEqual(body); expect(req.headers.get('Authorization')).toBe( mockCredentials.service.header({ onBehalfOf: await auth.getOwnServiceCredentials(), diff --git a/plugins/notifications-node/src/service/DefaultNotificationService.ts b/plugins/notifications-node/src/service/DefaultNotificationService.ts index ef06133d36..bd7712ff6f 100644 --- a/plugins/notifications-node/src/service/DefaultNotificationService.ts +++ b/plugins/notifications-node/src/service/DefaultNotificationService.ts @@ -22,7 +22,6 @@ import { NotificationPayload } from '@backstage/plugin-notifications-common'; export type NotificationServiceOptions = { auth: AuthService; discovery: DiscoveryService; - pluginId: string; }; /** @public */ @@ -45,17 +44,12 @@ export class DefaultNotificationService implements NotificationService { private constructor( private readonly discovery: DiscoveryService, private readonly auth: AuthService, - private readonly pluginId: string, ) {} static create( options: NotificationServiceOptions, ): DefaultNotificationService { - return new DefaultNotificationService( - options.discovery, - options.auth, - options.pluginId, - ); + return new DefaultNotificationService(options.discovery, options.auth); } async send(notification: NotificationSendOptions): Promise { @@ -65,13 +59,10 @@ export class DefaultNotificationService implements NotificationService { onBehalfOf: await this.auth.getOwnServiceCredentials(), targetPluginId: 'notifications', }); + const response = await fetch(`${baseUrl}/`, { method: 'POST', - body: JSON.stringify({ - ...notification, - // TODO: Should retrieve this in the backend from service auth instead - origin: `plugin-${this.pluginId}`, - }), + body: JSON.stringify(notification), headers: { 'Content-Type': 'application/json', Accept: 'application/json',