Integrate authorization into catalog-backend delete entities endpoint (#8805)

* Integrate authorization into catalog-backend delete entities endpoint

Signed-off-by: Joon Park <joonp@spotify.com>

* Remove entity ref from request and handle conditional response

Signed-off-by: Joon Park <joonp@spotify.com>
This commit is contained in:
Joon Park
2022-01-17 12:22:24 +00:00
committed by GitHub
parent 4df6c3ccb3
commit e15ce5c16e
7 changed files with 151 additions and 12 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend': patch
---
Integrate authorization into the delete entities endpoint
+6 -1
View File
@@ -830,7 +830,12 @@ export function durationText(startTimestamp: [number, number]): string;
// @public (undocumented)
export type EntitiesCatalog = {
entities(request?: EntitiesRequest): Promise<EntitiesResponse>;
removeEntityByUid(uid: string): Promise<void>;
removeEntityByUid(
uid: string,
options?: {
authorizationToken?: string;
},
): Promise<void>;
batchAddOrUpdateEntities?(
requests: EntityUpsertRequest[],
options?: {
+4 -1
View File
@@ -113,7 +113,10 @@ export type EntitiesCatalog = {
*
* @param uid - The metadata.uid of the entity
*/
removeEntityByUid(uid: string): Promise<void>;
removeEntityByUid(
uid: string,
options?: { authorizationToken?: string },
): Promise<void>;
/**
* Writes a number of entities efficiently to storage.
@@ -14,6 +14,7 @@
* limitations under the License.
*/
import { NotAllowedError } from '@backstage/errors';
import { AuthorizeResult } from '@backstage/plugin-permission-common';
import { createConditionTransformer } from '@backstage/plugin-permission-node';
import { isEntityKind } from '../permissions/rules/isEntityKind';
@@ -93,4 +94,86 @@ describe('AuthorizedEntitiesCatalog', () => {
});
});
});
describe('removeEntityByUid', () => {
it('throws error on DENY', async () => {
fakeCatalog.entities.mockResolvedValue({
entities: [
{ kind: 'component', namespace: 'default', name: 'my-component' },
],
});
fakePermissionApi.authorize.mockResolvedValue([
{ result: AuthorizeResult.DENY },
]);
const catalog = new AuthorizedEntitiesCatalog(
fakeCatalog,
fakePermissionApi,
createConditionTransformer([]),
);
await expect(() =>
catalog.removeEntityByUid('uid', { authorizationToken: 'abcd' }),
).rejects.toThrowError(NotAllowedError);
});
it('throws error on CONDITIONAL authorization that evaluates to 0 entities', async () => {
fakePermissionApi.authorize.mockResolvedValue([
{
result: AuthorizeResult.CONDITIONAL,
conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] },
},
]);
fakeCatalog.entities.mockResolvedValue({ entities: [] });
const catalog = new AuthorizedEntitiesCatalog(
fakeCatalog,
fakePermissionApi,
createConditionTransformer([isEntityKind]),
);
await expect(() =>
catalog.removeEntityByUid('uid', { authorizationToken: 'abcd' }),
).rejects.toThrowError(NotAllowedError);
});
it('calls underlying catalog method on CONDITIONAL authorization that evaluates to nonzero entities', async () => {
fakePermissionApi.authorize.mockResolvedValue([
{
result: AuthorizeResult.CONDITIONAL,
conditions: { rule: 'IS_ENTITY_KIND', params: [['b']] },
},
]);
fakeCatalog.entities.mockResolvedValue({
entities: [{ kind: 'b', namespace: 'default', name: 'my-component' }],
});
const catalog = new AuthorizedEntitiesCatalog(
fakeCatalog,
fakePermissionApi,
createConditionTransformer([isEntityKind]),
);
await catalog.removeEntityByUid('uid', { authorizationToken: 'abcd' });
expect(fakeCatalog.removeEntityByUid).toHaveBeenCalledWith('uid');
});
it('calls underlying catalog method on ALLOW', async () => {
fakeCatalog.entities.mockResolvedValue({
entities: [
{ kind: 'component', namespace: 'default', name: 'my-component' },
],
});
fakePermissionApi.authorize.mockResolvedValue([
{ result: AuthorizeResult.ALLOW },
]);
const catalog = new AuthorizedEntitiesCatalog(
fakeCatalog,
fakePermissionApi,
createConditionTransformer([]),
);
await catalog.removeEntityByUid('uid', { authorizationToken: 'abcd' });
expect(fakeCatalog.removeEntityByUid).toHaveBeenCalledWith('uid');
});
});
});
@@ -14,7 +14,11 @@
* limitations under the License.
*/
import { catalogEntityReadPermission } from '@backstage/plugin-catalog-common';
import { NotAllowedError } from '@backstage/errors';
import {
catalogEntityDeletePermission,
catalogEntityReadPermission,
} from '@backstage/plugin-catalog-common';
import {
AuthorizeResult,
PermissionAuthorizer,
@@ -27,6 +31,7 @@ import {
EntityAncestryResponse,
EntityFilter,
} from '../catalog/types';
import { basicEntityFilter } from './request/basicEntityFilter';
export class AuthorizedEntitiesCatalog implements EntitiesCatalog {
constructor(
@@ -65,8 +70,32 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog {
return this.entitiesCatalog.entities(request);
}
removeEntityByUid(uid: string): Promise<void> {
// TODO: Implement permissioning
async removeEntityByUid(
uid: string,
options?: { authorizationToken?: string },
): Promise<void> {
const authorizeResponse = (
await this.permissionApi.authorize(
[{ permission: catalogEntityDeletePermission }],
{ token: options?.authorizationToken },
)
)[0];
if (authorizeResponse.result === AuthorizeResult.DENY) {
throw new NotAllowedError();
}
if (authorizeResponse.result === AuthorizeResult.CONDITIONAL) {
const permissionFilter: EntityFilter = this.transformConditions(
authorizeResponse.conditions,
);
const { entities } = await this.entitiesCatalog.entities({
filter: {
allOf: [permissionFilter, basicEntityFilter({ 'metadata.uid': uid })],
},
});
if (entities.length === 0) {
throw new NotAllowedError();
}
}
return this.entitiesCatalog.removeEntityByUid(uid);
}
@@ -216,10 +216,14 @@ describe('createNextRouter readonly disabled', () => {
it('can remove', async () => {
entitiesCatalog.removeEntityByUid.mockResolvedValue(undefined);
const response = await request(app).delete('/entities/by-uid/apa');
const response = await request(app)
.delete('/entities/by-uid/apa')
.set('authorization', 'Bearer someauthtoken');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa', {
authorizationToken: 'someauthtoken',
});
expect(response.status).toEqual(204);
});
@@ -228,10 +232,14 @@ describe('createNextRouter readonly disabled', () => {
new NotFoundError('nope'),
);
const response = await request(app).delete('/entities/by-uid/apa');
const response = await request(app)
.delete('/entities/by-uid/apa')
.set('authorization', 'Bearer someauthtoken');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa', {
authorizationToken: 'someauthtoken',
});
expect(response.status).toEqual(404);
});
});
@@ -372,10 +380,14 @@ describe('createNextRouter readonly enabled', () => {
describe('DELETE /entities/by-uid/:uid', () => {
// this delete is allowed as there is no other way to remove entities
it('is allowed', async () => {
const response = await request(app).delete('/entities/by-uid/apa');
const response = await request(app)
.delete('/entities/by-uid/apa')
.set('authorization', 'Bearer someauthtoken');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledTimes(1);
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa');
expect(entitiesCatalog.removeEntityByUid).toHaveBeenCalledWith('apa', {
authorizationToken: 'someauthtoken',
});
expect(response.status).toEqual(204);
});
});
@@ -119,7 +119,9 @@ export async function createNextRouter(
})
.delete('/entities/by-uid/:uid', async (req, res) => {
const { uid } = req.params;
await entitiesCatalog.removeEntityByUid(uid);
await entitiesCatalog.removeEntityByUid(uid, {
authorizationToken: getBearerToken(req.header('authorization')),
});
res.status(204).end();
})
.get('/entities/by-name/:kind/:namespace/:name', async (req, res) => {