From 8d07b541d7eb037cea5a8a79c01135a02b98cab9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 27 Aug 2020 20:54:56 +0200 Subject: [PATCH] catalog-backend: switch catalog rules to deny by default --- .../src/ingestion/CatalogRules.test.ts | 42 ++++++++++--------- .../src/ingestion/CatalogRules.ts | 27 +++++------- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts b/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts index 7f2ccdc254..1168db1080 100644 --- a/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts +++ b/plugins/catalog-backend/src/ingestion/CatalogRules.test.ts @@ -45,11 +45,11 @@ const location: Record = { }; describe('CatalogRulesEnforcer', () => { - it('should allow by default', () => { + it('should deny by default', () => { const enforcer = new CatalogRulesEnforcer([]); - expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); - expect(enforcer.isAllowed(entity.group, location.y)).toBe(true); - expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); + expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); + expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); + expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); }); it('should deny all', () => { @@ -59,8 +59,10 @@ describe('CatalogRulesEnforcer', () => { expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); }); - it('should allow all with override', () => { - const enforcer = new CatalogRulesEnforcer([{ allow: [] }, { deny: [] }]); + it('should allow all', () => { + const enforcer = new CatalogRulesEnforcer([ + { allow: [{ kind: 'User' }, { kind: 'Group' }, { kind: 'Component' }] }, + ]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); expect(enforcer.isAllowed(entity.group, location.y)).toBe(true); expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); @@ -68,7 +70,7 @@ describe('CatalogRulesEnforcer', () => { it('should deny groups', () => { const enforcer = new CatalogRulesEnforcer([ - { allow: [], deny: [{ kind: 'Group' }] }, + { allow: [{ kind: 'User' }, { kind: 'Component' }] }, ]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); expect(enforcer.isAllowed(entity.group, location.x)).toBe(false); @@ -79,7 +81,8 @@ describe('CatalogRulesEnforcer', () => { it('should deny groups from github', () => { const enforcer = new CatalogRulesEnforcer([ - { allow: [], deny: [{ kind: 'Group' }], locations: [{ type: 'github' }] }, + { allow: [{ kind: 'User' }, { kind: 'Component' }] }, + { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] }, ]); expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); expect(enforcer.isAllowed(entity.group, location.x)).toBe(false); @@ -88,27 +91,26 @@ describe('CatalogRulesEnforcer', () => { expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); }); - it('should override to allow groups from files', () => { + it('should allow groups from files', () => { const enforcer = new CatalogRulesEnforcer([ - { allow: [], deny: [{ kind: 'Group' }] }, - { allow: [{ kind: 'Group' }], deny: [], locations: [{ type: 'file' }] }, + { allow: [{ kind: 'Group' }], locations: [{ type: 'file' }] }, ]); - expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); + expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); expect(enforcer.isAllowed(entity.group, location.x)).toBe(false); expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); expect(enforcer.isAllowed(entity.group, location.z)).toBe(true); - expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); + expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); }); it('should not be sensitive to kind case', () => { const enforcer = new CatalogRulesEnforcer([ - { allow: [], deny: [{ kind: 'group' }] }, - { allow: [], deny: [{ kind: 'Component' }] }, + { allow: [{ kind: 'group' }] }, + { allow: [{ kind: 'Component' }] }, ]); - expect(enforcer.isAllowed(entity.user, location.x)).toBe(true); - expect(enforcer.isAllowed(entity.group, location.x)).toBe(false); - expect(enforcer.isAllowed(entity.group, location.y)).toBe(false); - expect(enforcer.isAllowed(entity.group, location.z)).toBe(false); - expect(enforcer.isAllowed(entity.component, location.z)).toBe(false); + expect(enforcer.isAllowed(entity.user, location.x)).toBe(false); + expect(enforcer.isAllowed(entity.group, location.x)).toBe(true); + expect(enforcer.isAllowed(entity.group, location.y)).toBe(true); + expect(enforcer.isAllowed(entity.group, location.z)).toBe(true); + expect(enforcer.isAllowed(entity.component, location.z)).toBe(true); }); }); diff --git a/plugins/catalog-backend/src/ingestion/CatalogRules.ts b/plugins/catalog-backend/src/ingestion/CatalogRules.ts index a03d93852a..eda93b552f 100644 --- a/plugins/catalog-backend/src/ingestion/CatalogRules.ts +++ b/plugins/catalog-backend/src/ingestion/CatalogRules.ts @@ -38,8 +38,7 @@ type LocationMatcher = { * An undefined list of matchers means match all, an empty list of matchers means match none */ type CatalogRule = { - deny?: EntityMatcher[]; - allow?: EntityMatcher[]; + allow: EntityMatcher[]; locations?: LocationMatcher[]; }; @@ -51,8 +50,7 @@ export class CatalogRulesEnforcer { */ static readonly defaultRules: CatalogRule[] = [ { - deny: [{ kind: 'User' }, { kind: 'Group' }], - allow: [], + allow: [{ kind: 'Component' }, { kind: 'API' }], }, ]; @@ -60,8 +58,10 @@ export class CatalogRulesEnforcer { * Loads catalog rules from config. * * This reads `catalog.rules` and defaults to the default rules if no value is present. - * The value of the config should be a list of config objects, each with a single `deny` - * field which in turn is a list of entity kind to deny. + * The value of the config should be a list of config objects, each with a single `allow` + * field which in turn is a list of entity kinds to allow. + * + * If there is no matching rule to allow an ingested entity, it will be rejected by the catalog. * * It also reads in rules from `catalog.locations`, where each location can have a list * of allowed entity for the location, specified in an `allow` field. @@ -71,7 +71,7 @@ export class CatalogRulesEnforcer { * ```yaml * catalog: * rules: - * - deny: [User, Group, System] + * - allow: [Component, API] * * locations: * - type: github @@ -87,8 +87,7 @@ export class CatalogRulesEnforcer { if (config.has('catalog.rules')) { const globalRules = config.getConfigArray('catalog.rules').map(sub => ({ - deny: sub.getStringArray('deny').map(kind => ({ kind })), - allow: [], + allow: sub.getStringArray('allow').map(kind => ({ kind })), })); rules.push(...globalRules); } else { @@ -105,7 +104,6 @@ export class CatalogRulesEnforcer { return [ { - deny: [], allow: sub.getStringArray('allow').map(kind => ({ kind })), locations: [ { @@ -130,22 +128,17 @@ export class CatalogRulesEnforcer { * according to the configured rules. */ isAllowed(entity: Entity, location: LocationSpec) { - let result = true; - for (const rule of this.rules) { if (!this.matchLocation(location, rule.locations)) { continue; } if (this.matchEntity(entity, rule.allow)) { - result = true; - } - if (this.matchEntity(entity, rule.deny)) { - result = false; + return true; } } - return result; + return false; } private matchLocation(