diff --git a/.changeset/eight-fireants-crash.md b/.changeset/eight-fireants-crash.md new file mode 100644 index 0000000000..a4eb903585 --- /dev/null +++ b/.changeset/eight-fireants-crash.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-playlist-backend': patch +--- + +Migrated to support new auth services. diff --git a/plugins/playlist-backend/api-report.md b/plugins/playlist-backend/api-report.md index 40b05c25d4..0a6aa3e3bf 100644 --- a/plugins/playlist-backend/api-report.md +++ b/plugins/playlist-backend/api-report.md @@ -3,19 +3,21 @@ > 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 { BackstageIdentityResponse } from '@backstage/plugin-auth-node'; import { ConditionalPolicyDecision } from '@backstage/plugin-permission-common'; import { Conditions } from '@backstage/plugin-permission-node'; import express from 'express'; +import { HttpAuthService } from '@backstage/backend-plugin-api'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { Logger } from 'winston'; import { Permission } from '@backstage/plugin-permission-common'; import { PermissionCondition } from '@backstage/plugin-permission-common'; import { PermissionCriteria } from '@backstage/plugin-permission-common'; -import { PermissionEvaluator } from '@backstage/plugin-permission-common'; import { PermissionPolicy } from '@backstage/plugin-permission-node'; import { PermissionRule } from '@backstage/plugin-permission-node'; +import { PermissionsService } from '@backstage/backend-plugin-api'; import { PlaylistMetadata } from '@backstage/plugin-playlist-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; @@ -87,15 +89,19 @@ export default playlistPlugin; // @public (undocumented) export interface RouterOptions { + // (undocumented) + auth?: AuthService; // (undocumented) database: PluginDatabaseManager; // (undocumented) discovery: PluginEndpointDiscovery; // (undocumented) + httpAuth?: HttpAuthService; + // (undocumented) identity: IdentityApi; // (undocumented) logger: Logger; // (undocumented) - permissions: PermissionEvaluator; + permissions: PermissionsService; } ``` diff --git a/plugins/playlist-backend/src/service/DatabaseHandler.ts b/plugins/playlist-backend/src/service/DatabaseHandler.ts index 6118f22ff9..afcaf92d24 100644 --- a/plugins/playlist-backend/src/service/DatabaseHandler.ts +++ b/plugins/playlist-backend/src/service/DatabaseHandler.ts @@ -15,7 +15,7 @@ */ import { resolvePackagePath } from '@backstage/backend-common'; -import { BackstageUserIdentity } from '@backstage/plugin-auth-node'; +import { BackstageUserPrincipal } from '@backstage/backend-plugin-api'; import { Playlist, PlaylistMetadata } from '@backstage/plugin-playlist-common'; import { Knex } from 'knex'; import { v4 as uuid } from 'uuid'; @@ -108,7 +108,7 @@ export class DatabaseHandler { private playlistColumns = ['id', 'name', 'description', 'owner', 'public']; async listPlaylists( - user: BackstageUserIdentity, + user: BackstageUserPrincipal, filter?: ListPlaylistsFilter, ): Promise { let playlistQuery = this.database>( @@ -177,7 +177,7 @@ export class DatabaseHandler { async getPlaylist( id: string, - user?: BackstageUserIdentity, + user?: BackstageUserPrincipal, ): Promise { const playlist = await this.database>( 'playlists', @@ -263,14 +263,14 @@ export class DatabaseHandler { .del(); } - async followPlaylist(playlistId: string, user: BackstageUserIdentity) { + async followPlaylist(playlistId: string, user: BackstageUserPrincipal) { await this.database('followers') .insert({ playlist_id: playlistId, user_ref: user.userEntityRef }) .onConflict(['playlist_id', 'user_ref']) .ignore(); } - async unfollowPlaylist(playlistId: string, user: BackstageUserIdentity) { + async unfollowPlaylist(playlistId: string, user: BackstageUserPrincipal) { await this.database('followers') .where({ playlist_id: playlistId, user_ref: user.userEntityRef }) .del(); diff --git a/plugins/playlist-backend/src/service/router.test.ts b/plugins/playlist-backend/src/service/router.test.ts index 3f90a8b8af..d28eae9041 100644 --- a/plugins/playlist-backend/src/service/router.test.ts +++ b/plugins/playlist-backend/src/service/router.test.ts @@ -20,13 +20,13 @@ import { PluginEndpointDiscovery, } from '@backstage/backend-common'; import { ConfigReader } from '@backstage/config'; -import { IdentityApi } from '@backstage/plugin-auth-node'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { permissions } from '@backstage/plugin-playlist-common'; import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; +import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; const sampleEntities = [ { @@ -60,10 +60,6 @@ jest.mock('@backstage/catalog-client', () => ({ .mockImplementation(() => ({ getEntities: mockGetEntties })), })); -jest.mock('@backstage/plugin-auth-node', () => ({ - getBearerTokenFromAuthorizationHeader: () => 'token', -})); - const mockConditionFilter = { key: 'test', values: ['test-val'] }; jest.mock('../permissions', () => ({ ...jest.requireActual('../permissions'), @@ -128,17 +124,6 @@ describe('createRouter', () => { authorizeConditional: mockedAuthorizeConditional, }; - const mockUser = { - type: 'user', - ownershipEntityRefs: ['user:default/me', 'group:default/owner'], - userEntityRef: 'user:default/me', - }; - const mockIdentityClient = { - getIdentity: jest - .fn() - .mockImplementation(async () => ({ identity: mockUser })), - } as unknown as IdentityApi; - const discovery: jest.Mocked = { getBaseUrl: jest.fn(), getExternalBaseUrl: jest.fn(), @@ -148,9 +133,11 @@ describe('createRouter', () => { const router = await createRouter({ database: createDatabase(), discovery, - identity: mockIdentityClient, + identity: mockServices.identity(), logger: getVoidLogger(), permissions: mockPermissionEvaluator, + auth: mockServices.auth(), + httpAuth: mockServices.httpAuth(), }); app = express().use(router); @@ -173,7 +160,7 @@ describe('createRouter', () => { expect(mockedAuthorizeConditional).toHaveBeenCalledWith( [{ permission: permissions.playlistListRead }], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.listPlaylists).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -182,7 +169,7 @@ describe('createRouter', () => { it('should get playlists correctly', async () => { let response = await request(app).get('/').send(); expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( - mockUser, + mockCredentials.user().principal, undefined, ); expect(response.status).toEqual(200); @@ -193,7 +180,7 @@ describe('createRouter', () => { ]); response = await request(app).get('/').send(); expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( - mockUser, + mockCredentials.user().principal, mockConditionFilter, ); expect(response.status).toEqual(200); @@ -205,7 +192,7 @@ describe('createRouter', () => { .get('/?filter=mock=test&filter=foo=bar') .send(); expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( - mockUser, + mockCredentials.user().principal, mockRequestFilter, ); expect(response.status).toEqual(200); @@ -217,9 +204,12 @@ describe('createRouter', () => { response = await request(app) .get('/?filter=mock=test&filter=foo=bar') .send(); - expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, { - allOf: [mockRequestFilter, mockConditionFilter], - }); + expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( + mockCredentials.user().principal, + { + allOf: [mockRequestFilter, mockConditionFilter], + }, + ); expect(response.status).toEqual(200); expect(response.body).toEqual([mockPlaylist]); }); @@ -228,10 +218,10 @@ describe('createRouter', () => { let response = await request(app).get('/?editable=true').send(); expect(mockedAuthorizeConditional).toHaveBeenCalledWith( [{ permission: permissions.playlistListUpdate }], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( - mockUser, + mockCredentials.user().principal, undefined, ); expect(response.status).toEqual(200); @@ -241,7 +231,7 @@ describe('createRouter', () => { .get('/?editable=true&filter=mock=test&filter=foo=bar') .send(); expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( - mockUser, + mockCredentials.user().principal, mockRequestFilter, ); expect(response.status).toEqual(200); @@ -251,9 +241,10 @@ describe('createRouter', () => { { result: AuthorizeResult.CONDITIONAL }, ]); response = await request(app).get('/?editable=true').send(); - expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, { - allOf: [mockConditionFilter, mockConditionFilter], - }); + expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( + mockCredentials.user().principal, + { allOf: [mockConditionFilter, mockConditionFilter] }, + ); expect(response.status).toEqual(200); expect(response.body).toEqual([mockPlaylist]); @@ -263,12 +254,15 @@ describe('createRouter', () => { response = await request(app) .get('/?editable=true&filter=mock=test&filter=foo=bar') .send(); - expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith(mockUser, { - allOf: [ - { allOf: [mockRequestFilter, mockConditionFilter] }, - mockConditionFilter, - ], - }); + expect(mockDbHandler.listPlaylists).toHaveBeenLastCalledWith( + mockCredentials.user().principal, + { + allOf: [ + { allOf: [mockRequestFilter, mockConditionFilter] }, + mockConditionFilter, + ], + }, + ); expect(response.status).toEqual(200); expect(response.body).toEqual([mockPlaylist]); }); @@ -285,7 +279,7 @@ describe('createRouter', () => { expect(mockedAuthorize).toHaveBeenCalledWith( [{ permission: permissions.playlistListCreate }], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.createPlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -313,7 +307,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.getPlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -323,7 +317,7 @@ describe('createRouter', () => { const response = await request(app).get('/playlist-id').send(); expect(mockDbHandler.getPlaylist).toHaveBeenCalledWith( 'playlist-id', - mockUser, + mockCredentials.user().principal, ); expect(response.status).toEqual(200); expect(response.body).toEqual(mockPlaylist); @@ -346,7 +340,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.updatePlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -375,7 +369,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.deletePlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -404,7 +398,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.addPlaylistEntities).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -436,7 +430,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.getPlaylistEntities).not.toHaveBeenCalled(); expect(mockGetEntties).not.toHaveBeenCalled(); @@ -463,7 +457,12 @@ describe('createRouter', () => { }, ], }, - { token: 'token' }, + { + token: mockCredentials.service.token({ + onBehalfOf: mockCredentials.user(), + targetPluginId: 'catalog', + }), + }, ); expect(response.status).toEqual(200); expect(response.body).toEqual(sampleEntities); @@ -486,7 +485,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.removePlaylistEntities).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -518,7 +517,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.followPlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -528,7 +527,7 @@ describe('createRouter', () => { const response = await request(app).post('/playlist-id/followers').send(); expect(mockDbHandler.followPlaylist).toHaveBeenCalledWith( 'playlist-id', - mockUser, + mockCredentials.user().principal, ); expect(response.status).toEqual(200); }); @@ -550,7 +549,7 @@ describe('createRouter', () => { resourceRef: 'playlist-id', }, ], - { token: 'token' }, + { credentials: mockCredentials.user() }, ); expect(mockDbHandler.unfollowPlaylist).not.toHaveBeenCalled(); expect(response.status).toEqual(403); @@ -562,7 +561,7 @@ describe('createRouter', () => { .send(); expect(mockDbHandler.unfollowPlaylist).toHaveBeenCalledWith( 'playlist-id', - mockUser, + mockCredentials.user().principal, ); expect(response.status).toEqual(200); }); diff --git a/plugins/playlist-backend/src/service/router.ts b/plugins/playlist-backend/src/service/router.ts index 0397bff2ff..b73c7279f7 100644 --- a/plugins/playlist-backend/src/service/router.ts +++ b/plugins/playlist-backend/src/service/router.ts @@ -15,6 +15,7 @@ */ import { + createLegacyAuthAdapters, errorHandler, PluginDatabaseManager, PluginEndpointDiscovery, @@ -22,14 +23,10 @@ import { import { CatalogClient } from '@backstage/catalog-client'; import { parseEntityRef } from '@backstage/catalog-model'; import { NotAllowedError } from '@backstage/errors'; -import { - getBearerTokenFromAuthorizationHeader, - IdentityApi, -} from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { AuthorizePermissionRequest, AuthorizeResult, - PermissionEvaluator, QueryPermissionRequest, } from '@backstage/plugin-permission-common'; import { createPermissionIntegrationRouter } from '@backstage/plugin-permission-node'; @@ -44,6 +41,11 @@ import { Logger } from 'winston'; import { rules, transformConditions } from '../permissions'; import { DatabaseHandler } from './DatabaseHandler'; import { parseListPlaylistsFilterParams } from './ListPlaylistsFilter'; +import { + AuthService, + HttpAuthService, + PermissionsService, +} from '@backstage/backend-plugin-api'; /** * @public @@ -53,7 +55,9 @@ export interface RouterOptions { discovery: PluginEndpointDiscovery; identity: IdentityApi; logger: Logger; - permissions: PermissionEvaluator; + permissions: PermissionsService; + auth?: AuthService; + httpAuth?: HttpAuthService; } /** @@ -65,11 +69,12 @@ export async function createRouter( const { database, discovery, - identity, logger, permissions: permissionEvaluator, } = options; + const { auth, httpAuth } = createLegacyAuthAdapters(options); + logger.info('Initializing Playlist backend'); const catalogClient = new CatalogClient({ discoveryApi: discovery }); @@ -81,26 +86,21 @@ export async function createRouter( permission: AuthorizePermissionRequest | QueryPermissionRequest, conditional: boolean = false, ) => { - const token = getBearerTokenFromAuthorizationHeader( - request.header('authorization'), - ); - - const user = await identity.getIdentity({ request }); - if (!user) { - throw new NotAllowedError('Unauthorized'); - } + const credentials = await httpAuth.credentials(request, { + allow: ['user'], + }); const decision = conditional ? ( await permissionEvaluator.authorizeConditional( [permission as QueryPermissionRequest], - { token }, + { credentials }, ) )[0] : ( await permissionEvaluator.authorize( [permission as AuthorizePermissionRequest], - { token }, + { credentials }, ) )[0]; @@ -108,7 +108,7 @@ export async function createRouter( throw new NotAllowedError('Unauthorized'); } - return { decision, user: user.identity }; + return { decision, user: credentials.principal }; }; const permissionIntegrationRouter = createPermissionIntegrationRouter({ @@ -227,11 +227,12 @@ export async function createRouter( }; }); - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: await httpAuth.credentials(req), + targetPluginId: 'catalog', + }); - // TODO(kuanpg): entities in this playlist that no longer exist in the catalog will be + // TODO(kuangp): entities in this playlist that no longer exist in the catalog will be // excluded from this response, we need a way to clean up these orphaned refs potentially // via catalog events (https://github.com/backstage/backstage/issues/8219) //