fix(rbac): complete fix bug janus-idp#1103

Complete fix bug, when after removing admin from app configuration, admin still present. Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
cloud-eda · Jan 31, 2024 · fdbbd46 · fdbbd46
1 parent 365876a
commit fdbbd46
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 23 deletions.
diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts
@@ -118,15 +118,17 @@ export class EnforcerDelegate {
     externalTrx?: Knex.Transaction,
     isUpdate?: boolean,
   ): Promise<void> {
+    const trx = externalTrx || (await this.knex.transaction());
     const entityRef = policy[1];
     let metadata;
 
     if (entityRef.startsWith(`role:`)) {
-      metadata = await this.roleMetadataStorage.findRoleMetadata(entityRef);
+      metadata = await this.roleMetadataStorage.findRoleMetadata(
+        entityRef,
+        trx,
+      );
     }
 
-    const trx = externalTrx || (await this.knex.transaction());
-
     try {
       await this.policyMetadataStorage.createPolicyMetadata(
         source,

diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts
@@ -33,7 +33,6 @@ const useAdmins = async (
   roleMetadataStorage: RoleMetadataStorage,
   knex: Knex,
 ) => {
-  let legacy = false;
   const rbacAdminsGroupPolicies: string[][] = [];
   const groupPoliciesToCompare: string[] = [];
   const addedGroupPolicies: string[] = [];
@@ -45,36 +44,34 @@ const useAdmins = async (
     const groupPolicy = [entityRef, adminRoleName];
     if (!(await enf.hasGroupingPolicy(...groupPolicy))) {
       rbacAdminsGroupPolicies.push(groupPolicy);
-      addedGroupPolicies.push(entityRef);
     }
+    addedGroupPolicies.push(entityRef);
   });
 
   const adminRoleMeta =
     await roleMetadataStorage.findRoleMetadata(adminRoleName);
 
-  if (adminRoleMeta?.source === 'legacy') {
-    const trx = await knex.transaction();
-    try {
+  const trx = await knex.transaction();
+  try {
+    if (!adminRoleMeta) {
+      await roleMetadataStorage.createRoleMetadata(
+        { source: 'configuration' },
+        adminRoleName,
+        trx,
+      );
+    } else if (adminRoleMeta.source === 'legacy') {
       await roleMetadataStorage.removeRoleMetadata(adminRoleName, trx);
-      await trx.commit();
-      legacy = true;
-    } catch (error) {
-      await trx.rollback(error);
-    }
-  }
-
-  if (!adminRoleMeta || legacy) {
-    const trx = await knex.transaction();
-    try {
       await roleMetadataStorage.createRoleMetadata(
         { source: 'configuration' },
         adminRoleName,
         trx,
       );
-      await trx.commit();
-    } catch (error) {
-      await trx.rollback(error);
     }
+
+    await trx.commit();
+  } catch (error) {
+    await trx.rollback(error);
+    throw error;
   }
 
   await enf.addOrUpdateGroupingPolicies(
@@ -235,8 +232,17 @@ export class RBACPermissionPolicy implements PermissionPolicy {
       await removedOldPermissionPoliciesFileData(enforcerDelegate);
     }
 
-    if (adminUsers) {
-      await useAdmins(adminUsers, enforcerDelegate, roleMetadataStorage, knex);
+    if (adminUsers && adminUsers.length > 0) {
+      await useAdmins(
+        adminUsers || [],
+        enforcerDelegate,
+        roleMetadataStorage,
+        knex,
+      );
+    } else {
+      logger.warn(
+        'There are no admins configured for the RBAC-backend plugin. The plugin may not work properly.',
+      );
     }
 
     return new RBACPermissionPolicy(