Skip to content

Commit dede150

Browse files
waleedlatif1claude
andcommitted
fix(scim): certification audit round
Three read-only auditors (IdP compatibility and docs, correctness and regressions, code economy and tests) reviewed the merged branch. Every verified finding is addressed here. Sync-breaking and regressions: - Accept provider extension URNs in `schemas` (Okta custom attributes, Entra custom extensions) and echo them on the resource; the check now lives in the write contracts. - A lapsed enterprise plan no longer keeps refusing manual membership changes, invitations, or SSO JIT on a directory's behalf. - Leaving a workspace no longer signs the leaver out (session spared). - OAuth access and refresh tokens are refused for a directory-suspended account, like sessions and personal keys. - Workspace-role changes for a directory-managed member are refused under membership locking, as the docs already said. - A permission group moved back to governing everyone no longer fails a sync with a 500; the grant is left on record and logged. - The explicit-membership flip after auto-mapping now happens after the projection, keeping the permission-group lock a leaf. - Request-log prune runs before each pass, so a failing pass still bounds the log; contract-validation failures now carry scimType and detail in the activity log; `application/scim+json; charset=utf-8`. Economy: shared helpers replace local copies (postgres error code, SHA-256 base64url, bearer parsing, email syntax, batch reconcile loop); unreachable defensive checks behind the contracts removed; half-applied base-URL injection removed from the route builder; casts, restating comments, and unused exports dropped. Docs: withdrawal semantics under locking stated precisely; SCIM env vars added to the self-hosted, SSO, and environment-variable references; group name uniqueness noted. Tests: extension URN acceptance and echo, managed-membership guard and its entitlement bypass, directory-only SSO admission, OAuth suspension, permissions-route guard, settle step ordering, and the weak assertions the audit flagged strengthened. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DGXcwsHzYGp6pUDWRtJKpz
1 parent 26817ff commit dede150

47 files changed

Lines changed: 577 additions & 261 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/docs/content/docs/platform/enterprise/scim.mdx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,11 @@ Mapping a permission group to a directory group switches that permission group t
133133

134134
## How access is withdrawn
135135

136-
Sim records every grant it makes on your behalf. When someone leaves a group, only what the directory granted is taken back — access a workspace administrator granted by hand stays.
136+
Sim records every grant it makes on your behalf. When someone leaves a group, what the directory granted is taken back.
137137

138-
The one exception is **managed membership locking**, which is on by default. With it on, the directory is the source of truth: Sim refuses invitations, workspace grants, and role changes for provisioned members, because the next sync would revert them anyway. Removals stay possible so an administrator can always act in an emergency. Turn it off if you want to layer manual access on top of directory access.
138+
**Managed membership locking**, on by default, makes the directory the source of truth for provisioned members: Sim refuses invitations, workspace grants, workspace role changes, and organization role changes for them, because the next sync would revert them anyway. Access a member already held by hand when a mapping started covering it counts as directory access from then on, so it is withdrawn with the mapping. Removals stay possible so an administrator can always act in an emergency.
139+
140+
With locking off, manual access layers on top of directory access: access granted by hand stays when a group is left, and a workspace role raised by hand above what the directory set is left alone.
139141

140142
## Provisioning and SSO together
141143

@@ -156,7 +158,8 @@ Sim also re-applies every group mapping once an hour, so drift cannot persist. Y
156158
- Resources: `/Users`, `/Groups`, plus `/ServiceProviderConfig`, `/ResourceTypes`, and `/Schemas`
157159
- Filters: `eq` only, up to ten terms joined with `and`. Users: `id`, `userName`, `externalId`, `emails.value` (also `emails[type eq "work"].value`), `active`. Groups: `id`, `displayName`, `externalId`
158160
- Limits: 1,500 requests per minute per connection, 1 MB per request, 5,000 members per group
159-
- `userName` is stored and returned lower-cased; attributes Sim does not model are kept and returned as sent, and a PUT preserves ones it omits
161+
- `userName` is stored and returned lower-cased; top-level attributes and schema extensions Sim does not model (custom attributes included) are kept and returned as sent, and a PUT preserves ones it omits
162+
- Group display names are unique within a connection, ignoring case
160163
- Page size: up to 100 per request
161164

162165
<FAQ items={[

apps/docs/content/docs/platform/enterprise/self-hosted.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Three features do not need a flag at all: **custom branding**, **session policie
4848
| Organizations | `ORGANIZATIONS_ENABLED` | `NEXT_PUBLIC_ORGANIZATIONS_ENABLED` |
4949
| Permission groups | `ACCESS_CONTROL_ENABLED` | `NEXT_PUBLIC_ACCESS_CONTROL_ENABLED` |
5050
| SAML and OIDC sign-in | `SSO_ENABLED` | `NEXT_PUBLIC_SSO_ENABLED` |
51+
| Directory provisioning (SCIM) | `SCIM_ENABLED` | `NEXT_PUBLIC_SCIM_ENABLED` |
5152
| Custom branding — on by default | `WHITELABELING_ENABLED` | `NEXT_PUBLIC_WHITELABELING_ENABLED` |
5253
| Audit logs | `AUDIT_LOGS_ENABLED` | `NEXT_PUBLIC_AUDIT_LOGS_ENABLED` |
5354
| Usage tracking | `USAGE_MONITORING_ENABLED` | `NEXT_PUBLIC_USAGE_MONITORING_ENABLED` |

apps/docs/content/docs/platform/enterprise/sso.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,10 @@ Self-hosted deployments use environment variables instead of the billing/plan ch
342342
SSO_ENABLED=true
343343
NEXT_PUBLIC_SSO_ENABLED=true
344344

345+
# Optional: directory provisioning (SCIM), configured from the SSO settings page
346+
SCIM_ENABLED=true
347+
NEXT_PUBLIC_SCIM_ENABLED=true
348+
345349
# Required if you want users auto-added to your organization on first SSO sign-in
346350
ORGANIZATIONS_ENABLED=true
347351
NEXT_PUBLIC_ORGANIZATIONS_ENABLED=true

apps/docs/content/docs/platform/self-hosting/environment-variables.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ Enterprise features are unlocked by configuration rather than billing on self-ho
251251
|----------|-------------|
252252
| `ENTERPRISE_ENABLED`, `NEXT_PUBLIC_ENTERPRISE_ENABLED` | Enable the whole enterprise feature set |
253253
| `SSO_ENABLED`, `NEXT_PUBLIC_SSO_ENABLED` | Enable SAML and OIDC single sign-on on its own. See [Authentication](/platform/self-hosting/authentication#sso-saml-and-oidc) |
254+
| `SCIM_ENABLED`, `NEXT_PUBLIC_SCIM_ENABLED` | Enable directory provisioning on its own. Needs SSO. See [Directory provisioning](/platform/enterprise/scim) |
254255
| `INSTANCE_ORG_NAME` | Name of the organization every user joins automatically at signup |
255256
| `INSTANCE_ORG_SLUG` | Slug for that organization (derived from the name when omitted) |
256257
| `INSTANCE_ORG_OWNER_EMAIL` | Owner of that organization (defaults to the first user to sign up) |

apps/sim/app/api/cron/scim-reconcile/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { toError } from '@sim/utils/errors'
2+
import { getErrorMessage } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { verifyCronAuth } from '@/lib/auth/internal'
55
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -25,7 +25,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
2525
logger.info('SCIM reconciliation sweep complete', sweep)
2626
return NextResponse.json({ success: true, ...sweep })
2727
} catch (error) {
28-
logger.error('SCIM reconciliation sweep failed', { error: toError(error).message })
28+
logger.error('SCIM reconciliation sweep failed', { error: getErrorMessage(error) })
2929
return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
3030
}
3131
})

apps/sim/app/api/scim/v2/Groups/[id]/route.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
patchScimGroup,
1111
replaceScimGroup,
1212
} from '@/ee/scim/lib/application/groups/manage-groups'
13-
import { assertGroupSchemas, toCanonicalGroup } from '@/ee/scim/lib/protocol/canonical'
13+
import { toCanonicalGroup } from '@/ee/scim/lib/protocol/canonical'
1414
import { parseAttributeProjection } from '@/ee/scim/lib/protocol/resources'
1515
import { defineScimRoute } from '@/ee/scim/lib/route'
1616

@@ -32,7 +32,6 @@ export const PUT = defineScimRoute({
3232
operation: replaceScimGroup.operation,
3333
useCase: replaceScimGroup,
3434
mapInput: ({ params, body }) => {
35-
assertGroupSchemas(body.schemas)
3635
return { groupId: params.id, group: toCanonicalGroup(body) }
3736
},
3837
present: (result) => result.resource,

apps/sim/app/api/scim/v2/Groups/route.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { createScimGroupContract, listScimGroupsContract } from '@/lib/api/contracts/scim'
22
import { createScimGroup, listScimGroups } from '@/ee/scim/lib/application/groups/manage-groups'
3-
import { assertGroupSchemas, toCanonicalGroup } from '@/ee/scim/lib/protocol/canonical'
3+
import { toCanonicalGroup } from '@/ee/scim/lib/protocol/canonical'
44
import { parseAttributeProjection, toListResponse } from '@/ee/scim/lib/protocol/resources'
55
import { defineScimRoute } from '@/ee/scim/lib/route'
66

@@ -24,7 +24,6 @@ export const POST = defineScimRoute({
2424
operation: createScimGroup.operation,
2525
useCase: createScimGroup,
2626
mapInput: ({ body }) => {
27-
assertGroupSchemas(body.schemas)
2827
return { group: toCanonicalGroup(body) }
2928
},
3029
present: (result) => result.resource,

apps/sim/app/api/scim/v2/Users/[id]/route.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
import { deprovisionScimUser } from '@/ee/scim/lib/application/users/deprovision-user'
88
import { getScimUser } from '@/ee/scim/lib/application/users/read-users'
99
import { patchScimUser, replaceScimUser } from '@/ee/scim/lib/application/users/update-user'
10-
import { assertUserSchemas, toCanonicalUser } from '@/ee/scim/lib/protocol/canonical'
10+
import { toCanonicalUser } from '@/ee/scim/lib/protocol/canonical'
1111
import { parseAttributeProjection } from '@/ee/scim/lib/protocol/resources'
1212
import { defineScimRoute } from '@/ee/scim/lib/route'
1313

@@ -29,7 +29,6 @@ export const PUT = defineScimRoute({
2929
operation: replaceScimUser.operation,
3030
useCase: replaceScimUser,
3131
mapInput: ({ params, body }) => {
32-
assertUserSchemas(body.schemas)
3332
return { scimUserId: params.id, attributes: toCanonicalUser(body) }
3433
},
3534
present: (result) => result.resource,

apps/sim/app/api/scim/v2/Users/route.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createScimUserContract, listScimUsersContract } from '@/lib/api/contracts/scim'
22
import { provisionScimUser } from '@/ee/scim/lib/application/users/provision-user'
33
import { listScimUsers } from '@/ee/scim/lib/application/users/read-users'
4-
import { assertUserSchemas, toCanonicalUser } from '@/ee/scim/lib/protocol/canonical'
4+
import { toCanonicalUser } from '@/ee/scim/lib/protocol/canonical'
55
import { parseAttributeProjection, toListResponse } from '@/ee/scim/lib/protocol/resources'
66
import { defineScimRoute } from '@/ee/scim/lib/route'
77

@@ -31,7 +31,6 @@ export const POST = defineScimRoute({
3131
operation: provisionScimUser.operation,
3232
useCase: provisionScimUser,
3333
mapInput: ({ body }) => {
34-
assertUserSchemas(body.schemas)
3534
return { attributes: toCanonicalUser(body) }
3635
},
3736
present: (result) => result.resource,

apps/sim/app/api/workspaces/[id]/permissions/route.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,18 @@ import {
1616
} from '@sim/testing'
1717
import { beforeEach, describe, expect, it, vi } from 'vitest'
1818

19-
const { mockSyncWorkspaceEnvCredentials, mockGetEffectiveWorkspacePermission } = vi.hoisted(() => ({
19+
const {
20+
mockSyncWorkspaceEnvCredentials,
21+
mockGetEffectiveWorkspacePermission,
22+
mockAssertMembershipNotScimManaged,
23+
} = vi.hoisted(() => ({
2024
mockSyncWorkspaceEnvCredentials: vi.fn(),
2125
mockGetEffectiveWorkspacePermission: vi.fn(),
26+
mockAssertMembershipNotScimManaged: vi.fn(),
27+
}))
28+
29+
vi.mock('@/ee/scim/lib/managed-membership', () => ({
30+
assertMembershipNotScimManaged: mockAssertMembershipNotScimManaged,
2231
}))
2332

2433
vi.mock('@sim/audit', () => auditMock)
@@ -37,6 +46,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
3746
getEffectiveWorkspacePermission: mockGetEffectiveWorkspacePermission,
3847
}))
3948

49+
import { ForbiddenOperationError } from '@/lib/core/application'
4050
import { PATCH } from '@/app/api/workspaces/[id]/permissions/route'
4151

4252
const mockGetSession = authMockFns.mockGetSession
@@ -558,6 +568,37 @@ describe('workspace permissions route', () => {
558568
})
559569
})
560570

571+
it('refuses a role change for a member the directory manages', async () => {
572+
queueOrgWorkspace([], [permissionRow(MEMBER_ID, 'read')])
573+
mockAssertMembershipNotScimManaged.mockRejectedValueOnce(
574+
new ForbiddenOperationError('SCIM_MANAGED_MEMBERSHIP', 'Managed by the directory')
575+
)
576+
577+
const response = await PATCH(
578+
createMockRequest('PATCH', { updates: [{ userId: MEMBER_ID, permissions: 'write' }] }),
579+
routeContext
580+
)
581+
582+
expect(response.status).toBe(403)
583+
await expect(response.json()).resolves.toMatchObject({ error: 'Managed by the directory' })
584+
expect(mockAssertMembershipNotScimManaged).toHaveBeenCalledWith(
585+
expect.objectContaining({ organizationId: ORG_ID, userId: MEMBER_ID })
586+
)
587+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
588+
})
589+
590+
it('does not consult the directory for a personal workspace', async () => {
591+
queuePersonalWorkspace([permissionRow(ADMIN_ID, 'admin'), permissionRow(MEMBER_ID, 'read')])
592+
dbChainMockFns.returning.mockResolvedValue([{ id: 'perm-1' }])
593+
594+
await PATCH(
595+
createMockRequest('PATCH', { updates: [{ userId: MEMBER_ID, permissions: 'write' }] }),
596+
routeContext
597+
)
598+
599+
expect(mockAssertMembershipNotScimManaged).not.toHaveBeenCalled()
600+
})
601+
561602
it('refuses to change the role of an organization admin', async () => {
562603
queueOrgWorkspace([{ userId: MEMBER_ID }], [permissionRow(MEMBER_ID, 'admin')])
563604

0 commit comments

Comments
 (0)