diff --git a/crates/api-core/src/auth/internal_rbac_rules.rs b/crates/api-core/src/auth/internal_rbac_rules.rs index 14a2ec0fb7..e3e144b98b 100644 --- a/crates/api-core/src/auth/internal_rbac_rules.rs +++ b/crates/api-core/src/auth/internal_rbac_rules.rs @@ -965,9 +965,19 @@ impl RuleInfo { .iter() .flat_map(|x| match *x { RulePrincipal::ForgeAdminCLI => { + // The group is NOT compared: `is_proper_subset_of` + // matches any ExternalUser against any ExternalUser, + // so this value is documentation-only. What actually + // gates admin-CLI access is the client cert mapping to + // an ExternalUser at all (issuer CN listed in + // `auth.additional_issuer_cns`). Keep the value in + // sync with the helm-prereqs `nicoCliClientRole` OU so + // audit logs read sensibly; its previous placeholder + // ("Invalid") leaked into chart defaults as if it were + // load-bearing (issue #3662). vec![Principal::ExternalUser(ExternalUserInfo::new( None, - "Invalid".to_string(), + "nico-cli-client".to_string(), None, ))] } diff --git a/helm-prereqs/README.md b/helm-prereqs/README.md index 5877b5c953..51197eabad 100644 --- a/helm-prereqs/README.md +++ b/helm-prereqs/README.md @@ -112,8 +112,9 @@ The tables below summarize the keys that must be set per site. |-----|---------|-------------|-------------| | `siteName` | `"TMP_SITE"` | **Yes** | Site identifier, injected into postgres pods as `TMP_SITE` | | `imagePullSecrets.ngcNicoPull` | `""` | No (auto) | Pull secret for NICo Core images. Set automatically by `setup.sh` from `REGISTRY_PULL_SECRET` when provided. | -| `vault.nicoCliClientRole.enabled` | `false` | No | Create an optional Vault PKI role for short-lived NICo CLI client certificates. This only defines the certificate profile; issuance access must be granted separately. | -| `vault.nicoCliClientRole.name` | `"nico-cli-client"` | No | Vault role name and certificate `SubjectOU` used to identify NICo CLI client certificates. | +| `vault.nicoCliClientRole.enabled` | `true` | No | Create the Vault PKI role for short-lived NICo CLI client certificates. This only defines the certificate profile; issuance access must be granted separately. | +| `vault.nicoCliClientRole.name` | `"nico-cli-client"` | No | Vault role name, and the certificate `SubjectOU` when `ou` is empty. | +| `vault.nicoCliClientRole.ou` | `""` | No | Certificate `SubjectOU` stamped on issued CLI client certs; empty means use `name`. nico-api maps the OU to the ExternalUser group, but admin-CLI authorization is gated by the issuer CN (`auth.additionalIssuerCns`), not the OU value. Do not set `"Invalid"`. | | `vault.nicoCliClientRole.organization` | `""` | No | Optional certificate `SubjectO` value for deployments that want an additional identity marker. | | `postgresql.instances` | `3` | No | Number of PostgreSQL replicas | | `postgresql.volumeSize` | `"10Gi"` | No | PVC size per PostgreSQL replica | diff --git a/helm-prereqs/values.yaml b/helm-prereqs/values.yaml index 1576d86cb3..41b7c10f1f 100644 --- a/helm-prereqs/values.yaml +++ b/helm-prereqs/values.yaml @@ -51,11 +51,14 @@ vault: nicoCliClientRole: enabled: true # create the admin-cli PKI client role at install name: "nico-cli-client" - ## SubjectOU stamped on issued CLI client certs = the ExternalUser "group" nico-api - ## authorizes. Must match the group the build's ForgeAdminCLI principal expects. - ## Empty = use `name`. (Current builds hardcode "Invalid" — set ou: "Invalid" until the - ## code fix lands; pair with nico-api auth.additionalIssuerCns: ["site-root"].) - ou: "Invalid" # BUILD-SPECIFIC (not site): match this build's hardcoded ForgeAdminCLI group + ## SubjectOU stamped on issued CLI client certs. nico-api maps it to the + ## ExternalUser "group" (authz identifier "external-role/"), but the + ## admin-CLI RBAC rules accept any ExternalUser regardless of group — what + ## actually gates authorization is the ISSUER: the site CA's CN must be + ## listed in nico-api auth.additionalIssuerCns (e.g. ["site-root"]). + ## Empty = use `name`. Do NOT set "Invalid": it was a placeholder copied + ## from a never-compared constant in the authz code (see issue #3662). + ou: "" # empty = `name` (nico-cli-client) organization: "" ## Kubernetes auth role for nico-api to read and manage Vault KV ## credentials used by SiteExplorer and related credential flows. diff --git a/helm-prereqs/values/nico-core.yaml b/helm-prereqs/values/nico-core.yaml index 5f48690ec8..fa66b46b57 100644 --- a/helm-prereqs/values/nico-core.yaml +++ b/helm-prereqs/values/nico-core.yaml @@ -52,8 +52,9 @@ nico-api: hostname: "" # REQUIRED: external nico-api hostname (e.g. api-.) ## Trust admin-CLI client certs minted from the site CA (CN=site-root) so admin-cli works - ## with NO manual ConfigMap edit. Pair with helm-prereqs vault.nicoCliClientRole (ou: "Invalid"). + ## with NO manual ConfigMap edit. Pair with helm-prereqs vault.nicoCliClientRole. ## "site-root" is the CN of the cert-manager site root created by nico-prereqs — generic, keep as-is. + ## This issuer-CN entry is what authorizes admin-CLI certs; the cert's OU only names the group. auth: additionalIssuerCns: - "site-root"