fix(rbac): lowercase user and group entity references

workspaces/rbac/.changeset/new-moose-nail.md

@@ -0,0 +1,5 @@
+---
+'@backstage-community/plugin-rbac-backend': minor
+---
+
+Roles and permissions were not correctly applied for users and groups with names containing uppercase letters. To address this issue, we now convert user and group references in all user inputs to lowercase. This change migrates `v0` column in `casbin_rule` table in `backstage_plugin_permission` database. Conditions containing claims with uppercase letters are not resolved yet.

workspaces/rbac/plugins/rbac-backend/__fixtures__/data/invalid-csv/duplicate-policy.csv

@@ -3,10 +3,15 @@ g, user:default/guest, role:default/catalog-deleter
 
 g, user:default/guest, role:default/catalog-updater
 
+g, group:default/READER-GROUP, role:default/CATALOG-USER
+g, group:default/READER-GROUP, role:default/CATALOG-USER
+
 p, role:default/catalog-writer, catalog.entity.create, use, allow
 p, role:default/catalog-writer, catalog.entity.create, use, allow
 
 p, role:default/catalog-writer, catalog-entity, delete, allow
 
 p, role:default/duplication-effect, catalog-entity, update, allow
 p, role:default/duplication-effect, catalog-entity, update, deny
+
+p, role:default/CATALOG-USER, catalog-entity, read, allow

workspaces/rbac/plugins/rbac-backend/__fixtures__/data/valid-csv/rbac-policy.csv

@@ -8,7 +8,10 @@ p, role:default/legacy, catalog-entity, update, allow
 p, role:default/catalog-writer, catalog-entity, read, allow
 p, role:default/catalog-writer, catalog.entity.create, use, allow
 p, role:default/catalog-deleter, catalog-entity, delete, deny
+p, role:default/CATALOG-USER, catalog-entity, read, allow
 
 p, role:default/known_role, test.resource.deny, use, allow
 
 g, user:default/known_user, role:default/known_role
+g, user:default/TOM, role:default/CATALOG-USER
+g, group:default/READER-GROUP, role:default/CATALOG-USER

workspaces/rbac/plugins/rbac-backend/__fixtures__/data/valid-csv/uppercase-policy.csv

@@ -0,0 +1,7 @@
+p, role:default/CATALOG-USER, catalog-entity, read, allow
+p, role:default/known_role, test.resource.deny, use, allow
+
+g, user:default/known_user, role:default/known_role
+g, user:default/TOM, role:default/CATALOG-USER
+g, group:default/READER-GROUP, role:default/CATALOG-USER
+g, group:default/READER-GROUP, role:default/known_role

workspaces/rbac/plugins/rbac-backend/migrations/20241108093910_migrations.js

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+exports.up = async function up(knex) {
+  const casbinExists = await knex.schema.hasTable('casbin_rule');
+  if (casbinExists) {
+    await knex('casbin_rule')
+      .whereNotNull('v0')
+      .where(function groups() {
+        this.where('v0', 'like', 'user:%').orWhere('v0', 'like', 'group:%');
+      })
+      .update({
+        v0: knex.raw('LOWER(??)', ['v0']),
+      });
+  }
+};
+
+/**
+ * @param { import("knex").Knex } knex
+ * @returns { Promise<void> }
+ */
+exports.down = async function down(_knex) {};

workspaces/rbac/plugins/rbac-backend/src/admin-permissions/admin-creation.ts

