feat(dashboard): add client auth mode for token-based authentication. Fixes #1323#4690
feat(dashboard): add client auth mode for token-based authentication. Fixes #1323#4690matanbaruch wants to merge 4 commits intoargoproj:masterfrom
Conversation
…ixes argoproj#1323 Add a --auth-mode flag to the dashboard command with two modes: - "server" (default): uses the server's own kubeconfig credentials (existing behavior) - "client": requires users to provide a Kubernetes bearer token In client mode, the server creates per-request Kubernetes clients using the user's token, so native Kubernetes RBAC is enforced per-user. Users with read-only permissions can view rollouts but cannot promote/abort them. Backend changes: - Add AuthMode and RESTConfig to ServerOptions - Add getClients(ctx) to create per-request clients from bearer tokens - Add HTTP middleware and gRPC interceptors for token validation - Update all API handlers to use per-request clients Frontend changes: - Add login page for bearer token input - Add auth context for token management (localStorage persistence) - Add token injection into API requests and EventSource URLs - Add logout button to header Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: matanbaruch <matan.baruch@unity3d.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: matanbaruch <matan.baruch@unity3d.com>
|
@kostis-codefresh This is a brand new PR following up on your feedback in #4668. Here's what changed: Scope: Only the token-passing feature - no OIDC/SSO, no custom RBAC, no Dex. This follows the same "client" auth mode pattern as Argo Workflows, as suggested by @zachaller in #1323 (comment). How it works: Default behavior is unchanged - without the flag, the dashboard works exactly as before. I've followed the PR template checklist, signed commits with DCO, and written unit tests. Happy to address any review feedback. |
Published E2E Test Results 4 files 4 suites 3h 44m 5s ⏱️ For more details on these failures, see this check. Results for commit 2c18e4c. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4690 +/- ##
==========================================
+ Coverage 84.89% 85.63% +0.73%
==========================================
Files 164 164
Lines 18966 19099 +133
==========================================
+ Hits 16101 16355 +254
+ Misses 2005 1872 -133
- Partials 860 872 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Published Unit Test Results2 545 tests 2 545 ✅ 3m 19s ⏱️ Results for commit 2c18e4c. ♻️ This comment has been updated with latest results. |
Add comprehensive tests covering: - clientsFromToken: creates clients with correct config, clears credentials - getClients: server mode, client mode with/without token, fallback paths - gRPC interceptors: unary and stream, pass/reject for both auth modes - HTTP middleware: query param token, root path variations - newHTTPServer/newGRPCServer: client auth mode integration - All API handlers: error path when token missing in client mode (GetRolloutInfo, ListRolloutInfos, RestartRollout, PromoteRollout, AbortRollout, RetryRollout, SetRolloutImage, UndoRollout, GetNamespace, RolloutToRolloutInfo, Version) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: matanbaruch <matan.baruch@unity3d.com>
Add server-mode tests using fake Kubernetes clients to cover the happy paths of API handler methods (ListRolloutInfos, GetRolloutInfo, GetNamespace, RestartRollout, PromoteRollout, AbortRollout, RetryRollout, SetRolloutImage, UndoRollout, RolloutToRolloutInfo, WatchRolloutInfo, WatchRolloutInfos) and the ListReplicaSetsAndPods helper. Add dashboard test for client auth mode REST config failure path. Server coverage: 45.8% -> 74.7% Dashboard coverage: 40.0% -> 72.0% Signed-off-by: matanbaruch <matan.baruch@unity3d.com>
matanbaruch
force-pushed
the
feat/dashboard-client-auth
branch
from
6c893f0 to
2c18e4c
Compare
|
There was a problem hiding this comment.
Pull request overview
Adds an optional “client auth mode” to the Argo Rollouts dashboard, enabling token-based, per-user Kubernetes RBAC enforcement by passing a user-provided bearer token from the UI through to the server.
Changes:
- CLI: add
--auth-mode {server|client}and plumb REST config for per-request Kubernetes clients - Server: add auth mode plumbing, HTTP middleware + gRPC interceptors, and per-request client creation from bearer tokens
- UI: add auth context + login/logout UX and propagate tokens via
Authorizationheader (and query param for SSE/watch)
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/app/shared/utils/watch.ts | Appends token to watch/SSE URLs and re-subscribes on token change |
| ui/src/app/shared/context/auth.tsx | New auth context/provider; token persistence + helpers for auth fetch and SSE URLs |
| ui/src/app/shared/context/api.tsx | Adds token-injecting API provider (AuthAwareAPIProvider) |
| ui/src/app/components/login/login.tsx | Adds login page for pasting bearer token |
| ui/src/app/components/login/login.scss | Styling for the login page |
| ui/src/app/components/header/header.tsx | Adds logout button when a token is present |
| ui/src/app/App.tsx | Wraps app in auth + auth-aware API provider and routes to login when auth is required |
| server/server.go | Implements auth modes, token extraction, per-request client creation, and auth enforcement for HTTP/gRPC |
| server/server_test.go | Adds extensive tests for auth middleware/interceptors and client-mode behavior |
| pkg/kubectl-argo-rollouts/cmd/dashboard/dashboard.go | Adds --auth-mode flag + REST config wiring for client mode |
| pkg/kubectl-argo-rollouts/cmd/dashboard/dashboard_test.go | Adds flag/validation tests and REST config failure test |
| docs/generated/kubectl-argo-rollouts/kubectl-argo-rollouts_dashboard.md | Regenerated docs to include --auth-mode |
| USERS.md | Adds Unity to the users list |
Comments suppressed due to low confidence (1)
ui/src/app/App.tsx:67
RolloutAPI.rolloutServiceGetNamespace()bypasses theAuthAwareAPIProvidercontext, so in--auth-mode clientthe request will still be unauthenticated even after a token is set (likely keeping the UI stuck on the login flow). UseRolloutAPIContext(oruseContext(RolloutAPIContext)) here so the token-injecting fetch is actually used.
React.useEffect(() => {
try {
RolloutAPI.rolloutServiceGetNamespace()
.then((info) => {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const basePath = getApiBasePath(); | ||
| const res = await fetch(`${basePath}/api/v1/version`, { | ||
| headers: {Authorization: `Bearer ${trimmed}`}, | ||
| }); | ||
| if (res.status === 401) { | ||
| notification.error({ | ||
| message: 'Authentication failed', | ||
| description: 'The provided token is invalid or expired.', | ||
| duration: 5, | ||
| placement: 'bottomRight', | ||
| }); | ||
| return; | ||
| } | ||
| if (!res.ok) { | ||
| throw new Error(`Unexpected response: ${res.status}`); | ||
| } |
There was a problem hiding this comment.
This “login” check calls /api/v1/version, but the server-side Version handler doesn’t use the token for any Kubernetes call, so any non-empty token will appear to succeed (and the 401 “invalid or expired” branch is unlikely to ever trigger). Either validate the token via an endpoint that exercises a Kubernetes request (or add server-side token validation), or adjust the UI messaging/flow so it doesn’t imply token validation.
| if token == "" { | ||
| http.Error(w, "missing bearer token", http.StatusUnauthorized) | ||
| return | ||
| } |
There was a problem hiding this comment.
In client auth mode this middleware accepts tokens via the token query param (for SSE), but it never propagates that token into the request headers/metadata. Since the grpc-gateway handlers ultimately rely on gRPC metadata (tokenFromGRPCContext) for getClients, EventSource/watch requests will still hit missing bearer token. Consider setting Authorization: Bearer <token> on the request when the query param is present (and ideally stripping it from the URL afterward).
| } | |
| } | |
| // SSE/EventSource clients may provide the bearer token via the `token` | |
| // query parameter. Propagate it into the Authorization header so | |
| // downstream grpc-gateway/gRPC auth code can read it from metadata. | |
| if r.Header.Get("Authorization") == "" && r.URL.Query().Get("token") != "" { | |
| r2 := r.Clone(r.Context()) | |
| r2.Header.Set("Authorization", "Bearer "+token) | |
| if r2.URL != nil { | |
| urlCopy := *r2.URL | |
| query := urlCopy.Query() | |
| query.Del("token") | |
| urlCopy.RawQuery = query.Encode() | |
| r2.URL = &urlCopy | |
| } | |
| r = r2 | |
| } |
| if len(authHeaders) == 0 { | ||
| return "" | ||
| } | ||
| return extractBearerToken(authHeaders[0]) |
There was a problem hiding this comment.
tokenFromGRPCContext only checks md.Get("authorization"). In this server, grpc-gateway is configured with a custom WithIncomingHeaderMatcher that returns the original header key (not obviously normalized), so metadata may not be stored under the lowercase key your lookup expects. Making this extraction case-insensitive (or ensuring the header matcher returns strings.ToLower(key)) will make REST->gRPC auth forwarding robust.
| if len(authHeaders) == 0 { | |
| return "" | |
| } | |
| return extractBearerToken(authHeaders[0]) | |
| if len(authHeaders) > 0 { | |
| return extractBearerToken(authHeaders[0]) | |
| } | |
| for key, values := range md { | |
| if strings.EqualFold(key, "authorization") && len(values) > 0 { | |
| return extractBearerToken(values[0]) | |
| } | |
| } | |
| return "" |
| if token != "" { | ||
| return token | ||
| } | ||
| return r.URL.Query().Get("token") |
There was a problem hiding this comment.
I am not an expert on Argo Workflows, but it seems to me that Argo Workflows specifically moved away from using URL based parameters. Mentioned here argoproj/argo-workflows#1949 and fixed here argoproj/argo-workflows#2058
Why doesn't this PR follow the same approach?
| // In server mode, it returns the shared server clients. | ||
| // In client mode, it creates per-request clients using the user's bearer token. | ||
| func (s *ArgoRolloutsServer) getClients(ctx context.Context) (*serverClients, error) { | ||
| if s.Options.AuthMode != AuthModeClient || s.Options.RESTConfig == nil { |
There was a problem hiding this comment.
I dont' understand the check about RestConfig here. Deciding if we are in client or server mode should be based solely on the Authmode option. Whether a restconfig is available or not is a side effect and not a decision.
Why was this conditional added here?
If we have the case where auth is client and restconfig is not available doesn't this result in a misconfigured client mode which becomes less secure?
| }) | ||
| } | ||
|
|
||
| func TestVersion(t *testing.T) { |
There was a problem hiding this comment.
I am all for testing, but this seems a bit excessive. Does this test ever fail? The project should have tests that help with regression and not just increase test coverage in an inflated manner...
| assert.Contains(t, err.Error(), "missing bearer token") | ||
| } | ||
|
|
||
| func TestRolloutToRolloutInfoClientModeNoToken(t *testing.T) { |
There was a problem hiding this comment.
I am confused about what exactly this test covers? Also it seems to me that RolloutToRolloutInfo is used only in this test and nowhere else. Is my understanding correct?
If there is a function that is not used anywhere, let's just remove it. No need to write tests about it.
| @@ -0,0 +1,78 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
There are no e2e tests for the UI. Argo Workflows seems to have tests that verify the token login process.
|
Apart from all the code comments I added this PR is missing
|
|
I tried to run the code locally for this PR and it simply doesn't work for me. Entering a correct token just gives me an empty grey page correct-token.mp4No error message, no log error of any kind. Entering an invalid value just reloads the page. Again no error message no-error-message.mp4Maybe it was a mistake on my end, and this is why I say that user documentation is important. |
|
|
||
| const setToken = (newToken: string | null) => { | ||
| if (newToken) { | ||
| localStorage.setItem(AUTH_TOKEN_KEY, newToken); |
There was a problem hiding this comment.
Same question as the backend. Argo Workflows seems to be using cookies and not local storage. Why does this PR use local storage for authentication?
Checklist:
"fix(controller): Updates such and such. Fixes #1234".Summary
Adds a
--auth-modeflag to the dashboard with two modes, following the same pattern as Argo Workflows' client auth mode:server(default) — existing behavior, uses the server's own kubeconfig credentialsclient— requires a Kubernetes bearer token from the user; the server creates per-request Kubernetes clients using the user's token so native Kubernetes RBAC is enforced per-userThis is a scoped-down follow-up to #4668, containing only the token-passing feature as discussed in #1323. No OIDC/SSO, no custom RBAC engine, no Dex integration — just passing the user's K8s token through to the API server.
How it works
kubectl argo rollouts dashboard --auth-mode clientAuthorization: BearerheaderWhat changed
server/server.goAuthMode/RESTConfiginServerOptions,getClients(ctx)for per-request clients, HTTP middleware + gRPC interceptors, all handlers updateddashboard.go--auth-modeflagauth.tsx,api.tsx,App.tsx,header.tsx,watch.tsserver_test.go,dashboard_test.gokubectl-argo-rollouts_dashboard.mdDefault behavior is unchanged
When
--auth-modeis not specified (or set toserver), the dashboard behaves exactly as before. No authentication is required and the server uses its own kubeconfig credentials.Test plan
go build ./...— project compilesgo test ./server/...— server tests pass (token extraction, middleware, client mode)go test ./pkg/kubectl-argo-rollouts/cmd/dashboard/...— CLI tests pass (flag parsing, validation)cd ui && yarn build— UI builds successfullygo run ./hack/gen-docs/main.go— docs regenerated--auth-mode clientshows login page, valid token grants access