From 15dac91b673c63a4e7ac41f95296651df2ef8053 Mon Sep 17 00:00:00 2001 From: Patrick Knight Date: Tue, 28 May 2024 09:46:47 -0400 Subject: [PATCH] feat(rbac): improve conditional policy validation (#1673) * feat(rbac): improve conditional policy validation * feat(rbac): add more documentation for conditional policies * fix(rbac): allow for use to be an option for permission policy actions * fix(rbac): address review comments --- plugins/rbac-backend/docs/apis.md | 131 +------ plugins/rbac-backend/docs/conditions.md | 353 ++++++++++++++++++ .../src/file-permissions/csv-file-watcher.ts | 2 +- .../src/service/permission-policy.ts | 2 +- .../src/service/policies-rest-api.test.ts | 2 +- .../src/service/policies-rest-api.ts | 11 +- .../condition-validation.test.ts} | 224 +++++++++++ .../condition-validation.ts | 65 +++- .../policies-validation.test.ts | 69 ++++ .../policies-validation.ts | 49 --- 10 files changed, 723 insertions(+), 185 deletions(-) create mode 100644 plugins/rbac-backend/docs/conditions.md rename plugins/rbac-backend/src/{service/condition.validation.test.ts => validation/condition-validation.test.ts} (75%) rename plugins/rbac-backend/src/{service => validation}/condition-validation.ts (55%) rename plugins/rbac-backend/src/{service => validation}/policies-validation.test.ts (74%) rename plugins/rbac-backend/src/{service => validation}/policies-validation.ts (80%) diff --git a/plugins/rbac-backend/docs/apis.md b/plugins/rbac-backend/docs/apis.md index 03ae51a33b..3c7000afa3 100644 --- a/plugins/rbac-backend/docs/apis.md +++ b/plugins/rbac-backend/docs/apis.md @@ -461,34 +461,13 @@ Returns: ## Conditions -The Backstage permission framework provides conditions, and the RBAC backend plugin supports this feature. Conditions work like content filters for Backstage resources (provided by plugins). The RBAC backend API stores conditions assigned to the role in the database. When a user requests access to the frontend resources, the RBAC backend API searches for corresponding conditions and delegates the condition for this resource to the corresponding plugin by its plugin ID. If a user was assigned to multiple roles, and each of these roles contains its own condition, the RBAC backend merges conditions using the anyOf criteria. +Conditional permission policies are fairly complex. For more information on how to structure your conditional policies, consult our documentation on [conditions](./conditions.md). -The corresponding plugin analyzes conditional parameters and makes a decision about which part of the content the user should see. Consequently, the user can view not all resource content but only some allowed parts. The RBAC backend plugin supports conditions bounded to the RBAC role. +### GET conditional rules -A Backstage condition can be a simple condition with a rule and parameters. But also a Backstage condition could consists of a parameter or an array of parameters joined by criteria. The list of supported conditional criteria includes: +GET -- allOf -- anyOf -- not - -The plugin defines the supported condition parameters. API users can retrieve the conditional object schema from the RBAC API endpoint to determine how to build a condition JSON object and utilize it through the RBAC backend plugin API. - -The structure of the condition JSON object is as follows: - -| Json field | Description | Type | -| ----------------- | --------------------------------------------------------------------- | ------------ | -| result | Always has the value "CONDITIONAL" | String | -| roleEntityRef | String entity reference to the RBAC role ('role:default/dev') | String | -| pluginId | Corresponding plugin ID (e.g., "catalog") | String | -| permissionMapping | Array permission actions (['read', 'update', 'delete']) | String array | -| resourceType | Resource type provided by the plugin (e.g., "catalog-entity") | String | -| conditions | Condition JSON with parameters or array parameters joined by criteria | JSON | - -### GET - -GET - -Provides condition parameters schemas. +Provides conditional rule parameter schemas. ```json [ @@ -633,108 +612,6 @@ Provides condition parameters schemas. ] ``` -From this condition schema, the RBAC backend API user can determine how to build a condition JSON object. - -For example, consider a condition without criteria: displaying catalogs only if the user is a member of the owner group. The Catalog plugin schema "IS_ENTITY_OWNER" can be utilized to achieve this goal. To construct the condition JSON object based on this schema, the following information should be used: - -- rule: the parameter name is "IS_ENTITY_OWNER" in this case -- resourceType: "catalog-entity" -- criteria: in this example, criteria are not used since we need to use only one conditional parameter -- params: from the schema, it is evident that it should be an object named "claims" with a string array. This string array constitutes a list of user or group string entity references. - -Based on the above schema condition is: - -```json -{ - "rule": "IS_ENTITY_OWNER", - "resourceType": "catalog-entity", - "params": { - "claims": ["group:default/team-a"] - } -} -``` - -To utilize this condition to the RBAC REST api you need to wrap it with more info - -```json -{ - "result": "CONDITIONAL", - "roleEntityRef": "role:default/test", - "pluginId": "catalog", - "resourceType": "catalog-entity", - "permissionMapping": ["read"], - "conditions": { - "rule": "IS_ENTITY_OWNER", - "resourceType": "catalog-entity", - "params": { - "claims": ["group:default/team-a"] - } - } -} -``` - -**Example condition with criteria**: display catalogs only if user is a member of owner group "OR" display list of all catalog user groups. - -We can reuse previous condition parameter to display catalogs only for owner. Also we can use one more condition "IS_ENTITY_KIND" to display catalog groups for any user: - -- rule - the parameter name is "IS_ENTITY_KIND" in this case. -- resource type: "catalog-entity". -- criteria - "anyOf". -- params - from the schema, it is evident that it should be an object named "kinds" with string array. This string array is a list of catalog kinds. It should be array with single element "Group" in our case. - -Based on the above schema: - -```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"] - } - } - ] -} -``` - -To utilize this condition to the RBAC REST api you need to wrap it with more info: - -```json -{ - "result": "CONDITIONAL", - "roleEntityRef": "role:default/test", - "pluginId": "catalog", - "resourceType": "catalog-entity", - "permissionMapping": ["read"], - "conditions": { - "anyOf": [ - { - "rule": "IS_ENTITY_OWNER", - "resourceType": "catalog-entity", - "params": { - "claims": ["group:default/team-a"] - } - }, - { - "rule": "IS_ENTITY_KIND", - "resourceType": "catalog-entity", - "params": { - "kinds": ["Group"] - } - } - ] - } -} -``` - --- ### POST condition diff --git a/plugins/rbac-backend/docs/conditions.md b/plugins/rbac-backend/docs/conditions.md new file mode 100644 index 0000000000..32bdb70e74 --- /dev/null +++ b/plugins/rbac-backend/docs/conditions.md @@ -0,0 +1,353 @@ +# Conditional Permission Policies + +The Backstage permission framework provides conditions, and the RBAC backend plugin supports this feature. Conditions work like content filters for Backstage resources (provided by plugins). The RBAC backend API stores conditions assigned to the role in the database. When a user requests access to the frontend resources, the RBAC backend API searches for corresponding conditions and delegates the condition for this resource to the corresponding plugin by its plugin ID. If a user was assigned to multiple roles, and each of these roles contains its own condition, the RBAC backend merges conditions using the anyOf criteria. + +The corresponding plugin analyzes conditional parameters and makes a decision about which part of the content the user should see. Consequently, the user can view not all resource content but only some allowed parts. The RBAC backend plugin supports conditions bounded to the RBAC role. + +A Backstage condition can be a simple condition with a rule and parameters. But also a Backstage condition could consists of a parameter or an array of parameters joined by criteria. The list of supported conditional criteria includes: + +- allOf +- anyOf +- not + +The plugin defines the supported condition parameters. API users can retrieve the conditional object schema from the RBAC API endpoint to determine how to build a condition JSON object and utilize it through the RBAC backend plugin API. + +The structure of the condition JSON object is as follows: + +| Json field | Description | Type | +| ----------------- | --------------------------------------------------------------------- | ------------ | +| result | Always has the value "CONDITIONAL" | String | +| roleEntityRef | String entity reference to the RBAC role ('role:default/dev') | String | +| pluginId | Corresponding plugin ID (e.g., "catalog") | String | +| permissionMapping | Array permission actions (['read', 'update', 'delete']) | String array | +| resourceType | Resource type provided by the plugin (e.g., "catalog-entity") | String | +| conditions | Condition JSON with parameters or array parameters joined by criteria | JSON | + +To get the available conditional rules that can be used to create conditional permission policies, use the GET API request `api/plugins/condition-rules` as seen below. + +GET + +Provides condition parameters schemas. + +```json +[ + { + "pluginId": "catalog", + "rules": [ + { + "name": "HAS_ANNOTATION", + "description": "Allow entities with the specified annotation", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "annotation": { + "type": "string", + "description": "Name of the annotation to match on" + }, + "value": { + "type": "string", + "description": "Value of the annotation to match on" + } + }, + "required": [ + "annotation" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + }, + { + "name": "HAS_LABEL", + "description": "Allow entities with the specified label", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "label": { + "type": "string", + "description": "Name of the label to match on" + } + }, + "required": [ + "label" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + }, + { + "name": "HAS_METADATA", + "description": "Allow entities with the specified metadata subfield", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "key": { + "type": "string", + "description": "Property within the entities metadata to match on" + }, + "value": { + "type": "string", + "description": "Value of the given property to match on" + } + }, + "required": [ + "key" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + }, + { + "name": "HAS_SPEC", + "description": "Allow entities with the specified spec subfield", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "key": { + "type": "string", + "description": "Property within the entities spec to match on" + }, + "value": { + "type": "string", + "description": "Value of the given property to match on" + } + }, + "required": [ + "key" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + }, + { + "name": "IS_ENTITY_KIND", + "description": "Allow entities matching a specified kind", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "kinds": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of kinds to match at least one of" + } + }, + "required": [ + "kinds" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + }, + { + "name": "IS_ENTITY_OWNER", + "description": "Allow entities owned by a specified claim", + "resourceType": "catalog-entity", + "paramsSchema": { + "type": "object", + "properties": { + "claims": { + "type": "array", + "items": { + "type": "string" + }, + "description": "List of claims to match at least one on within ownedBy" + } + }, + "required": [ + "claims" + ], + "additionalProperties": false, + "$schema": "http://json-schema.org/draft-07/schema#" + } + } + ] + } + ... +] +``` + +From this condition schema, the RBAC backend API user can determine how to build a condition JSON object. + +For example, consider a condition without criteria: displaying catalogs only if the user is a member of the owner group. The Catalog plugin schema "IS_ENTITY_OWNER" can be utilized to achieve this goal. To construct the condition JSON object based on this schema, the following information should be used: + +- rule: the parameter name is "IS_ENTITY_OWNER" in this case +- resourceType: "catalog-entity" +- criteria: in this example, criteria are not used since we need to use only one conditional parameter +- params: from the schema, it is evident that it should be an object named "claims" with a string array. This string array constitutes a list of user or group string entity references. + +Based on the above schema condition is: + +```json +{ + "rule": "IS_ENTITY_OWNER", + "resourceType": "catalog-entity", + "params": { + "claims": ["group:default/team-a"] + } +} +``` + +To utilize this condition to the RBAC REST api you need to wrap it with more info + +```json +{ + "result": "CONDITIONAL", + "roleEntityRef": "role:default/test", + "pluginId": "catalog", + "resourceType": "catalog-entity", + "permissionMapping": ["read"], + "conditions": { + "rule": "IS_ENTITY_OWNER", + "resourceType": "catalog-entity", + "params": { + "claims": ["group:default/team-a"] + } + } +} +``` + +**Example condition with criteria**: display catalogs only if user is a member of owner group "OR" display list of all catalog user groups. + +We can reuse previous condition parameter to display catalogs only for owner. Also we can use one more condition "IS_ENTITY_KIND" to display catalog groups for any user: + +- rule - the parameter name is "IS_ENTITY_KIND" in this case. +- resource type: "catalog-entity". +- criteria - "anyOf". +- params - from the schema, it is evident that it should be an object named "kinds" with string array. This string array is a list of catalog kinds. It should be array with single element "Group" in our case. + +Based on the above schema: + +```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"] + } + } + ] +} +``` + +To utilize this condition to the RBAC REST api you need to wrap it with more info: + +```json +{ + "result": "CONDITIONAL", + "roleEntityRef": "role:default/test", + "pluginId": "catalog", + "resourceType": "catalog-entity", + "permissionMapping": ["read"], + "conditions": { + "anyOf": [ + { + "rule": "IS_ENTITY_OWNER", + "resourceType": "catalog-entity", + "params": { + "claims": ["group:default/team-a"] + } + }, + { + "rule": "IS_ENTITY_KIND", + "resourceType": "catalog-entity", + "params": { + "kinds": ["Group"] + } + } + ] + } +} +``` + +## Examples of Conditional Policies + +Below are a few examples that can be used on some of the Janus IDP plugins. These can help in determining how based to define conditional policies + +### Keycloak plugin + +```JSON +{ + "result": "CONDITIONAL", + "roleEntityRef": "role:default/developer", + "pluginId": "catalog", + "resourceType": "catalog-entity", + "permissionMapping": ["update", "delete"], + "conditions": { + "not": { + "rule": "HAS_ANNOTATION", + "resourceType": "catalog-entity", + "params": { "annotation": "keycloak.org/realm", "value": "" } + } + } +} +``` + +This example will prevent users in the role `role:default/developer` from updating or deleting users that ingested into the catalog from the Keycloak plugin. + +Notice the use of the annotation `keycloak.org/realm` requires the value of `` + +### Quay Actions + +```JSON +{ + "result": "CONDITIONAL", + "roleEntityRef": "role:default/developer", + "pluginId": "scaffolder", + "resourceType": "scaffolder-action", + "permissionMapping": ["use"], + "conditions": { + "not": { + "rule": "HAS_ACTION_ID", + "resourceType": "scaffolder-action", + "params": { "actionId": "quay:create-repository" } + } + } +} +``` + +This example will prevent users from using the Quay scaffolder action if they are a part of the role `role:default/developer`. + +Notice, we use the `permissionMapping` field with `use`. This is because the `scaffolder-action` resource type permission does not have a permission policy. More information can be found in our documentation on [permissions](./permissions.md). + +**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"] } + } +} +``` 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 1890c93421..976e6727ae 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 cbe6655794..81834c7b2d 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 04e4476a35..fc9db4e62f 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 f0a3e5a78d..21bbe231f7 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( @@ -999,7 +999,8 @@ export class PoliciesServer { const perm = rule.permissions.find( permission => permission.type === 'resource' && - action === permission.attributes.action, + (action === permission.attributes.action || + (action === 'use' && permission.attributes.action === undefined)), ); if (!perm) { throw new Error( diff --git a/plugins/rbac-backend/src/service/condition.validation.test.ts b/plugins/rbac-backend/src/validation/condition-validation.test.ts similarity index 75% rename from plugins/rbac-backend/src/service/condition.validation.test.ts rename to plugins/rbac-backend/src/validation/condition-validation.test.ts index 48cf9d770f..d6b5246519 100644 --- a/plugins/rbac-backend/src/service/condition.validation.test.ts +++ b/plugins/rbac-backend/src/validation/condition-validation.test.ts @@ -283,6 +283,30 @@ describe('condition-validation', () => { } expect(unexpectedErr).toBeUndefined(); }); + + it('should validate role-condition.conditions with permission policy action of use without errors', () => { + const condition: any = { + pluginId: 'scaffolder', + resourceType: 'scaffolder-action', + roleEntityRef: 'role:default/test', + result: AuthorizeResult.CONDITIONAL, + permissionMapping: ['use'], + conditions: { + rule: 'HAS_ACTION_ID', + resourceType: 'scaffolder-action', + params: { + actionId: 'quay:create-repository', + }, + }, + }; + let unexpectedErr; + try { + validateRoleCondition(condition); + } catch (err) { + unexpectedErr = err; + } + expect(unexpectedErr).toBeUndefined(); + }); }); describe('validate "not" criteria', () => { @@ -351,6 +375,7 @@ describe('condition-validation', () => { } catch (err) { unexpectedErr = err; } + expect(unexpectedErr).toBeUndefined(); }); }); @@ -724,4 +749,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 f87052b938..6957c86099 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 99188fd977..cc9c54b45d 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 ba8e2536b8..356832331d 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[],