@@ -63,7 +63,7 @@ export const useAdminsFromConfig = async (
   const newGroupPolicies = new Map<string, string>();
 
   for (const admin of admins) {
-    const entityRef = admin.getString('name');
+    const entityRef = admin.getString('name').toLocaleLowerCase('en-US');
     validateEntityReference(entityRef);
 
     addedGroupPolicies.set(entityRef, ADMIN_ROLE_NAME);

workspaces/rbac/plugins/rbac-backend/src/file-permissions/csv-file-watcher.test.ts

@@ -147,6 +147,7 @@ const currentPermissionPolicies = [
   ['role:default/catalog-writer', 'catalog-entity', 'read', 'allow'],
   ['role:default/catalog-writer', 'catalog.entity.create', 'use', 'allow'],
   ['role:default/catalog-deleter', 'catalog-entity', 'delete', 'deny'],
+  ['role:default/CATALOG-USER', 'catalog-entity', 'read', 'allow'],
   ['role:default/known_role', 'test.resource.deny', 'use', 'allow'],
 ];
 
@@ -156,6 +157,8 @@ const currentRoles = [
   ['user:default/guest', 'role:default/catalog-reader'],
   ['user:default/guest', 'role:default/catalog-deleter'],
   ['user:default/known_user', 'role:default/known_role'],
+  ['user:default/tom', 'role:default/CATALOG-USER'],
+  ['group:default/reader-group', 'role:default/CATALOG-USER'],
 ];
 
 describe('CSVFileWatcher', () => {
@@ -207,6 +210,27 @@ describe('CSVFileWatcher', () => {
     );
   }
 
+  describe('parse', () => {
+    test('should parse users and groups in lowercase', async () => {
+      csvFileName = resolve(
+        __dirname,
+        '../../__fixtures__/data/valid-csv/uppercase-policy.csv',
+      );
+
+      const csvFileWatcher = createCSVFileWatcher(csvFileName);
+      const content = await csvFileWatcher.parse();
+      const expected = [
+        ['p', 'role:default/CATALOG-USER', 'catalog-entity', 'read', 'allow'],
+        ['p', 'role:default/known_role', 'test.resource.deny', 'use', 'allow'],
+        ['g', 'user:default/known_user', 'role:default/known_role'],
+        ['g', 'user:default/tom', 'role:default/CATALOG-USER'],
+        ['g', 'group:default/reader-group', 'role:default/CATALOG-USER'],
+        ['g', 'group:default/reader-group', 'role:default/known_role'],
+      ];
+      expect(content).toStrictEqual(expected);
+    });
+  });
+
   describe('initialize', () => {
     it('should be able to add permission policies during initialization', async () => {
       const csvFileWatcher = createCSVFileWatcher(csvFileName);
@@ -238,6 +262,7 @@ describe('CSVFileWatcher', () => {
           'allow',
         ],
         ['role:default/catalog-deleter', 'catalog-entity', 'delete', 'deny'],
+        ['role:default/CATALOG-USER', 'catalog-entity', 'read', 'allow'],
         ['role:default/known_role', 'test.resource.deny', 'use', 'allow'],
       ];
 
@@ -279,6 +304,8 @@ describe('CSVFileWatcher', () => {
         ['user:default/guest', 'role:default/catalog-reader'],
         ['user:default/guest', 'role:default/catalog-deleter'],
         ['user:default/known_user', 'role:default/known_role'],
+        ['user:default/tom', 'role:default/CATALOG-USER'],
+        ['group:default/reader-group', 'role:default/CATALOG-USER'],
       ];
 
       await enforcerDelegate.addGroupingPolicy(legacyRole, legacyRoleMetadata);
@@ -327,6 +354,10 @@ describe('CSVFileWatcher', () => {
         'user:default/guest',
         'role:default/catalog-deleter',
       ];
+      const duplicateGroupRole = [
+        'group:default/reader-group', // changed to lowercase
+        'role:default/CATALOG-USER',
+      ];
 
       const duplicatePolicyWithDifferentEffect = [
         'role:default/duplication-effect',
@@ -360,6 +391,10 @@ describe('CSVFileWatcher', () => {
         6,
         `Duplicate role: ${duplicateRole} found in the file ${csvFileName}`,
       );
+      expect(mockLoggerService.warn).toHaveBeenNthCalledWith(
+        7,
+        `Duplicate role: ${duplicateGroupRole} found in the file ${csvFileName}`,
+      );
     });
 
     it('should fail to add policies with errors', async () => {

workspaces/rbac/plugins/rbac-backend/src/file-permissions/csv-file-watcher.ts

@@ -16,7 +16,7 @@
 import type { LoggerService } from '@backstage/backend-plugin-api';
 
 import type { AuditLogger } from '@janus-idp/backstage-plugin-audit-log-node';
-import { Enforcer, FileAdapter, newEnforcer, newModelFromString } from 'casbin';
+import { Enforcer, newEnforcer, newModelFromString } from 'casbin';
 import { parse } from 'csv-parse/sync';
 import { difference } from 'lodash';
 
@@ -37,6 +37,7 @@ import {
   metadataStringToPolicy,
   policyToString,
   transformArrayToPolicy,
+  transformPolicyGroupToLowercase,
 } from '../helper';
 import { EnforcerDelegate } from '../service/enforcer-delegate';
 import { MODEL } from '../service/permission-model';
@@ -48,6 +49,7 @@ import {
   validateSource,
 } from '../validation/policies-validation';
 import { AbstractFileWatcher } from './file-watcher';
+import { LowercaseFileAdapter } from './lowercase-file-adapter';
 
 export const CSV_PERMISSION_POLICY_FILE_AUTHOR = 'csv permission policy file';
 
@@ -86,13 +88,17 @@ export class CSVFileWatcher extends AbstractFileWatcher<string[][]> {
    */
   parse(): string[][] {
     const content = this.getCurrentContents();
-    const parser = parse(content, {
+    const data = parse(content, {
       skip_empty_lines: true,
       relax_column_count: true,
       trim: true,
     });
 
-    return parser;
+    for (const policy of data) {
+      transformPolicyGroupToLowercase(policy);
+    }
+
+    return data;
   }
 
   /**
@@ -114,7 +120,7 @@ export class CSVFileWatcher extends AbstractFileWatcher<string[][]> {
 
     const tempEnforcer = await newEnforcer(
       newModelFromString(MODEL),
-      new FileAdapter(this.filePath),
+      new LowercaseFileAdapter(this.filePath),
     );
 
     // Check for any old policies that will need to be removed
@@ -197,7 +203,7 @@ export class CSVFileWatcher extends AbstractFileWatcher<string[][]> {
 
     const tempEnforcer = await newEnforcer(
       newModelFromString(MODEL),
-      new FileAdapter(this.filePath!),
+      new LowercaseFileAdapter(this.filePath!),
     );
 
     const currentFlatContent = this.currentContent.flatMap(data => {

workspaces/rbac/plugins/rbac-backend/src/file-permissions/lowercase-file-adapter.ts

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { FileAdapter, Helper, Model, mustGetDefaultFileSystem } from 'casbin';
+
+export class LowercaseFileAdapter extends FileAdapter {
+  public async loadPolicy(model: Model): Promise<void> {
+    if (!this.filePath) {
+      return;
+    }
+    await this.loadLowercasePolicyFile(model, Helper.loadPolicyLine);
+  }
+
+  private transformLineToLowercaseGroupsUsers(line: string): string {
+    if (line.trim().startsWith('g')) {
+      const policyArray = line.split(',');
+      if (policyArray.length >= 1 && policyArray[0].trim().startsWith('g')) {
+        policyArray[1] = policyArray[1].toLocaleLowerCase('en-US');
+      }
+      return policyArray.join(',');
+    }
+    return line;
+  }
+
+  private async loadLowercasePolicyFile(
+    model: Model,
+    handler: (line: string, model: Model) => void,
+  ): Promise<void> {
+    // Reference: https://github.com/casbin/node-casbin/blob/master/src/persist/fileAdapter.ts#L34-#L43
+    const bodyBuf = await (
+      this.fs ? this.fs : mustGetDefaultFileSystem()
+    ).readFileSync(this.filePath);
+    const lines = bodyBuf.toString().split('\n');
+
+    lines.forEach((line: string) => {
+      if (!line) {
+        return;
+      }
+      const lowercasedLine = this.transformLineToLowercaseGroupsUsers(line);
+      handler(lowercasedLine, model);
+    });
+  }
+}

workspaces/rbac/plugins/rbac-backend/src/helper.test.ts

@@ -24,6 +24,8 @@ import {
   policyToString,
   removeTheDifference,
   transformArrayToPolicy,
+  transformPolicyGroupToLowercase,
+  transformRolesGroupToLowercase,
   typedPoliciesToString,
   typedPolicyToString,
 } from './helper';
@@ -123,6 +125,63 @@ describe('helper.ts', () => {
     });
   });
 
+  describe('transformPolicyGroupToLowercase', () => {
+    it.each([
+      [
+        ['g', 'user:default/TOM', 'role:default/CATALOG-USER'],
+        ['g', 'user:default/tom', 'role:default/CATALOG-USER'],
+      ],
+      [
+        ['g', 'group:default/Developers', 'role:default/CATALOG-USER'],
+        ['g', 'group:default/developers', 'role:default/CATALOG-USER'],
+      ],
+    ])('should convert group in %s to lowercase', (input, expected) => {
+      transformPolicyGroupToLowercase(input);
+      expect(input).toEqual(expected);
+    });
+
+    it('should not transform policy to lowercase', () => {
+      const policyArray = [
+        'p',
+        'role:default/CATALOG-USER',
+        'catalog-entity',
+        'read',
+        'allow',
+      ];
+      const expected = [...policyArray];
+      transformPolicyGroupToLowercase(policyArray);
+      expect(policyArray).toEqual(expected);
+    });
+
+    it('should handle invalid input', () => {
+      const policyArray = ['g'];
+      transformPolicyGroupToLowercase(policyArray);
+      expect(policyArray).toEqual(['g']);
+    });
+  });
+
+  describe('transformRolesGroupToLowercase', () => {
+    it('should convert users and groups in roles to lowercase', () => {
+      const roles = [
+        ['user:default/test', 'role:default/test-provider'],
+        ['group:default/Developers', 'role:default/Reader'],
+      ];
+      const expectedRoles = [
+        ['user:default/test', 'role:default/test-provider'],
+        ['group:default/developers', 'role:default/Reader'],
+      ];
+      expect(transformRolesGroupToLowercase(roles)).toEqual(expectedRoles);
+    });
+
+    it.each([[[['user:default/test']]], [[[]]]])(
+      'should handle invalid input %d',
+      input => {
+        const result = transformRolesGroupToLowercase(input);
+        expect(result).toEqual(input);
+      },
+    );
+  });
+
   describe('removeTheDifference', () => {
     const mockEnforcerDelegate: Partial<EnforcerDelegate> = {
       removeGroupingPolicies: jest.fn().mockImplementation(),

workspaces/rbac/plugins/rbac-backend/src/helper.ts

@@ -135,6 +135,24 @@ export function transformArrayToPolicy(policyArray: string[]): RoleBasedPolicy {
   return { entityReference, permission, policy, effect };
 }
 
+export function transformPolicyGroupToLowercase(policyArray: string[]) {
+  if (
+    policyArray.length > 1 &&
+    policyArray[0].startsWith('g') &&
+    (policyArray[1].startsWith('user') || policyArray[1].startsWith('group'))
+  ) {
+    policyArray[1] = policyArray[1].toLocaleLowerCase('en-US');
+  }
+}
+
+export function transformRolesGroupToLowercase(roles: string[][]) {
+  return roles.map(role =>
+    role.length >= 1
+      ? [role[0].toLocaleLowerCase('en-US'), ...role.slice(1)]
+      : role,
+  );
+}
+
 export function deepSortedEqual(
   obj1: Record<string, any>,
   obj2: Record<string, any>,