fix(rbac): lowercase user and group entity references

[dzemanov]

Hey, I just made a Pull Request!

Roles and permissions are not applied correctly when users or groups contain uppercase.
Auth can return lowercased sub and lowercased ownershipEntityRefs, which means we are not getting matches when comparing with casbin_rule v0 that has original references that could contain uppercase, or possibly in other places, like 1, 2
Entity references are case insensitive.
Entity references are saved in relations_table in lowercase.

This change:

• migrates users and groups to lowercase in casbin_rule v0
• lowercases users and groups in user inputs so we are always working with lowercase (can confuse users when we will refer to their inputs in lowercase)
• Still left to do: conditions

Other possible solutions:

• Modify casbin matcher to be case insensitive (performance?), but we would still need to handle cases when we manually compare like here

How to test

Login as a user with a username containing uppercase - in all example queries/data, please change user:default/User to your user.

1. Add to Catalog locations a file with these users/groups

apiVersion: backstage.io/v1alpha1
kind: Group
metadata:
 name: group-A
spec:
 type: team
 children: ["group-B"]
---
apiVersion: backstage.io/v1alpha1
kind: Group
metadata:
 name: group-B
spec:
 type: team
 children: []
---
apiVersion: backstage.io/v1alpha1
kind: Group
metadata:
 name: group-e
spec:
 type: team
 children: []
---
apiVersion: backstage.io/v1alpha1
kind: User
metadata:
 name: User
spec:
 memberOf:
   - group-B

    - type: file
      target: /path/to/entities.yaml
      rules:
        - allow: [User, Group]

2. Test Config

permission:
enabled: true
rbac:
 pluginsWithPermission:
   - catalog
   - permission
   - scaffolder
 admin:
   users:
     - name: user:default/User

You should be able to see RBAC page

3. Test API

Create role:

curl -X POST "http://localhost:7007/api/permission/roles" -d '{ "memberReferences":  ["group:default/group-B" ], "name": "role:default/Test" }' -H "Content-Type: application/json" -H "Authorization: Bearer $token"

Create permissions:

curl -X POST "http://localhost:7007/api/permission/policies" -d '[{"entityReference": "role:default/Test", "permission": "catalog-entity", "policy": "update", "effect":"allow"}]' -H "Content-Type: application/json" -H "Authorization: Bearer $token"

curl -X POST "http://localhost:7007/api/permission/policies" -d '[{"entityReference": "role:default/Test", "permission": "catalog-entity", "policy": "delete", "effect":"allow"}]' -H "Content-Type: application/json" -H "Authorization: Bearer $token"

curl -X POST "http://localhost:7007/api/permission/policies" -d '[{"entityReference": "role:default/Test", "permission": "catalog.entity.create", "policy": "create", "effect":"allow"}]' -H "Content-Type: application/json" -H "Authorization: Bearer $token"

You should be able to create and delete Catalog entities.

Update role:

curl -X PUT "http://localhost:7007/api/permission/roles/role/default/Test" -d '{ "oldRole": { "memberReferences":  [ "group:default/group-B" ], "name": "role:default/Test" }, "newRole": { "name": "role:default/Test", "memberReferences": [ "user:default/User", "group:default/group-e" ] } }' -H "Content-Type: application/json" -H "Authorization: Bearer $token"

DELETE your user from role:

curl -X DELETE "http://localhost:7007/api/permission/roles/role/default/Test?memberReferences=user:default/User" -H "Content-Type: application/json" -H "Authorization: Bearer $token"

4. Test csv

Create uppercase.csv:

p, role:default/test-Delete, catalog-entity, delete, allow
g, group:default/group-A, role:default/test-Delete
p, role:default/test-Create, catalog.entity.create, create, allow
p, role:default/test-Create, catalog.location.create, create, allow
g, group:default/group-e, role:default/test-Create
g, group:default/group-B, role:default/test-Create

Add to app-config.yaml:

permission:
 enabled: true
 rbac:
   policies-csv-file: path/to/uppercase.csv

Confirm your User can delete and create entities.
You can add csv reload:

permission:
 enabled: true
 rbac:
   policies-csv-file: path/to/uppercase.csv
   policyFileReload: true

And change uppercase.csv to for example:

p, role:default/test-Changed, catalog-entity, delete, allow
g, user:default/User, role:default/test-Changed
p, role:default/test-Create, catalog.entity.create, create, allow
g, group:default/group-e, role:default/test-Create

2. Test Migration

Run the old code to populate database with uppercase users/groups.
You can use csv file for this, for example:

p, role:default/test-Delete, catalog-entity, delete, allow
g, group:default/group-A, role:default/test-Delete
g, group:default/group-e, role:default/test-Delete
g, user:default/User, role:default/test-Delete
p, role:default/test-ok, catalog-entity.create, create, allow

Go back to this branch.
Change knexfile.js to your database, e.g:

module.exports = {
  client: pg,
  connection: {
    host: '127.0.0.1',
    port: 5432,
    user: USER,
    password: PASSWORD,
  },
  useNullAsDefault: true,
  migrations: {
    directory: './migrations',
  },
};

Apply migrations:

yarn knex migrate:latest --knexfile /path/to/rbac-backend/knexfile.js 

Verify casbin_rule v0 column has now lowercased users and groups, but roles stayed the same (e.g test-Delete)

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)
• All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage-community/plugin-rbac-backend	workspaces/rbac/plugins/rbac-backend	minor	v5.2.9

[dzemanov]

We will need to also lowercase users and groups in claims in conditional policies.

[dzemanov]

/cc @AndrienkoAleksandr @PatAKnight

[PatAKnight]

@dzemanov I am not certain that we need to lowercase users and groups for conditional policy claims. I was testing a variety of scenarios and it seems like whether I used my GitHub username as is or all lowercase, the conditional policy would still work for me. Could you share an example of where it didn't work for you?

[AndrienkoAleksandr]

Your current changes looks good to me, but I think you also need to handle upperCases for test cases:

• user provided user or group entity reference with help of REST API( via curl or postman...)
• user provider user entity reference with help of application config(permission/rbac/admin/users)

[dzemanov]

2024-12-02.12-25-33.mp4

@PatAKnight it would be amazing if we don't have to lowercase users and groups for conditional policy claims. Demo for using lowercase as opposed to uppercase:

[PatAKnight]

Yeah, let me try it again. I was using the GitHub auth provider with the usernameMatchingUserEntityName sign in resolver. Maybe the different auth providers also handle lowercase / uppercase differently?

[awanlin]

Hi @dzemanov, thanks for the contribution, you'll need to add a change set so that these changes get released, details on that are here: https://github.com/backstage/community-plugins/blob/main/CONTRIBUTING.md#creating-changesets

Also left one small optional comment 👍

[awanlin]

Hi @AndrienkoAleksand, @divyanshiGupta, @PatAKnight, as owners of this plugin can you please review this PR? Once one of you have then I'm fine with hitting merge if you can't 👍

[awanlin]

Waiting for the owners to review and approve 👍

[dzemanov]

Waiting on #2166 to be merged first.

[AndrienkoAleksandr]

Tested, works like expected. Gread job, @dzemanov!

[AndrienkoAleksandr]

@awanlin, ping. I think @dzemanov handled code review feedback.

[04kash]

All requested changes were made, plugin owners have approved

Dismissing review as changeset was added