Skip to content

fix(auth): check the target org's role, not the active org's, in org-SSO gates#4943

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/org-sso-target-org-role
Jul 21, 2026
Merged

fix(auth): check the target org's role, not the active org's, in org-SSO gates#4943
pedrofrxncx merged 1 commit into
mainfrom
fix/org-sso-target-org-role

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Follows the same bug class fixed in #4934 (member-add/update-role) and #4926 (AccessControl fast-path). isOrgAdmin/isOrgOwner in org-sso.ts gated SSO config read/write/enforce/delete on ctx.auth.user?.role, which reflects the session's active organization — not ctx.organization (the path-resolved target org that resolveOrgFromPath sets for /api/:org/sso/* routes). A user who is owner/admin of one org but only a plain member of another could read, create, enforce, or delete that other org's SSO config (OIDC issuer/client secret) simply by navigating to its org-scoped SSO settings route, since ctx.organization.role is exactly the field resolveOrgFromPath documents for this purpose (apps/mesh/src/core/studio-context.ts:230-236) but the file never used it.

Fix: read ctx.organization?.role instead, via the existing hasAdminRole helper (also picks up comma-joined multi-role owners/admins, matching #4926).

Failure scenario + regression test: apps/mesh/src/api/routes/org-sso.test.ts constructs a context where the active-org role is "owner" but the target org's (ctx.organization.role) is "user" — asserts isOrgAdmin/isOrgOwner return false. This test fails on the pre-fix code (confirmed locally by reverting the fix) and passes after.

Reviewer check: cd apps/mesh && bun test src/api/routes/org-sso.test.ts

Locally verified: bun run fmt, bunx tsc --noEmit (apps/mesh), and the targeted test above. Full CI covers the rest.


Summary by cubic

Fixes org SSO access control by checking the target org’s role, not the session’s active org. Prevents users with admin/owner role in a different org from managing another org’s SSO settings.

  • Bug Fixes
    • isOrgAdmin and isOrgOwner now read ctx.organization.role and use hasAdminRole.
    • Support comma-joined multi-role memberships and correctly detect owner.
    • Added regression tests to cover active-org vs target-org role mismatches and missing roles.

Written for commit 5e1ffd9. Summary will update on new commits.

Review in cubic

…SSO gates

isOrgAdmin/isOrgOwner read ctx.auth.user.role, which reflects the session's
active org, not ctx.organization (the path-resolved target org set by
resolveOrgFromPath). An owner/admin of one org could manage SSO config
(read, create, enforce, delete) for a different org they only hold a
lower-privileged membership in.

Also recognizes comma-joined multi-role owner/admin via hasAdminRole,
matching the same fix already applied to canAssignRole/AccessControl.
@pedrofrxncx
pedrofrxncx enabled auto-merge (squash) July 21, 2026 16:44
@pedrofrxncx
pedrofrxncx merged commit 0b5b8d7 into main Jul 21, 2026
15 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/org-sso-target-org-role branch July 21, 2026 16:47
decocms Bot pushed a commit that referenced this pull request Jul 21, 2026
PR: #4943 fix(auth): check the target org's role, not the active org's, in org-SSO gates
Bump type: patch

- decocms (apps/mesh/package.json): 4.83.5 -> 4.83.6

Deploy-Scope: server
pedrofrxncx added a commit that referenced this pull request Jul 21, 2026
…n dispatch-run (#4947)

Follows #4943/#4944 hardening. `fetchModelPermissions` in dispatchRunAndWait's
resume/automation re-entry path used `ctx.auth.user?.role` (the session's
active-org role) instead of the path-resolved role for `input.organizationId`,
letting an owner/admin of one org bypass model-tier restrictions when a
dispatch targets a different, lower-privileged org. #4944 already fixed the
same fetchModelPermissions call in routes.ts/key-list.ts/list-models.ts but
missed this call site.
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.

1 participant