fix(auth): harden existing WebAuthn ceremonies

[alexj11324]

Summary / 摘要

This PR is now the security-only layer requested by maintainer review. It preserves OpenList's existing WebAuthn routes, raw credential storage shape, discoverable login, registration, list, and delete behavior while hardening the ceremonies at their existing owner.

• Enforces the configured absolute HTTPS site_url, exact origin/RP ID, and required user verification.

• Uses cryptographically random, one-time, server-side challenges with expiry and bounded anonymous admission.

• Persists assertion counters before token issuance, rejects clone warnings and stale counters, and serializes credential updates.

• Uses the existing authenticated user/session middleware instead of a parallel authentication owner.

• Adds passkey_trusted_proxies for explicit reverse-proxy client-address trust; the safe default trusts no forwarded address.

• Deliberately excludes credential names, timestamps, rename, and other product-management behavior; that layer is in feat(auth): add managed passkey credentials #2861.

• This PR has breaking changes.
  / 此 PR 包含破坏性变更。

• This PR changes public API, config, storage format, or migration behavior.
  / 此 PR 修改了公开 API、配置、存储格式或迁移行为。

• This PR requires corresponding changes in related repositories.
  / 此 PR 需要关联仓库同步修改。

Related repository PRs / 关联仓库 PR:

• OpenList-Frontend: none; existing WebAuthn UI/API behavior is preserved
• OpenList-Docs: docs(auth): document passkey deployment security OpenList-Docs#348

Related Issues / 关联 Issue

Relates to #2861

Testing / 测试

Authoritative Oracle OCI container, Go 1.26.4, exact commit e727a925481cc2ea3d4238cce382c1a5abf487cc:

• go test -count=1 -tags=jsoniter ./internal/authn ./internal/db ./server/handles — passed.

• go build -tags=jsoniter ./... — passed.

• go test -count=1 -tags=jsoniter ./... — attempted; the same command fails on upstream/main because of existing Go 1.26 vet findings, internal/net proxy assumptions, and aria2 tests requiring a local service on port 6800. Passkey-related packages pass in both runs.

• Independent P0/P1 security review found no blocking issue and confirmed the old API/storage product shape remains outside the split.

• The same security commit was exercised in the isolated HTTPS ol2 end-to-end lifecycle together with the stacked feature commit.

• go test ./...

• Manual test / 手动测试: isolated HTTPS ol2 lifecycle

Checklist / 检查清单

• I have read CONTRIBUTING.
  / 我已阅读 CONTRIBUTING。
• I confirm this contribution follows the repository license, contribution policy, and code of conduct.
  / 我确认此贡献符合仓库许可证、贡献规范和行为准则。
• I have formatted the changed code with gofmt, go fmt, or prettier where applicable.
  / 我已按适用情况使用 gofmt、go fmt 或 prettier 格式化变更代码。
• I have requested review from relevant maintainers or code owners where applicable.
  / 我已在适用情况下请求相关维护者或代码所有者审查。

AI Disclosure / AI 使用声明

• This PR includes AI-assisted content.
  / 此 PR 包含 AI 辅助内容。

Tools used / 使用工具:

• ChatGPT
• Codex
• GitHub Copilot
• Claude
• Gemini
• Other (please specify) / 其他（请注明）:

Usage scope / 使用范围:

• Code generation / 代码生成

• Refactoring / 重构

• Documentation / 文档

• Tests / 测试

• Translation / 翻译

• Review assistance / 审查辅助

• I have reviewed and validated all AI-assisted content included in this PR.
  / 我已审核并验证此 PR 中的所有 AI 辅助内容。

• I have ensured that all AI-assisted commits include Co-Authored-By attribution.
  / 我已确保所有 AI 辅助提交都包含 Co-Authored-By 归属信息。

• I can reproduce all AI-assisted content included in this PR without any AI tools.
  / 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af3539fbf4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jyxjjj]

I have a few concerns about scope, deployment documentation, and the existing WebAuthn settings.

First, could the FRONTEND_DIST_DIR change be moved to a separate PR?

This appears to be a general build feature rather than something required specifically for passkeys. It allows build.sh to replace the bundled frontend with the contents of a local directory, which could be useful for many unrelated backend/frontend changes.

It also changes the trust boundary of the build process: when enabled, arbitrary files from the supplied directory may be included in the final backend artifact. Please document the intended use of this option and confirm that release workflows cannot set it from untrusted pull-request input, downloaded artifacts, or uncontrolled external paths.

Keeping this change separate would make both the passkey implementation and the build/release security impact easier to review.

Second, the PR describes several deployment constraints that should be documented before the feature is merged:

• production requires an absolute HTTPS site_url;
• the configured RP ID and exact origin are enforced;
• authentication challenges are currently stored in memory;
• multi-replica deployments therefore require a shared store with atomic consume semantics.

These details can directly affect registration and login behind reverse proxies or load balancers, especially when requests are routed between instances. Is there a corresponding documentation PR, or could the required documentation be added as part of this work?

Finally, please clarify how this interacts with the existing WebAuthn login setting.

In particular:

• Does enabling the existing setting automatically make passkey login available?
• What happens to WebAuthn credentials that were created before this change?
• Can users still view, add, or remove registered passkeys when login through this method is disabled?
• Is any migration or administrator action required?

The PR mentions backward-compatible credential decoding, which addresses the storage format, but the user-visible behavior and migration expectations should also be documented.

[alexj11324]

