Skip to content

Fix NodeWarden vault passkey prompts to use extension flow#162

Closed
shuaiplus wants to merge 3 commits intomainfrom
codex/fix-passkey-authentication-issue-in-nodewarden-o1nryr
Closed

Fix NodeWarden vault passkey prompts to use extension flow#162
shuaiplus wants to merge 3 commits intomainfrom
codex/fix-passkey-authentication-issue-in-nodewarden-o1nryr

Conversation

@shuaiplus
Copy link
Copy Markdown
Owner

Summary

  • fixed WebAuthn origin resolution to prefer Origin (then Referer) instead of always using the API request URL
  • fixed RP ID resolution for passkey registration/login to derive from the resolved vault origin host
  • applied the same origin validation logic in both passkey registration and login finish handlers

Why

When the vault UI and API/identity routes are accessed through different hostnames, using request.url can produce an RP/origin mismatch. This causes browsers to fall back to platform passkey UI instead of the Bitwarden extension prompt on the NodeWarden vault domain.

Scope

  • src/handlers/passkeys.ts only

Notes

  • no new configurable variables were added; behavior is hardcoded as requested.

Codex Task

Copilot AI review requested due to automatic review settings March 31, 2026 15:12
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 31, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
nodewarden b901a7c Mar 31 2026, 03:31 PM

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba5c4b1e92

ℹ️ 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".

Comment on lines +42 to +43
function resolveWebAuthnRpId(request: Request): string {
return new URL(resolveWebAuthnOrigin(request)).hostname || rpIdFromUrl(request.url);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle opaque origins before deriving RP ID

resolveWebAuthnRpId assumes resolveWebAuthnOrigin always returns a parseable URL, but URL.origin can legitimately be the literal string "null" for opaque origins (for example Origin: file:///... or Referer: about:client). In that case new URL(resolveWebAuthnOrigin(request)) throws, so both begin-registration and begin-login can fail with a 500 instead of falling back to request.url as before this commit. This is a regression for clients/proxies that emit opaque origins and is trivially triggerable on public endpoints by sending such headers.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts NodeWarden’s passkey (WebAuthn) registration/login handlers to derive the expected WebAuthn origin and RP ID from browser-provided headers (preferring Origin, then Referer) rather than always using request.url, improving compatibility when the Vault UI and API/identity endpoints are served from different hostnames.

Changes:

  • Add resolveWebAuthnOrigin() to prefer Origin/Referer for WebAuthn origin resolution (fallback to request.url).
  • Add resolveWebAuthnRpId() to derive RP ID from the resolved origin host.
  • Apply the same origin-resolution/validation logic across both passkey registration and login flows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

try {
return new URL(originHeader).origin;
} catch {
// ignore malformed Origin header and fall back to request URL origin
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment in the Origin parsing catch block says it falls back to the request URL origin, but the actual control flow falls back to checking Referer next (and only then to request.url). Consider adjusting the comment so it matches the real fallback order to avoid confusion during future changes.

Suggested change
// ignore malformed Origin header and fall back to request URL origin
// ignore malformed Origin header and fall back to Referer header or request URL origin

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +44
function resolveWebAuthnRpId(request: Request): string {
return new URL(resolveWebAuthnOrigin(request)).hostname || rpIdFromUrl(request.url);
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new URL(resolveWebAuthnOrigin(request)).hostname will always be a non-empty string for any valid HTTP(S) origin, so the || rpIdFromUrl(request.url) fallback is effectively dead code. Either remove the fallback (and potentially rpIdFromUrl if it becomes unused) or wrap the URL parsing in a try/catch if the fallback is intended for error cases.

Copilot uses AI. Check for mistakes.
@shuaiplus shuaiplus closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants