diff --git a/FEATURE_FLAGS.md b/FEATURE_FLAGS.md index 6b4121a..e100402 100644 --- a/FEATURE_FLAGS.md +++ b/FEATURE_FLAGS.md @@ -25,8 +25,8 @@ The `/api/v1/auth/config` response includes a `features` object: ```json { "authDisabled": false, - "issuer": "https://...", - "clientId": "openshell-dashboard", + "adminRole": "admin", + "logoutUrl": "/oauth2/sign_out", "features": { "terminal": true, "fileTransfer": true, diff --git a/README.md b/README.md index 3c055e8..7e6cc0e 100644 --- a/README.md +++ b/README.md @@ -28,9 +28,9 @@ make dev Open http://localhost:3000, click **Continue as developer**, and you're in. -### Local dev with OIDC (full auth stack) +### Local dev with an OIDC gateway (full stack) -To test with real OIDC authentication against a local Keycloak and OpenShell gateway, use the included dev environment script. This sets up self-signed TLS, a Keycloak instance in Podman, and builds the gateway from source. +To develop against a gateway that has real OIDC configured (Keycloak), use the included dev environment script. This sets up self-signed TLS, a Keycloak instance in Podman, and builds the gateway from source. The dashboard itself runs in dev mode (the gateway allows unauthenticated calls locally); Keycloak mints real JWTs for exercising the Bearer relay path with curl or the OpenShell CLI. To test the full browser-auth flow, put oauth2-proxy in front of the BFF (see Auth below). **Additional prereqs:** Podman (with `podman machine start` on macOS), Rust toolchain (`cargo`), and the [OpenShell](https://github.com/NVIDIA/OpenShell) repo cloned locally. @@ -77,26 +77,61 @@ All flags have env var fallbacks: |------|---------|---------|-------------| | `-port` | `PORT` | `8080` | BFF listen port | | `-gateway-url` | `OPENSHELL_GATEWAY_URL` | `localhost:50051` | Gateway gRPC endpoint (`grpcs://` prefix for TLS) | -| | `OIDC_ISSUER` |: | OIDC issuer URL (enables standalone mode) | -| | `OIDC_CLIENT_ID` |: | OIDC client ID | -| | `OIDC_CLIENT_SECRET` |: | Optional client secret. Without it the BFF is a public client (PKCE only) | -| | `OIDC_SCOPES` | `openid profile email groups` | Requested scopes. `groups` is Keycloak/Dex-shaped; Entra ID rejects it — override for other IdPs | -| | `SESSION_SECRET` |: | Key for encrypted session cookies (standalone OIDC). **Required** unless `DEPLOYMENT_CONTEXT=dev` — the BFF fails closed without it | -| | `DEPLOYMENT_CONTEXT` | `standalone` | `dev` permits an ephemeral session secret for local use | | `-static-dir` | `STATIC_DIR` |: | Serve built frontend from this directory | | `-auth-disabled` | `AUTH_DISABLED` | `false` | Skip auth: **dev only** | +| `-auth-token-header` | `AUTH_TOKEN_HEADER` | `x-forwarded-access-token` | Header the auth proxy injects the bearer into | +| `-auth-user-header` | `AUTH_USER_HEADER` | `x-auth-request-user` | Header the auth proxy injects the username into | +| `-admin-role` | `ADMIN_ROLE` | `admin` | Role name the frontend treats as platform admin (display gating only) | +| `-logout-url` | `LOGOUT_URL` | `/oauth2/sign_out` | Auth proxy sign-out URL the frontend redirects to on logout | | `-gateway-ca-cert` | `GATEWAY_CA_CERT` |: | Path to CA cert for self-signed gateway TLS | -| `-allowed-origins` | `ALLOWED_ORIGINS` |: | Comma-separated extra CORS/WebSocket origins (same-origin is always allowed) | ## Auth -OIDC only (no mTLS, no OpenShift OAuth). Three modes, one middleware: - -- **Standalone OIDC** (`OIDC_ISSUER` + `OIDC_CLIENT_ID` set): the frontend runs an Authorization Code + PKCE flow against your IdP; the BFF exchanges the code server-side and seals the tokens into an encrypted, HttpOnly session cookie (`__Host-openshell-session`). The browser never sees a token, and the cookie authenticates everything — REST calls and the terminal's WebSocket handshake alike. Expired sessions are refreshed against the IdP transparently, server-side (requires the IdP to issue a refresh token — some providers need `offline_access` added to `OIDC_SCOPES`), up to a 12h absolute lifetime. Any spec-compliant OIDC provider works, but defaults are Keycloak/Dex-shaped: adjust `OIDC_SCOPES` for IdPs without a `groups` scope (e.g. Entra ID), register the BFF as a confidential client and set `OIDC_CLIENT_SECRET` where possible, and ensure the **gateway's configured audience matches `OIDC_CLIENT_ID`** — the BFF forwards the ID token as the bearer, and the gateway validates its `aud` against the client ID. -- **Federated** (behind oauth2-proxy / kube-auth-proxy): the proxy injects the user's token as `x-forwarded-access-token`; the BFF forwards it. -- **Dev** (`AUTH_DISABLED=true`): no auth, synthetic dev-user, no tokens forwarded. - -The BFF never validates JWTs — it forwards the bearer to the gateway on every gRPC call, and the gateway makes all RBAC decisions. See `docs/adrs/0010-cookie-session-standalone-auth.md` for the full design. +**The BFF is a token relay.** It runs no OIDC flows, holds no sessions, and +never validates tokens. Browser authentication is owned by an auth proxy in +front of it; the BFF reads the bearer the proxy injects +(`x-forwarded-access-token`, configurable) — or an explicit `Authorization: +Bearer` from API clients — and forwards it to the gateway on every gRPC +call. The gateway validates the JWT against its own OIDC JWKS and makes all +RBAC decisions. + +- **Production / standalone with auth:** run [oauth2-proxy](https://oauth2-proxy.github.io/oauth2-proxy/) + (or kube-auth-proxy on OpenShift) in front of the BFF, registered as an + OIDC client with the **same IdP the gateway trusts**, with an audience the + gateway accepts. oauth2-proxy handles login, cookie sessions, refresh, and + sign-out (`/oauth2/sign_out` — the BFF's default `LOGOUT_URL`), and it + authenticates WebSocket upgrades (the terminal) like any other request. + The secure-agent-workspace validated pattern ships exactly this setup. + **Deployment requirement:** the BFF must only be reachable through the + proxy — anything that can reach the BFF directly can present any header. + + A verified sidecar configuration (Dex as IdP, gateway audience = + `client_id`): + + ``` + --provider=oidc + --oidc-issuer-url=https:// # same issuer the gateway trusts + --client-id=openshell-dashboard # must match the gateway's audience + --redirect-url=https:///oauth2/callback + --upstream=http://127.0.0.1:8080/ # the BFF + --http-address=0.0.0.0:4180 # point the Service/Route here + --scope=openid profile email groups + --pass-authorization-header=true # forwards the ID token as the bearer + --pass-user-headers=true # then set AUTH_USER_HEADER=x-forwarded-user + --email-domain=* + --reverse-proxy=true + --insecure-oidc-allow-unverified-email # needed for IdPs that map a username + # into the email claim without + # email_verified (e.g. Dex's + # OpenShift connector) + ``` + + Note the client must be **confidential** (oauth2-proxy requires a client + secret) — a PKCE-only public client registration is not enough. +- **Dev** (`AUTH_DISABLED=true`): no auth, synthetic dev-user, no tokens + forwarded. `make dev-full` runs the gateway with unauthenticated calls + allowed; Keycloak still mints real JWTs for exercising the Bearer relay + path with curl or the CLI. ## Make targets diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 683845c..89ddb41 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -2,18 +2,19 @@ // (optionally) the built static assets, talking to the OpenShell gateway over // gRPC with per-request bearer token forwarding. // -// Authentication is delegated to an external auth proxy (oauth2-proxy, -// kube-rbac-proxy, etc.) which injects the bearer token as an HTTP header. -// The BFF reads that header and forwards the token to the gateway. +// The BFF is a token relay (ADR 0014): authentication is owned by an external +// auth proxy (oauth2-proxy, kube-auth-proxy, ...) which injects the user's +// bearer token as an HTTP header. The BFF reads that header — or an explicit +// Authorization: Bearer from API clients — and forwards the token to the +// gateway, which validates it against its own OIDC JWKS. The BFF never runs +// OIDC flows, never holds sessions, and never validates tokens. package main import ( - "crypto/rand" "flag" "log/slog" "net/http" "os" - "strings" "time" "github.com/Gkrumbach07/openshell-dashboard/backend/internal/api" @@ -40,11 +41,10 @@ func main() { gatewayCACert = flag.String("gateway-ca-cert", envOr("GATEWAY_CA_CERT", ""), "path to CA cert for gateway TLS (env GATEWAY_CA_CERT)") staticDir = flag.String("static-dir", envOr("STATIC_DIR", ""), "frontend static assets directory (env STATIC_DIR)") authDisabled = flag.Bool("auth-disabled", envOr("AUTH_DISABLED", "false") == "true", "skip auth — dev only (env AUTH_DISABLED)") - origins = flag.String("allowed-origins", envOr("ALLOWED_ORIGINS", ""), "comma-separated CORS origins (env ALLOWED_ORIGINS)") tokenHeader = flag.String("auth-token-header", envOr("AUTH_TOKEN_HEADER", "x-forwarded-access-token"), "header injected by auth proxy containing the bearer token (env AUTH_TOKEN_HEADER)") userHeader = flag.String("auth-user-header", envOr("AUTH_USER_HEADER", "x-auth-request-user"), "header injected by auth proxy containing the username (env AUTH_USER_HEADER)") adminRole = flag.String("admin-role", envOr("ADMIN_ROLE", "admin"), "role name that grants platform admin access (env ADMIN_ROLE)") - logoutURL = flag.String("logout-url", envOr("LOGOUT_URL", "/oauth2/sign_out"), "URL to redirect to on logout (env LOGOUT_URL)") + logoutURL = flag.String("logout-url", envOr("LOGOUT_URL", "/oauth2/sign_out"), "auth proxy sign-out URL to redirect to on logout (env LOGOUT_URL)") ) flag.Parse() @@ -58,54 +58,15 @@ func main() { slog.Warn("AUTH_DISABLED=true — authentication is OFF; never use this outside local development") } - // Federated mode = an auth proxy fronts the BFF (no in-app OIDC issuer). - // Only then is the x-forwarded-access-token header trustworthy; in - // standalone mode any client could forge it to bypass the session cookie. - federated := os.Getenv("OIDC_ISSUER") == "" && !*authDisabled - authMiddleware := auth.New(auth.Config{ - Disabled: *authDisabled, - TokenHeader: *tokenHeader, - UserHeader: *userHeader, - TrustProxyHeader: federated, + Disabled: *authDisabled, + TokenHeader: *tokenHeader, + UserHeader: *userHeader, }) - // Cookie sessions are only used in standalone OIDC mode. SESSION_SECRET - // must be set explicitly outside dev: an auto-generated secret means every - // restart invalidates all sessions, and each replica gets a different key - // so cookies sealed on one pod fail to decrypt on another — surfacing as - // intermittent random logouts. Fail closed unless DEPLOYMENT_CONTEXT=dev. - var sessionCodec *auth.SessionCodec - if issuer := os.Getenv("OIDC_ISSUER"); issuer != "" && !*authDisabled { - secret := os.Getenv("SESSION_SECRET") - if secret == "" { - if os.Getenv("DEPLOYMENT_CONTEXT") != "dev" { - slog.Error("SESSION_SECRET is required when OIDC is configured (set DEPLOYMENT_CONTEXT=dev to allow an ephemeral secret for local development)") - os.Exit(1) - } - generated := make([]byte, 32) - if _, err := rand.Read(generated); err != nil { - slog.Error("failed to generate a session secret", "error", err) - os.Exit(1) - } - secret = string(generated) - slog.Warn("SESSION_SECRET not set — using an ephemeral dev secret; sessions won't survive restarts") - } - codec, err := auth.NewSessionCodec([]byte(secret)) - if err != nil { - slog.Error("session codec setup failed", "error", err) - os.Exit(1) - } - sessionCodec = codec - } - authCfg := api.AuthConfigResponse{ AuthDisabled: *authDisabled, - Issuer: envOr("OIDC_ISSUER", ""), - ClientID: envOr("OIDC_CLIENT_ID", ""), - Scopes: envOr("OIDC_SCOPES", "openid profile email groups"), AdminRole: *adminRole, - UserRole: envOr("OIDC_USER_ROLE", ""), LogoutURL: *logoutURL, Features: api.FeatureFlags{ Terminal: envOr("FEATURE_TERMINAL", "true") == "true", @@ -115,9 +76,6 @@ func main() { CredentialRefresh: envOr("FEATURE_CREDENTIAL_REFRESH", "true") == "true", Services: envOr("FEATURE_SERVICES", "true") == "true", DraftPolicy: envOr("FEATURE_DRAFT_POLICY", "true") == "true", - DeploymentContext: envOr("DEPLOYMENT_CONTEXT", "standalone"), - WorkspaceBinding: envOr("FEATURE_WORKSPACE_BINDING", "false") == "true", - ResourceLinks: envOr("FEATURE_RESOURCE_LINKS", "false") == "true", }, } @@ -128,19 +86,7 @@ func main() { } defer gatewayClient.Close() - var allowedOrigins []string - for _, o := range strings.Split(*origins, ",") { - if trimmed := strings.TrimSpace(o); trimmed != "" { - allowedOrigins = append(allowedOrigins, trimmed) - } - } - - app := api.NewApp(gatewayClient, authMiddleware, sessionCodec, *staticDir, allowedOrigins, authCfg) - // Optional confidential-client secret for the IdP token endpoint (env - // only — never a flag, so it can't leak into process listings). - if secret := os.Getenv("OIDC_CLIENT_SECRET"); secret != "" { - app.SetOIDCClientSecret(secret) - } + app := api.NewApp(gatewayClient, authMiddleware, *staticDir, authCfg) addr := ":" + *port slog.Info("openshell-dashboard BFF listening", diff --git a/backend/internal/api/app.go b/backend/internal/api/app.go index 71fe7d8..7552c57 100644 --- a/backend/internal/api/app.go +++ b/backend/internal/api/app.go @@ -16,38 +16,23 @@ import ( // App wires the gateway client, auth middleware, and REST routes. type App struct { //nolint:govet // fieldalignment: readability over padding - gateway gateway.Interface - auth *auth.Middleware - sessions *auth.SessionCodec + gateway gateway.Interface + auth *auth.Middleware // authConfig is serialized to the browser via GET /auth/config — never // put secrets in it. - authConfig AuthConfigResponse - // oidcClientSecret authenticates the BFF to the IdP token endpoint as a - // confidential client (client_secret_post). Empty = public client (PKCE - // only). Kept outside authConfig so it cannot leak to the frontend. - oidcClientSecret string - staticDir string - allowedOrigins []string - maxUploadSize int64 - execTimeout uint32 + authConfig AuthConfigResponse + staticDir string + maxUploadSize int64 + execTimeout uint32 } -// SetOIDCClientSecret configures confidential-client authentication for -// token-endpoint calls. Optional; without it the BFF acts as a public client. -func (app *App) SetOIDCClientSecret(secret string) { - app.oidcClientSecret = secret -} - -// NewApp builds the application. sessions may be nil, which disables cookie -// sessions (federated and dev deployments don't need them). -func NewApp(gw gateway.Interface, authMiddleware *auth.Middleware, sessions *auth.SessionCodec, staticDir string, allowedOrigins []string, authCfg AuthConfigResponse) *App { +// NewApp builds the application. +func NewApp(gw gateway.Interface, authMiddleware *auth.Middleware, staticDir string, authCfg AuthConfigResponse) *App { app := &App{ - gateway: gw, - auth: authMiddleware, - sessions: sessions, - authConfig: authCfg, - staticDir: staticDir, - allowedOrigins: allowedOrigins, + gateway: gw, + auth: authMiddleware, + authConfig: authCfg, + staticDir: staticDir, } if app.maxUploadSize == 0 { app.maxUploadSize = 64 << 20 // 64 MiB @@ -55,9 +40,6 @@ func NewApp(gw gateway.Interface, authMiddleware *auth.Middleware, sessions *aut if app.execTimeout == 0 { app.execTimeout = 30 } - if sessions != nil && authMiddleware != nil { - authMiddleware.SetSessionAuthenticator(&sessionManager{codec: sessions, app: app}) - } return app } @@ -67,17 +49,10 @@ func (app *App) Routes() http.Handler { r.Use(chimiddleware.RequestID) r.Use(chimiddleware.Logger) r.Use(chimiddleware.Recoverer) - r.Use(app.corsMiddleware) - r.Use(app.csrfMiddleware) r.Route("/api/v1", func(r chi.Router) { - // Public: frontend bootstrap config and OIDC endpoints, no token needed. + // Public: frontend bootstrap config, no token needed. r.Get("/auth/config", app.GetAuthConfig) - r.Get("/auth/discovery", app.GetOIDCDiscovery) - r.Post("/auth/token-exchange", app.TokenExchange) - // Logout is POST: it clears the session (state-changing), so it must - // pass the CSRF Origin check rather than be triggerable by a bare GET. - r.Post("/auth/logout", app.Logout) // BFF liveness (does not call the gateway). r.Get("/healthz", app.GetHealthz) r.Get("/readyz", app.GetReadyz) @@ -85,7 +60,6 @@ func (app *App) Routes() http.Handler { r.Group(func(r chi.Router) { r.Use(app.auth.Handler) - r.Get("/auth/session", app.GetSession) r.Get("/auth/whoami", app.GetWhoAmI) r.Get("/gateway", app.GetGateway) r.Get("/draft-summary", app.GetDraftSummary) @@ -167,59 +141,6 @@ func (app *App) Routes() http.Handler { return r } -// csrfMiddleware rejects cross-origin mutating requests. Cookie-based -// sessions reintroduce CSRF exposure that Bearer headers never had; -// SameSite=Strict on the session cookie is the primary defense, and this -// Origin check is defense-in-depth. Requests without an Origin header -// (curl, server-to-server) pass — they cannot carry a browser's cookies. -func (app *App) csrfMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.Method { - case http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch: - origin := r.Header.Get("Origin") - if origin != "" && !app.originAllowed(origin, requestOrigin(r)) { - writeError(w, http.StatusForbidden, "cross_origin_rejected", "cross-origin request rejected") - return - } - } - next.ServeHTTP(w, r) - }) -} - -func (app *App) originAllowed(origin, requestOrigin string) bool { - // Full-origin (scheme+host) match, so http:// can't pass for an https - // request. requestOrigin is the BFF's own scheme://host for this request. - if origin == requestOrigin { - return true - } - for _, allowed := range app.allowedOrigins { - if origin == allowed { - return true - } - } - return false -} - -func (app *App) corsMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - origin := r.Header.Get("Origin") - for _, allowed := range app.allowedOrigins { - if origin == allowed { - w.Header().Set("Access-Control-Allow-Origin", origin) - w.Header().Set("Vary", "Origin") - w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") - w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type") - break - } - } - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return - } - next.ServeHTTP(w, r) - }) -} - // serveStatic serves the built frontend with SPA fallback to index.html. func (app *App) serveStatic(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/api/") { diff --git a/backend/internal/api/csrf_test.go b/backend/internal/api/csrf_test.go deleted file mode 100644 index fc29946..0000000 --- a/backend/internal/api/csrf_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package api - -import ( - "net/http" - "net/http/httptest" - "testing" -) - -func TestCSRFMiddleware(t *testing.T) { - app := &App{allowedOrigins: []string{"https://allowed.example.com"}} - handler := app.csrfMiddleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - })) - - tests := []struct { - name string - method string - origin string - want int - }{ - {"same-origin post", http.MethodPost, "https://dashboard.example.com", http.StatusOK}, - {"cross-origin post rejected", http.MethodPost, "https://evil.example.com", http.StatusForbidden}, - {"allowlisted origin post", http.MethodPost, "https://allowed.example.com", http.StatusOK}, - {"no origin post (non-browser)", http.MethodPost, "", http.StatusOK}, - {"cross-origin get allowed", http.MethodGet, "https://evil.example.com", http.StatusOK}, - {"cross-origin delete rejected", http.MethodDelete, "https://evil.example.com", http.StatusForbidden}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - req := httptest.NewRequest(tc.method, "https://dashboard.example.com/api/v1/workspaces", nil) - req.Host = "dashboard.example.com" - if tc.origin != "" { - req.Header.Set("Origin", tc.origin) - } - w := httptest.NewRecorder() - handler.ServeHTTP(w, req) - if w.Code != tc.want { - t.Fatalf("status = %d, want %d", w.Code, tc.want) - } - }) - } -} diff --git a/backend/internal/api/gateway_handler.go b/backend/internal/api/gateway_handler.go index 030d2ea..0760633 100644 --- a/backend/internal/api/gateway_handler.go +++ b/backend/internal/api/gateway_handler.go @@ -24,26 +24,19 @@ func (app *App) GetGateway(w http.ResponseWriter, r *http.Request) { // FeatureFlags controls which optional features the frontend should render. type FeatureFlags struct { - DeploymentContext string `json:"deploymentContext"` - Terminal bool `json:"terminal"` - FileTransfer bool `json:"fileTransfer"` - Settings bool `json:"settings"` - GlobalPolicy bool `json:"globalPolicy"` - CredentialRefresh bool `json:"credentialRefresh"` - Services bool `json:"services"` - DraftPolicy bool `json:"draftPolicy"` - WorkspaceBinding bool `json:"workspaceBinding"` - ResourceLinks bool `json:"resourceLinks"` + Terminal bool `json:"terminal"` + FileTransfer bool `json:"fileTransfer"` + Settings bool `json:"settings"` + GlobalPolicy bool `json:"globalPolicy"` + CredentialRefresh bool `json:"credentialRefresh"` + Services bool `json:"services"` + DraftPolicy bool `json:"draftPolicy"` } // AuthConfigResponse tells the frontend whether auth is enabled and which // features are available. type AuthConfigResponse struct { - Issuer string `json:"issuer,omitempty"` - ClientID string `json:"clientId,omitempty"` - Scopes string `json:"scopes,omitempty"` AdminRole string `json:"adminRole,omitempty"` - UserRole string `json:"userRole,omitempty"` LogoutURL string `json:"logoutUrl,omitempty"` Features FeatureFlags `json:"features"` AuthDisabled bool `json:"authDisabled"` @@ -69,10 +62,13 @@ func (app *App) GetAuthConfig(w http.ResponseWriter, _ *http.Request) { // GetCurrentUser or when auth is disabled. func (app *App) GetWhoAmI(w http.ResponseWriter, r *http.Request) { if app.auth.Disabled() { + // The dev user's roles must include the *configured* admin role — + // hardcoding gateway-default role names here made admin pages + // silently inaccessible whenever ADMIN_ROLE differed. writeJSON(w, http.StatusOK, models.CurrentUser{ Subject: "dev-user", DisplayName: "Development User", - Roles: []string{"openshell-admin", "openshell-user"}, + Roles: []string{app.authConfig.AdminRole}, }) return } diff --git a/backend/internal/api/oidc_handler.go b/backend/internal/api/oidc_handler.go deleted file mode 100644 index 3bc857d..0000000 --- a/backend/internal/api/oidc_handler.go +++ /dev/null @@ -1,369 +0,0 @@ -package api - -import ( - "encoding/base64" - "encoding/json" - "fmt" - "io" - "log/slog" - "net/http" - "net/url" - "strings" - "sync" - "time" - - "github.com/Gkrumbach07/openshell-dashboard/backend/internal/auth" -) - -// oidcHTTPClient talks to the IdP. The timeout bounds discovery and token -// calls so a hanging or slow IdP cannot pin goroutines indefinitely. -var oidcHTTPClient = &http.Client{Timeout: 15 * time.Second} - -type discoveryDoc struct { //nolint:govet // fieldalignment: readability over padding - tokenEndpoint string - endSessionEndpoint string - fetchedAt time.Time -} - -// discoveryTTL caches the discovery document so token exchange and refresh -// don't re-fetch it on every call — which matters because refresh happens -// under a lock, and a per-call round trip would lengthen the hold. -const discoveryTTL = 15 * time.Minute - -var ( - discoveryMu sync.Mutex - discoveryCache = map[string]discoveryDoc{} -) - -func discoverOIDCEndpoints(issuer string) (tokenEndpoint, endSessionEndpoint string, err error) { - discoveryMu.Lock() - if cached, ok := discoveryCache[issuer]; ok && time.Since(cached.fetchedAt) < discoveryTTL { - discoveryMu.Unlock() - return cached.tokenEndpoint, cached.endSessionEndpoint, nil - } - discoveryMu.Unlock() - - discoveryURL := strings.TrimRight(issuer, "/") + "/.well-known/openid-configuration" - resp, err := oidcHTTPClient.Get(discoveryURL) - if err != nil { - return "", "", fmt.Errorf("identity provider is unreachable: %w", err) - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return "", "", fmt.Errorf("discovery returned status %d", resp.StatusCode) - } - var discovery struct { - TokenEndpoint string `json:"token_endpoint"` - EndSessionEndpoint string `json:"end_session_endpoint"` - } - if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&discovery); err != nil { - return "", "", fmt.Errorf("failed to parse discovery document: %w", err) - } - if discovery.TokenEndpoint == "" { - return "", "", fmt.Errorf("discovery document has no token_endpoint") - } - - discoveryMu.Lock() - discoveryCache[issuer] = discoveryDoc{ - tokenEndpoint: discovery.TokenEndpoint, - endSessionEndpoint: discovery.EndSessionEndpoint, - fetchedAt: time.Now(), - } - discoveryMu.Unlock() - - return discovery.TokenEndpoint, discovery.EndSessionEndpoint, nil -} - -// GetOIDCDiscovery proxies the OIDC discovery document from the issuer, -// avoiding CORS issues when the frontend and IdP are on different origins. -func (app *App) GetOIDCDiscovery(w http.ResponseWriter, _ *http.Request) { - if app.authConfig.Issuer == "" { - writeError(w, http.StatusServiceUnavailable, "no_issuer", "OIDC issuer not configured") - return - } - issuer := strings.TrimRight(app.authConfig.Issuer, "/") - resp, err := oidcHTTPClient.Get(issuer + "/.well-known/openid-configuration") - if err != nil { - writeError(w, http.StatusBadGateway, "discovery_failed", "identity provider is unreachable") - return - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - writeError(w, http.StatusBadGateway, "discovery_failed", "identity provider returned an error") - return - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _, _ = io.Copy(w, resp.Body) -} - -// tokenResponse is the subset of the IdP token endpoint response we use. -// ExpiresIn is json.Number so a provider that renders it as a JSON string -// (older Azure AD v1.0, non-strict OAuth servers) doesn't fail the decode. -type tokenResponse struct { - AccessToken string `json:"access_token"` - IDToken string `json:"id_token"` - RefreshToken string `json:"refresh_token"` - ExpiresIn json.Number `json:"expires_in"` -} - -// session builds the server-side session from an IdP token response. The ID -// token is preferred as the gateway bearer (it carries the sub/groups claims -// the gateway's RBAC reads); the access token is the fallback. -func (t *tokenResponse) session() *auth.Session { - bearer := t.IDToken - if bearer == "" { - bearer = t.AccessToken - } - now := time.Now().Unix() - s := &auth.Session{Token: bearer, RefreshToken: t.RefreshToken, CreatedAt: now} - - // Expiry must track the *forwarded bearer*, not whichever token expires_in - // happened to describe. expires_in per RFC 6749 is the access token's - // lifetime; when we forward the ID token (which can expire much sooner or - // later — Okta pins ID tokens to 60m), scheduling refresh off expires_in - // leaves the session "live" after the ID token is dead, so the gateway - // 401s and never triggers a refresh. Prefer the bearer's own `exp` claim. - if exp := jwtExpiry(bearer); exp > 0 { - s.ExpiresAt = exp - } - if secs, err := t.ExpiresIn.Int64(); err == nil && secs > 0 { - accessExpiry := now + secs - // Take the earlier of the two so we never overrun the live bearer. - if s.ExpiresAt == 0 || accessExpiry < s.ExpiresAt { - s.ExpiresAt = accessExpiry - } - } - return s -} - -// jwtExpiry reads the `exp` claim (unix seconds) from a JWT's payload without -// verifying the signature. This is NOT token validation — the gateway remains -// the sole authority on token validity; the BFF reads exp only to schedule -// its own refresh. Returns 0 for a non-JWT (opaque) token or a missing claim. -func jwtExpiry(token string) int64 { - parts := strings.Split(token, ".") - if len(parts) != 3 { - return 0 - } - payload, err := base64.RawURLEncoding.DecodeString(parts[1]) - if err != nil { - return 0 - } - var claims struct { - Exp int64 `json:"exp"` - } - if err := json.Unmarshal(payload, &claims); err != nil { - return 0 - } - return claims.Exp -} - -// writeOAuthError surfaces a non-2xx token-endpoint response as a 400 with the -// provider's error/description. A non-2xx is an OAuth error (invalid_grant, -// invalid_client, redirect_uri_mismatch, …); returning a flat 401 would loop -// the frontend through a login that can't succeed on a misconfiguration. -func writeOAuthError(w http.ResponseWriter, tokenResp *http.Response) { - var oauthErr struct { - Error string `json:"error"` - Description string `json:"error_description"` - } - _ = json.NewDecoder(io.LimitReader(tokenResp.Body, 1<<16)).Decode(&oauthErr) - slog.Warn("token endpoint rejected the request", "status", tokenResp.StatusCode, "error", oauthErr.Error, "description", oauthErr.Description) - msg := "identity provider rejected the sign-in" - if oauthErr.Error != "" { - msg = oauthErr.Error - if oauthErr.Description != "" { - msg += ": " + oauthErr.Description - } - } - writeError(w, http.StatusBadRequest, "token_exchange_rejected", msg) -} - -// TokenExchange swaps an authorization code for tokens via the IdP's token -// endpoint, then seals them into the encrypted session cookie. Tokens are -// never returned to the browser — the cookie is the session. -func (app *App) TokenExchange(w http.ResponseWriter, r *http.Request) { - noStore(w) - if app.authConfig.Issuer == "" || app.authConfig.ClientID == "" { - writeError(w, http.StatusBadRequest, "not_configured", "OIDC is not configured") - return - } - if app.sessions == nil { - writeError(w, http.StatusInternalServerError, "no_session_codec", "session support is not configured") - return - } - - var body struct { - Code string `json:"code"` - CodeVerifier string `json:"codeVerifier"` - RedirectURI string `json:"redirectUri"` - } - // decodeBody bounds the body (MaxBytesReader) and rejects unknown fields — - // this is a public, unauthenticated endpoint. - if !decodeBody(w, r, &body) { - return - } - if body.Code == "" { - writeError(w, http.StatusBadRequest, "invalid_request", "code is required") - return - } - - tokenEndpoint, _, err := discoverOIDCEndpoints(app.authConfig.Issuer) - if err != nil { - slog.Warn("OIDC discovery failed", "error", err) - writeError(w, http.StatusBadGateway, "discovery_failed", "identity provider is unreachable") - return - } - - form := url.Values{ - "grant_type": {"authorization_code"}, - "client_id": {app.authConfig.ClientID}, - "code": {body.Code}, - "redirect_uri": {body.RedirectURI}, - "code_verifier": {body.CodeVerifier}, - } - if app.oidcClientSecret != "" { - form.Set("client_secret", app.oidcClientSecret) - } - tokenResp, err := oidcHTTPClient.PostForm(tokenEndpoint, form) - if err != nil { - writeError(w, http.StatusBadGateway, "token_exchange_failed", "identity provider is unreachable") - return - } - defer tokenResp.Body.Close() - - if tokenResp.StatusCode != http.StatusOK { - writeOAuthError(w, tokenResp) - return - } - - var tokens tokenResponse - if err := json.NewDecoder(tokenResp.Body).Decode(&tokens); err != nil { - writeError(w, http.StatusBadGateway, "token_parse_failed", "failed to parse token response") - return - } - if tokens.AccessToken == "" && tokens.IDToken == "" { - writeError(w, http.StatusBadGateway, "token_exchange_failed", "no token received from identity provider") - return - } - - if err := app.sessions.SetSession(w, tokens.session()); err != nil { - writeError(w, http.StatusInternalServerError, "session_failed", err.Error()) - return - } - - writeJSON(w, http.StatusOK, map[string]bool{"authenticated": true}) -} - -// refreshSession exchanges a refresh token for new tokens server-side. Called -// by the session manager when a session's bearer has expired. -func (app *App) refreshSession(refreshToken string) (*auth.Session, error) { - if app.authConfig.Issuer == "" || app.authConfig.ClientID == "" { - return nil, fmt.Errorf("OIDC is not configured") - } - - tokenEndpoint, _, err := discoverOIDCEndpoints(app.authConfig.Issuer) - if err != nil { - return nil, err - } - - form := url.Values{ - "grant_type": {"refresh_token"}, - "client_id": {app.authConfig.ClientID}, - "refresh_token": {refreshToken}, - } - if app.oidcClientSecret != "" { - form.Set("client_secret", app.oidcClientSecret) - } - tokenResp, err := oidcHTTPClient.PostForm(tokenEndpoint, form) - if err != nil { - return nil, fmt.Errorf("identity provider is unreachable: %w", err) - } - defer tokenResp.Body.Close() - - if tokenResp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("identity provider rejected the refresh (status %d)", tokenResp.StatusCode) - } - - var tokens tokenResponse - if err := json.NewDecoder(tokenResp.Body).Decode(&tokens); err != nil { - return nil, fmt.Errorf("failed to parse token response: %w", err) - } - if tokens.AccessToken == "" && tokens.IDToken == "" { - return nil, fmt.Errorf("no token received from identity provider") - } - - session := tokens.session() - if session.RefreshToken == "" { - // IdP did not rotate the refresh token; keep using the old one. - session.RefreshToken = refreshToken - } - return session, nil -} - -// requestOrigin returns the BFF's own scheme://host for this request, honoring -// the standard reverse-proxy forwarding header for the scheme. -func requestOrigin(r *http.Request) string { - scheme := "https" - if r.TLS == nil && r.Header.Get("X-Forwarded-Proto") != "https" { - if fwd := r.Header.Get("X-Forwarded-Proto"); fwd != "" { - scheme = fwd - } else { - scheme = "http" - } - } - return scheme + "://" + r.Host -} - -// GetSession reports whether the request carries a valid session. It sits -// behind the auth middleware, so reaching it at all means the request -// authenticated (header, cookie, or dev mode). The frontend uses this as its -// login probe — unlike whoami it never calls the gateway, so an unreachable -// gateway doesn't log the user out. -func (app *App) GetSession(w http.ResponseWriter, _ *http.Request) { - noStore(w) - writeJSON(w, http.StatusOK, map[string]bool{"authenticated": true}) -} - -// Logout clears the session cookie and returns the OIDC end-session URL so -// the frontend can redirect the browser to the IdP to clear the SSO session. -func (app *App) Logout(w http.ResponseWriter, r *http.Request) { - noStore(w) - // Capture the session's ID token before clearing: RP-Initiated Logout - // expects id_token_hint alongside post_logout_redirect_uri, and some OPs - // show a confirmation page or reject the redirect without it. - var idTokenHint string - if app.sessions != nil { - if session, err := app.sessions.LoadSession(r); err == nil && session != nil { - idTokenHint = session.Token - } - } - auth.ClearSession(w) - if app.authConfig.Issuer == "" { - writeJSON(w, http.StatusOK, map[string]string{"redirect": "/login"}) - return - } - - _, endSessionEndpoint, err := discoverOIDCEndpoints(app.authConfig.Issuer) - if err != nil || endSessionEndpoint == "" { - writeJSON(w, http.StatusOK, map[string]string{"redirect": "/login"}) - return - } - - // Build an absolute post-logout URI from the BFF's own origin. OPs - // validate this against registered values and often reject relative URIs, - // so it must be absolute — and it must not come from a client-supplied - // header (Referer), which an attacker could steer. - params := url.Values{ - "client_id": {app.authConfig.ClientID}, - "post_logout_redirect_uri": {requestOrigin(r) + "/login"}, - } - if idTokenHint != "" { - params.Set("id_token_hint", idTokenHint) - } - - writeJSON(w, http.StatusOK, map[string]string{ - "redirect": endSessionEndpoint + "?" + params.Encode(), - }) -} diff --git a/backend/internal/api/oidc_handler_test.go b/backend/internal/api/oidc_handler_test.go deleted file mode 100644 index fa99669..0000000 --- a/backend/internal/api/oidc_handler_test.go +++ /dev/null @@ -1,517 +0,0 @@ -package api - -import ( - "encoding/base64" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "testing" - "time" - - "github.com/Gkrumbach07/openshell-dashboard/backend/internal/auth" -) - -func newTestSessionCodec(t *testing.T) *auth.SessionCodec { - t.Helper() - codec, err := auth.NewSessionCodec([]byte("test-secret")) - if err != nil { - t.Fatalf("NewSessionCodec: %v", err) - } - return codec -} - -// newFakeIssuer serves an OIDC discovery document and token endpoint. -func newFakeIssuer(t *testing.T, tokenJSON string) *httptest.Server { - t.Helper() - var issuer *httptest.Server - issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/openid-configuration": - _, _ = fmt.Fprintf(w, `{"token_endpoint":%q,"end_session_endpoint":%q}`, - issuer.URL+"/token", issuer.URL+"/logout") - case "/token": - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(tokenJSON)) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(issuer.Close) - return issuer -} - -func findCookie(cookies []*http.Cookie, name string) *http.Cookie { - for _, cookie := range cookies { - if cookie.Name == name { - return cookie - } - } - return nil -} - -func TestTokenExchangeSetsSessionCookie(t *testing.T) { - issuer := newFakeIssuer(t, `{"id_token":"signed-id-token","refresh_token":"refresh-token","expires_in":300}`) - codec := newTestSessionCodec(t) - app := &App{ - sessions: codec, - authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}, - } - - body := `{"code":"code","codeVerifier":"verifier","redirectUri":"https://dashboard.example.com/auth/callback"}` - req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) - w := httptest.NewRecorder() - app.TokenExchange(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) - } - if strings.Contains(w.Body.String(), "signed-id-token") { - t.Fatal("token leaked into the response body — tokens must live only in the cookie") - } - - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil { - t.Fatal("no session cookie set") - } - readReq := httptest.NewRequest(http.MethodGet, "/", nil) - readReq.AddCookie(cookie) - session, err := codec.LoadSession(readReq) - if err != nil || session == nil { - t.Fatalf("LoadSession: session=%v err=%v", session, err) - } - if session.Token != "signed-id-token" || session.RefreshToken != "refresh-token" { - t.Fatalf("session = %+v, want id token + refresh token", session) - } - if session.ExpiresAt < time.Now().Unix() { - t.Fatalf("session.ExpiresAt = %d, want future", session.ExpiresAt) - } -} - -// makeJWT builds an unsigned JWT with the given exp claim, for expiry parsing. -func makeJWT(exp int64) string { - header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none"}`)) - payload := base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, `{"exp":%d}`, exp)) - return header + "." + payload + ".sig" -} - -func TestSessionExpiryPrefersIDTokenExp(t *testing.T) { - // ID token expires in 60s; expires_in claims 3600 (the access token's). - // The session must track the ID token (the forwarded bearer), not 3600. - idExp := time.Now().Add(60 * time.Second).Unix() - tr := &tokenResponse{IDToken: makeJWT(idExp), ExpiresIn: "3600"} - s := tr.session() - if s.ExpiresAt != idExp { - t.Fatalf("ExpiresAt = %d, want ID token exp %d (not access-token expires_in)", s.ExpiresAt, idExp) - } -} - -func TestSessionExpiryTakesEarlierOfIDExpAndExpiresIn(t *testing.T) { - // ID token valid for an hour, but expires_in says the access token dies in - // 30s. When the ID token is the bearer we key off its exp; verify we never - // overrun a shorter access-token window either. - idExp := time.Now().Add(1 * time.Hour).Unix() - tr := &tokenResponse{IDToken: makeJWT(idExp), ExpiresIn: "30"} - s := tr.session() - accessExpiry := time.Now().Add(30 * time.Second).Unix() - if s.ExpiresAt > accessExpiry+2 { - t.Fatalf("ExpiresAt = %d, want <= access expiry %d", s.ExpiresAt, accessExpiry) - } -} - -func TestSessionExpiryToleratesStringExpiresIn(t *testing.T) { - // Azure AD v1.0 renders expires_in as a JSON string; the decode must not - // choke. Round-trip through JSON to exercise the json.Number path. - var tr tokenResponse - if err := json.Unmarshal([]byte(`{"access_token":"opaque","expires_in":"3599"}`), &tr); err != nil { - t.Fatalf("decode with string expires_in: %v", err) - } - s := tr.session() - if s.ExpiresAt == 0 { - t.Fatal("ExpiresAt = 0, want it set from a string expires_in") - } -} - -func TestTokenExchangeSurfacesOAuthError(t *testing.T) { - var issuer *httptest.Server - issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/openid-configuration": - _, _ = fmt.Fprintf(w, `{"token_endpoint":%q}`, issuer.URL+"/token") - case "/token": - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"error":"invalid_grant","error_description":"code expired"}`)) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(issuer.Close) - - app := &App{sessions: newTestSessionCodec(t), authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - body := `{"code":"stale","codeVerifier":"v","redirectUri":"https://d/cb"}` - w := httptest.NewRecorder() - app.TokenExchange(w, httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))) - - if w.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want 400 (not a 401 login loop)", w.Code) - } - if !strings.Contains(w.Body.String(), "invalid_grant") || !strings.Contains(w.Body.String(), "code expired") { - t.Fatalf("body = %s, want the OAuth error surfaced", w.Body.String()) - } -} - -func TestRefreshSessionKeepsOldRefreshToken(t *testing.T) { - // IdP that does not rotate the refresh token on refresh. - issuer := newFakeIssuer(t, `{"id_token":"new-id-token","expires_in":300}`) - app := &App{authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - - session, err := app.refreshSession("old-refresh-token") - if err != nil { - t.Fatalf("refreshSession: %v", err) - } - if session.Token != "new-id-token" { - t.Fatalf("token = %q, want new-id-token", session.Token) - } - if session.RefreshToken != "old-refresh-token" { - t.Fatalf("refresh token = %q, want the original preserved", session.RefreshToken) - } -} - -func TestSessionManagerRefreshesExpiredSession(t *testing.T) { - issuer := newFakeIssuer(t, `{"id_token":"refreshed-id-token","refresh_token":"rotated-refresh","expires_in":300}`) - codec := newTestSessionCodec(t) - app := &App{ - sessions: codec, - authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}, - } - sm := &sessionManager{codec: codec, app: app} - - // Seed a request with an expired session. - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{ - Token: "stale-token", - RefreshToken: "old-refresh", - ExpiresAt: time.Now().Unix() - 60, - }); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - - w := httptest.NewRecorder() - token := sm.TokenFromSession(w, req) - - if token != "refreshed-id-token" { - t.Fatalf("token = %q, want refreshed-id-token", token) - } - // The refreshed session must be re-set on the response. - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil { - t.Fatal("refreshed session cookie not set on response") - } - readReq := httptest.NewRequest(http.MethodGet, "/", nil) - readReq.AddCookie(cookie) - session, err := codec.LoadSession(readReq) - if err != nil || session == nil { - t.Fatalf("LoadSession after refresh: session=%v err=%v", session, err) - } - if session.RefreshToken != "rotated-refresh" { - t.Fatalf("refresh token = %q, want rotated-refresh", session.RefreshToken) - } -} - -func TestSessionManagerParallelRequestsShareOneRefresh(t *testing.T) { - // An IdP with single-use refresh tokens: the second redemption of the - // same token fails, as Dex does by default. - var tokenCalls int - var issuer *httptest.Server - issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/openid-configuration": - _, _ = fmt.Fprintf(w, `{"token_endpoint":%q}`, issuer.URL+"/token") - case "/token": - tokenCalls++ - if tokenCalls > 1 { - http.Error(w, `{"error":"invalid_grant"}`, http.StatusBadRequest) - return - } - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"id_token":"refreshed-id-token","refresh_token":"rotated-refresh","expires_in":300}`)) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(issuer.Close) - - codec := newTestSessionCodec(t) - app := &App{ - sessions: codec, - authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}, - } - sm := &sessionManager{codec: codec, app: app} - - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{ - Token: "stale-token", - RefreshToken: "old-refresh", - ExpiresAt: time.Now().Unix() - 60, - }); err != nil { - t.Fatalf("SetSession: %v", err) - } - - // Two requests from the same page load, both carrying the old cookie. - for i := 0; i < 2; i++ { - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - w := httptest.NewRecorder() - if token := sm.TokenFromSession(w, req); token != "refreshed-id-token" { - t.Fatalf("request %d: token = %q, want refreshed-id-token", i+1, token) - } - } - if tokenCalls != 1 { - t.Fatalf("IdP token endpoint called %d times, want 1 (single-flight)", tokenCalls) - } -} - -func TestSessionManagerEnforcesAbsoluteLifetime(t *testing.T) { - codec := newTestSessionCodec(t) - // Refresh would succeed, but the session is past its absolute ceiling. - issuer := newFakeIssuer(t, `{"id_token":"refreshed","refresh_token":"r","expires_in":300}`) - app := &App{sessions: codec, authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - sm := &sessionManager{codec: codec, app: app} - - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{ - Token: "stale", - RefreshToken: "old-refresh", - ExpiresAt: time.Now().Unix() - 60, - CreatedAt: time.Now().Add(-13 * time.Hour).Unix(), - }); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - - w := httptest.NewRecorder() - if token := sm.TokenFromSession(w, req); token != "" { - t.Fatalf("token = %q, want empty — session past absolute lifetime must end", token) - } - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil || cookie.MaxAge != -1 { - t.Fatalf("over-age session cookie not cleared: %#v", cookie) - } -} - -func TestSessionManagerRefreshPreservesCreatedAt(t *testing.T) { - issuer := newFakeIssuer(t, `{"id_token":"refreshed-id-token","refresh_token":"rotated","expires_in":300}`) - codec := newTestSessionCodec(t) - app := &App{sessions: codec, authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - sm := &sessionManager{codec: codec, app: app} - - created := time.Now().Add(-2 * time.Hour).Unix() - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{ - Token: "stale", - RefreshToken: "old-refresh", - ExpiresAt: time.Now().Unix() - 60, - CreatedAt: created, - }); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - - w := httptest.NewRecorder() - sm.TokenFromSession(w, req) - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil { - t.Fatal("no refreshed cookie") - } - readReq := httptest.NewRequest(http.MethodGet, "/", nil) - readReq.AddCookie(cookie) - session, err := codec.LoadSession(readReq) - if err != nil || session == nil { - t.Fatalf("LoadSession: %v", err) - } - if session.CreatedAt != created { - t.Fatalf("CreatedAt = %d, want %d preserved across refresh", session.CreatedAt, created) - } -} - -func TestSessionManagerExpiredWithoutRefreshEndsSession(t *testing.T) { - codec := newTestSessionCodec(t) - sm := &sessionManager{codec: codec, app: &App{sessions: codec}} - - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{ - Token: "stale-token", - ExpiresAt: time.Now().Unix() - 60, - }); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - - w := httptest.NewRecorder() - if token := sm.TokenFromSession(w, req); token != "" { - t.Fatalf("token = %q, want empty — an unrenewable expired session must end", token) - } - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil || cookie.MaxAge != -1 { - t.Fatalf("expired session cookie not cleared: %#v", cookie) - } -} - -func TestSessionManagerGarbageCookieCleared(t *testing.T) { - codec := newTestSessionCodec(t) - sm := &sessionManager{codec: codec, app: &App{sessions: codec}} - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.AddCookie(&http.Cookie{Name: auth.SessionCookieName, Value: "not-a-session"}) - w := httptest.NewRecorder() - - if token := sm.TokenFromSession(w, req); token != "" { - t.Fatalf("token = %q, want empty for garbage cookie", token) - } - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil || cookie.MaxAge != -1 { - t.Fatalf("garbage session cookie not cleared: %#v", cookie) - } -} - -func TestLogoutClearsSessionCookie(t *testing.T) { - app := &App{} - w := httptest.NewRecorder() - app.Logout(w, httptest.NewRequest(http.MethodPost, "/api/v1/auth/logout", nil)) - - cookie := findCookie(w.Result().Cookies(), auth.SessionCookieName) - if cookie == nil || cookie.MaxAge != -1 { - t.Fatalf("clear session cookie = %#v, want MaxAge -1", cookie) - } -} - -func TestLogoutReturnsEndSessionRedirect(t *testing.T) { - issuer := newFakeIssuer(t, `{}`) - app := &App{authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - - w := httptest.NewRecorder() - app.Logout(w, httptest.NewRequest(http.MethodPost, "/api/v1/auth/logout", nil)) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", w.Code) - } - if !strings.Contains(w.Body.String(), issuer.URL+"/logout") { - t.Fatalf("body = %s, want end-session URL", w.Body.String()) - } -} - -func TestLogoutRedirectUsesRequestOriginNotReferer(t *testing.T) { - issuer := newFakeIssuer(t, `{}`) - app := &App{authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}} - - req := httptest.NewRequest(http.MethodPost, "https://dashboard.example.com/api/v1/auth/logout", nil) - req.Host = "dashboard.example.com" - req.Header.Set("Referer", "https://evil.example.com/attack") - w := httptest.NewRecorder() - app.Logout(w, req) - - body := w.Body.String() - if strings.Contains(body, "evil.example.com") { - t.Fatalf("logout redirect honored attacker Referer: %s", body) - } - if !strings.Contains(body, url.QueryEscape("https://dashboard.example.com/login")) { - t.Fatalf("logout redirect should point at the request origin's /login: %s", body) - } -} - -func TestLogoutIncludesIDTokenHint(t *testing.T) { - issuer := newFakeIssuer(t, `{}`) - codec := newTestSessionCodec(t) - app := &App{ - sessions: codec, - authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}, - } - - seed := httptest.NewRecorder() - if err := codec.SetSession(seed, &auth.Session{Token: "the-id-token"}); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/logout", nil) - for _, cookie := range seed.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - - w := httptest.NewRecorder() - app.Logout(w, req) - - if !strings.Contains(w.Body.String(), "id_token_hint=the-id-token") { - t.Fatalf("body = %s, want id_token_hint per RP-Initiated Logout", w.Body.String()) - } -} - -func TestTokenEndpointCallsIncludeClientSecret(t *testing.T) { - var gotSecret string - var issuer *httptest.Server - issuer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.well-known/openid-configuration": - _, _ = fmt.Fprintf(w, `{"token_endpoint":%q}`, issuer.URL+"/token") - case "/token": - _ = r.ParseForm() - gotSecret = r.PostFormValue("client_secret") - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"id_token":"signed-id-token","expires_in":300}`)) - default: - http.NotFound(w, r) - } - })) - t.Cleanup(issuer.Close) - - app := &App{ - sessions: newTestSessionCodec(t), - authConfig: AuthConfigResponse{Issuer: issuer.URL, ClientID: "dashboard"}, - oidcClientSecret: "confidential-secret", - } - - body := `{"code":"code","codeVerifier":"verifier","redirectUri":"https://dashboard.example.com/auth/callback"}` - w := httptest.NewRecorder() - app.TokenExchange(w, httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))) - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) - } - if gotSecret != "confidential-secret" { - t.Fatalf("token exchange client_secret = %q, want confidential-secret", gotSecret) - } - - gotSecret = "" - if _, err := app.refreshSession("refresh-token"); err != nil { - t.Fatalf("refreshSession: %v", err) - } - if gotSecret != "confidential-secret" { - t.Fatalf("refresh client_secret = %q, want confidential-secret", gotSecret) - } -} diff --git a/backend/internal/api/respond.go b/backend/internal/api/respond.go index 70f5e47..d079260 100644 --- a/backend/internal/api/respond.go +++ b/backend/internal/api/respond.go @@ -24,13 +24,6 @@ func writeJSON(w http.ResponseWriter, statusCode int, payload any) { } } -// noStore marks a response as uncacheable — used on auth endpoints, whose -// bodies and Set-Cookie headers must never be stored by a proxy or the -// browser cache. -func noStore(w http.ResponseWriter) { - w.Header().Set("Cache-Control", "no-store") -} - func writeError(w http.ResponseWriter, statusCode int, code, message string) { writeJSON(w, statusCode, ErrorResponse{Code: code, Message: message}) } diff --git a/backend/internal/api/session_manager.go b/backend/internal/api/session_manager.go deleted file mode 100644 index 7d44042..0000000 --- a/backend/internal/api/session_manager.go +++ /dev/null @@ -1,113 +0,0 @@ -package api - -import ( - "log/slog" - "net/http" - "sync" - "time" - - "github.com/Gkrumbach07/openshell-dashboard/backend/internal/auth" -) - -const ( - // refreshSkew renews sessions slightly before the bearer actually expires - // so in-flight requests don't race the deadline. - refreshSkew = 30 * time.Second - // maxSessionLifetime caps how long a session may be renewed before the - // user must re-authenticate, regardless of the IdP refresh token's own - // lifetime. Bounds the blast radius of a captured cookie. - maxSessionLifetime = 12 * time.Hour - // refreshReuseWindow is how long a completed refresh answers for other - // requests that arrived carrying the same (now-invalidated) refresh token. - // A page load fires many parallel requests with the same expired cookie; - // only the first may hit the IdP when refresh tokens are single-use. Kept - // short: it must outlast a burst of concurrent requests but not linger as - // a window where a replayed stale cookie is honored (which would blunt the - // IdP's refresh-token replay detection). - refreshReuseWindow = 10 * time.Second -) - -// sessionManager implements auth.SessionAuthenticator: it opens the encrypted -// session cookie and, when the bearer inside is expired, refreshes it against -// the IdP server-side and re-sets the cookie. The browser never sees a token. -type sessionManager struct { //nolint:govet // fieldalignment: readability over padding - codec *auth.SessionCodec - app *App - - // refreshMu serializes refreshes and guards the single-flight state - // below. IdPs commonly rotate refresh tokens on use (Dex does by - // default), so of N parallel requests carrying the same expired cookie, - // only the first can redeem the refresh token — the rest must reuse its - // result rather than fail against an already-invalidated token. - refreshMu sync.Mutex - lastRefreshedRT string - lastResult *auth.Session - lastRefreshedAt time.Time -} - -// TokenFromSession returns the session's bearer, refreshing first if expired. -// Returns "" when there is no session or the refresh fails — the caller then -// rejects the request with 401 and the frontend redirects to login. -func (sm *sessionManager) TokenFromSession(w http.ResponseWriter, r *http.Request) string { - session, err := sm.codec.LoadSession(r) - if err != nil { - slog.Debug("session cookie rejected", "error", err) - auth.ClearSession(w) - return "" - } - if session == nil { - return "" - } - // Absolute lifetime cap: a session past its ceiling ends even if the IdP - // would still refresh it. CreatedAt is preserved across refreshes. - if session.CreatedAt > 0 && time.Since(time.Unix(session.CreatedAt, 0)) > maxSessionLifetime { - auth.ClearSession(w) - return "" - } - if !session.Expired(refreshSkew) { - return session.Token - } - if session.RefreshToken == "" { - // Expired with no way to renew: end the session. Passing the stale - // token through would have the gateway 401 every request while the - // session probe keeps reporting "logged in" — a redirect loop. - auth.ClearSession(w) - return "" - } - - sm.refreshMu.Lock() - defer sm.refreshMu.Unlock() - - refreshed := sm.recentRefreshLocked(session.RefreshToken) - if refreshed == nil { - refreshed, err = sm.app.refreshSession(session.RefreshToken) - if err != nil { - slog.Warn("server-side session refresh failed", "error", err) - auth.ClearSession(w) - return "" - } - // Preserve the original session start so the absolute lifetime cap - // counts from first login, not from the latest refresh. - refreshed.CreatedAt = session.CreatedAt - sm.lastRefreshedRT = session.RefreshToken - sm.lastResult = refreshed - sm.lastRefreshedAt = time.Now() - } - - if err := sm.codec.SetSession(w, refreshed); err != nil { - slog.Warn("failed to re-set session cookie after refresh", "error", err) - } - return refreshed.Token -} - -// recentRefreshLocked returns the result of a just-completed refresh of the -// same refresh token, or nil. Caller must hold refreshMu. -func (sm *sessionManager) recentRefreshLocked(refreshToken string) *auth.Session { - if sm.lastResult == nil || sm.lastRefreshedRT != refreshToken { - return nil - } - if time.Since(sm.lastRefreshedAt) > refreshReuseWindow { - return nil - } - return sm.lastResult -} diff --git a/backend/internal/api/terminal_handler.go b/backend/internal/api/terminal_handler.go index 4fdcee9..db32d26 100644 --- a/backend/internal/api/terminal_handler.go +++ b/backend/internal/api/terminal_handler.go @@ -72,20 +72,10 @@ func (app *App) Terminal(w http.ResponseWriter, r *http.Request) { cols, rows := parseDimensions(r) upgrader := websocket.Upgrader{ - CheckOrigin: app.checkWebSocketOrigin, + CheckOrigin: checkWebSocketOrigin, } - // The 101 response is built from scratch by the upgrader and ignores - // w.Header(). If the auth middleware refreshed the session on this - // request, its Set-Cookie is on w.Header() and would be silently dropped — - // leaving the browser holding a stale (rotated-away) refresh token. Carry - // those cookies onto the handshake response so the refresh actually lands. - var upgradeHeader http.Header - if cookies := w.Header().Values("Set-Cookie"); len(cookies) > 0 { - upgradeHeader = http.Header{"Set-Cookie": cookies} - } - - ws, err := upgrader.Upgrade(w, r, upgradeHeader) + ws, err := upgrader.Upgrade(w, r, nil) if err != nil { slog.Error("websocket upgrade failed", "error", err) return @@ -154,25 +144,17 @@ func (app *App) Terminal(w http.ResponseWriter, r *http.Request) { } } -// checkWebSocketOrigin validates the Origin header against the configured -// allowed origins. When no origins are configured (e.g. same-origin -// deployment behind a proxy), it falls back to same-origin matching. -func (app *App) checkWebSocketOrigin(r *http.Request) bool { +// checkWebSocketOrigin enforces same-origin on browser WebSocket handshakes, +// as defense-in-depth against cross-site WebSocket hijacking. The BFF is +// same-origin-only by design (ADR 0014): browsers reach it via its own origin +// or through a fronting proxy on that origin — there is no cross-origin +// consumer to allow for. +func checkWebSocketOrigin(r *http.Request) bool { origin := r.Header.Get("Origin") if origin == "" { // Browsers always send Origin on a WebSocket handshake; a missing one - // is a non-browser client that carries no victim's cookies. - return true - } - // Same-origin is always allowed — independent of ALLOWED_ORIGINS, which - // exists to permit *additional* cross-origin callers (federated embedding). - if origin == "http://"+r.Host || origin == "https://"+r.Host { + // is a non-browser client that carries no victim's ambient credentials. return true } - for _, allowed := range app.allowedOrigins { - if origin == allowed { - return true - } - } - return false + return origin == "http://"+r.Host || origin == "https://"+r.Host } diff --git a/backend/internal/api/terminal_handler_test.go b/backend/internal/api/terminal_handler_test.go index efa69c3..8df7961 100644 --- a/backend/internal/api/terminal_handler_test.go +++ b/backend/internal/api/terminal_handler_test.go @@ -7,72 +7,45 @@ import ( func TestCheckWebSocketOrigin(t *testing.T) { tests := []struct { //nolint:govet // fieldalignment: test readability - name string - allowedOrigins []string - origin string - host string - want bool + name string + origin string + host string + want bool }{ { - name: "allowed origin matches", - allowedOrigins: []string{"https://dashboard.example.com"}, - origin: "https://dashboard.example.com", - want: true, + name: "empty origin allowed (non-browser client)", + origin: "", + host: "dashboard.example.com", + want: true, }, { - name: "disallowed origin rejected", - allowedOrigins: []string{"https://dashboard.example.com"}, - origin: "https://evil.com", - want: false, + name: "same-origin http match", + origin: "http://localhost:8080", + host: "localhost:8080", + want: true, }, { - name: "empty origin allowed", - allowedOrigins: []string{"https://dashboard.example.com"}, - origin: "", - want: true, + name: "same-origin https match", + origin: "https://dashboard.example.com", + host: "dashboard.example.com", + want: true, }, { - name: "no allowed origins same-origin http match", - allowedOrigins: nil, - origin: "http://localhost:8080", - host: "localhost:8080", - want: true, + name: "cross-origin rejected", + origin: "https://evil.com", + host: "dashboard.example.com", + want: false, }, { - name: "no allowed origins same-origin https match", - allowedOrigins: nil, - origin: "https://dashboard.example.com", - host: "dashboard.example.com", - want: true, - }, - { - name: "no allowed origins cross-origin rejected", - allowedOrigins: nil, - origin: "https://evil.com", - host: "dashboard.example.com", - want: false, - }, - { - name: "multiple allowed origins second matches", - allowedOrigins: []string{"https://a.com", "https://b.com"}, - origin: "https://b.com", - want: true, - }, - { - // Regression: same-origin must be allowed even when an allowlist - // is configured (the allowlist adds cross-origin callers, it does - // not replace the same-origin default). - name: "same-origin allowed despite non-empty allowlist", - allowedOrigins: []string{"https://other.example.com"}, - origin: "https://dashboard.example.com", - host: "dashboard.example.com", - want: true, + name: "subdomain rejected", + origin: "https://evil.dashboard.example.com", + host: "dashboard.example.com", + want: false, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - app := &App{allowedOrigins: tc.allowedOrigins} req, _ := http.NewRequest(http.MethodGet, "/terminal", nil) if tc.origin != "" { req.Header.Set("Origin", tc.origin) @@ -80,7 +53,7 @@ func TestCheckWebSocketOrigin(t *testing.T) { if tc.host != "" { req.Host = tc.host } - got := app.checkWebSocketOrigin(req) + got := checkWebSocketOrigin(req) if got != tc.want { t.Errorf("checkWebSocketOrigin() = %v, want %v", got, tc.want) } diff --git a/backend/internal/auth/proxy.go b/backend/internal/auth/proxy.go index dbf4ab8..2524dfe 100644 --- a/backend/internal/auth/proxy.go +++ b/backend/internal/auth/proxy.go @@ -3,13 +3,14 @@ // The middleware only decides where the bearer for a request comes from, in // precedence order: // -// 1. The auth-proxy header (federated mode: oauth2-proxy / kube-auth-proxy -// injects `x-forwarded-access-token`). +// 1. The auth-proxy header (oauth2-proxy / kube-auth-proxy injects +// `x-forwarded-access-token`). // 2. An explicit `Authorization: Bearer` header (API clients, tests). -// 3. The encrypted session cookie (standalone OIDC mode; see session.go). // -// Cookie sessions cover WebSocket upgrades too — browsers cannot attach an -// Authorization header to a WebSocket handshake, but they do send cookies. +// There is deliberately no third source: the BFF holds no sessions and runs +// no OIDC flows (ADR 0014). Deployments that need browser login put an auth +// proxy in front; the proxy authenticates WebSocket upgrades too, since it +// injects the token header on the upgrade request like any other. package auth import ( @@ -26,32 +27,17 @@ const ( userContextKey ) -// SessionAuthenticator resolves a bearer token from a request's session -// cookie, transparently refreshing (and re-setting the cookie) when the -// session is expired. Implemented by the api package, which owns the OIDC -// client configuration. Returns "" when the request carries no usable session. -type SessionAuthenticator interface { - TokenFromSession(w http.ResponseWriter, r *http.Request) string -} - // Config holds auth middleware settings. type Config struct { TokenHeader string UserHeader string Disabled bool - // TrustProxyHeader enables reading the bearer from TokenHeader - // (x-forwarded-access-token). Only safe in federated mode, where an auth - // proxy in front of the BFF sets and sanitizes that header. In standalone - // mode there is no such proxy, so a client could forge the header to - // bypass the session cookie — leave this false there. - TrustProxyHeader bool } // Middleware extracts the request's bearer token and stores it on the // request context for the gateway client to forward. -type Middleware struct { //nolint:govet // fieldalignment: readability over padding - cfg Config - session SessionAuthenticator +type Middleware struct { + cfg Config } // New builds the middleware. @@ -65,12 +51,6 @@ func New(cfg Config) *Middleware { return &Middleware{cfg: cfg} } -// SetSessionAuthenticator enables cookie-session authentication (standalone -// OIDC mode). Optional; without it only header-based auth is accepted. -func (m *Middleware) SetSessionAuthenticator(sa SessionAuthenticator) { - m.session = sa -} - // Disabled reports whether auth validation is turned off. func (m *Middleware) Disabled() bool { return m.cfg.Disabled @@ -81,10 +61,10 @@ func (m *Middleware) TokenHeader() string { return m.cfg.TokenHeader } -// Handler resolves the request's bearer token (header, then cookie session) -// and stores it on the request context. When auth is disabled, a synthetic -// dev-user identity is injected and any tokens on the request are ignored, so -// a misconfigured proxy cannot leak credentials to the gateway in dev mode. +// Handler resolves the request's bearer token and stores it on the request +// context. When auth is disabled, a synthetic dev-user identity is injected +// and any tokens on the request are ignored, so a misconfigured proxy cannot +// leak credentials to the gateway in dev mode. func (m *Middleware) Handler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if m.cfg.Disabled { @@ -93,18 +73,12 @@ func (m *Middleware) Handler(next http.Handler) http.Handler { return } - var token string - if m.cfg.TrustProxyHeader { - token = r.Header.Get(m.cfg.TokenHeader) - } + token := r.Header.Get(m.cfg.TokenHeader) if token == "" { if bearer := r.Header.Get("Authorization"); strings.HasPrefix(bearer, "Bearer ") { token = strings.TrimPrefix(bearer, "Bearer ") } } - if token == "" && m.session != nil { - token = m.session.TokenFromSession(w, r) - } if token == "" { writeUnauthorized(w, "not authenticated") return diff --git a/backend/internal/auth/proxy_test.go b/backend/internal/auth/proxy_test.go index aa28127..e42e580 100644 --- a/backend/internal/auth/proxy_test.go +++ b/backend/internal/auth/proxy_test.go @@ -6,11 +6,10 @@ import ( "testing" ) -func TestHandler_Federated_TrustsProxyHeader(t *testing.T) { +func TestHandler_ProxyHeader(t *testing.T) { m := New(Config{ - TokenHeader: "x-forwarded-access-token", - UserHeader: "x-auth-request-user", - TrustProxyHeader: true, + TokenHeader: "x-forwarded-access-token", + UserHeader: "x-auth-request-user", }) var gotToken, gotUser string @@ -37,29 +36,7 @@ func TestHandler_Federated_TrustsProxyHeader(t *testing.T) { } } -func TestHandler_Standalone_IgnoresProxyHeader(t *testing.T) { - // No TrustProxyHeader: standalone mode. A forged x-forwarded-access-token - // must not be honored — only the cookie session (or a real Bearer) counts. - m := New(Config{TokenHeader: "x-forwarded-access-token"}) - m.SetSessionAuthenticator(&staticSessionAuth{token: "session-jwt"}) - - var gotToken string - handler := m.Handler(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotToken = TokenFromContext(r.Context()) - })) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Header.Set("x-forwarded-access-token", "forged-proxy-token") - w := httptest.NewRecorder() - - handler.ServeHTTP(w, req) - - if gotToken != "session-jwt" { - t.Fatalf("token = %q, want session-jwt — forged proxy header must be ignored in standalone mode", gotToken) - } -} - -func TestHandler_AuthEnabled_MissingToken(t *testing.T) { +func TestHandler_MissingToken(t *testing.T) { m := New(Config{}) handler := m.Handler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { @@ -76,7 +53,7 @@ func TestHandler_AuthEnabled_MissingToken(t *testing.T) { } } -func TestHandler_AuthEnabled_BearerFallback(t *testing.T) { +func TestHandler_BearerFallback(t *testing.T) { m := New(Config{}) var gotToken string @@ -95,37 +72,8 @@ func TestHandler_AuthEnabled_BearerFallback(t *testing.T) { } } -type staticSessionAuth struct{ token string } - -func (s *staticSessionAuth) TokenFromSession(http.ResponseWriter, *http.Request) string { - return s.token -} - -func TestHandler_AuthEnabled_SessionFallback(t *testing.T) { +func TestHandler_ProxyHeaderBeatsBearer(t *testing.T) { m := New(Config{}) - m.SetSessionAuthenticator(&staticSessionAuth{token: "session-jwt"}) - - var gotToken string - handler := m.Handler(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - gotToken = TokenFromContext(r.Context()) - })) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - w := httptest.NewRecorder() - - handler.ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", w.Code) - } - if gotToken != "session-jwt" { - t.Errorf("token = %q, want session-jwt", gotToken) - } -} - -func TestHandler_Federated_HeaderBeatsSession(t *testing.T) { - m := New(Config{TrustProxyHeader: true}) - m.SetSessionAuthenticator(&staticSessionAuth{token: "session-jwt"}) var gotToken string handler := m.Handler(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { @@ -134,30 +82,31 @@ func TestHandler_Federated_HeaderBeatsSession(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) req.Header.Set("x-forwarded-access-token", "proxy-jwt") + req.Header.Set("Authorization", "Bearer bearer-jwt") w := httptest.NewRecorder() handler.ServeHTTP(w, req) if gotToken != "proxy-jwt" { - t.Errorf("token = %q, want proxy-jwt (trusted proxy header takes precedence)", gotToken) + t.Errorf("token = %q, want proxy-jwt (proxy header takes precedence)", gotToken) } } -func TestHandler_AuthEnabled_EmptySessionRejected(t *testing.T) { +func TestHandler_MalformedAuthorizationRejected(t *testing.T) { m := New(Config{}) - m.SetSessionAuthenticator(&staticSessionAuth{token: ""}) handler := m.Handler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { t.Fatal("handler should not be called") })) req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Authorization", "Basic dXNlcjpwYXNz") w := httptest.NewRecorder() handler.ServeHTTP(w, req) if w.Code != http.StatusUnauthorized { - t.Fatalf("status = %d, want 401", w.Code) + t.Fatalf("status = %d, want 401 for non-Bearer Authorization", w.Code) } } diff --git a/backend/internal/auth/session.go b/backend/internal/auth/session.go deleted file mode 100644 index 22c95d6..0000000 --- a/backend/internal/auth/session.go +++ /dev/null @@ -1,191 +0,0 @@ -// Encrypted cookie sessions for standalone OIDC mode. -// -// The BFF keeps the user's OIDC tokens out of the browser entirely: after the -// PKCE code exchange, tokens are sealed into an AES-256-GCM encrypted cookie -// (the oauth2-proxy client-side session pattern). The cookie is HttpOnly, -// Secure, SameSite=Strict, and __Host- prefixed, so JavaScript can never read -// it and it rides along on every same-origin request — including WebSocket -// upgrade handshakes, which cannot carry an Authorization header. -// -// Sessions larger than one cookie (e.g. Keycloak JWTs with many groups) are -// split across numbered chunk cookies and reassembled on read. -package auth - -import ( - "crypto/aes" - "crypto/cipher" - "crypto/rand" - "crypto/sha256" - "encoding/base64" - "encoding/json" - "fmt" - "net/http" - "time" -) - -// SessionCookieName is the base name of the encrypted session cookie. Chunk -// overflow cookies append "-1", "-2", … to this name. -const SessionCookieName = "__Host-openshell-session" - -const ( - // maxCookieValueLen keeps each chunk under the 4KB per-cookie limit with - // headroom for the name and attributes (matches oauth2-proxy's budget). - maxCookieValueLen = 3800 - // maxSessionChunks caps reassembly so a hostile client cannot make the - // server concatenate unbounded cookie data. - maxSessionChunks = 8 -) - -// Session is the server-managed state sealed inside the cookie. Field names -// are compressed to keep the encrypted payload small. -type Session struct { - // Token is the bearer forwarded to the gateway (ID token, or access - // token when the IdP issued no ID token). - Token string `json:"t"` - // RefreshToken lets the BFF renew the session server-side. - RefreshToken string `json:"r,omitempty"` - // ExpiresAt is the bearer's expiry as unix seconds; 0 means unknown. - ExpiresAt int64 `json:"e,omitempty"` - // CreatedAt is when the session first began (unix seconds). Preserved - // across refreshes so an absolute lifetime cap can be enforced — - // refreshing renews the bearer but never resets this. - CreatedAt int64 `json:"c,omitempty"` -} - -// Expired reports whether the session's bearer is past (or within skew of) -// its expiry. Sessions with unknown expiry never report expired — the -// gateway remains the authority and will reject a stale token. -func (s *Session) Expired(skew time.Duration) bool { - if s.ExpiresAt == 0 { - return false - } - return time.Now().Add(skew).Unix() >= s.ExpiresAt -} - -// SessionCodec seals and opens session cookies with AES-256-GCM. -type SessionCodec struct { - aead cipher.AEAD -} - -// NewSessionCodec derives a 256-bit key from secret (any length) via SHA-256. -func NewSessionCodec(secret []byte) (*SessionCodec, error) { - if len(secret) == 0 { - return nil, fmt.Errorf("session secret must not be empty") - } - key := sha256.Sum256(secret) - block, err := aes.NewCipher(key[:]) - if err != nil { - return nil, err - } - aead, err := cipher.NewGCM(block) - if err != nil { - return nil, err - } - return &SessionCodec{aead: aead}, nil -} - -func (c *SessionCodec) seal(s *Session) (string, error) { - // G117 flags marshaling a struct with a secret-named field, but this - // plaintext is immediately sealed with AES-256-GCM below and never leaves - // the process unencrypted — that is the whole point of this function. - plaintext, err := json.Marshal(s) //nolint:gosec // G117: sealed before use - if err != nil { - return "", err - } - nonce := make([]byte, c.aead.NonceSize()) - if _, err := rand.Read(nonce); err != nil { - return "", err - } - sealed := c.aead.Seal(nonce, nonce, plaintext, nil) - return base64.RawURLEncoding.EncodeToString(sealed), nil -} - -func (c *SessionCodec) open(value string) (*Session, error) { - sealed, err := base64.RawURLEncoding.DecodeString(value) - if err != nil { - return nil, fmt.Errorf("malformed session cookie: %w", err) - } - if len(sealed) < c.aead.NonceSize() { - return nil, fmt.Errorf("malformed session cookie: too short") - } - nonce, ciphertext := sealed[:c.aead.NonceSize()], sealed[c.aead.NonceSize():] - plaintext, err := c.aead.Open(nil, nonce, ciphertext, nil) - if err != nil { - return nil, fmt.Errorf("session cookie failed decryption: %w", err) - } - var s Session - if err := json.Unmarshal(plaintext, &s); err != nil { - return nil, fmt.Errorf("malformed session payload: %w", err) - } - return &s, nil -} - -func chunkName(i int) string { - if i == 0 { - return SessionCookieName - } - return fmt.Sprintf("%s-%d", SessionCookieName, i) -} - -func newSessionCookie(name, value string, maxAge int) *http.Cookie { - cookie := &http.Cookie{ - Name: name, - Value: value, - Path: "/", - Secure: true, - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - } - if maxAge < 0 { - cookie.MaxAge = -1 - cookie.Expires = time.Unix(1, 0) - } - return cookie -} - -// SetSession seals the session into the response cookies, chunking when the -// payload exceeds a single cookie. Stale higher-numbered chunks from a -// previous, larger session are expired so reads never mix generations. -func (c *SessionCodec) SetSession(w http.ResponseWriter, s *Session) error { - value, err := c.seal(s) - if err != nil { - return err - } - chunks := (len(value) + maxCookieValueLen - 1) / maxCookieValueLen - if chunks > maxSessionChunks { - return fmt.Errorf("session too large: %d bytes across %d chunks (max %d)", len(value), chunks, maxSessionChunks) - } - for i := 0; i < chunks; i++ { - end := min((i+1)*maxCookieValueLen, len(value)) - http.SetCookie(w, newSessionCookie(chunkName(i), value[i*maxCookieValueLen:end], 0)) - } - for i := chunks; i < maxSessionChunks; i++ { - http.SetCookie(w, newSessionCookie(chunkName(i), "", -1)) - } - return nil -} - -// LoadSession reassembles and opens the session cookie from the request. -// Returns nil (no error) when no session cookie is present. -func (c *SessionCodec) LoadSession(r *http.Request) (*Session, error) { - base, err := r.Cookie(SessionCookieName) - if err != nil { - return nil, nil //nolint:nilnil // absence of a session is not an error - } - value := base.Value - for i := 1; i < maxSessionChunks; i++ { - chunk, err := r.Cookie(chunkName(i)) - if err != nil { - break - } - value += chunk.Value - } - return c.open(value) -} - -// ClearSession expires the session cookie and all possible chunks. -func ClearSession(w http.ResponseWriter) { - for i := 0; i < maxSessionChunks; i++ { - http.SetCookie(w, newSessionCookie(chunkName(i), "", -1)) - } -} diff --git a/backend/internal/auth/session_test.go b/backend/internal/auth/session_test.go deleted file mode 100644 index a77f62d..0000000 --- a/backend/internal/auth/session_test.go +++ /dev/null @@ -1,177 +0,0 @@ -package auth - -import ( - "net/http" - "net/http/httptest" - "strings" - "testing" - "time" -) - -func newTestCodec(t *testing.T) *SessionCodec { - t.Helper() - codec, err := NewSessionCodec([]byte("test-secret")) - if err != nil { - t.Fatalf("NewSessionCodec: %v", err) - } - return codec -} - -// roundTrip writes the session via SetSession and reads it back through a -// request carrying the resulting cookies, as a browser would. -func roundTrip(t *testing.T, write *SessionCodec, read *SessionCodec, s *Session) (*Session, error) { - t.Helper() - w := httptest.NewRecorder() - if err := write.SetSession(w, s); err != nil { - t.Fatalf("SetSession: %v", err) - } - req := httptest.NewRequest(http.MethodGet, "/", nil) - for _, cookie := range w.Result().Cookies() { - if cookie.MaxAge >= 0 { - req.AddCookie(cookie) - } - } - return read.LoadSession(req) -} - -func TestSessionRoundTrip(t *testing.T) { - codec := newTestCodec(t) - in := &Session{Token: "id-token", RefreshToken: "refresh", ExpiresAt: time.Now().Unix() + 300} - - out, err := roundTrip(t, codec, codec, in) - if err != nil { - t.Fatalf("LoadSession: %v", err) - } - if out.Token != in.Token || out.RefreshToken != in.RefreshToken || out.ExpiresAt != in.ExpiresAt { - t.Fatalf("session = %+v, want %+v", out, in) - } -} - -func TestSessionChunking(t *testing.T) { - codec := newTestCodec(t) - // A Keycloak-sized token with many group claims easily exceeds one cookie. - in := &Session{Token: strings.Repeat("x", 9000), RefreshToken: "refresh"} - - w := httptest.NewRecorder() - if err := codec.SetSession(w, in); err != nil { - t.Fatalf("SetSession: %v", err) - } - var live int - for _, cookie := range w.Result().Cookies() { - if cookie.MaxAge >= 0 && strings.HasPrefix(cookie.Name, SessionCookieName) { - live++ - if len(cookie.Value) > maxCookieValueLen { - t.Fatalf("chunk %s is %d bytes, want <= %d", cookie.Name, len(cookie.Value), maxCookieValueLen) - } - } - } - if live < 2 { - t.Fatalf("live chunks = %d, want >= 2 for a %d-byte token", live, len(in.Token)) - } - - out, err := roundTrip(t, codec, codec, in) - if err != nil { - t.Fatalf("LoadSession: %v", err) - } - if out.Token != in.Token { - t.Fatalf("token corrupted across chunks: got %d bytes, want %d", len(out.Token), len(in.Token)) - } -} - -func TestSessionTooLargeRejected(t *testing.T) { - codec := newTestCodec(t) - in := &Session{Token: strings.Repeat("x", maxSessionChunks*maxCookieValueLen)} - - w := httptest.NewRecorder() - if err := codec.SetSession(w, in); err == nil { - t.Fatal("SetSession accepted an oversized session, want error") - } -} - -func TestSessionWrongKeyRejected(t *testing.T) { - writeCodec := newTestCodec(t) - readCodec, err := NewSessionCodec([]byte("different-secret")) - if err != nil { - t.Fatalf("NewSessionCodec: %v", err) - } - - if _, err := roundTrip(t, writeCodec, readCodec, &Session{Token: "id-token"}); err == nil { - t.Fatal("LoadSession decrypted with the wrong key, want error") - } -} - -func TestSessionTamperRejected(t *testing.T) { - codec := newTestCodec(t) - w := httptest.NewRecorder() - if err := codec.SetSession(w, &Session{Token: "id-token"}); err != nil { - t.Fatalf("SetSession: %v", err) - } - cookie := w.Result().Cookies()[0] - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.AddCookie(&http.Cookie{Name: cookie.Name, Value: cookie.Value[:len(cookie.Value)-4] + "AAAA"}) - - if _, err := codec.LoadSession(req); err == nil { - t.Fatal("LoadSession accepted a tampered cookie, want error") - } -} - -func TestLoadSessionAbsent(t *testing.T) { - codec := newTestCodec(t) - session, err := codec.LoadSession(httptest.NewRequest(http.MethodGet, "/", nil)) - if err != nil { - t.Fatalf("LoadSession on cookieless request: %v", err) - } - if session != nil { - t.Fatalf("session = %+v, want nil", session) - } -} - -func TestClearSessionExpiresAllChunks(t *testing.T) { - w := httptest.NewRecorder() - ClearSession(w) - cookies := w.Result().Cookies() - if len(cookies) != maxSessionChunks { - t.Fatalf("cleared %d cookies, want %d", len(cookies), maxSessionChunks) - } - for _, cookie := range cookies { - if cookie.MaxAge != -1 { - t.Fatalf("cookie %s MaxAge = %d, want -1", cookie.Name, cookie.MaxAge) - } - } -} - -func TestSessionCookieAttributes(t *testing.T) { - codec := newTestCodec(t) - w := httptest.NewRecorder() - if err := codec.SetSession(w, &Session{Token: "id-token"}); err != nil { - t.Fatalf("SetSession: %v", err) - } - cookie := w.Result().Cookies()[0] - if !cookie.HttpOnly || !cookie.Secure || cookie.SameSite != http.SameSiteStrictMode || cookie.Path != "/" { - t.Fatalf("cookie attributes = %+v, want HttpOnly Secure SameSite=Strict Path=/", cookie) - } - if !strings.HasPrefix(cookie.Name, "__Host-") { - t.Fatalf("cookie name %q must carry the __Host- prefix", cookie.Name) - } -} - -func TestSessionExpired(t *testing.T) { - now := time.Now().Unix() - tests := []struct { - name string - session Session - want bool - }{ - {"no expiry never expires", Session{Token: "t"}, false}, - {"future expiry", Session{Token: "t", ExpiresAt: now + 3600}, false}, - {"past expiry", Session{Token: "t", ExpiresAt: now - 10}, true}, - {"within skew", Session{Token: "t", ExpiresAt: now + 5}, true}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if got := tc.session.Expired(30 * time.Second); got != tc.want { - t.Fatalf("Expired = %v, want %v", got, tc.want) - } - }) - } -} diff --git a/frontend/src/api/auth.ts b/frontend/src/api/auth.ts index 5cb4a82..b243eb1 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -16,33 +16,6 @@ export const useAuthConfig = () => retry: 1, }); -// useSession probes the BFF's session endpoint to decide whether the browser -// is logged in (the session cookie is HttpOnly, so JS cannot check directly). -// Unlike whoami this never touches the gateway, so a gateway outage does not -// log the user out. -// -// A 401 is a definitive "not logged in" and resolves to { authenticated: -// false } — it must NOT be conflated with a transient failure. A network -// error or 5xx throws and is retried, so a momentary BFF blip does not bounce -// an authenticated user to the login page. -export const useSession = (enabled: boolean) => - useQuery({ - queryKey: authKeys.session, - queryFn: async (): Promise<{ authenticated: boolean }> => { - const resp = await fetch('/api/v1/auth/session'); - if (resp.status === 401) { - return { authenticated: false }; - } - if (!resp.ok) { - throw new Error(`session probe failed (${resp.status})`); - } - return (await resp.json()) as { authenticated: boolean }; - }, - enabled, - retry: 2, - staleTime: STALE_5_MIN, - }); - export const getCurrentUser = (): Promise => get('/api/v1/auth/whoami'); @@ -64,9 +37,6 @@ export const useFeatureFlags = () => { credentialRefresh: true, services: true, draftPolicy: true, - deploymentContext: 'standalone', - workspaceBinding: false, - resourceLinks: false, }; return data?.features ?? defaults; }; diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 20a69ce..0d3185e 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,3 +1,5 @@ +import { isDevSession } from '../app/authStore'; + export type ApiError = Error & { status: number; code?: string; @@ -16,6 +18,7 @@ const buildError = ( let apiBasePath = ''; let onSessionExpired: (() => void) | null = null; +let reloadedFor401 = false; export const setApiBasePath = (basePath: string): void => { apiBasePath = basePath.replace(/\/+$/, ''); @@ -34,8 +37,8 @@ export const apiFetch = async ( ...((init?.headers as Record) ?? {}), }; - // Auth rides on the BFF's HttpOnly session cookie (sent automatically on - // same-origin requests) — no Authorization header, no token in JS. + // Auth is injected by the deployment's auth proxy (ADR 0014) — no + // Authorization header, no token in JS. const response = await fetch(`${apiBasePath}${path}`, { ...init, headers }); if (!response.ok) { let code: string | undefined; @@ -56,8 +59,16 @@ export const apiFetch = async ( if (response.status === 401) { if (onSessionExpired) { onSessionExpired(); - } else { + } else if (isDevSession()) { + // Dev mode registers a /login route; send the user back to it. window.location.assign('/login'); + } else if (!reloadedFor401) { + // Proxied deployments have no /login route — the auth proxy owns + // sign-in. Reload the page so the proxy can re-authenticate the + // browser (it intercepts the document request and redirects to the + // IdP). Guarded so repeated API 401s in one page life can't loop. + reloadedFor401 = true; + window.location.reload(); } throw buildError(401, code, 'Session expired'); } diff --git a/frontend/src/api/queryKeys.ts b/frontend/src/api/queryKeys.ts index cad6cf7..e693ced 100644 --- a/frontend/src/api/queryKeys.ts +++ b/frontend/src/api/queryKeys.ts @@ -49,8 +49,6 @@ export const gatewayKeys = { export const authKeys = { config: ['auth', 'config'] as const, - session: ['auth', 'session'] as const, - userInfo: ['auth', 'userinfo'] as const, whoami: ['auth', 'whoami'] as const, }; diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 39ba465..ab7d682 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -26,8 +26,7 @@ import GlobalPolicyPage from '../pages/GlobalPolicyPage'; import SettingsPage from '../pages/SettingsPage'; import { AlertProvider } from './AlertContext'; import AppLayout from './AppLayout'; -import AuthCallbackPage from './AuthCallbackPage'; -import { useAuthConfig, useSession } from '../api/auth'; +import { useAuthConfig } from '../api/auth'; import { useUserRole } from '../api/rbac'; import { isDevSession } from './authStore'; @@ -185,12 +184,6 @@ const AuthenticatedApp: React.FC = () => ( const AppRoutes: React.FC = () => { const { data: config, isLoading } = useAuthConfig(); const [devAuthenticated, setDevAuthenticated] = useState(isDevSession()); - const standalone = Boolean( - config && !config.authDisabled && config.issuer && config.clientId, - ); - // The session lives in an HttpOnly cookie, so login state is probed via the - // BFF rather than read from browser storage. - const session = useSession(standalone); if (isLoading) { return null; @@ -216,44 +209,8 @@ const AppRoutes: React.FC = () => { ); } - // Standalone OIDC mode: issuer/clientId configured, frontend handles login. - if (standalone && config) { - if (session.isLoading) { - return null; - } - // A 401 resolves to { authenticated: false }, so isSuccess alone would - // wave an unauthenticated user through — read the explicit flag. A - // post-retry transient error leaves data undefined and falls through to - // the login page (the terminal fallback when the BFF is unreachable). - const authenticated = session.data?.authenticated === true; - return ( - - } /> - - ) : ( - - ) - } - /> - - ) : ( - - ) - } - /> - - ); - } - - // Proxy-delegated mode: no issuer configured, auth proxy handles login. + // Every non-dev deployment sits behind an auth proxy (ADR 0014): the proxy + // authenticated this request before it reached us, so render directly. return ; }; diff --git a/frontend/src/app/AuthCallbackPage.tsx b/frontend/src/app/AuthCallbackPage.tsx deleted file mode 100644 index 993808f..0000000 --- a/frontend/src/app/AuthCallbackPage.tsx +++ /dev/null @@ -1,86 +0,0 @@ -import { useEffect, useState } from 'react'; -import { Alert, Bullseye, Button, Spinner } from '@patternfly/react-core'; - -import { clearLoginState, getCodeVerifier, getState } from './oidc'; - -const AuthCallbackPage: React.FC = () => { - const [error, setError] = useState(); - - useEffect(() => { - const params = new URLSearchParams(window.location.search); - - // The IdP can redirect back with an error (access_denied, consent - // required, …) instead of a code. Surface it rather than silently - // bouncing to /login, where the user would loop with no explanation. - const idpError = params.get('error'); - if (idpError) { - clearLoginState(); - setError(params.get('error_description') || idpError); - return; - } - - const code = params.get('code'); - const returnedState = params.get('state'); - const codeVerifier = getCodeVerifier(); - const expectedState = getState(); - - // Reject the callback unless the IdP echoed back the exact `state` we - // generated: this is the CSRF / code-injection guard for the redirect. - if ( - !code || - !codeVerifier || - !expectedState || - returnedState !== expectedState - ) { - clearLoginState(); - window.location.assign('/login'); - return; - } - - const exchange = async () => { - // The BFF exchanges the code server-side and sets the HttpOnly session - // cookie on this response. No tokens ever reach JavaScript. - const resp = await fetch('/api/v1/auth/token-exchange', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - code, - codeVerifier, - redirectUri: `${window.location.origin}/auth/callback`, - }), - }); - - clearLoginState(); - window.location.assign(resp.ok ? '/workspaces' : '/login'); - }; - - exchange(); - }, []); - - if (error) { - return ( - - - {error} -
- -
- - - ); - } - - return ( - - - - ); -}; - -export default AuthCallbackPage; diff --git a/frontend/src/app/__tests__/oidc.spec.ts b/frontend/src/app/__tests__/oidc.spec.ts deleted file mode 100644 index df28e04..0000000 --- a/frontend/src/app/__tests__/oidc.spec.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { clearLoginState, getCodeVerifier, getState } from '../oidc'; - -// startLogin's PKCE path uses Web Crypto (SubtleCrypto + TextEncoder), which -// jsdom does not provide; it is exercised in the live dev/e2e flow. These -// tests cover the login-state lifecycle that unit tests can assert. -describe('oidc login state', () => { - beforeEach(() => { - window.sessionStorage.clear(); - }); - - it('reads back a stored verifier and state', () => { - window.sessionStorage.setItem('oidc.code_verifier', 'verifier-value'); - window.sessionStorage.setItem('oidc.state', 'state-value'); - expect(getCodeVerifier()).toBe('verifier-value'); - expect(getState()).toBe('state-value'); - }); - - it('clearLoginState removes both verifier and state', () => { - window.sessionStorage.setItem('oidc.code_verifier', 'v'); - window.sessionStorage.setItem('oidc.state', 's'); - clearLoginState(); - expect(getCodeVerifier()).toBeNull(); - expect(getState()).toBeNull(); - }); -}); diff --git a/frontend/src/app/logout.ts b/frontend/src/app/logout.ts index f36182f..f6fc313 100644 --- a/frontend/src/app/logout.ts +++ b/frontend/src/app/logout.ts @@ -1,11 +1,9 @@ import { getAuthConfig } from '../api/auth'; import { clearDevSession } from './authStore'; -// Logout per auth mode: +// Logout per auth mode (ADR 0014): // - Dev (AUTH_DISABLED): the "session" is a client-side flag; clear it. -// - Standalone OIDC: the BFF clears the HttpOnly session cookie and returns -// the IdP's end-session URL so the SSO session is cleared too. -// - Federated: the auth proxy owns the session; redirect to its sign-out URL +// - Proxied: the auth proxy owns the session; redirect to its sign-out URL // (LOGOUT_URL, e.g. /oauth2/sign_out for oauth2-proxy). export const logout = async (): Promise => { clearDevSession(); @@ -18,14 +16,7 @@ export const logout = async (): Promise => { return; } - if (config.issuer && config.clientId) { - const resp = await fetch('/api/v1/auth/logout', { method: 'POST' }); - const body = (await resp.json()) as { redirect?: string }; - window.location.assign(body.redirect ?? '/login'); - return; - } - - window.location.assign(config.logoutUrl ?? '/oauth2/sign_out'); + window.location.assign(config.logoutUrl || '/oauth2/sign_out'); } catch { window.location.assign('/login'); } diff --git a/frontend/src/app/oidc.ts b/frontend/src/app/oidc.ts deleted file mode 100644 index fab9811..0000000 --- a/frontend/src/app/oidc.ts +++ /dev/null @@ -1,66 +0,0 @@ -// Generic OIDC Authorization Code + PKCE flow. -// Works with any standard OIDC provider (Dex, Keycloak, Okta, Entra, etc.). - -const VERIFIER_KEY = 'oidc.code_verifier'; -const STATE_KEY = 'oidc.state'; - -const randomHex = (bytes: number): string => { - const array = new Uint8Array(bytes); - crypto.getRandomValues(array); - return Array.from(array, (b) => b.toString(16).padStart(2, '0')).join(''); -}; - -const sha256 = async (plain: string): Promise => { - const encoder = new TextEncoder(); - return crypto.subtle.digest('SHA-256', encoder.encode(plain)); -}; - -const base64url = (buffer: ArrayBuffer): string => - btoa(String.fromCharCode(...new Uint8Array(buffer))) - .replace(/\+/g, '-') - .replace(/\//g, '_') - .replace(/=+$/, ''); - -export const startLogin = async ( - issuer: string, - clientId: string, - scopes: string, - redirectUri: string, -): Promise => { - const codeVerifier = randomHex(32); - const challenge = base64url(await sha256(codeVerifier)); - // `state` binds the callback to this browser: the IdP echoes it back and - // the callback rejects any mismatch, defeating login CSRF / code - // injection. Required by the OAuth browser-based-apps BCP even with PKCE. - const state = randomHex(16); - - sessionStorage.setItem(VERIFIER_KEY, codeVerifier); - sessionStorage.setItem(STATE_KEY, state); - - const params = new URLSearchParams({ - response_type: 'code', - client_id: clientId, - redirect_uri: redirectUri, - scope: scopes, - state, - code_challenge: challenge, - code_challenge_method: 'S256', - }); - - const discoveryResp = await fetch('/api/v1/auth/discovery'); - const discovery = (await discoveryResp.json()) as { - authorization_endpoint: string; - }; - - window.location.assign(`${discovery.authorization_endpoint}?${params}`); -}; - -export const getCodeVerifier = (): string | null => - sessionStorage.getItem(VERIFIER_KEY); - -export const getState = (): string | null => sessionStorage.getItem(STATE_KEY); - -export const clearLoginState = (): void => { - sessionStorage.removeItem(VERIFIER_KEY); - sessionStorage.removeItem(STATE_KEY); -}; diff --git a/frontend/src/components/sandbox/SandboxPolicyTab.tsx b/frontend/src/components/sandbox/SandboxPolicyTab.tsx deleted file mode 100644 index f041472..0000000 --- a/frontend/src/components/sandbox/SandboxPolicyTab.tsx +++ /dev/null @@ -1,203 +0,0 @@ -import { useState } from 'react'; -import { - Alert, - Bullseye, - Button, - CodeBlock, - CodeBlockCode, - Label, - Modal, - ModalBody, - ModalFooter, - ModalHeader, - Spinner, - TextArea, - Title, - Toolbar, - ToolbarContent, - ToolbarItem, -} from '@patternfly/react-core'; - -import { useSandbox } from '../../api/sandboxes'; -import { useSandboxPolicy, useUpdateSandboxPolicy } from '../../api/policy'; -import { useWorkspaceRole } from '../../api/rbac'; -import { useJsonValidation } from '../../hooks/useJsonValidation'; -import PolicyRevisionTable from '../policy/PolicyRevisionTable'; -import type { ApiError } from '../../api/client'; -import type { SandboxPolicy } from '../../types'; - -type SandboxPolicyTabProps = { - workspace: string; - sandboxName: string; -}; - -// Policy tab: current policy, revision history with load status, and an -// editor. Only network_policies (and inference fields) can change after -// create — filesystem/landlock/process are immutable, so the editor keeps -// the static fields from the current policy and replaces networkPolicies. -const SandboxPolicyTab: React.FC = ({ - workspace, - sandboxName, -}) => { - const { isWorkspaceAdmin } = useWorkspaceRole(workspace); - const policyView = useSandboxPolicy(workspace, sandboxName); - const sandbox = useSandbox(workspace, sandboxName); - const updatePolicy = useUpdateSandboxPolicy(workspace, sandboxName); - const [isEditOpen, setEditOpen] = useState(false); - const [networkText, setNetworkText] = useState(''); - - // Prefer the latest revision's policy payload; fall back to spec.policy. - const currentPolicy: SandboxPolicy | undefined = - policyView.data?.latest?.policy ?? sandbox.data?.spec.policy; - - const { error: networkError, parsed: parsedNetwork } = useJsonValidation( - networkText || '{}', - ); - - if (policyView.isLoading) { - return ( - - - - ); - } - - // A 404 just means no revisions recorded yet — fall through and render - // the create-time policy from spec.policy with an empty history. - const notFound = - policyView.isError && (policyView.error as ApiError).status === 404; - if (policyView.isError && !notFound) { - return ( - policyView.refetch()}> - Retry - - } - > - {(policyView.error as Error).message} - - ); - } - - const openEditor = () => { - setNetworkText( - JSON.stringify(currentPolicy?.networkPolicies ?? {}, null, 2), - ); - updatePolicy.reset(); - setEditOpen(true); - }; - - const submitEdit = () => { - if (networkError || !currentPolicy || !parsedNetwork) { - return; - } - const policy: SandboxPolicy = { - ...currentPolicy, - networkPolicies: parsedNetwork as SandboxPolicy['networkPolicies'], - }; - updatePolicy.mutate( - { - policy, - expectedResourceVersion: sandbox.data?.metadata.resourceVersion, - }, - { onSuccess: () => setEditOpen(false) }, - ); - }; - - return ( - <> - - - - - - {isWorkspaceAdmin && ( - - - - )} - - - - Revision history - - - {currentPolicy && ( - <> - - Current policy - - - - {JSON.stringify(currentPolicy, null, 2)} - - - - )} - - setEditOpen(false)} - aria-label="Edit network rules" - > - - -