Addressed in 02ab2b2 and the coordinated documentation PR OpenListTeam/OpenList-Docs#348:

• Removed the FRONTEND_DIST_DIR build change from this PR; build.sh now matches upstream main.
• Documented the absolute HTTPS site_url, exact origin/RP validation, reverse-proxy headers, five-minute in-memory challenges, single-process topology, and shared atomic-store requirement for multiple replicas.
• Clarified setting behavior: webauthn_login_enabled gates both login and management ceremonies; disabling it does not delete stored credentials, and re-enabling restores them.
• Existing WebAuthn credentials are decoded compatibly; no database migration, re-registration, or administrator action is required. Older credentials receive generated display names/unknown creation metadata until renamed, and last_used_at is populated after a successful assertion.

Evidence: focused backend tests pass (go test ./internal/authn ./internal/db ./server/handles), docs formatting passes, and the docs SSG build passes.

[alexj11324]

Final Oracle ARM64 verification for head 02ab2b2c:

• go test -p=1 ./internal/authn ./internal/db ./server/handles passed from the exact PR source.
• go test -p=1 ./... completed. Passkey-related packages passed; the remaining failures are pre-existing/environmental: non-constant format-string vet findings in unrelated drivers, missing fuse.h, tests requiring a local aria2 service on localhost:6800, and the existing internal/net proxy transport assertion.
• A source-stamped Linux/arm64 image built successfully with frontend head e39da3e.
• Chromium over public HTTPS completed password login → discoverable passkey registration → passkey login → replay rejection (code=400) → metadata/rename → revoke → revoked-credential rejection → password-login regression. Console errors and unexpected network failures: zero.

GitHub's Test Build and Beta Release (Docker) runs remain action_required with zero jobs because fork workflows require a repository maintainer's approval; there is no failed CI job to address yet.

[jyxjjj]

I rechecked the current head and the resolved P1 thread. The change mitigates the original failure mode, but I do not think the unauthenticated challenge-store DoS has been fully addressed.

At the 10,000-entry ceiling, every public begin-login request acquires the global lock, scans the entire challenge map, and evicts the oldest live challenge. Sustained unauthenticated traffic can therefore:

• repeatedly trigger O(n) scans while holding the store lock;
• churn or evict legitimate pending ceremonies;
• consume CPU without needing a valid account.

Evicting the oldest live challenge prevents the store from rejecting every new insertion, but it does not provide admission control and can replace the original lockout with challenge churn and a CPU-heavy hot path.

Please add rate or admission controls that cannot be monopolized by anonymous traffic, avoid an O(n) scan on every insertion at capacity, and add a regression test that demonstrates a legitimate ceremony can still complete during sustained hostile begin-login traffic.

Please also remove the feature-specific changes to the repository-level agent instructions. Those instructions are not part of the passkey implementation or operator documentation and make the PR scope harder to review.

After these changes, this PR and the coordinated frontend PR should receive another security review before merge.

[jyxjjj]

After reviewing the updated scope, I suggest considering a split between the existing WebAuthn security hardening and the broader passkey product changes.

The current PR contains two categories of work with different review and maintenance requirements:

1. Security hardening for the existing WebAuthn implementation, including server-side one-time challenges, ceremony and user binding, replay protection, exact origin/RP validation, required user verification, and assertion-counter persistence.

2. Broader passkey functionality, including named credentials, additional metadata, rename and management behavior, expanded frontend flows, visual assets, and user/operator documentation.

Separating them could make the security-sensitive portion easier to audit and allow it to proceed independently of the product decision around the expanded passkey experience. It would also provide a clearer rollback boundary and keep the coordinated frontend and documentation changes attached to the feature portion.

A possible split would be:

• a minimal backend security PR preserving the current WebAuthn behavior and credential compatibility, including resolution of the remaining challenge-store concern;
• a separate feature PR for named credentials, metadata, management APIs, frontend UX, assets, and documentation.

This is a design suggestion rather than a request to split at this stage. Before making further changes, please provide your own assessment of:

• whether this split is technically practical;
• which backend changes must remain together and why;
• what coupling would remain between the two parts;
• whether you personally intend to maintain the resulting authentication behavior.

The recent responses across the coordinated PRs are highly uniform and read more like generated status reports than a design discussion. Under the repository's automated-contribution policy, another automated acknowledgement or implementation summary is not sufficient. Please confirm that you have personally reviewed the implementation and explain the trade-offs of splitting it in your own words.

[alexj11324]

The security PR keeps the changes that must be audited together: server-side one-time challenges, admission control, ceremony/user binding, exact origin and RP validation, required user verification, replay protection, and assertion-counter persistence. These all belong to the existing WebAuthn authentication path and should remain independent of new product features.

The second PR contains the optional product layer: credential names, created/last-used metadata, rename and management APIs, frontend UX, and the corresponding user documentation. Its only intentional dependency is on the hardened backend behavior from the first PR.

This split gives the security work a smaller audit and rollback boundary, while allowing the broader Passkey experience to be reviewed as a separate product decision. I personally intend to maintain both the security behavior and the Passkey management functionality after merge.

[alexj11324]

@ILoveScratch2 The requested changes have been addressed in the current security-only head e727a925: the feature-specific AGENTS.md change and non-standard comments were removed, anonymous challenge admission is bounded, and expiry handling no longer scans the full challenge map. The broader Passkey management functionality is now split into #2861. All inline review threads are resolved, and the focused Oracle tests and full backend build pass. Could you please re-review #2849 when available?