Conversation
… plan (v2) Reviewed the original Phase 7 SCIM plans (2026-03-01) against the current codebase and found significant drift after 18 days of active development. Key revisions: drop unviable marcelom97/scimgateway library (0 importers) in favor of direct chi handler implementation, renumber migrations 000042-000045, add security events pipeline integration, change scim_configs.sso_connection_id from ON DELETE CASCADE to ON DELETE RESTRICT, document three-level access denial hierarchy, and add configurable SCIM rate limiter. Three rounds of subagent-readiness review applied 22 fixes for ambiguity, context gaps, and pitfall alignment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add SCIM RFC specification references (7643, 7644) with section links - Add mandatory TDD discipline section: preamble, completion check, and 3-round review loop requirements for every task and wave - Add testing-pitfalls inline warnings (§1 TOCTOU, §3 error format, §4 PATCH validation, §7 event emission, §10 RLS AppStore, §11 auth tokens, §16 setup errors and CSRF) - Improve assertion specificity for Tasks 12-14, 22-23 — each test case now specifies exact assertion (what value to check, not just "verify") - Add implementation-pitfall warnings to Tasks 17, 19, 24 (API-2 pointer types, AUTH-10 constant-time compare, DB-17 transaction helper selection) - Add explicit 3-round review checkpoints with verification criteria to all 7 execution waves Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add critical note to both v1 documents pointing to the v2 revisions. The originals are preserved for historical reference only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
General deactivation feature (admin-settable, SCIM-automatable). Partial index on (org_id) WHERE deactivated_at IS NULL for active member queries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tracks whether notification group membership was SCIM-synced. SCIM removal only deletes scim_managed=true rows; manual memberships preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SCIM provisioning config (1:1 with orgs via sso_connections). ON DELETE RESTRICT on sso_connection_id prevents silent cascade. Bearer token stored as sha256 hash with display prefix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
IdP groups with optional role and notification group mappings. scim_groups references organizations directly (survives scim_config deletion). scim_group_members has denormalized org_id for RLS protection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10 new event types for SCIM auth failures, token lifecycle, provisioning operations, and rate limiting. Severity mappings and exhaustiveness test updated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ctxSCIMConfigID context key for SCIM auth middleware. SCIMRateLimit config field (default 50 req/sec per org). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New types: ScimConfig, ScimGroup, ScimGroupMember. Updated: OrgMember (deactivated_at, scim_exempt), GroupMember (scim_managed). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add org_members queries: DeactivateOrgMember, ReactivateOrgMember, GetOrgMemberFull, CountActiveOrgMembers, CountActiveOrgOwners, UpdateOrgMemberSCIMExempt. Add group_members queries: AddGroupMemberSCIMManaged, RemoveSCIMManagedGroupMember, IsGroupMemberSCIMManaged, GetGroupIfActive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add store methods: DeactivateOrgMember, ReactivateOrgMember, GetOrgMemberFull, CountActiveOrgMembers, CountActiveOrgOwners, UpdateOrgMemberSCIMExempt. All use withOrgTx for RLS isolation. Tests cover: deactivation, reactivation, active member/owner counting, SCIM exempt flag toggling, RLS isolation across orgs, and not-found returning nil,nil. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add migration 042 creating scim_configs table with RLS policy and sqlc queries for SCIM config CRUD (create, get by org/token/SSO conn, update, rotate token, delete). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Store methods wrapping sqlc queries with correct transaction helpers: - withOrgTx for org-scoped CRUD (create, get, update, rotate, delete) - withBypassTx for token hash and SSO connection lookups (pre-org auth) - sql.ErrNoRows returns nil, nil (not error) 9 integration tests covering all methods, duplicate constraint, and RLS isolation between orgs via AppStore. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ged, configs, groups) Copied from Wave 1 worktree — these migrations are prerequisites for SCIM group sqlc queries and store methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
14 queries for SCIM group CRUD, membership management, and mapping count operations. All org-scoped via RLS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
12 store methods covering SCIM group CRUD, membership management, mapping updates, and cross-group mapping counts. 11 integration tests including RLS isolation, cascade deletes, idempotent adds, and member count aggregation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # internal/store/generated/models.go
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add GetOrgMemberRoleAndStatus sqlc query that returns both role and deactivated_at. RequireOrgRole middleware now checks deactivation status before role check — deactivated members receive 403 with a specific message regardless of their role level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add SCIM wire protocol types (SCIMUser, SCIMGroup, SCIMListResponse, SCIMPatchOp, SCIMError, SCIMMeta) and helper functions for writing RFC 7644-compliant error responses with application/scim+json content type. Includes parseSCIMBool for Entra ID string boolean compat and parseSCIMFilter for minimal filter grammar (eq operator only). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements recomputeSCIMRole which recalculates a user's org role based on SCIM group mapped_role values. Owner protection prevents downgrade, SCIM-exempt users are skipped, and when no mapped roles exist the defaultRole is applied. Uses role hierarchy: admin > member > viewer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Expand PATCH /members/{user_id} to accept optional active and
scim_exempt fields alongside the existing role field. All fields
are now pointer types for proper PATCH semantics.
- active=false: deactivates member (sole owner protected with 400)
- active=true: reactivates member
- scim_exempt=true/false: sets SCIM exemption flag
- GET /members response now includes active, deactivated_at, scim_exempt
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…security events Adds scim_configs table (1:1 with orgs via sso_connections), sqlc queries for CRUD + token hash lookup, store method LookupSCIMConfigByTokenHash with withBypassTx (pre-org-context), and SCIM security event constants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements requireSCIMAuth per design doc §2: extract org_id from URL, validate Bearer token via sha256 hash lookup + subtle.ConstantTimeCompare, verify org match and enabled status. Fires security events on auth failures. All error responses use RFC 7644 §3.12 SCIM error format (application/scim+json). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements syncNotifGroupAdd and syncNotifGroupRemove for propagating SCIM group membership changes to notification groups. Respects soft-deleted target groups (no-op), manual membership precedence (ON CONFLICT DO NOTHING), multi-mapping edge case (keeps membership if another SCIM group maps same), and scim_managed flag (only removes SCIM-managed memberships). Also adds store wrapper methods for GetGroupIfActive, AddGroupMemberSCIMManaged, and RemoveSCIMManagedGroupMember. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add pre-flight check in deleteSSOHandler that looks up any SCIM config linked to the SSO connection. If found, returns 409 Conflict with a message directing the admin to remove SCIM config first. This prevents accidental destruction of SCIM provisioning state and provides a clear error instead of the FK RESTRICT 500. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Per-org rate limiter for SCIM endpoints, separate from the API rate limiter. Default 50 req/sec, configurable via SCIM_RATE_LIMIT env var. Returns RFC 7644 SCIM error format on 429. Fires security event on rate limit exceeded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # internal/api/context.go # internal/secure/events.go # internal/store/generated/models.go # internal/store/generated/scim_config.sql.go # internal/store/queries/scim_config.sql # internal/store/scim_config.go
Both Lane A (middleware) and Lane C (types) independently implemented writeSCIMError. Keep the canonical version in scim_types.go. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix wrong plan filename in Task 6 agent instructions - Fix merge.Ingest signature (merge.Store interface, not *store.Store) - Add NVD cursor requirement for Task 11 SeedCorpus - Replace raw SQL seeding with merge pipeline in EPSS test - Add Subagent Execution Protocol (TDD preamble, completion checks, review loops) - Tighten assertions in Tasks 10A-10G with exact field names and nil checks - Add phase review loop markers with specific verification checklists - Fix discarded errors in test code (testing-pitfalls §16) - Add FEED-5 warning to Task 7 ZIP iteration - Scope test runs to changed packages only (Docker overload) - Add autonomous decision log appendix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements all six SCIM 2.0 Group operations per RFC 7644:
- POST /Groups: create with optional initial members
- GET /Groups/{id}: read with member list
- GET /Groups: list with displayName/externalId/id eq filter
- PUT /Groups/{id}: full replace with member diff
- PATCH /Groups/{id}: add/remove members, replace displayName
- DELETE /Groups/{id}: cascade delete with role recomputation
Supports both standard and Entra ID PATCH remove formats
(path filter vs value array). Case-insensitive op handling.
Audit logging with SCIM metadata on all mutations.
Also adds GetSCIMGroupByDisplayName store wrapper and wires
SCIM group routes in server.go with requireSCIMAuth + rate limiting.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement all 6 SCIM 2.0 User resource handlers per RFC 7643/7644:
- POST /Users: provision with 3-step identity matching (externalId, email, new)
- GET /Users/{id}: lookup with org-scoped RLS isolation
- GET /Users: list with filter support (userName, externalId, id, active) and pagination
- PUT /Users/{id}: full resource replacement (Okta's primary update path)
- PATCH /Users/{id}: partial update with case-insensitive ops and string boolean coercion
- DELETE /Users/{id}: soft-deactivate with sole-owner protection
Key behaviors:
- Transaction splitting: bypass RLS for users/identities, org-scoped for memberships
- SCIM-exempt users return success without modification (prevents IdP retry storms)
- Tier member limit enforcement on new provisioning
- Entra ID quirks: case-insensitive op names, string booleans, pathless PATCH ops
Store additions:
- UpdateUserEmail, UpdateUserDisplayName, UpdateUserProfile
- GetIdentityByProviderAndUser
23 integration tests covering provisioning, read/list, update, and deprovision paths.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… effect Task 27: Admin SCIM config CRUD — POST/GET/PATCH/DELETE for SCIM config, token rotation, SCIM group listing. Enterprise-tier-gated, owner-only for mutations, admin+ for reads. Standard auth (cookie-based), RFC 9457 errors. Security events fired on token create/rotate. Task 28: PATCH group mapping with immediate recomputation — sets mapped_role and/or mapped_group_id on SCIM groups. When mapping changes, all current group members get immediate role recomputation and notification group sync. Cross-org and soft-deleted group validation on mapped_group_id. 15 integration tests covering: config CRUD, SSO prerequisite, enterprise tier gating, duplicate prevention, token masking, enable/disable, delete idempotency, group survival on config delete, token rotation with hash verification, RBAC matrix (viewer/admin/owner), role mapping with immediate effect, notification group sync, mapping clear with default role fallback, cross-org group rejection, soft-deleted group rejection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add ServiceProviderConfig, Schemas, ResourceTypes static discovery
endpoints per RFC 7643/7644. Mount User CRUD handlers alongside
existing Group handlers under /api/v1/orgs/{org_id}/scim/v2/.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add missing security event emissions for 4 SCIM event constants that
were defined but never emitted:
- EventSCIMUserProvisioned: fired on successful user creation/reactivation
- EventSCIMUserDeprovisioned: fired on successful user deactivation
- EventSCIMSoleOwnerProtected: fired when sole-owner protection blocks deactivation
- EventSCIMExemptSuppressed: fired when SCIM-exempt flag suppresses an operation
Add suppressed audit log entries for exempt user operations with
metadata: {"source": "scim", "suppressed": true, "reason": "scim_exempt"}
Add 8 integration tests verifying audit and security event correctness:
- UserProvisionAuditEntry: POST /Users creates audit with entity_type=member, action=create
- UserDeprovisionAuditEntry: DELETE /Users creates audit with action=update
- AuthFailureSecurityEvent: invalid token creates scim.auth_failed event
- TokenCreateSecurityEvent: POST /sso/scim creates scim.token_created event
- UserProvisionedSecurityEvent: POST /Users creates scim.user_provisioned event
- UserDeprovisionedSecurityEvent: DELETE /Users creates scim.user_deprovisioned event
- ExemptSuppressedSecurityEvent: PATCH exempt user creates scim.exempt_suppressed event
- SoleOwnerProtectedSecurityEvent: DELETE sole owner creates scim.sole_owner_protected event
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 of 10 SCIM security event constants were defined but never emitted (testing-pitfalls §7). Added fireSCIMEvent calls for: - EventSCIMUserProvisioned (POST /Users new user creation) - EventSCIMUserDeprovisioned (DELETE /Users deactivation) - EventSCIMSoleOwnerProtected (all sole-owner protection paths) - EventSCIMExemptSuppressed (all exempt-user skip paths) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # internal/api/scim_users.go
Seven e2e tests covering real IdP provisioning workflows: - Full lifecycle (create → verify → deactivate → reactivate → delete) - Group role mapping (add/remove member, verify role recomputation) - Entra ID compatibility (capitalized ops, string booleans, value-array removal) - Test connection patterns (Entra ID filter + Okta pagination) - Cross-org isolation (user in org A invisible from org B) - Token rotation (old token rejected, new token accepted) - Error content-type (all errors return application/scim+json) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Documents 5 decisions made during overnight agent team execution: config type choice, bypassTx for group methods, writeSCIMError duplicate resolution, worktree isolation failure handling, suppressed audit entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Post-implementation review of Phase 7 SCIM found 6 actionable issues.
This commit addresses all of them:
S1: SCIM group store methods now accept orgID and use withOrgTx instead
of withBypassTx, adding AND org_id to all SQL queries for defense-
in-depth tenant isolation. All callers and tests updated.
S2: Extract shared scimScheme() helper that validates X-Forwarded-Proto
(only "http"/"https" accepted). Fixes inconsistency between
scimUserLocation and scimBaseURL.
S5: Replace N+1 per-member GetIdentityByProviderAndUser in list users
with batch ListIdentitiesByProviderAndUsers using ANY($2::uuid[]).
P2: SCIM audit log actor_id now nil (SQL NULL) instead of uuid.Nil
(all-zeros UUID), matching design doc and enabling IS NULL queries.
P3: matchesSCIMGroupFilter now logs slog.Warn for unsupported filter
attributes instead of silently returning false.
P4: Entry-level slog.InfoContext with org_id/scim_config_id added to
all 11 SCIM handlers for consistent observability.
All SCIM tests passing (auth: 3, store: 21, api: 55+). Lint clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GetSCIMGroup now queries with AND org_id, so groups from other orgs return nil. The nil check already handles this — the manual group.OrgID != orgID comparisons were dead code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Same pattern as the SCIM group store refactor: withBypassTx → withOrgTx, AND org_id added to SQL query. Removes redundant manual group.OrgID check in patchSCIMGroupMappingHandler (now dead code — query returns nil for cross-org groups). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The MSRC adapter's /cvrf/v3.0/csaf/{id} endpoint never existed on
Microsoft's API. Plan covers switching to the real CSAF 2.0 static
file distribution at msrc.microsoft.com/csaf/advisories/, capturing
golden fixtures, writing EPSS/MSRC golden tests, adding MSRC to
SeedCorpus, and completing Phase 10 verification.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RemoveSCIMManagedGroupMember was missing orgID entirely — SQL had no AND org_id clause and used withBypassTx, bypassing RLS. Added orgID parameter and AND org_id = $3 to the DELETE query. AddGroupMemberSCIMManaged already had orgID in the INSERT but used withBypassTx. Switched to withOrgTx for dual-layer isolation. Both methods now follow the same withOrgTx + AND org_id pattern as all other org-scoped store methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…/pitfall/plan review) from scarson/fix/scim-review-hardening fix(scim): harden Phase 7 SCIM after security/pitfall/plan review
Documents CVE-2026-3909 → CVE-2026-32194 substitution made during overnight execution when the original CVE had no CSAF file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 7: SCIM 2.0 Provisioning
Full SCIM 2.0 implementation for enterprise IdP integration (Entra ID, Okta):
Hardening (PR #56, merged to dev)
withBypassTx→withOrgTx+AND org_idin all SQL queriesGetGroupIfActive+RemoveSCIMManagedGroupMember+AddGroupMemberSCIMManaged: same treatmentX-Forwarded-Protovalidated (only http/https) via sharedscimScheme()helperANY($2::uuid[])actor_id:uuid.Nil→nil(SQL NULL)org_id/scim_config_idon all 11 handlersgroup.OrgID != orgIDchecks removedTest plan
internal/authSCIM tests: 3 PASSinternal/storeSCIM tests: 21 PASS (testcontainers)internal/apiSCIM tests: 55+ PASS (e2e with testcontainers)golangci-lint run: 0 issues/pitfall-check: clean/security-review: all actionable findings fixed/plan-checkagainst design doc: all gaps addressed🤖 Generated with Claude Code