From 4f8ecd65f974500ac43937e03d79868a7af3814d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 16 Feb 2024 13:39:07 +0100 Subject: [PATCH] entity-feedback-backend: migrated to support 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/fast-buses-exercise.md | 5 ++ plugins/entity-feedback-backend/api-report.md | 6 ++ .../src/service/router.test.ts | 86 ++++++++++--------- .../src/service/router.ts | 56 ++++++------ 4 files changed, 84 insertions(+), 69 deletions(-) create mode 100644 .changeset/fast-buses-exercise.md diff --git a/.changeset/fast-buses-exercise.md b/.changeset/fast-buses-exercise.md new file mode 100644 index 0000000000..def8ce2ca6 --- /dev/null +++ b/.changeset/fast-buses-exercise.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-entity-feedback-backend': patch +--- + +Migrated to support new auth services. diff --git a/plugins/entity-feedback-backend/api-report.md b/plugins/entity-feedback-backend/api-report.md index 1ec466e256..573d56efb5 100644 --- a/plugins/entity-feedback-backend/api-report.md +++ b/plugins/entity-feedback-backend/api-report.md @@ -3,8 +3,10 @@ > 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 express from 'express'; +import { HttpAuthService } from '@backstage/backend-plugin-api'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { Logger } from 'winston'; import { PluginDatabaseManager } from '@backstage/backend-common'; @@ -19,11 +21,15 @@ export default entityFeedbackPlugin; // @public (undocumented) export interface RouterOptions { + // (undocumented) + auth?: AuthService; // (undocumented) database: PluginDatabaseManager; // (undocumented) discovery: PluginEndpointDiscovery; // (undocumented) + httpAuth?: HttpAuthService; + // (undocumented) identity: IdentityApi; // (undocumented) logger: Logger; diff --git a/plugins/entity-feedback-backend/src/service/router.test.ts b/plugins/entity-feedback-backend/src/service/router.test.ts index 5df0c5cf9d..73940d96df 100644 --- a/plugins/entity-feedback-backend/src/service/router.test.ts +++ b/plugins/entity-feedback-backend/src/service/router.test.ts @@ -21,11 +21,11 @@ import { } from '@backstage/backend-common'; import { stringifyEntityRef } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; -import { IdentityApi } from '@backstage/plugin-auth-node'; import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; const sampleOwnedEntities = [ { @@ -65,7 +65,7 @@ const sampleEntities = [ }, ]; -const mockGetEntties = jest +const mockGetEntities = jest .fn() .mockImplementation(async () => ({ items: sampleOwnedEntities })); @@ -75,15 +75,11 @@ const mockGetEnttiesByRefs = jest jest.mock('@backstage/catalog-client', () => ({ CatalogClient: jest.fn().mockImplementation(() => ({ - getEntities: mockGetEntties, + getEntities: mockGetEntities, getEntitiesByRefs: mockGetEnttiesByRefs, })), })); -jest.mock('@backstage/plugin-auth-node', () => ({ - getBearerTokenFromAuthorizationHeader: () => 'token', -})); - const mockRatings = [ { userRef: 'user:default/foo', rating: 'LIKE' }, { userRef: 'user:default/bar', rating: 'LIKE' }, @@ -149,12 +145,6 @@ describe('createRouter', () => { }), ).forPlugin('entity-feedback'); - const mockIdentityClient = { - getIdentity: jest.fn().mockImplementation(async () => ({ - identity: { userEntityRef: 'user:default/me' }, - })), - } as unknown as IdentityApi; - const discovery: jest.Mocked = { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), @@ -164,8 +154,10 @@ describe('createRouter', () => { const router = await createRouter({ database: createDatabase(), discovery, - identity: mockIdentityClient, + identity: mockServices.identity(), logger: getVoidLogger(), + auth: mockServices.auth(), + httpAuth: mockServices.httpAuth(), }); app = express().use(router); @@ -174,44 +166,38 @@ describe('createRouter', () => { describe('GET /ratings', () => { it('should get ratings for all entities correctly', async () => { - const response = await request(app).get('/ratings').send(); + const response = await request(app) + .get('/ratings') + .set('authorization', mockCredentials.user.header()) + .send(); + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + entityRef: 'component:default/foo', + entityTitle: 'Foo Component', + ratings: { LIKE: 3, DISLIKE: 1 }, + }, + { + entityRef: 'component:default/bar', + entityTitle: 'Bar Component', + ratings: { LIKE: 5 }, + }, + ]); expect(mockDbHandler.getAllRatedEntities).toHaveBeenCalled(); expect(mockDbHandler.getRatingsAggregates).toHaveBeenCalledWith( sampleEntities .filter(Boolean) .map((ent: any) => stringifyEntityRef(ent)), ); - expect(response.status).toEqual(200); - expect(response.body).toEqual([ - { - entityRef: 'component:default/foo', - entityTitle: 'Foo Component', - ratings: { LIKE: 3, DISLIKE: 1 }, - }, - { - entityRef: 'component:default/bar', - entityTitle: 'Bar Component', - ratings: { LIKE: 5 }, - }, - ]); }); it('should get ratings for all owned entities correctly', async () => { const response = await request(app) .get('/ratings?ownerRef=group:default/test-team') + .set('authorization', mockCredentials.user.header()) .send(); - expect(mockGetEntties).toHaveBeenCalledWith( - expect.objectContaining({ - filter: { 'relations.ownedBy': 'group:default/test-team' }, - }), - { token: 'token' }, - ); - expect(mockDbHandler.getAllRatedEntities).not.toHaveBeenCalled(); - expect(mockDbHandler.getRatingsAggregates).toHaveBeenCalledWith( - sampleOwnedEntities.map((ent: any) => stringifyEntityRef(ent)), - ); expect(response.status).toEqual(200); expect(response.body).toEqual([ { @@ -225,6 +211,21 @@ describe('createRouter', () => { ratings: { LIKE: 5 }, }, ]); + expect(mockGetEntities).toHaveBeenCalledWith( + expect.objectContaining({ + filter: { 'relations.ownedBy': 'group:default/test-team' }, + }), + { + token: mockCredentials.service.token({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'catalog', + }), + }, + ); + expect(mockDbHandler.getAllRatedEntities).not.toHaveBeenCalled(); + expect(mockDbHandler.getRatingsAggregates).toHaveBeenCalledWith( + sampleOwnedEntities.map((ent: any) => stringifyEntityRef(ent)), + ); }); }); @@ -233,10 +234,11 @@ describe('createRouter', () => { const body = { rating: 'LIKE' }; const response = await request(app) .post('/ratings/component%3Adefault%2Fservice') + .set('authorization', mockCredentials.user.header()) .send(body); expect(mockDbHandler.recordRating).toHaveBeenCalledWith({ entityRef: 'component:default/service', - userRef: 'user:default/me', + userRef: 'user:default/mock', ...body, }); expect(response.status).toEqual(201); @@ -247,6 +249,7 @@ describe('createRouter', () => { it('should get ratings for an entity correctly', async () => { const response = await request(app) .get('/ratings/component%3Adefault%2Fservice') + .set('authorization', mockCredentials.user.header()) .send(); expect(mockDbHandler.getRatings).toHaveBeenCalledWith( 'component:default/service', @@ -262,6 +265,7 @@ describe('createRouter', () => { it('should get aggregated ratings for an entity correctly', async () => { const response = await request(app) .get('/ratings/component%3Adefault%2Fservice/aggregate') + .set('authorization', mockCredentials.user.header()) .send(); expect(mockDbHandler.getRatings).toHaveBeenCalledWith( 'component:default/service', @@ -279,10 +283,11 @@ describe('createRouter', () => { const body = { response: 'blah', comments: 'feedback', consent: true }; const response = await request(app) .post('/responses/component%3Adefault%2Fservice') + .set('authorization', mockCredentials.user.header()) .send(body); expect(mockDbHandler.recordResponse).toHaveBeenCalledWith({ entityRef: 'component:default/service', - userRef: 'user:default/me', + userRef: 'user:default/mock', ...body, }); expect(response.status).toEqual(201); @@ -293,6 +298,7 @@ describe('createRouter', () => { it('should get responses for an entity correctly', async () => { const response = await request(app) .get('/responses/component%3Adefault%2Fservice') + .set('authorization', mockCredentials.user.header()) .send(); expect(mockDbHandler.getResponses).toHaveBeenCalledWith( 'component:default/service', diff --git a/plugins/entity-feedback-backend/src/service/router.ts b/plugins/entity-feedback-backend/src/service/router.ts index 4953c531b8..7cf89bfd5c 100644 --- a/plugins/entity-feedback-backend/src/service/router.ts +++ b/plugins/entity-feedback-backend/src/service/router.ts @@ -15,16 +15,15 @@ */ import { + createLegacyAuthAdapters, errorHandler, PluginDatabaseManager, PluginEndpointDiscovery, } from '@backstage/backend-common'; +import { AuthService, HttpAuthService } from '@backstage/backend-plugin-api'; import { CatalogClient } from '@backstage/catalog-client'; import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; -import { - getBearerTokenFromAuthorizationHeader, - IdentityApi, -} from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { EntityRatingsData, Ratings, @@ -43,6 +42,8 @@ export interface RouterOptions { discovery: PluginEndpointDiscovery; identity: IdentityApi; logger: Logger; + auth?: AuthService; + httpAuth?: HttpAuthService; } /** @@ -51,9 +52,10 @@ export interface RouterOptions { export async function createRouter( options: RouterOptions, ): Promise { - const { database, discovery, identity, logger } = options; + const { database, discovery, logger } = options; logger.info('Initializing Entity Feedback backend'); + const { auth, httpAuth } = createLegacyAuthAdapters(options); const catalogClient = new CatalogClient({ discoveryApi: discovery }); const db = await database.getClient(); @@ -63,9 +65,10 @@ export async function createRouter( router.use(express.json()); router.get('/ratings', async (req, res) => { - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await httpAuth.credentials(req), + targetPluginId: 'catalog', + }); const requestedEntities: { [ref: string]: Entity } = {}; if (req.query.ownerRef) { @@ -131,11 +134,12 @@ export async function createRouter( }); router.post('/ratings/:entityRef', async (req, res) => { - const user = await identity.getIdentity({ request: req }); + const credentials = await httpAuth.credentials(req, { allow: ['user'] }); + const rating = req.body.rating; - if (!user || !rating) { + if (!rating) { logger.warn( - `Can't save rating because there is not enough info: user=${user}, rating=${rating}`, + `Can't save rating because there is not enough info: user=${credentials.principal.userEntityRef}, rating=${rating}`, ); res.status(400).end(); return; @@ -144,7 +148,7 @@ export async function createRouter( await dbHandler.recordRating({ entityRef: req.params.entityRef, rating, - userRef: user.identity.userEntityRef, + userRef: credentials.principal.userEntityRef, }); res.status(201).end(); @@ -153,9 +157,10 @@ export async function createRouter( router.get('/ratings/:entityRef', async (req, res) => { const ratings = await dbHandler.getRatings(req.params.entityRef); - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await httpAuth.credentials(req), + targetPluginId: 'catalog', + }); // Filter ratings via user refs to only expose entity refs accessible by current user const accessibleEntityRefs = ( @@ -177,9 +182,7 @@ export async function createRouter( const entityRatings = ( await dbHandler.getRatings(req.params.entityRef) ).reduce((ratings: Ratings, { rating }) => { - ratings[rating] = ratings[rating] ?? 0; - ratings[rating]++; - + ratings[rating] = (ratings[rating] ?? 0) + 1; return ratings; }, {}); @@ -187,21 +190,15 @@ export async function createRouter( }); router.post('/responses/:entityRef', async (req, res) => { - const user = await identity.getIdentity({ request: req }); const { response, comments, consent } = req.body; - - if (!user) { - logger.warn(`Could not identify user to save responses, user=${user}`); - res.status(400).end(); - return; - } + const credentials = await httpAuth.credentials(req, { allow: ['user'] }); await dbHandler.recordResponse({ entityRef: req.params.entityRef, response, comments, consent, - userRef: user.identity.userEntityRef, + userRef: credentials.principal.userEntityRef, }); res.status(201).end(); @@ -210,9 +207,10 @@ export async function createRouter( router.get('/responses/:entityRef', async (req, res) => { const responses = await dbHandler.getResponses(req.params.entityRef); - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await httpAuth.credentials(req), + targetPluginId: 'catalog', + }); // Filter responses via user refs to only expose entity refs accessible by current user const accessibleEntityRefs = (