fix(authentik): one passkey touch per login, password fallback when none enrolled #491

Merged
k.renner merged 2 commits from fix-authentik-double-passkey-touch into master 2026-08-16 08:22:11 +00:00
Member

Symptom

Every fresh login to a passkey app costs two passkey touches — one to log in, a second immediately after.

Confirmed in the logs for both recent interactive logins:

  • Gitea, 08-16 06:55 — passkey-passwordless-authentication completes at 06:55:54 (login event, Yubikey Nano), then passkey-stepup-authorization challenges at 06:55:55 and is answered at 06:56:08 — a 13s human gap.
  • Home Assistant, 08-10 09:41 — login at 09:41:41.8, step-up challenge → 302 at 09:41:44.7.

Root cause

last_auth_threshold is not session state. It is a single browser cookie, authentik_mfa, holding a JWT signed with sha256(<uid>:<stage.pk>) that also carries "stage": <stage.pk> in its payload (authentik/stages/authenticator_validate/stage.py:361-419, v2026.5.3). check_mfa_cookie() rejects on payload["stage"] != stage.pk.hex, so a validation stage only ever honours a cookie it wrote itself.

The two WebAuthn validations were distinct stage objects:

stage uuid not_configured_action threshold
passkey-passwordless-webauthn-validation 7d10ae15… deny minutes=5
passkey-stepup-webauthn-validation 3f9b2bb7… configure hours=8

So the login touch wrote a cookie the step-up stage rejected → it always challenged. And with only one cookie name, the two stages overwrote each other, so neither window ever survived. Structural, not intermittent: it fired on every fresh login.

