-
Notifications
You must be signed in to change notification settings - Fork 88
[INJICERT-976] Add Pre auth code md and update claim169 md file #599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Piyush7034 <[email protected]>
Signed-off-by: Piyush7034 <[email protected]>
WalkthroughReorganized Claim-169 QR documentation to nest QR fields under a Changes
Sequence Diagram(s)sequenceDiagram
participant Certify
participant IssuerPortal
participant Wallet
participant TokenEndpoint
participant CredentialEndpoint
Certify->>IssuerPortal: Prepare credential offer (pre-authorized-data)
IssuerPortal-->>Certify: Expose issuer metadata (/.well-known/*)
Wallet->>IssuerPortal: Retrieve credential-offer-data/{offer_id}
Wallet->>TokenEndpoint: POST token (grant_type=pre-authorized_code, pre-authorized_code [+ optional PIN])
TokenEndpoint-->>Wallet: Access token
Wallet->>CredentialEndpoint: GET /credential (Authorization: Bearer)
CredentialEndpoint-->>Wallet: Issued VC (may include claim169 QR entries)
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/Claim-169-QR-Code-Support.md (1)
25-33: Make JSON example valid (quote QR value).The example uses
identityQRCode: $claim_169_values[0]without quotes, which is invalid JSON and inconsistent with later examples. Use a quoted placeholder string.✅ Suggested fix
- "identityQRCode": $claim_169_values[0] + "identityQRCode": "$claim_169_values[0]"
🤖 Fix all issues with AI agents
In `@docs/Claim-169-QR-Code-Support.md`:
- Around line 180-181: The sentence contains a dangling inline code placeholder
``; update it to either the actual method name used for QR code signing in
com.mosip.certify.credential.Credential (for example Credential.signQRCode or
the exact method name) or remove the backticks and reword to read “the default
implementation in com.mosip.certify.credential.Credential” so the reference is
concrete and not empty.
In `@docs/Pre-Authorized-Code.md`:
- Line 10: The emphasis marker in the bullet "Interoperability" is malformed;
update the text that currently reads "**Interoperability:* Aligns with
OpenID4VCI standards, improving compatibility with other identity systems and
wallets." to properly close the bold markers—e.g., change "**Interoperability:*"
to "**Interoperability:**" so the line becomes "**Interoperability:** Aligns
with OpenID4VCI standards, improving compatibility with other identity systems
and wallets."
🧹 Nitpick comments (2)
docs/Pre-Authorized-Code.md (2)
4-4: Call out that this flow is optional/config-driven.Given the service initializes even when pre-authorized flow is not configured, it’s worth noting in this doc that the flow is optional and depends on configuration (e.g., credential-offer URL). This avoids overstating it as mandatory behavior. Based on learnings, the feature is optional and defaults to empty config.
Based on learnings: “the pre-authorized code flow is optional… defaults to an empty string to allow the component to initialize even when this feature is not configured or used.”
48-48: Reduce repetitive sentence openings for readability.Three consecutive sentences start with “Certify…”. Consider rephrasing one or two to improve flow.
Signed-off-by: Piyush7034 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/Pre-Authorized-Code.md`:
- Around line 146-148: Change the British spelling "authorisation" to the
American spelling "authorization" in the "## Limitations" section sentence that
currently reads "Client authorisation is not supported." (look for the "##
Limitations" heading and the sentence about single issuance mode for an issuer)
so the document uses "authorization" consistently with the rest of the file.
🧹 Nitpick comments (1)
docs/Claim-169-QR-Code-Support.md (1)
180-180: Consider using proper heading syntax for "Extensibility".The "Extensibility" line uses bold formatting instead of proper heading syntax. For better document structure, navigation, and accessibility, consider using a heading level (e.g.,
### Extensibility).📋 Suggested change
-**Extensibility** +### Extensibility
Signed-off-by: Piyush7034 <[email protected]>
|



Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.