Skip to content

fix(auth): refuse an empty-string password on both login paths - #8261

Merged
JohnMcLear merged 1 commit into
developfrom
fix/empty-password-login
Sep 21, 2026
Merged

JohnMcLear merged 1 commit into
developfrom
fix/empty-password-login

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Follow-up hardening from Wenhao Wu (Southeast University), raised while verifying the fix for GHSA-62cj-9j72-mfrh in 3.3.6.

The gap

A settings.users entry with "password": "" logged in anyone who submitted an empty password — on the OIDC interaction path and on HTTP Basic. Both paths already fail closed for a nullish password; an empty string slipped through because it is a string and compares equal to an empty submission.

Not a vulnerability: only explicit misconfiguration produces it, and Wenhao classified it that way too. But an empty password is never what an operator means, so it should be refused like a missing one.

Fix

  • verifyInteractiveLogin() now rejects password === '' alongside non-string values.
  • webaccess.ts rejects an empty password in the same condition that already rejects a nullish one.

Tests

  • OidcProviderSecurity.ts: rejects an empty-string password against both an empty and a non-empty submission.
  • webaccess.ts: three credential shapes (admin:, admin, admin:anything) all get 401, mirroring the existing nullish-password spec.

Without the fix 3 of these fail; with it all 91 in those two files pass. Full backend suite: 1810 passing. tsc --noEmit clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw

An account configured as `"password": ""` authenticated anyone who
submitted an empty password, on the OIDC interaction path and on HTTP
Basic. Only explicit misconfiguration produces it, so this is hardening
rather than a vulnerability, but both paths should fail closed the way
they already do for a nullish password.

Reported by Wenhao Wu (Southeast University) while verifying the fix for
GHSA-62cj-9j72-mfrh.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013S4pYSjwUsiZtdtMMpW7bw
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Reject empty configured passwords across login paths

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Reject empty configured passwords during OIDC interactive and HTTP Basic authentication.
• Fail closed consistently with existing nullish and non-string password handling.
• Add regression coverage for empty, omitted, and non-empty credential submissions.
Diagram

graph TD
  R["Login request"] --> B["HTTP Basic"] --> U["Configured user"] --> V{"Non-empty string?"} -->|Yes| C["Compare password"] -->|Match| A["Accept login"]
  R --> O["OIDC interaction"] --> U
  V -->|No| D["Reject login"]
  C -->|Mismatch| D
Loading
High-Level Assessment

The targeted fail-closed checks are appropriate because they preserve each authentication path's existing behavior while closing the same narrow misconfiguration gap. A shared password-validation helper was considered, but the paths have different surrounding semantics and the duplicated invariant is too small to justify broader coupling or refactoring.

Files changed (4) +29 / -2

Bug fix (2) +8 / -2
webaccess.tsReject empty passwords in HTTP Basic authentication +5/-1

Reject empty passwords in HTTP Basic authentication

• Treats an empty configured password like a nullish password and returns an authentication failure before comparing submitted credentials. This prevents empty or omitted Basic passwords from authenticating a misconfigured account.

src/node/hooks/express/webaccess.ts

OidcProviderSecurity.tsReject empty passwords during interactive OIDC login +3/-1

Reject empty passwords during interactive OIDC login

• Extends fail-closed password validation to reject empty strings before the constant-time comparison. Valid non-empty string passwords continue through the existing comparison flow.

src/node/security/OidcProviderSecurity.ts

Tests (2) +21 / -0
OidcProviderSecurity.tsCover empty-password rejection for OIDC login +10/-0

Cover empty-password rejection for OIDC login

• Adds regression assertions confirming that an account configured with an empty password rejects both empty and non-empty submissions.

src/tests/backend/specs/OidcProviderSecurity.ts

webaccess.tsCover empty-password rejection for HTTP Basic +11/-0

Cover empty-password rejection for HTTP Basic

• Adds parameterized regression tests for empty, delimiter-free, and non-empty submitted passwords. Each credential form must receive HTTP 401 when the configured password is empty.

src/tests/backend/specs/webaccess.ts

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@JohnMcLear
JohnMcLear merged commit eddb2de into develop Sep 21, 2026
34 of 35 checks passed
@JohnMcLear
JohnMcLear deleted the fix/empty-password-login branch September 21, 2026 18:49
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