1762 - Postman collection for sunbird registry#1986
Open
nandhu-kumar wants to merge 3 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 32 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR extends the eSignet Postman collection with Sunbird KBI support: OIDC client management and registry operations, DPoP/UserInfo and token exchange updates, wallet-linked OTP/signup refactor, well-known endpoint discovery, and an AuthCode flow using Sunbird client keys and KBI challenge generation. ChangesSunbird KBI Integration
Sequence DiagramsequenceDiagram
participant PostmanCollection
participant SunbirdAuthServer
participant SunbirdRegistry
PostmanCollection->>SunbirdAuthServer: OAuth Details Request (V2) (PKCE) -> receive transactionId + oauth_details_hash
PostmanCollection->>SunbirdAuthServer: Authorization Request (uses sunbird_client_id, scope, acrValues)
PostmanCollection->>SunbirdAuthServer: KBI Authenticate (sends sunbird_kbi_challenge)
SunbirdAuthServer-->>PostmanCollection: Authorization Code
PostmanCollection->>SunbirdAuthServer: Get Tokens (client_assertion signed with sunbird_client_private_key, X-XSRF-TOKEN, client_id=sunbird_client_id)
SunbirdAuthServer-->>PostmanCollection: Access Token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@postman-collection/eSignet.postman_collection.json`:
- Around line 561-563: The collection currently persists signing material via
pm.collectionVariables.set for "sunbird_client_private_key" (and similar sets)
which stores secrets in the collection; change the code to use environment scope
for secrets by replacing
pm.collectionVariables.set("sunbird_client_private_key", ...) with
pm.environment.set("sunbird_client_private_key", ...) and keep only non-secret
identifiers (e.g., "sunbird_client_id" and optionally
"sunbird_client_public_key") in collection variables; apply the same replacement
for the other occurrences referenced (lines around 4255-4264) so private keys
are not saved to collection scope.
- Around line 3355-3360: The collection extracts id_token into the variable
id_token and sets pm.globals.set("id_token_hint", id_token) but no downstream
requests consume {{id_token_hint}}; update the follow-up OIDC request(s) (the
signup-redirect / OIDC continuation requests) to read and use the captured value
by referencing {{id_token_hint}} where the id_token_hint is expected (e.g., as a
query param or in the Authorization header), or switch storage to
pm.environment.set("id_token_hint", id_token) if environment scope is required;
ensure the request(s) that should exercise the hinted flow include that template
variable so the flow actually continues.
- Around line 703-708: The POST request raw body currently hardcodes
"abhishek@gmail.com" in the request payload (see the "request" -> "body" ->
"raw" JSON in the eSignet.postman_collection entry); replace that hardcoded
email with a variable or generated value (for example use an environment
variable like {{sunbird_email}} or a Postman dynamic variable like
{{$randomEmail}}) and update the collection environment or seed logic to provide
that value so tests no longer write PII to a fixed mailbox.
- Around line 468-470: Replace the hard-coded client_secret value for the
"client_secret" key in the collection with a reference to a runtime/Environment
variable (e.g. set the value to a placeholder like "{{PMS_CLIENT_SECRET}}") and
remove the literal "ysd1exB4FXsZebbA" from the JSON; add the actual secret to
your Postman/CI environment or secret manager instead and rotate/ revoke the
exposed secret as needed.
- Around line 2773-2781: The detached JWS payload must always include the
snake_case fields and not conditionally omit them; remove the Array.isArray
guards and always set payload["accepted_claims"] and
payload["permitted_authorized_scopes"] after parsing, coercing non-array parse
results into an empty array (or throwing if you prefer strictness) and then
sorting the arrays (use the existing acceptedClaims and
permittedAuthorizedScopes variables and their sort calls) so the payload shape
remains fixed for signature reconstruction/verification.
- Around line 2566-2577: The headers in this request use underscores
("oauth_details_key" and "oauth_details_hash") but should match the hyphenated
header names used elsewhere; update the header keys to "oauth-details-key" and
"oauth-details-hash" (preserving their values like
{{oauth_details_key}}/{{oauth_details_hash}}) so the link-code request sends the
same oauth-details contract as other auth requests and validation will succeed;
leave "X-XSRF-TOKEN" unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3636d57-1bf7-46f7-9ef6-30c6b9545f51
📒 Files selected for processing (2)
postman-collection/eSignet-with-sunbird.postman_environment.jsonpostman-collection/eSignet.postman_collection.json
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
nandhu-kumar
force-pushed
the
develop-1762-local
branch
from
4bdb4a5 to
2fb7ffb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
postman-collection/eSignet.postman_collection.json (4)
2566-2577:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the hyphenated oauth-details headers here too.
This request is still sending
oauth_details_key/oauth_details_hashwhile the rest of the changed linked-auth flow usesoauth-details-key/oauth-details-hash. That mismatch is enough for this step to miss the oauth-details context and fail validation.Suggested fix
- { - "key": "oauth_details_key", - "value": "{{oauth_details_key}}" - }, - { - "key": "oauth_details_hash", - "value": "{{oauth_details_hash}}" - } + { + "key": "oauth-details-key", + "value": "{{oauth_details_key}}" + }, + { + "key": "oauth-details-hash", + "value": "{{oauth_details_hash}}" + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@postman-collection/eSignet.postman_collection.json` around lines 2566 - 2577, The request header keys oauth_details_key and oauth_details_hash are using underscores instead of the hyphenated form used elsewhere; update the header array in this request to use oauth-details-key and oauth-details-hash (replace oauth_details_key → oauth-details-key and oauth_details_hash → oauth-details-hash) so the request matches the linked-auth flow and validation.
3355-3360:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
id_token_hintis still captured but never used.This step stores
id_token_hint, but no later request in the provided collection reads{{id_token_hint}}. The signup-redirect flow still stops at extraction instead of exercising the hinted OIDC continuation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@postman-collection/eSignet.postman_collection.json` around lines 3355 - 3360, The test captures id_token into a global (pm.globals.set("id_token_hint", id_token)) but no subsequent request uses {{id_token_hint}}, so the signup-redirect OIDC continuation never runs; update the request that starts the OIDC continuation (the signup-redirect request or the request that should receive an id_token_hint) to include the variable {{id_token_hint}} in the appropriate place (querystring parameter, form field, or Authorization header) and ensure the saved token scope matches where it’s read (use pm.environment if other requests read environment variables); locate pm.globals.set("id_token_hint", id_token) and modify the downstream request(s) to reference {{id_token_hint}} so the hinted flow is exercised.
2773-2781:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the detached JWS payload shape fixed.
Conditionally omitting
accepted_claimsandpermitted_authorized_scopeschanges the signed payload for a flow where those fields are required. That breaks backend signature reconstruction as soon as either variable is absent or parses differently than expected.Suggested fix
- var payload = {} - var acceptedClaims = JSON.parse(pm.environment.get('accepted_claims')); - if(Array.isArray(acceptedClaims)){ - payload["accepted_claims"] = acceptedClaims.sort() - } - var permittedAuthorizedScopes = JSON.parse(pm.environment.get('permitted_authorized_scopes')); - if(Array.isArray(permittedAuthorizedScopes)){ - payload["permitted_authorized_scopes"] = permittedAuthorizedScopes.sort() - } + var payload = {}; + var acceptedClaims = JSON.parse(pm.environment.get('accepted_claims')); + payload["accepted_claims"] = acceptedClaims.sort(); + var permittedAuthorizedScopes = JSON.parse(pm.environment.get('permitted_authorized_scopes')); + payload["permitted_authorized_scopes"] = permittedAuthorizedScopes.sort();Based on learnings, this collection's detached JWS consent payload must keep the snake_case fields and treat both arrays as mandatory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@postman-collection/eSignet.postman_collection.json` around lines 2773 - 2781, The detached JWS payload currently omits accepted_claims or permitted_authorized_scopes when the environment values are missing or not arrays, which changes the signed payload; always include both snake_case fields in payload (payload["accepted_claims"] and payload["permitted_authorized_scopes"]) and treat them as mandatory arrays: parse each env var (accepted_claims and permitted_authorized_scopes), if parsing yields an array sort it and assign it, otherwise assign an empty array, ensuring the fields are always present and sorted before signing.Source: Learnings
561-563:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the Sunbird private key out of collection scope.
This flow still writes
sunbird_client_private_keyintopm.collectionVariables, reads it back from collection scope during token exchange, and declares it in the collection variable list. Saving or exporting the collection will carry signing material with it, and the token step will break as soon as the earlier secret-scope fix is applied only at client creation time.Suggested fix
- pm.collectionVariables.set("sunbird_client_id", clientId); - pm.collectionVariables.set("sunbird_client_private_key", JSON.stringify(privateKey_jwk)); - pm.collectionVariables.set("sunbird_client_public_key", JSON.stringify(publicKey_jwk)); + pm.collectionVariables.set("sunbird_client_id", clientId); + pm.environment.set("sunbird_client_private_key", JSON.stringify(privateKey_jwk)); + pm.collectionVariables.set("sunbird_client_public_key", JSON.stringify(publicKey_jwk));- const private_key_jwk = JSON.parse(pm.collectionVariables.get("sunbird_client_private_key")); + const private_key_jwk = JSON.parse(pm.environment.get("sunbird_client_private_key"));Also drop
sunbird_client_private_keyfrom the top-levelvariablearray.Also applies to: 4011-4014, 4255-4264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@postman-collection/eSignet.postman_collection.json` around lines 561 - 563, Do not store the Sunbird private key in collection scope: remove calls to pm.collectionVariables.set("sunbird_client_private_key", ...) and delete the "sunbird_client_private_key" entry from the collection-level "variable" array, and instead set the private key into a non-exported/secret scope used at runtime (e.g., environment or a transient runtime variable) and update the token-exchange step to read the key from that same secure scope; ensure all occurrences (the pm.collectionVariables.set usage, the token exchange read, and the collection variable declaration) are updated so only non-exportable secret storage contains the private key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@postman-collection/eSignet.postman_collection.json`:
- Line 3859: The request body in the "raw" payload hard-codes the OAuth "state"
value ("state": "eree2311"), breaking per-run correlation and CSRF protection;
change that literal to use the collection/environment variable (e.g., "state":
"{{state}}" or a Postman dynamic helper like "state": "{{$randomAlphaNumeric}}")
so the request uses a dynamic state value; update the "raw" JSON in the same
request object where the "state" property appears to reference the variable
instead of the fixed string.
---
Duplicate comments:
In `@postman-collection/eSignet.postman_collection.json`:
- Around line 2566-2577: The request header keys oauth_details_key and
oauth_details_hash are using underscores instead of the hyphenated form used
elsewhere; update the header array in this request to use oauth-details-key and
oauth-details-hash (replace oauth_details_key → oauth-details-key and
oauth_details_hash → oauth-details-hash) so the request matches the linked-auth
flow and validation.
- Around line 3355-3360: The test captures id_token into a global
(pm.globals.set("id_token_hint", id_token)) but no subsequent request uses
{{id_token_hint}}, so the signup-redirect OIDC continuation never runs; update
the request that starts the OIDC continuation (the signup-redirect request or
the request that should receive an id_token_hint) to include the variable
{{id_token_hint}} in the appropriate place (querystring parameter, form field,
or Authorization header) and ensure the saved token scope matches where it’s
read (use pm.environment if other requests read environment variables); locate
pm.globals.set("id_token_hint", id_token) and modify the downstream request(s)
to reference {{id_token_hint}} so the hinted flow is exercised.
- Around line 2773-2781: The detached JWS payload currently omits
accepted_claims or permitted_authorized_scopes when the environment values are
missing or not arrays, which changes the signed payload; always include both
snake_case fields in payload (payload["accepted_claims"] and
payload["permitted_authorized_scopes"]) and treat them as mandatory arrays:
parse each env var (accepted_claims and permitted_authorized_scopes), if parsing
yields an array sort it and assign it, otherwise assign an empty array, ensuring
the fields are always present and sorted before signing.
- Around line 561-563: Do not store the Sunbird private key in collection scope:
remove calls to pm.collectionVariables.set("sunbird_client_private_key", ...)
and delete the "sunbird_client_private_key" entry from the collection-level
"variable" array, and instead set the private key into a non-exported/secret
scope used at runtime (e.g., environment or a transient runtime variable) and
update the token-exchange step to read the key from that same secure scope;
ensure all occurrences (the pm.collectionVariables.set usage, the token exchange
read, and the collection variable declaration) are updated so only
non-exportable secret storage contains the private key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1aff3e55-62e0-41f2-85eb-00719399c294
📒 Files selected for processing (2)
postman-collection/eSignet-with-sunbird.postman_environment.jsonpostman-collection/eSignet.postman_collection.json
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit