fix(core): normalize selfie response identifier - #313
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
7fc1ec6 to
e36c660
Compare
e36c660 to
4d93be0
Compare
4d93be0 to
be8ad61
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be8ad619d4
ℹ️ 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".
| let normalized_identifier = | ||
| normalize_response_identifier(item.identifier.clone(), item.issuer_schema_id); | ||
| let signal_hash = context.signal_hashes.get(&normalized_identifier).cloned(); |
There was a problem hiding this comment.
When proofResponseToIDKitResult receives a schema-11 response identified as face and its public signal_hashes option is keyed by that incoming identifier, this lookup now checks only selfie, so the resulting response silently loses its signal hash; the same input worked before this commit. Check the normalized key first, but fall back to item.identifier when no normalized-key entry exists.
Useful? React with 👍 / 👎.
igorosip0v
left a comment
igorosip0v
left a comment
There was a problem hiding this comment.
@Takaros999 the JS native transport still returns face for legacy responses (native.ts#L517-L522, #L538-L544), while the v4 branch goes through WASM and gets the new value. Would a mini app see face and web via bridge selfie for the same wallet response?
| let identifier = match self.verification_level { | ||
| VerificationLevel::Face => CredentialType::Selfie.to_string(), | ||
| verification_level => verification_level.to_string(), | ||
| }; |
There was a problem hiding this comment.
@Takaros999 ResponseItem::V3 has no verification_level field (types.rs#L702-L714), and the request still sends verification_level: "face". Does an RP verifying a legacy proof now get selfie where it used to read the 3.0 level? Is selfie accepted there, or should the mapping cover only v4 and Session?
2 participants
Our current IDKit response looks like this:
{ "protocol_version": "4.0", "nonce": "a1b2c3d4-e5f6-7890-abcd-ef1234567890", "action": "my-action", "environment": "production", "responses": [ { "identifier": "proof_of_human", // or `face`, `passport`, `mnc` "issuer_schema_id": 1, ... } ], "user_presence_completed": false }The field
identifier: faceis legacy from the days Selfie Check was used to called Face Check. This confuses a lot of developers.identifieris of type string and not an enum. I think it's better to do this change now and bite the bullet than wait for a major bump.Relevant to: worldcoin/developer-portal#2160