feat(xaa): add cross-app access (ID-JAG) mapping types - #1664
Conversation
Adds XAAIssuerSettings + XAAJWTBearerSettings describing the per-issuer JIT + attribute/group-role mapping returned under a tenant's idJagSettings. Required for: descope/etc#17573 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersThis PR adds Cross-App Access (ID-JAG) mapping types to the SSO settings management module. All three candidate reviewers have direct experience with the affected file (descope/management/sso_settings.py), making them well-suited to review this change. Since the changes are localized to a single file and all candidates have relevant commit history, we recommend including the most active contributors to ensure thorough review coverage.
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. |
![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
Small, clean addition of two new type-only classes (XAAIssuerSettings, XAAJWTBearerSettings) for Cross-App Access (ID-JAG) trust config, mirroring the existing SAML/OIDC mapping types.
Sniffed out 1 issue:
- 1 🟡 MEDIUM: new types aren't exported from
descope/__init__.pylike every sibling type in this file
See inline comment for details. Woof!
Declared coverage: FULL — 1/1 changed files reviewed.
Coverage reportThe coverage rate went from
Diff Coverage details (click to unfold)descope/management/_sso_settings_base.py
descope/management/common.py
descope/management/sso_settings_async.py
descope/management/sso_settings.py
|
dorsha
changed the title
…ackage root Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Follow-up pass: the only change since the last review is commit 5b8e8b6, which re-exports XAAIssuerSettings and XAAJWTBearerSettings from descope/__init__.py.
Previous 🟡 MEDIUM finding (missing export) is now fixed and confirmed in-thread — no new issues found.
Good bones! Woof! 🐕
Declared coverage: PARTIAL — 1/2 changed files reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Big follow-up push: this PR now wires up the actual configure_xaa_settings / load_xaa_settings / load_all_xaa_settings / delete_xaa_settings API (sync + async), plus README docs, a sample app, and thorough request-body tests.
No new issues found — the wire-format composers, tests, README, and sample app are all internally consistent, and the async module correctly mirrors the sync one. The only prior finding (missing __init__.py export) was already fixed and confirmed resolved.
Good bones! Woof! 🐕
Declared coverage: FULL — 8/8 changed files reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Latest push is just ruff format (line-wrapping) on _sso_settings_base.py and test_sso_settings.py — no logic changes.
No new issues found. The prior finding remains fixed and resolved.
Clean paws, nothing to fetch here. Woof! 🐕
Declared coverage: PARTIAL — 2/8 changed files reviewed.
|
@shuni review |
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Adds Cross-App Access (ID-JAG) trust config: two new type-only classes plus the full configure/load/load_all/delete_xaa_settings API (sync + async), wire-format composers, README docs, a sample app, and thorough request-body tests.
No new issues found. The wire-format composers correctly reuse existing _role_mapping_to_dict/_attribute_mapping_to_dict/_fga_mappings_to_dict helpers, sso_id omission is handled consistently across configure/load/delete, and the sync/async modules mirror each other. The prior 🟡 MEDIUM finding (missing __init__.py export) is already fixed and confirmed resolved; the latest commit is formatting-only.
Good bones! Woof! 🐕
Declared coverage: FULL — 8/8 changed files reviewed.
Add provider_id to XAASettings and emit providerID on the XAA configure body when set, mirroring SSO SAML/OIDC so the chosen IdP round-trips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
|
🐕 Review complete — View session on Shuni Portal 🐾 |
Update the XAAIssuerSettings/XAAJWTBearerSettings docstrings + README to drop the "returned by the tenant get-settings API" claim - XAA is no longer surfaced on tenant load; read it via load_xaa_settings / load_all_xaa_settings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
|
@shuni fix coverage comments |
|
🐕 Done! Updated PR: #1664 📋 Actions TakenView AI responseThe coverage bot flagged two uncovered lines in
Full suite: 1057 passed, coverage 98.27% (was 98.23%), |
Adds cases for configure_xaa_settings with no jwt-bearer settings and with an issuer entry left at defaults (None), covering the two guard clauses flagged as missing by the coverage bot.
|
🐕 Review complete — View session on Shuni Portal 🐾 |
2 participants
Required for:
https://github.com/descope/etc/issues/17573
Adds Cross-App Access (ID-JAG) per-issuer JIT + attribute/group-role mapping types.