Skip to content

feat: Declarative OIDC enablement capability (Plan B Units 1-4)#215

Merged
mairas merged 4 commits into
mainfrom
feat/declarative-oidc
Jun 15, 2026
Merged

feat: Declarative OIDC enablement capability (Plan B Units 1-4)#215
mairas merged 4 commits into
mainfrom
feat/declarative-oidc

Conversation

@mairas

@mairas mairas commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What & why

Makes OIDC enablement declarative (Plan B of the #207 series). grafana and signalk hand-roll ~80–180 lines of prestart each to register as Authelia OIDC clients (secret gen, port resolution, container env vars, client snippet) — the duplication that shipped the HALOS_DOMAIN bug (#177). Now an app declares routing.auth.mode: oidc + a routing.auth.oidc block and the generator produces the lifecycle.

Builds on the composable prestart (#212, merged) and the legacy cleanup (#214, merged), so it lands on a clean slate.

Approach (Option X)

mode: oidc is the native-OAuth contract: it adds no Traefik edge gate (configure-container-routing only chains Authelia middleware for forward_auth), but makes is_oidc_app true, reusing the existing scaffolding — postinst provisions the secret once (mode 600), service.j2 orders After=halos-authelia-container.service, postrm cleans up. So OIDC client config lives under routing.auth.oidc, not an orthogonal stanza that would strand that scaffolding.

  • Schema (OidcConfig): client_name, scopes, consent_mode, token_endpoint_auth_method, a discriminated redirect {style: port|path, path}, and a free-form env {container_var: source} map. The map is free-form because the two consumers differ: grafana needs only secret + external_port (derives its redirect from GF_SERVER_ROOT_URL); signalk needs secret + issuer + redirect. A fixed triad can't express both.
  • Prestart OIDC section: resolves the external port (port-based, integer-validated from the registry), appends the mapped env vars to runtime.env, and writes the Authelia client snippet — at prestart time for all OIDC apps (the port-based redirect embeds the runtime port, and the merger only expands ${HALOS_DOMAIN}). This removes the dead build-time rules.j2 install entirely (one code path), resolving the dangling-install issue noted in refactor: Remove dead legacy traefik/OIDC code (Plan C) #214's review.

Verification

  • 561 unit tests pass; ruff, ruff format --check, uvx ty check src/ clean.
  • The generated prestart is byte-checked against grafana (port-based, client_secret_basic) and signalk (path-based, client_secret_post) and bash -n valid — it reproduces their hand-rolled snippets/env, with literal ${HALOS_DOMAIN} (merger expands) and the port resolved at write.
  • No version bump: the 0.8.0 cycle is already open (CI walks +N).

Threat model

Documented in AGENTS.md: secret is postinst-provisioned (root, 600) and env-delivered via runtime.env (600); hostnames.conf is the redirect allow-list trust anchor (generator emits literal ${HALOS_DOMAIN} only, port integer-validated); the merger fixes public: false / one_factor (apps needing other policies are unsupported here).

Scope

Plan B Units 1–4 (capability + docs). Unit 5 (migrate grafana + signalk to the stanza) is deliberately not here — it's the marine repo, and it merges with Plan A's avnav/influxdb migration in one coordinated CONTAINER_TOOLS_REF window after this releases (the rollout entanglement).

Post-Deploy Monitoring & Validation

No production runtime impact from merging — changes generated package output, not a running service. Validation happens when grafana/signalk migrate (Unit 5, separate PR): on-device tools/test-oidc-all.sh for login/callback/SSO, and confirm the generated snippet matches today's hand-rolled one.

Part of #211.

mairas added 4 commits June 15, 2026 21:48
OidcConfig (+ OidcRedirect) under routing.auth.oidc models a native-OAuth app's
Authelia client: client_name/scopes/consent_mode/token_endpoint_auth_method, a
discriminated redirect {style: port|path, path}, and a free-form env map
{container_var: secret|issuer|redirect|external_port|client_id}. mode: oidc now
requires the oidc block. Round-trips both real consumers (grafana port-based +
client_secret_basic; signalk path-based + client_secret_post).
The generated prestart, when routing.auth.oidc is present, resolves the external
port (port-based redirects, integer-validated from the port registry), appends the
container's OIDC env vars to runtime.env per the env map, and writes the Authelia
client snippet to /etc/halos/oidc-clients.d/<client_id>.yml — literal ${HALOS_DOMAIN}
(merger expands), port resolved at write. Snippet is written at prestart time (not
build time) so port-based redirects work; the dead rules.j2 build-time install is
removed. postinst remains the sole secret-gen owner; is_oidc_app still drives Authelia
ordering and postrm cleanup. Reproduces grafana (port) and signalk (path) generatively.

Plan B Units 2-3.
Address code-review findings on the OIDC capability:
- Charset-constrain client_id, client_name, scopes, env var names, and redirect
  path at the schema level — they are interpolated into root-executed generated
  bash (heredoc/echo/path/grep); a newline+EOF in client_name was a root-RCE path,
  relevant since imported-containers auto-converts third-party app definitions.
- Make oidc/mode bidirectional: an oidc block is only valid with mode: oidc, and
  the prestart OIDC section gates on mode == oidc — so it can't diverge from
  is_oidc_app (postinst secret-gen, Authelia ordering). Prevents writing a snippet
  whose secret is never provisioned.
- Reject env source external_port with a non-port redirect (would emit an empty port).
- Tests: injection rejections, bidirectional gating, end-to-end prestart wiring,
  bash -n validity, and the client_id/issuer env sources.
@mairas

mairas commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Code review — declarative OIDC capability

Automated multi-persona review (correctness, security, adversarial, testing, project-standards lenses). It caught real P1s in the bash-generating path; all are fixed in 5a3860c.

Verdict: Ready to merge (after the fixes). Project-standards clean (version cycle open → no bump; no hardcoded hostnames; changelog untouched; doc accurate). Secret-at-rest handling verified sound (runtime.env 600 before write; client_secret_file matches postinst's path). The byte-for-byte reproduction of grafana (port) and signalk (path) checks out.

Fixed in this PR

# Severity Issue Fix
1 P1 Gating divergence: the prestart OIDC section fired on oidc-block presence, but is_oidc_app (postinst secret-gen, Authelia ordering) fires on mode == "oidc". The schema only enforced one direction, so an oidc block under mode: none (a plausible migration slip) would write a snippet whose secret is never provisioned → silent broken login. Bidirectional validator (oidcmode: oidc) and the prestart section now gates on mode == "oidc".
2 P1 Command injection: client_name, env-var names, client_id, scopes, and redirect path were interpolated raw into a root-executed heredoc/echo/path/grep. A newline+EOF in client_name was root RCE — and imported-containers auto-converts third-party app definitions (a real supply-chain path). Charset constraints at the schema level on all five fields (heredoc/echo/path/grep metacharacters now rejected); both real consumers still validate.
3 P1 Test gaps: the prestart wiring was never exercised end-to-end; no bash -n; client_id env source untested. Added end-to-end generate_prestart_script OIDC tests (incl. forward_auth-omits-section + app_id fallback), a bash -n validity check, injection-rejection tests, and the missing env-source tests.
4 P2 env source external_port with a path-style redirect emitted an unbound ${EXTERNAL_PORT} (empty value). Validator rejects that combination.

Deferred to Unit 5 (the marine migration, separate PR)

Verification

  • 576 unit tests pass; ruff / format / uvx ty check src/ clean.
  • Generated grafana + signalk prestarts bash -n valid; both consumers validate against the hardened schema; injection vectors rejected.

Part of #211.

@mairas mairas merged commit 44c13b8 into main Jun 15, 2026
4 checks passed
@mairas mairas deleted the feat/declarative-oidc branch June 15, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant