Kubernetes SubjectAccessReview authz doesn't support using user's groups from Kubernetes TokenReview authn

[dhirajsb]

Describe the bug
Kubernetes TokenReview Authn retrieves user group information. This group information is ignored when making Kubernetes SubjectAccessReview (SAR) call for authz. However, the API doc for SAR suggests that the groups passed in that call should be the list of user's groups. Otherwise it acts as an OR condition. I.e. access is authorized if the user has rolebindings that grant permissions, OR a static list of groups in AuthConfig has rolebindings, irrespective of whether the user is a member of the static list of groups or not.

Help us Reproduce it

Given the following in the namespace odh-model-registries:

1. Service modelregistry-sample
2. User Group modelregistry-sample-users
3. A K8s Role registry-user-modelregistry-sample with a GET permission for the service modelregistry-sample
4. And the following AuthConfig

spec:
  authentication:
    cluster-users:
      credentials:
        authorizationHeader: {}
      kubernetesTokenReview:
        audiences:
        - https://*********
  authorization:
    k8s-rbac:
      kubernetesSubjectAccessReview:
        groups:
        - modelregistry-sample-users
        resourceAttributes:
          group:
            value: ""
          name:
            value: modelregistry-sample
          namespace:
            value: odh-model-registries
          resource:
            value: services
          subresource:
            value: ""
          verb:
            value: get
        user:
          selector: auth.identity.username

The following behavior is observed:

1. Authorization against this service with a User not in the allowed Group modelregistry-sample-users is allowed
2. The call is incorrectly authorized as the Group itself is allowed
3. Remove the static group modelregistry-sample-users from AuthConfig above
4. Add User to Group to grant it GET permission
5. Authorization fails despite User having permission to call GET indirectly through modelregistry-sample-users Group

Expected behavior
AuthConfig should add support for user defined expressions to get group information from a prior AuthN step in the pipeline. Which, will allow end users to create AuthConfig like:

spec:
  authentication:
    cluster-users:
      credentials:
        authorizationHeader: {}
      kubernetesTokenReview:
        audiences:
        - https://**********
  authorization:
    k8s-rbac:
      kubernetesSubjectAccessReview:
        authorizationGroups:
         selector: auth.identity.groups
        resourceAttributes:
          group:
            value: ""
          name:
            value: modelregistry-sample
          namespace:
            value: odh-model-registries
          resource:
            value: services
          subresource:
            value: ""
          verb:
            value: get
        user:
          selector: auth.identity.username

Environment (please complete the following information):

• Cluster Version: OpenShift 4.15
• Authorino version: Red Hat - Authorino (Technical Preview) 1.0.2

Additional context

The test service and AuthConfig can be be replicated using the opendatahub modelregistry component at https://github.com/opendatahub-io/model-registry-operator. Or, any test service and group along with an AuthConfig similar to above can replicated this issue.

[guicassolato]

Kubernetes TokenReview Authn retrieves user group information. This group information is ignored when making Kubernetes SubjectAccessReview (SAR) call for authz.

I understand the request and in general I support this change. Just to clarify though, the groups returned in the TokenReview response not being automatically used by Authorino in the SubjectAccessReview is not a bug. It is by design.

Authorino's authentication and authorization methods are not tight to each other. One can cambine whatever authentication method desired (e.g. TokenReview, API key, JWT, OAuth2, etc) to whatever authorization method desired (SubjectAccessReview, pattern matching, OPA, etc), including no method at all. Therefore it is up to AuthConfig owner to write it in a way that reuses data from one phase into the other, if that's what's needed to implement the use case.

What's being asked here is to make the groups field of the SubjectAccessReview authorization config dynamic, i.e., like user and the resource attribute fields are. My thumbs-up to this proposal.