From f68e54fefb6e85cd2a8ef62009baf3e4c6b6ec09 Mon Sep 17 00:00:00 2001
From: Oleksandr Andriienko <oandriie@redhat.com>
Date: Sun, 28 Jan 2024 11:42:57 +0200
Subject: [PATCH] fix(rbac): complete fix bug #1103 Complete fix bug, when
 after removing admin from app configuration, admin still present.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
---
 .../src/service/enforcer-delegate.ts          |  8 ++--
 .../src/service/permission-policy.ts          | 46 +++++++++++--------
 2 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts
index 4127521101..1ded34e2cd 100644
--- 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
index 4f97120afa..38cbefdeab 100644
--- 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(
@@ -226,8 +223,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(