Multi-User Authorization: Add support for K8s RBAC via SubjectAccessReview

[yanniszark]

As per the latest design doc: https://docs.google.com/document/d/1R9bj1uI0As6umCTZ2mv_6_tjgFshIKxkSt00QLYjNV4/edit?ts=5e4d8fbb#heading=h.5s8rbufek1ax

Add support for using K8s RBAC for authorization in Kubeflow Pipelines.
Backend endpoints will be mapped to K8s RBAC permissions.
Authorization checks will be done using SubjectAccessReview.

Related:
#1223
kubeflow/dashboard#43
kubeflow/kubeflow#4909 (comment)

/kind feature
/area backend

/cc @IronPan @Bobgy

[Bobgy]

Thanks for driving this!
also
/cc @chensun

[jbottum]

@Bobgy @yanniszark @chensun @jessiezcc @jlewi This feature is scheduled for delivery with Kubeflow 1.1. Several users asked for it in the latest Kubeflow user survey. Will this feature be delivered in 1.1? What is the next step? Should we set-up a review with users to make sure we are delivering what they expect? Could we provide a status in the next Community or Pipelines call ?

[jlewi]

@jbottum I think #3693 is tracking the release of multiuser KFP.
#1223 is the uber issue for multiuser

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[yanniszark]

/remove-lifecycle stale
/lifecycle frozen

[jlewi]

@yanniszark What is the remaining work to close this issue out?

[elikatsis]

Hi everyone,
I'm writing to provide an update on some decisions we've made as Arrikto with respect to performing authorization via SubjectAccessReview on KFP.

Since SubjectAccessReview works with Kubernetes RBAC we need a mapping from KFP resources and actions to Kubernetes resources, which belong to some apiVersion, and verbs.

So this is what we need to decide:

• apiVersion
• KFP-to-Kubernetes resource name mapping
• Verb-to-action mapping

Here are some decisions we've made along the way:

apiVersion

apiVersion = <apiGroup>/<version>

Group

API group usually is a URL pointing to an organization owning the resources. All Kubeflow projects are using kubeflow.org (as a base domain at least).
For example, KFServing uses the subdomain serving.kubeflow.org.

Along the KFP codebase we see that we the annotations we are setting use the subdomain pipelines.kubeflow.org. Moreover, we can't use kubeflow.org because we need the name experiment as a resource and there will be conflict with Katib's Experiment CR

Decision: pipelines.kubeflow.org

Version

This repo is on v1beta1 version. We can see this in the API where KFP is served (apis/v1beta1/...) and also in the CRs we have defined in this repo (Viewer, ScheduledWorkflow).

Decision: v1beta1

Resources - Verbs

Here is a list of resources and verbs we need RBAC for:

Resource	Verbs	Comments
experiments	archive, create, delete, get, list, unarchive
runs	archive, create, delete, get, list, retry, terminate, unarchive
jobs	create, delete, disable, enable, get, list	Jobs are recurring runs (this is how they are mentioned in the codebase). We are not using scheduledworkflows because it's an actual CR which is different than jobs
pipelines	create, delete, get, list	Pipelines are not namespaced yet #4022 (or #4197)
pipelines/versions	create, delete, get, list	versions is a subresource of pipelines
viewers	create, get, delete
visualizations	create

[Bobgy]

Thanks @elikatsis !
That sounds good mostly.

A few nitpickings:

• Do we need upload for pipelines and versions? It was only a technical details that we needed to build two endpoints. I think we can converge to just use create.
• visualizations, we plan to deprecate this: Deprecate python based visualization #4643
• Shall we also guard logs and artifacts endpoint by rbac?

[elikatsis]

Do we need upload for pipelines and versions? It was only a technical details that we needed to build two endpoints. I think we can converge to just use create.

That is correct! Uploading is just a way to create. Thanks! I'll update the table

visualizations, we plan to deprecate this: kubeflow/kubeflow#4643

I'd say that as long as they still exist and they have authorization checks (which they do both now), we should include them in the PR. Then, a distinct PR will purge them.

Shall we also guard logs and artifacts endpoint by rbac?

Of course! We should perform authorization checks for all endpoints. This includes the ones you mention, the persistence agent's logs, and any other endpoint that's not authorized.
However, similarly to the previous point, I think it should be part of a different PR since it's more of a new feature than part of the refactoring of the authorization mechanism.

