fix(rbac): fix sonarcloud issues

cloud-eda · Jan 29, 2024 · 271d6ea · 271d6ea
1 parent c340485
commit 271d6ea
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 41 deletions.
diff --git a/plugins/rbac-backend/src/helper.ts b/plugins/rbac-backend/src/helper.ts
@@ -26,10 +26,12 @@ export async function removeTheDifference(
   roleName: string,
   enf: EnforcerDelegate,
 ): Promise<void> {
-  const missing = difference(originalGroup.sort(), addedGroup.sort());
+  originalGroup.sort((a, b) => a.localeCompare(b));
+  addedGroup.sort((a, b) => a.localeCompare(b));
+  const missing = difference(originalGroup, addedGroup);
 
-  missing.forEach(async name => {
-    const role = [name, roleName];
+  for (const missingRole of missing) {
+    const role = [missingRole, roleName];
     await enf.removeGroupingPolicy(role, source, true);
-  });
+  }
 }
diff --git a/plugins/rbac-backend/src/service/enforcer-delegate.ts b/plugins/rbac-backend/src/service/enforcer-delegate.ts
@@ -58,7 +58,7 @@ export class EnforcerDelegate {
     source: Source,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
 
     try {
       await this.policyMetadataStorage.createPolicyMetadata(
@@ -95,7 +95,7 @@ export class EnforcerDelegate {
           trx,
         );
       }
-      const ok = this.enforcer.addPolicies(policies);
+      const ok = await this.enforcer.addPolicies(policies);
       if (!ok) {
         throw new Error(
           `Failed to store policies ${policiesToString(policies)}`,
@@ -118,7 +118,7 @@ export class EnforcerDelegate {
     externalTrx?: Knex.Transaction,
     isUpdate?: boolean,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     const entityRef = policy[1];
     let metadata;
 
@@ -165,7 +165,7 @@ export class EnforcerDelegate {
     externalTrx?: Knex.Transaction,
     isUpdate?: boolean,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
 
     try {
       for (const policy of policies) {
@@ -215,7 +215,7 @@ export class EnforcerDelegate {
     allowToDeleteCSVFilePolicy?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     const newRoleName = newRole.at(0)?.at(1)!;
     const oldRoleName = oldRole.at(0)?.at(1)!;
     try {
@@ -250,7 +250,7 @@ export class EnforcerDelegate {
     allowToDeleteCSVFilePolicy?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
 
     try {
       await this.removePolicies(
@@ -277,7 +277,7 @@ export class EnforcerDelegate {
     allowToDeleteCSVFilePolicy?: boolean,
     externalTrx?: Knex.Transaction,
   ) {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
 
     try {
       await this.checkIfPolicyModifiable(
@@ -308,7 +308,7 @@ export class EnforcerDelegate {
     allowToDeleCSVFilePolicy?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
 
     try {
       for (const policy of policies) {
@@ -345,7 +345,7 @@ export class EnforcerDelegate {
     allowToDeleCSVFilePolicy?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     const roleEntity = policy[1];
 
     try {
@@ -381,7 +381,7 @@ export class EnforcerDelegate {
     isUpdate?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     try {
       for (const policy of policies) {
         const roleEntity = policy[1];
@@ -421,7 +421,7 @@ export class EnforcerDelegate {
     isCSV: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     try {
       if (!(await this.enforcer.hasPolicy(...policy))) {
         await this.addPolicy(policy, source, trx);
@@ -446,7 +446,7 @@ export class EnforcerDelegate {
     isCSV?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     try {
       if (!(await this.hasGroupingPolicy(...groupPolicy))) {
         await this.addGroupingPolicy(groupPolicy, source, trx);
@@ -478,7 +478,7 @@ export class EnforcerDelegate {
     isCSV?: boolean,
     externalTrx?: Knex.Transaction,
   ): Promise<void> {
-    const trx = externalTrx || (await this.knex.transaction());
+    const trx = externalTrx ?? (await this.knex.transaction());
     try {
       for (const groupPolicy of groupPolicies) {
         if (!(await this.hasGroupingPolicy(...groupPolicy))) {
@@ -576,7 +576,7 @@ export class EnforcerDelegate {
     policy: string[],
     source: Source,
     externalTrx?: Knex.Transaction,
-  ): Promise<Boolean> {
+  ): Promise<boolean> {
     const metadata = await this.policyMetadataStorage.findPolicyMetadata(
       policy,
       externalTrx,

diff --git a/plugins/rbac-backend/src/service/permission-policy.ts b/plugins/rbac-backend/src/service/permission-policy.ts
@@ -33,20 +33,15 @@ const useAdmins = async (
   roleMetadataStorage: RoleMetadataStorage,
   knex: Knex,
 ) => {
-  const rbacAdminsGroupPolicies: string[][] = [];
   const groupPoliciesToCompare: string[] = [];
   const addedGroupPolicies = new Map<string, string>();
 
-  admins.forEach(async localConfig => {
-    const entityRef = localConfig.getString('name');
+  for (const admin of admins) {
+    const entityRef = admin.getString('name');
     validateEntityReference(entityRef);
 
-    const groupPolicy = [entityRef, adminRoleName];
-    if (!(await enf.hasGroupingPolicy(...groupPolicy))) {
-      rbacAdminsGroupPolicies.push(groupPolicy);
-    }
     addedGroupPolicies.set(entityRef, adminRoleName);
-  });
+  }
 
   const adminRoleMeta =
     await roleMetadataStorage.findRoleMetadata(adminRoleName);
@@ -131,7 +126,7 @@ const removedOldPermissionPoliciesFileData = async (
   fileEnf?: Enforcer,
 ) => {
   const tempEnforcer =
-    fileEnf || (await newEnforcer(newModelFromString(MODEL)));
+    fileEnf ?? (await newEnforcer(newModelFromString(MODEL)));
   const oldFilePolicies = new Set<string[]>();
   const policiesMetadata = await enf.getFilteredPolicyMetadata('csv-file');
   for (const policyMetadata of policiesMetadata) {
@@ -213,6 +208,14 @@ export class RBACPermissionPolicy implements PermissionPolicy {
       'permission.rbac.policies-csv-file',
     );
 
+    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.',
+      );
+    }
+
     if (policiesFile) {
       await addPermissionPoliciesFileData(
         policiesFile,
@@ -223,19 +226,6 @@ export class RBACPermissionPolicy implements PermissionPolicy {
       await removedOldPermissionPoliciesFileData(enforcerDelegate);
     }
 
-    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(
       enforcerDelegate,
       logger,

diff --git a/plugins/rbac-backend/src/service/policies-rest-api.ts b/plugins/rbac-backend/src/service/policies-rest-api.ts
@@ -771,7 +771,7 @@ export class PolicesServer {
       const err = validatePolicy(policy);
       if (err) {
         throw new InputError(
-          `Invalid ${errorMessage || 'policy'} definition. Cause: ${
+          `Invalid ${errorMessage ?? 'policy'} definition. Cause: ${
             err.message
           }`,
         ); // 400