feat(sdk): implement management sdk parity between php-sdk and go-sdk - #122
Conversation
Implement the management endpoints the PHP SDK was missing relative to the Go SDK, wiring them into the Management component: - Tenant: create/update/delete/load/loadAll/searchAll - Permission: create/update/delete/loadAll - Role: create/update/delete/loadAll/search (CRUD alongside existing JWT validation) - AccessKey: create/load/searchAll/update/activate/deactivate/delete - SSOApplication: OIDC/SAML create+update, delete/load/loadAll - SSOSettings: loadSettings, configure OIDC/SAML (+ by metadata), deleteSettings - JWT: updateJWT, impersonate - Flow: listFlows/delete/exportFlow/importFlow/exportTheme/importTheme Add flow/theme import endpoint paths to MgmtV1, document every new component in the README, and add a runnable ManagementParityTest plus per-module integration tests matching the existing self-skipping convention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersThe review assignment strategy prioritizes recent contributors who worked on overlapping files and areas of the SDK, ensuring knowledgeable review coverage of the new module implementations, test integration, and overall project consistency.
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-dev[bot]](https://avatars.githubusercontent.com/u/97479186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐕 Shuni's Review
Adds 8 Management SDK modules (Tenant, Permission, Role, AccessKey, SSOApplication, SSOSettings, JWT, Flow) plus wiring and tests to reach parity with the Go SDK.
Sniffed out 1 issue:
- 1 🟠 HIGH:
impersonate()typesrefreshDurationas string, sending a JSON string instead of a number
Solid parity work — good bones, just one type mismatch to fix. Woof!
|
@shuni let's add also the missing "auth" (such as logout..) and not only "mgmt". |
|
🐕 Done! Updated PR: #122 📋 Actions TakenView AI responseDone. Both requests handled and committed to Code review commentsFixed the one finding from my earlier review: Auth parity (new)Added the auth-side gaps whose endpoint paths were already scaffolded in
All reuse the existing Commits: Two honesty flags for the human reviewer:
|
Addresses code review: JWT::impersonate() declared $refreshDuration as ?string while the sibling updateJWT() uses ?int and the field is a duration in seconds. A string would serialize as a JSON string instead of a number. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends auth-side parity with the Go/Python SDKs alongside the existing management modules: - OTP module (signUp, signIn, signUpOrIn, verifyCode) - MagicLink module (signUp, signIn, signUpOrIn, verify) - DescopeSDK session methods: selectTenant, exchangeAccessKey, history All reuse the existing API doPost/doGet pattern and scaffolded EndpointsV1 paths. Adds mock-based AuthParityTest (wiring + behavior, no network) and README docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@shuni fix ci failure (PR title) |
|
🐕 Done! Updated PR: #122 📋 Actions TakenView AI responseI've confirmed the root cause, but hit a hard constraint. Let me be direct about it. The CI failure is the The fix is a one-line PR-title change — there's no code/repo change that can satisfy a PR-title validator. I attempted Set the PR title to: Why this passes:
Alternatives that also pass: No commits were made this turn — a PR-title check can only be cleared by editing the title, which needs PR-edit permissions not present in this environment. |
|
|
|
@shuni just fix the PR title |
|
🐕 Done! Updated PR: #122 📋 Actions TakenView AI responseI've now exhausted the credential paths available in this environment, and hit a hard permissions wall:
So I genuinely cannot change the PR title from inside this session — editing PR metadata requires write credentials that aren't exposed here (by design, PR operations run in the outer workflow, not the coding session). The title must be set to (by the workflow or a maintainer with write access): That clears the This is a genuine blocker — a PR-title validator can only be satisfied by editing the title, and no code/commit change on the branch can affect it. No commits were made this turn. |
|
|
dorsha
changed the title
dorsha
changed the title
2 participants
descope/etc#16608
View workflow run
Done. All quality gates green (65 tests / 277 assertions, PSR2 clean, license check passes) and committed to the task branch.
What I did
Closed the PHP↔Go management-SDK gaps by implementing the 8 modules whose endpoint paths were already scaffolded in
MgmtV1.phpbut had no implementation class, then wired them intoManagement:TenantPermissionRole(CRUD added)AccessKeySSOApplicationSSOSettings(->sso)JWTFlowAlso: added flow/theme import paths to
MgmtV1, documented every component in the README, and added a runnableManagementParityTest(wiring + method-existence, no network) plus per-module integration tests matching the repo's existing self-skip convention.All new modules reuse the established
API::doPost/doGet/doDelete(..., true)management-key pattern — no new abstractions.[8 modules + Role CRUD, tested, PSR2-clean] → skipped: Project (export/import/clone), Authz/FGA, and Group modules — no endpoint paths exist in MgmtV1 for them, so they're a larger separate surface; add when those backend endpoints are prioritized.Two things worth flagging for review: the SAML create/update signatures are wide (many nullable params modeled on the Go/Python SDKs) and the exact request-body field names for the newer endpoints should be confirmed against the current backend OpenAPI before release, since I couldn't reach the backend specs from here.
Created by Shuni 🐕