WDYT?

[Bobgy]

All SGTM, thanks!

[Bobgy]

/reopen
Let's keep this tracking documentation

[k8s-ci-robot]

@Bobgy: Reopened this issue.

In response to this:

/reopen
Let's keep this tracking documentation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[Jeffwan]

@elikatsis verbs will be used to create SubjectAccessReview and they should be Kubernetes API verbs? Here, I see the table we have Kubeflow verbs. Do we need mapping between kubeflow actions and kubernetes RBAC verbs?

[elikatsis]

@Jeffwan
What do you mean by saying Kubernetes API verbs?

In general, if I'm not wrong, we can use any string we like as a verb. When SubjectAccessReview takes place, the creator populates the request with the desired verb. Then, K8s API server checks bindings and roles to find whether an entity has permission to perform a verb on a resource.

It doesn't matter if the rest of K8s knows what upload is (we don't have upload, I just wanted to make an example that stands out). As long as the one asking whether the permission (here the KFP API server) knows what the verb is for, then everything is ok.

Does this cover your question?

[Jeffwan]

@elikatsis I may misunderstand the authorization process. Correct me if I am wrong.

1. I thought verb has to be k8s API verbs like what we using in role bindings.
   from https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#resourceattributes-v1-authorization-k8s-io
   "Verb is a kubernetes resource API verb, like: get, list, watch, create, update, delete, proxy. "*" means all."

2. SubjectAccessReview request is sent to K8s APISever, and K8s APIServer check RBAC to determine whether or not to allow each API action. However, Pipeline/experiment/runs are not k8s concepts. so I assume k8s apiserver can not do this? It needs some authorizer? But I didn't see authorizer (like webhook) to do the subjectAccessReview? Am I missing something?

[elikatsis]

@Jeffwan
The K8s API server doesn't check whether a CRD exists (e.g., pipelines.kubeflow.org/runs) or what a verb is.
To accept/deny a SAR it only checks RBAC resources ([Cluster]Roles & [Cluster]RoleBindings) and what's declared in them. It's just string matching and doesn't require strings to make sense for the K8s API server.

Let's say we have a role with literally this exact rule

- apiGroup: my-group
  resources: ["my-resource"]
  verbs: ["my-verb"]

bound to the default service account.

The K8s API server doesn't have any reference to my-group or my-resource, nor it knows any action connected to my-verb.

If I ask the Kubernetes API server if the default service account has permission to my-verb a resource of type my-resource in the apiGroup my-group (via a SubjectAccessReview), it will return a positive reply.

[Jeffwan]

@elikatsis Got it. Thanks for the explanation. Then we need to create some roles for each user like below. I was confused if this doesn't exist, how could SAR get reviewed by API server.. It's clear now. Do we want to put this part into profile controller or just give user * to user's namespace unless user/admin wants to have some fine grain control

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
  namespace: userA
  name: pipeline-role
rules:
- apiGroups: ["pipelines.kubeflow.org"]
  resources: ["runs"]
  verbs: ["get", "unarchive", "upload"]
... 

[elikatsis]

For coarse-grained tuning, aggregating kubeflow-view and kubeflow-edit should be sufficient. See this kustomization.

Then, if, for example, kubeflow-edit is bound to some service account/user, they should have the corresponding permissions.

[Bobgy]

Exactly, and the widely used default-editor already has kubeflow-edit role.
Therefore, this change is effectively an internal refactoring as long as users do not create the new bindings by themselves

[Bobgy]

The remaining TODO is to integrate kubeflow/manifests with updated multi-user mode manifests.
It's probably time we move multi user manifests from kubeflow/manifests back to kubeflow/pipelines: #4816.

[chensun]

The remaining TODO is to integrate kubeflow/manifests with updated multi-user mode manifests.
It's probably time we move multi user manifests from kubeflow/manifests back to kubeflow/pipelines: kubeflow/kubeflow#4816.

@Bobgy, any update on the remaining works? Other than the code location of the manifests, do we need some documentation refresh?

Therefore, this change is effectively an internal refactoring as long as users do not create the new bindings by themselves

This probably needs some documentation. I think we asked users to create bindings by themselves in the earlier versions. What needs to be changed for the existing custom bindings?

[Bobgy]

Yes, I'll work on this targetting KF 1.3 release by the end of March 15