refactor(catalog-backend): permissions cleanup step 1
Make `permissionsRegistry` required and `permissions` always a `PermissionsService` in `CatalogEnvironment`. Remove the deprecated `PermissionAuthorizer` fallback path and the `createPermissionIntegrationRouter` fallback — catalog now exclusively uses `permissionsRegistry.addResourceType` to register its permission resource type. Signed-off-by: Fredrik Adelöw <freben@spotify.com> Made-with: Cursor
This commit is contained in:
@@ -43,10 +43,7 @@ import {
|
||||
UrlReaderService,
|
||||
} from '@backstage/backend-plugin-api';
|
||||
import { Config, readDurationFromConfig } from '@backstage/config';
|
||||
import {
|
||||
catalogPermissions,
|
||||
RESOURCE_TYPE_CATALOG_ENTITY,
|
||||
} from '@backstage/plugin-catalog-common/alpha';
|
||||
import { catalogPermissions } from '@backstage/plugin-catalog-common/alpha';
|
||||
import {
|
||||
CatalogProcessor,
|
||||
CatalogProcessorParser,
|
||||
@@ -55,15 +52,8 @@ import {
|
||||
ScmLocationAnalyzer,
|
||||
} from '@backstage/plugin-catalog-node';
|
||||
import { EventsService } from '@backstage/plugin-events-node';
|
||||
import {
|
||||
Permission,
|
||||
PermissionAuthorizer,
|
||||
toPermissionEvaluator,
|
||||
} from '@backstage/plugin-permission-common';
|
||||
import {
|
||||
createConditionTransformer,
|
||||
createPermissionIntegrationRouter,
|
||||
} from '@backstage/plugin-permission-node';
|
||||
import { Permission } from '@backstage/plugin-permission-common';
|
||||
import { createConditionTransformer } from '@backstage/plugin-permission-node';
|
||||
import { durationToMilliseconds } from '@backstage/types';
|
||||
import { DefaultCatalogDatabase } from '../database/DefaultCatalogDatabase';
|
||||
import { DefaultProcessingDatabase } from '../database/DefaultProcessingDatabase';
|
||||
@@ -124,8 +114,8 @@ export type CatalogEnvironment = {
|
||||
database: DatabaseService;
|
||||
config: RootConfigService;
|
||||
reader: UrlReaderService;
|
||||
permissions: PermissionsService | PermissionAuthorizer;
|
||||
permissionsRegistry?: PermissionsRegistryService;
|
||||
permissions: PermissionsService;
|
||||
permissionsRegistry: PermissionsRegistryService;
|
||||
scheduler: SchedulerService;
|
||||
auth: AuthService;
|
||||
httpAuth: HttpAuthService;
|
||||
@@ -479,16 +469,6 @@ export class CatalogBuilder {
|
||||
enableRelationsCompatibility,
|
||||
});
|
||||
|
||||
let permissionsService: PermissionsService;
|
||||
if ('authorizeConditional' in permissions) {
|
||||
permissionsService = permissions as PermissionsService;
|
||||
} else {
|
||||
logger.warn(
|
||||
'PermissionAuthorizer is deprecated. Please use an instance of PermissionEvaluator instead of PermissionAuthorizer in PluginEnvironment#permissions',
|
||||
);
|
||||
permissionsService = toPermissionEvaluator(permissions);
|
||||
}
|
||||
|
||||
const orchestrator = new DefaultCatalogProcessingOrchestrator({
|
||||
processors,
|
||||
integrations,
|
||||
@@ -500,14 +480,12 @@ export class CatalogBuilder {
|
||||
|
||||
const entitiesCatalog = new AuthorizedEntitiesCatalog(
|
||||
unauthorizedEntitiesCatalog,
|
||||
permissionsService,
|
||||
permissionsRegistry
|
||||
? createConditionTransformer(
|
||||
permissionsRegistry.getPermissionRuleset(
|
||||
catalogEntityPermissionResourceRef,
|
||||
),
|
||||
)
|
||||
: createConditionTransformer(this.permissionRules),
|
||||
permissions,
|
||||
createConditionTransformer(
|
||||
permissionsRegistry.getPermissionRuleset(
|
||||
catalogEntityPermissionResourceRef,
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
const getResources = async (resourceRefs: string[]) => {
|
||||
@@ -519,24 +497,12 @@ export class CatalogBuilder {
|
||||
return entitiesResponseToObjects(items).map(e => e || undefined);
|
||||
};
|
||||
|
||||
let permissionIntegrationRouter:
|
||||
| ReturnType<typeof createPermissionIntegrationRouter>
|
||||
| undefined;
|
||||
if (permissionsRegistry) {
|
||||
permissionsRegistry.addResourceType({
|
||||
resourceRef: catalogEntityPermissionResourceRef,
|
||||
getResources,
|
||||
permissions: this.permissions,
|
||||
rules: this.permissionRules,
|
||||
});
|
||||
} else {
|
||||
permissionIntegrationRouter = createPermissionIntegrationRouter({
|
||||
resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
|
||||
getResources,
|
||||
permissions: this.permissions,
|
||||
rules: this.permissionRules,
|
||||
});
|
||||
}
|
||||
permissionsRegistry.addResourceType({
|
||||
resourceRef: catalogEntityPermissionResourceRef,
|
||||
getResources,
|
||||
permissions: this.permissions,
|
||||
rules: this.permissionRules,
|
||||
});
|
||||
|
||||
const scmEventHandlingConfig = readScmEventHandlingConfig(config);
|
||||
const locationStore = new DefaultLocationStore(
|
||||
@@ -589,7 +555,7 @@ export class CatalogBuilder {
|
||||
this.locationAnalyzer ??
|
||||
new AuthorizedLocationAnalyzer(
|
||||
new RepoLocationAnalyzer(logger, integrations, this.locationAnalyzers),
|
||||
permissionsService,
|
||||
permissions,
|
||||
);
|
||||
const locationService = new AuthorizedLocationService(
|
||||
new DefaultLocationService(locationStore, orchestrator, {
|
||||
@@ -599,11 +565,11 @@ export class CatalogBuilder {
|
||||
'catalog.defaultLocationConflictStrategy',
|
||||
) as 'refresh' | 'reject') || 'reject',
|
||||
}),
|
||||
permissionsService,
|
||||
permissions,
|
||||
);
|
||||
const refreshService = new AuthorizedRefreshService(
|
||||
new DefaultRefreshService({ database: catalogDatabase }),
|
||||
permissionsService,
|
||||
permissions,
|
||||
);
|
||||
|
||||
const router = await createRouter({
|
||||
@@ -614,10 +580,9 @@ export class CatalogBuilder {
|
||||
refreshService,
|
||||
logger,
|
||||
config,
|
||||
permissionIntegrationRouter,
|
||||
auth,
|
||||
httpAuth,
|
||||
permissionsService,
|
||||
permissionsService: permissions,
|
||||
auditor,
|
||||
enableRelationsCompatibility,
|
||||
});
|
||||
|
||||
@@ -31,17 +31,10 @@ import {
|
||||
} from '@backstage/catalog-model';
|
||||
import { ConfigReader } from '@backstage/config';
|
||||
import { NotFoundError } from '@backstage/errors';
|
||||
import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/alpha';
|
||||
import { LocationAnalyzer } from '@backstage/plugin-catalog-node';
|
||||
import { AuthorizeResult } from '@backstage/plugin-permission-common';
|
||||
import {
|
||||
createPermissionIntegrationRouter,
|
||||
createPermissionRule,
|
||||
} from '@backstage/plugin-permission-node';
|
||||
import express from 'express';
|
||||
import { Server } from 'node:http';
|
||||
import request from 'supertest';
|
||||
import { z } from 'zod/v3';
|
||||
import { Cursor, EntitiesCatalog } from '../catalog/types';
|
||||
import { applyDatabaseMigrations } from '../database/migrations';
|
||||
import { DbLocationsRow } from '../database/tables';
|
||||
@@ -98,7 +91,6 @@ describe('createRouter readonly disabled', () => {
|
||||
logger: mockServices.logger.mock(),
|
||||
refreshService,
|
||||
config: new ConfigReader(undefined),
|
||||
permissionIntegrationRouter: express.Router(),
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
locationAnalyzer,
|
||||
@@ -1386,7 +1378,6 @@ describe('createRouter readonly and raw json enabled', () => {
|
||||
readonly: true,
|
||||
},
|
||||
}),
|
||||
permissionIntegrationRouter: express.Router(),
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
orchestrator: { process: jest.fn() },
|
||||
@@ -1558,110 +1549,6 @@ describe('createRouter readonly and raw json enabled', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('NextRouter permissioning', () => {
|
||||
let entitiesCatalog: jest.Mocked<EntitiesCatalog>;
|
||||
let locationService: jest.Mocked<LocationService>;
|
||||
let app: express.Express;
|
||||
let refreshService: RefreshService;
|
||||
const permissionsService = mockServices.permissions();
|
||||
|
||||
const fakeRule = createPermissionRule({
|
||||
name: 'FAKE_RULE',
|
||||
description: 'fake rule',
|
||||
resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
|
||||
paramsSchema: z.object({
|
||||
foo: z.string(),
|
||||
}),
|
||||
apply: () => true,
|
||||
toQuery: () => ({ key: '', values: [] }),
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
entitiesCatalog = {
|
||||
entities: jest.fn(),
|
||||
entitiesBatch: jest.fn(),
|
||||
removeEntityByUid: jest.fn(),
|
||||
entityAncestry: jest.fn(),
|
||||
facets: jest.fn(),
|
||||
queryEntities: jest.fn(),
|
||||
};
|
||||
locationService = {
|
||||
getLocation: jest.fn(),
|
||||
createLocation: jest.fn(),
|
||||
queryLocations: jest.fn(),
|
||||
listLocations: jest.fn(),
|
||||
deleteLocation: jest.fn(),
|
||||
getLocationByEntity: jest.fn(),
|
||||
};
|
||||
refreshService = { refresh: jest.fn() };
|
||||
const router = await createRouter({
|
||||
entitiesCatalog,
|
||||
locationService,
|
||||
logger: mockServices.logger.mock(),
|
||||
refreshService,
|
||||
config: new ConfigReader(undefined),
|
||||
permissionIntegrationRouter: createPermissionIntegrationRouter({
|
||||
resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
|
||||
rules: [fakeRule],
|
||||
getResources: jest.fn((resourceRefs: string[]) =>
|
||||
Promise.resolve(
|
||||
resourceRefs.map(resourceRef => ({ id: resourceRef })),
|
||||
),
|
||||
),
|
||||
}),
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
orchestrator: { process: jest.fn() },
|
||||
permissionsService,
|
||||
auditor: mockServices.auditor.mock(),
|
||||
});
|
||||
app = express().use(router);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('accepts and evaluates conditions at the apply-conditions endpoint', async () => {
|
||||
const spideySense: Entity = {
|
||||
apiVersion: 'a',
|
||||
kind: 'component',
|
||||
metadata: {
|
||||
name: 'spidey-sense',
|
||||
},
|
||||
};
|
||||
entitiesCatalog.entities.mockResolvedValueOnce({
|
||||
entities: { type: 'object', entities: [spideySense] },
|
||||
pageInfo: { hasNextPage: false },
|
||||
});
|
||||
|
||||
const requestBody = {
|
||||
items: [
|
||||
{
|
||||
id: '123',
|
||||
resourceType: 'catalog-entity',
|
||||
resourceRef: 'component:default/spidey-sense',
|
||||
conditions: {
|
||||
rule: 'FAKE_RULE',
|
||||
resourceType: 'catalog-entity',
|
||||
params: {
|
||||
foo: 'user:default/spiderman',
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const response = await request(app)
|
||||
.post('/.well-known/backstage/permissions/apply-conditions')
|
||||
.send(requestBody);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual({
|
||||
items: [{ id: '123', result: AuthorizeResult.ALLOW }],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /locations/by-query works end to end', () => {
|
||||
const databases = TestDatabases.create();
|
||||
|
||||
|
||||
@@ -79,7 +79,6 @@ export interface RouterOptions {
|
||||
refreshService?: RefreshService;
|
||||
logger: LoggerService;
|
||||
config: Config;
|
||||
permissionIntegrationRouter?: express.Router;
|
||||
auth: AuthService;
|
||||
httpAuth: HttpAuthService;
|
||||
permissionsService: PermissionsService;
|
||||
@@ -108,7 +107,6 @@ export async function createRouter(
|
||||
refreshService,
|
||||
config,
|
||||
logger,
|
||||
permissionIntegrationRouter,
|
||||
permissionsService,
|
||||
auth,
|
||||
httpAuth,
|
||||
@@ -156,10 +154,6 @@ export async function createRouter(
|
||||
});
|
||||
}
|
||||
|
||||
if (permissionIntegrationRouter) {
|
||||
router.use(permissionIntegrationRouter);
|
||||
}
|
||||
|
||||
if (entitiesCatalog) {
|
||||
router
|
||||
.get('/entities', async (req, res) => {
|
||||
|
||||
Reference in New Issue
Block a user