feat(identity): add tenant-bound SCIM machine foundation - #89
Conversation
a365e27 to
fffdcc8
Compare
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR adds SCIM discovery, bearer-token security, credential lifecycle administration, directory-user reconciliation, provisioning persistence rules, OpenAPI contract separation, and a web interface for managing SCIM connections and credentials. ChangesSCIM provisioning
OpenAPI contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Admin
participant AdminProvisioningController
participant ScimCredentialAdministrationService
participant ScimTokenCodec
participant ProvisioningLedgerService
Admin->>AdminProvisioningController: Issue or rotate credential
AdminProvisioningController->>ScimCredentialAdministrationService: Request credential lifecycle operation
ScimCredentialAdministrationService->>ScimTokenCodec: Generate token and verifier
ScimCredentialAdministrationService->>ProvisioningLedgerService: Persist credential verifier
ProvisioningLedgerService-->>AdminProvisioningController: Credential metadata and one-time token
AdminProvisioningController-->>Admin: Return credential response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java (1)
167-200: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftGuarantee SCIM single-authority before saving
scim_user_resources.
existsByOrganizationIdAndAppUserId(...)runs afterprovisionFromDirectoryhas already modified/committed theAppUserrow, andensureNewUserResource()only checks connection-scopedexternalId/userNameduplicates. A second concurrent registration can read the sameAppUser, pass exists/app-user conflict checks, and insert a duplicatescim_user_resourcesrow until it loses against the added unique constraint. Capture theuq_scim_user_app_userviolation and rethrow asProvisioningConflictException, or lock the target user/SCIM row beforeprovisionFromDirectory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java` around lines 167 - 200, Update registerUserResource to guarantee single-authority before persisting the SCIM resource: either lock the target AppUser/SCIM record before provisionFromDirectory, or catch the uq_scim_user_app_user unique-constraint violation from users.save and rethrow ProvisioningConflictException. Preserve the existing conflict message and ensure concurrent registrations cannot leak the database constraint exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.java`:
- Around line 64-66: Restrict the IllegalArgumentException catch in
ScimAuthenticationProvider to only the SCIM credential parsing step, leaving
ledger access, authority construction, and markCredentialUsed outside that catch
so downstream faults propagate normally. Preserve ProvisioningNotFoundException
handling as appropriate, and pass the caught parsing exception as the cause when
creating BadCredentialsException.
- Around line 60-61: Throttle the credential usage write in the authentication
flow around markCredentialUsed so authenticated SCIM requests do not
synchronously update the same credential row on every request. Preserve the
first-use update and only perform subsequent updates when lastUsedAt is older
than a coarse interval such as one minute, while keeping
authenticatedDiscoveryIsTruthfulAndUsesScimMediaType’s first-request behavior
intact.
In `@apps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.java`:
- Around line 20-26: Update ScimErrorWriter.escape to produce valid JSON for
every control character in U+0000–U+001F, including newline, form feed, tab, and
backspace, while preserving backslash and quote escaping. Prefer the existing
JSON serializer if available; otherwise implement complete JSON string escaping
and add a regression test covering control characters in detail.
In `@apps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.java`:
- Around line 46-52: Update ScimRequestGuardFilter to consume a rate-limit
bucket when authentication does not produce a ScimMachinePrincipal, using a
stable fallback key such as the parsed public token identifier or client
address. Preserve the existing connectionId-based bucket for authenticated
principals, and return the same 429 response when either bucket is exhausted so
malformed, revoked, and incorrect bearer-token attempts are throttled.
- Around line 56-69: Update the rate-limiting implementation around
ScimRequestGuardFilter.consume and its windows state to use a shared counter
store such as Redis or Postgres, ensuring requests across replicas share the
configured per-minute limit and counters survive individual instance restarts;
if retaining the local ConcurrentHashMap design, explicitly document that the
limit is per instance and resets on deployment.
- Around line 38-41: Update ScimRequestGuardFilter’s request-size enforcement so
chunked requests cannot bypass the configured maximumRequestSize limit: reject
unknown content lengths or enforce the cap while reading via a counting/limiting
request stream. Preserve the existing 413 response and allow requests whose
actual body remains within the configured limit.
In
`@apps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.java`:
- Around line 32-53: Update the filter ordering in the security configuration
around ScimRequestGuardFilter and BearerTokenAuthenticationFilter so TLS and
maximumRequestSize guards execute before bearer token validation, while the
per-connection rate limit remains in the authenticated-principal path. Adjust
the guard filter or split its placement as needed without changing the existing
authorization rules.
In `@apps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.java`:
- Around line 59-66: Update ScimTokenCodec.matches and its key configuration to
accept the current verifier key plus a bounded set of previous key versions
during rotation, using a version-to-key map and selecting the matching key for
digest verification. Preserve rejection for unknown or expired versions, and
ensure rotation no longer invalidates all existing SCIM tokens immediately.
In
`@apps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.java`:
- Around line 89-95: Update provisionVerifiedSignIn to scope the
directory-managed email lookup to the authenticated tenant/organization before
binding the OIDC issuer and subject. Prefer the organization-aware users lookup,
such as findByOrganizationIdAndEmailIgnoreCase, and preserve the existing
invitation-adoption behavior for collisions if no scoped lookup is available.
In `@apps/api/src/main/resources/application.yml`:
- Around line 143-150: Update the scim.token-ttl default in application.yml from
365d to a materially shorter duration such as 90d, while preserving the
ORGMEMORY_SCIM_TOKEN_TTL override so operators can tune it for their
deployments.
In
`@apps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.java`:
- Around line 79-114: Update seedTenantAndDisabledConnection to replace the
global deletes of provisioning_credentials and provisioning_connections with
organization-scoped deletes limited to ORGANIZATION_ID and
OTHER_ORGANIZATION_ID, using the appropriate organization ownership columns and
parameter binding. Preserve the existing cleanup ordering and tenant setup.
- Around line 149-232: Add integration tests in
ScimMachineSecurityIntegrationTests for ScimRequestGuardFilter covering
rate-limit exhaustion at requestsPerMinute with HTTP 429 and Retry-After,
oversized request bodies returning HTTP 413, require-tls rejecting non-TLS
requests with HTTP 403, and invalid inbound X-Request-ID values being replaced
by a valid fallback ID. Reuse the existing MVC, configuration, token, and
assertion helpers while preserving current authentication and scope coverage.
In
`@core/src/main/java/com/orgmemory/core/organization/UserProvisioningService.java`:
- Around line 171-215: Add an explicit null/blank validation for command.email()
in provisionFromDirectory before calling UserInvitation.normalizeEmail, and
throw the service’s BusinessValidationException with the expected directory-user
validation key/message. Preserve normalization and subsequent provisioning
behavior for valid emails.
---
Outside diff comments:
In
`@core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java`:
- Around line 167-200: Update registerUserResource to guarantee single-authority
before persisting the SCIM resource: either lock the target AppUser/SCIM record
before provisionFromDirectory, or catch the uq_scim_user_app_user
unique-constraint violation from users.save and rethrow
ProvisioningConflictException. Preserve the existing conflict message and ensure
concurrent registrations cannot leak the database constraint exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d0ad8264-a477-453c-87da-d67bb212140c
⛔ Files ignored due to path filters (6)
contracts/openapi.jsonis excluded by!contracts/openapi.jsondocs/decisions/0016-native-scim-behind-keycloak-broker.mdis excluded by!docs/**docs/increments/active/2026-07-27-scim-provisioning-foundation/design.mdis excluded by!docs/**docs/increments/active/2026-07-27-scim-provisioning-foundation/plan.mdis excluded by!docs/**docs/increments/active/2026-07-27-scim-provisioning-foundation/protocol-profile.mdis excluded by!docs/**docs/roadmap.mdis excluded by!docs/**
📒 Files selected for processing (44)
apps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.javaapps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.javaapps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.javaapps/api/src/main/resources/application-prod.ymlapps/api/src/main/resources/application.ymlapps/api/src/test/java/com/orgmemory/api/OpenApiContractTests.javaapps/api/src/test/java/com/orgmemory/api/admin/AdminProvisioningIntegrationTests.javaapps/api/src/test/java/com/orgmemory/api/identityprovisioning/ProvisioningUserAdoptionIntegrationTests.javaapps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.javaapps/api/src/test/java/com/orgmemory/api/security/OidcCurrentActorProviderTests.javaapps/api/src/test/java/com/orgmemory/api/security/ProductionConfigurationGuardTests.javacontracts/scim-openapi.jsoncore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConflictException.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConnectionRepository.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredential.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredentialRepository.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningNotFoundException.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResource.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResourceRepository.javacore/src/main/java/com/orgmemory/core/organization/AppUser.javacore/src/main/java/com/orgmemory/core/organization/AppUserRepository.javacore/src/main/java/com/orgmemory/core/organization/UserInvitationRepository.javacore/src/main/java/com/orgmemory/core/organization/UserProvisioningService.javacore/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sqlcore/src/test/java/com/orgmemory/core/organization/UserProvisioningServiceTests.javaweb/src/components/app-shell/admin-sidebar.tsxweb/src/features/admin/admin-queries.tsweb/src/features/admin/components/admin-access-page.tsxweb/src/features/admin/components/admin-scim-page.tsxweb/src/routeTree.gen.tsweb/src/routes/admin/access.tsx
💤 Files with no reviewable changes (3)
- web/src/features/admin/components/admin-access-page.tsx
- web/src/routes/admin/access.tsx
- web/src/routeTree.gen.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend · Java 25
🧰 Additional context used
📓 Path-based instructions (7)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Record current behavior in architecture/specification documentation only after it exists in code; keep intended behavior in vision, roadmap, or an active increment, and do not duplicate state.
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, Gradle, React, Vite, Tailwind, or TypeScript APIs, consult current official documentation via Context7 and the projectorgmemory-*verification skills.
Before retrieval, AI, MCP, permission, upload, graph, or export work, readdocs/guidelines/agent-safety.md.
Never commit.envfiles, provider keys, tokens, or customer data.
Run the relevant verification gates fromdocs/guidelines/testing-harness.md; use a terminating clean test as the context gate, and do not treatbootRunas verification.
Files:
core/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sqlapps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConflictException.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.javacore/src/main/java/com/orgmemory/core/organization/AppUserRepository.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.javaapps/api/src/main/resources/application-prod.ymlcore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConnectionRepository.javacontracts/scim-openapi.jsoncore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningNotFoundException.javaapps/api/src/main/resources/application.ymlapps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.javacore/src/main/java/com/orgmemory/core/organization/UserInvitationRepository.javacore/src/main/java/com/orgmemory/core/organization/AppUser.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredential.javaapps/api/src/test/java/com/orgmemory/api/OpenApiContractTests.javaapps/api/src/test/java/com/orgmemory/api/security/OidcCurrentActorProviderTests.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.javaapps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.javaweb/src/components/app-shell/admin-sidebar.tsxweb/src/features/admin/admin-queries.tsapps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.javaapps/api/src/test/java/com/orgmemory/api/identityprovisioning/ProvisioningUserAdoptionIntegrationTests.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResourceRepository.javacore/src/test/java/com/orgmemory/core/organization/UserProvisioningServiceTests.javaapps/api/src/test/java/com/orgmemory/api/admin/AdminProvisioningIntegrationTests.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResource.javaweb/src/features/admin/components/admin-scim-page.tsxapps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.javaapps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredentialRepository.javaapps/api/src/test/java/com/orgmemory/api/security/ProductionConfigurationGuardTests.javaapps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.javaapps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.javacore/src/main/java/com/orgmemory/core/organization/UserProvisioningService.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java
core/src/main/resources/db/migration/*.sql
⚙️ CodeRabbit configuration file
core/src/main/resources/db/migration/*.sql: The repository is pre-release: V1 is the intentionally resettable clean
baseline and development data carries no migration cost. Once a release
baseline is frozen, later Flyway migrations are immutable. Check tenant
isolation, foreign keys, uniqueness, indexes, append-only evidence
semantics, safe defaults, and PostgreSQL 18 plus pgvector compatibility.
Files:
core/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sql
**/*.{java,kt}
📄 CodeRabbit inference engine (CLAUDE.md)
JetBrains IDE inspection is a verification gate for the Java backend.
Files:
apps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConflictException.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.javacore/src/main/java/com/orgmemory/core/organization/AppUserRepository.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConnectionRepository.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningNotFoundException.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.javacore/src/main/java/com/orgmemory/core/organization/UserInvitationRepository.javacore/src/main/java/com/orgmemory/core/organization/AppUser.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredential.javaapps/api/src/test/java/com/orgmemory/api/OpenApiContractTests.javaapps/api/src/test/java/com/orgmemory/api/security/OidcCurrentActorProviderTests.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.javaapps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.javaapps/api/src/test/java/com/orgmemory/api/identityprovisioning/ProvisioningUserAdoptionIntegrationTests.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResourceRepository.javacore/src/test/java/com/orgmemory/core/organization/UserProvisioningServiceTests.javaapps/api/src/test/java/com/orgmemory/api/admin/AdminProvisioningIntegrationTests.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResource.javaapps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.javaapps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredentialRepository.javaapps/api/src/test/java/com/orgmemory/api/security/ProductionConfigurationGuardTests.javaapps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.javaapps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.javacore/src/main/java/com/orgmemory/core/organization/UserProvisioningService.javacore/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java
apps/api/src/main/java/**/*.java
⚙️ CodeRabbit configuration file
apps/api/src/main/java/**/*.java: Enforce the browser-BFF and resource-server boundaries. Authentication
must resolve an active internal actor through the explicit issuer and
subject binding. Reject identity, tenant, roles, or permissions supplied
by request payloads, JWT email, or untrusted JWT role claims.
Files:
apps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.javaapps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.javaapps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.javaapps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.javaapps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.java
**/application*.{yml,yaml,properties}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep JPA
ddl-auto=validate; pair JPA schema or entity changes with a Flyway migration.
Files:
apps/api/src/main/resources/application-prod.ymlapps/api/src/main/resources/application.yml
**/*.{ts,tsx,js,jsx,css,scss,html}
📄 CodeRabbit inference engine (CLAUDE.md)
For frontend files, run Oxlint, TypeScript typecheck, the production build, and browser tests when the UI flow matters; do not run JetBrains IDE inspection on TypeScript, TSX, or web configuration.
Files:
web/src/components/app-shell/admin-sidebar.tsxweb/src/features/admin/admin-queries.tsweb/src/features/admin/components/admin-scim-page.tsx
web/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
web/src/**/*.{ts,tsx}: OAuth access and refresh tokens must never enter browser JavaScript or
browser storage. Use the HttpOnly BFF session, CSRF-protected mutations,
generated Hey API data clients, accessible states, and both light and
dark themes. Handwritten transport is reserved for documented protocol
flows such as navigation redirects and streaming.
Files:
web/src/components/app-shell/admin-sidebar.tsxweb/src/features/admin/admin-queries.tsweb/src/features/admin/components/admin-scim-page.tsx
🧠 Learnings (3)
📚 Learning: 2026-07-23T23:30:44.585Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 30
File: core/src/main/resources/db/migration/V32__evidence_scoped_graph_semantics.sql:0-0
Timestamp: 2026-07-23T23:30:44.585Z
Learning: For OrgMemory PostgreSQL Flyway migrations under core/src/main/resources/db/migration, do not recommend using `CREATE INDEX CONCURRENTLY` or `DROP INDEX CONCURRENTLY` inside application-owned Flyway migration SQL. Flyway’s schema-history connection may hold a transaction that can cause concurrent index operations to wait indefinitely (e.g., on a `virtualxid`), and docs/conventions.md forbids this pattern. If you need large production-table index replacement, pre-stage online index operations via the deployment pipeline (outside Flyway) rather than inside the migration; “ordinary” index replacement is acceptable for unreleased projections before production traffic.
Applied to files:
core/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sql
📚 Learning: 2026-07-26T05:46:47.443Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 61
File: apps/mcp/src/main/java/com/orgmemory/mcp/McpSecurityConfiguration.java:50-52
Timestamp: 2026-07-26T05:46:47.443Z
Learning: In OrgMemory, treat the `apps/mcp` and `apps/api` as independent protocol adapter modules. When adjusting OAuth/wire-level scopes, do not introduce a shared Java constant or create a code dependency from `apps/mcp` to `apps/api` solely to deduplicate scope values. Instead, keep OAuth/scope constants adapter-local (e.g., in the relevant adapter/security configuration classes) and ensure cross-adapter consistency via automated realm/OAuth/authorization tests, rather than via shared wiring-level constants or cross-module references.
Applied to files:
apps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.javaapps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.javaapps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.javaapps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.javaapps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.javaapps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.javaapps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.java
📚 Learning: 2026-07-23T03:36:09.053Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 16
File: web/src/features/admin/components/admin-mappings-page.tsx:190-211
Timestamp: 2026-07-23T03:36:09.053Z
Learning: In the OrgMemory admin UI, preserve the principal display ordering returned by the backend instead of re-sorting on the client. The server-owned order (e.g., unmapped views first, then deterministic ordering by source-system, connection, kind, and external-key as defined in SourcePrincipalAdminService#listPrincipals) must be used as-is to avoid mismatches with the server’s intended admin mappings/principals list.
Applied to files:
web/src/features/admin/components/admin-scim-page.tsx
🪛 ast-grep (0.44.1)
apps/api/src/main/java/com/orgmemory/api/scim/ScimErrorWriter.java
[warning] 19-21: Avoid writing untrusted input to the HTTP response
Context: response.getWriter().write("""
{"schemas":["%s"],"status":"%d","detail":"%s"}
""".formatted(ERROR_SCHEMA, status, escape(detail)))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(xss-protection-java)
apps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.java
[warning] 31-33: Do not disable CSRF
Context: http
.securityMatcher("/scim/v2/**")
.csrf(AbstractHttpConfigurer::disable)
Note: [CWE-352] Cross-Site Request Forgery (CSRF).
(spring-csrf-disable)
apps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.java
[warning] 165-165: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: session.setAttribute(SPRING_SECURITY_CONTEXT_KEY, context)
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
🪛 Checkov (3.3.8)
contracts/scim-openapi.json
[high] 1: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
🪛 PMD (7.26.0)
apps/api/src/main/java/com/orgmemory/api/scim/ScimAuthenticationProvider.java
[Medium] 65-65: PreserveStackTrace (Best Practices): Thrown exception does not preserve the stack trace of exception 'failure' on all code paths
(PreserveStackTrace (Best Practices))
🪛 Squawk (2.59.0)
core/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sql
[warning] 5-6: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 5-6: Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads and writes to the table while the index is built. Create an index CONCURRENTLY and create the constraint using the index.
(disallowed-unique-constraint)
🔇 Additional comments (46)
apps/api/src/main/java/com/orgmemory/api/OpenApiGroupingConfiguration.java (1)
9-41: LGTM!apps/api/src/main/java/com/orgmemory/api/scim/ScimDiscoveryController.java (1)
12-76: LGTM!apps/api/src/test/java/com/orgmemory/api/OpenApiContractTests.java (1)
22-81: LGTM!contracts/scim-openapi.json (1)
1-1: LGTM!apps/api/src/main/java/com/orgmemory/api/admin/AdminProvisioningController.java (1)
1-244: LGTM!apps/api/src/test/java/com/orgmemory/api/admin/AdminProvisioningIntegrationTests.java (1)
1-312: LGTM!web/src/features/admin/admin-queries.ts (1)
29-32: LGTM!Also applies to: 95-105, 189-192
web/src/features/admin/components/admin-scim-page.tsx (2)
65-65: 🎯 Functional Correctness | ⚡ Quick winCredential badge ignores expiry/overlap, so dead tokens still show "Active".
revoked = Boolean(credential.revokedAt)is the only signal used for the "Active"/"Revoked" badge, butCredentialResponsealso carriesexpiresAtandoverlapEndsAt. PerScimMachineSecurityIntegrationTests.expiredRevokedAndEndedOverlapCredentialsAreGenericUnauthorized, a credential whose overlap window or TTL has elapsed is rejected server-side whilerevokedAtstays null — so this admin page will keep showing it as "Active" indefinitely, which undermines the credential-lifecycle review this page is meant to support.🩹 Proposed fix
- const revoked = Boolean(credential.revokedAt) + const now = Date.now() + const revoked = Boolean(credential.revokedAt) + const expired = + !revoked && + ((credential.overlapEndsAt && new Date(credential.overlapEndsAt).getTime() < now) || + (credential.expiresAt && new Date(credential.expiresAt).getTime() < now))- <Badge variant={revoked ? "outline" : "secondary"}>{revoked ? "Revoked" : "Active"}</Badge> + <Badge variant={revoked || expired ? "outline" : "secondary"}> + {revoked ? "Revoked" : expired ? "Expired" : "Active"} + </Badge>Also applies to: 104-104
142-142: 🎯 Functional CorrectnessNo change needed.
/scim/v2is served at the same browser-facing origin in this deployment; there is no separate reverse-proxy prefix for the SCIM resource-server.apps/api/src/test/java/com/orgmemory/api/identityprovisioning/ProvisioningUserAdoptionIntegrationTests.java (1)
1-234: LGTM!apps/api/src/main/java/com/orgmemory/api/scim/ScimCredentialAdministrationService.java (1)
1-125: LGTM!core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConflictException.java (1)
1-14: LGTM!core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredential.java (1)
91-144: LGTM!Also applies to: 168-195
core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningCredentialRepository.java (1)
3-13: LGTM!Also applies to: 14-17, 19-32, 33-34, 36-47
core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningLedgerService.java (2)
3-7: LGTM!Also applies to: 24-37, 57-63, 104-166
227-269: LGTM!Also applies to: 307-369, 380-386
core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningNotFoundException.java (1)
3-12: LGTM!core/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResource.java (1)
93-108: LGTM!apps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.java (1)
38-44: LGTM!Also applies to: 108-108
apps/api/src/test/java/com/orgmemory/api/security/OidcCurrentActorProviderTests.java (1)
40-41: LGTM!Also applies to: 124-125, 134-135, 160-160
core/src/main/resources/db/migration/V11__enforce_single_scim_authority_per_user.sql (1)
1-6: Squawk'sCONCURRENTLY/NOT VALIDwarnings don't apply pre-release here.Static analysis flags the blocking unique-constraint rebuild, but per prior guidance for this repo, ordinary constraint/index replacement (without
CONCURRENTLY/NOT VALID) is accepted for unreleased projections before production traffic, andCONCURRENTLYis explicitly discouraged inside Flyway migrations here due to schema-history connection lock risk.Based on learnings: "For OrgMemory PostgreSQL Flyway migrations... do not recommend using
CREATE INDEX CONCURRENTLYorDROP INDEX CONCURRENTLY... 'ordinary' index replacement is acceptable for unreleased projections before production traffic."Sources: Learnings, Linters/SAST tools
core/src/main/java/com/orgmemory/core/identityprovisioning/ProvisioningConnectionRepository.java (1)
4-4: LGTM!Also applies to: 20-21
core/src/main/java/com/orgmemory/core/identityprovisioning/ScimUserResourceRepository.java (1)
13-23: LGTM!core/src/main/java/com/orgmemory/core/organization/AppUser.java (2)
100-109: LGTM!
155-158: LGTM!core/src/main/java/com/orgmemory/core/organization/AppUserRepository.java (1)
12-13: LGTM!core/src/main/java/com/orgmemory/core/organization/UserInvitationRepository.java (1)
30-41: LGTM!core/src/main/java/com/orgmemory/core/organization/UserProvisioningService.java (4)
50-85: LGTM!
115-125: LGTM!
4-4: LGTM!Also applies to: 16-26, 153-164, 239-262
171-215: 🗄️ Data Integrity & IntegrationNo change needed:
registerUserResource’s SCIM conflict exception is unchecked and the method runs under@Transactional, so the later check rolls back prior directory-user mutations.core/src/test/java/com/orgmemory/core/organization/UserProvisioningServiceTests.java (1)
15-15: LGTM!Also applies to: 89-130, 246-327
web/src/components/app-shell/admin-sidebar.tsx (1)
32-39: LGTM!apps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityProperties.java (1)
20-48: LGTM!apps/api/src/main/java/com/orgmemory/api/scim/ScimTokenCodec.java (1)
26-57: LGTM!apps/api/src/main/java/com/orgmemory/api/scim/ScimMachinePrincipal.java (1)
6-12: LGTM!apps/api/src/main/java/com/orgmemory/api/security/SecurityConfig.java (2)
65-68: LGTM!Also applies to: 192-213
56-56: 🩺 Stability & AvailabilityNo change needed.
The SCIM chain is already registered with
@Order(1)and asecurityMatcher("/scim/v2/**"), so the browser@Order(3)catch-all chain is isolated from SCIM requests.apps/api/src/test/java/com/orgmemory/api/scim/ScimMachineSecurityIntegrationTests.java (2)
116-147: LGTM!Also applies to: 234-294
49-51: 📐 Maintainability & Code QualityNo change needed.
org.testcontainers.postgresql.PostgreSQLContaineris the current Testcontainers PostgreSQL module import, and using the raw type matches the existing PostgreSQL container declarations in this module.apps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.java (1)
34-37: 🔒 Security & PrivacyNo change needed for
isSecure().Production enables
server.forward-headers-strategy: frameworkinapplication-prod.yml, while SCIM TLS enforcement is disabled by default and only required in prod.apps/api/src/main/java/com/orgmemory/api/scim/ScimSecurityConfiguration.java (1)
1-30: LGTM!apps/api/src/main/java/com/orgmemory/api/security/ProductionConfigurationGuard.java (1)
21-47: LGTM!Also applies to: 76-83
apps/api/src/main/java/com/orgmemory/api/security/ProductionSecurityConfiguration.java (1)
3-3: LGTM!Also applies to: 25-26, 41-42
apps/api/src/main/resources/application-prod.yml (1)
48-52: LGTM!apps/api/src/test/java/com/orgmemory/api/security/ProductionConfigurationGuardTests.java (1)
6-15: LGTM!Also applies to: 58-97, 125-176, 194-251
| var authentication = SecurityContextHolder.getContext().getAuthentication(); | ||
| Object principal = authentication == null ? null : authentication.getPrincipal(); | ||
| if (principal instanceof ScimMachinePrincipal machine && !consume(machine.connectionId())) { | ||
| response.setHeader(HttpHeaders.RETRY_AFTER, "60"); | ||
| ScimErrorWriter.write(response, 429, "Request rate exceeded"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Rate limiting only applies to already-authenticated callers, so credential guessing is unthrottled.
The counter keys on ScimMachinePrincipal.connectionId(), which exists only after ScimAuthenticationProvider succeeds. Requests with a missing, malformed, revoked, or simply wrong bearer token never consume a slot, so an attacker can hammer /scim/v2/** with candidate tokens at full speed — each attempt also triggers a ledger lookup. Add a fallback bucket for unauthenticated attempts (e.g., keyed by parsed public token id or client address) so failed authentication is throttled too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/main/java/com/orgmemory/api/scim/ScimRequestGuardFilter.java`
around lines 46 - 52, Update ScimRequestGuardFilter to consume a rate-limit
bucket when authentication does not produce a ScimMachinePrincipal, using a
stable fallback key such as the parsed public token identifier or client
address. Preserve the existing connectionId-based bucket for authenticated
principals, and return the same 429 response when either bucket is exhausted so
malformed, revoked, and incorrect bearer-token attempts are throttled.
| private AppUser provisionVerifiedSignIn(ExternalSubject external) { | ||
| if (!external.emailVerified() || !StringUtils.hasText(external.email())) { | ||
| throw new OrgMemoryAccessDeniedException( | ||
| "A verified OIDC email is required to accept an invitation"); | ||
| "A verified OIDC email is required for first sign-in"); | ||
| } | ||
| return provisioning | ||
| .provisionFromInvitation( | ||
| .provisionForVerifiedSignIn( |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the full provisionForVerifiedSignIn implementation and the
# AppUserRepository.findByEmailIgnoreCase contract for tenant scoping.
ast-grep run --pattern 'Optional<AppUser> provisionForVerifiedSignIn($$$) { $$$ }' \
--lang java core/src/main/java/com/orgmemory/core/organization/UserProvisioningService.java
rg -nP -B2 -A5 'findByEmailIgnoreCase' core/src/main/java/com/orgmemory/core/organizationRepository: kl3inIT/OrgMemory
Length of output: 2472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
git ls-files | rg '(^|/)(AppUserRepository|UserProvisioningService|OidcCurrentActorProvider)\.java$'
echo
echo "== AppUserRepository =="
cat -n core/src/main/java/com/orgmemory/core/organization/AppUserRepository.java
echo
echo "== UserProvisioningService relevant section =="
sed -n '1,140p' core/src/main/java/com/orgmemory/core/organization/UserProvisioningService.java | cat -n
echo
echo "== OidcCurrentActorProvider relevant section =="
sed -n '1,160p' apps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.java | cat -nRepository: kl3inIT/OrgMemory
Length of output: 14218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for issuer/organization mapping and directory-managed provisioning =="
rg -n "provisionForVerifiedSignIn|ExternalSubject|directoryManaged|DirectoryManaged|organizationId.*email|email.*organization|findByOrganizationIdAndEmailIgnoreCase|findByIssuer|issuer" apps core -g '*.java'Repository: kl3inIT/OrgMemory
Length of output: 20920
Scope the first-sign-in directory lookup by tenant
provisionForVerifiedSignIn() uses users.findByEmailIgnoreCase(normalized), which matches directory-managed users across all organizations, then binds the OIDC issuer/subject to the first unique email. If two organizations have the same directory-managed email, a sign-in from one issuer can be bound to a user in the other tenant. Use an org-scoped email lookup, such as findByOrganizationIdAndEmailIgnoreCase, or reject multiple colliding directory-managed users the same way invitation adoption does.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/api/src/main/java/com/orgmemory/api/security/OidcCurrentActorProvider.java`
around lines 89 - 95, Update provisionVerifiedSignIn to scope the
directory-managed email lookup to the authenticated tenant/organization before
binding the OIDC issuer and subject. Prefer the organization-aware users lookup,
such as findByOrganizationIdAndEmailIgnoreCase, and preserve the existing
invitation-adoption behavior for collisions if no scoped lookup is available.
1 participant
Summary
/scim/v2/**security chain with tenant-bound bearer credentials, TLS/request/rate guards, expiry, bounded rotation overlap, immediate revocation, and last-use metadatacontracts/scim-openapi.jsonwhile keeping SCIM routes out of the product/browser contract(issuer, subject)loginWhy
Keycloak remains the interactive IdP/broker, but OrgMemory needs to own organization tenancy, workforce lifecycle, provisioning evidence, and authorization. This foundation establishes that machine boundary without making Keycloak the account source of truth or exposing partial User/Group mutation.
Product and security impact
DISABLED/api/**, and OIDC/browser credentials cannot fall through into/scim/v2/**CurrentActorand guarded bycan_manage_membersValidation
./gradlew.bat --no-daemon clean testcorepack pnpm -C web build(lint, TypeScript, production build)git diff --check, mechanical Java scan, and literal bearer-token scanJetBrains semantic inspection was unavailable because this worktree was not the project open in the IDE; the full Gradle suite and mechanical checks were used as fallback.
Deferred
Summary by CodeRabbit