feat(mgmt): load external groups scoped to a specific SSO config - #1641
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersThe review assignment targets contributors with substantial and varied coverage of the core group management modules, including both sync and async components, to ensure thorough review of the new SSO scope features.
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. |
There was a problem hiding this comment.
🐕 Shuni's Review
Adds an optional sso_id filter to the three group-loading methods (sync + async), sent only when provided so older backends keep working.
Sniffed out 1 issue:
- 1 🟢 LOW:
ssoIdresponse field documented in only one of the three docstrings; README group section still lacks asso_idexample
Logic is correct and sync/async stay in lockstep; the conditional-body idiom matches the rest of descope/management. Only load_all_groups got an sso_id test — the other two methods are identical in shape, so that's a judgment call, but worth a copy-paste. Good bones! Woof!
Declared coverage: FULL — 3/3 changed files reviewed.
…lter # Conflicts: # descope/management/group.py # descope/management/group_async.py
Coverage reportThe coverage rate went from
Diff Coverage details (click to unfold)descope/management/group_async.py
descope/management/group.py
|
Renovate bumped the python_version < '3.10' mypy pin to 2.3.0 (#1569), which requires Python 3.10+, making the 3.9 resolution split unsatisfiable and breaking `uv sync --locked` for every CI job. Use a bounded range like the pytest entry so renovate cannot re-bump it past the last 3.9-compatible line. Co-Authored-By: Claude <noreply@anthropic.com>
…-sdk into feat/groups-sso-id-filter
|
🐕 Review complete — View session on Shuni Portal 🐾 |
There was a problem hiding this comment.
🐕 Shuni's Review
Re-review of the new push: docstrings for all three group loaders (sync + async) now document source and ssoId, README gained an sso_id example, and a bundled pyproject/uv.lock fix restores the mypy>=1.11.2,<1.12 bound for Python 3.9.
No new issues. My earlier note about ssoId missing from two of the three return-format docstrings is fixed.
The mypy pin mirrors the existing pytest>=8.4,<9 guard convention and realigns pyproject with the lock (they had drifted on main), and the lock specifier ordering matches the coverage[toml]>=7.3.1,<8 style, so uv lock --check stays happy. Only load_all_groups has an sso_id test — still a copy-paste away for the other two. Clean scent! Woof!
Declared coverage: PARTIAL — 5/6 changed files reviewed.
Related Issues
Required for:
https://github.com/descope/etc/issues/17382
Related PRs
Upstream PRs
In a Nutshell
sso_idparam on the three group-loading methods (sync + async)Description
Adds SSO-config scoping to
load_all_groups,load_all_groups_for_membersandload_all_group_members, matching the new backendssoIdfilter. The reserved iddefault_ssoidtargets the tenant's default SSO configuration; responses now include anssoIdfield per group identifying its origin SSO configuration.Must
🤖 Generated with Claude Code