feat(DATA-3217): Docs for maadhaar - #451
Conversation
Checklist to merge a PR 🚀To merge this pull request, please take time to complete the checklist. What action did you perform?Review the corresponding checklist items for the action you performed and mark them done. Edit an existing content (MDX) pageChecklist
Edit an existing API reference pageChecklist
Add a new content (MDX) pageChecklist
Add a new API reference pageChecklist
|
hr1shabh
left a comment
hr1shabh
left a comment
There was a problem hiding this comment.
Here is a detailed line-by-line review. I've uncovered some critical schema validation issues, API contradictions, and potential deep link bugs that should be addressed.
| "$ref": "#/components/schemas/AadhaarOutput" | ||
| }, | ||
| { | ||
| "type": "null" |
There was a problem hiding this comment.
In the WebhookResponse schema, aadhaar_output uses anyOf with {"type": "null"}. However, the document specifies "openapi": "3.0.0", and OpenAPI 3.0 does not support the null type (it was introduced in 3.1). To make this valid OpenAPI 3.0, you should remove anyOf and instead use "nullable": true alongside allOf: [{ $ref: ... }] (similar to how it's done for StatusResponse).
There was a problem hiding this comment.
made the change
| } | ||
| }, | ||
| "WebhookResponse": { | ||
| "type": "object", |
There was a problem hiding this comment.
There is a systemic naming inconsistency between camelCase traceId and snake_case trace_id across the API:
QRGenerateResponse,IntentPayloadResponse, andErrorResponsereturntraceIdat the root.StatusResponseandWebhookResponsereturntrace_idat the root.- More confusingly,
StatusResponsereturnstrace_idat the root, but the nestederrorobject inside it usestraceId.
Is this intentional? Mixing casing conventions in the same API will likely cause parsing bugs for developers.
There was a problem hiding this comment.
made consistent to traceId
| "type": "object", | ||
| "required": ["txn_id", "sd_jwt"], | ||
| "properties": { | ||
| "txn_id": { |
There was a problem hiding this comment.
In the /api/maadhaar/process-sd-jwt endpoint, the request body requires txn_id, but the documentation instructs the developer to "Use the original workflow_id as txn_id." If these represent the exact same value in this flow, should the API field simply be renamed to workflow_id for consistency?
There was a problem hiding this comment.
valid point, will raise with Product, but right now its txn_id in request and some clients have started using the apis so we shouldnt be changing it right now
| <CodeBlockWithCopy> | ||
| {`var components = URLComponents() | ||
| components.scheme = "pehchaan" | ||
| components.host = "in.gov.uidai.pehchan" |
There was a problem hiding this comment.
For iOS App-to-App (and Web-to-App), the intent scheme is pehchaan (double 'a'), but the host is in.gov.uidai.pehchan (single 'a'). Meanwhile, the Android intent uses in.gov.uidai.pehchaan.INTENT_REQUEST (double 'a'). Is the single 'a' in pehchan a typo in the iOS URL host, or is it an actual inconsistency imposed by UIDAI's mobile apps?
There was a problem hiding this comment.
its intentional, the working code has the exact same intent link usage, this was provided by UIDAI
| If the user aborts authentication or UIDAI returns an authentication error, the | ||
| webhook is sent with `status: "error"`, the error code and message, and no | ||
| Aadhaar output. For example, UIDAI error code `303` means that the user | ||
| aborted the authentication process: |
There was a problem hiding this comment.
The docs note "UIDAI error code 303 means that the user aborted". However, the HTTP API examples return text-based error codes (e.g., "INVALID_REQUEST", "NO_ERROR"). Will the API return error codes as numeric strings (e.g., "303") in some cases and text constants in others? Mixed typing here could easily break client-side switch statements.
There was a problem hiding this comment.
added in description that in webhook response we'll send uidai response code which is string in data type but contains numberic data
| "error": { | ||
| "code": "INVALID_REQUEST", | ||
| "detail": "List of desired Aadhaar fields", | ||
| "traceId": "2feb81d0-a84f-4885-9e3e-ba6f2946b288" |
There was a problem hiding this comment.
In the 400 Bad Request error examples, the detail message is literally "List of desired Aadhaar fields". This reads like a property description rather than an actionable error message. Should this be updated to a more descriptive backend error like "Missing or invalid list of desired Aadhaar fields"?
There was a problem hiding this comment.
changed example to a better one
| "status": "success", | ||
| "trace_id": "1-68d122f6-3732177863083023409ea933", | ||
| "txn": "ca44473f-00ad-4744-a430-39b83c01cfb6", | ||
| "workflow_id": "ca44473f-00ad-4744-a430-39b83c01cfb6" |
There was a problem hiding this comment.
In the success examples, the returned txn (UIDAI transaction identifier) exactly matches the workflow_id (a UUID generated by the merchant). Does UIDAI actually echo back the merchant's UUID as the transaction ID? If txn is generated independently by UIDAI, these examples should use different strings so developers don't falsely assume they will always match.
There was a problem hiding this comment.
no these are exactly the same for now in the current implementation, so these have to be same
there is a separate implementation (not implemented by uidai yet) where they use jti, then the txn will be different so sending txn makes sense
2 participants
No description provided.