Merge pull request #583 from telefonicaid/fix/local_pdp_permit_when_any_role

Fix/local pdp not stop at first match

Author: fgalan

CHANGES_NEXT_RELEASE

@@ -1 +1,2 @@
 - Upgrade express from 4.21.2 to 4.22.1
+- Fix: local PDP evaluates all roles and grants access if any applicable role permits the action, not stop at first match (#582)

lib/services/pdp.js

@@ -50,15 +50,14 @@ const ACTION_MAP = {
 
 
 /**
- * Helper para cacheo y respuesta
+ * Helper for caching and response
  */
 function cacheAndReturn(cacheKey, decision, callback) {
     const finalDecision = decision || 'Invalid';
     cacheUtils.get().data.validation.set(cacheKey, finalDecision);
     callback(null, finalDecision);
 }
 
-
 /**
  * validation request using local PDP implementation
  */
@@ -77,47 +76,51 @@ function validationRequest(logger, roles, frn, action, headers, callback) {
             return callback(new Error('Invalid FRN format'));
         }
 
-        const component = frnParts[1].toUpperCase();                  // ORION, PERSEO, IOTA, STH
-        const subserviceRaw = frnParts[3];                            // "/tourism", "///", etc
+        const component = frnParts[1].toUpperCase();                  // ORION, PERSEO, IOTAGENT, STH
+        const subserviceRaw = frnParts[3];                            // "/tourism", "/", etc
         const subservice = subserviceRaw.replace(/\//g, '') || null;  // "tourism" o null
 
         const isServiceOperation = subservice === null;
         const isSubserviceOperation = subservice !== null;
 
         // 2. For each role: get roleType and component
-        let matchedRole = null;
+        let hasMatchingRole = false;
+        let isPermitted = false;
 
         for (const role of roles) {
             const name = (role.name || '').trim();
 
             // if name role with '#', then get right part; otherwise name as is
             const hashParts = name.split('#');
             const roleInfoRaw = (hashParts.length === 2 ? hashParts[1] : hashParts[0]).trim();
+
             // Alias: admin (without #) = ServiceAdmin for all components
             const roleInfo = /^admin$/i.test(roleInfoRaw) ? 'ServiceAdmin' : roleInfoRaw;
 
-            // Try extrat type and component (i.e.: ServiceCustomerORION)
+            // Try extract type and component (i.e.: ServiceCustomerORION)
             let match = roleInfo.match(
-                /(ServiceCustomer|ServiceAdmin|SubServiceCustomer|SubServiceAdmin)([A-Z]+)$/i
+                /^(ServiceCustomer|ServiceAdmin|SubServiceCustomer|SubServiceAdmin)([A-Z]+)$/i
             );
 
-            let roleType, roleComponent;
+            let roleType;
+            let roleComponent;
 
-            // Case 1: rol includes component
+            // Case 1: role includes component
             if (match) {
                 roleType = match[1];
                 roleComponent = match[2].toUpperCase();
             }
-            // Caso 2: rol does NOT includes component -> apply over all components
+            // Case 2: role does NOT include component -> apply over all components
             else {
                 match = roleInfo.match(
                     /^(ServiceCustomer|ServiceAdmin|SubServiceCustomer|SubServiceAdmin)$/i
                 );
+
                 if (!match) {
                     continue;
                 }
 
-                roleType = match[1]; 
+                roleType = match[1];
                 roleComponent = 'ANY';
             }
 
@@ -137,27 +140,20 @@ function validationRequest(logger, roles, frn, action, headers, callback) {
                 continue;
             }
 
-            matchedRole = { roleType, roleComponent };
-            break;
-        } // end for
-
-        if (!matchedRole) {
-            cacheAndReturn(cacheKey, 'Deny', callback);
-            return;
-        }
-
-        // 3. Final decision
-        const { roleType } = matchedRole;
-        const ROLE_CLASS = roleType.endsWith('Customer') ? 'CUSTOMER' : 'ADMIN';
+            hasMatchingRole = true;
 
-        let allowedActions = [];
+            // 3) Check whether THIS role permits the action
+            const ROLE_CLASS = roleType.endsWith('Customer') ? 'CUSTOMER' : 'ADMIN';
+            const allowedActions =
+                (ACTION_MAP[component] && ACTION_MAP[component][ROLE_CLASS]) || [];
 
-        if (ACTION_MAP[component] && ACTION_MAP[component][ROLE_CLASS]) {
-            allowedActions = ACTION_MAP[component][ROLE_CLASS];
+            if (allowedActions.includes(action)) {
+                isPermitted = true;
+                break; // one permitting role is enough
+            }
         }
 
-        const decision = allowedActions.includes(action) ? 'Permit' : 'Deny';
-
+        const decision = (hasMatchingRole && isPermitted) ? 'Permit' : 'Deny';
         cacheAndReturn(cacheKey, decision, callback);
 
     } catch (err) {
@@ -166,5 +162,4 @@ function validationRequest(logger, roles, frn, action, headers, callback) {
     }
 }
 
-
 exports.validationRequest = validationRequest;

test/unit/validate_user_local_pdp_action_test.js

@@ -133,6 +133,21 @@ describe('Local PDP validationRequest decision tree', function () {
         .catch(done);
     });
 
+    it('admin and ServiceCustomer without component can CREATE in ORION', function (done) {
+        runValidation({
+            roles: [{ id: '1', name: 'x#ServiceCustomer' },
+                    { id: '2', name: 'admin' }                    
+                   ],
+            frn: 'fiware:orion:smartcity:/:::',
+            action: 'create'
+        })
+        .then(function (decision) {
+            decision.should.equal('Permit');
+            done();
+        })
+        .catch(done);
+    });
+
     it('ServiceAdminORION can DELETE in ORION', function (done) {
         runValidation({
             roles: [{ id: '1', name: 'x#ServiceAdminORION' }],
@@ -146,9 +161,24 @@ describe('Local PDP validationRequest decision tree', function () {
         .catch(done);
     });
 
+    it('ServiceCustomer and ServiceAdminORION can DELETE in ORION', function (done) {
+        runValidation({
+            roles: [{ id: '1', name: 'x#ServiceAdminORION' },
+                    { id: '2', name: 'x#ServiceCustomer' }],
+            frn: 'fiware:orion:smartcity:/:::',
+            action: 'delete'
+        })
+        .then(function (decision) {
+            decision.should.equal('Permit');
+            done();
+        })
+        .catch(done);
+    });
+
     it('ServiceAdminORION cannot operate on PERSEO', function (done) {
         runValidation({
-            roles: [{ id: '1', name: 'x#ServiceAdminORION' }],
+            roles: [{ id: '1', name: 'x#ServiceAdminORION' },
+                    { id: '3', name: 'x#ServiceAdminSTH' } ],
             frn: 'fiware:perseo:smartcity:/:::',
             action: 'readRule'
         })
@@ -161,7 +191,8 @@ describe('Local PDP validationRequest decision tree', function () {
 
     it('SubServiceCustomer can READ ORION at subservice level', function (done) {
         runValidation({
-            roles: [{ id: '1', name: 'x#SubServiceCustomer' }],
+            roles: [{ id: '4', name: 'x#SubServiceCustomerPERSEO' },
+                    { id: '1', name: 'x#SubServiceCustomer' }],
             frn: 'fiware:orion:smartcity:/tourism:::',
             action: 'read'
         })