fix(rbac): reduce the number of permissions returned, add isResourced flag

[AndrienkoAleksandr]

What does this pull request do:

reduce the number of permissions returned, add isResourced flag.

Notice: this pr reduces permission set in combination with current UI implementation.

Screenshots

Before:

After:

Referenced issues:

https://issues.redhat.com/browse/RHIDP-1926
https://issues.redhat.com/browse/RHIDP-1467

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.8% Duplication on New Code

See analysis details on SonarCloud

[divyanshiGupta]

@AndrienkoAleksandr this issue also reduces the number of permissions returned or it just adds a isResourced param so that the permissions can be identified as resourced or named?

[AndrienkoAleksandr]

@AndrienkoAleksandr this issue also reduces the number of permissions returned or it just adds a isResourced param so that the permissions can be identified as resourced or named?

@divyanshiGupta this pr reduces permission set in combination with current UI implementation. I attached screenshots. The idea of this pr - reduce permission list without UI modification and big backend changes. But we can consider another approach if you can see disadvantages.

[divyanshiGupta]

@AndrienkoAleksandr this issue also reduces the number of permissions returned or it just adds a isResourced param so that the permissions can be identified as resourced or named?

@divyanshiGupta this pr reduces permission set in combination with current UI implementation. I attached screenshots. The idea of this pr - reduce permission list without UI modification and big backend changes. But we can consider another approach if you can see disadvantages.

@AndrienkoAleksandr the changes looks fine to me. I asked this because @ShiranHi 's original ask was, if we can only have resource type permissions for example if we could have only catalog-entity and catalog-location but for now the changes in this PR looks good to me as it reduces the permissions to some extent. Thanks for the PR!

[divyanshiGupta]

@AndrienkoAleksandr can we get this PR merged?

[ShiranHi]

@AndrienkoAleksandr the changes looks fine to me. I asked this because @ShiranHi 's original ask was, if we can only have resource type permissions for example if we could have only catalog-entity and catalog-location but for now the changes in this PR looks good to me as it reduces the permissions to some extent. Thanks for the PR!

It's great to see fewer results in the dropdown. Two questions:

1. Why we're showing "catalog.entity.create" instead of just "catalog.entity"?
2. Why "catalog.entity.refresh" is no longer present and why there isn't a "Refresh" checkbox under the action options?

I'm not totally clear on the reasoning behind these changes, I'd love to hear the reasoning behind it. We need to make sure we cover all scenarios with this change.

[AndrienkoAleksandr]

Hello, @ShiranHi, thanks for your questions.

"catalog-entity" item in the dropdown should have only three actions: "read", "update", "delete". So this "catalog-enitity" item joins three permissions with names: 'catalog.entity.read', 'catalog.entity.delete', 'catalog.entity.refresh'

Let's take a look each of them:

export const RESOURCE_TYPE_CATALOG_ENTITY = 'catalog-entity';

export const catalogEntityReadPermission = createPermission({
  name: 'catalog.entity.read',
  attributes: {
    action: 'read',
  },
  resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
});


export const catalogEntityDeletePermission = createPermission({
  name: 'catalog.entity.delete',
  attributes: {
    action: 'delete',
  },
  resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
});

export const catalogEntityRefreshPermission = createPermission({
  name: 'catalog.entity.refresh',
  attributes: {
    action: 'update',
  },
  resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
});

See more: https://github.com/backstage/backstage/blob/99305a0/plugins/catalog-common/src/permissions.ts

Answer 1.

@ShiranHi: Why we're showing "catalog.entity.create" instead of just "catalog.entity"?

catalog.entity.create is permission with "basic" type, not "resource", so it doesn't have resourceType "catalog-entity". So it should not be joined to the 'catalog-entity' item:

export const catalogEntityCreatePermission = createPermission({
  name: 'catalog.entity.create',
  attributes: {
    action: 'create',
  },
});

At the end of the doc https://backstage.io/docs/permissions/writing-a-policy/ :

Note: Some catalog permissions do not have the 'catalog-entity' resource type, such as catalogEntityCreatePermission. In those cases, a definitive decision is required because conditions can't be applied to an entity that does not exist yet.

Answer 2.

@ShiranHi: Why "catalog.entity.refresh" is no longer present ...?

catalog.entity.refresh - this is permission with "resource" type and "update" action. So we join this permission to "catalog-entity" item in the dropdown. I will provide definition of this permission one more time:

export const catalogEntityRefreshPermission = createPermission({
  name: 'catalog.entity.refresh',
  attributes: {
    action: 'update',
  },
  resourceType: RESOURCE_TYPE_CATALOG_ENTITY,
});

Answer 3

@ShiranHi: Why there isn't a "Refresh" checkbox under the action options?

Permission framework has only four actions, so there is no "refresh action".

export type PermissionAttributes = {
  action?: 'create' | 'read' | 'update' | 'delete';
};

From code: https://github.com/backstage/backstage/blob/99305a0/plugins/permission-common/src/types/permission.ts#L24-L26

[ShiranHi]

@AndrienkoAleksandr thank you for the clarification. I didn't understand there are follow up tickets to this. I'll continue following

[PatAKnight]

/lgtm
/approve

[divyanshiGupta]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divyanshiGupta, PatAKnight

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [divyanshiGupta]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment