feat(add-factor): challenge API for universal add-factor by orekav · Pull Request #232 · worldcoin/backup-service

orekav · 2026-08-06T13:42:30Z

Summary

Optional existingFactorKind on /v1/add-factor/challenge (default Passkey).
New-factor PASSKEY_REGISTRATION (iOS/Android) with registration_hash in existing-factor challenge context.
OIDC-new challenge path unchanged for current clients.

Split from #136 (PR 2/4). Stacked on #231.

Test plan

cargo test --test add_factor_challenge_shapes
cargo clippy --all-targets -- -D warnings
Confirm Passkey→OIDC challenge shape still works for existing clients

Note

Medium Risk
Touches main-factor challenge issuance and token binding for backup factor changes; defaults preserve existing OIDC-new clients, but new paths affect authentication ceremony state and must stay consistent with downstream /add-factor verification (stacked PR).

Overview
Extends /v1/add-factor/challenge so add-factor flows can use either passkey or OIDC as the existing main factor and can register a new passkey (not only OIDC).

Clients may send optional existingFactorKind (PASSKEY or OIDC_ACCOUNT); it defaults to passkey so current callers keep the same behavior. The existing-factor challenge token now uses the matching challenge type (Passkey vs Keypair).

For PASSKEY_REGISTRATION (iOS/Android), the handler starts the same WebAuthn registration paths as create-passkey, returns newFactorChallenge as a JSON object (not a base64 string), and stores registration state in the new-factor token. A registration_hash (SHA-256 of that state) is embedded in NewFactorType on the existing-factor token so the old factor cannot authorize a swapped registration ceremony—mirroring OIDC token binding.

OIDC-new-factor requests are unchanged: newFactorChallenge stays a string and tokens/context for that path match prior clients. Platform is Clone so add-factor can reuse it from the passkey challenge route.

Adds integration tests asserting response shapes for OIDC, iOS/Android passkey registration, and that Android challenges complete with a mock WebAuthn client.

^{Reviewed by Cursor Bugbot for commit 1c6fd87. Bugbot is set up for automated code reviews on this repo. Configure here.}

Distinguish own factor-id retries from concurrent same-identity winners on ambiguous S3 puts, add exact-match key-only upgrades with ambiguous reconcile, and expose consistent DynamoDB factor lookup after races.

Add optional existingFactorKind, PasskeyRegistration new-factor challenges (with registration_hash binding material), and Android platform support.

Trim redundant commentary and correct resolve_put_object_outcome docs so Inserted requires a matching factor id for every ambiguous put error.

…llenge

paolodamico · 2026-08-07T16:49:10Z

+    /// existing-factor token prevents swapping a different registration ceremony after the old
+    /// factor has signed.
+    #[serde(rename_all = "camelCase")]
+    PasskeyRegistration { registration_hash: String },


wouldn't it be easier to encode just the public key? I wonder if there's a recommendation in the webauthn standard for this, not sure anything else needs to be bound

paolodamico

no blockers but I strongly recommend addressing the feedback

