diff --git a/.github/scripts/check_crd_compat.py b/.github/scripts/check_crd_compat.py index 6a45ad9..9d4c27c 100755 --- a/.github/scripts/check_crd_compat.py +++ b/.github/scripts/check_crd_compat.py @@ -15,6 +15,9 @@ Tightened validation (bounds, patterns, new CEL rules) is reported as a warning: Kubernetes validation ratcheting lets existing objects keep updating, but the change still belongs in the upgrade notes. + +APPROVED_REMOVALS lists one-off exceptions to the policy, each recorded in the +App CRD reference and the upgrade notes. They are reported as warnings. """ from __future__ import annotations @@ -27,6 +30,12 @@ errors: list[str] = [] warnings: list[str] = [] +# Fields removed from v1alpha1 by an explicit, documented exception (see +# "Versioning and deprecation"). Entries can go once no base branch has the field. +APPROVED_REMOVALS = { + "v1alpha1.spec.access.users", # group-only access; see Upgrading +} + def compare(path: str, old: dict[str, Any], new: dict[str, Any]) -> None: if old.get("type") != new.get("type"): @@ -56,7 +65,10 @@ def compare(path: str, old: dict[str, Any], new: dict[str, Any]) -> None: old_props, new_props = old.get("properties", {}), new.get("properties", {}) for name, schema in old_props.items(): if name not in new_props: - errors.append(f"{path}.{name}: removed") + if f"{path}.{name}" in APPROVED_REMOVALS: + warnings.append(f"{path}.{name}: removed (approved exception)") + else: + errors.append(f"{path}.{name}: removed") else: compare(f"{path}.{name}", schema, new_props[name]) newly_required = set(new.get("required", [])) - set(old.get("required", [])) diff --git a/AGENTS.md b/AGENTS.md index ddc5ef1..fb3e423 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,8 +17,8 @@ Keycloak auth. See [`docs/src/content/docs/architecture.md`](docs/src/content/do | Path | What | Key files | |---|---|---| -| `operator/` | Go operator (controller-runtime) | `api/v1alpha1/app_types.go` (the CRD), `internal/controller/app_controller.go` (reconcile pipeline), `internal/controller/workload.go` (pod spec), `internal/controller/authz.go` (the authz sidecar for Apps with groups/users, that can idle, or that run an image), `internal/controller/idle.go` (scale to zero: activity polling, the Idle phase, the activator ReferenceGrant), `internal/controller/activator.go` (the page idle Apps serve while they wake), `internal/controller/sources.go` (source volumes and init containers), `internal/controller/imageref.go` (image references and the registry allowlist for `source.type: image`), `internal/controller/routing.go` (NebariApp) | -| `authz/` | Go sidecar: verifies the forwarded access token and enforces `access.groups`/`access.users` (403 otherwise); in passthrough mode (unrestricted Apps that can idle) it only proxies. Either way it reports activity for the idle check | `main.go` (env config), `internal/gate/verify.go` (token checks), `internal/gate/proxy.go` (proxy + probes), `internal/gate/activity.go` (`/activity`) | +| `operator/` | Go operator (controller-runtime) | `api/v1alpha1/app_types.go` (the CRD), `internal/controller/app_controller.go` (reconcile pipeline), `internal/controller/workload.go` (pod spec), `internal/controller/authz.go` (the authz sidecar for Apps with groups, that can idle, or that run an image), `internal/controller/idle.go` (scale to zero: activity polling, the Idle phase, the activator ReferenceGrant), `internal/controller/activator.go` (the page idle Apps serve while they wake), `internal/controller/sources.go` (source volumes and init containers), `internal/controller/imageref.go` (image references and the registry allowlist for `source.type: image`), `internal/controller/routing.go` (NebariApp) | +| `authz/` | Go sidecar: verifies the forwarded access token and enforces `access.groups` (403 otherwise; Apps restrict access by group only); in passthrough mode (unrestricted Apps that can idle) it only proxies. Either way it reports activity for the idle check | `main.go` (env config), `internal/gate/verify.go` (token checks), `internal/gate/proxy.go` (proxy + probes), `internal/gate/activity.go` (`/activity`) | | `api/` | FastAPI REST API over App CRs | `src/nebari_apps_api/main.py` (routes), `auth.py` (JWT + owner/admin checks), `models.py` (request/response models), `cr.py` (model ↔ CR), `images.py` (image allowlist, mirrors `imageref.go`), `k8s.py` (store) | | `mcp/` | FastMCP server, agent tools, resources and prompts at `/mcp` | `src/nebari_apps_mcp/server.py` (tools, `nebari-apps://` resources, prompts), `auth.py` (device flow, sessions), `untrusted.py` (fencing app output) | | `ui/` | React + Vite UI on the Nebari design registry | `src/pages/`, `src/components/` (`app-form/`: state, sections), `src/hooks/use-app-actions.ts`, `src/lib/api.ts`, `src/lib/types.ts` (over the generated `crd.gen.ts`) | diff --git a/README.md b/README.md index 7e6ee87..64d0993 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ start again on the next request | apps-api | REST API for CRUD, status, logs, events, analytics and zip/`.html` upload; writes `App` resources (and each App's write-only Secret) for the UI and the MCP server. | `apps-api` | | apps-ui | The apps list as the home page (with a Needs attention banner, a Mine filter and Open links), app detail with logs and events, the launch form, and a Settings page (the UI and API versions, your groups, the cluster's settings, and how to connect an agent). | `apps-ui` | | apps-mcp | MCP server at `/mcp` that lets coding agents launch and manage apps (tools), attach an app's spec, status, logs and events as context (resources), and follow the triage, launch and access-review workflows (prompts), with Keycloak device-flow login. | `apps-mcp` | -| apps-authz | Sidecar in the pods of Apps that list `access.groups` or `access.users` (returns `403` to everyone else) or that can idle (records when the app last served a request). | `apps-authz` | +| apps-authz | Sidecar in the pods of Apps that list `access.groups` (returns `403` to everyone else) or that can idle (records when the app last served a request). | `apps-authz` | Images are published to `ghcr.io/nebari-dev/apps-pack/` and `quay.io/nebari/apps-pack-`. The UI signs in with keycloak-js and the API validates its @@ -104,10 +104,9 @@ The user guide is at [packs.nebari.dev/apps-pack](https://packs.nebari.dev/apps- ## Known limitations -- `access.groups` and `access.users` are enforced in the app pod by the apps-authz sidecar, not - at the gateway; group changes apply when the user's token refreshes (5 minutes by default). -- Landing-page tile visibility is group-based only, so the launch form's landing-page switch is - locked for now; register a tile through the API, the MCP server or `kubectl`. +- `access.groups` is enforced in the app pod by the apps-authz sidecar, not + at the gateway (Apps restrict access by Keycloak group only); group changes apply when the user's token refreshes (5 minutes by default). +- The launch form's landing-page switch is locked for now; register a tile through the API, the MCP server or `kubectl`. - Users see and manage the Apps they launched or are listed as managers of (`access.managers`); members of `api.adminGroups` manage all of them. Ownership cannot be transferred yet. diff --git a/api/src/nebari_apps_api/auth.py b/api/src/nebari_apps_api/auth.py index d47a220..7418d5e 100644 --- a/api/src/nebari_apps_api/auth.py +++ b/api/src/nebari_apps_api/auth.py @@ -60,8 +60,7 @@ def app_managers(cr: dict[str, Any]) -> dict[str, list[str]]: def is_manager(user: User, cr: dict[str, Any]) -> bool: - """Listed in spec.access.managers by username (case-insensitive, like - access.users) or through a group in the caller's verified token (a leading + """Listed in spec.access.managers by username (case-insensitive) or through a group in the caller's verified token (a leading "/" ignored on both sides, like api.adminGroups). """ managers = app_managers(cr) diff --git a/api/src/nebari_apps_api/cr.py b/api/src/nebari_apps_api/cr.py index f55a558..a4a75ca 100644 --- a/api/src/nebari_apps_api/cr.py +++ b/api/src/nebari_apps_api/cr.py @@ -108,7 +108,6 @@ def to_cr(req: AppCreate, owner: str) -> dict[str, Any]: "access": { "public": req.access.public, "groups": req.access.groups, - "users": req.access.users, "managers": req.access.managers.model_dump() if req.access.managers else None, "subdomain": req.access.subdomain, }, diff --git a/api/src/nebari_apps_api/main.py b/api/src/nebari_apps_api/main.py index a89d023..ba8e05f 100644 --- a/api/src/nebari_apps_api/main.py +++ b/api/src/nebari_apps_api/main.py @@ -13,7 +13,9 @@ import httpx import structlog from fastapi import Body, Depends, FastAPI, File, Form, HTTPException, Query, UploadFile +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse, Response +from pydantic import ValidationError from . import __version__ from . import cr as crmod @@ -309,7 +311,11 @@ def create_from_upload( data_bytes = read_capped(file.file, settings.max_upload_bytes) files = files_from_upload(file.filename or "upload", data_bytes, pixi=pixi) data["source"] = {"type": "inline", "inline": {"files": files}} - req = AppCreate.model_validate(data) + try: + req = AppCreate.model_validate(data) + except ValidationError as exc: + # A 422 like a JSON body gets, not a 500. + raise RequestValidationError(exc.errors()) from exc return create_checked(store, user, req) @@ -539,16 +545,15 @@ def entry(ns: str, name: str) -> dict[str, int]: def _check_access(access: Any) -> None: - """Groups and users reach the authz sidecar as comma-separated lists, so - an entry may not be blank or contain a comma (the operator refuses such an - App; this says so up front). + """Groups reach the authz sidecar as a comma-separated list, so an entry + may not be blank or contain a comma (the operator refuses such an App; + this says so up front). """ if access is None: return - for field in ("groups", "users"): - for entry in getattr(access, field): - if not entry.strip() or "," in entry: - raise HTTPException(422, f"access.{field}: entries must be non-empty and must not contain ','") + for entry in access.groups: + if not entry.strip() or "," in entry: + raise HTTPException(422, "access.groups: entries must be non-empty and must not contain ','") if access.managers is not None: for field in ("users", "groups"): if any(not entry.strip() for entry in getattr(access.managers, field)): diff --git a/api/src/nebari_apps_api/models.py b/api/src/nebari_apps_api/models.py index fa801b9..926e1e0 100644 --- a/api/src/nebari_apps_api/models.py +++ b/api/src/nebari_apps_api/models.py @@ -156,18 +156,28 @@ class AppManagers(BaseModel): class AppAccess(BaseModel): + # Unknown keys are ignored on reads: an App stored before the users list was + # removed from access may still carry it (the apiserver prunes it on the next write). public: bool = False - # A private app with groups or users is enforced in the app pod by the - # authz sidecar: a member of any group OR a listed username gets in, - # everyone else gets 403. Both empty: any signed-in user. + # A private app with groups is enforced in the app pod by the authz + # sidecar: a member of any group gets in, everyone else gets 403. Empty: + # any signed-in user. groups: list[str] = Field(default_factory=list) - users: list[str] = Field(default_factory=list) # Users and groups who may manage the App like its owner; only the owner # and admins may change this list. managers: AppManagers | None = None subdomain: str = Field(max_length=63, pattern=r"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") +class AppAccessRequest(AppAccess): + """access in a create or patch body. Unknown keys are refused, so a client + still sending the removed users list gets a 422 instead of an App that is + silently open to more people than it asked for. + """ + + model_config = ConfigDict(extra="forbid") + + class AppLandingPage(BaseModel): """Whether the app is registered as a tile on the Nebari landing page. @@ -190,7 +200,7 @@ class AppCreate(BaseModel): thumbnail: str = "" source: AppSource runtime: AppRuntime = Field(default_factory=AppRuntime) - access: AppAccess + access: AppAccessRequest landingPage: AppLandingPage | None = None # Write-only secret values: stored in the App's Secret, never in the App # and never returned; runtime.secretKeys lists their names. @@ -207,7 +217,7 @@ class AppPatch(BaseModel): thumbnail: str | None = None source: AppSource | None = None runtime: AppRuntime | None = None - access: AppAccess | None = None + access: AppAccessRequest | None = None landingPage: AppLandingPage | None = None # Set or rotate (a value) or remove (null) secret keys; keys not sent are # kept. Write-only, like AppCreate.secrets. diff --git a/api/tests/test_apps.py b/api/tests/test_apps.py index a2b188b..c52e0da 100644 --- a/api/tests/test_apps.py +++ b/api/tests/test_apps.py @@ -436,55 +436,37 @@ def test_landing_page_opt_in_and_out(client, store): assert "landingPage" not in store.apps[("apps", "docs-site")]["spec"] -def test_access_users_round_trip_on_create(client, store): - body = make_app_body() - body["access"]["users"] = ["alice", "bob"] - body["access"]["groups"] = ["analysts"] - resp = client.post("/api/v1/apps", json=body) - assert resp.status_code == 201 - stored = store.apps[("apps", "docs-site")]["spec"]["access"] - assert stored["users"] == ["alice", "bob"] - assert stored["groups"] == ["analysts"] - assert client.get("/api/v1/apps/apps/docs-site").json()["access"]["users"] == ["alice", "bob"] - - -def test_access_users_round_trip_on_upload(client, store): +def test_access_users_is_refused(client, store): + """The users list was removed from access: a client still sending it gets a 422, not an + App open to more people than it asked for. + """ import json - manifest = {k: v for k, v in make_app_body().items() if k != "source"} - manifest["access"]["users"] = ["alice"] + body = make_app_body() + body["access"]["users"] = ["alice"] + assert client.post("/api/v1/apps", json=body).status_code == 422 + + manifest = {k: v for k, v in body.items() if k != "source"} resp = client.post( "/api/v1/apps/upload", data={"manifest": json.dumps(manifest)}, files={"file": ("index.html", b"

hi

", "text/html")}, ) - assert resp.status_code == 201 - assert store.apps[("apps", "docs-site")]["spec"]["access"]["users"] == ["alice"] - + assert resp.status_code == 422 + assert store.apps == {} -def test_access_users_round_trip_on_patch(client, store): assert client.post("/api/v1/apps", json=make_app_body()).status_code == 201 resp = client.patch( "/api/v1/apps/apps/docs-site", json={"access": {"public": False, "users": ["alice"], "subdomain": "docs-site"}}, ) - assert resp.status_code == 200 - assert store.apps[("apps", "docs-site")]["spec"]["access"]["users"] == ["alice"] - assert resp.json()["access"]["users"] == ["alice"] - # An empty list clears it. - resp = client.patch( - "/api/v1/apps/apps/docs-site", - json={"access": {"public": False, "users": [], "groups": ["team"], "subdomain": "docs-site"}}, - ) - assert resp.status_code == 200 - access = store.apps[("apps", "docs-site")]["spec"]["access"] - assert "users" not in access - assert access["groups"] == ["team"] + assert resp.status_code == 422 + assert "users" not in store.apps[("apps", "docs-site")]["spec"]["access"] -@pytest.mark.parametrize("field", ["groups", "users"]) @pytest.mark.parametrize("entry", ["a,b", " ", ""]) -def test_access_list_entries_must_be_single_names(client, store, field, entry): +def test_access_group_entries_must_be_single_names(client, store, entry): + field = "groups" body = make_app_body() body["access"][field] = [entry] resp = client.post("/api/v1/apps", json=body) @@ -500,10 +482,14 @@ def test_access_list_entries_must_be_single_names(client, store, field, entry): def test_cr_with_users_still_readable(client, store): - """A kubectl-authored CR that sets users must not break reads.""" + """A CR stored before the users list was removed from access must not break reads; the + field is not returned. + """ assert client.post("/api/v1/apps", json=make_app_body()).status_code == 201 store.apps[("apps", "docs-site")]["spec"]["access"]["users"] = ["alice"] - assert client.get("/api/v1/apps/apps/docs-site").status_code == 200 + resp = client.get("/api/v1/apps/apps/docs-site") + assert resp.status_code == 200 + assert "users" not in resp.json()["access"] assert client.get("/api/v1/apps").status_code == 200 diff --git a/authz/internal/gate/config.go b/authz/internal/gate/config.go index 6d388a3..f17c84b 100644 --- a/authz/internal/gate/config.go +++ b/authz/internal/gate/config.go @@ -1,7 +1,6 @@ // Package gate is the apps-authz sidecar: a reverse proxy in front of an app // container that lets a request through only when it carries a valid -// Keycloak access token whose groups claim names one of the allowed groups -// or whose username is one of the allowed users (either is enough). +// Keycloak access token whose groups claim names one of the allowed groups. package gate import ( @@ -41,16 +40,10 @@ type Config struct { // GroupsClaim is the top-level claim holding the user's groups // (AUTHZ_GROUPS_CLAIM, default "groups"). GroupsClaim string - // Users lets in these usernames, compared case-insensitively - // (AUTHZ_USERS, comma-separated). - Users []string - // UsersClaim is the top-level claim holding the username - // (AUTHZ_USERS_CLAIM, default "preferred_username"). - UsersClaim string // Passthrough proxies every request without checking a token // (AUTHZ_PASSTHROUGH=true). The operator sets it on Apps that restrict // nobody but may be scaled to zero when idle, where the sidecar only - // records activity. It cannot be combined with groups or users. + // records activity. It cannot be combined with groups. Passthrough bool } @@ -65,8 +58,6 @@ func FromEnv(getenv func(string) string) (Config, error) { ClientID: strings.TrimSpace(getenv("AUTHZ_CLIENT_ID")), Groups: splitList(getenv("AUTHZ_GROUPS")), GroupsClaim: orDefault(getenv("AUTHZ_GROUPS_CLAIM"), "groups"), - Users: splitList(getenv("AUTHZ_USERS")), - UsersClaim: orDefault(getenv("AUTHZ_USERS_CLAIM"), "preferred_username"), Passthrough: strings.TrimSpace(getenv("AUTHZ_PASSTHROUGH")) == "true", } @@ -83,8 +74,8 @@ func FromEnv(getenv func(string) string) (Config, error) { } if cfg.Passthrough { // Refuse rather than silently open an App that meant to restrict. - if len(cfg.Groups) > 0 || len(cfg.Users) > 0 { - errs = append(errs, errors.New("AUTHZ_PASSTHROUGH cannot be combined with AUTHZ_GROUPS or AUTHZ_USERS")) + if len(cfg.Groups) > 0 { + errs = append(errs, errors.New("AUTHZ_PASSTHROUGH cannot be combined with AUTHZ_GROUPS")) } return cfg, errors.Join(errs...) } @@ -99,10 +90,10 @@ func FromEnv(getenv func(string) string) (Config, error) { if cfg.ClientID == "" { errs = append(errs, errors.New("AUTHZ_CLIENT_ID is required")) } - if len(cfg.Groups) == 0 && len(cfg.Users) == 0 { + if len(cfg.Groups) == 0 { // The sidecar exists only to restrict; with nothing to allow it // would deny everyone, which is a misconfiguration, not a policy. - errs = append(errs, errors.New("AUTHZ_GROUPS or AUTHZ_USERS is required (comma-separated)")) + errs = append(errs, errors.New("AUTHZ_GROUPS is required (comma-separated)")) } return cfg, errors.Join(errs...) } diff --git a/authz/internal/gate/gate_test.go b/authz/internal/gate/gate_test.go index e3027f3..f015db7 100644 --- a/authz/internal/gate/gate_test.go +++ b/authz/internal/gate/gate_test.go @@ -137,7 +137,6 @@ func testConfig(r *realm) Config { ClientID: clientID, Groups: []string{"analysts", "admin"}, GroupsClaim: "groups", - UsersClaim: "preferred_username", } } @@ -227,31 +226,6 @@ func TestVerify(t *testing.T) { {name: "not a JWT", token: func() string { return "not.a.jwt" }}, - // AUTHZ_USERS: OR semantics with the groups. - {name: "listed user with no groups claim", allowed: true, - cfg: func(c *Config) { c.Users = []string{"ada"} }, - token: func() string { return r.mint(t, with(base, map[string]any{"groups": nil})) }}, - {name: "listed user outside every group", allowed: true, - cfg: func(c *Config) { c.Users = []string{"grace", "ada"} }, - token: func() string { return r.mint(t, with(base, map[string]any{"groups": []string{"test-group"}})) }}, - {name: "users only, case-insensitive", allowed: true, - cfg: func(c *Config) { c.Groups = nil; c.Users = []string{" Ada "} }, - token: func() string { return r.mint(t, with(base, map[string]any{"preferred_username": "ADA"})) }}, - {name: "users only, member of a group is not enough", - cfg: func(c *Config) { c.Groups = nil; c.Users = []string{"grace"} }, - token: func() string { return r.mint(t, base) }}, - {name: "unlisted user and non-member", - cfg: func(c *Config) { c.Users = []string{"grace"} }, - token: func() string { return r.mint(t, with(base, map[string]any{"groups": []string{"test-group"}})) }}, - {name: "listed user, no username claim", - cfg: func(c *Config) { c.Groups = nil; c.Users = []string{"ada"} }, - token: func() string { return r.mint(t, with(base, map[string]any{"preferred_username": nil})) }}, - {name: "listed user, username claim of the wrong type", - cfg: func(c *Config) { c.Groups = nil; c.Users = []string{"ada"} }, - token: func() string { return r.mint(t, with(base, map[string]any{"preferred_username": []string{"ada"}})) }}, - {name: "custom username claim", allowed: true, - cfg: func(c *Config) { c.Groups = nil; c.Users = []string{"ada@example.com"}; c.UsersClaim = "email" }, - token: func() string { return r.mint(t, base) }}, {name: "custom groups claim", allowed: true, cfg: func(c *Config) { c.GroupsClaim = "teams" }, token: func() string { return r.mint(t, with(base, map[string]any{"groups": nil, "teams": []string{"admin"}})) }}, @@ -386,23 +360,6 @@ func TestHandler(t *testing.T) { } } -func TestHandlerForbiddenBodyHidesUsernames(t *testing.T) { - r := newRealm(t) - gate, _ := newGate(t, r, newApp(t).URL, func(c *Config) { c.Users = []string{"grace"} }) - tok := r.mint(t, with(keycloakClaims(time.Now()), map[string]any{"groups": []string{"test-group"}})) - req, _ := http.NewRequest(http.MethodGet, gate.URL, nil) - req.Header.Set("Authorization", "Bearer "+tok) - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) - } - defer func() { _ = resp.Body.Close() }() - body, _ := io.ReadAll(resp.Body) - if resp.StatusCode != http.StatusForbidden || strings.Contains(string(body), "grace") || strings.Contains(string(body), "analysts") { - t.Fatalf("status %d body %q: want a generic 403", resp.StatusCode, body) - } -} - func TestHandlerNoXFF(t *testing.T) { r := newRealm(t) gate, _ := newGate(t, r, newApp(t).URL, nil) @@ -590,26 +547,18 @@ func TestFromEnv(t *testing.T) { if err != nil { t.Fatal(err) } - if cfg.Listen != ":8080" || cfg.HealthListen != ":8082" || cfg.GroupsClaim != "groups" || cfg.UsersClaim != "preferred_username" { + if cfg.Listen != ":8080" || cfg.HealthListen != ":8082" || cfg.GroupsClaim != "groups" { t.Errorf("defaults: %+v", cfg) } if len(cfg.Issuers) != 2 || cfg.Issuers[1] != publicIssuer || len(cfg.Groups) != 2 || cfg.Groups[1] != "admin" { t.Errorf("lists: %+v", cfg) } - usersOnly := maps.Clone(full) - delete(usersOnly, "AUTHZ_GROUPS") - usersOnly["AUTHZ_USERS"] = "ada,grace" - usersOnly["AUTHZ_USERS_CLAIM"] = "email" - if cfg, err := FromEnv(env(usersOnly)); err != nil || len(cfg.Users) != 2 || cfg.UsersClaim != "email" { - t.Errorf("users only: %+v %v", cfg, err) - } - cases := map[string]struct { edit map[string]string want string }{ - "nothing to allow": {map[string]string{"AUTHZ_GROUPS": " , "}, "AUTHZ_GROUPS or AUTHZ_USERS is required"}, + "nothing to allow": {map[string]string{"AUTHZ_GROUPS": " , "}, "AUTHZ_GROUPS is required"}, "no upstream": {map[string]string{"AUTHZ_UPSTREAM": ""}, "AUTHZ_UPSTREAM is required"}, "bad upstream": {map[string]string{"AUTHZ_UPSTREAM": "127.0.0.1:8081"}, "AUTHZ_UPSTREAM"}, "no issuers": {map[string]string{"AUTHZ_ISSUERS": ""}, "AUTHZ_ISSUERS is required"}, @@ -709,15 +658,12 @@ func TestFromEnvPassthrough(t *testing.T) { if err != nil || !cfg.Passthrough { t.Fatalf("passthrough needs only an upstream: %+v %v", cfg, err) } - for _, v := range []string{"AUTHZ_GROUPS", "AUTHZ_USERS"} { - _, err := FromEnv(env(map[string]string{ - "AUTHZ_UPSTREAM": "http://127.0.0.1:8081", - "AUTHZ_PASSTHROUGH": "true", - v: "analysts", - })) - if err == nil || !strings.Contains(err.Error(), "cannot be combined") { - t.Errorf("passthrough with %s: %v", v, err) - } + if _, err := FromEnv(env(map[string]string{ + "AUTHZ_UPSTREAM": "http://127.0.0.1:8081", + "AUTHZ_PASSTHROUGH": "true", + "AUTHZ_GROUPS": "analysts", + })); err == nil || !strings.Contains(err.Error(), "cannot be combined") { + t.Errorf("passthrough with AUTHZ_GROUPS: %v", err) } if _, err := FromEnv(env(map[string]string{"AUTHZ_PASSTHROUGH": "true"})); err == nil { t.Error("passthrough without an upstream should fail") diff --git a/authz/internal/gate/proxy.go b/authz/internal/gate/proxy.go index d2eb57f..c292b97 100644 --- a/authz/internal/gate/proxy.go +++ b/authz/internal/gate/proxy.go @@ -21,8 +21,7 @@ type Handler struct { Activity *Activity proxy *httputil.ReverseProxy log *slog.Logger - // forbidden is the 403 body: it names the groups when only groups are - // configured, and stays generic when usernames (PII) are configured. + // forbidden is the 403 body: it names the allowed groups. forbidden string } @@ -61,10 +60,7 @@ func NewHandler(cfg Config, v *Verifier, transport http.RoundTripper, log *slog. http.Error(w, "Bad Gateway: the app is not responding", http.StatusBadGateway) }, } - forbidden := "Forbidden: this app is restricted to specific groups or users" - if len(cfg.Users) == 0 { - forbidden = "Forbidden: this app is restricted to groups: " + strings.Join(cfg.Groups, ", ") - } + forbidden := "Forbidden: this app is restricted to groups: " + strings.Join(cfg.Groups, ", ") return &Handler{verifier: v, proxy: proxy, log: log, forbidden: forbidden} } diff --git a/authz/internal/gate/verify.go b/authz/internal/gate/verify.go index e19d4ca..64cf8a3 100644 --- a/authz/internal/gate/verify.go +++ b/authz/internal/gate/verify.go @@ -38,8 +38,6 @@ type Verifier struct { clientID string groups map[string]bool groupsClaim string - users map[string]bool - usersClaim string now func() time.Time } @@ -50,16 +48,11 @@ func NewVerifier(cfg Config, keys oidc.KeySet) *Verifier { clientID: cfg.ClientID, groups: map[string]bool{}, groupsClaim: cfg.GroupsClaim, - users: map[string]bool{}, - usersClaim: cfg.UsersClaim, now: time.Now, } for _, g := range cfg.Groups { v.groups[normalizeGroup(g)] = true } - for _, u := range cfg.Users { - v.users[normalizeUser(u)] = true - } // go-oidc checks the signature and algorithm only; the claim checks // below accept several issuers, use azp and allow clock skew both ways. v.jwt = oidc.NewVerifier("", keys, &oidc.Config{ @@ -120,10 +113,10 @@ func (v *Verifier) Verify(ctx context.Context, raw string) error { return deny("token was issued to another client") } - if v.groupsAllow(c) || v.usersAllow(c) { + if v.groupsAllow(c) { return nil } - return deny("user is not in an allowed group or user list") + return deny("user is not in an allowed group") } // groupsAllow reports whether the groups claim (a string array, or a single @@ -150,23 +143,7 @@ func (v *Verifier) groupsAllow(c map[string]json.RawMessage) bool { return false } -// usersAllow reports whether the username claim is an allowed user, -// trimmed and case-insensitive (Keycloak lowercases usernames). -func (v *Verifier) usersAllow(c map[string]json.RawMessage) bool { - raw, ok := c[v.usersClaim] - if !ok || len(v.users) == 0 { - return false - } - var name string - if err := json.Unmarshal(raw, &name); err != nil { - return false - } - name = normalizeUser(name) - return name != "" && v.users[name] -} - func normalizeGroup(g string) string { return strings.TrimPrefix(strings.TrimSpace(g), "/") } -func normalizeUser(u string) string { return strings.ToLower(strings.TrimSpace(u)) } // optional decodes claim name into dst when it is present. func optional(c map[string]json.RawMessage, name string, dst any) error { diff --git a/authz/main.go b/authz/main.go index cc9aa44..eaa1ea6 100644 --- a/authz/main.go +++ b/authz/main.go @@ -8,16 +8,13 @@ // AUTHZ_ISSUERS accepted iss values, comma-separated (public and in-cluster realm URL) // AUTHZ_JWKS_URL the realm's certs endpoint // AUTHZ_CLIENT_ID the gateway's OIDC client; must equal azp or appear in aud -// AUTHZ_GROUPS allowed groups, comma-separated +// AUTHZ_GROUPS allowed groups, comma-separated (required) // AUTHZ_GROUPS_CLAIM claim holding the user's groups (default groups) -// AUTHZ_USERS allowed usernames, comma-separated, case-insensitive -// AUTHZ_USERS_CLAIM claim holding the username (default preferred_username) // AUTHZ_PASSTHROUGH true: check no tokens, only record activity (unrestricted Apps that may idle) // // A request passes when its bearer access token verifies (signature against // the realm's JWKS, exp/nbf, iss, client) and the user is in any allowed -// group OR is an allowed user. At least one of AUTHZ_GROUPS and AUTHZ_USERS -// must be set. A request without an Authorization header gets 401 (the +// group. AUTHZ_GROUPS is required unless AUTHZ_PASSTHROUGH is set. A request without an Authorization header gets 401 (the // gateway forwarded no token); every other refusal is 403. // // In passthrough mode every request is proxied unchecked. Either way the @@ -46,7 +43,7 @@ func main() { ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() log.Info("starting", "upstream", cfg.Upstream.String(), "issuers", cfg.Issuers, - "clientID", cfg.ClientID, "groups", len(cfg.Groups), "users", len(cfg.Users), "passthrough", cfg.Passthrough) + "clientID", cfg.ClientID, "groups", len(cfg.Groups), "passthrough", cfg.Passthrough) if err := gate.Run(ctx, cfg, log); err != nil { log.Error("server failed", "error", err.Error()) os.Exit(1) diff --git a/charts/nebari-apps/README.md b/charts/nebari-apps/README.md index a1a6e35..5a86ec7 100644 --- a/charts/nebari-apps/README.md +++ b/charts/nebari-apps/README.md @@ -56,7 +56,7 @@ wrong type fails the install. | gitImage | string | `"alpine/git:v2.47.2"` | Image used by init containers to fetch git sources. | | keycloak.internalUrl | string | `""` | Optional in-cluster Keycloak URL (split horizon) the API, the MCP server and the authz sidecar fetch JWKS from. The sidecar accepts tokens issued under either URL. | | keycloak.realm | string | `"nebari"` | Keycloak realm. | -| keycloak.url | string | `""` | Browser-facing Keycloak base URL, e.g. `https://keycloak.example.ai/auth`. Required when `api.auth.enabled`, and (or `internalUrl`) for apps that list `access.groups`/`access.users`. | +| keycloak.url | string | `""` | Browser-facing Keycloak base URL, e.g. `https://keycloak.example.ai/auth`. Required when `api.auth.enabled`, and (or `internalUrl`) for apps that list `access.groups`. | | mcp.enabled | bool | `true` | Deploy the MCP server. Agents connect at `https:///mcp` (streamable HTTP); the UI's nginx proxies `/mcp` to it, so there is no separate hostname. When off, `/mcp` returns 404. | | mcp.extraEnv | list | `[]` | Extra environment variables for apps-mcp, after the chart's own (an entry with the same name wins). | | mcp.image.pullPolicy | string | `"IfNotPresent"` | Image pull policy. | diff --git a/charts/nebari-apps/crds/apps.nebari.dev_apps.yaml b/charts/nebari-apps/crds/apps.nebari.dev_apps.yaml index 7bf1c61..bb4e11b 100644 --- a/charts/nebari-apps/crds/apps.nebari.dev_apps.yaml +++ b/charts/nebari-apps/crds/apps.nebari.dev_apps.yaml @@ -65,8 +65,9 @@ spec: Groups are the Keycloak/OIDC groups allowed to open a private app. Enforced in the app pod by the authz sidecar, which checks the access token the gateway forwards: everyone else gets 403. A user in any - listed group OR listed in users is let in. Empty (with no users) allows - any signed-in realm user. Changes apply when the user's access token + listed group is let in. Empty allows any signed-in realm user. Also + forwarded to the NebariApp, where it sets landing-page tile visibility. + Changes apply when the user's access token refreshes (Keycloak's access token lifespan, 5 minutes by default). items: type: string @@ -76,7 +77,7 @@ spec: Managers may manage the app (edit, start, stop, restart, delete, read logs and metrics) through apps-api, the UI and the MCP server, like its owner. Only the owner and api.adminGroups members may change this list. - It does not grant opening the app: that is still public, groups and users. + It does not grant opening the app: that is still public and groups. properties: groups: description: |- @@ -107,15 +108,6 @@ spec: x-kubernetes-validations: - message: access.subdomain is immutable rule: self == oldSelf - users: - description: |- - Users are Keycloak usernames (preferred_username, case-insensitive) - allowed to open a private app, in addition to the members of groups. - Enforced in the app pod like groups. Not forwarded to the NebariApp, - so they do not affect landing-page tile visibility. - items: - type: string - type: array required: - subdomain type: object @@ -339,7 +331,7 @@ spec: environment from the app source and executes `pixi run `. Not allowed with source type image, which runs its own server. The task must start a server on $PORT: 8080, or 8081 when access.groups - or access.users restricts the app (the authz sidecar takes 8080). + restricts the app or it can idle (the authz sidecar takes 8080). maxLength: 64 type: string replicas: diff --git a/charts/nebari-apps/values.schema.json b/charts/nebari-apps/values.schema.json index 7370cad..4b5bc57 100644 --- a/charts/nebari-apps/values.schema.json +++ b/charts/nebari-apps/values.schema.json @@ -1056,7 +1056,7 @@ }, "authz": { "type": "object", - "description": "The apps-authz sidecar that enforces access.groups and access.users in app pods.", + "description": "The apps-authz sidecar that enforces access.groups in app pods.", "properties": { "image": { "type": "object", diff --git a/charts/nebari-apps/values.yaml b/charts/nebari-apps/values.yaml index c5695f7..48ac329 100644 --- a/charts/nebari-apps/values.yaml +++ b/charts/nebari-apps/values.yaml @@ -302,7 +302,7 @@ mcp: keycloak: # -- Browser-facing Keycloak base URL, e.g. # `https://keycloak.example.ai/auth`. Required when `api.auth.enabled`, - # and (or `internalUrl`) for apps that list `access.groups`/`access.users`. + # and (or `internalUrl`) for apps that list `access.groups`. url: "" # -- Keycloak realm. realm: nebari @@ -362,7 +362,7 @@ gitImage: alpine/git:v2.47.2 # `pixi` binary and run as a non-root user. pythonImage: ghcr.io/prefix-dev/pixi:0.68.1-noble -# The apps-authz sidecar. Apps that list `access.groups` or `access.users` +# The apps-authz sidecar. Apps that list `access.groups` # get it in front of the app container: it checks the access token the # gateway forwards against the Keycloak realm (`keycloak.url`, and # `keycloak.internalUrl` when set) and returns 403 to everyone else. diff --git a/dev/Makefile b/dev/Makefile index 90975e2..72deaea 100644 --- a/dev/Makefile +++ b/dev/Makefile @@ -53,7 +53,7 @@ UI_IMG := apps-ui:dev MCP_IMG := apps-mcp:dev AUTHZ_IMG := apps-authz:dev # In-cluster Keycloak from the nebari-operator dev scripts: the authz sidecar -# of apps that list access.groups/users fetches the realm's keys from it. +# of apps that list access.groups fetches the realm's keys from it. KEYCLOAK_INTERNAL_URL := http://keycloak-keycloakx-http.keycloak.svc.cluster.local:8080 APP_NAMESPACE := apps diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 96b21da..9bea1f8 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -289,7 +289,8 @@ tears down routing/cert/OIDC client. ### Authorization model - **Who can launch / manage an app:** enforced by apps-api using the caller's Keycloak groups. - An app's `access.groups`/`access.users` plus an `owner` field define management rights. + An app's `owner` field plus `access.managers` define management rights; `access.groups` + (the only access restriction) decides who may open it. - **Who can *view/use* a running app:** enforced at the gateway by the `NebariApp` `SecurityPolicy` (`allowedGroups`) — same mechanism every Nebari app uses. `public: true` disables it for anonymous apps. @@ -407,7 +408,7 @@ Jotai, Keycloak via the standard Nebari SSO. Exposed as a `NebariApp`. - **Source**: tabs for *Upload* (a zip or a single `.html` file — rendered into the CR as `inline` or `pvc`) and *Git*. - **Resources** (cpu/mem/replicas) and **env vars** (key-value editor). - - **Access**: public toggle, groups/users selector, subdomain. + - **Access**: public toggle, groups selector, subdomain. - Submit → `POST /apps` → redirect to the app detail page (spawn-pending → running). 3. **App detail / observability** — status + conditions timeline, live URL, **logs viewer** (streamed), **events**, metrics (cpu/mem), and edit/stop/start/delete actions. diff --git a/docs/src/content/docs/api-reference.md b/docs/src/content/docs/api-reference.md index bb80399..f97e46c 100644 --- a/docs/src/content/docs/api-reference.md +++ b/docs/src/content/docs/api-reference.md @@ -107,8 +107,8 @@ create. To launch a **Python/pixi app**, add `"pixiTask": ""` to `runtime` — the source (any type) must then contain a `pixi.toml` or `pyproject.toml` at its root, and the task -must start a server on `0.0.0.0:$PORT` (8080, or 8081 when `access.groups` or -`access.users` is set or the app can idle). See [Launching apps](/launching-apps/#python-apps-pixi). +must start a server on `0.0.0.0:$PORT` (8080, or 8081 when `access.groups` is set +or the app can idle). See [Launching apps](/launching-apps/#python-apps-pixi). To launch a **container image**, send an `image` source: @@ -194,7 +194,7 @@ curl -X POST https://apps.example.ai/api/v1/apps/upload \ | `404` | App (or its pods, for logs) not found. | | `409` | An app with that name already exists in the namespace, or the subdomain is already used by another app. | | `413` | Request body over 2 MiB, secret values over 256 KiB, or inline files over the ~900 KB inline cap (`MAX_UPLOAD_BYTES`), whether they come from an upload (or its extracted contents) or from `source.inline.files` in a JSON `POST`/`PATCH`. Zip members are checked against the cap before they are decompressed. | -| `422` | Invalid request: unsupported source type, missing required fields, an `access.groups`/`access.users` entry that is blank or contains a comma, an invalid, duplicated or env-clashing secret name, or a `runtime.env` reference to a Secret or to another app's ConfigMap. | +| `422` | Invalid request: unsupported source type, missing required fields, an `access.groups` entry that is blank or contains a comma, an `access` field the API does not know (such as the removed `users`), an invalid, duplicated or env-clashing secret name, or a `runtime.env` reference to a Secret or to another app's ConfigMap. | | `502` | The app's Secret could not be written (the reason is in the API's log, never a value). | Error bodies are `{"detail": "..."}` with an actionable message. diff --git a/docs/src/content/docs/app-crd-reference.md b/docs/src/content/docs/app-crd-reference.md index 080d834..8866e98 100644 --- a/docs/src/content/docs/app-crd-reference.md +++ b/docs/src/content/docs/app-crd-reference.md @@ -172,16 +172,15 @@ operator and apps-api refuse to start with a malformed one. | `replicas` | int | No | `1` | Desired replicas, `0`–`10`. `0` stops the app (phase `Stopped`). | | `keepAlive` | bool | No | `false` | `true` exempts the app from [scale to zero](#scale-to-zero): it keeps running however long it goes without a request, whatever `idleTimeoutMinutes` or the cluster default say. | | `idleTimeoutMinutes` | int | No | chart `apps.idle.timeoutMinutes` | Minutes without a request before the operator scales the app to zero (phase `Idle`), `5`–`43200`. Unset follows the cluster default (`0` there keeps apps running), so an App can opt in on a cluster that does not idle by default. Ignored when `keepAlive` is `true` or `replicas` is `0`. See [Scale to zero](#scale-to-zero). | -| `pixiTask` | string | No | — | Not allowed with `source.type: image`. When set, the app runs as a **Python/pixi service**: the operator copies the source into a writable workspace, runs `pixi install` (`--locked` when a `pixi.lock` is present), then `pixi run `. The task must start a server on `0.0.0.0:$PORT` (`PORT` and `HOME=/app` are injected; `PORT` is 8080, or 8081 when the sidecar takes 8080: `access.groups` or `access.users` is set, or the app can idle). Max 64 chars. | +| `pixiTask` | string | No | — | Not allowed with `source.type: image`. When set, the app runs as a **Python/pixi service**: the operator copies the source into a writable workspace, runs `pixi install` (`--locked` when a `pixi.lock` is present), then `pixi run `. The task must start a server on `0.0.0.0:$PORT` (`PORT` and `HOME=/app` are injected; `PORT` is 8080, or 8081 when the sidecar takes 8080: `access.groups` is set, or the app can idle). Max 64 chars. | ## spec.access | Field | Type | Required | Description | |---|---|---|---| | `public` | bool | No | `true` disables authentication entirely (anonymous access). | -| `groups` | []string | No | Keycloak groups allowed to open a private app. **Enforced in the app pod** by the authz sidecar (everyone else gets `403`); also passed to the `NebariApp` as `auth.groups`, which hides the landing-page tile from non-members. A user in **any** group **or** listed in `users` gets in; with both empty, any signed-in realm user does. Changes apply when the user's access token refreshes (Keycloak's lifespan, 5 minutes by default). See [Access control](/launching-apps/#access-control). | -| `users` | []string | No | Keycloak usernames (`preferred_username`, case-insensitive) allowed to open a private app, in addition to the members of `groups`. Enforced in the app pod like `groups`. Not passed to the `NebariApp` (it has no per-user rule), so it does not affect landing-page visibility. | -| `managers.users` | []string | No | Keycloak usernames (`preferred_username`, case-insensitive) who may **manage** the app through apps-api, the UI and the MCP server: everything its owner can do (edit, stop, start, restart, delete, read status, logs, events and metrics) except change `managers`. Only the owner and `api.adminGroups` members may change the list. Does **not** let them open a restricted app: that is still `public`, `groups` and `users`. Ignored by the operator. See [Managers](/launching-apps/#managers). | +| `groups` | []string | No | Keycloak groups allowed to open a private app. **Enforced in the app pod** by the authz sidecar (everyone else gets `403`); also passed to the `NebariApp` as `auth.groups`, which hides the landing-page tile from non-members. A user in **any** group gets in; with none listed, any signed-in realm user does. Access is by group only: to let one person in, add them to a group. Changes apply when the user's access token refreshes (Keycloak's lifespan, 5 minutes by default). See [Access control](/launching-apps/#access-control). | +| `managers.users` | []string | No | Keycloak usernames (`preferred_username`, case-insensitive) who may **manage** the app through apps-api, the UI and the MCP server: everything its owner can do (edit, stop, start, restart, delete, read status, logs, events and metrics) except change `managers`. Only the owner and `api.adminGroups` members may change the list. Does **not** let them open a restricted app: that is still `public` and `groups`. Ignored by the operator. See [Managers](/launching-apps/#managers). | | `managers.groups` | []string | No | Keycloak groups whose members may manage the app, like `managers.users`. Membership is read from the caller's verified access token (a leading `/` is ignored), the same way `api.adminGroups` is checked, so a group change applies when their token refreshes. | | `subdomain` | string | Yes | Lowercase DNS label, at most 63 characters. The app is served at `https://.`. **Unique across all Apps** in the cluster: the API returns `409` for a taken subdomain, and if two Apps claim one anyway (e.g. via `kubectl`), the oldest keeps it and the other goes `Failed` with `Validated=False`, reason `SubdomainTaken`, and no `NebariApp`. **Immutable** after creation. The operator mirrors it into the `apps.nebari.dev/subdomain` label. | @@ -221,7 +220,7 @@ For each `App` the operator creates (and owns, via `ownerReferences`): |---|---|---| | ConfigMap | `app--content` | Inline source files (inline apps only). | | ConfigMap | `app--server` | The nginx server config with security headers (static apps only). | -| Deployment | `app-` | The app workload (hardened: non-root, no privilege escalation, seccomp `RuntimeDefault`). The `app` container comes first; an App that lists `access.groups` or `access.users`, that can idle, or that runs an image also gets the `authz` sidecar on 8080, with the app on 8081 (an image app on its own `source.image.port`). For an unrestricted App the sidecar runs in passthrough mode and only records activity. Scaled to zero while the App is `Idle`. | +| Deployment | `app-` | The app workload (hardened: non-root, no privilege escalation, seccomp `RuntimeDefault`). The `app` container comes first; an App that lists `access.groups`, that can idle, or that runs an image also gets the `authz` sidecar on 8080, with the app on 8081 (an image app on its own `source.image.port`). For an unrestricted App the sidecar runs in passthrough mode and only records activity. Scaled to zero while the App is `Idle`. | | Service | `app-` | ClusterIP on port 8080 (the app, or the authz sidecar in front of it). | | NebariApp | `app-` | Routing + TLS + auth + (opt-in) landing-page tile, reconciled by the nebari-operator (contract pinned to `%NEBARI_OPERATOR_VERSION%`). While the App is `Idle` or waking, `spec.service` points at the activator Service in the pack's namespace. | @@ -300,6 +299,10 @@ existing field required, removes an enum value, or changes what an existing valu "CRD stays compatible with stored Apps" check in CI compares every PR's CRD with the base branch and fails on any of those. +**One exception so far:** `access.users` was removed from `v1alpha1` without a deprecation +period, when Apps moved to group-only access. The CRD check allows that one removal; see +[Upgrading to group-only access](/upgrading/#upgrading-to-group-only-access) before upgrading. + **Tightening validation** (a new bound, pattern or CEL rule, such as the 10-replica cap or the immutable subdomain) is allowed but called out: Kubernetes [validation ratcheting](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-ratcheting) diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md index af059ca..4eda151 100644 --- a/docs/src/content/docs/architecture.md +++ b/docs/src/content/docs/architecture.md @@ -34,7 +34,7 @@ For each `App`, the operator runs an ordered, idempotent pipeline: `RuntimeDefault`): nginx serving the app's content, with git sources cloned by a non-root init container. Static apps run an operator-written nginx config (a `-server` ConfigMap mounted over `/etc/nginx/conf.d`) that adds security headers. - An App restricted to groups or users also gets the `authz` sidecar (next section), and so + An App restricted to groups also gets the `authz` sidecar (next section), and so does an App that can idle, where the sidecar records activity. An `image` App runs the given image as the `app` container instead, always behind the sidecar, which keeps 8080 as the one port the Service and NetworkPolicy target while the image listens on its own port. Before the Deployment is @@ -109,15 +109,15 @@ Apps never contain auth code. A private app's `NebariApp` creates an Envoy Gatew `access.public: true` skips the policy entirely. This keeps user workloads honest — there is no token to mishandle inside an app pod. -The gateway only signs users in; it has no group or user rule +The gateway only signs users in; it has no group rule ([nebari-operator#153](https://github.com/nebari-dev/nebari-operator/issues/153)). So when a -private App lists `access.groups` or `access.users`, the operator enforces them in the pod: +private App lists `access.groups`, the operator enforces them in the pod: the `NebariApp` sets `forwardAccessToken: true`, and the Deployment gets the **apps-authz** sidecar on port 8080 in front of the app container (moved to 8081). The sidecar verifies the forwarded Keycloak access token against the realm's keys and lets in a member of any -listed group or a listed user; everyone else gets `403`. `access.groups` also goes to the -`NebariApp` as `auth.groups` (landing-page tile visibility); `access.users` does not, since -`NebariApp` has no per-user field. See [Access control](/launching-apps/#access-control). +listed group; everyone else gets `403`. Access is by group only: Apps have no per-user +allow list. `access.groups` also goes to the `NebariApp` as `auth.groups` (landing-page tile +visibility). See [Access control](/launching-apps/#access-control). ## Secrets diff --git a/docs/src/content/docs/configuration.md b/docs/src/content/docs/configuration.md index 208257f..b9595a0 100644 --- a/docs/src/content/docs/configuration.md +++ b/docs/src/content/docs/configuration.md @@ -17,7 +17,7 @@ only `clusterDomain` and `keycloak.url` need changing. | `appsDomain` | `apps.` | Domain apps are served under: `https://.`. | | `gateway` | `public` | Shared Gateway the UI's and every app's `NebariApp` attach to (`public` \| `internal`). | | `tls.enabled` | `true` | TLS for the UI and every launched app. `false` serves plain HTTP (no certificates); local dev only. | -| `keycloak.url` | — (required when `api.auth.enabled`) | Browser-facing Keycloak base URL, e.g. `https://keycloak.example.ai/auth`. The authz sidecar also accepts tokens issued under it; apps that list `access.groups`/`access.users` need it or `keycloak.internalUrl`. | +| `keycloak.url` | — (required when `api.auth.enabled`) | Browser-facing Keycloak base URL, e.g. `https://keycloak.example.ai/auth`. The authz sidecar also accepts tokens issued under it; apps that list `access.groups` need it or `keycloak.internalUrl`. | | `keycloak.realm` | `nebari` | Realm the API validates tokens against and the UI/MCP log in to. | | `keycloak.internalUrl` | — | In-cluster Keycloak URL (split horizon). The API and MCP server fetch JWKS from it; the authz sidecar fetches keys from it and accepts it as a token issuer. | @@ -35,7 +35,7 @@ only `clusterDomain` and `keycloak.url` need changing. | `staticImage` | `nginxinc/nginx-unprivileged:1.27-alpine` | Serves static app content; must listen on 8080 as non-root. The operator mounts its nginx server config (security headers, `try_files`) over `/etc/nginx/conf.d`, so a replacement image should be nginx-based or ignore that directory. | | `gitImage` | `alpine/git:v2.47.2` | Init-container image that clones `git` sources. | | `pythonImage` | `ghcr.io/prefix-dev/pixi:0.68.1-noble` | Runs Python/pixi apps (`runtime.pixiTask`); must provide `pixi` and run as non-root. | -| `authz.image.repository` / `tag` / `pullPolicy` | `ghcr.io/nebari-dev/apps-pack/apps-authz` / appVersion / `IfNotPresent` | The sidecar the operator adds to apps that list `access.groups` or `access.users`: it takes port 8080, checks the forwarded access token and returns `403` to everyone else (see [Access control](/launching-apps/#access-control)). Apps that can idle and image apps get it too; without groups or users it runs in passthrough mode and only records when the app last served a request. | +| `authz.image.repository` / `tag` / `pullPolicy` | `ghcr.io/nebari-dev/apps-pack/apps-authz` / appVersion / `IfNotPresent` | The sidecar the operator adds to apps that list `access.groups`: it takes port 8080, checks the forwarded access token and returns `403` to everyone else (see [Access control](/launching-apps/#access-control)). Apps that can idle and image apps get it too; without groups it runs in passthrough mode and only records when the app last served a request. | | `authz.resources` | `10m`/`32Mi` requests, `100m`/`64Mi` limits | The sidecar's requests/limits, on top of the app container's, in every pod that runs it. | ### Image registries @@ -268,12 +268,11 @@ The pack integrates with the platform through two kinds of `NebariApp` | `auth.enabled` | `!spec.access.public` | App | | `auth.provider` / `provisionClient` / `scopes` | `keycloak` / `true` / `openid profile email groups` | fixed (private apps) | | `auth.groups` | `spec.access.groups` | App — landing-page tile visibility; the gateway does not enforce it, the authz sidecar in the app pod does (see [Access control](/launching-apps/#access-control)) | -| `auth.forwardAccessToken` | `true` when `spec.access.groups` or `spec.access.users` is set (private apps) | App — the gateway forwards the access token the authz sidecar checks | +| `auth.forwardAccessToken` | `true` when `spec.access.groups` is set (private apps) | App — the gateway forwards the access token the authz sidecar checks | | `auth.enforceAtGateway` | `true`, with `forwardAccessToken` | fixed — the `NebariApp` CRD requires it for `forwardAccessToken` (it is also the default) | | `landingPage.enabled` | `spec.landingPage.enabled`, else `apps.landingPage.enabled` | App, then chart | | `landingPage.displayName` / `description` / `icon` / `category` | `spec.displayName` / `spec.description` / `spec.thumbnail` / `Apps` | App (only when the tile is enabled) | -Fields the pack never sets: `auth.users` (nebari-operator has no per-user rule; -`spec.access.users` is enforced only by the authz sidecar, so it does not hide the landing -tile), `auth.enforceAtGateway` on unrestricted apps (defaults to enforced), +Fields the pack never sets: `auth.users` (Apps restrict access by group only), +`auth.enforceAtGateway` on unrestricted apps (defaults to enforced), `landingPage.priority`, `landingPage.iconLight`/`iconDark`, `landingPage.externalUrl`. diff --git a/docs/src/content/docs/getting-started.md b/docs/src/content/docs/getting-started.md index b2489a5..3e95da2 100644 --- a/docs/src/content/docs/getting-started.md +++ b/docs/src/content/docs/getting-started.md @@ -73,7 +73,7 @@ kubectl apply -n argocd -f examples/argocd-application.yaml ``` This deploys four components. A fifth image, **apps-authz**, is not a Deployment: the -operator adds it as a sidecar to Apps that list `access.groups` or `access.users`. +operator adds it as a sidecar to Apps that list `access.groups`, that can idle, or that run an image. | Component | Where it ends up | |---|---| @@ -166,15 +166,15 @@ it the HTTPRoute, certificate, and OIDC client) are all garbage-collected. The pack is alpha: the happy path works, and these are the gaps to know about before a demo or pilot. -- **`access.groups` / `access.users` are enforced in the app pod, not at the gateway.** The +- **`access.groups` is enforced in the app pod, not at the gateway.** The nebari-operator `SecurityPolicy` only signs users in ([nebari-operator#153](https://github.com/nebari-dev/nebari-operator/issues/153)), so a - private app that lists groups or users gets the apps-authz sidecar, which checks the + private app that lists groups gets the apps-authz sidecar, which checks the forwarded access token and returns `403` to everyone else. Group changes apply when the - user's token refreshes (5 minutes by default). A private app with neither list is open to - any signed-in realm user. See [Access control](/launching-apps/#access-control). -- **Landing-page tile visibility is group-based only.** `access.users` cannot hide a tile, so - the launch form's landing-page switch is locked for now. Tiles are off by default for + user's token refreshes (5 minutes by default). A private app with no groups is open to any + signed-in realm user. Access is by group only; there is no per-user allow list. See + [Access control](/launching-apps/#access-control). +- **The launch form's landing-page switch is locked for now.** Tiles are off by default for launched apps (`apps.landingPage.enabled` / `spec.landingPage.enabled`); only the pack's own "Apps" tile is registered. See [Landing page](/launching-apps/#landing-page). - **Authorization is owner, manager or admin.** The API lets a user see and manage the Apps diff --git a/docs/src/content/docs/launching-apps.md b/docs/src/content/docs/launching-apps.md index 95c9b9d..f271d18 100644 --- a/docs/src/content/docs/launching-apps.md +++ b/docs/src/content/docs/launching-apps.md @@ -11,7 +11,7 @@ This pack deploys three kinds of app: - **Python apps** — launched by a [pixi](https://pixi.sh) task (`runtime.pixiTask`); the platform runs `pixi install` then `pixi run `, and the task must start a server on **0.0.0.0:$PORT** (`PORT` is injected: 8080, or 8081 when the app is - [restricted to groups or users](#access-control)). + [restricted to groups](#access-control) or can [idle](#idle-apps)). - **Container images** — a published image (Streamlit, Gradio, Shiny, Dash, a Node service) run as is, when the cluster allows its registry. See [Container images](#container-images). @@ -49,7 +49,7 @@ Open `https://apps.`: as environment variables from its own Kubernetes Secret (see [Secrets](#secrets)). On an edit, saved secrets show their names only: leave a value empty to keep it, type to replace it, or remove the row. -5. **Access** — public toggle, allowed groups and users, and the subdomain, which follows +5. **Access** — public toggle, allowed groups, and the subdomain, which follows the name (lowercased, other characters turned into hyphens) until you edit it. The landing-page option is locked off for now (see [Landing page](#landing-page)). 6. **Managers** — other users and groups who may manage the app with you (see @@ -139,7 +139,7 @@ The contract: `pixi run `. - The task must start a server listening on **0.0.0.0:$PORT**; `PORT` and `HOME=/app` are injected. `PORT` is 8080, or 8081 when the authz sidecar takes 8080 (the app lists - groups or users, or it can [idle](#idle-apps)), so read it rather than hard-coding a port. pixi task strings + groups, or it can [idle](#idle-apps)), so read it rather than hard-coding a port. pixi task strings support `$PORT` but not `${PORT:-8080}`; run `PORT=8080 pixi run start` locally. Routing, TLS, and SSO are identical to static apps. - First start resolves and downloads the environment, so cold starts take longer — @@ -185,7 +185,7 @@ What to know: every start; a digest pins the image. - **The port.** Set `port` to where the server listens (on `0.0.0.0` or `127.0.0.1`), from 1024 up and not 8080 or 8082: the pack's sidecar always sits in front of an image app on - 8080, enforcing `access.groups`/`access.users` and recording activity for + 8080, enforcing `access.groups` and recording activity for [idling](#idle-apps). The port is also passed as `$PORT`. - **Non-root.** The image must run as a non-root user, with no capabilities and no Kubernetes credentials. An image whose `USER` is root never starts: the app shows @@ -280,20 +280,19 @@ set, rotate and remove secrets. - **Private (default)** — the app's `NebariApp` creates a gateway `SecurityPolicy`: users are redirected to Keycloak and must sign in. The app itself never sees or handles auth. -- **Private with `access.groups` and/or `access.users`** — only those users get in: a - member of **any** listed group **or** a listed username (`preferred_username`, - case-insensitive). Everyone else who signs in gets **403**. +- **Private with `access.groups`** — only members of **any** listed Keycloak group get in. + Everyone else who signs in gets **403**. Access is granted by group only: to let a + particular person in, add them to one of the app's groups in Keycloak. -:::note[How groups and users are enforced] +:::note[How groups are enforced] The gateway's `SecurityPolicy` only signs users in ([nebari-operator#153](https://github.com/nebari-dev/nebari-operator/issues/153)), so the -pack enforces the lists in the app pod. For an App that lists groups or users, the +pack enforces the groups in the app pod. For an App that lists groups, the operator sets `forwardAccessToken: true` on its `NebariApp`, so the gateway passes the user's Keycloak access token to the pod as `Authorization: Bearer …`, and adds the **apps-authz** sidecar on port 8080 in front of the app container (which moves to 8081). The sidecar verifies the token against the realm's keys (signature, expiry, issuer, and -that it was issued to the app's own client) and checks its `groups` and -`preferred_username` claims. A request with no token gets `401`; every other refusal is +that it was issued to the app's own client) and checks its `groups` claim. A request with no token gets `401`; every other refusal is `403`. - **Group changes apply when the user's access token refreshes**: Keycloak's access-token @@ -302,9 +301,8 @@ that it was issued to the app's own client) and checks its `groups` and - Needs nebari-operator `%NEBARI_OPERATOR_VERSION%` or later (for `forwardAccessToken`) and the chart's `keycloak.url` or `keycloak.internalUrl`; without them the App fails validation instead of running open. -- `access.users` is not sent to the `NebariApp` (it has no per-user rule), so the - landing page cannot hide a tile from users outside the list. That is why the launch - form's landing-page switch is locked for now (see below). +- The groups are also sent to the `NebariApp` as `auth.groups`, so a landing-page tile + is shown only to their members. - With the chart's `apps.networkPolicy.enabled`, only the gateway can reach the pod, and only on 8080. Static apps listen on loopback behind the sidecar; a Python task that binds `0.0.0.0` is reachable on 8081 from inside the cluster unless that policy is on. @@ -322,7 +320,7 @@ app shows up in their Apps list and under **Mine**. - **Only the owner and admins** (`api.adminGroups`) can change the manager list. A manager's edit may send the list unchanged or leave it out; changing it is refused with `403`. - **Managing is not opening.** Managers of a restricted app still need to be in its - `access.groups` or `access.users` to open it; add them there too if they should. + `access.groups` to open it; add them to one of those groups if they should. - Group membership comes from the caller's verified access token, so adding someone to a manager group applies when their token refreshes. - Ownership itself cannot be transferred yet. @@ -350,10 +348,8 @@ cluster default with the chart value `apps.landingPage.enabled: true`, after whi *out* with `enabled: false`. Either way the app is reachable at its URL and listed in the Apps UI. -The launch form's "Show on the landing page" option is **locked** for now: the landing -page can hide a tile only from users outside an app's groups, not outside its allowed -users, so the form registers new apps with `enabled: false` and an edit keeps whatever the -App already has. Register a tile through the API, the MCP tool or `kubectl` if you need +The launch form's "Show on the landing page" option is **locked** for now: the form +registers new apps with `enabled: false` and an edit keeps whatever the App already has. Register a tile through the API, the MCP tool or `kubectl` if you need one. ## Idle apps diff --git a/docs/src/content/docs/troubleshooting.md b/docs/src/content/docs/troubleshooting.md index 2f1e757..1177e3e 100644 --- a/docs/src/content/docs/troubleshooting.md +++ b/docs/src/content/docs/troubleshooting.md @@ -62,10 +62,10 @@ kubectl -n logs deploy/app- -c app # the app itself `pixi.lock` makes the install `--locked` and faster. - **Python app is `Running` but never ready.** The task must listen on `0.0.0.0:$PORT` (the pod gets `PORT=8080`). A server bound to `127.0.0.1` fails the readiness probe. An App - that lists `access.groups` or `access.users` gets `PORT=8081`, because the authz sidecar + that lists `access.groups` (or can idle) gets `PORT=8081`, because the authz sidecar takes 8080: a task that hard-codes 8080 then collides with the sidecar. - **A restricted app returns `403` to someone who should get in.** The sidecar's log says - why (`kubectl logs deploy/app- -c authz`): not in an allowed group or user list, + why (`kubectl logs deploy/app- -c authz`): not in an allowed group, token issued to another client, issuer not accepted (check `keycloak.url` and `keycloak.internalUrl`), or expired. Group changes reach the token on its next refresh (5 minutes by default). A `401` means the gateway forwarded no token: the `NebariApp` has diff --git a/docs/src/content/docs/upgrading.md b/docs/src/content/docs/upgrading.md index ad1a249..8bc987e 100644 --- a/docs/src/content/docs/upgrading.md +++ b/docs/src/content/docs/upgrading.md @@ -69,7 +69,9 @@ helm rollback nebari-apps -n nebari-apps `helm rollback` restores the workloads, RBAC and values, but not the CRD. CRD changes within `v1alpha1` are additive ([versioning policy](/app-crd-reference/#versioning-and-deprecation), -checked in CI), so the newer CRD keeps working with the older operator. +checked in CI), so the newer CRD keeps working with the older operator. The exception is +[group-only access](#upgrading-to-group-only-access), which removed `access.users`: rolling +back does not restore the lists the newer CRD pruned. If Apps were lost, re-apply the backup: ```bash @@ -80,6 +82,34 @@ Status fields in the backup are ignored; the operator recomputes them. ## Version notes +### Upgrading to group-only access + +**Breaking:** Apps restrict who may open them by Keycloak group only. `spec.access.users` +(the per-user allow list) is removed from the CRD, the API, the MCP `launch_app` tool, the +UI and the apps-authz sidecar (`AUTHZ_USERS`, `AUTHZ_USERS_CLAIM`). +`spec.access.managers.users`, the users who may *manage* an App, is unchanged. + +- **An App that restricted only by users becomes open to every signed-in user.** The new + CRD's structural schema prunes `access.users` from stored Apps, so an App that listed + users and no groups is left with neither list: the operator drops its authz sidecar and + any user who can sign in to the realm gets in. An App that listed both keeps its groups, + and the people it let in only by username are refused (`403`). +- **Before upgrading, find the Apps that list users:** + + ```bash + kubectl get apps.apps.nebari.dev -A -o json | jq -r '.items[] | select(.spec.access.users // [] | length > 0) | "\(.metadata.namespace)/\(.metadata.name)"' + ``` + + For each one, put those users in a Keycloak group (an existing one, or a new group per + App) and add the group to the App's `access.groups` before you apply the new CRD. Group + membership reaches users' tokens on their next refresh (5 minutes by default). Take the + [backup](#before-you-upgrade) after this step too: a rollback does not bring the pruned + lists back, the backup does. +- **Clients that still send `access.users`** get a `422` from the API (and from the upload + route's manifest) instead of an App that is silently open to more people. Update scripts, + `nebari-app.yaml` manifests and GitOps manifests to use groups; for GitOps and `kubectl` + the apiserver prunes the field without an error, so check those by hand. + ### Upgrading to container images - **Re-apply the CRD first.** It adds `source.type: image` and `source.image`, plus CEL rules diff --git a/examples/python-app/pixi.toml b/examples/python-app/pixi.toml index 339ba93..f2f2c77 100644 --- a/examples/python-app/pixi.toml +++ b/examples/python-app/pixi.toml @@ -9,5 +9,5 @@ streamlit = ">=1.44" [tasks] # The platform runs `pixi run start` and expects a server on 0.0.0.0:$PORT -# (8080, or 8081 when access.groups/users is set). Locally: PORT=8080 pixi run start +# (8080, or 8081 when access.groups is set or the app can idle). Locally: PORT=8080 pixi run start start = "streamlit run app.py --server.address 0.0.0.0 --server.port $PORT --server.headless true" diff --git a/mcp/src/nebari_apps_mcp/server.py b/mcp/src/nebari_apps_mcp/server.py index f8703cf..010be7b 100644 --- a/mcp/src/nebari_apps_mcp/server.py +++ b/mcp/src/nebari_apps_mcp/server.py @@ -417,16 +417,7 @@ async def launch_app( Field( description=( "Keycloak groups allowed to open the app (private apps; enforced in the app pod, " - "everyone else gets 403). Empty, with no users = any signed-in user" - ) - ), - ] = None, - users: Annotated[ - list[str] | None, - Field( - description=( - "Keycloak usernames allowed to open the app, in addition to groups " - "(a listed user OR a member of a listed group gets in)" + "everyone else gets 403). Empty = any signed-in user" ) ), ] = None, @@ -502,7 +493,7 @@ async def launch_app( image["runAsUser"] = image_run_as_user source["image"] = image - access: dict[str, Any] = {"public": public, "groups": groups or [], "users": users or [], "subdomain": subdomain} + access: dict[str, Any] = {"public": public, "groups": groups or [], "subdomain": subdomain} if manager_users is not None or manager_groups is not None: access["managers"] = {"users": manager_users or [], "groups": manager_groups or []} body = { @@ -606,7 +597,7 @@ async def update_app( patch: Annotated[ dict[str, Any], Field( - description="Fields to change: any of displayName, description, source, runtime, access ({public, groups, users, subdomain, managers: {users, groups}}; send the whole object, an empty list clears it; omit managers to keep them, only the owner or an admin may change them), landingPage ({enabled: bool}; {} = cluster default), secrets ({NAME: value} sets or rotates a credential injected from the app's Secret, {NAME: null} removes it, names not sent are kept; write-only, never returned; never put credentials in runtime.env)" + description="Fields to change: any of displayName, description, source, runtime, access ({public, groups, subdomain, managers: {users, groups}}; send the whole object, an empty list clears it; omit managers to keep them, only the owner or an admin may change them), landingPage ({enabled: bool}; {} = cluster default), secrets ({NAME: value} sets or rotates a credential injected from the app's Secret, {NAME: null} removes it, names not sent are kept; write-only, never returned; never put credentials in runtime.env)" ), ], ) -> Any: @@ -769,7 +760,7 @@ async def apps_resource(ctx: Context) -> Any: f"{SCHEME}://apps/{{namespace}}/{{name}}", name="app", description="One app's full spec and status, as get_app returns it: source, runtime (secret names " - "only, never values), access (public, groups, users, managers), URL, conditions and your role. " + "only, never values), access (public, groups, managers), URL, conditions and your role. " "Inline files are fenced untrusted content.", mime_type=JSON, annotations=READ_ONLY_RESOURCE, @@ -895,7 +886,7 @@ def launch_from_manifest( source.type image -> image_reference, image_port, image_command, image_args, image_run_as_user; runtime.env -> env as {{NAME: value}}; runtime.replicas, resources, pixiTask, keepAlive, idleTimeoutMinutes -> \ replicas, resources, pixi_task, keep_alive, idle_timeout_minutes; - access.public, groups, users, subdomain -> public, groups, users, subdomain; access.managers.users and .groups -> \ + access.public, groups, subdomain -> public, groups, subdomain; access.managers.users and .groups -> \ manager_users, manager_groups (omit both when the manifest has no managers, so a re-launch keeps the current list); landingPage.enabled -> landing_page (omit when the manifest omits it). 2. Read {SCHEME}://cluster (tool: describe_cluster). Check that the namespace is listed and the source type is in \ @@ -914,8 +905,8 @@ def launch_from_manifest( @mcp.prompt( name="review_access", title="Review an app's access", - description="Review who can open and who can manage an app: public or private, restricted groups and " - "users, managers, and your own role.", + description="Review who can open and who can manage an app: public or private, restricted groups, " + "managers, and your own role.", ) def review_access( namespace: Annotated[str, Field(description="The app's namespace")], @@ -929,22 +920,21 @@ def review_access( 1. Who can open it (access): - public: true means anyone with the URL, with no sign-in. Flag this if the app looks internal or lists \ secretKeys. - - public: false with empty groups and users means any user who can sign in to the cluster's Keycloak. - - public: false with groups and/or users is restricted: a member of any listed group OR a listed username \ -gets in, everyone else gets 403. Group changes take effect when the user's access token refreshes (minutes). - - groups and users have no effect while public is true; point out that combination. + - public: false with empty groups means any user who can sign in to the cluster's Keycloak. + - public: false with groups is restricted: a member of any listed group gets in, everyone else gets 403. Group changes take effect when the user's access token refreshes (minutes). + - groups have no effect while public is true; point out that combination. 2. Who can manage it: the owner, every user in access.managers.users, members of access.managers.groups, \ and cluster admins. Managers can edit, start, stop, restart and delete the app and read its logs, but only the \ owner or an admin can change the manager list. Being a manager does not let someone open a restricted app. 3. Your role (the role field): owner, manager or admin. If you are a manager, say that you cannot change the \ managers. -4. Other exposure: landingPage.enabled (a tile on the Nebari landing page; it honors groups but not users, so a \ -users-only app shows its tile to everyone) and runtime.env values (visible to anyone who can read the app: \ +4. Other exposure: landingPage.enabled (a tile on the Nebari landing page; when the \ +app lists groups, only their members see it) and runtime.env values (visible to anyone who can read the app: \ flag anything that looks like a credential and suggest moving it to secrets). runtime.secretKeys shows names \ only; values are never readable. Finish with concrete recommendations. To change anything, prepare an update_app patch with the whole access \ -object ({{public, groups, users, subdomain, managers}}) and apply it only after the user confirms.""" +object ({{public, groups, subdomain, managers}}) and apply it only after the user confirms.""" def main() -> None: diff --git a/mcp/tests/test_resources.py b/mcp/tests/test_resources.py index 922ef49..763a673 100644 --- a/mcp/tests/test_resources.py +++ b/mcp/tests/test_resources.py @@ -242,7 +242,6 @@ async def test_review_access_prompt_covers_managers_and_restrictions(): for needle in ( "public: true", "groups", - "users", "access.managers", "only the", "role", diff --git a/mcp/tests/test_tools.py b/mcp/tests/test_tools.py index 5b05c5e..06e0e3f 100644 --- a/mcp/tests/test_tools.py +++ b/mcp/tests/test_tools.py @@ -115,23 +115,22 @@ async def test_launch_git_source(store): assert {"name": "LOG_LEVEL", "value": "debug"} in cr["spec"]["runtime"]["env"] -async def test_launch_and_update_access_users(store): - await call("launch_app", {**LAUNCH_ARGS, "public": False, "groups": ["analysts"], "users": ["alice"]}) +async def test_launch_and_update_access_groups(store): + await call("launch_app", {**LAUNCH_ARGS, "public": False, "groups": ["analysts"]}) access = store.apps[("apps", "docs-site")]["spec"]["access"] assert access["groups"] == ["analysts"] - assert access["users"] == ["alice"] + assert "users" not in access await call( "update_app", { "namespace": "apps", "name": "docs-site", - "patch": {"access": {"public": False, "users": ["bob"], "subdomain": "docs-site"}}, + "patch": {"access": {"public": False, "groups": ["team"], "subdomain": "docs-site"}}, }, ) access = store.apps[("apps", "docs-site")]["spec"]["access"] - assert access["users"] == ["bob"] - assert access["groups"] == ["analysts"] # a field the patch did not send is kept + assert access["groups"] == ["team"] async def test_launch_and_update_managers(store): diff --git a/operator/api/v1alpha1/app_types.go b/operator/api/v1alpha1/app_types.go index ef68539..9e9184e 100644 --- a/operator/api/v1alpha1/app_types.go +++ b/operator/api/v1alpha1/app_types.go @@ -175,7 +175,7 @@ type AppRuntime struct { // environment from the app source and executes `pixi run `. // Not allowed with source type image, which runs its own server. // The task must start a server on $PORT: 8080, or 8081 when access.groups - // or access.users restricts the app (the authz sidecar takes 8080). + // restricts the app or it can idle (the authz sidecar takes 8080). // +kubebuilder:validation:MaxLength=64 // +optional PixiTask string `json:"pixiTask,omitempty"` @@ -190,23 +190,17 @@ type AppAccess struct { // Groups are the Keycloak/OIDC groups allowed to open a private app. // Enforced in the app pod by the authz sidecar, which checks the access // token the gateway forwards: everyone else gets 403. A user in any - // listed group OR listed in users is let in. Empty (with no users) allows - // any signed-in realm user. Changes apply when the user's access token + // listed group is let in. Empty allows any signed-in realm user. Also + // forwarded to the NebariApp, where it sets landing-page tile visibility. + // Changes apply when the user's access token // refreshes (Keycloak's access token lifespan, 5 minutes by default). // +optional Groups []string `json:"groups,omitempty"` - // Users are Keycloak usernames (preferred_username, case-insensitive) - // allowed to open a private app, in addition to the members of groups. - // Enforced in the app pod like groups. Not forwarded to the NebariApp, - // so they do not affect landing-page tile visibility. - // +optional - Users []string `json:"users,omitempty"` - // Managers may manage the app (edit, start, stop, restart, delete, read // logs and metrics) through apps-api, the UI and the MCP server, like its // owner. Only the owner and api.adminGroups members may change this list. - // It does not grant opening the app: that is still public, groups and users. + // It does not grant opening the app: that is still public and groups. // +optional Managers *AppManagers `json:"managers,omitempty"` diff --git a/operator/api/v1alpha1/zz_generated.deepcopy.go b/operator/api/v1alpha1/zz_generated.deepcopy.go index 22720a2..95074a1 100644 --- a/operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/operator/api/v1alpha1/zz_generated.deepcopy.go @@ -45,11 +45,6 @@ func (in *AppAccess) DeepCopyInto(out *AppAccess) { *out = make([]string, len(*in)) copy(*out, *in) } - if in.Users != nil { - in, out := &in.Users, &out.Users - *out = make([]string, len(*in)) - copy(*out, *in) - } if in.Managers != nil { in, out := &in.Managers, &out.Managers *out = new(AppManagers) diff --git a/operator/cmd/main.go b/operator/cmd/main.go index ef05ec1..96ba330 100644 --- a/operator/cmd/main.go +++ b/operator/cmd/main.go @@ -53,7 +53,7 @@ func main() { flag.StringVar(&cfg.PythonImage, "python-image", envOr("PYTHON_IMAGE", "ghcr.io/prefix-dev/pixi:0.68.1-noble"), "Image running Python/pixi apps (runtime.pixiTask); must provide pixi and run as non-root.") flag.StringVar(&cfg.AuthzImage, "authz-image", envOr("AUTHZ_IMAGE", ""), - "apps-authz sidecar image, added to Apps that restrict access.groups or access.users (required for those Apps).") + "apps-authz sidecar image, added to Apps that restrict access.groups (required for those Apps).") var authzPullPolicy string flag.StringVar(&authzPullPolicy, "authz-image-pull-policy", envOr("AUTHZ_IMAGE_PULL_POLICY", ""), "Pull policy for the authz sidecar image (Always|IfNotPresent|Never; empty: the Kubernetes default).") diff --git a/operator/internal/controller/app_controller.go b/operator/internal/controller/app_controller.go index d898353..63fc212 100644 --- a/operator/internal/controller/app_controller.go +++ b/operator/internal/controller/app_controller.go @@ -59,7 +59,7 @@ type OperatorConfig struct { StaticResources corev1.ResourceRequirements PythonResources corev1.ResourceRequirements // AuthzImage is the apps-authz sidecar added in front of Apps that - // restrict access to groups or users (authz.go). + // restrict access to groups (authz.go). AuthzImage string // AuthzImagePullPolicy is the sidecar's pull policy (empty: the // Kubernetes default). diff --git a/operator/internal/controller/authz.go b/operator/internal/controller/authz.go index b67a84d..cd9ffaf 100644 --- a/operator/internal/controller/authz.go +++ b/operator/internal/controller/authz.go @@ -13,7 +13,7 @@ import ( ) // The gateway's SecurityPolicy only signs users in (nebari-operator #153), so -// an App that lists access.groups or access.users gets the apps-authz +// an App that lists access.groups gets the apps-authz // sidecar: it takes AppPort, checks the access token the gateway forwards // (NebariApp auth.forwardAccessToken) and proxies allowed requests to the // app container on AppInternalPort. Unrestricted Apps get no sidecar and an @@ -43,14 +43,14 @@ var DefaultAuthzResources = corev1.ResourceRequirements{ } // restricted reports whether an App limits who may open it beyond "any -// signed-in user": a private App with groups or users. +// signed-in user": a private App with groups. func restricted(app *appsv1alpha1.App) bool { a := app.Spec.Access - return !a.Public && (len(a.Groups) > 0 || len(a.Users) > 0) + return !a.Public && len(a.Groups) > 0 } // hasSidecar reports whether the App's pod runs the apps-authz sidecar: -// restricted Apps (it enforces groups and users), Apps that can idle (in +// restricted Apps (it enforces groups), Apps that can idle (in // passthrough mode it only records activity for the idle check) and image // Apps (it gives their own port the pack's one front port, AppPort). func (c OperatorConfig) hasSidecar(app *appsv1alpha1.App) bool { @@ -128,16 +128,14 @@ func (c OperatorConfig) checkAuthz(app *appsv1alpha1.App) error { return nil } if c.AuthzImage == "" { - return fmt.Errorf("access.groups/access.users need the authz sidecar, but the operator has no --authz-image (chart authz.image)") + return fmt.Errorf("access.groups need the authz sidecar, but the operator has no --authz-image (chart authz.image)") } if len(c.authzIssuers()) == 0 { - return fmt.Errorf("access.groups/access.users need the realm's URL, but the operator has no --keycloak-url or --keycloak-internal-url (chart keycloak.url, keycloak.internalUrl)") + return fmt.Errorf("access.groups need the realm's URL, but the operator has no --keycloak-url or --keycloak-internal-url (chart keycloak.url, keycloak.internalUrl)") } - for _, list := range [][]string{app.Spec.Access.Groups, app.Spec.Access.Users} { - for _, v := range list { - if strings.TrimSpace(v) == "" || strings.Contains(v, ",") { - return fmt.Errorf("access.groups and access.users entries must be non-empty and must not contain ','") - } + for _, v := range app.Spec.Access.Groups { + if strings.TrimSpace(v) == "" || strings.Contains(v, ",") { + return fmt.Errorf("access.groups entries must be non-empty and must not contain ','") } } return nil @@ -183,13 +181,8 @@ func (r *AppReconciler) addAuthzSidecar(app *appsv1alpha1.App, pod *corev1.PodSp corev1.EnvVar{Name: "AUTHZ_ISSUERS", Value: strings.Join(r.Config.authzIssuers(), ",")}, corev1.EnvVar{Name: "AUTHZ_JWKS_URL", Value: r.Config.authzJWKSURL()}, corev1.EnvVar{Name: "AUTHZ_CLIENT_ID", Value: oidcClientID(app)}, + corev1.EnvVar{Name: "AUTHZ_GROUPS", Value: strings.Join(app.Spec.Access.Groups, ",")}, ) - if g := app.Spec.Access.Groups; len(g) > 0 { - env = append(env, corev1.EnvVar{Name: "AUTHZ_GROUPS", Value: strings.Join(g, ",")}) - } - if u := app.Spec.Access.Users; len(u) > 0 { - env = append(env, corev1.EnvVar{Name: "AUTHZ_USERS", Value: strings.Join(u, ",")}) - } } else { env = append(env, corev1.EnvVar{Name: "AUTHZ_PASSTHROUGH", Value: "true"}) } diff --git a/operator/internal/controller/authz_test.go b/operator/internal/controller/authz_test.go index 16bd239..11b94ef 100644 --- a/operator/internal/controller/authz_test.go +++ b/operator/internal/controller/authz_test.go @@ -16,10 +16,9 @@ import ( const testAuthzImage = "ghcr.io/nebari-dev/apps-pack/apps-authz:test" -func restrictedApp(app *appsv1alpha1.App, groups, users []string) *appsv1alpha1.App { +func restrictedApp(app *appsv1alpha1.App, groups []string) *appsv1alpha1.App { app.Spec.Access.Public = false app.Spec.Access.Groups = groups - app.Spec.Access.Users = users return app } @@ -55,7 +54,7 @@ func deployed(t *testing.T, app *appsv1alpha1.App) (*appsv1.Deployment, *corev1. } func TestAuthzSidecarStaticApp(t *testing.T) { - app := restrictedApp(inlineApp("apps"), []string{"analysts", "admin"}, nil) + app := restrictedApp(inlineApp("apps"), []string{"analysts", "admin"}) app.Name = "web-terminal" d, svc, cm, _ := deployed(t, app) pod := d.Spec.Template.Spec @@ -124,7 +123,7 @@ func TestAuthzSidecarStaticApp(t *testing.T) { } func TestAuthzSidecarPixiApp(t *testing.T) { - app := restrictedApp(pixiApp("apps"), nil, []string{"alice"}) + app := restrictedApp(pixiApp("apps"), []string{"analysts"}) d, _, _, _ := deployed(t, app) pod := d.Spec.Template.Spec if len(pod.Containers) != 2 { @@ -143,11 +142,8 @@ func TestAuthzSidecarPixiApp(t *testing.T) { t.Error("the cold-start budget of the startup probe should be kept") } env := envOf(pod.Containers[1]) - if env["AUTHZ_USERS"] != "alice" { - t.Errorf("AUTHZ_USERS = %q", env["AUTHZ_USERS"]) - } - if _, has := env["AUTHZ_GROUPS"]; has { - t.Error("AUTHZ_GROUPS should be absent when the App lists only users") + if env["AUTHZ_GROUPS"] != "analysts" { + t.Errorf("AUTHZ_GROUPS = %q", env["AUTHZ_GROUPS"]) } } @@ -156,7 +152,6 @@ func TestAuthzSidecarPixiApp(t *testing.T) { func TestUnrestrictedAppsHaveNoSidecar(t *testing.T) { publicWithLists := inlineApp("team-a") publicWithLists.Spec.Access.Groups = []string{"x"} - publicWithLists.Spec.Access.Users = []string{"u"} private := inlineApp("team-a") private.Spec.Access.Public = false privatePixi := pixiApp("team-a") @@ -211,7 +206,7 @@ func TestAuthzMisconfigurationFailsValidation(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - app := restrictedApp(inlineApp("team-a"), tc.groups, nil) + app := restrictedApp(inlineApp("team-a"), tc.groups) r, c := newReconciler(t, managedNamespace("team-a"), app) tc.edit(&r.Config) reconcile(t, r, app) diff --git a/operator/internal/controller/envtest_test.go b/operator/internal/controller/envtest_test.go index a422e52..3466672 100644 --- a/operator/internal/controller/envtest_test.go +++ b/operator/internal/controller/envtest_test.go @@ -319,7 +319,6 @@ func TestEnvtestRestrictedAppGetsAuthzSidecar(t *testing.T) { app.Generation = 0 app.Spec.Access.Public = false app.Spec.Access.Groups = []string{"analysts"} - app.Spec.Access.Users = []string{"alice"} if err := c.Create(ctx, app); err != nil { t.Fatal(err) } @@ -353,7 +352,7 @@ func TestEnvtestRestrictedAppGetsAuthzSidecar(t *testing.T) { if err := c.Get(ctx, key, edit); err != nil { t.Fatal(err) } - edit.Spec.Access.Groups, edit.Spec.Access.Users = nil, nil + edit.Spec.Access.Groups = nil if err := c.Update(ctx, edit); err != nil { t.Fatal(err) } diff --git a/operator/internal/controller/image_test.go b/operator/internal/controller/image_test.go index e00660e..ba03fa6 100644 --- a/operator/internal/controller/image_test.go +++ b/operator/internal/controller/image_test.go @@ -126,7 +126,7 @@ func TestReconcileImageApp(t *testing.T) { func TestImageAppRestrictedAndDigest(t *testing.T) { digest := "sha256:" + strings.Repeat("b", 64) - app := restrictedApp(imageApp("apps", "nginx@"+digest), []string{"analysts"}, nil) + app := restrictedApp(imageApp("apps", "nginx@"+digest), []string{"analysts"}) app.Spec.Source.Image.RunAsUser = ptr.To(int64(1000)) r, c, _ := imageReconciler(t, managedNamespace("apps"), app) reconcile(t, r, app) diff --git a/operator/internal/controller/nebariapp_contract_test.go b/operator/internal/controller/nebariapp_contract_test.go index 4cb59c9..554488e 100644 --- a/operator/internal/controller/nebariapp_contract_test.go +++ b/operator/internal/controller/nebariapp_contract_test.go @@ -123,20 +123,18 @@ func TestNebariAppContractPrivateApp(t *testing.T) { } } -// access.users is enforced only by the sidecar: the token is forwarded, but -// NebariApp has no per-user field, so neither users nor groups are emitted. -func TestNebariAppContractUsersOnly(t *testing.T) { +// A group-restricted App forwards the token for the sidecar and never emits +// an auth.users field. +func TestNebariAppContractGroupsOnly(t *testing.T) { app := inlineApp("team-a") - app.Spec.Access = appsv1alpha1.AppAccess{Users: []string{"alice"}, Subdomain: "docs-site"} + app.Spec.Access = appsv1alpha1.AppAccess{Groups: []string{"analysts"}, Subdomain: "docs-site"} auth, _, _ := unstructured.NestedMap(testReconciler().buildNebariApp(app).Object, "spec", "auth") if auth["enabled"] != true || auth["forwardAccessToken"] != true || auth["enforceAtGateway"] != true { t.Errorf("auth = %+v, want sign-in with the token forwarded", auth) } - for _, k := range []string{"users", "groups"} { - if _, has := auth[k]; has { - t.Errorf("auth.%s should not be emitted for a users-only App", k) - } + if _, has := auth["users"]; has { + t.Error("auth.users should never be emitted") } } diff --git a/operator/internal/controller/routing.go b/operator/internal/controller/routing.go index d83b49b..20a1f58 100644 --- a/operator/internal/controller/routing.go +++ b/operator/internal/controller/routing.go @@ -45,10 +45,10 @@ func (r *AppReconciler) buildNebariApp(app *appsv1alpha1.App) *unstructured.Unst } auth["groups"] = groups } - // access.users is deliberately not forwarded: NebariApp has no - // per-user rule, and its groups only drive landing-tile visibility. + // NebariApp's groups only drive landing-tile visibility; the authz + // sidecar is what enforces them. if restricted(app) { - // The authz sidecar (authz.go) enforces groups and users on the + // The authz sidecar (authz.go) enforces the groups on the // access token the gateway forwards. forwardAccessToken requires // enforceAtGateway (a CEL rule on the NebariApp CRD), which // defaults to true; it is set explicitly so the rule holds. diff --git a/operator/internal/controller/workload.go b/operator/internal/controller/workload.go index 8c80755..1c00785 100644 --- a/operator/internal/controller/workload.go +++ b/operator/internal/controller/workload.go @@ -13,9 +13,9 @@ import ( const ( // AppPort is the port the Service and the chart's NetworkPolicy target. - // The app container listens on it, unless the App restricts access to - // groups or users: then the authz sidecar takes it and the app moves to - // AppInternalPort (authz.go). Sign-in is enforced at the gateway by the + // The app container listens on it, unless the App runs the authz sidecar + // (it restricts access to groups, can idle, or runs an image): then the + // sidecar takes it and the app moves to AppInternalPort (authz.go). Sign-in is enforced at the gateway by the // NebariApp SecurityPolicy, so apps serve plain HTTP here. AppPort = 8080 diff --git a/pack-metadata.yaml b/pack-metadata.yaml index 9308601..7cc4370 100644 --- a/pack-metadata.yaml +++ b/pack-metadata.yaml @@ -17,6 +17,6 @@ scope: last_promoted_pr: 33 # last_promoted_at: YYYY-MM-DD demo_notes: >- - access.groups/users are enforced in the app pod by an authz sidecar (403 - for everyone else); a private app with neither is "any signed-in user". + access.groups is enforced in the app pod by an authz sidecar (403 for + everyone else); a private app with no groups is "any signed-in user". Any authenticated user can manage every app. Launched apps are not on the landing page unless opted in. diff --git a/skill/new-nebari-app/SKILL.md b/skill/new-nebari-app/SKILL.md index 024f310..71d4801 100644 --- a/skill/new-nebari-app/SKILL.md +++ b/skill/new-nebari-app/SKILL.md @@ -14,7 +14,7 @@ and maps the manifest onto the `nebari-apps` MCP server's `launch_app` tool so - **Python app (pixi)**: a source tree with a `pixi.toml`; the platform runs `pixi install` then `pixi run ` (set in `runtime.pixiTask`), and the task must start a server on **0.0.0.0:$PORT** (`PORT` is injected: 8080, or 8081 when the app - lists allowed groups or users). + lists allowed groups or can idle). A third kind needs no scaffolding: an app that already ships as a **container image**. When `describe_cluster` lists `image` in `sourceTypes`, write a manifest with `source.type: image` diff --git a/skill/new-nebari-app/assets/python/pixi.toml b/skill/new-nebari-app/assets/python/pixi.toml index 7b53d36..95da1f8 100644 --- a/skill/new-nebari-app/assets/python/pixi.toml +++ b/skill/new-nebari-app/assets/python/pixi.toml @@ -9,6 +9,6 @@ fastapi = "*" uvicorn = "*" [tasks] -# The platform injects PORT (8080, or 8081 when access.groups/users is set) +# The platform injects PORT (8080, or 8081 when access.groups is set or the app can idle) # and expects the task to serve on 0.0.0.0:$PORT. Locally: PORT=8080 pixi run start start = "uvicorn app:app --host 0.0.0.0 --port $PORT" diff --git a/skill/new-nebari-app/references/manifest.md b/skill/new-nebari-app/references/manifest.md index e97f09e..61f5fdf 100644 --- a/skill/new-nebari-app/references/manifest.md +++ b/skill/new-nebari-app/references/manifest.md @@ -52,7 +52,7 @@ runtime: # optional # Python/pixi apps only: the pixi task that launches the app. When set, the # platform runs `pixi install` + `pixi run ` instead of serving the # source with nginx. The task must serve on 0.0.0.0:$PORT (PORT is injected: - # 8080, or 8081 when groups or users are set or the app can idle). + # 8080, or 8081 when groups are set or the app can idle). pixiTask: start # Scale to zero after this many minutes without a request (5-43200); the next # visit wakes it. Omit for the cluster default (often off). @@ -63,8 +63,8 @@ runtime: # optional access: public: false # true = anonymous; false = Keycloak SSO at the gateway - groups: ["analytics"] # allowed groups (enforced in the app pod; 403 for others) - users: ["alice"] # allowed usernames, in addition to the groups + groups: ["analytics"] # allowed groups (enforced in the app pod; 403 for others). + # Access is by group only: no per-user allow list managers: # optional: who else may manage (edit, stop, delete) the app; users: ["bob"] # not who may open it. Only the owner or an admin can change it groups: ["data-team"] @@ -72,7 +72,7 @@ access: landingPage: # optional enabled: true # tile on the Nebari landing page; omit = cluster default (off). - # The tile honors groups only: users-only apps show it to everyone + # Only members of the app's groups see the tile ``` ## Manifest → MCP `launch_app` mapping @@ -92,7 +92,7 @@ landingPage: # optional | `runtime.pixiTask` | `pixi_task` | | `runtime.keepAlive` | `keep_alive` | | `runtime.idleTimeoutMinutes` | `idle_timeout_minutes` (omit when the manifest omits it) | -| `access.public` / `access.groups` / `access.users` | `public` / `groups` / `users` | +| `access.public` / `access.groups` | `public` / `groups` | | `access.managers.users` / `access.managers.groups` | `manager_users` / `manager_groups` (omit both when the manifest has no `managers`, so a re-launch keeps the current list) | | `access.subdomain` | `subdomain` | | `landingPage.enabled` | `landing_page` (omit when the manifest omits it) | @@ -141,7 +141,7 @@ content files and the manifest. copied into a writable workspace, `pixi install` resolves the environment (use a committed `pixi.lock` for reproducible, faster starts), and `pixi run ` launches the app. The task must start a server on **0.0.0.0:$PORT** — `PORT` is injected (8080, or -8081 when `access.groups`/`access.users` or idling puts the authz sidecar on 8080), so use +8081 when `access.groups` or idling puts the authz sidecar on 8080), so use `--port $PORT` and run `PORT=8080 pixi run start` locally (pixi tasks do not support `${PORT:-8080}`). No Dockerfile needed. diff --git a/ui/e2e/app-detail.spec.ts b/ui/e2e/app-detail.spec.ts index 8e04201..bf278b9 100644 --- a/ui/e2e/app-detail.spec.ts +++ b/ui/e2e/app-detail.spec.ts @@ -31,9 +31,7 @@ test.describe("app detail", () => { ).toBeVisible(); await expect(page.getByText("of 1 ready", { exact: true })).toBeVisible(); await expect( - page.getByText( - "Keycloak SSO at the gateway · restricted to groups: analytics · users: grace", - ), + page.getByText("Keycloak SSO at the gateway · restricted to groups: analytics"), ).toBeVisible(); await expect(page.getByRole("link", { name: "Open" })).toHaveAttribute( "href", diff --git a/ui/e2e/data.ts b/ui/e2e/data.ts index efb24f7..4fa9a88 100644 --- a/ui/e2e/data.ts +++ b/ui/e2e/data.ts @@ -98,7 +98,6 @@ export const DOCS_SITE: App = { access: { public: false, groups: ["analytics"], - users: ["grace"], managers: { users: ["dave"], groups: ["docs-team"] }, subdomain: "docs-site", }, diff --git a/ui/e2e/launch.spec.ts b/ui/e2e/launch.spec.ts index 6445cfc..1e23f59 100644 --- a/ui/e2e/launch.spec.ts +++ b/ui/e2e/launch.spec.ts @@ -29,7 +29,7 @@ test.describe("launch form", () => { namespace: "apps", displayName: "My Site", runtime: { replicas: 1, env: [], resources: null, pixiTask: "" }, - access: { public: false, groups: [], users: [], subdomain: "my-site" }, + access: { public: false, groups: [], subdomain: "my-site" }, landingPage: { enabled: false }, source: { type: "git", git: { url: "https://github.com/example/my-site", ref: "main" } }, }, @@ -91,15 +91,14 @@ test.describe("launch form", () => { ]); }); - test("restricts a private app to groups and users", async ({ page, api }) => { + test("restricts a private app to groups", async ({ page, api }) => { await openLaunchForm(page); await page.getByLabel("Name", { exact: true }).fill("team-site"); await page.getByLabel("Display name").fill("Team Site"); await page.getByRole("tab", { name: "Git" }).click(); await page.getByLabel("Repository URL").fill("https://github.com/example/team-site"); - await page.getByLabel("Allowed groups").fill("analysts, finance"); - await page.getByLabel("Allowed users").fill("alice , bob,"); + await page.getByLabel("Allowed groups").fill("analysts , finance,"); await page.getByLabel("Manager users").fill("dave"); await page.getByLabel("Manager groups").fill("web-team"); // The landing-page option is locked: a note, not a switch. @@ -109,9 +108,7 @@ test.describe("launch form", () => { await expect(page).toHaveURL("/apps/apps/team-site"); await expect( - page.getByText( - "Keycloak SSO at the gateway · restricted to groups: analysts, finance · users: alice, bob", - ), + page.getByText("Keycloak SSO at the gateway · restricted to groups: analysts, finance"), ).toBeVisible(); await expect(page.getByTestId("managers")).toHaveText( "Managed by users: dave · groups: web-team · admins", @@ -124,7 +121,6 @@ test.describe("launch form", () => { access: { public: false, groups: ["analysts", "finance"], - users: ["alice", "bob"], managers: { users: ["dave"], groups: ["web-team"] }, subdomain: "team-site", }, @@ -319,11 +315,11 @@ test.describe("launch form", () => { await page.keyboard.press("Tab"); const id = await focused.getAttribute("id"); if (id) visited.push(id); - if (id === "users") await page.keyboard.type("alice"); + if (id === "groups") await page.keyboard.type("analysts"); } await expect(launch).toBeFocused(); - expect(visited).toEqual(expect.arrayContaining(["groups", "users", "subdomain"])); - expect(visited.indexOf("users")).toBe(visited.indexOf("groups") + 1); + expect(visited).toEqual(expect.arrayContaining(["groups", "subdomain"])); + expect(visited.indexOf("subdomain")).toBe(visited.indexOf("groups") + 1); await page.keyboard.press("Enter"); await expect(page).toHaveURL("/apps/team-data/kb-site"); @@ -335,7 +331,7 @@ test.describe("launch form", () => { name: "kb-site", namespace: "team-data", displayName: "Keyboard Site", - access: { users: ["alice"] }, + access: { groups: ["analysts"] }, source: { type: "git", git: { url: "https://github.com/example/kb-site" } }, }, }, diff --git a/ui/src/components/app-form/sections.tsx b/ui/src/components/app-form/sections.tsx index 857ed81..eeb2ef2 100644 --- a/ui/src/components/app-form/sections.tsx +++ b/ui/src/components/app-form/sections.tsx @@ -825,7 +825,7 @@ export function RuntimeSection({ form }: { form: FormContext }) {

Python apps must serve on 0.0.0.0:$PORT (8080, or - 8081 when groups or users restrict the app or it can idle). + 8081 when groups restrict the app or it can idle).

@@ -836,7 +836,7 @@ export function RuntimeSection({ form }: { form: FormContext }) { /** Public or signed-in access, who may open it, the subdomain, and the landing-page tile. */ export function AccessSection({ form }: { form: FormContext }) { - const { isPublic, groups, users, subdomain, onLandingPage } = form.state; + const { isPublic, groups, subdomain, onLandingPage } = form.state; const { set, fieldProps, isEdit, appsDomain } = form; const row = "py-4 first:pt-0 last:pb-0"; return ( @@ -867,34 +867,20 @@ export function AccessSection({ form }: { form: FormContext }) { /> {!isPublic ? ( -
- + - set("groups", e.target.value)} - /> - - - set("users", e.target.value)} - /> - -
+ placeholder="comma-separated" + aria-describedby="groups-help" + value={groups} + onChange={(e) => set("groups", e.target.value)} + /> + ) : null} ["a", "b", "c"]: the comma-separated groups and users fields. */ +/** "a, b ,c," -> ["a", "b", "c"]: the comma-separated list fields. */ export function splitList(raw: string): string[] { return raw .split(",") @@ -445,7 +442,6 @@ export function buildPayload( access: { public: s.isPublic, groups: splitList(s.groups), - users: splitList(s.users), subdomain: initial?.access?.subdomain ?? s.subdomain, }, }; @@ -474,7 +470,6 @@ export function buildPayload( access: { public: s.isPublic, groups: splitList(s.groups), - users: splitList(s.users), subdomain: s.subdomain, ...(buildManagers(s) ? { managers: buildManagers(s) } : {}), }, diff --git a/ui/src/lib/access.ts b/ui/src/lib/access.ts index b84c4c2..23d828d 100644 --- a/ui/src/lib/access.ts +++ b/ui/src/lib/access.ts @@ -1,19 +1,14 @@ import type { App } from "@/lib/types"; /** - * Who can open the app: "Public", any signed-in user, or the groups and users - * the authz sidecar lets in. + * Who can open the app: "Public", any signed-in user, or the groups the + * authz sidecar lets in. */ export function accessLine(a: App): string { if (a.access?.public) return "Public - no authentication"; const groups = a.access?.groups ?? []; - const users = a.access?.users ?? []; - if (!groups.length && !users.length) return "Keycloak SSO at the gateway · any signed-in user"; - const parts = [ - groups.length ? `groups: ${groups.join(", ")}` : "", - users.length ? `users: ${users.join(", ")}` : "", - ].filter(Boolean); - return `Keycloak SSO at the gateway · restricted to ${parts.join(" · ")}`; + if (!groups.length) return "Keycloak SSO at the gateway · any signed-in user"; + return `Keycloak SSO at the gateway · restricted to groups: ${groups.join(", ")}`; } /** Owned or managed by the caller: what the Mine filter keeps. */ diff --git a/ui/src/lib/crd.gen.ts b/ui/src/lib/crd.gen.ts index 7ce6fda..0e6875a 100644 --- a/ui/src/lib/crd.gen.ts +++ b/ui/src/lib/crd.gen.ts @@ -16,8 +16,6 @@ export type AppAccess = { public?: boolean; /** Subdomain under the cluster domain: https://.. */ subdomain: string; - /** Users are Keycloak usernames (preferred_username, case-insensitive) allowed to open a private app, in addition to the members of groups. */ - users?: string[]; }; export type AppCondition = { diff --git a/ui/tests/components/app-form-state.test.ts b/ui/tests/components/app-form-state.test.ts index ac8a4cd..11639d3 100644 --- a/ui/tests/components/app-form-state.test.ts +++ b/ui/tests/components/app-form-state.test.ts @@ -137,12 +137,12 @@ describe("splitList", () => { }); describe("initialState", () => { - it("joins the App's groups and users for editing", () => { + it("joins the App's groups for editing", () => { const s = initialState( - { access: { public: false, groups: ["a", "b"], users: ["alice"], subdomain: "x" } } as App, + { access: { public: false, groups: ["a", "b"], subdomain: "x" } } as App, true, ); - expect([s.groups, s.users]).toEqual(["a, b", "alice"]); + expect(s.groups).toBe("a, b"); }); }); @@ -155,14 +155,6 @@ describe("accessLine", () => { { public: false, groups: ["a", "b"], subdomain: "x" }, "Keycloak SSO at the gateway · restricted to groups: a, b", ], - [ - { public: false, groups: ["a"], users: ["alice"], subdomain: "x" }, - "Keycloak SSO at the gateway · restricted to groups: a · users: alice", - ], - [ - { public: false, users: ["alice", "bob"], subdomain: "x" }, - "Keycloak SSO at the gateway · restricted to users: alice, bob", - ], ])("describes %o", (access, want) => { expect(accessLine(app(access))).toBe(want); }); diff --git a/ui/tests/components/app-form.test.tsx b/ui/tests/components/app-form.test.tsx index f0468c6..a3dd0a2 100644 --- a/ui/tests/components/app-form.test.tsx +++ b/ui/tests/components/app-form.test.tsx @@ -124,7 +124,7 @@ describe("AppForm (create)", () => { const details = summary.closest("details") as HTMLDetailsElement; expect(details).not.toHaveAttribute("open"); expect(details).toHaveTextContent( - "Python apps must serve on 0.0.0.0:$PORT (8080, or 8081 when groups or users restrict the app or it can idle).", + "Python apps must serve on 0.0.0.0:$PORT (8080, or 8081 when groups restrict the app or it can idle).", ); for (const label of [ "Replicas", @@ -163,13 +163,12 @@ describe("AppForm (create)", () => { expect(screen.getByRole("button", { name: "About PVC" })).toBeInTheDocument(); }); - it("submits an upload with the manifest, allowed groups and users, and no landing tile", async () => { + it("submits an upload with the manifest, allowed groups, and no landing tile", async () => { const { user, onSubmit, submit } = setup(); await user.type(screen.getByLabelText("Name"), "sales"); await user.type(screen.getByLabelText("Display name"), "Sales"); await user.type(screen.getByLabelText("Description"), "Quarterly numbers"); await user.type(screen.getByLabelText("Allowed groups"), "analytics, finance ,"); - await user.type(screen.getByLabelText("Allowed users"), " alice,bob "); await user.upload(screen.getByLabelText(/Drop a \.zip/), site()); await submit(); @@ -187,7 +186,6 @@ describe("AppForm (create)", () => { access: { public: false, groups: ["analytics", "finance"], - users: ["alice", "bob"], subdomain: "sales", }, // The landing-page switch is locked: new apps are registered off. @@ -195,14 +193,11 @@ describe("AppForm (create)", () => { }); }); - it("describes who the groups and users fields let in", () => { + it("describes who the groups field lets in", () => { setup(); expect(screen.getByLabelText("Allowed groups")).toHaveAccessibleDescription( /Only members of these Keycloak groups can open the app; everyone else gets 403/, ); - expect(screen.getByLabelText("Allowed users")).toHaveAccessibleDescription( - "Keycloak usernames who can open the app even if they are not in an allowed group.", - ); }); it("shows the locked landing-page option as a note, not a switch", () => { @@ -245,7 +240,7 @@ describe("AppForm (create)", () => { }, pixiTask: "", }); - expect(payload.body.access).toEqual({ public: true, groups: [], users: [], subdomain: "docs" }); + expect(payload.body.access).toEqual({ public: true, groups: [], subdomain: "docs" }); expect(screen.getByTestId("url-preview")).toHaveTextContent("https://docs.apps.example.ai"); }); @@ -465,7 +460,6 @@ describe("AppForm (edit)", () => { expect(payload.body.access).toEqual({ public: false, groups: ["analytics"], - users: [], subdomain: "docs-site", }); // Resources the form does not show (set with kubectl) survive the edit. @@ -507,26 +501,25 @@ describe("AppForm (edit)", () => { expect(payload.body).not.toHaveProperty("secrets"); }); - it("round-trips allowed users and never changes a registered tile", async () => { + it("round-trips allowed groups and never changes a registered tile", async () => { const listed = { ...EXISTING, - access: { public: false, groups: [], users: ["alice", "bob"], subdomain: "docs-site" }, + access: { public: false, groups: ["alice-team", "bob-team"], subdomain: "docs-site" }, landingPage: { enabled: true }, } as App; const { user, onSubmit, submit } = setup({ mode: "edit", initial: listed }); expect(screen.getByText(/^Listed\./)).toBeInTheDocument(); - const users = screen.getByLabelText("Allowed users"); - expect(users).toHaveValue("alice, bob"); - await user.clear(users); - await user.type(users, "carol"); + const groups = screen.getByLabelText("Allowed groups"); + expect(groups).toHaveValue("alice-team, bob-team"); + await user.clear(groups); + await user.type(groups, "carol-team"); await submit(); const payload = onSubmit.mock.calls[0][0]; if (payload.kind !== "patch") throw new Error(`expected a patch, got ${payload.kind}`); expect(payload.body.access).toEqual({ public: false, - groups: [], - users: ["carol"], + groups: ["carol-team"], subdomain: "docs-site", }); expect(payload.body).not.toHaveProperty("landingPage"); diff --git a/ui/tests/lib/yaml.test.ts b/ui/tests/lib/yaml.test.ts index 11a9ad3..45956c6 100644 --- a/ui/tests/lib/yaml.test.ts +++ b/ui/tests/lib/yaml.test.ts @@ -25,8 +25,6 @@ describe("toYaml", () => { " public: false", " groups:", " - analytics", - " users:", - " - grace", " managers:", " users:", " - dave",