Skip to content

#1840 | Add Sunbird KBI flow and registry to postman collection#2170

Closed
nandhu-kumar wants to merge 1 commit into
mosip:develop-gofrom
nandhu-kumar:develop-go-local-1
Closed

#1840 | Add Sunbird KBI flow and registry to postman collection#2170
nandhu-kumar wants to merge 1 commit into
mosip:develop-gofrom
nandhu-kumar:develop-go-local-1

Conversation

@nandhu-kumar

@nandhu-kumar nandhu-kumar commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Closes #2179

Summary by CodeRabbit

  • New Features
    • Added Postman flows for Sunbird Registry Insurance, including creating, retrieving, and searching policies with seeded identity data.
    • Added optional policy deletion support controllable via an environment flag.
    • Added Postman flows for Sunbird KBI OAuth using PKCE, covering authorization, flow execution, callback, token exchange, and UserInfo validation.
    • Extended the local Postman environment with Sunbird-specific variables for identity details, registry URL, insurance record reference, and cleanup behavior.
  • Chores
    • Updated chart lint/publish workflow dispatch inputs to use YES/NO choice types.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The Postman collection adds Sunbird Registry Insurance CRUD operations and a PKCE-based Sunbird KBI flow. The Postman environment adds Sunbird variables, while chart workflow flags now use choice inputs.

Changes

Sunbird integration flows

Layer / File(s) Summary
Registry policy setup and CRUD
postman-collection/Go-eSignet.postman_environment.json, postman-collection/Go-eSignet.postman_collection.json
Adds identity and Registry variables, creates an Insurance policy, captures sunbird_insurance_id, and adds Get, Search, and optional Delete requests.
KBI PKCE authentication flow
postman-collection/Go-eSignet.postman_collection.json
Adds PKCE authorization, flow execution, callback, token exchange, and UserInfo retrieval with identity validation and sub assertions.

Chart workflow input typing

Layer / File(s) Summary
Typed chart dispatch inputs
.github/workflows/chart-lint-publish.yml
Changes CHART_PUBLISH and INCLUDE_ALL_CHARTS from string inputs to YES/NO choice inputs.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant Postman
  participant SunbirdRegistry
  participant SunbirdKBI
  participant SunbirdAuth
  Postman->>SunbirdRegistry: Create Insurance policy with seeded identity
  SunbirdRegistry-->>Postman: Return insurance OSID
  Postman->>SunbirdKBI: Authorize and execute identity flow with PKCE
  SunbirdKBI-->>Postman: Return authorization code and assertion
  Postman->>SunbirdAuth: Exchange code for tokens
  SunbirdAuth-->>Postman: Return access token
  Postman->>SunbirdAuth: Request UserInfo
Loading

Suggested reviewers: zesu22, anushasunkada

Poem

Policies bloom with names and dates,
PKCE unlocks identity gates.
Choices guide the charts along,
Tokens travel, flows stay strong.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding Sunbird KBI flow and registry content to the Postman collection.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/Go-eSignet.postman_collection.json`:
- Around line 1272-1361: The “Get Policy”, “Search Policy”, and “Delete Policy”
requests lack response test assertions. Add test event scripts to each request
that validate the expected HTTP status codes and response shape, matching the
assertion style and CRUD coverage used by “Create Policy (Identity)”.
- Around line 1234-1239: Update the “Capture insurance osid” test to safely
access result and Insurance before reading osid, so missing nested properties
produce a readable assertion failure rather than a TypeError. Keep the existing
undefined assertion and environment update for valid responses.
🪄 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: 6622f90c-c8b2-4daa-a9b7-06d7444f76a7

📥 Commits

Reviewing files that changed from the base of the PR and between 5de5762 and 00953b0.

📒 Files selected for processing (2)
  • postman-collection/Go-eSignet.postman_collection.json
  • postman-collection/Go-eSignet.postman_environment.json

Comment thread postman-collection/Go-eSignet.postman_collection.json
Comment thread postman-collection/Go-eSignet.postman_collection.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/Go-eSignet.postman_collection.json`:
- Around line 1211-1213: Update the comment in the request script to reference
the actual “5 — Sunbird KBI” folder instead of the stale “AuthCode Flow with
Sunbird KBI login” name, without changing the script behavior.
- Around line 1222-1224: Rename the Sunbird flow variables set in this script
from individual_id, full_name, and dob to dedicated sunbird_individual_id,
sunbird_full_name, and sunbird_dob variables. Apply the same rename consistently
in the Search Policy filters, Flow execute — individual ID prerequest guard and
request body, and UserInfo sub assertion, while leaving the shared identity
variables used by folders 1–3 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: 5b44df5b-2bb9-46df-91db-d25ec923778e

📥 Commits

Reviewing files that changed from the base of the PR and between 00953b0 and b6a99a4.

📒 Files selected for processing (2)
  • postman-collection/Go-eSignet.postman_collection.json
  • postman-collection/Go-eSignet.postman_environment.json

