handle CatalogProcessingOrchestrator permissions check at router level
Signed-off-by: Kashish Mittal <kmittal@redhat.com>
This commit is contained in:
@@ -20,7 +20,6 @@ import {
|
||||
DeferredEntity,
|
||||
EntityRelationSpec,
|
||||
} from '@backstage/plugin-catalog-node';
|
||||
import { BackstageCredentials } from '@backstage/backend-plugin-api';
|
||||
|
||||
/**
|
||||
* The request to process an entity.
|
||||
@@ -29,7 +28,6 @@ import { BackstageCredentials } from '@backstage/backend-plugin-api';
|
||||
export type EntityProcessingRequest = {
|
||||
entity: Entity;
|
||||
state?: JsonObject; // Versions for multiple deployments etc
|
||||
credentials?: BackstageCredentials;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
+12
-8
@@ -18,9 +18,9 @@ import { NotAllowedError } from '@backstage/errors';
|
||||
import { AuthorizeResult } from '@backstage/plugin-permission-common';
|
||||
import { ServerPermissionClient } from '@backstage/plugin-permission-node';
|
||||
import { mockCredentials } from '@backstage/backend-test-utils';
|
||||
import { AuthorizedCatalogProcessingOrchestrator } from './AuthorizedCatalogProcessingOrchestrator';
|
||||
import { AuthorizedValidationService } from './AuthorizedValidationService';
|
||||
|
||||
describe('AuthorizedCatalogProcessingOrchestrator', () => {
|
||||
describe('AuthorizedValidationService', () => {
|
||||
const orchestratorService = {
|
||||
process: jest.fn(),
|
||||
};
|
||||
@@ -38,7 +38,7 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => {
|
||||
result: AuthorizeResult.DENY,
|
||||
},
|
||||
]);
|
||||
const authorizedService = new AuthorizedCatalogProcessingOrchestrator(
|
||||
const authorizedService = new AuthorizedValidationService(
|
||||
orchestratorService,
|
||||
permissionApi as unknown as ServerPermissionClient,
|
||||
);
|
||||
@@ -56,10 +56,12 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => {
|
||||
owner: 'team-a',
|
||||
},
|
||||
},
|
||||
credentials: mockCredentials.none(),
|
||||
};
|
||||
await expect(() =>
|
||||
authorizedService.process(entityProcessingRequest),
|
||||
authorizedService.process(
|
||||
entityProcessingRequest,
|
||||
mockCredentials.none(),
|
||||
),
|
||||
).rejects.toThrow(NotAllowedError);
|
||||
});
|
||||
|
||||
@@ -69,7 +71,7 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => {
|
||||
result: AuthorizeResult.ALLOW,
|
||||
},
|
||||
]);
|
||||
const authorizedService = new AuthorizedCatalogProcessingOrchestrator(
|
||||
const authorizedService = new AuthorizedValidationService(
|
||||
orchestratorService,
|
||||
permissionApi as unknown as ServerPermissionClient,
|
||||
);
|
||||
@@ -87,9 +89,11 @@ describe('AuthorizedCatalogProcessingOrchestrator', () => {
|
||||
owner: 'team-a',
|
||||
},
|
||||
},
|
||||
credentials: mockCredentials.none(),
|
||||
};
|
||||
await authorizedService.process(entityProcessingRequest);
|
||||
await authorizedService.process(
|
||||
entityProcessingRequest,
|
||||
mockCredentials.none(),
|
||||
);
|
||||
expect(orchestratorService.process).toHaveBeenCalledWith(
|
||||
entityProcessingRequest,
|
||||
);
|
||||
+4
-10
@@ -26,23 +26,16 @@ import {
|
||||
PermissionsService,
|
||||
} from '@backstage/backend-plugin-api';
|
||||
|
||||
export class AuthorizedCatalogProcessingOrchestrator
|
||||
implements CatalogProcessingOrchestrator
|
||||
{
|
||||
export class AuthorizedValidationService {
|
||||
constructor(
|
||||
private readonly service: CatalogProcessingOrchestrator,
|
||||
private readonly permissionApi: PermissionsService,
|
||||
) {}
|
||||
|
||||
async process(
|
||||
request: EntityProcessingRequest,
|
||||
credentials: BackstageCredentials,
|
||||
): Promise<EntityProcessingResult> {
|
||||
if (request.credentials) {
|
||||
await this.ensureAuthorized(request.credentials);
|
||||
}
|
||||
return this.service.process(request);
|
||||
}
|
||||
|
||||
private async ensureAuthorized(credentials: BackstageCredentials) {
|
||||
const authorizeDecision = (
|
||||
await this.permissionApi.authorize(
|
||||
[
|
||||
@@ -57,5 +50,6 @@ export class AuthorizedCatalogProcessingOrchestrator
|
||||
if (authorizeDecision.result !== AuthorizeResult.ALLOW) {
|
||||
throw new NotAllowedError();
|
||||
}
|
||||
return this.service.process(request);
|
||||
}
|
||||
}
|
||||
@@ -74,7 +74,6 @@ import { DefaultCatalogProcessingEngine } from '../processing/DefaultCatalogProc
|
||||
import { DefaultLocationService } from './DefaultLocationService';
|
||||
import { DefaultEntitiesCatalog } from './DefaultEntitiesCatalog';
|
||||
import { DefaultCatalogProcessingOrchestrator } from '../processing/DefaultCatalogProcessingOrchestrator';
|
||||
import { AuthorizedCatalogProcessingOrchestrator } from './AuthorizedCatalogProcessingOrchestrator';
|
||||
import { DefaultStitcher } from '../stitching/DefaultStitcher';
|
||||
import { createRouter } from './createRouter';
|
||||
import { DefaultRefreshService } from './DefaultRefreshService';
|
||||
@@ -534,18 +533,15 @@ export class CatalogBuilder {
|
||||
permissionsService = toPermissionEvaluator(permissions);
|
||||
}
|
||||
|
||||
const orchestrator = new AuthorizedCatalogProcessingOrchestrator(
|
||||
new DefaultCatalogProcessingOrchestrator({
|
||||
processors,
|
||||
integrations,
|
||||
rulesEnforcer,
|
||||
logger,
|
||||
parser,
|
||||
policy,
|
||||
legacySingleProcessorValidation: this.legacySingleProcessorValidation,
|
||||
}),
|
||||
permissionsService,
|
||||
);
|
||||
const orchestrator = new DefaultCatalogProcessingOrchestrator({
|
||||
processors,
|
||||
integrations,
|
||||
rulesEnforcer,
|
||||
logger,
|
||||
parser,
|
||||
policy,
|
||||
legacySingleProcessorValidation: this.legacySingleProcessorValidation,
|
||||
});
|
||||
|
||||
const entitiesCatalog = new AuthorizedEntitiesCatalog(
|
||||
unauthorizedEntitiesCatalog,
|
||||
@@ -632,6 +628,7 @@ export class CatalogBuilder {
|
||||
permissionIntegrationRouter,
|
||||
auth,
|
||||
httpAuth,
|
||||
permissionsService,
|
||||
});
|
||||
|
||||
await connectEntityProviders(providerDatabase, entityProviders);
|
||||
|
||||
@@ -42,6 +42,7 @@ import { wrapInOpenApiTestServer } from '@backstage/backend-openapi-utils';
|
||||
import { Server } from 'http';
|
||||
import { mockCredentials, mockServices } from '@backstage/backend-test-utils';
|
||||
import { LocationAnalyzer } from '@backstage/plugin-catalog-node';
|
||||
import { PermissionsService } from '@backstage/backend-plugin-api';
|
||||
|
||||
describe('createRouter readonly disabled', () => {
|
||||
let entitiesCatalog: jest.Mocked<EntitiesCatalog>;
|
||||
@@ -50,6 +51,7 @@ describe('createRouter readonly disabled', () => {
|
||||
let app: express.Express | Server;
|
||||
let refreshService: RefreshService;
|
||||
let locationAnalyzer: jest.Mocked<LocationAnalyzer>;
|
||||
let permissionsService: jest.Mocked<PermissionsService>;
|
||||
|
||||
beforeAll(async () => {
|
||||
entitiesCatalog = {
|
||||
@@ -71,6 +73,11 @@ describe('createRouter readonly disabled', () => {
|
||||
locationAnalyzer = {
|
||||
analyzeLocation: jest.fn(),
|
||||
};
|
||||
permissionsService = {
|
||||
authorize: jest.fn(),
|
||||
authorizeConditional: jest.fn(),
|
||||
};
|
||||
|
||||
refreshService = { refresh: jest.fn() };
|
||||
orchestrator = { process: jest.fn() };
|
||||
const router = await createRouter({
|
||||
@@ -84,6 +91,7 @@ describe('createRouter readonly disabled', () => {
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
locationAnalyzer,
|
||||
permissionsService: permissionsService,
|
||||
});
|
||||
app = wrapInOpenApiTestServer(express().use(router));
|
||||
});
|
||||
@@ -725,6 +733,12 @@ describe('createRouter readonly disabled', () => {
|
||||
metadata: { name: 'n' },
|
||||
};
|
||||
|
||||
permissionsService.authorize.mockResolvedValueOnce([
|
||||
{
|
||||
result: AuthorizeResult.ALLOW,
|
||||
},
|
||||
]);
|
||||
|
||||
orchestrator.process.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
state: {},
|
||||
@@ -742,7 +756,6 @@ describe('createRouter readonly disabled', () => {
|
||||
expect(response.status).toEqual(200);
|
||||
expect(orchestrator.process).toHaveBeenCalledTimes(1);
|
||||
expect(orchestrator.process).toHaveBeenCalledWith({
|
||||
credentials: mockCredentials.user(),
|
||||
entity: {
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
@@ -766,6 +779,12 @@ describe('createRouter readonly disabled', () => {
|
||||
metadata: { name: 'invalid*name' },
|
||||
};
|
||||
|
||||
permissionsService.authorize.mockResolvedValueOnce([
|
||||
{
|
||||
result: AuthorizeResult.ALLOW,
|
||||
},
|
||||
]);
|
||||
|
||||
orchestrator.process.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
errors: [new Error('Invalid entity name')],
|
||||
@@ -780,7 +799,6 @@ describe('createRouter readonly disabled', () => {
|
||||
expect(response.body.errors[0].message).toEqual('Invalid entity name');
|
||||
expect(orchestrator.process).toHaveBeenCalledTimes(1);
|
||||
expect(orchestrator.process).toHaveBeenCalledWith({
|
||||
credentials: mockCredentials.user(),
|
||||
entity: {
|
||||
apiVersion: 'a',
|
||||
kind: 'b',
|
||||
@@ -804,6 +822,12 @@ describe('createRouter readonly disabled', () => {
|
||||
metadata: { name: 'n' },
|
||||
};
|
||||
|
||||
permissionsService.authorize.mockResolvedValueOnce([
|
||||
{
|
||||
result: AuthorizeResult.ALLOW,
|
||||
},
|
||||
]);
|
||||
|
||||
const response = await request(app)
|
||||
.post('/validate-entity')
|
||||
.send({ entity, location: null });
|
||||
@@ -850,6 +874,7 @@ describe('createRouter readonly enabled', () => {
|
||||
let entitiesCatalog: jest.Mocked<EntitiesCatalog>;
|
||||
let app: express.Express;
|
||||
let locationService: jest.Mocked<LocationService>;
|
||||
let permissionsService: jest.Mocked<PermissionsService>;
|
||||
|
||||
beforeAll(async () => {
|
||||
entitiesCatalog = {
|
||||
@@ -879,6 +904,7 @@ describe('createRouter readonly enabled', () => {
|
||||
permissionIntegrationRouter: express.Router(),
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
permissionsService: permissionsService,
|
||||
});
|
||||
app = express().use(router);
|
||||
});
|
||||
@@ -1048,6 +1074,7 @@ describe('NextRouter permissioning', () => {
|
||||
let locationService: jest.Mocked<LocationService>;
|
||||
let app: express.Express;
|
||||
let refreshService: RefreshService;
|
||||
let permissionsService: jest.Mocked<PermissionsService>;
|
||||
|
||||
const fakeRule = createPermissionRule({
|
||||
name: 'FAKE_RULE',
|
||||
@@ -1094,6 +1121,7 @@ describe('NextRouter permissioning', () => {
|
||||
}),
|
||||
auth: mockServices.auth(),
|
||||
httpAuth: mockServices.httpAuth(),
|
||||
permissionsService: permissionsService,
|
||||
});
|
||||
app = express().use(router);
|
||||
});
|
||||
|
||||
@@ -53,8 +53,10 @@ import {
|
||||
HttpAuthService,
|
||||
LoggerService,
|
||||
SchedulerService,
|
||||
PermissionsService,
|
||||
} from '@backstage/backend-plugin-api';
|
||||
import { LocationAnalyzer } from '@backstage/plugin-catalog-node';
|
||||
import { AuthorizedValidationService } from './AuthorizedValidationService';
|
||||
|
||||
/**
|
||||
* Options used by {@link createRouter}.
|
||||
@@ -74,6 +76,7 @@ export interface RouterOptions {
|
||||
permissionIntegrationRouter?: express.Router;
|
||||
auth: AuthService;
|
||||
httpAuth: HttpAuthService;
|
||||
permissionsService: PermissionsService;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -98,6 +101,7 @@ export async function createRouter(
|
||||
config,
|
||||
logger,
|
||||
permissionIntegrationRouter,
|
||||
permissionsService,
|
||||
auth,
|
||||
httpAuth,
|
||||
} = options;
|
||||
@@ -346,20 +350,26 @@ export async function createRouter(
|
||||
}
|
||||
|
||||
const credentials = await httpAuth.credentials(req);
|
||||
const processingResult = await orchestrator.process({
|
||||
entity: {
|
||||
...entity,
|
||||
metadata: {
|
||||
...entity.metadata,
|
||||
annotations: {
|
||||
[ANNOTATION_LOCATION]: body.location,
|
||||
[ANNOTATION_ORIGIN_LOCATION]: body.location,
|
||||
...entity.metadata.annotations,
|
||||
const authorizedValidationService = new AuthorizedValidationService(
|
||||
orchestrator,
|
||||
permissionsService,
|
||||
);
|
||||
const processingResult = await authorizedValidationService.process(
|
||||
{
|
||||
entity: {
|
||||
...entity,
|
||||
metadata: {
|
||||
...entity.metadata,
|
||||
annotations: {
|
||||
[ANNOTATION_LOCATION]: body.location,
|
||||
[ANNOTATION_ORIGIN_LOCATION]: body.location,
|
||||
...entity.metadata.annotations,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
credentials: credentials,
|
||||
});
|
||||
credentials,
|
||||
);
|
||||
|
||||
if (!processingResult.ok)
|
||||
res.status(400).json({
|
||||
|
||||
Reference in New Issue
Block a user