Skip to content

fix: preserve idpinitiated.enabled:false on SAML connection export instead of emitting undefined_clientId - #1465

Open
harshithRai wants to merge 2 commits into
masterfrom
ESD-65927
Open

fix: preserve idpinitiated.enabled:false on SAML connection export instead of emitting undefined_clientId#1465
harshithRai wants to merge 2 commits into
masterfrom
ESD-65927

Conversation

@harshithRai

@harshithRai harshithRai commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

🔧 Changes

Fixes a serialization bug where exporting a SAML (samlp) connection with IDP-initiated login disabled produced a malformed idpinitiated block. Previously the exporter unconditionally injected a client_id, and when none existed the helper substituted the literal string undefined_clientId. On import this present block was treated as enabled, silently flipping IDP-initiated login ON in the target tenant.

  • getFormattedOptions in src/utils.ts (export path, ID to name) now only adds client_id when one is actually present; otherwise the idpinitiated block passes through untouched, preserving enabled: false.
  • getFormattedOptions in src/tools/auth0/handlers/connections.ts (diff/import path, name to ID) receives the same guard so both directions stay symmetric. A third resolver in src/tools/calculateDryRunChanges.ts was reviewed and is already guarded by ?.client_id, so it needs no change.

Note on scope: the guard omits client_id whenever it is falsy (enabled: false, empty string, null, or absent), not only the disabled case. This is the intended, more-correct behavior, but the output change is slightly wider than "disabled connections" alone.

Exported config shape change (both YAML and JSON/directory formats):

For a connection with IDP-initiated login disabled, the export now emits:

idpinitiated:
  enabled: false
"idpinitiated": {
  "enabled": false
}

instead of the previous malformed output:

idpinitiated:
  client_id: undefined_clientId
"idpinitiated": {
  "client_id": "undefined_clientId"
}

Connections that have IDP-initiated login enabled are unaffected and continue to export their resolved client_id.

🔬 Testing

  • Added a disabled IDP-initiated connection to the YAML dump test, asserting enabled: false round-trips without a bogus client_id. This covers both the YAML and directory export paths, since both share utils.getFormattedOptions.
  • Added a focused unit test on the handler's getFormattedOptions (the import/diff name-to-ID path) verifying no client_id is re-injected when a disabled { enabled: false } block is processed - directly pinning the reported "login flipped ON" symptom.
  • Full connections, dry-run, and YAML/directory context suites pass.

Verified end-to-end against a real tenant:

  • Created a samlp connection with idpinitiated.enabled: false via import.
  • Pre-fix build reproduced the bug: the export emitted enabled: false and a stray client_id: undefined_clientId.
  • Fixed build exported the block as enabled: false only, with no undefined_clientId placeholder, so the disabled state round-trips correctly.

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.51%. Comparing base (0a0bd11) to head (5154bb0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1465      +/-   ##
==========================================
+ Coverage   80.49%   80.51%   +0.02%     
==========================================
  Files         163      163              
  Lines        7741     7745       +4     
  Branches     1719     1721       +2     
==========================================
+ Hits         6231     6236       +5     
+ Misses        810      808       -2     
- Partials      700      701       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@harshithRai
harshithRai marked this pull request as ready for review August 20, 2026 05:28
@harshithRai
harshithRai requested a review from a team as a code owner August 20, 2026 05:28
ankita10119
ankita10119 previously approved these changes Aug 20, 2026

@ankita10119 ankita10119 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add a parallel test case in test/context/directory/connections.test.js for a disabled idpinitiated block to match the YAML coverage?

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.

3 participants