Only require UV flag when userVerification is "required" per WebAuthn spec#263
Only require UV flag when userVerification is "required" per WebAuthn spec#263Copilot wants to merge 4 commits into
Conversation
…Authn spec The WebAuthn spec (step 16) says the UV flag should only be verified when the Relying Party explicitly requires user verification. Previously, factor "first" always required UV. Now UV is only required when userVerification is set to "required" in the expected options. Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adjusts WebAuthn authenticator flag validation so the UV flag is only enforced when the RP explicitly requires user verification, aligning assertion verification with WebAuthn spec step 16.
Changes:
- Extend
factorToFlags()to acceptuserVerificationand only requireUVwhenuserVerification === "required". - Forward
expected.userVerificationfromassertionResult()/attestationResult()intofactorToFlags(). - Add/expand tests to cover assertion behavior for
factor: "first"with and withoutuserVerification: "required".
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lib/main.js | Adds conditional UV enforcement based on expected.userVerification, and plumbs it through assertion/attestation verification paths. |
| test/main.test.js | Adds assertionResult tests to confirm UV is not required unless userVerification is explicitly "required". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * If "first", this requires that the authenticator performed user verification (e.g. - biometric authentication, PIN authentication, etc.). | ||
| * If "second", this requires that the authenticator performed user presence (e.g. - user pressed a button). | ||
| * If "either", then either "first" or "second" is acceptable | ||
| * @param {String} [expected.userVerification] The expected user verification requirement. Valid values are "required", "preferred", or "discouraged". | ||
| * If "required", the authenticator must have performed user verification and the UV flag is checked. |
| * If "first", this requires that the authenticator performed user verification (e.g. - biometric authentication, PIN authentication, etc.). | ||
| * If "second", this requires that the authenticator performed user presence (e.g. - user pressed a button). | ||
| * If "either", then either "first" or "second" is acceptable | ||
| * @param {String} [expected.userVerification] The expected user verification requirement. Valid values are "required", "preferred", or "discouraged". | ||
| * If "required", the authenticator must have performed user verification and the UV flag is checked. |
| async attestationResult(res, expected) { | ||
| expected.flags = factorToFlags(expected.factor, ["AT"]); | ||
| expected.flags = factorToFlags(expected.factor, ["AT"], expected.userVerification); | ||
| delete expected.factor; | ||
| delete expected.userVerification; | ||
| return await Fido2AttestationResult.create(res, expected); |
| async assertionResult(res, expected) { | ||
| expected.flags = factorToFlags(expected.factor, []); | ||
| expected.flags = factorToFlags(expected.factor, [], expected.userVerification); | ||
| delete expected.factor; | ||
| delete expected.userVerification; | ||
| return Fido2AssertionResult.create(res, expected); |
| @@ -818,14 +824,15 @@ function setOpt(obj, prop, val) { | |||
| */ | |||
|
|
|||
|
|
|||
| function factorToFlags(expectedFactor, flags) { | |||
| // var flags = ["AT"]; | |||
| function factorToFlags(expectedFactor, flags, userVerification) { | |||
|
@copilot apply changes based on the comments in this thread |
…ests - Update factor JSDoc in attestationResult/assertionResult to clarify UV is only checked when userVerification is "required" - Update factorToFlags JSDoc to document the userVerification parameter - Add userVerification to TypeScript ExpectedAttestationResult and ExpectedAssertionResult interfaces - Add attestation tests for factor "first" with/without userVerification Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
All review comments addressed in
All 866 tests passing. |
|
As this has security impact to existing users, we should either keep the old behavior as default or make the new release a major version bump |
factor: "first"unconditionally required the UV flag, but WebAuthn spec step 16 says UV should only be verified when the RP setsuserVerification: "required".Changes
factorToFlags()— accepts optionaluserVerificationparam; UV is only added forfactor: "first"whenuserVerification === "required"assertionResult()/attestationResult()— forwardexpected.userVerificationtofactorToFlags()userVerificationparameter on both methods andfactorToFlags(), and updates thefactor: "first"description to reflect the new behavioruserVerification?: UserVerificationtoExpectedAttestationResultandExpectedAssertionResultinterfaces intypes/main.d.tsUsage
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.