Map SCIM groups to organization teams

[OmarMcAdam]

Summary

• Adds SCIM Groups endpoints that map groups to org Teams and group members to TeamMember rows.
• Supports group list/get/create/replace/patch/delete with bearer-token validation, duplicate team-name conflicts, and active org-member validation.
• Updates SCIM setup docs text and route-guard expectations now that Groups are supported.

Tests

• pnpm install --frozen-lockfile
• pnpm --filter @openwork-ee/den-db build
• pnpm --filter @openwork/email build
• ee/apps/den-api/node_modules/.bin/tsc -p ee/apps/den-api/tsconfig.json --noEmit
• pnpm --filter @openwork-ee/den-api exec bun test test/scim-auth-sync.test.ts test/route-guard-policy.test.ts
• pnpm --filter @openwork-ee/den-api build

Reviewer Evidence

No screen recording attached: this is a backend SCIM API change without a UI flow. The commands above cover typechecking, focused SCIM/route policy tests, and the den-api build.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
openwork-app	Ready	Preview, Comment	Jun 23, 2026 5:27pm
openwork-den	Ready	Preview, Comment	Jun 23, 2026 5:27pm
openwork-den-worker-proxy	Ready	Preview, Comment	Jun 23, 2026 5:27pm
openwork-landing	Ready	Preview, Comment, Open in v0	Jun 23, 2026 5:27pm

[cubic-dev-ai]

2 issues found across 5 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[Pablosinyores]

Went through this one carefully given it's SCIM provisioning, where tenancy isolation is the property that matters most. The good news: the org scoping is implemented correctly — I traced every Groups operation (list/get/create/replace/patch/delete) and they all resolve the team through an org-filtered lookup, so I couldn't find an IDOR:

.where(and(eq(TeamTable.id, teamId), eq(TeamTable.organizationId, input.provider.organizationId)))

A token for org A hitting an org-B group id gets a flat 404 (same as nonexistent), so there's no cross-org existence leak either. Members are validated as org members before insert, and the de-dup + unique TeamMember index prevent duplicate rows. PATCH parsing is also solid — op names lowercased, members[value eq "..."] filter handled case-insensitively, both Okta filter-remove and Azure value-array remove covered, and removing an absent member is a safe no-op.

A few things I'd want addressed before merge:

1. The tenancy property has no test. This is the single most important behavior in the PR and the new tests only exercise the pure helpers (applyScimGroupPatch, isDuplicateTeamNameError). There's no test that a token for org A can't get/put/patch/delete an org-B team, and no test of the 401 bad-token path or the member-org validation. Worth a DB-backed test locking in cross-org 404 + 401, since a future refactor could silently drop the org predicate. (Related defense-in-depth: getTeamMemberUserIds has no internal org filter and trusts callers to pass org-scoped ids — safe today, but an org predicate or an asserting comment would prevent a future caller leaking cross-org members.)

2. The Groups list ignores startIndex/count and has no cap. It returns every team in one response with totalResults == itemsPerPage == teams.length and no limit/offset. That (a) is unbounded — a large org returns all teams plus a member fan-out query in a single response, and (b) breaks paginating IdPs (Okta/Azure send ?startIndex=101&count=100); ignoring startIndex and re-returning the full set makes clients re-process or loop. Suggest reading startIndex/count, applying .limit(count).offset(startIndex-1) with count clamped to a hard max (e.g. 100), and computing totalResults from a separate count() query.

3. Duplicate-name race on replace/patch throws an uncaught 500. Create wraps its insert and maps the unique-constraint violation to 409 via isDuplicateTeamNameError, but updateScimGroupForProvider (used by both PUT and PATCH) only does the check-then-act pre-check and runs the transaction with no try/catch. The team_organization_name unique index keeps data consistent, but a concurrent rename into the same name after the pre-check throws ER_DUP_ENTRY uncaught → a generic non-SCIM 500 (and no sync-failure event recorded). Suggest wrapping the update with the same isDuplicateTeamNameError→409 handling as create, and having the route handlers emit a SCIM-shaped error on unexpected failures.

Smaller:

• An empty or display-name-only PATCH still deletes and re-inserts every TeamMember row (and bumps updatedAt) — functionally a no-op but it churns rows and briefly empties membership inside the txn. Diffing the member set and skipping the rewrite when unchanged would avoid that.
• parseScimGroupMembers returns a hard 400 for the whole request if any member value isn't a Den user id (and for path-less add/replace with a members array in value, which some Azure flows send). Fine if the integration contract guarantees Den user ids, but worth documenting.

Nice to see the delete path deliberately mirror the canonical app team-delete (SkillHubMember → TeamMember → Team) rather than inventing new cleanup. Mostly hardening + the pagination/error-shape items above.