Comment thread postman-collection/Go-eSignet.postman_collection.json Outdated
Comment thread postman-collection/Go-eSignet.postman_collection.json Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/Go-eSignet.postman_collection.json`:
- Around line 1558-1599: Update the “Flow execute — select acr” test script to
clear the existing challenge_token and action_ref environment values before
setting values from the response. Ensure a successful response lacking
challengeToken or the next action leaves both variables unset, preventing the
subsequent flow request from reusing stale transition state.
- Around line 1339-1366: Move the “Delete Policy” request to after the Sunbird
KBI/UserInfo requests so top-to-bottom runs retain the seeded insurance record
until those flows complete. Keep deletion opt-in by gating the request with the
existing sunbird_cleanup mechanism if one is available.
🪄 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: f524c2ee-cf33-42aa-9fa1-18c5e950f0be

📥 Commits

Reviewing files that changed from the base of the PR and between b6a99a4 and 59db2c5.

📒 Files selected for processing (2)
  • postman-collection/Go-eSignet.postman_collection.json
  • postman-collection/Go-eSignet.postman_environment.json

Comment thread postman-collection/Go-eSignet.postman_collection.json
Comment thread postman-collection/Go-eSignet.postman_collection.json
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
postman-collection/Go-eSignet.postman_collection.json (1)

1349-1352: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require the literal true before deleting.

Line 1349 treats every non-empty value as opt-in, so sunbird_cleanup=false still sends the DELETE and removes the policy before KBI. Postman skips the request only when pm.execution.skipRequest() is reached. (learning.postman.com)

Proposed fix
-if (!pm.environment.get('sunbird_cleanup')) {
+if (pm.environment.get('sunbird_cleanup') !== 'true') {
   console.log('Skipping Delete Policy — set sunbird_cleanup=true to enable.');
   pm.execution.skipRequest();
 }
🤖 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/Go-eSignet.postman_collection.json` around lines 1349 -
1352, Update the cleanup guard in the delete-policy pre-request script to enable
deletion only when the `sunbird_cleanup` environment value is exactly the
literal string `true`; treat missing, empty, and `false` values as disabled and
preserve the existing skipRequest behavior.
🤖 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/Go-eSignet.postman_collection.json`:
- Around line 1534-1545: Clear the stale transition state before validating
responses in both the shown resume test and the corresponding select acr test:
unset action_ref and challenge_token before parsing or assertions, then retain
the guarded action_ref assignment only when a returned action exists. Apply the
same two unset calls and guarded assignment consistently in both request
handlers.

---

Duplicate comments:
In `@postman-collection/Go-eSignet.postman_collection.json`:
- Around line 1349-1352: Update the cleanup guard in the delete-policy
pre-request script to enable deletion only when the `sunbird_cleanup`
environment value is exactly the literal string `true`; treat missing, empty,
and `false` values as disabled and preserve the existing skipRequest behavior.
🪄 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: 13bbe38a-af43-425f-951c-c5c790bc44e3

📥 Commits

Reviewing files that changed from the base of the PR and between 59db2c5 and 69647c6.

📒 Files selected for processing (3)
  • .github/workflows/chart-lint-publish.yml
  • postman-collection/Go-eSignet.postman_collection.json
  • postman-collection/Go-eSignet.postman_environment.json

Comment on lines +1534 to +1545
"pm.test('resume 200', () => pm.response.to.have.status(200));",
"const j = pm.response.json();",
"pm.test('INCOMPLETE', () => pm.expect(j.flowStatus).to.eql('INCOMPLETE'));",
"pm.test('username prompt', () => {",
" const ids = (j.data && j.data.inputs || []).map(i => i.identifier);",
" pm.expect(ids).to.include('username');",
"});",
"pm.test('challenge_token', () => pm.expect(j.challengeToken).to.be.a('string').and.not.empty);",
"pm.environment.set('challenge_token', j.challengeToken);",
"if (j.data && j.data.actions && j.data.actions[0]) {",
" pm.environment.set('action_ref', j.data.actions[0].ref);",
"}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clear transition state before validating each response.

resume preserves an old action_ref when no action is returned. select acr can similarly fail its test and throw before replacing the old value. Since environment values persist, later requests can reuse a previous transition. (learning.postman.com)

Proposed fix
 pm.test('resume 200', () => pm.response.to.have.status(200));
+pm.environment.unset('challenge_token');
+pm.environment.unset('action_ref');
 const j = pm.response.json();
 ...
-if (j.data && j.data.actions && j.data.actions[0]) {
-  pm.environment.set('action_ref', j.data.actions[0].ref);
-}
+const nextAction = j.data && j.data.actions && j.data.actions[0];
+pm.test('action_ref', () => pm.expect(nextAction && nextAction.ref).to.be.a('string').and.not.empty);
+if (nextAction && nextAction.ref) pm.environment.set('action_ref', nextAction.ref);

Apply the same two unset calls and guarded assignment in select acr.

Also applies to: 1583-1588

🤖 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/Go-eSignet.postman_collection.json` around lines 1534 -
1545, Clear the stale transition state before validating responses in both the
shown resume test and the corresponding select acr test: unset action_ref and
challenge_token before parsing or assertions, then retain the guarded action_ref
assignment only when a returned action exists. Apply the same two unset calls
and guarded assignment consistently in both request handlers.

required: false
default: 'NO'
type: string
type: choice

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of this change in this kbi related pr?

@sacrana0

Copy link
Copy Markdown
Collaborator

In the sunbird postman collection I think we should directly pass only the kbi-acr in the acr_values field in authorize request.
It will directly skip the select acr stage and directly proceed with kbi login. @anushasunkada what is your view on it?

@nandhu-kumar

Copy link
Copy Markdown
Contributor Author

closing this PR to open new one to verify the action checks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants