diff --git a/plugins/rbac-backend/docs/apis.md b/plugins/rbac-backend/docs/apis.md index 03ae51a33bd..207e9648cb5 100644 --- a/plugins/rbac-backend/docs/apis.md +++ b/plugins/rbac-backend/docs/apis.md @@ -705,6 +705,34 @@ Based on the above schema: } ``` +**NOTE**: We do not support the ability to run conditions in parallel during creation. An example can be found below, notice that `anyOf` and `not` are on the same level. Consider making separate condition requests, or nest your conditions based on the available criteria. + +```json +{ + "anyOf": [ + { + "rule": "IS_ENTITY_OWNER", + "resourceType": "catalog-entity", + "params": { + "claims": ["group:default/team-a"] + } + }, + { + "rule": "IS_ENTITY_KIND", + "resourceType": "catalog-entity", + "params": { + "kinds": ["Group"] + } + } + ], + "not": { + "rule": "IS_ENTITY_KIND", + "resourceType": "catalog-entity", + "params": { "kinds": ["Api"] } + } +} +``` + To utilize this condition to the RBAC REST api you need to wrap it with more info: ```json diff --git a/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts b/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts index 1890c93421d..976e6727aec 100644 --- a/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts +++ b/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts @@ -19,7 +19,7 @@ import { checkForDuplicatePolicies, validateGroupingPolicy, validatePolicy, -} from '../service/policies-validation'; +} from '../validation/policies-validation'; export const CSV_PERMISSION_POLICY_FILE_AUTHOR = 'csv permission policy file'; diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts index cbe66557946..81834c7b2da 100644 --- a/plugins/rbac-backend/src/service/permission-policy.ts +++ b/plugins/rbac-backend/src/service/permission-policy.ts @@ -29,8 +29,8 @@ import { } from '../database/role-metadata'; import { CSVFileWatcher } from '../file-permissions/csv-file-watcher'; import { metadataStringToPolicy, removeTheDifference } from '../helper'; +import { validateEntityReference } from '../validation/policies-validation'; import { EnforcerDelegate } from './enforcer-delegate'; -import { validateEntityReference } from './policies-validation'; export const ADMIN_ROLE_NAME = 'role:default/rbac_admin'; export const ADMIN_ROLE_AUTHOR = 'application configuration'; diff --git a/plugins/rbac-backend/src/service/policies-rest-api.test.ts b/plugins/rbac-backend/src/service/policies-rest-api.test.ts index 04e4476a35e..fc9db4e62fb 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.test.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.test.ts @@ -148,7 +148,7 @@ const conditionalStorage = { }; const validateRoleConditionMock = jest.fn().mockImplementation(); -jest.mock('./condition-validation', () => { +jest.mock('../validation/condition-validation', () => { return { validateRoleCondition: jest .fn() diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts index f0a3e5a78d7..5041b2cb5fb 100644 --- a/plugins/rbac-backend/src/service/policies-rest-api.ts +++ b/plugins/rbac-backend/src/service/policies-rest-api.ts @@ -52,14 +52,14 @@ import { RoleMetadataStorage, } from '../database/role-metadata'; import { deepSortedEqual, isPermissionAction, policyToString } from '../helper'; -import { validateRoleCondition } from './condition-validation'; -import { EnforcerDelegate } from './enforcer-delegate'; -import { PluginPermissionMetadataCollector } from './plugin-endpoints'; +import { validateRoleCondition } from '../validation/condition-validation'; import { validateEntityReference, validatePolicy, validateRole, -} from './policies-validation'; +} from '../validation/policies-validation'; +import { EnforcerDelegate } from './enforcer-delegate'; +import { PluginPermissionMetadataCollector } from './plugin-endpoints'; export class PoliciesServer { constructor( diff --git a/plugins/rbac-backend/src/service/condition.validation.test.ts b/plugins/rbac-backend/src/validation/condition-validation.test.ts similarity index 77% rename from plugins/rbac-backend/src/service/condition.validation.test.ts rename to plugins/rbac-backend/src/validation/condition-validation.test.ts index 48cf9d770f9..aafd7061abb 100644 --- a/plugins/rbac-backend/src/service/condition.validation.test.ts +++ b/plugins/rbac-backend/src/validation/condition-validation.test.ts @@ -351,6 +351,7 @@ describe('condition-validation', () => { } catch (err) { unexpectedErr = err; } + expect(unexpectedErr).toBeUndefined(); }); }); @@ -724,4 +725,203 @@ describe('condition-validation', () => { expect(unexpectedErr).toBeUndefined(); }); }); + + describe('complex conditions', () => { + it('should fail validation of role-condition.conditions in parallel with condition rule', () => { + const condition: RoleConditionalPolicyDecision = { + id: 1, + pluginId: 'catalog', + resourceType: 'catalog-entity', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['read'], + conditions: { + allOf: [ + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + }; + expect(() => validateRoleCondition(condition)).toThrow( + `RBAC plugin does not support parallel conditions alongside rules, consider reworking request to include nested condition criteria. Conditional criteria causing the error allOf, 'rule: IS_ENTITY_OWNER'.`, + ); + }); + + it('should fail validation of role-condition.conditions criteria (allOf, not) in parallel', () => { + const condition: RoleConditionalPolicyDecision = { + id: 1, + pluginId: 'catalog', + resourceType: 'catalog-entity', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['read'], + conditions: { + allOf: [ + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + not: { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + }, + }; + expect(() => validateRoleCondition(condition)).toThrow( + `RBAC plugin does not support parallel conditions, consider reworking request to include nested condition criteria. Conditional criteria causing the error allOf,not.`, + ); + }); + + it('should fail validation of role-condition.conditions criteria (allOf, anyOf) in parallel', () => { + const condition: RoleConditionalPolicyDecision = { + id: 1, + pluginId: 'catalog', + resourceType: 'catalog-entity', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['read'], + conditions: { + allOf: [ + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + anyOf: [ + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + }, + }; + expect(() => validateRoleCondition(condition)).toThrow( + `RBAC plugin does not support parallel conditions, consider reworking request to include nested condition criteria. Conditional criteria causing the error allOf,anyOf.`, + ); + }); + + it('should fail validation of role-condition.conditions criteria (not, anyOf) in parallel', () => { + const condition: RoleConditionalPolicyDecision = { + id: 1, + pluginId: 'catalog', + resourceType: 'catalog-entity', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['read'], + conditions: { + not: { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + anyOf: [ + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + }, + }; + expect(() => validateRoleCondition(condition)).toThrow( + `RBAC plugin does not support parallel conditions, consider reworking request to include nested condition criteria. Conditional criteria causing the error anyOf,not.`, + ); + }); + + it('should validate role-condition.conditions that are nested', () => { + const condition: RoleConditionalPolicyDecision = { + id: 1, + pluginId: 'catalog', + resourceType: 'catalog-entity', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['read'], + conditions: { + anyOf: [ + { + not: { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + }, + { + rule: 'IS_ENTITY_OWNER', + resourceType: 'catalog-entity', + params: { + claims: ['user:default/logarifm', 'group:default/team-a'], + }, + }, + { + rule: 'IS_ENTITY_KIND', + resourceType: 'catalog-entity', + params: { kinds: ['Group'] }, + }, + ], + }, + }; + + let unexpectedErr; + try { + validateRoleCondition(condition); + } catch (err) { + unexpectedErr = err; + } + expect(unexpectedErr).toBeUndefined(); + }); + }); }); diff --git a/plugins/rbac-backend/src/service/condition-validation.ts b/plugins/rbac-backend/src/validation/condition-validation.ts similarity index 55% rename from plugins/rbac-backend/src/service/condition-validation.ts rename to plugins/rbac-backend/src/validation/condition-validation.ts index f87052b9387..6957c86099f 100644 --- a/plugins/rbac-backend/src/service/condition-validation.ts +++ b/plugins/rbac-backend/src/validation/condition-validation.ts @@ -55,12 +55,20 @@ export function validateRoleCondition( } } +/** + * validatePermissionCondition validate conditional permission policies using validateCriteria and validateRule. + * @param conditionOrCriteria The Permission Criteria of the conditional permission. + * @param jsonPathLocator The location in the JSON of the current check. + * @returns undefined. + */ function validatePermissionCondition( conditionOrCriteria: PermissionCriteria< PermissionCondition >, jsonPathLocator: string, ) { + validateCriteria(conditionOrCriteria, jsonPathLocator); + if ('not' in conditionOrCriteria) { validatePermissionCondition( conditionOrCriteria.not, @@ -68,6 +76,7 @@ function validatePermissionCondition( ); return; } + if ('allOf' in conditionOrCriteria) { if ( !Array.isArray(conditionOrCriteria.allOf) || @@ -82,6 +91,7 @@ function validatePermissionCondition( } return; } + if ('anyOf' in conditionOrCriteria) { if ( !Array.isArray(conditionOrCriteria.anyOf) || @@ -94,8 +104,20 @@ function validatePermissionCondition( for (const [index, elem] of conditionOrCriteria.anyOf.entries()) { validatePermissionCondition(elem, `${jsonPathLocator}.anyOf[${index}]`); } - return; } +} + +/** + * validateRule ensures that there is a rule and resource type associated with each conditional permission. + * @param conditionOrCriteria The Permission Criteria of the conditional permission. + * @param jsonPathLocator The location in the JSON of the current check. + */ +function validateRule( + conditionOrCriteria: PermissionCriteria< + PermissionCondition + >, + jsonPathLocator: string, +) { if (!('resourceType' in conditionOrCriteria)) { throw new Error( `'resourceType' must be specified in the ${jsonPathLocator}.condition`, @@ -107,3 +129,44 @@ function validatePermissionCondition( ); } } + +/** + * validateCriteria ensures that there is only one of the following criteria: allOf, anyOf, and not, at any given level. + * We want to make sure that there are no parallel conditional criteria for conditional permission policies as this is + * not support by the permission framework. + * + * If more than one criteria are at a given level, we throw an error about the inability to support parallel conditions. + * If no criteria are found, we validate the rule. + * + * @param conditionOrCriteria The Permission Criteria of the conditional permission. + * @param jsonPathLocator The location in the JSON of the current check. + */ +function validateCriteria( + conditionOrCriteria: PermissionCriteria< + PermissionCondition + >, + jsonPathLocator: string, +) { + const criteriaList = ['allOf', 'anyOf', 'not']; + const found: string[] = []; + + for (const crit of criteriaList) { + if (crit in conditionOrCriteria) { + found.push(crit); + } + } + + if (found.length > 1) { + throw new Error( + `RBAC plugin does not support parallel conditions, consider reworking request to include nested condition criteria. Conditional criteria causing the error ${found}.`, + ); + } else if (found.length === 0) { + validateRule(conditionOrCriteria, jsonPathLocator); + } + + if (found.length === 1 && 'rule' in conditionOrCriteria) { + throw new Error( + `RBAC plugin does not support parallel conditions alongside rules, consider reworking request to include nested condition criteria. Conditional criteria causing the error ${found}, 'rule: ${conditionOrCriteria.rule}'.`, + ); + } +} diff --git a/plugins/rbac-backend/src/service/policies-validation.test.ts b/plugins/rbac-backend/src/validation/policies-validation.test.ts similarity index 74% rename from plugins/rbac-backend/src/service/policies-validation.test.ts rename to plugins/rbac-backend/src/validation/policies-validation.test.ts index 99188fd977c..cc9c54b45da 100644 --- a/plugins/rbac-backend/src/service/policies-validation.test.ts +++ b/plugins/rbac-backend/src/validation/policies-validation.test.ts @@ -1,11 +1,20 @@ import { RoleBasedPolicy } from '@janus-idp/backstage-plugin-rbac-common'; +import { RoleMetadataStorage } from '../database/role-metadata'; import { validateEntityReference, + validateGroupingPolicy, validatePolicy, validateRole, } from './policies-validation'; +const roleMetadataStorageMock: RoleMetadataStorage = { + findRoleMetadata: jest.fn().mockImplementation(), + createRoleMetadata: jest.fn().mockImplementation(), + updateRoleMetadata: jest.fn().mockImplementation(), + removeRoleMetadata: jest.fn().mockImplementation(), +}; + describe('rest data validation', () => { describe('validate entity referenced policy', () => { it('should return an error when entity reference is empty', () => { @@ -196,4 +205,64 @@ describe('rest data validation', () => { expect(err).toBeUndefined(); }); }); + + describe('validateGroupingPolicy', () => { + it('should fail validation if the group policy length is greater than two', async () => { + const groupPolicy = ['user:default/test', 'role:default/test', 'invalid']; + + const err = await validateGroupingPolicy( + groupPolicy, + './test', + roleMetadataStorageMock, + 'csv-file', + ); + expect(err).toBeTruthy(); + expect(err?.message).toEqual(`Group policy should have length 2`); + }); + + it('should fail validation if the member of the group policy starts with role:', async () => { + const groupPolicy = ['role:default/test', 'role:default/test']; + + const err = await validateGroupingPolicy( + groupPolicy, + './test', + roleMetadataStorageMock, + 'csv-file', + ); + expect(err).toBeTruthy(); + expect(err?.message).toEqual( + `Group policy is invalid: ${groupPolicy}. rbac-backend plugin doesn't support role inheritance.`, + ); + }); + + it('should fail validation if the group policy contains two groups for group inheritance', async () => { + const groupPolicy = ['group:default/test', 'group:default/test']; + + const err = await validateGroupingPolicy( + groupPolicy, + './test', + roleMetadataStorageMock, + 'csv-file', + ); + expect(err).toBeTruthy(); + expect(err?.message).toEqual( + `Group policy is invalid: ${groupPolicy}. Group inheritance information could be provided only with help of Catalog API.`, + ); + }); + + it('should fail validation if the group policy is a user to a group for inheritance', async () => { + const groupPolicy = ['user:default/test', 'group:default/test']; + + const err = await validateGroupingPolicy( + groupPolicy, + './test', + roleMetadataStorageMock, + 'csv-file', + ); + expect(err).toBeTruthy(); + expect(err?.message).toEqual( + `Group policy is invalid: ${groupPolicy}. User membership information could be provided only with help of Catalog API.`, + ); + }); + }); }); diff --git a/plugins/rbac-backend/src/service/policies-validation.ts b/plugins/rbac-backend/src/validation/policies-validation.ts similarity index 80% rename from plugins/rbac-backend/src/service/policies-validation.ts rename to plugins/rbac-backend/src/validation/policies-validation.ts index ba8e2536b8c..356832331d5 100644 --- a/plugins/rbac-backend/src/service/policies-validation.ts +++ b/plugins/rbac-backend/src/validation/policies-validation.ts @@ -10,7 +10,6 @@ import { } from '@janus-idp/backstage-plugin-rbac-common'; import { RoleMetadataStorage } from '../database/role-metadata'; -import { EnforcerDelegate } from './enforcer-delegate'; export function validatePolicy(policy: RoleBasedPolicy): Error | undefined { const err = validateEntityReference(policy.entityReference); @@ -160,54 +159,6 @@ export async function validateGroupingPolicy( return undefined; } -export async function validateAllPredefinedPolicies( - policies: string[][], - groupPolicies: string[][], - preDefinedPoliciesFile: string, - roleMetadataStorage: RoleMetadataStorage, - enforcer: EnforcerDelegate, -): Promise { - for (const policy of policies) { - const err = validateEntityReference(policy[0]); - if (err) { - return new Error( - `Failed to validate policy from file ${preDefinedPoliciesFile}. Cause: ${err.message}`, - ); - } - - if (await enforcer.hasPolicy(...policy)) { - const source = (await enforcer.getMetadata(policy)).source; - if (source !== 'csv-file') { - return new Error( - `Duplicate policy: ${policy} found in the file ${preDefinedPoliciesFile}, originates from source: ${source}`, - ); - } - } - } - - for (const groupPolicy of groupPolicies) { - const validationError = await validateGroupingPolicy( - groupPolicy, - preDefinedPoliciesFile, - roleMetadataStorage, - `csv-file`, - ); - if (validationError) { - return validationError; - } - - if (await enforcer.hasGroupingPolicy(...groupPolicy)) { - const source = (await enforcer.getMetadata(groupPolicy)).source; - if (source !== 'csv-file') { - return new Error( - `Duplicate role: ${groupPolicy} found in the file ${preDefinedPoliciesFile}, originates from source: ${source}`, - ); - } - } - } - return undefined; -} - export const checkForDuplicatePolicies = async ( fileEnf: Enforcer, policy: string[],