paolodamico · 2026-08-07T17:22:42Z

 pub enum NewFactorType {
+    /// Registering a new passkey (`WebAuthn` credential creation).
+    ///
+    /// `registration_hash` is the hex-encoded SHA-256 digest of the `WebAuthn` registration-state


if we do keep the registration_hash, let's simply link in the doc comment to the other method, let's avoid duplicating the same content as it can become outdated

paolodamico · 2026-08-07T17:23:51Z

+    let (new_factor_type, new_factor_challenge_value, new_factor_token) = match &request.new_factor
+    {
+        NewFactor::PasskeyRegistration { platform } => {
+            let (challenge, registration) = match platform {


this code seems duplicated (create_challenge_passkey), should we refactor it?

paolodamico · 2026-08-07T17:25:09Z

+            rand::thread_rng().fill_bytes(&mut new_factor_challenge);
+            let token = challenge_manager
+                .create_challenge_token(
+                    ChallengeType::Keypair,


I know this predates this change, but we should add a small comment on why this uses a Keypair challenge instead of OIDC

paolodamico · 2026-08-07T17:26:48Z

+use serde_json::json;
+
+#[tokio::test]
+async fn test_add_factor_challenge_shapes() {


nit. name of the test and the module is confusing. shouldn't this live in add_factor_integration?

paolodamico · 2026-08-07T17:27:06Z

+    assert!(value["newFactorChallenge"].is_object());
+    assert!(value["newFactorChallenge"]["publicKey"].is_object());
+    assert!(value["newFactorToken"].is_string());
+


wouldn't we want to verify here that the challenge token was created correctly? e.g. checking on redis, decrypting the token and verifying it's contents?

paolodamico · 2026-08-07T17:27:48Z

cursor review

paolodamico · 2026-08-07T17:29:45Z

@codex review xhigh, particularly analyze the challenge token binding on registration. I worry that this doesn't necessarily mean the user is "committing" to their new passkey, couldn't a registration request still be MITM and a different passkey registered. also check for any webauthn spec flags that may affect this.

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 1c6fd87. Configure here.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c6fd87ec7

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

chatgpt-codex-connector · 2026-08-07T17:40:45Z

+            };
+            let challenge_json: serde_json::Value = serde_json::to_value(&challenge)?;
+            let registration_json = serde_json::to_string(&registration)?;
+            let registration_hash = registration_state_hash(registration_json.as_bytes());


Bind authorization to the resulting passkey

Hashing registration_json commits only to the server-generated ceremony state (challenge and policy), not to the credential ID or public key subsequently returned by navigator.credentials.create(). A compromised same-origin client or relay that obtains the paired challenges can therefore have the victim approve the existing-factor ceremony, complete the new-factor challenge with the attacker's authenticator, and submit that valid registration response; the WebAuthn UP/UV flags only prove interaction and verification on whichever authenticator produced that response, so they do not link it to the existing-factor user. Finish and validate registration before old-factor authorization, then bind the old-factor token to the resulting credential ID and public key (or a canonical digest of the validated credential response).

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-07T17:40:45Z

+    {
+        NewFactor::PasskeyRegistration { platform } => {
+            let (challenge, registration) = match platform {
+                Platform::Ios => environment.webauthn_config().start_passkey_registration(


Require a discoverable credential for the iOS path

For an iOS request completed with a security key or another non-resident authenticator, webauthn-rs 0.5.2's start_passkey_registration emits residentKey: discouraged with no authenticator attachment constraint, so registration can succeed for a non-discoverable credential. The service later starts only a discoverable authentication ceremony in retrieve_challenge_passkey.rs, without an allowCredentials list, leaving that newly added factor impossible to select for recovery. Use registration options that require a resident/discoverable credential (and, if IOS specifically means the platform authenticator, constrain the attachment accordingly).

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-07T17:40:45Z


    // Challenge for the new factor
-    new_factor_challenge: String,
+    new_factor_challenge: serde_json::Value,


Preserve the response schema as a typed union

Because this response derives JsonSchema and is published through /openapi.json, replacing the former String with serde_json::Value changes newFactorChallenge into unconstrained JSON. Whenever existing OIDC consumers regenerate their clients, they lose the established string type even though that runtime branch still returns a string, while passkey consumers receive no schema for the WebAuthn creation options either. Represent the field as a typed oneOf/untagged union of the string challenge and CreationChallengeResponse so generated clients retain both contracts.

Useful? React with 👍 / 👎.

The base branch was changed.

orekav added 2 commits August 6, 2026 15:41

fix(storage): harden factor metadata put reconcile and lookup reads

e1fb42a

Distinguish own factor-id retries from concurrent same-identity winners on ambiguous S3 puts, add exact-match key-only upgrades with ambiguous reconcile, and expose consistent DynamoDB factor lookup after races.

feat(add-factor): extend challenge API for OIDC existing and passkey-new

27aba26

Add optional existingFactorKind, PasskeyRegistration new-factor challenges (with registration_hash binding material), and Android platform support.

orekav requested review from aurel-fr, murph, paolodamico and thomas-waite as code owners August 6, 2026 13:42

This was referenced Aug 6, 2026

feat(add-factor): OIDC existing main factor authorization #233

Open

Universal Add Factor #136

Closed

orekav and others added 5 commits August 7, 2026 11:20

docs(storage): simplify factor put reconcile comments

c171464

Trim redundant commentary and correct resolve_put_object_outcome docs so Inserted requires a matching factor id for every ambiguous put error.

docs(storage): restore # Errors on add_encryption_key_only

7cf5dae

revert comment changes

7ebb2dd

revert comment changes

c7223dd

Merge branch 'uaf/1-factor-write-reconcile' into uaf/2-add-factor-cha…

1c6fd87

…llenge

paolodamico reviewed Aug 7, 2026

View reviewed changes

paolodamico previously approved these changes Aug 7, 2026

View reviewed changes

cursor Bot reviewed Aug 7, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 7, 2026

View reviewed changes

Base automatically changed from uaf/1-factor-write-reconcile to main August 7, 2026 19:03

Uh oh!

Conversation

orekav commented Aug 6, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico left a comment

Choose a reason for hiding this comment

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

paolodamico commented Aug 7, 2026

Uh oh!

paolodamico commented Aug 7, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

orekav commented Aug 6, 2026 •

edited by cursor Bot

Loading