feat(dashboard): add authenticated server mode with login, RBAC, OIDC, and TLS. Fixes #4807#4808
Open
hdt12a1 wants to merge 67 commits into
Open
feat(dashboard): add authenticated server mode with login, RBAC, OIDC, and TLS. Fixes #4807#4808hdt12a1 wants to merge 67 commits into
hdt12a1 wants to merge 67 commits into
Conversation
Design for adding full-parity authentication/authorization (OIDC/SSO, local accounts, JWT sessions, Casbin RBAC, TLS) to the Argo Rollouts dashboard via a new shared in-cluster server mode. Local mode unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan for the dashboard RBAC package: resource/action constants, casbin model, built-in admin/readonly/operator policy, enforcer, user policy + default-role fallback. First of seven stacked plans for dashboard auth. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Fixed ModelConf to use standard casbin field name 'eft' (was 'eff') so casbin v2 built-in effect matching works correctly; adds Casbin v2 dep. Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
- Add sync.RWMutex to Enforcer; Enforce/EnforceWithDefault take RLock, SetUserPolicy builds outside lock then takes Lock only for pointer swap - go mod tidy: casbin/v2 moved from indirect to direct-require block - TestListsCoverConstants: assert.ElementsMatch with all constant identifiers - TestBuiltinRoleMatrix: add operator-cannot-create and operator-cannot-undo - TestEnforcerConcurrency: race-detector exercise (20 readers + 2 writers) - TestEnforceWithDefaultDirectlyAllowed: exercises ok==true early-return path - BuiltinPolicyCSV doc: note bare-* glob caveat and user-policy append order Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan for two pure auth packages: bcrypt password hashing/verification, and an HS256 JWT session manager with create + parse (signature/algorithm/issuer/expiry validation, alg-confusion guard). Second of seven stacked plans. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
…-reject test - M1: VerifyPassword now rejects passwords >72 bytes before bcrypt (mirrors HashPassword guard) - M2: TestParseRejectsRS256 — algorithm-confusion attack guard - M4: Parse now requires exp claim via jwt.WithExpirationRequired(); TestParseRejectsMissingExpiry added - M5: NewSessionManager godoc documents minimum 32-byte key requirement Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan for server/auth/settings: SettingsManager reading the dashboard cm/secret from k8s (fake-client tested), signing-key accessor (>=32B enforced), local account store + VerifyUsernamePassword (reuses bcrypt), and RBAC/anonymous/url config accessors. Third of seven stacked plans. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan for the server/auth package: claims-context helper, token-from-metadata extraction (Authorization/cookie), and unary+stream gRPC authentication interceptors (whitelist, anonymous, fail-closed on bad token). First of three Plan-4 sub-plans (interceptor / login+authz / server-mode wiring). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
- interceptor.go: after Parse succeeds with nil claims, normalize to
jwt.MapClaims{} so authenticated requests always carry a non-nil map
(Fix A — invariant required by Plan 4b authz layer)
- claims.go: document TokenVerifier.Parse contract in interface comment
- token_test.go: add case-insensitive Bearer stripping tests (lower/UPPER)
and empty-credential sentinel test
- stream_test.go: make fail-closed guarantee explicit with handlerCalled
assertion in TestStreamInvalidTokenRejected
- interceptor_test.go: add TestUnaryNilClaimsNormalized proving Fix A
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan: claims-based EnforceClaims helper (subject+groups, default-role, deny-by-default), RolloutService method->permission map, and login/logout HTTP handlers with anti-enumeration (generic error + dummy bcrypt). Object extraction + interceptor wiring deferred to P4c. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
- init() panics if bcrypt hash for timing equalizer fails (was silently dropped) - LogoutHandler now sets SameSite=Strict to close forced-logout CSRF vector - TestLoginBadCredentialsGeneric asserts byte-identical status+body across all three variants - TestLoginCookieSecureFlag covers Secure=true and Secure=false paths explicitly - TestLogoutClearsCookie asserts SameSite=Strict on logout cookie Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Move timing-equalizer bcrypt into VerifyUsernamePassword (not-found and disabled paths now each run one dummy bcrypt via dummyPasswordHash/init). Drop the handler-level burn (was redundant, made wrong-password 2x slower). Add MaxBytesReader(4096) cap before JSON decode as cheap pre-auth DoS guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Bite-sized TDD plan: duck-typed RBAC object extraction from requests (no rollout-proto coupling) + unary authorization interceptor (PermissionForMethod -> EnforceClaims, deny-by-default, unmapped methods pass through). Stream/server wiring deferred to P4d. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Adds LoginPage component (AntD Form) posting JSON to /api/login with credentials:include, onSuccess callback, error display, and SSO anchor. Includes jsdom test with window.matchMedia mock for AntD v5 compatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Add shouldRedirectToLogin guard in auth-fetch + api.tsx (Fix A1) and skip the namespace bootstrap useEffect when already on /login (Fix A2); fix misleading jsdom-smoke comment (Fix B). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Kustomize overlay manifests/dashboard-install-server/: the three config objects (cm/secret stub/rbac-cm), least-privilege RBAC for the dashboard SA to read them, and a --auth-mode=server patch over the existing no-auth base. Empty signing-key secret fails safe; README documents setup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
buildTLSConfig returned a tls.Config without NextProtos. The in-process grpc-gateway dials the gRPC server over the same TLS-wrapped port and requires an h2 ALPN selection; without it the handshake fails with "missing selected ALPN property" and every gateway call surfaces as HTTP 503 (gRPC UNAVAILABLE), leaving the dashboard blank in the default (non --insecure) server mode. Advertise h2 and http/1.1 via ALPN so both the gateway's gRPC dial and browser HTTP/1.1 traffic negotiate correctly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Replace the bare centered form with the Argo CD split-screen layout: a branded gradient panel (tagline + Argo logo) on the left and a white sign-in box on the right with the Argo Rollouts wordmark, an Argo-orange primary button, an "or" separator, the SSO link, and a footer logo. The left panel collapses on narrow viewports. Markup keeps the same form contract (Username/Password labels, "Log in" submit, SSO link to /auth/login) so existing login tests pass unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
…ard manifest - Always set the Secure flag on the session and OIDC state cookies instead of deriving it from whether TLS is enabled. The session token must never travel over cleartext HTTP; localhost remains a browser secure context so local development still works, and any non-localhost deployment must terminate TLS for login to function. Removes the now-unused Secure plumbing from LoginHandler, the OIDC Handler, and ProviderConfig. - Give the OIDC state-clearing cookie the same HttpOnly/Secure/SameSite attributes as the cookie it overwrites (CodeQL: missing HttpOnly/Secure). - Set memory/CPU requests and a memory limit on the dashboard server-mode container (Sonar: memory limits should be enforced). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
- gofmt/goimports on server/auth files flagged by golangci-lint. - Regenerate docs/generated kubectl-argo-rollouts_dashboard.md to include the new --auth-mode and --insecure flags (Verify Codegen). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
…iles Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
| // InsecureSkipVerify is intentional and safe here: this is a loopback in-process | ||
| // dial (gateway → local gRPC on the same host); external clients still validate | ||
| // against the served certificate. | ||
| opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{InsecureSkipVerify: true}))) //nolint:gosec |
CodeQL flagged the in-process grpc-gateway → gRPC dial for disabling TLS certificate verification (InsecureSkipVerify: true). Replace it with a pinned-certificate verifier: trust exactly the served certificate as the sole root and set ServerName to one of its own SANs. The loopback dial is now fully verified for both the generated self-signed certificate and an operator-provided one, with no certificate check disabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Contributor
Published E2E Test Results 4 files 4 suites 3h 45m 19s ⏱️ For more details on these failures, see this check. Results for commit 48a8aa1. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4808 +/- ##
==========================================
+ Coverage 85.03% 85.04% +0.01%
==========================================
Files 166 187 +21
Lines 19089 19724 +635
==========================================
+ Hits 16232 16774 +542
- Misses 2007 2056 +49
- Partials 850 894 +44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Published Unit Test Results2 666 tests 2 666 ✅ 3m 22s ⏱️ Results for commit 48a8aa1. ♻️ This comment has been updated with latest results. |
Raise patch coverage on the dashboard auth wiring: unit-test loopbackDialCreds (ServerName SAN selection, leaf parsing, error path) and the authenticated newGRPCServer / newHTTPServer paths (interceptor chaining, login/logout route mounts under TLS). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Add focused unit tests for previously-uncovered auth paths so the PR clears the codecov patch target: - OIDC provider: discovery wiring, token exchange (id_token present/ missing/endpoint error), and real RS256 ID-token verification via a JWKS test server (including wrong-audience rejection). - setupAuth OIDC branch: handler built with config+url, url-required error, and discovery failure; anonymous-enabled path. - login issuer-error 500, session non-HMAC rejection, cookie parsing, and SettingsManager API-error propagation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Add reachable-branch tests: OIDC login route mount + redirect, checkServeErr graceful path, enforcer default-role fallback, expired session token rejection, GetAccount API-error propagation, and parseBoolDefault fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
Propagate-on-error tests for AnonymousEnabled/GetURL/GetOIDCConfig, plus GetRBACConfig matchMode override and GetTLSCertificate invalid-keypair rejection, to lift dashboard auth patch coverage over the threshold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Duck <duchuynhtran12a1@gmail.com>
|
kostis-codefresh
requested changes
Jun 29, 2026
kostis-codefresh
left a comment
kostis-codefresh
left a comment
Member
There was a problem hiding this comment.
Please read all comments on this issue #1323 (comment)
Also #4668 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the dashboard authenticated server mode proposed in #4807.
What this PR does
Adds an opt-in authenticated server mode to the Argo Rollouts dashboard, porting Argo CD's auth stack so the dashboard can be exposed as a shared in-cluster service instead of localhost-only.
Enable with
--auth-mode=server. The default staysnone— the existing localkubectl argo rollouts dashboardis byte-for-byte unchanged, so there is zero breakage for current users.Included
coreos/go-oidcwith audience + signature verification and a CSRF-protected callback; mints a normal dashboard token carryingsub+groupsso SSO and local accounts share the same RBAC path.role:admin,role:readonly,role:operator; full verb set (get/create/update/delete + promote/abort/retry/restart/pause/skip/setimage/undo) across rollouts/analysisruns/analysistemplates/clusteranalysistemplates/experiments.--insecurefor upstream-terminated TLS.manifests/dashboard-install-server/kustomize overlay (config objects, least-privilege RBAC, no shipped secret).Config lives in
argo-rollouts-dashboard-{cm,secret,rbac-cm}.Verification
End-to-end run against a local cluster: local login + OIDC path, RBAC allow/deny, and a
promotewrite RPC through the authz interceptor all confirmed via the UI. Go tests race-clean + vet-clean; UI jest suites green; manifests build underkubectl kustomize.Notes for reviewers
nonemode is unchanged; all auth lives behind--auth-mode=server.Closes #4807
Checklist:
"fix(controller): Updates such and such. Fixes #1234".