busola icon indicating copy to clipboard operation
busola copied to clipboard

Incorrect permission check for restricted users

Open Akatuoro opened this issue 1 year ago • 4 comments

Description

The permission check is currently case-sensitive, while set.resources entries are always lower case. https://github.com/kyma-project/busola/blob/3e43f204e766f87ce1686bd62d8712cf41524d10/src/state/navigation/filters/permissions.ts#L31 https://github.com/kyma-project/busola/blob/3e43f204e766f87ce1686bd62d8712cf41524d10/src/state/navigation/filters/permissions.ts#L39

This leads to an incorrect check when the resource kind is given in PascalCase, e.g. here: https://github.com/kyma-project/busola/blob/3e43f204e766f87ce1686bd62d8712cf41524d10/src/state/navigation/extensionsAtom.ts#L92-L96

The resulting behavior is that the doesUserHavePermission check fails and the namespaced url is taken. https://github.com/kyma-project/busola/blob/3e43f204e766f87ce1686bd62d8712cf41524d10/src/state/navigation/extensionsAtom.ts#L98-L99

This only happens when the user is restricted. Usually, the user has access to all resources ('*') in the namespace, in which case doesUserHavePermission is successful. https://github.com/kyma-project/busola/blob/main/src/state/navigation/filters/permissions.ts#L40

This also means that if a user has access to all resources in a single namespace and no access to cluster resources, fetching extensions etc. fails because it is tried on cluster level.

Expected result

  1. If a restricted user (cluster configmaps + namespace configmaps + namespace pods) has access to specific cluster-wide config maps, they get fetched on cluster level.

  2. If a restricted user has no access on cluster level, but full access on namespace level ('*' for resources), then namespace-wide config maps get fetched.

Actual result

For 1., ConfigMaps get fetched on namespace level

For 2., ConfigMaps get fetched on cluster level, resulting in 403s

Steps to reproduce

For 1.:

  • Create a ServiceAccount
  • Create a role 'configmap-access' in namespace with access to resource configmaps
  • Bind role 'configmap-access' to service account
  • Create a cluster role 'configmap-access' with access to resource configmaps
  • Bind cluster role 'configmap-access' to service account
  • Check network requests for configmaps?labelSelector (should all be namespace scoped)

For 2.:

  • Create a ServiceAccount
  • Create a role 'namespace-admin' in namespace with access to all resources (*)
  • Bind role 'namespace-admin' to service account
  • Check network requests for configmaps?labelSelector (should be cluster scoped and fail with 403)

Troubleshooting

The case-sensitivity bug can be fixed easily by adding toLowerCase():

const resourceKindPlural = pluralize(resourceKind).toLowerCase();

This at first makes the situation worse, because the permissionSetsSelector points to the currently active namespace while const hasAccessToClusterCMList = doesUserHavePermission(...) expects cluster permissions.

As solution, it would make sense to have 2 permission sets, one for the namespace level and one for the cluster level. Then each can be used accordingly.

Akatuoro avatar Aug 04 '23 16:08 Akatuoro