Add Consumer API OpenID4VP verifier site - #1536
Conversation
63b3d48 to
3e8d6db
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 59581c999c
ℹ️ 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".
| if (credentialResult.isValid) { | ||
| return { | ||
| display: extractDisplayFromObject(payload), | ||
| isValid: true | ||
| }; |
There was a problem hiding this comment.
When TokenContentVerifiablePresentation.value is a standalone signed JWT VC, this branch marks it valid after issuer-signature verification without ever comparing the expected nonce or audience. The same credential can therefore be replayed under any reference and displayed as valid; require a request-bound VP or reject standalone formats that cannot prove the nonce and audience.
AGENTS.md reference: Applications/ConsumerApi/src/OpenId4VpVerifierSite/AGENTS.md:L29-L35
Useful? React with 👍 / 👎.
| if (Array.isArray(value)) { | ||
| return value.flatMap(normalizeToken); | ||
| } |
There was a problem hiding this comment.
For an input such as [validPresentation, 42], flatMap(normalizeToken) silently removes the invalid member because primitive values normalize to an empty array. Only the valid artifact is subsequently checked, so the whole response is reported valid even though not every supplied presentation element passed validation.
AGENTS.md reference: Applications/ConsumerApi/src/OpenId4VpVerifierSite/AGENTS.md:L37-L39
Useful? React with 👍 / 👎.
| const isValid = verifiedArtifacts.length > 0 && verifiedArtifacts.every((artifact) => artifact.isValid); | ||
| const firstCredential = verifiedArtifacts.map((artifact) => artifact.display).find(Boolean) ?? {}; | ||
| const firstErrorCode = verifiedArtifacts.find((artifact) => !artifact.isValid)?.errorCode; |
There was a problem hiding this comment.
When value contains multiple presentations or credentials, validation checks every artifact but retains only the first available display. Claims from all subsequent valid artifacts are discarded, so the verifier does not show all of the subject data that was actually presented.
AGENTS.md reference: Applications/ConsumerApi/src/OpenId4VpVerifierSite/AGENTS.md:L68-L68
Useful? React with 👍 / 👎.
| const detailFields = ["createdAt", "expiresAt", "issuer", "publicKey"] as const; | ||
| type DetailField = (typeof detailFields)[number]; | ||
| const validationErrorMessages: Record<PresentationValidationErrorCode, string> = { | ||
| [PresentationValidationErrorCode.AudienceMismatch]: "Dieser Nachweis wurde für eine andere Prüfung erstellt. (AudienceMismatch)", |
There was a problem hiding this comment.
Every mapped message exposes an English implementation identifier such as (AudienceMismatch) to screen-reader users through the data-error element. These identifiers are technical rather than user-facing German text and should be removed from all entries while retaining the understandable explanation.
AGENTS.md reference: Applications/ConsumerApi/src/OpenId4VpVerifierSite/AGENTS.md:L66-L66
Useful? React with 👍 / 👎.
| display: none !important; | ||
| } | ||
|
|
||
| .verifier { |
There was a problem hiding this comment.
The stylesheet is loaded globally on the onboarding page, but this selector and most following selectors use generic names such as .intro, .details, and .result. They can consequently restyle unrelated markup on this shared page; prefix them with .openid4vp-verifier or #openid4vp-verifier-root as required for this bundle.
AGENTS.md reference: Applications/ConsumerApi/src/OpenId4VpVerifierSite/AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| </head> | ||
| <body> | ||
| <body class="openid4vp-verifier-visible"> |
There was a problem hiding this comment.
When JavaScript is enabled but the module fails to download, parse, or initialize, this server-rendered class immediately hides #app-onboarding-content and leaves every /r/{referenceId} page on the verifier spinner indefinitely. The <noscript> fallback does not cover module-load failures, while main.ts already adds this class after it has successfully found the verifier DOM, so the initial markup should not enable it.
Useful? React with 👍 / 👎.
f9cb1c5 to
5b79be5
Compare
1 participant
Summary
/r/{referenceId}onboarding page with an embedded mobile-first OpenID4VP verifierAppOnboarding.cshtml; the TypeScript bundle now only controls existing DOM nodes and performs browser-only loading, decryption, validation, and state updates@nmshd/crypto, and fall back to the previous onboarding flow when the content is not a VP tokenTokenContentVerifiablePresentation.valuewith the existing Credo-based verifier and render loading/valid/invalid/closed states without a mock app/phone shellOpenId4VpVerifierSite/distand let the Consumer API MSBuild target copy it intowwwroot/openid4vp-verifierat local build time; generatedwwwrootassets are no longer committedNotes
3/XCHACHA20_POLY1305is handled.node:fsexternalization and directevalcome from transitive crypto/Credo dependencies.Screenshot