paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 22 Packages Projects Releases Wiki Activity

Merge pull request #23059 from backstage/rugvip/pr/f1b7c9

Browse Source 
playlist-backend: migrate to support new auth services
This commit is contained in:
Patrik Oldsberg
2024-02-27 09:09:46 +01:00
committed by GitHub
parent d91eb8c619 19e3a21e5f
commit 7da7685eb8
5 changed files with 89 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/eight-fireants-crash.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-playlist-backend': patch
---
Migrated to support new auth services.
plugins/playlist-backend/api-report.md
+8 -2
View File
@@ -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;
}
```
plugins/playlist-backend/src/service/DatabaseHandler.ts
+5 -5
View File
@@ -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<Playlist[]> {
let playlistQuery = this.database<Omit<Playlist, 'isFollowing'>>(
@@ -177,7 +177,7 @@ export class DatabaseHandler {
async getPlaylist(
id: string,
user?: BackstageUserIdentity,
user?: BackstageUserPrincipal,
): Promise<Playlist | undefined> {
const playlist = await this.database<Omit<Playlist, 'isFollowing'>>(
'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();
plugins/playlist-backend/src/service/router.test.ts
+48 -49
View File
@@ -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<PluginEndpointDiscovery> = {
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);
});
plugins/playlist-backend/src/service/router.ts
+23 -22
View File
@@ -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)
//