improve: self-service UI adding viewers in addition to editors

[adrianAzoitei]

Description

Proposal on handling both "viewers" (binding for kubeflow-view) and "editors" (binding for kubeflow-edit) as "contributors" in the Kubeflow Central Dashboard. See cluster roles in the community distribution.

Demo setup

During testing the implementation, the following setup was used:

• kind cluster
• oauth2-proxy + dex + keycloak for auth, where keycloak is configured with two users
  • user@example.com - cluster admin
  • user1@example.com
• 2 Profiles
  • namespace-1 and namespace-2, both owned by user@example.com

Adding contributors

user@example.com adding:

1. user1@example.com as a "edit" contributor to namespace-1 --> RoleBinding to kubeflow-edit
2. user1@example.com as a "view" contributor to namespace-1 --> RoleBinding to kubeflow-view

The role selection dropdown is placed on the side of the input field, but disabled as long as input is empty.

When input is no longer empty, the role dropdown is enabled.

Labels indicating the level of access of a contributor are applied on each contributor chip.

API endpoints split

add-contributor and add-viewer are now two separate endpoints in the UI backend.
LE: same for remove-contributor and remove-viewer for the same reason as below.

This allows cluster admins to block users from adding edit contributors on clusters where security guardrails need to be narrower by e.g. using an Istio AuthorizationPolicy such as:

apiVersion: security.istio.io/v1
kind: AuthorizationPolicy
metadata:
  name: deny-add-contributor
  namespace: kubeflow
spec:
  selector:
    matchLabels:
      app: centraldashboard
  action: DENY
  rules:
  - to:
    - operation:
        methods: ["POST"]
        paths: ["/api/workgroup/add-contributor/*"]

Adding a viewer still works, while adding a contributor gets a 403 from Istio and shows a clear error message:

Open questions

Q: user1@example.com can be added twice, since a rejection only happens when an error bubbles all the way up from kfam because a RoleBinding name with the same name exists (e.g. rolebindings.rbac.authorization.k8s.io "user-user1-example-com-clusterrole-edit" already exists.). user-user1-example-com-clusterrole-edit and user-user1-example-com-clusterrole-view can co-exist just fine as far as the kubelet is concerned, so no error bubbles up to the dashboard. Should we
1) accept this,
2) have kfam reject the action when user-user1-example-com-clusterrole-* exists?
3) have edit override view (perhaps kfam could do the cleanup), or
4) have the latest action take precedence (e.g. adding someone with the view role takes their edit role away and vice-versa)?
A: Went with option 4).

AI Policy Disclosure

See https://www.kubeflow.org/docs/about/ai_policy/.
claude-sonnet-4.6 was used as a coding assistant, mostly on the frontend - centraldashboard - component. The model did NOT run in agentic mode, every generated snippet of code was reviewed and / or modified by the submitter.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign thesuperzapper for approval. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• components/centraldashboard/OWNERS

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

[adrianAzoitei]

Will fix the failing frontend unit tests.

[juliusvonkohout]

/ok-to-test

[Copilot]

Pull request overview

Adds configurable contributor access levels to the Central Dashboard.

Changes:

• Adds Edit/View role selection and role labels.
• Splits contributor and viewer API endpoints.
• Adds role replacement/removal handling and tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
manage-users-view-contributor.pug	Adds role selection and labels.
manage-users-view-contributor.js	Selects endpoints and handles role-aware data.
manage-users-view-contributor.css	Styles the role selector layout.
manage-users-view-contributor_test.js	Updates frontend tests.
logout-button.js	Applies formatting cleanup.
api_workgroup.ts	Adds viewer APIs and role management.
api_workgroup_test.ts	Tests contributor/viewer operations.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

components/centraldashboard/app/api_workgroup.ts:242

• find makes role replacement depend on binding order when this user already has both roles. If the requested-role binding is returned first, oldBinding remains null, createBinding conflicts, and the opposite binding is never removed, so the stated “latest action takes precedence” behavior cannot repair existing duplicates (including duplicates left by a failed cleanup). Inspect all bindings for the user and remove every opposite-role binding while treating an existing requested-role binding as already created.

                const match = existing.find((b) => b.user === contributor);
                if (match && match.role !== role) {

components/centraldashboard/app/api_workgroup.ts:268

• If this delete fails during a contributor-to-viewer downgrade, the user retains the old edit binding as well as the new view binding. Returning an error does not restore the requested lower privilege, and a retry may hit the existing-view conflict before cleaning up edit access. Roll back the newly created binding when cleanup fails, or use an atomic role-replacement operation; rollback failure must also be surfaced explicitly.

                    await profilesService.deleteBinding(oldBinding, {headers});
                } catch (cleanupErr) {
                    return surfaceProfileControllerErrors({
                        res,
                        msg: `Role updated but failed to remove existing assignment` +
                            ` for ${contributor} in ${namespace}.`,
                        err: cleanupErr,

[juliusvonkohout]

Yes "have the latest action take precedence (e.g. adding someone with the view role takes their edit role away and vice-versa)?" sounds good to me. @thesuperzapper @christian-heusel what do you think?

I am in favor of merging this PR in general and could approve. @Raakshass may you do a technical review ?

[Raakshass]

verified locally. 72 of 72 server tests (184 expects), 75 of 75 client tests pass.

reviewed the api changes in api_workgroup.ts. the existing roleMap already handles viewer to view and contributor to edit mapping, so the new role parameter flows through mapSimpleBindingToWorkgroupBinding without needing changes there.

the upsert ordering (create new binding first, then delete old) is the right call — on cleanup failure the user temporarily has both bindings rather than losing access, and the error message at line 266 surfaces this explicitly.

one observation: centraldashboard-angular has its own api_workgroup.ts that still uses the old handleContributor signature without the role parameter. not blocking since they are separate deployments, but worth a follow-up for parity.
@juliusvonkohout
/lgtm

[adrianAzoitei]

Hi! Is this ok to merge then?

[google-oss-prow]

New changes are detected. LGTM label has been removed.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.