feat(management): add enforceSSOExclusions to tenant create and update - #775
feat(management): add enforceSSOExclusions to tenant create and update#775hagaikali wants to merge 4 commits into
Conversation
The /v1/mgmt/tenant/create and /v1/mgmt/tenant/update endpoints accept enforceSSOExclusions and the Go SDK sends it on every write, but the Node SDK had no way to supply it. Since tenant update is a full replace, any Node SDK tenant write silently cleared an enforceSSOExclusions list set from the console or another SDK. Also adds enforceSSOExclusions and roleInheritance to the Tenant read type; both are returned by GET /v1/mgmt/tenant but were missing, which made roleInheritance write-only and unpreservable in a read-modify-write.
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersThe review assignment balances breadth of coverage across core implementation, testing, and documentation, leveraging contributors with deep and focused experience on tenant management for an efficient and comprehensive review process.
Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best. |
|
@dorsha @itaihanski |
![shuni-bot[bot]](https://avatars.githubusercontent.com/in/2536750?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐕 Shuni's Review
Threads an optional enforceSSOExclusions?: string[] through tenant create/createWithId/update and adds enforceSSOExclusions + roleInheritance to the read-side Tenant type.
The code is clean — I checked every positional slot in all three signatures against the README examples and the test call sites, and they line up exactly. Appending the param last keeps existing callers source-compatible. All findings below are in the README.
Sniffed out 4 issues:
- 1 🟡 MEDIUM:
enforceSSOExclusionslabel says "user IDs" but the example passes an email - 3 🟢 LOW: stale
createWithIdexample, overbroad omit-warning, hardcodedenforceSSO: truein copy-paste examples
See inline comments. Good bones! Woof!
Use the README's user-ID placeholder instead of an email for enforceSSOExclusions, expand the createWithId example to show the same optional arguments, scope the full-replace warning to the parameters update actually takes, and note that the sample values are illustrative rather than defaults.
|
issues handled, requesting review @dorsha |
1 participant
The /v1/mgmt/tenant/create and /v1/mgmt/tenant/update endpoints accept enforceSSOExclusions and the Go SDK sends it on every write, but the Node SDK had no way to supply it. Since tenant update is a full replace, any Node SDK tenant write silently cleared an enforceSSOExclusions list set from the console or another SDK.
Also adds enforceSSOExclusions and roleInheritance to the Tenant read type; both are returned by GET /v1/mgmt/tenant but were missing, which made roleInheritance write-only and unpreservable in a read-modify-write.
Related Issues
No Issue submitted for it.
Noticed during day-to-day working and submitted a PR.
Description
management.tenant.create,createWithId, andupdatehad no way to sendenforceSSOExclusions, even though the endpoints they POST to accept it. Because tenantupdate is a full replace, this additionally made every Node SDK tenant write silently
erase an exclusions list configured from the console or another SDK.
This appends
enforceSSOExclusions?: string[]as the last parameter of all threemethods and sends it in the request body. With this change, the Node SDK's tenant
create/update request surface matches the Go SDK's field for field.
Must
create,createWithId, andupdateblocks inlib/management/tenant.test.tsto pass the new argument and assert it in the expectedPOST body. The existing parent-tenant test variants were intentionally left without the
new argument, so they continue to cover the omitted case. Full suite: 451/451 passing,
coverage above the configured thresholds (statements 97.89%, functions 98.41%,
lines 98.91% against a 97% gate).
createandupdateexamples now showenforceSSOExclusions, plus an explicit warning that fields omitted fromupdatearecleared.