diff --git a/Makefile b/Makefile index 32bb1c0d..20e74fd5 100644 --- a/Makefile +++ b/Makefile @@ -273,6 +273,7 @@ KIND_CLUSTER_UI_E2E ?= solar-test-e2e-ui UI_WORK_DIR ?= $(BUILD_PATH)/tmp/ui UI_DEV_WORK_DIR ?= $(BUILD_PATH)/tmp/ui-dev UI_E2E_WORK_DIR ?= $(BUILD_PATH)/tmp/ui-e2e +UI_DEV_PORT ?= 8090 .PHONY: ui-install ui-install: ## Install frontend dependencies @@ -296,7 +297,7 @@ ui-dev-cluster: ocm-transfer-demo ## Create a Kind cluster with SolAr + Dex for $(MAKE) docker-build-local-images TAG=$(DEV_TAG) $(MAKE) kind-load-local-images TAG=$(DEV_TAG) KIND_CLUSTER=$(KIND_CLUSTER_UI_DEV) TAG=$(DEV_TAG) KIND_CLUSTER=$(KIND_CLUSTER_UI_DEV) $(HACK_DIR)/dev-cluster.sh - KIND_CLUSTER=$(KIND_CLUSTER_UI_DEV) $(HACK_DIR)/setup-dex.sh + KIND_CLUSTER=$(KIND_CLUSTER_UI_DEV) UI_DEV_PORT=$(UI_DEV_PORT) $(HACK_DIR)/setup-dex.sh .PHONY: ui-cleanup-dev-cluster ui-cleanup-dev-cluster: ## Tear down the UI dev cluster @@ -315,7 +316,7 @@ ui-dev: ui-install ## Start Go backend + Vite dev server against the UI dev clus esac @test -f test/fixtures/dex-ca.crt || { echo "Dex CA cert not found. Run 'make ui-dev-cluster' first."; exit 1; } @echo "Starting Dex port-forward + Vite dev server + solar-ui backend..." - @echo "Open http://localhost:8090 in your browser." + @echo "Open http://localhost:$(UI_DEV_PORT) in your browser." @echo "" @mkdir -p $(UI_DEV_WORK_DIR) @$(KIND) get kubeconfig --name $(KIND_CLUSTER_UI_DEV) > $(UI_DEV_WORK_DIR)/kubeconfig @@ -323,16 +324,66 @@ ui-dev: ui-install ## Start Go backend + Vite dev server against the UI dev clus "KUBECONFIG=$(UI_DEV_WORK_DIR)/kubeconfig $(KUBECTL) port-forward -n dex service/dex 5556:5556" \ "$(PNPM) dev --port 5173" \ "sleep 2 && cd $(BUILD_PATH) && $(GO) run ./cmd/solar-ui \ - --listen=0.0.0.0:8090 \ + --listen=0.0.0.0:$(UI_DEV_PORT) \ --kubeconfig=$(UI_DEV_WORK_DIR)/kubeconfig \ --oidc-issuer=https://localhost:5556 \ --oidc-ca-cert=$(BUILD_PATH)/test/fixtures/dex-ca.crt \ --oidc-client-id=solar-ui \ --oidc-client-secret=solar-ui-secret \ - --oidc-redirect-url=http://localhost:8090/api/auth/callback \ + --oidc-redirect-url=http://localhost:$(UI_DEV_PORT)/api/auth/callback \ --auth-mode=token \ --dev-vite-url=http://localhost:5173" +# The remote Zitadel our production deployments authenticate against +ZITADEL_ISSUER ?= https://zitadel.opendefense.cloud +ZITADEL_CLIENT_ID ?= 387085129840888657 +ZITADEL_REDIRECT_URL ?= http://localhost:$(UI_DEV_PORT)/api/auth/callback +# Kubernetes username to grant cluster-admin in the dev cluster — your Zitadel +# email. Without it you can log in but every API call is denied. +ZITADEL_USER ?= +# How the identity reaches Kubernetes. "impersonate" needs no cluster config: +# the BFF authenticates with the admin kubeconfig and impersonates you. +# "token" is what production uses — the API server validates the id_token +# itself, so the issuer has to be registered in its authentication config, +# which ui-dev-zitadel does for you. +ZITADEL_AUTH_MODE ?= impersonate + +.PHONY: ui-dev-zitadel +ui-dev-zitadel: ui-install ## Start Go backend + Vite dev server against the remote Zitadel (PKCE, no client secret) + @case "$$($(KIND) get clusters 2>/dev/null)" in \ + *"$(KIND_CLUSTER_UI_DEV)"*) ;; \ + *) echo "UI dev cluster not found. Creating it..."; $(MAKE) ui-dev-cluster ;; \ + esac + @mkdir -p $(UI_DEV_WORK_DIR) + @$(KIND) get kubeconfig --name $(KIND_CLUSTER_UI_DEV) > $(UI_DEV_WORK_DIR)/kubeconfig + @if [ "$(ZITADEL_AUTH_MODE)" = "token" ]; then \ + KIND_CLUSTER=$(KIND_CLUSTER_UI_DEV) KUBECTL=$(KUBECTL) \ + KUBECONFIG="$(UI_DEV_WORK_DIR)/kubeconfig" WORK_DIR="$(UI_DEV_WORK_DIR)" \ + ZITADEL_ISSUER=$(ZITADEL_ISSUER) ZITADEL_CLIENT_ID=$(ZITADEL_CLIENT_ID) \ + $(HACK_DIR)/trust-zitadel-issuer.sh; \ + fi + @if [ -n "$(ZITADEL_USER)" ]; then \ + echo "Granting cluster-admin to $(ZITADEL_USER)..."; \ + KUBECONFIG=$(UI_DEV_WORK_DIR)/kubeconfig $(KUBECTL) create clusterrolebinding solar-ui-zitadel-admin \ + --clusterrole=cluster-admin --user='$(ZITADEL_USER)' --dry-run=client -o yaml \ + | KUBECONFIG=$(UI_DEV_WORK_DIR)/kubeconfig $(KUBECTL) apply -f -; \ + else \ + echo "WARNING: ZITADEL_USER is unset — you will log in but see 403s."; \ + echo " Re-run with: make ui-dev-zitadel ZITADEL_USER=you@example.com"; \ + fi + @echo "Open http://localhost:$(UI_DEV_PORT) in your browser." + @echo "" + cd web && $(PNPM) exec concurrently --kill-others --names "vite,bff" --prefix-colors "cyan,yellow" \ + "$(PNPM) dev --port 5173" \ + "sleep 2 && cd $(BUILD_PATH) && $(GO) run ./cmd/solar-ui \ + --listen=0.0.0.0:$(UI_DEV_PORT) \ + --kubeconfig=$(UI_DEV_WORK_DIR)/kubeconfig \ + --oidc-issuer=$(ZITADEL_ISSUER) \ + --oidc-client-id=$(ZITADEL_CLIENT_ID) \ + --oidc-redirect-url=$(ZITADEL_REDIRECT_URL) \ + --auth-mode=$(ZITADEL_AUTH_MODE) \ + --dev-vite-url=http://localhost:5173" + .PHONY: ui-e2e-cluster ui-e2e-cluster: ocm-transfer-demo ## Create a Kind cluster with Dex + SolAr for UI e2e testing WORK_DIR=$(UI_E2E_WORK_DIR) $(HACK_DIR)/generate-dex-certs.sh @@ -342,7 +393,7 @@ ui-e2e-cluster: ocm-transfer-demo ## Create a Kind cluster with Dex + SolAr for $(MAKE) kind-load-local-images TAG=$(TAG) KIND_CLUSTER=$(KIND_CLUSTER_UI_E2E) REGISTRY=$(REGISTRY); \ fi REGISTRY=$(REGISTRY) TAG=$(TAG) KIND_CLUSTER=$(KIND_CLUSTER_UI_E2E) $(HACK_DIR)/dev-cluster.sh - KIND_CLUSTER=$(KIND_CLUSTER_UI_E2E) $(HACK_DIR)/setup-dex.sh + KIND_CLUSTER=$(KIND_CLUSTER_UI_E2E) UI_DEV_PORT=$(UI_DEV_PORT) $(HACK_DIR)/setup-dex.sh .PHONY: ui-cleanup-e2e-cluster ui-cleanup-e2e-cluster: ## Tear down the UI e2e cluster @@ -365,7 +416,7 @@ ui-test-e2e: ui-build ## Run Playwright UI e2e tests (auto-creates cluster if ne esac @# Build and run the compiled binary directly rather than `go run`: `go run` @# spawns a child process that outlives a `kill` of its parent, leaving an - @# orphaned backend bound to :8090 that breaks (and flakes) subsequent runs. + @# orphaned backend bound to :$(UI_DEV_PORT) that breaks (and flakes) subsequent runs. @$(GO) build -o $(LOCALBIN)/solar-ui ./cmd/solar-ui @mkdir -p $(UI_E2E_WORK_DIR) @$(KIND) get kubeconfig --name $(KIND_CLUSTER_UI_E2E) > $(UI_E2E_WORK_DIR)/kubeconfig @@ -380,21 +431,21 @@ ui-test-e2e: ui-build ## Run Playwright UI e2e tests (auto-creates cluster if ne done; \ echo "Starting solar-ui backend..."; \ $(LOCALBIN)/solar-ui \ - --listen=0.0.0.0:8090 \ + --listen=0.0.0.0:$(UI_DEV_PORT) \ --kubeconfig=$(UI_E2E_WORK_DIR)/kubeconfig \ --oidc-issuer=https://localhost:5556 \ --oidc-ca-cert=$(BUILD_PATH)/test/fixtures/dex-ca.crt \ --oidc-client-id=solar-ui \ --oidc-client-secret=solar-ui-secret \ - --oidc-redirect-url=http://localhost:8090/api/auth/callback \ + --oidc-redirect-url=http://localhost:$(UI_DEV_PORT)/api/auth/callback \ --auth-mode=token >$(UI_E2E_WORK_DIR)/bff.log 2>&1 & \ UI_PID=$$!; \ - echo "Waiting for solar-ui backend (http://localhost:8090)..."; \ + echo "Waiting for solar-ui backend (http://localhost:$(UI_DEV_PORT))..."; \ for i in $$(seq 1 60); do \ - curl -sf http://localhost:8090/api/auth/me >/dev/null 2>&1 && break; \ + curl -sf http://localhost:$(UI_DEV_PORT)/api/auth/me >/dev/null 2>&1 && break; \ sleep 1; \ done; \ - cd web && DEX_LOCAL_PORT=5556 $(PNPM) exec playwright test; \ + cd web && DEX_LOCAL_PORT=5556 UI_DEV_PORT=$(UI_DEV_PORT) $(PNPM) exec playwright test; \ exit $$? ifeq ($(OS),darwin) ui-test-e2e: ui-playwright-browser diff --git a/docs/developer-guide/frontend-development.md b/docs/developer-guide/frontend-development.md index f5bd4b75..e0499fc2 100644 --- a/docs/developer-guide/frontend-development.md +++ b/docs/developer-guide/frontend-development.md @@ -77,6 +77,31 @@ The UI uses OIDC against the in-cluster Dex. After opening `http://localhost:809 All passwords are the literal string `password`. Cluster RBAC bindings live in `test/fixtures/e2e/dex/dex-rbac.yaml` (inlined from `docs/developer-guide/manifests/`). +## Testing against the remote Zitadel + +Production authenticates against Zitadel, not Dex. To point the dev UI at it: + +```bash +make ui-dev-zitadel ZITADEL_USER=you@example.com +``` + +`ZITADEL_USER` is your Zitadel email. It becomes your Kubernetes username, and the target grants it `cluster-admin` in the dev cluster. Issuer and client ID default to the real ones (see Makefile); override `ZITADEL_ISSUER`, `ZITADEL_CLIENT_ID` or `ZITADEL_REDIRECT_URL` to point elsewhere. + +Three things differ from the Dex flow: + +- **Public client with PKCE.** The BFF holds no client secret; it authenticates the code exchange with an S256 challenge. Zitadel registers us as a _native app_, which is what permits the loopback redirect URI. PKCE is sent on every login regardless of IdP, so there is nothing to switch. +- **`--auth-mode=impersonate` by default.** The BFF authenticates with the admin kubeconfig and impersonates you, so the cluster needs no OIDC configuration at all. Production uses token mode; to run the dev cluster the same way: + + ```bash + make ui-dev-zitadel ZITADEL_USER=you@example.com ZITADEL_AUTH_MODE=token + ``` + + That runs `hack/trust-zitadel-issuer.sh`, which registers the issuer in the API server's authentication config (audience = client ID, `email` claim as the username) and waits for the hot reload. The Kind node needs egress and DNS to fetch the issuer's JWKS. The Dex issuer stays registered alongside it, so `make ui-dev` keeps working. + +- **No groups.** Zitadel emits no `groups` claim, and the BFF reads only that claim, so sessions come back with an empty group list. Nothing depends on it: cluster RBAC binds on the `email` claim, and the UI decides what to show by asking Kubernetes (`SelfSubjectAccessReview` / `SelfSubjectRulesReview`) rather than by inspecting groups. The application does need `idTokenUserinfoAssertion` enabled so `email` and `name` are in the ID token at all — the BFF never calls the userinfo endpoint. + +Zitadel ignores the port of a loopback redirect URI, so if `:8090` is busy you can run on another port as long as `ZITADEL_REDIRECT_URL` and the browser agree. + ## Namespace selector The sidebar's namespace dropdown is the global scope for every list page (Targets, Releases, Components, Profiles, …). It has two modes: diff --git a/docs/operator-manual/installation/installation.md b/docs/operator-manual/installation/installation.md index 24672f1b..a1495bae 100644 --- a/docs/operator-manual/installation/installation.md +++ b/docs/operator-manual/installation/installation.md @@ -21,3 +21,31 @@ To install SolAr, navigate to the [releases page](https://github.com/opendefense #### Helm See [Helm installation](./helm.md) for more information. + +### Web UI (OIDC) + +The UI is off by default because it needs an OIDC issuer. The backend is a **public client**: it holds no client secret and authenticates the authorization-code exchange with PKCE (S256) instead. Set `ui.oidc.existingSecret` only if your IdP issues a confidential client. + +Installing against the Open Defense Cloud Zitadel: + +```yaml +ui: + enabled: true + oidc: + issuer: https://zitadel.opendefense.cloud + clientID: '387085129840888657' + # The externally reachable /api/auth/callback of this UI. SolAr ships no + # Ingress, so access is via `kubectl port-forward` and the browser sees + # loopback. Registered as a native app in Zitadel, which permits loopback + # redirect URIs without putting the application into development mode. + redirectURL: http://localhost:8090/api/auth/callback + existingSecret: '' # public client — PKCE, no secret +``` + +Then port-forward and open `http://localhost:8090`: + +```bash +kubectl port-forward -n solar-system svc/solar-ui 8090:8090 +``` + +Kubernetes RBAC binds to the `email` claim, so the API server must be configured to trust the same issuer with the client ID as audience. See [Roles](../../developer-guide/roles.md) for the persona bindings. diff --git a/hack/setup-dex.sh b/hack/setup-dex.sh index 6afdd710..6c5ade29 100755 --- a/hack/setup-dex.sh +++ b/hack/setup-dex.sh @@ -6,6 +6,7 @@ KUBECTL="${KUBECTL:-kubectl}" SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PROJECT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" CERT_DIR="${CERT_DIR:-$PROJECT_DIR/test/fixtures}" +UI_DEV_PORT="${UI_DEV_PORT:-8090}" # Guard against accidentally applying cluster-admin RBAC to a non-local cluster. # This script is for local Kind-based dev/test only. @@ -37,7 +38,9 @@ $KUBECTL create secret tls dex-tls -n dex \ --dry-run=client -o yaml | $KUBECTL apply -f - # Deploy Dex config and deployment -$KUBECTL apply -f "$PROJECT_DIR/test/fixtures/e2e/dex/dex-config.yaml" +$KUBECTL apply -f - <<<"$(sed \ + "s|- http://localhost:[0-9]*/api/auth/callback|- http://localhost:${UI_DEV_PORT}/api/auth/callback|" \ + "$PROJECT_DIR/test/fixtures/e2e/dex/dex-config.yaml")" $KUBECTL apply -f "$PROJECT_DIR/test/fixtures/e2e/dex/dex-deployment.yaml" echo "Waiting for Dex deployment to be available..." diff --git a/hack/trust-zitadel-issuer.sh b/hack/trust-zitadel-issuer.sh new file mode 100755 index 00000000..dbf75d9b --- /dev/null +++ b/hack/trust-zitadel-issuer.sh @@ -0,0 +1,93 @@ +#!/usr/bin/env bash +# +# Registers the remote Zitadel as a JWT issuer in the UI dev cluster's API +# server, so it accepts SolAr UI id_tokens directly (--auth-mode=token). +# Only needed for token mode. + +set -euo pipefail + +KUBECTL="${KUBECTL:-kubectl}" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +WORK_DIR="${WORK_DIR:-$PROJECT_DIR/tmp/ui-dev}" + +AUTH_CONFIG="$WORK_DIR/dex-auth-config.yaml" +AUTH_CONFIG_BASE="$WORK_DIR/dex-auth-config.base.yaml" + +ISSUER="${ZITADEL_ISSUER:?ZITADEL_ISSUER is required}" +CLIENT_ID="${ZITADEL_CLIENT_ID:?ZITADEL_CLIENT_ID is required}" + +# Guard against reconfiguring authentication on a non-local cluster. +CURRENT_CONTEXT="$($KUBECTL config current-context 2>/dev/null || true)" +EXPECTED_CONTEXT="${KIND_CLUSTER:+kind-${KIND_CLUSTER}}" +if [[ "${ALLOW_NON_LOCAL_CLUSTER:-false}" != "true" ]]; then + if [[ -n "$EXPECTED_CONTEXT" && "$CURRENT_CONTEXT" != "$EXPECTED_CONTEXT" ]]; then + echo "Refusing to run against context '${CURRENT_CONTEXT:-}' (expected '$EXPECTED_CONTEXT')." >&2 + echo "Switch context or set ALLOW_NON_LOCAL_CLUSTER=true to override intentionally." >&2 + exit 1 + fi + if [[ -z "$EXPECTED_CONTEXT" && ! "$CURRENT_CONTEXT" =~ ^kind- ]]; then + echo "Refusing to reconfigure authentication on non-kind context: ${CURRENT_CONTEXT:-}" >&2 + echo "Set ALLOW_NON_LOCAL_CLUSTER=true to override intentionally." >&2 + exit 1 + fi +fi + +[[ -f "$AUTH_CONFIG" ]] || { echo "Missing $AUTH_CONFIG — run 'make ui-dev-cluster' first." >&2; exit 1; } + +# Rebuilt from a Dex-only base, so a changed issuer or client ID can't leave a +# stale entry behind. The base must not already contain our block. +if [[ ! -f "$AUTH_CONFIG_BASE" ]]; then + if [[ "$(grep -c '^ - issuer:' "$AUTH_CONFIG")" -ne 1 ]]; then + echo "$AUTH_CONFIG has extra issuers and no $AUTH_CONFIG_BASE to rebuild from." >&2 + echo "Regenerate it with hack/generate-dex-certs.sh." >&2 + exit 1 + fi + cp "$AUTH_CONFIG" "$AUTH_CONFIG_BASE" +fi + +# No groups mapping: Zitadel's roles claim is a map, which the API server can't +# turn into group names. RBAC binds on the username. +DESIRED="$(printf '%s\n' "$(<"$AUTH_CONFIG_BASE")" " - issuer: + url: $ISSUER + audiences: + - \"$CLIENT_ID\" + claimMappings: + username: + claim: email + prefix: \"\"")" + +if [[ "$(cat "$AUTH_CONFIG")" == "$DESIRED" ]]; then + echo "$ISSUER already registered in $AUTH_CONFIG." + exit 0 +fi + +echo "Registering $ISSUER in $AUTH_CONFIG..." +# Taken before the write: a reload logged earlier isn't evidence about ours. +SINCE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" +printf '%s\n' "$DESIRED" > "$AUTH_CONFIG" + +echo "Waiting for the API server to reload it..." +for _ in $(seq 1 45); do + sleep 2 + LOG="$($KUBECTL logs -n kube-system -l component=kube-apiserver --since-time="$SINCE" 2>/dev/null || true)" + + ERRORS="$(grep -E "failed to (load|validate|update) authentication config" <<<"$LOG" || true)" + MINE="$(grep -F "\"$ISSUER\"" <<<"$ERRORS" || true)" + UNATTRIBUTED="$(grep -vF 'issuer "' <<<"$ERRORS" || true)" + if [[ -n "$MINE$UNATTRIBUTED" ]]; then + echo "The API server rejected the authentication config:" >&2 + printf '%s\n' "$MINE$UNATTRIBUTED" | tail -1 >&2 + exit 1 + fi + + if grep -q "reloaded authentication config" <<<"$LOG"; then + echo "API server reloaded." + exit 0 + fi +done + +# Changed on disk but identical to what's loaded — e.g. reverting a config the +# API server refused. Nothing to reload. +echo "No API-server reload was observed after changing $AUTH_CONFIG." >&2 +exit 1 diff --git a/pkg/ui/auth/noop_test.go b/pkg/ui/auth/noop_test.go new file mode 100644 index 00000000..5b82c562 --- /dev/null +++ b/pkg/ui/auth/noop_test.go @@ -0,0 +1,72 @@ +// Copyright 2026 BWI GmbH and Solution Arsenal contributors +// SPDX-License-Identifier: Apache-2.0 + +package auth + +import ( + "net/http" + "net/http/httptest" + + "k8s.io/client-go/rest" + + "go.opendefense.cloud/solar/pkg/ui/session" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("NoopProvider", func() { + var ( + provider *NoopProvider + store *session.Store + ) + + BeforeEach(func() { + provider = NewNoopProvider() + var err error + store, err = session.NewStore("") + Expect(err).NotTo(HaveOccurred()) + }) + + It("establishes a synthetic session on login and redirects", func(ctx SpecContext) { + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/login", nil) + rec := httptest.NewRecorder() + + provider.HandleLogin(store)(rec, req) + + Expect(rec.Code).To(Equal(http.StatusFound)) + Expect(rec.Header().Get("Location")).To(Equal("/")) + + // The set cookie resolves to a noop session. + req2 := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) + req2.AddCookie(rec.Result().Cookies()[0]) + Expect(store.Get(req2).Username).To(Equal(noopUsername)) + }) + + It("establishes a session on callback and redirects", func(ctx SpecContext) { + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback", nil) + rec := httptest.NewRecorder() + + provider.HandleCallback(store)(rec, req) + + Expect(rec.Code).To(Equal(http.StatusFound)) + + // The set cookie resolves to a noop session. + cookies := rec.Result().Cookies() + Expect(cookies).NotTo(BeEmpty()) + req2 := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) + req2.AddCookie(cookies[0]) + Expect(store.Get(req2).Username).To(Equal(noopUsername)) + }) + + It("returns the base config unchanged", func() { + base := &rest.Config{Host: "https://example"} + + before := *base + result := provider.WrapConfig(base, &session.Data{Username: "alice"}) + + // Same pointer back, and its contents were not mutated in place. + Expect(result).To(BeIdenticalTo(base)) + Expect(*result).To(Equal(before)) + }) +}) diff --git a/pkg/ui/auth/oidc.go b/pkg/ui/auth/oidc.go index 1e7787a0..ab48c8d0 100644 --- a/pkg/ui/auth/oidc.go +++ b/pkg/ui/auth/oidc.go @@ -36,8 +36,10 @@ const ( // OIDCConfig holds the configuration for the OIDC provider. type OIDCConfig struct { - Issuer string - ClientID string + Issuer string + ClientID string + // ClientSecret is empty for a public client, which authenticates the code + // exchange with PKCE instead. ClientSecret string //nolint:gosec // config field, not a hardcoded credential RedirectURL string AuthMode AuthMode @@ -72,11 +74,18 @@ func NewOIDCProvider(cfg OIDCConfig) (*OIDCProvider, error) { return nil, fmt.Errorf("failed to create OIDC provider for issuer %q: %w", cfg.Issuer, err) } + endpoint := provider.Endpoint() + if cfg.ClientSecret == "" { + // Public client (PKCE): the client_id goes in the request body instead + // of the Authorization header. + endpoint.AuthStyle = oauth2.AuthStyleInParams + } + oauthCfg := oauth2.Config{ ClientID: cfg.ClientID, ClientSecret: cfg.ClientSecret, RedirectURL: cfg.RedirectURL, - Endpoint: provider.Endpoint(), + Endpoint: endpoint, Scopes: []string{oidc.ScopeOpenID, "profile", "email", "groups"}, } @@ -130,8 +139,13 @@ func newHTTPClient(caCertFile string) (*http.Client, error) { func (p *OIDCProvider) HandleLogin(store *session.Store) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { state := generateState() - store.SetState(w, state) - http.Redirect(w, r, p.oauth.AuthCodeURL(state), http.StatusFound) + // PKCE is sent unconditionally: it is required for public clients and + // harmless for confidential ones + verifier := oauth2.GenerateVerifier() + store.SetState(w, state, verifier) + http.Redirect(w, r, + p.oauth.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)), + http.StatusFound) } } @@ -145,7 +159,7 @@ func (p *OIDCProvider) HandleCallback(store *session.Store) http.HandlerFunc { return } - expectedState := store.GetState(r) + expectedState, verifier := store.GetState(r) actualState := r.URL.Query().Get("state") if expectedState == "" || actualState != expectedState { http.Error(w, "invalid state parameter", http.StatusBadRequest) @@ -156,7 +170,7 @@ func (p *OIDCProvider) HandleCallback(store *session.Store) http.HandlerFunc { ctx := oidc.ClientContext(r.Context(), p.httpClient) - token, err := p.oauth.Exchange(ctx, code) + token, err := p.oauth.Exchange(ctx, code, oauth2.VerifierOption(verifier)) if err != nil { http.Error(w, fmt.Sprintf("token exchange failed: %v", err), http.StatusInternalServerError) diff --git a/pkg/ui/auth/auth_test.go b/pkg/ui/auth/oidc_test.go similarity index 84% rename from pkg/ui/auth/auth_test.go rename to pkg/ui/auth/oidc_test.go index bf152bfa..584825e2 100644 --- a/pkg/ui/auth/auth_test.go +++ b/pkg/ui/auth/oidc_test.go @@ -19,6 +19,7 @@ import ( "math/big" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "time" @@ -31,9 +32,18 @@ import ( . "github.com/onsi/gomega" ) +// tokenForm captures the form the fake IdP's token endpoint last received, so +// tests can assert on what the client sent (e.g. the PKCE code_verifier). +var tokenForm url.Values + // fakeIDP is a minimal OIDC provider that issues a verifiable id_token, so the // full callback path (token exchange → JWT verification → claims) can be tested. -func fakeIDP(clientID string) *httptest.Server { +// userClaims is a JSON fragment (no braces, no leading comma) appended to the +// registered claims. It is what distinguishes a Dex-shaped token from a +// Zitadel-shaped one. +func fakeIDP(clientID, userClaims string) *httptest.Server { + tokenForm = nil + key, err := rsa.GenerateKey(rand.Reader, 2048) Expect(err).NotTo(HaveOccurred()) b64 := base64.RawURLEncoding.EncodeToString @@ -56,12 +66,15 @@ func fakeIDP(clientID string) *httptest.Server { "e": b64(big.NewInt(int64(key.PublicKey.E)).Bytes()), }}}) }) - mux.HandleFunc("/token", func(w http.ResponseWriter, _ *http.Request) { + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + _ = r.ParseForm() + tokenForm = r.Form + now := time.Now().Unix() header := b64([]byte(`{"alg":"RS256","typ":"JWT","kid":"test"}`)) payload := b64(fmt.Appendf(nil, - `{"iss":%q,"aud":%q,"sub":"user-1","exp":%d,"iat":%d,"email":"alice@example.com","groups":["devs"]}`, - srv.URL, clientID, now+3600, now)) + `{"iss":%q,"aud":%q,"sub":"user-1","exp":%d,"iat":%d,%s}`, + srv.URL, clientID, now+3600, now, userClaims)) digest := sha256.Sum256([]byte(header + "." + payload)) sig, _ := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA256, digest[:]) idToken := header + "." + payload + "." + b64(sig) @@ -115,50 +128,6 @@ func writeCertPEM() string { return p } -var _ = Describe("NoopProvider", func() { - var ( - provider *NoopProvider - store *session.Store - ) - - BeforeEach(func() { - provider = NewNoopProvider() - var err error - store, err = session.NewStore("") - Expect(err).NotTo(HaveOccurred()) - }) - - It("establishes a synthetic session on login and redirects", func(ctx SpecContext) { - req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/login", nil) - rec := httptest.NewRecorder() - - provider.HandleLogin(store)(rec, req) - - Expect(rec.Code).To(Equal(http.StatusFound)) - Expect(rec.Header().Get("Location")).To(Equal("/")) - - // The set cookie resolves to a noop session. - req2 := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) - req2.AddCookie(rec.Result().Cookies()[0]) - Expect(store.Get(req2).Username).To(Equal(noopUsername)) - }) - - It("establishes a session on callback and redirects", func(ctx SpecContext) { - req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback", nil) - rec := httptest.NewRecorder() - - provider.HandleCallback(store)(rec, req) - - Expect(rec.Code).To(Equal(http.StatusFound)) - Expect(rec.Result().Cookies()).NotTo(BeEmpty()) - }) - - It("returns the base config unchanged", func() { - base := &rest.Config{Host: "https://example"} - Expect(provider.WrapConfig(base, &session.Data{Username: "alice"})).To(BeIdenticalTo(base)) - }) -}) - var _ = Describe("newHTTPClient", func() { It("returns a plain client when no CA file is set", func() { c, err := newHTTPClient("") @@ -245,6 +214,27 @@ var _ = Describe("OIDCProvider handlers", func() { Expect(rec.Result().Cookies()).NotTo(BeEmpty()) }) + It("sends an S256 PKCE challenge derived from the stored verifier", func(ctx SpecContext) { + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/login", nil) + rec := httptest.NewRecorder() + + provider.HandleLogin(store)(rec, req) + + loc, err := url.Parse(rec.Header().Get("Location")) + Expect(err).NotTo(HaveOccurred()) + Expect(loc.Query().Get("code_challenge_method")).To(Equal("S256")) + + follow := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) + follow.AddCookie(rec.Result().Cookies()[0]) + state, verifier := store.GetState(follow) + Expect(state).To(Equal(loc.Query().Get("state"))) + Expect(verifier).NotTo(BeEmpty()) + + sum := sha256.Sum256([]byte(verifier)) + Expect(loc.Query().Get("code_challenge")). + To(Equal(base64.RawURLEncoding.EncodeToString(sum[:]))) + }) + It("rejects a callback with no code", func(ctx SpecContext) { req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback", nil) rec := httptest.NewRecorder() @@ -265,7 +255,7 @@ var _ = Describe("OIDCProvider handlers", func() { It("rejects a callback whose state does not match", func(ctx SpecContext) { setState := httptest.NewRecorder() - store.SetState(setState, "expected") + store.SetState(setState, "expected", "verifier") req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback?code=abc&state=wrong", nil) req.AddCookie(setState.Result().Cookies()[0]) @@ -279,7 +269,7 @@ var _ = Describe("OIDCProvider handlers", func() { It("returns 500 when the token exchange fails", func(ctx SpecContext) { // provider's issuer has no /token endpoint, so Exchange gets a 404. setState := httptest.NewRecorder() - store.SetState(setState, "s") + store.SetState(setState, "s", "verifier") req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback?code=abc&state=s", nil) req.AddCookie(setState.Result().Cookies()[0]) @@ -292,8 +282,9 @@ var _ = Describe("OIDCProvider handlers", func() { }) var _ = Describe("OIDCProvider callback success", func() { - It("exchanges the code, verifies the id_token, and establishes a session", func(ctx SpecContext) { - idp := fakeIDP("solar") + // login drives the full callback and returns the resulting session. + login := func(ctx SpecContext, userClaims string) *session.Data { + idp := fakeIDP("solar", userClaims) DeferCleanup(idp.Close) provider, err := NewOIDCProvider(OIDCConfig{Issuer: idp.URL, ClientID: "solar", RedirectURL: "http://app/cb"}) @@ -302,7 +293,7 @@ var _ = Describe("OIDCProvider callback success", func() { Expect(err).NotTo(HaveOccurred()) setState := httptest.NewRecorder() - store.SetState(setState, "s") + store.SetState(setState, "s", "verifier") req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/api/auth/callback?code=abc&state=s", nil) req.AddCookie(setState.Result().Cookies()[0]) rec := httptest.NewRecorder() @@ -317,11 +308,30 @@ var _ = Describe("OIDCProvider callback success", func() { for _, c := range rec.Result().Cookies() { follow.AddCookie(c) } - sess := store.Get(follow) + + return store.Get(follow) + } + + It("exchanges the code, verifies the id_token, and establishes a session", func(ctx SpecContext) { + sess := login(ctx, `"email":"alice@example.com","groups":["devs"]`) + Expect(sess).NotTo(BeNil()) Expect(sess.Username).To(Equal("alice@example.com")) Expect(sess.Groups).To(Equal([]string{"devs"})) }) + + It("sends the PKCE verifier to the token endpoint", func(ctx SpecContext) { + login(ctx, `"email":"alice@example.com"`) + + Expect(tokenForm.Get("code_verifier")).To(Equal("verifier")) + }) + + It("leaves groups empty when the token carries no groups claim", func(ctx SpecContext) { + sess := login(ctx, `"email":"alice@example.com"`) + + Expect(sess.Username).To(Equal("alice@example.com")) + Expect(sess.Groups).To(BeEmpty()) + }) }) var _ = Describe("OIDCProvider.WrapConfig", func() { diff --git a/pkg/ui/session/store.go b/pkg/ui/session/store.go index 0278474b..6fbb0918 100644 --- a/pkg/ui/session/store.go +++ b/pkg/ui/session/store.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "fmt" "net/http" + "strings" "sync" "time" ) @@ -15,6 +16,7 @@ import ( const ( cookieName = "solar-session" stateCookieName = "solar-oidc-state" //nolint:gosec // not a credential + stateSeparator = ":" ) // Data holds session data. @@ -219,11 +221,12 @@ func (s *Store) Clear(w http.ResponseWriter, r *http.Request) { }) } -// SetState stores the OIDC state parameter in a short-lived cookie. -func (s *Store) SetState(w http.ResponseWriter, state string) { +// SetState stores the OIDC state parameter and PKCE code verifier in a +// short-lived cookie. +func (s *Store) SetState(w http.ResponseWriter, state, verifier string) { http.SetCookie(w, &http.Cookie{ Name: stateCookieName, - Value: state, + Value: state + stateSeparator + verifier, Path: "/api/auth/", HttpOnly: true, Secure: true, @@ -232,14 +235,20 @@ func (s *Store) SetState(w http.ResponseWriter, state string) { }) } -// GetState retrieves the OIDC state parameter from the cookie. -func (s *Store) GetState(r *http.Request) string { +// GetState retrieves the OIDC state parameter and PKCE code verifier from the +// cookie. Both are empty when the cookie is missing or malformed. +func (s *Store) GetState(r *http.Request) (state, verifier string) { cookie, err := r.Cookie(stateCookieName) if err != nil { - return "" + return "", "" } - return cookie.Value + state, verifier, found := strings.Cut(cookie.Value, stateSeparator) + if !found || state == "" || verifier == "" { + return "", "" + } + + return state, verifier } // ClearState removes the OIDC state cookie. diff --git a/pkg/ui/session/store_test.go b/pkg/ui/session/store_test.go index 874f054d..4b8b5be0 100644 --- a/pkg/ui/session/store_test.go +++ b/pkg/ui/session/store_test.go @@ -194,18 +194,38 @@ var _ = Describe("Store", func() { }) Describe("OIDC state cookie", func() { - It("round-trips the state value", func(ctx SpecContext) { + withState := func(ctx SpecContext, state, verifier string) *http.Request { rec := httptest.NewRecorder() - store.SetState(rec, "xyz") + store.SetState(rec, state, verifier) req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) req.AddCookie(rec.Result().Cookies()[0]) - Expect(store.GetState(req)).To(Equal("xyz")) + + return req + } + + It("round-trips the state and the PKCE verifier", func(ctx SpecContext) { + state, verifier := store.GetState(withState(ctx, "xyz", "v3r1f13r")) + + Expect(state).To(Equal("xyz")) + Expect(verifier).To(Equal("v3r1f13r")) }) It("returns empty when the state cookie is absent", func(ctx SpecContext) { req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) - Expect(store.GetState(req)).To(BeEmpty()) + state, verifier := store.GetState(req) + + Expect(state).To(BeEmpty()) + Expect(verifier).To(BeEmpty()) + }) + + It("rejects a malformed cookie carrying no verifier", func(ctx SpecContext) { + req := httptest.NewRequestWithContext(ctx, http.MethodGet, "/", nil) + req.AddCookie(&http.Cookie{Name: stateCookieName, Value: "state-without-verifier"}) + state, verifier := store.GetState(req) + + Expect(state).To(BeEmpty()) + Expect(verifier).To(BeEmpty()) }) It("expires the state cookie on clear", func() { diff --git a/web/e2e/auth.setup.ts b/web/e2e/auth.setup.ts index b9d6d0fa..9cad3468 100644 --- a/web/e2e/auth.setup.ts +++ b/web/e2e/auth.setup.ts @@ -16,7 +16,7 @@ setup("authenticate via Dex", async ({ browser }) => { await page.click('button[type="submit"]'); // Wait for redirect back to the app - await page.waitForURL("http://localhost:8090/", { timeout: 15_000 }); + await page.waitForURL("/", { timeout: 15_000 }); // Verify authentication const me = await page.request.get("/api/auth/me"); diff --git a/web/e2e/oidc-login.spec.ts b/web/e2e/oidc-login.spec.ts index 8c20c381..33b5af03 100644 --- a/web/e2e/oidc-login.spec.ts +++ b/web/e2e/oidc-login.spec.ts @@ -9,7 +9,7 @@ import { test, expect } from "@playwright/test"; // // Prerequisites (handled by `make test-e2e-ui`): // - Dex port-forwarded to localhost:5556 -// - solar-ui backend running on :8090 with OIDC configured +// - solar-ui backend running on UI_DEV_PORT (default 8090) with OIDC configured // - DEX_LOCAL_PORT env var set to 5556 // - Static user: admin@solar.local / password @@ -58,7 +58,7 @@ test.describe("OIDC login flow", () => { // After successful auth, Dex redirects back to /api/auth/callback, // which sets the session and redirects to / - await page.waitForURL("http://localhost:8090/", { timeout: 15_000 }); + await page.waitForURL("/", { timeout: 15_000 }); // Verify we are authenticated const meAfter = await page.request.get("/api/auth/me"); diff --git a/web/playwright.config.ts b/web/playwright.config.ts index 38c312d9..717b983c 100644 --- a/web/playwright.config.ts +++ b/web/playwright.config.ts @@ -1,5 +1,7 @@ import { defineConfig, devices } from "@playwright/test"; +const uiPort = process.env.UI_DEV_PORT || "8090"; + export default defineConfig({ testDir: "./e2e", timeout: 60_000, @@ -12,7 +14,7 @@ export default defineConfig({ ? [["github"], ["html", { open: "never" }]] : "html", use: { - baseURL: "http://localhost:8090", + baseURL: `http://localhost:${uiPort}`, trace: "on-first-retry", screenshot: "only-on-failure", // Dex uses a self-signed TLS certificate in dev/test