Skip to content

fix(auth): never sign anyone in through OIDC without an allowlist - #15049

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.52from
HouMinXi:fix/oidc-empty-allowlist-fail-closed
Oct 6, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.52from
HouMinXi:fix/oidc-empty-allowlist-fail-closed

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The OIDC callback treated an empty oidcAllowedSubjects as "let every
account of the identity provider in", and it minted the dashboard admin
session for whoever that was. The settings route was meant to prevent the
empty state, but it only ran when the request body itself set
oidcEnabled to true. With OIDC already on, a later update that sent
oidcAllowedSubjects: [] (a key that needs no password confirmation)
passed, and so did any state that did not come through that route.

Check the state the update would produce instead: reject it when OIDC
would be enabled and the allowlist would hold no non-blank entry, whether
the request changes the switch, the list or both. The callback now also
counts an allowlist without a usable entry as not configured, before it
contacts the provider, so an instance already in that state stops
admitting everybody. Password login is unaffected.

Related Issues

  • None. This fixes a defect found by review, not a filed issue.

Validation

  • Change type: other
  • Focused tests: tests/unit/oidc-callback.test.ts, tests/unit/oidc-settings-allowlist-guard.test.ts
  • npm run lint
  • Reconciled with the current active release base
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/oidc-callback.test.ts
  • tests/unit/oidc-settings-allowlist-guard.test.ts

Coverage Notes

  • The change is covered by the test files listed above. No coverage drop is expected; the new tests exercise the paths this PR adds.

Reviewer Notes

  • An empty allowlist now refuses the sign-in instead of admitting every account. Operators who relied on the empty-means-open behaviour must set oidcAllowedSubjects before enabling OIDC.

The OIDC callback treated an empty oidcAllowedSubjects as "let every
account of the identity provider in", and it minted the dashboard admin
session for whoever that was. The settings route was meant to prevent the
empty state, but it only ran when the request body itself set
oidcEnabled to true. With OIDC already on, a later update that sent
oidcAllowedSubjects: [] (a key that needs no password confirmation)
passed, and so did any state that did not come through that route.

Check the state the update would produce instead: reject it when OIDC
would be enabled and the allowlist would hold no non-blank entry, whether
the request changes the switch, the list or both. The callback now also
counts an allowlist without a usable entry as not configured, before it
contacts the provider, so an instance already in that state stops
admitting everybody. Password login is unaffected.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@diegosouzapw diegosouzapw changed the title fix(auth): never sign anyone in through OIDC without an allowlist [defer] fix(auth): never sign anyone in through OIDC without an allowlist Sep 29, 2026
@diegosouzapw diegosouzapw added the deferred-v3.8.52 Grande demais / suspeito para o lote atual; precisa de sessão dedicada no ciclo v3.8.52 label Sep 29, 2026
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.51 to release/v3.8.52 September 29, 2026 11:17
@diegosouzapw

Copy link
Copy Markdown
Owner

Re-homed to release/v3.8.52: v3.8.51 entered its release freeze, so the branch now belongs to the release captain and development continues on the next cycle. Nothing is wrong with this PR — it just needed a live base. No action needed from you; CI will re-run against the new base.

@diegosouzapw diegosouzapw changed the title [defer] fix(auth): never sign anyone in through OIDC without an allowlist fix(auth): never sign anyone in through OIDC without an allowlist Oct 1, 2026
@diegosouzapw diegosouzapw removed the deferred-v3.8.52 Grande demais / suspeito para o lote atual; precisa de sessão dedicada no ciclo v3.8.52 label Oct 1, 2026
@diegosouzapw
diegosouzapw merged commit 3efc28e into diegosouzapw:release/v3.8.52 Oct 6, 2026
8 of 16 checks passed
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.

2 participants