fix: fix subject with HR scope matching on Policy target

restorecommerce · Apr 12, 2024 · 532befe · 532befe
1 parent da1b32e
commit 532befe
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 4 deletions.
diff --git a/src/core/accessController.ts b/src/core/accessController.ts
@@ -178,6 +178,14 @@ export class AccessController {
           ) {
             const rules: Map<string, Rule> = policy.combinables;
             this.logger.verbose(`Checking policy ${policy.name}`);
+            let policySubjectMatch: boolean;
+            // Subject set on Policy validate HR scope matching
+            if (policy?.target?.subjects?.length > 0) {
+              this.logger.verbose(`Checking Policy subject HR Scope match for ${policy.name}`);
+              policySubjectMatch = await checkHierarchicalScope(policy.target, request, this.urns, this, this.logger);
+            } else {
+              policySubjectMatch = true;
+            }
             // only apply a policy effect if there are no rules
             // combine rules otherwise
             if (rules.size == 0 && !!policy.effect) {
@@ -259,7 +267,7 @@ export class AccessController {
                     matches = await verifyACLList(rule.target, request, this.urns, this, this.logger);
                   }
 
-                  if (matches) {
+                  if (matches && policySubjectMatch) {
                     if (!evaluationCacheableRule) {
                       evaluation_cacheable = evaluationCacheableRule;
                     }
@@ -780,7 +788,7 @@ export class AccessController {
       return true;
     }
     ruleSubAttributes?.forEach((subjectObject) => {
-      if(subjectObject?.id === roleURN) {
+      if (subjectObject?.id === roleURN) {
         ruleRole = subjectObject?.value;
       }
     });
@@ -791,12 +799,12 @@ export class AccessController {
       return true;
     }
 
-    if(!ruleRole) {
+    if (!ruleRole) {
       this.logger.warn(`Subject does not match with rule attributes`, ruleSubAttributes);
       return false;
     }
     const context = (request as any)?.context as ContextWithSubResolved;
-    if(!context?.subject?.role_associations) {
+    if (!context?.subject?.role_associations) {
       this.logger.warn('Subject role associations missing', ruleSubAttributes);
       return false;
     }

diff --git a/test/core.spec.ts b/test/core.spec.ts
@@ -368,6 +368,38 @@ describe('Testing access control core', () => {
       await requestAndValidate(ac, request, Response_Decision.PERMIT);
     });
 
+    it('should INDETERMINATE based on rule R6 as subject HR scope does not match', async () => {
+      request = testUtils.buildRequest({
+        subjectID: 'Alice',
+        subjectRole: 'SimpleUser',
+        roleScopingEntity: 'urn:restorecommerce:acs:model:organization.Organization',
+        roleScopingInstance: 'Org1',
+        resourceType: 'urn:restorecommerce:acs:model:location.Location',
+        resourceID: 'Random',
+        actionType: 'urn:restorecommerce:acs:names:action:modify',
+        ownerIndicatoryEntity: 'urn:restorecommerce:acs:model:organization.Organization',
+        ownerInstance: 'Org4' // does not exist in HR scope
+      });
+
+      await requestAndValidate(ac, request, Response_Decision.INDETERMINATE);
+    });
+
+    it('should PERMIT based on rule R6 as subject HR scope matches', async () => {
+      request = testUtils.buildRequest({
+        subjectID: 'Alice',
+        subjectRole: 'SimpleUser',
+        roleScopingEntity: 'urn:restorecommerce:acs:model:organization.Organization',
+        roleScopingInstance: 'Org1',
+        resourceType: 'urn:restorecommerce:acs:model:location.Location',
+        resourceID: 'Random',
+        actionType: 'urn:restorecommerce:acs:names:action:modify',
+        ownerIndicatoryEntity: 'urn:restorecommerce:acs:model:organization.Organization',
+        ownerInstance: 'Org2' // exist in HR scope
+      });
+
+      await requestAndValidate(ac, request, Response_Decision.PERMIT);
+    });
+
     it('should DENY based on Rule BA2', async () => {
       request = testUtils.buildRequest({
         subjectID: 'External Bob',

diff --git a/test/fixtures/policy_sets_with_targets.yml b/test/fixtures/policy_sets_with_targets.yml
@@ -80,3 +80,30 @@ policy_sets:
                 - id: urn:oasis:names:tc:xacml:1.0:action:action-id
                   value: urn:restorecommerce:acs:names:action:read
             effect: PERMIT
+  - id: PS3
+    name: 'Policy set C'
+    description: 'A policy set targeting Org scoping with Subject scoping defined on Policy level instead of Rule'
+    combining_algorithm: urn:oasis:names:tc:xacml:3.0:rule-combining-algorithm:permit-overrides      
+    policies:
+      - id: P3
+        name: Policy with Org Scoping for Location Resource
+        description: A Policy targeting access for Org scope users for Location resource
+        combining_algorithm: urn:oasis:names:tc:xacml:3.0:rule-combining-algorithm:permit-overrides
+        target:
+          subjects:
+            - id: urn:restorecommerce:acs:names:role
+              value: SimpleUser
+            - id: urn:restorecommerce:acs:names:roleScopingEntity
+              value: urn:restorecommerce:acs:model:organization.Organization
+          resources:
+            - id: urn:restorecommerce:acs:names:model:entity
+              value: urn:restorecommerce:acs:model:location.Location
+        rules:
+          - id: R6
+            name: Rule 5
+            description: A rule targeting 'modify' actions on Location resource
+            target:
+              actions:
+                - id: urn:oasis:names:tc:xacml:1.0:action:action-id
+                  value: urn:restorecommerce:acs:names:action:modify
+            effect: PERMIT