Ruled out: the upstream flow-restart race (#11594, fixed in 2026.5.5, we run 2026.5.3) — only 3 logins in 7 days with no restart pairs; and the absent Redis — removed upstream in 2025.10, sessions and cache are Postgres-backed and multi-replica is supported without affinity.

Change

Collapse both into one passkey-webauthn-validation stage bound into both flows, so the login touch satisfies the step-up.

not_configured_action: deny — the shared stage is reached in the authentication flow with the user only identified (a typed username) and no password stage in front of it, so configure there would let anyone who knows a passkey-less username enroll their own passkey and take the account.

g.renner, j.renner and t.geiger currently hold zero passkeys (live count, 2026-08-16) and so lose access to the step-up apps until they enroll. Self-service enrollment is live: default-authenticator-webauthn-setup carries a configure_flow, so they can log in at auth.renner.uno with the default password flow and enroll from user settings.

The old stages are marked state: absent. Their flowstagebindings are FK-CASCADEd away with them, and the identification stage's webauthn_stage FK is SET_NULL (stages/identification/models.py:61-65) and is re-pointed in the same blueprint, so nothing dangles.

Accepted trade-off

The 8h cookie now also covers the login flow: a re-login in the same browser inside the window is satisfied by the cookie alone, leaving the username as the only thing typed. The cookie is keyed to that user's device PK, so it never carries across users. Chosen deliberately over the alternatives (a policy-gated step-up binding, or retiring the step-up flow and dropping password logins entirely).

Verification

  • kubectl kustomize kubernetes/apps/authentik builds clean.
  • Embedded blueprint parses; entry list is as intended, with both old stages absent last.
  • Expect one extra challenge on the first login after rollout: the new stage has a new PK, so existing authentik_mfa cookies miss once.

🤖 Generated with Claude Code

## Symptom Every fresh login to a passkey app costs **two** passkey touches — one to log in, a second immediately after. Confirmed in the logs for both recent interactive logins: - **Gitea, 08-16 06:55** — `passkey-passwordless-authentication` completes at `06:55:54` (login event, Yubikey Nano), then `passkey-stepup-authorization` challenges at `06:55:55` and is answered at `06:56:08` — a 13s human gap. - **Home Assistant, 08-10 09:41** — login at `09:41:41.8`, step-up challenge → `302` at `09:41:44.7`. ## Root cause `last_auth_threshold` is not session state. It is a single browser cookie, `authentik_mfa`, holding a JWT signed with `sha256(<uid>:<stage.pk>)` that also carries `"stage": <stage.pk>` in its payload (`authentik/stages/authenticator_validate/stage.py:361-419`, v2026.5.3). `check_mfa_cookie()` rejects on `payload["stage"] != stage.pk.hex`, so **a validation stage only ever honours a cookie it wrote itself**. The two WebAuthn validations were distinct stage objects: | stage | uuid | not_configured_action | threshold | |---|---|---|---| | `passkey-passwordless-webauthn-validation` | `7d10ae15…` | deny | minutes=5 | | `passkey-stepup-webauthn-validation` | `3f9b2bb7…` | configure | hours=8 | So the login touch wrote a cookie the step-up stage rejected → it always challenged. And with only one cookie *name*, the two stages overwrote each other, so neither window ever survived. Structural, not intermittent: it fired on every fresh login. Ruled out: the upstream flow-restart race ([#11594](https://github.com/goauthentik/authentik/issues/11594), fixed in 2026.5.5, we run 2026.5.3) — only 3 logins in 7 days with no restart pairs; and the absent Redis — removed upstream in 2025.10, sessions and cache are Postgres-backed and multi-replica is supported without affinity. ## Change Collapse both into one `passkey-webauthn-validation` stage bound into both flows, so the login touch satisfies the step-up. `not_configured_action: deny` — the shared stage is reached in the authentication flow with the user only *identified* (a typed username) and no password stage in front of it, so `configure` there would let anyone who knows a passkey-less username enroll their own passkey and take the account. `g.renner`, `j.renner` and `t.geiger` currently hold zero passkeys (live count, 2026-08-16) and so lose access to the step-up apps until they enroll. Self-service enrollment is live: `default-authenticator-webauthn-setup` carries a `configure_flow`, so they can log in at auth.renner.uno with the default password flow and enroll from user settings. The old stages are marked `state: absent`. Their `flowstagebinding`s are FK-CASCADEd away with them, and the identification stage's `webauthn_stage` FK is `SET_NULL` (`stages/identification/models.py:61-65`) and is re-pointed in the same blueprint, so nothing dangles. ## Accepted trade-off The 8h cookie now also covers the login flow: a re-login in the same browser inside the window is satisfied by the cookie alone, leaving the username as the only thing typed. The cookie is keyed to that user's device PK, so it never carries across users. Chosen deliberately over the alternatives (a policy-gated step-up binding, or retiring the step-up flow and dropping password logins entirely). ## Verification - `kubectl kustomize kubernetes/apps/authentik` builds clean. - Embedded blueprint parses; entry list is as intended, with both old stages `absent` last. - Expect **one** extra challenge on the first login after rollout: the new stage has a new PK, so existing `authentik_mfa` cookies miss once. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claude-bot added 1 commit 2026-08-16 07:26:48 +00:00
fix(authentik): share one WebAuthn stage across login and step-up
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
9e0ffae358
Every fresh login to a passkey app costs two touches: one in
passkey-passwordless-authentication, a second immediately after in
passkey-stepup-authorization.

last_auth_threshold is enforced with a single browser cookie
(authentik_mfa) whose JWT is signed with, and carries, the validation
stage's PK. A stage only honours a cookie it wrote itself, and both
stages write to the same cookie name — so two stage objects each demand
their own touch and clobber each other's window.

Collapse them into one passkey-webauthn-validation stage bound into both
flows. not_configured_action is deny, since the authentication flow
reaches the stage with the user only identified and no password stage in
front of it. Passkey-less users enroll from user settings after a
password login; default-authenticator-webauthn-setup has a
configure_flow, so that path is live.

The 8h cookie now also covers the login flow, so a re-login in the same
browser inside the window is satisfied by the cookie alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
review-bot left a comment
Member

🤖 Automated review — model CLI default (complexity: complex), commit 9e0ffae35860dfac394166a95b37a7158c9afa83.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

This PR collapses two separate authentik authenticatorvalidatestage objects (passkey-stepup-webauthn-validation and passkey-passwordless-webauthn-validation) into one shared passkey-webauthn-validation stage bound into both the passwordless authentication flow and the step-up authorization flow, in kubernetes/apps/authentik/blueprints-configmap.yaml. The stated root cause — the authentik_mfa cookie's JWT is signed with and carries the stage's own PK, so a validation stage only ever honours a cookie it wrote itself — is a plausible and internally consistent explanation for the observed double-touch, and the fix (share one stage object so the login touch satisfies the step-up) directly addresses it. I traced every !KeyOf/!Find reference to the old and new stage names across the whole tree (grep -rl for all three names only hits this one file) and found no dangling references. The diff is self-consistent, well-commented, and only touches this one file. No concrete code defect found — the review below is operational/risk callouts rather than blocking bugs.

🟡 Suggestions

  • Real users lose access on merge, not just in theory. kubernetes/apps/authentik/blueprints-configmap.yaml:1394 sets not_configured_action: deny on the now-shared stage. The PR body states g.renner, j.renner, and t.geiger currently hold zero enrolled passkeys (live check 2026-08-16) and will lose access to every step-up-gated app (Home Assistant, NetBird, Audiobookshelf, immich, Gitea, Grafana — all re-pointed to passkey-stepup-flow/passkey-passwordless-flow at lines 1474-1571) until they self-enroll. This is disclosed and the self-service enrollment path is confirmed live, but since authentik applies this blueprint automatically on the next reconcile (blueprint discovery via the Helm-deployed authentik server, not a manual step), consider notifying those three users to enroll before merging rather than after they hit the lockout.
  • The only stated verification is static. The PR's "Verification" section covers kubectl kustomize building and blueprint YAML parsing — both catch structural/YAML errors but not apply-time behavior against the live Postgres-backed authentik instance. Two runtime assumptions are load-bearing here and unverified against actual authentik source in this repo (no vendored copy to check): (1) that the whole blueprint file applies inside one transaction, so the identification stage's webauthn_stage re-point (line 1444) durably lands before the old stages are deleted (lines 1579-1587); (2) that FlowStageBinding.stage is on_delete=CASCADE, so the two old flowstagebindings (previously at order 10/20 pointing at the deleted stages) are removed automatically rather than orphaned. Both are cited from the PR author's reading of authentik's source at specific file:line references, which I could not independently confirm since authentik itself isn't in this repo. If wrong, the failure mode would surface as an apply-time error or an orphaned/duplicate flowstagebinding row on the live instance — not something CI would catch. Worth watching the first reconcile after merge (the repo already has precedent for this kind of orphan-row problem: see the "Orphan PolicyBindings to clean up" comment at blueprints-configmap.yaml:1181-1191, which had to be discovered and patched after the fact for a different model).
  • Security posture of the login path itself changes, not just step-up. last_auth_threshold: hours=8 (line 1407) now also governs the passwordless authentication flow's validation (binding at line 1456), which previously used minutes=5. Per the PR's disclosed "Accepted trade-off," a same-browser re-login within the 8h window is now satisfied by the authentik_mfa cookie alone — only the username gets typed, no fresh WebAuthn touch. This is a deliberate, reasoned choice (and the cookie is scoped to the user's device PK, so it doesn't cross users), but it is a real widening of the window during which a compromised/left-open browser session can re-authenticate as the user without a physical touch, on the login path, not just step-up. Worth a second pair of eyes signing off specifically on that behavioral/security change rather than just the bug fix.

Tests

No automated test exists or is expected for this kind of change in this repo — blueprints-configmap.yaml is validated by CI's kustomize-build step (.woodpecker/ci-manifests-test.yml), which checks the kustomization builds and (implicitly) that the embedded blueprint YAML is well-formed. That's necessary but not sufficient here: it can't catch a Django-level constraint violation or an incorrect cascade-delete assumption at actual blueprint-apply time, which is the main residual risk flagged above. There's no practical way to unit-test this in CI given the current pipeline (no live authentik/Postgres in the offline, secret-free ci-manifests-test.yml per its own doc comment), so the realistic mitigation is watching the first reconcile log after merge rather than adding a test.

Recommendation

Looks reasonable — no blocking issues; comments above are operational risk callouts worth a glance before/after merge, not code defects.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `9e0ffae35860dfac394166a95b37a7158c9afa83`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR collapses two separate authentik `authenticatorvalidatestage` objects (`passkey-stepup-webauthn-validation` and `passkey-passwordless-webauthn-validation`) into one shared `passkey-webauthn-validation` stage bound into both the passwordless authentication flow and the step-up authorization flow, in `kubernetes/apps/authentik/blueprints-configmap.yaml`. The stated root cause — the `authentik_mfa` cookie's JWT is signed with and carries the stage's own PK, so a validation stage only ever honours a cookie it wrote itself — is a plausible and internally consistent explanation for the observed double-touch, and the fix (share one stage object so the login touch satisfies the step-up) directly addresses it. I traced every `!KeyOf`/`!Find` reference to the old and new stage names across the whole tree (`grep -rl` for all three names only hits this one file) and found no dangling references. The diff is self-consistent, well-commented, and only touches this one file. No concrete code defect found — the review below is operational/risk callouts rather than blocking bugs. ## 🟡 Suggestions - **Real users lose access on merge, not just in theory.** `kubernetes/apps/authentik/blueprints-configmap.yaml:1394` sets `not_configured_action: deny` on the now-shared stage. The PR body states `g.renner`, `j.renner`, and `t.geiger` currently hold zero enrolled passkeys (live check 2026-08-16) and will lose access to every step-up-gated app (Home Assistant, NetBird, Audiobookshelf, immich, Gitea, Grafana — all re-pointed to `passkey-stepup-flow`/`passkey-passwordless-flow` at lines 1474-1571) until they self-enroll. This is disclosed and the self-service enrollment path is confirmed live, but since authentik applies this blueprint automatically on the next reconcile (blueprint discovery via the Helm-deployed authentik server, not a manual step), consider notifying those three users to enroll *before* merging rather than after they hit the lockout. - **The only stated verification is static.** The PR's "Verification" section covers `kubectl kustomize` building and blueprint YAML parsing — both catch structural/YAML errors but not apply-time behavior against the live Postgres-backed authentik instance. Two runtime assumptions are load-bearing here and unverified against actual authentik source in this repo (no vendored copy to check): (1) that the whole blueprint file applies inside one transaction, so the identification stage's `webauthn_stage` re-point (line 1444) durably lands before the old stages are deleted (lines 1579-1587); (2) that `FlowStageBinding.stage` is `on_delete=CASCADE`, so the two old flowstagebindings (previously at order 10/20 pointing at the deleted stages) are removed automatically rather than orphaned. Both are cited from the PR author's reading of authentik's source at specific file:line references, which I could not independently confirm since authentik itself isn't in this repo. If wrong, the failure mode would surface as an apply-time error or an orphaned/duplicate `flowstagebinding` row on the live instance — not something CI would catch. Worth watching the first reconcile after merge (the repo already has precedent for this kind of orphan-row problem: see the "Orphan PolicyBindings to clean up" comment at `blueprints-configmap.yaml:1181-1191`, which had to be discovered and patched after the fact for a different model). - **Security posture of the login path itself changes, not just step-up.** `last_auth_threshold: hours=8` (line 1407) now also governs the *passwordless authentication* flow's validation (binding at line 1456), which previously used `minutes=5`. Per the PR's disclosed "Accepted trade-off," a same-browser re-login within the 8h window is now satisfied by the `authentik_mfa` cookie alone — only the username gets typed, no fresh WebAuthn touch. This is a deliberate, reasoned choice (and the cookie is scoped to the user's device PK, so it doesn't cross users), but it is a real widening of the window during which a compromised/left-open browser session can re-authenticate as the user without a physical touch, on the *login* path, not just step-up. Worth a second pair of eyes signing off specifically on that behavioral/security change rather than just the bug fix. ## Tests No automated test exists or is expected for this kind of change in this repo — `blueprints-configmap.yaml` is validated by CI's `kustomize-build` step (`.woodpecker/ci-manifests-test.yml`), which checks the kustomization builds and (implicitly) that the embedded blueprint YAML is well-formed. That's necessary but not sufficient here: it can't catch a Django-level constraint violation or an incorrect cascade-delete assumption at actual blueprint-apply time, which is the main residual risk flagged above. There's no practical way to unit-test this in CI given the current pipeline (no live authentik/Postgres in the offline, secret-free `ci-manifests-test.yml` per its own doc comment), so the realistic mitigation is watching the first reconcile log after merge rather than adding a test. ## Recommendation Looks reasonable — no blocking issues; comments above are operational risk callouts worth a glance before/after merge, not code defects. <!-- pr-review:sha=9e0ffae35860dfac394166a95b37a7158c9afa83 -->
claude-bot added 1 commit 2026-08-16 07:45:34 +00:00
feat(authentik): fall back to password when no passkey is enrolled
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
803cdb3db8
The passkey apps were reachable only with a WebAuthn credential:
passkey-passwordless-authentication denies at its validation stage, and
passkey-stepup-authorization demands a passkey on every authorize. An
account with no passkey had no way in.

Branch both flows on enrollment with an expression policy,
passkey-user-has-webauthn. Enrolled accounts keep the mandatory passkey
at login and at every authorize. Accounts without one skip the WebAuthn
stage and get default-authentication-password followed by
default-authentication-mfa-validation — the same pair, in the same
order, that default-authentication-flow already runs, so the fallback is
not a weaker way in than auth.renner.uno's own login.

The policy runs against pending_user, so the branch follows the
identified account's enrollment and is not selectable by the requester.
Bindings keep evaluate_on_plan false and re_evaluate_policies true;
on-plan evaluation would run before a user is resolved.

The negated bindings carry failure_result: true. UserLoginStage defaults
a missing authentication backend to BACKEND_INBUILT rather than
refusing, so a flow that dropped both branches would log the user in on
a username alone. A PolicyException resolves to failure_result before
negate is applied, so a broken expression degrades to password + MFA
instead of to no factor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot changed title from fix(authentik): share one WebAuthn stage across login and step-up to fix(authentik): one passkey touch per login, password fallback when none enrolled 2026-08-16 07:45:39 +00:00
Author
Member

Second commit: password fallback when no passkey is enrolled

803cdb3d adds the fallback. Both flows now branch on enrollment via a new expression policy passkey-user-has-webauthn (return ak_user_has_authenticator(request.user, "webauthn")).

passkey-passwordless-authentication

order stage policy
10 passkey-passwordless-identification —
20 passkey-webauthn-validation has passkey
21 default-authentication-password not has passkey
22 default-authentication-mfa-validation not has passkey
30 default-authentication-login —

passkey-stepup-authorization — order 10 passkey-webauthn-validation, gated on has passkey. Without this the fallback would be useless: the user logs in with a password and is then denied at the app.

Why this isn't a downgrade path

ReevaluateMarker builds the policy engine with plan.context[PLAN_CONTEXT_PENDING_USER] (flows/markers.py), so request.user is the identified account, not the anonymous requester. The branch therefore follows the victim's enrollment, not the attacker's choice — an account holding a passkey can never be talked down to the password branch. This depends on the bindings keeping evaluate_on_plan: false / re_evaluate_policies: true; on-plan evaluation runs before the identification stage has resolved a user.

The fallback pairs the password stage with default-authentication-mfa-validation, the same two stages in the same order that default-authentication-flow already runs. So the new path into these apps is no weaker than the login at auth.renner.uno that already exists. That stage's not_configured_action: configure is safe here specifically because a first factor is established by the time it runs — it doubles as the enrollment path for the accounts holding no authenticator at all.

Fail-safe

UserLoginStage defaults a missing PLAN_CONTEXT_AUTHENTICATION_BACKEND to BACKEND_INBUILT rather than refusing (stages/user_login/stage.py:150-152), so a flow that dropped both branches would log the user in on a typed username alone. PolicyProcess.execute() resolves a PolicyException to binding.failure_result before negate is applied (policies/process.py:82-105), so the negated bindings carry failure_result: true: a broken expression pulls the password stage in and degrades everyone to password + MFA, never to no factor.

Effect per account (live counts, 2026-08-16)

  • k.renner (4), s.renner (4), akadmin (2), i.birzer (1) — passkey required, one touch.
  • g.renner (0 passkeys, 1 TOTP) — password + TOTP.
  • j.renner, t.geiger (no authenticators) — password, then prompted to enroll one.

Verification

kubectl kustomize builds clean and the blueprint parses with the branch bindings and negate/failure_result flags as intended. Not yet verified live — after merge, worth confirming both branches: a passkey account still gets challenged at login and at authorize, and a passkey-less account reaches password + MFA.

## Second commit: password fallback when no passkey is enrolled `803cdb3d` adds the fallback. Both flows now branch on enrollment via a new expression policy `passkey-user-has-webauthn` (`return ak_user_has_authenticator(request.user, "webauthn")`). **`passkey-passwordless-authentication`** | order | stage | policy | |---|---|---| | 10 | `passkey-passwordless-identification` | — | | 20 | `passkey-webauthn-validation` | has passkey | | 21 | `default-authentication-password` | **not** has passkey | | 22 | `default-authentication-mfa-validation` | **not** has passkey | | 30 | `default-authentication-login` | — | **`passkey-stepup-authorization`** — order 10 `passkey-webauthn-validation`, gated on *has passkey*. Without this the fallback would be useless: the user logs in with a password and is then denied at the app. ### Why this isn't a downgrade path `ReevaluateMarker` builds the policy engine with `plan.context[PLAN_CONTEXT_PENDING_USER]` (`flows/markers.py`), so `request.user` is the **identified account**, not the anonymous requester. The branch therefore follows the victim's enrollment, not the attacker's choice — an account holding a passkey can never be talked down to the password branch. This depends on the bindings keeping `evaluate_on_plan: false` / `re_evaluate_policies: true`; on-plan evaluation runs before the identification stage has resolved a user. The fallback pairs the password stage with `default-authentication-mfa-validation`, the same two stages in the same order that `default-authentication-flow` already runs. So the new path into these apps is no weaker than the login at auth.renner.uno that already exists. That stage's `not_configured_action: configure` is safe here specifically because a first factor is established by the time it runs — it doubles as the enrollment path for the accounts holding no authenticator at all. ### Fail-safe `UserLoginStage` defaults a missing `PLAN_CONTEXT_AUTHENTICATION_BACKEND` to `BACKEND_INBUILT` rather than refusing (`stages/user_login/stage.py:150-152`), so a flow that dropped **both** branches would log the user in on a typed username alone. `PolicyProcess.execute()` resolves a `PolicyException` to `binding.failure_result` *before* `negate` is applied (`policies/process.py:82-105`), so the negated bindings carry `failure_result: true`: a broken expression pulls the password stage in and degrades everyone to password + MFA, never to no factor. ### Effect per account (live counts, 2026-08-16) - `k.renner` (4), `s.renner` (4), `akadmin` (2), `i.birzer` (1) — passkey required, one touch. - `g.renner` (0 passkeys, 1 TOTP) — password + TOTP. - `j.renner`, `t.geiger` (no authenticators) — password, then prompted to enroll one. ### Verification `kubectl kustomize` builds clean and the blueprint parses with the branch bindings and negate/failure_result flags as intended. Not yet verified live — after merge, worth confirming both branches: a passkey account still gets challenged at login and at authorize, and a passkey-less account reaches password + MFA.
review-bot left a comment
Member

🤖 Automated review — model CLI default (complexity: complex), commit 803cdb3db86280b518d990f8820089f8d18f8c00.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

Collapses the two separate WebAuthn-validation stages (passkey-stepup-webauthn-validation, passkey-passwordless-webauthn-validation) into one shared passkey-webauthn-validation stage bound into both the step-up authorization flow and the passwordless authentication flow, so a single passkey touch satisfies both. It also adds a policy-gated fallback branch (password + MFA) in the passwordless flow and a policy gate on the step-up stage, for accounts with no enrolled passkey — a materially larger change than the title implies (it's not just a stage merge, it's a new conditional-branch flow topology). The blueprint logic is internally consistent and the reasoning in the PR body checks out against the diff; I found no confirmed blocking defect, but one verification gap (below) is worth closing before merge since it's the kind of thing this repo's own CI history says goes unnoticed by "it builds."

🟡 Suggestions

  • kubernetes/apps/authentik/blueprints-configmap.yaml:1534,1563 — these two !Find lookups (default-authentication-password, default-authentication-mfa-validation) are new; the only pre-existing !Find of this kind in the file was default-authentication-login (base commit only had that one — confirmed via git show 8257375:...). The PR's "Verification" section says kubectl kustomize builds clean and the blueprint parses, and .woodpecker/ci-manifests-test.yml confirms CI only runs kubectl kustomize (a fully offline templating check, no cluster/API access) — neither that nor a local YAML parse can confirm these two stage names actually exist in the live Authentik instance. If either name is wrong, !Find fails to resolve at blueprint-apply time inside the authentik pod, which (per Authentik's transactional blueprint apply) most likely fails the whole blueprint rather than partially applying it — but that means the fix in this PR silently never lands rather than erroring loudly in CI, which is easy to miss. Worth a one-time manual check against the running instance (API /api/v3/stages/password/ and /api/v3/stages/authenticator/validate/, or the admin UI) that both names exist verbatim, either before merge or in the rollout step.
  • PR body, "Change" section — the title says "one passkey touch per login, password fallback when none enrolled," but the diff also adds policy gating to the step-up flow's only stage (binding-stepup-webauthn, blueprints-configmap.yaml:1440-1449), so passkey-less accounts now skip the step-up authorization stage entirely rather than hitting not_configured_action: deny there. That's consistent with the stated design (they're gated to password+MFA earlier, in the passwordless flow), but it's a second behavioral change bundled into what reads like a single-purpose fix — worth calling out explicitly in the PR body for anyone auditing this later, since "skips step-up entirely for accounts without a passkey" is a meaningfully different security posture than "step-up demands enrollment."
  • blueprints-configmap.yaml:1552-1558 (comment on binding-passwordless-mfa) — the comment asserts default-authentication-mfa-validation carries not_configured_action: configure "safe here and nowhere else in this flow." That attribute lives on a stage this blueprint doesn't define (referenced only via !Find), so it's a claim about a system default that isn't checkable from this repo. Not a defect, just flagging it's an assumption riding on Authentik's shipped defaults rather than something this diff pins down.

Tests

No automated test can exercise this (Authentik blueprint semantics, live policy evaluation, WebAuthn ceremonies) — consistent with how the rest of kubernetes/apps/authentik/ is validated in this repo (kustomize build only, per .woodpecker/ci-manifests-test.yml). The PR body's manual verification (kustomize build, blueprint parse) is reasonable but — see the Suggestions item above — doesn't reach the one thing most likely to silently break: whether the two new !Find targets resolve on the live instance. Recommend confirming that against the running Authentik before or immediately after rollout, and watching the first few passkey-less logins (g.renner/j.renner/t.geiger's step-up-gated apps) for the fallback path actually working end-to-end.

Recommendation

Looks reasonable — no blocking issues; comments above are optional but the !Find-target verification is worth doing before merge given it can't be checked by this repo's CI.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `803cdb3db86280b518d990f8820089f8d18f8c00`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Collapses the two separate WebAuthn-validation stages (`passkey-stepup-webauthn-validation`, `passkey-passwordless-webauthn-validation`) into one shared `passkey-webauthn-validation` stage bound into both the step-up authorization flow and the passwordless authentication flow, so a single passkey touch satisfies both. It also adds a policy-gated fallback branch (password + MFA) in the passwordless flow and a policy gate on the step-up stage, for accounts with no enrolled passkey — a materially larger change than the title implies (it's not just a stage merge, it's a new conditional-branch flow topology). The blueprint logic is internally consistent and the reasoning in the PR body checks out against the diff; I found no confirmed blocking defect, but one verification gap (below) is worth closing before merge since it's the kind of thing this repo's own CI history says goes unnoticed by "it builds." ## 🟡 Suggestions - **`kubernetes/apps/authentik/blueprints-configmap.yaml:1534,1563`** — these two `!Find` lookups (`default-authentication-password`, `default-authentication-mfa-validation`) are new; the only pre-existing `!Find` of this kind in the file was `default-authentication-login` (base commit only had that one — confirmed via `git show 8257375:...`). The PR's "Verification" section says `kubectl kustomize` builds clean and the blueprint parses, and `.woodpecker/ci-manifests-test.yml` confirms CI only runs `kubectl kustomize` (a fully offline templating check, no cluster/API access) — neither that nor a local YAML parse can confirm these two stage names actually exist in the live Authentik instance. If either name is wrong, `!Find` fails to resolve at blueprint-apply time inside the authentik pod, which (per Authentik's transactional blueprint apply) most likely fails the whole blueprint rather than partially applying it — but that means the fix in this PR silently never lands rather than erroring loudly in CI, which is easy to miss. Worth a one-time manual check against the running instance (API `/api/v3/stages/password/` and `/api/v3/stages/authenticator/validate/`, or the admin UI) that both names exist verbatim, either before merge or in the rollout step. - **PR body, "Change" section** — the title says "one passkey touch per login, password fallback when none enrolled," but the diff also adds policy gating to the step-up flow's *only* stage (`binding-stepup-webauthn`, blueprints-configmap.yaml:1440-1449), so passkey-less accounts now skip the step-up authorization stage entirely rather than hitting `not_configured_action: deny` there. That's consistent with the stated design (they're gated to password+MFA earlier, in the passwordless flow), but it's a second behavioral change bundled into what reads like a single-purpose fix — worth calling out explicitly in the PR body for anyone auditing this later, since "skips step-up entirely for accounts without a passkey" is a meaningfully different security posture than "step-up demands enrollment." - **blueprints-configmap.yaml:1552-1558** (comment on `binding-passwordless-mfa`) — the comment asserts `default-authentication-mfa-validation` carries `not_configured_action: configure` "safe here and nowhere else in this flow." That attribute lives on a stage this blueprint doesn't define (referenced only via `!Find`), so it's a claim about a system default that isn't checkable from this repo. Not a defect, just flagging it's an assumption riding on Authentik's shipped defaults rather than something this diff pins down. ## Tests No automated test can exercise this (Authentik blueprint semantics, live policy evaluation, WebAuthn ceremonies) — consistent with how the rest of `kubernetes/apps/authentik/` is validated in this repo (`kustomize build` only, per `.woodpecker/ci-manifests-test.yml`). The PR body's manual verification (kustomize build, blueprint parse) is reasonable but — see the Suggestions item above — doesn't reach the one thing most likely to silently break: whether the two new `!Find` targets resolve on the live instance. Recommend confirming that against the running Authentik before or immediately after rollout, and watching the first few passkey-less logins (g.renner/j.renner/t.geiger's step-up-gated apps) for the fallback path actually working end-to-end. ## Recommendation Looks reasonable — no blocking issues; comments above are optional but the `!Find`-target verification is worth doing before merge given it can't be checked by this repo's CI. <!-- pr-review:sha=803cdb3db86280b518d990f8820089f8d18f8c00 -->
k.renner merged commit 352f0b5cff into master 2026-08-16 08:22:11 +00:00
Sign in to join this conversation.