🪜 feat: Add OpenID Role Sync#13415
Conversation
There was a problem hiding this comment.
Pull request overview
Adds generic OpenID role sync that maps OIDC role/group claim values to a single LibreChat user.role. Shared helpers parse OPENID_ROLE_SYNC_* config, extract role values from access/id/userinfo sources, validate configured LibreChat roles via a new tenant-scoped non-paginated findRolesByNames lookup, and select the highest-priority role (with optional fallback). The sync wires into browser OpenID login after admin handling and, when OPENID_ROLE_SYNC_API_ENABLED=true, into the remote agent API auth middleware (re-checking tenant policy after a role change). ADMIN can never be assigned by generic sync; OPENID_ADMIN_ROLE remains authoritative.
Changes:
- Add
packages/api/src/auth/openidRoleSync.ts(and tests) with shared option parsing, claim extraction, role validation, and selection logic; export it viaauth/index.ts. - Wire role sync into
api/strategies/openidStrategy.jsafter admin-role evaluation and intopackages/api/src/middleware/remoteAgentAuth.ts(with tenant context and post-change policy enforcement); threadfindRolesByNamesthrough DI inapi/server/routes/agents/middleware.js. - Add tenant-scoped
findRolesByNamestopackages/data-schemas/src/methods/role.tsand document newOPENID_ROLE_SYNC_*vars in.env.example.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/auth/openidRoleSync.ts | New shared helpers for options, claim extraction, role validation, and selection. |
| packages/api/src/auth/openidRoleSync.spec.ts | Unit tests covering options, claim extraction, validation, and selection rules. |
| packages/api/src/auth/index.ts | Re-export the new module. |
| packages/api/src/middleware/remoteAgentAuth.ts | Apply role sync when API sync is enabled and re-check tenant policy after a role change. |
| packages/api/src/middleware/remoteAgentAuth.spec.ts | Coverage for API role sync paths (enable gating, selection, fallback, ADMIN preserve, tenant context, policy re-check). |
| api/strategies/openidStrategy.js | Browser login role sync after admin handling; reuses required-role overage groups. |
| api/strategies/openidStrategy.spec.js | Tests for browser role sync behaviors (selection, ADMIN preserve, fallback, tenant context, overage reuse, missing roles). |
| packages/data-schemas/src/methods/role.ts | New findRolesByNames for tenant-scoped, non-paginated lookup by names. |
| packages/data-schemas/src/methods/role.methods.spec.ts | Tests for findRolesByNames (no cache use, case-insensitive, tenant-scoped). |
| api/server/routes/agents/middleware.js | Inject findRolesByNames into the remote agent auth middleware. |
| .env.example | Document new OPENID_ROLE_SYNC_* variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function escapeRegex(value: string): string { | ||
| return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| } |
There was a problem hiding this comment.
Done in bdc1908 — replaced the local escapeRegex with the shared escapeRegExp from ~/utils/string, matching prompt.ts/skill.ts/user.ts/userGroup.ts. The implementation is identical, so behavior is unchanged.
danny-avila
changed the title
|
@codex review |
GitNexus: 🚀 deployedThe |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80e2f865f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? await tenantStorage.run({ tenantId: user.tenantId }, () => | ||
| getLibreChatRolesForOpenIdSync(libreChatRoles), |
There was a problem hiding this comment.
For tenant users, this passes a non-async callback into tenantStorage.run before running Mongoose-backed role lookup. packages/data-schemas/src/config/tenantContext.ts explicitly requires async callbacks because sync callbacks returning thenables can lose ALS context during Mongoose query execution; in tenant-scoped OpenID login this can make findRolesByNames run without the intended tenant filter (or fail strict tenant mode) and validate against the wrong role set. Make the callback async when calling getLibreChatRolesForOpenIdSync.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in bdc1908 — the tenantStorage.run callback is now async. In practice context was retained here because the wrapped getLibreChatRolesForOpenIdSync is itself async and dispatches its Mongoose query as its first await (in-scope), but making the callback async honors the documented ALS contract and matches every other tenantStorage.run call site. Behavior is unchanged.
| return; | ||
| } | ||
|
|
||
| const loadLibreChatRoles = () => |
There was a problem hiding this comment.
When remote-agent API role sync handles a user from a different tenant context, loadLibreChatRoles is a non-async callback passed to tenantStorage.run. The tenant context helper documents that callbacks must be async for Mongoose query execution to retain ALS context, so this tenant-scoped getRolesByNames lookup can run outside the resolved user's tenant (or trip strict isolation) and select/validate roles from the wrong scope. Define loadLibreChatRoles as async before passing it to tenantStorage.run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in bdc1908 — loadLibreChatRoles is now async before being passed to tenantStorage.run. The wrapped lookup was already async (query dispatched in-scope), so this is a contract-alignment/no-behavior-change fix consistent with the other call sites.
| const openIdRoleValues = get(source, options.claim); | ||
| if (Array.isArray(openIdRoleValues) || typeof openIdRoleValues === 'string') { | ||
| return openIdRoleValues; |
There was a problem hiding this comment.
When the configured claim is present but empty (for example an IdP emits roles: "" for users with no mapped groups), this helper returns the empty string, but both role-sync call sites treat falsy openIdRoleValues as “claim missing” and return before selectOpenIdRole can apply the configured fallback role. That leaves the previous local role in place even though the fallback is documented as authoritative when no priority role matches; return undefined only for missing/invalid claims or make the callers distinguish undefined from an empty string.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 3c7612c. Rather than change the helper signature, both call sites now skip only when getOpenIdRolesForOpenIdSync returns undefined (missing/invalid claim); a present-but-empty "" now flows through to selectOpenIdRole, which applies the configured fallback — consistent with how an empty array was already handled. Added regression tests on both the OpenID strategy and remote-agent API auth paths covering the empty-claim → fallback case.
| const update = () => updateUser(userResolution.user.id, userResolution.updateData); | ||
| if (userResolution.user.tenantId && getTenantId() !== userResolution.user.tenantId) { | ||
| await tenantStorage.run({ tenantId: userResolution.user.tenantId }, update); |
There was a problem hiding this comment.
When a remote-agent request resolves a tenant user outside that tenant's current ALS context and role sync or migration adds updateData, this non-async callback is passed to tenantStorage.run before updateUser performs a tenant-isolated findByIdAndUpdate. The tenant context helper requires async callbacks for Mongoose execution; otherwise strict tenant isolation can reject the update or the write can execute without the intended tenant filter, leaving the request authenticated with an in-memory role that was not persisted. Make this callback async as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in bdc1908 — the update callback wrapping updateUser is now async. Same rationale as the role-lookup case: updateUser already awaits its tenant-isolated findByIdAndUpdate in-scope, so context was retained, but async satisfies the documented contract and keeps the call sites consistent.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
danny-avila
force-pushed
the
danny-avila/pr-13317-oidc-role-sync
branch
from
bdc1908 to
4d96b0a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d96b0ad45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!source || !options.claim) { | ||
| return; |
There was a problem hiding this comment.
When OPENID_ROLE_SYNC_FALLBACK_ROLE is configured and the selected token/userinfo source does not contain OPENID_ROLE_SYNC_CLAIM, this early return makes both browser and remote-agent callers skip selectOpenIdRole entirely, so an existing non-admin role is left unchanged instead of being demoted to the authoritative fallback. In deployments using fallback as documented in .env.example, an IdP that stops emitting the claim (or emits it only for unassigned users) leaves users on stale elevated LibreChat roles rather than assigning the fallback role.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd429fc. getOpenIdRolesForOpenIdSync now returns an empty list (rather than undefined) when the token source is present but carries no usable value for the configured claim, so both callers still run selectOpenIdRole and apply the configured fallback instead of leaving a stale elevated role. A genuinely unavailable source (no token at all) still returns undefined and skips sync. Added unit + strategy-level coverage for the absent-claim → fallback case.
| return; | ||
| } | ||
|
|
||
| if (options.claimSource === 'id' && options.claim === 'groups') { |
There was a problem hiding this comment.
This overage branch is restricted to claimSource === 'id', but Entra can also replace the groups claim in access tokens with _claim_names/_claim_sources overage markers. With OPENID_ROLE_SYNC_SOURCE=access and OPENID_ROLE_SYNC_CLAIM=groups—the only source supported by remote-agent API role sync and also a valid browser-login source—users over the group limit never have their groups resolved, so role sync silently sees no assignable roles for those users while users below the limit are synced correctly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd429fc. The overage branch is no longer gated to claimSource === 'id'; it now runs for groups on both id and access sources, so OPENID_ROLE_SYNC_SOURCE=access + CLAIM=groups (the only source supported by remote-agent API sync) resolves _claim_names/_claim_sources overage as well. Added a unit test covering access-token overage resolution.
| const { rolePriority, fallbackRole } = user?.tenantId | ||
| ? await tenantStorage.run({ tenantId: user.tenantId }, async () => | ||
| getLibreChatRolesForOpenIdSync(libreChatRoles), |
There was a problem hiding this comment.
For tenant-scoped users this validates every configured role inside the tenant context, but findRolesByNames only reads existing role documents and does not auto-create system roles the way getRoleByName does. Since USER is the documented fallback and system role creation is run at startup without a tenant context, a tenant user with OPENID_ROLE_SYNC_FALLBACK_ROLE=USER can have the login/API request fail with “configured roles do not exist: USER” unless a tenant-scoped USER document was created out of band.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd429fc. getLibreChatRolesForOpenIdSync now treats SystemRoles (e.g. USER) as always-available canonical names, since they are provisioned globally at startup and a tenant-scoped findRolesByNames lookup may not return them. This prevents the spurious "configured roles do not exist: USER" failure for tenant users using the documented system fallback. Added a test simulating a tenant lookup that omits the system role.
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
|
@peeeteeer can you confirm this is still working for you? I put it through another round of codex reviews when updating the branch and resolving conflicts. Due to touching sensitive modules (regarding openid) will merge this shortly after the v0.8.6 release |
Both role-sync call sites skipped on a falsy `openIdRoleValues`, treating an
empty claim string ('') the same as a missing claim and returning before
`selectOpenIdRole` could apply the configured fallback role. An IdP emitting
an empty roles claim for a user with no mapped groups left the stale local
role in place instead of the authoritative fallback.
Skip only when the helper returns `undefined` (missing/invalid), letting an
empty string flow through to fallback selection — consistent with how an
empty array is already handled. Adds regression coverage on both the OpenID
strategy and the remote-agent API auth paths.
- role.ts: reuse the shared escapeRegExp util instead of a local escapeRegex duplicate, matching prompt/skill/user/userGroup methods (Copilot). - openidStrategy.js / remoteAgentAuth.ts: make the tenantStorage.run callbacks async so the documented ALS contract is satisfied and tenant context cannot be lost during Mongoose execution; the wrapped lookups/updates are already async, so behavior is unchanged (codex P2).
Addresses the second Codex review cycle (P2 findings): - Apply fallback when the claim is absent: getOpenIdRolesForOpenIdSync now returns an empty list (not undefined) when the token source exists but has no usable claim value, so callers still run selection and assign the configured fallback instead of leaving a stale elevated role. A truly unavailable source still returns undefined and skips sync. - Resolve group overage for access tokens too: the _claim_names/_claim_sources overage path previously only ran for claimSource 'id'; Entra also moves an oversized groups claim into access tokens, so 'access'+'groups' (the only source supported by remote-agent API sync) now resolves overage as well. - Allow system fallback roles for tenant users: getLibreChatRolesForOpenIdSync treats SystemRoles (e.g. USER) as always-available canonical names, since they are provisioned globally at startup and a tenant-scoped lookup may not return them — preventing a spurious 'configured roles do not exist: USER'. Adds unit and strategy-level coverage for all three.
danny-avila
force-pushed
the
danny-avila/pr-13317-oidc-role-sync
branch
from
cd429fc to
80885e6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80885e6161
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let query = Role.find({ | ||
| $or: uniqueRoleNames.map((roleName) => ({ | ||
| name: new RegExp(`^${escapeRegExp(roleName)}$`, 'i'), | ||
| })), | ||
| }); |
There was a problem hiding this comment.
When OpenID role sync validates roles for a user without tenantId (the else paths in both browser OpenID login and remote-agent auth), this new lookup runs with no tenant ALS context, so the tenant-isolation plugin does not add any tenantId filter. In a multi-tenant deployment, a configured role name that exists only in some tenant will still be treated as valid and can be assigned to a base user; later permission lookups also run unscoped for that user and may resolve the tenant role's permissions. Please make the no-tenant case only match base/global roles (or require an explicit tenant context) instead of querying all tenants.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0188f41. findRolesByNames now constrains the query to base roles (tenantId unset) when no tenant ALS context is active, so a base user can no longer match or be assigned a role that only exists within a tenant. When a non-system tenant context is active, the isolation plugin continues to scope the query. Added a data-schemas test (real in-memory Mongo) asserting a no-context lookup excludes a tenant-only role.
| if (!adminRoleGranted) { | ||
| await applyOpenIdRoleSync({ | ||
| user, | ||
| username, | ||
| tokenset, | ||
| claims, | ||
| userinfo, | ||
| resolvedOverageGroups, | ||
| }); |
There was a problem hiding this comment.
For tenant users whose OpenID role sync changes user.role, the earlier tenant appConfig and registration.allowedDomains check were resolved with the user's previous role (see the pre-sync resolveAppConfigForUser(getAppConfig, user) call above). In a tenant that uses role-scoped config overrides to restrict login domains, a token can move the user into a stricter role but still complete login under the old role's policy. Please re-resolve/enforce the tenant login policy after applyOpenIdRoleSync mutates the role, similar to the remote-agent path's post-sync policy check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0188f41. After applyOpenIdRoleSync mutates a tenant user's role, the strategy now re-resolves the tenant appConfig via resolveAppConfigForUser and re-checks allowedDomains, blocking login if the new role's policy disallows the domain — so a token can no longer complete login under the previous role's looser policy. Added a strategy-level test where the post-sync config rejects the domain.
| if (!['access', 'id', 'userinfo'].includes(claimSource)) { | ||
| throw new Error( | ||
| `[openidRoleSync] OPENID_ROLE_SYNC_SOURCE must be one of: access, id, userinfo`, | ||
| ); |
There was a problem hiding this comment.
Because getOpenIdRoleSyncOptions() is called before both OpenID paths check options.enabled, this validation still runs when OPENID_ROLE_SYNC_ENABLED=false. If an operator leaves an experimental or mistyped OPENID_ROLE_SYNC_SOURCE value in the environment while the feature is disabled, every OpenID login/API request hits this throw instead of simply skipping role sync. Please only validate role-sync-specific settings after the feature is enabled, or return disabled options first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0188f41. getOpenIdRoleSyncOptions now returns disabled options before validating any role-sync-specific settings (source/claim/ADMIN constraints), so a disabled deployment with a stale or mistyped OPENID_ROLE_SYNC_* value no longer fails OpenID login — validation only runs once the feature is enabled. Added a test asserting disabled options are returned without throwing on an invalid source.
GitNexus: 🚀 deployedThe |
Addresses the third Codex review cycle: - Constrain base-user role lookups to base roles (P2): findRolesByNames now filters to roles with an unset tenantId when no tenant ALS context is active, so a base user cannot match — and be assigned — a role that only exists within a tenant. Tenant-scoped lookups remain controlled by the isolation plugin. - Re-enforce tenant login policy after role sync (P2): when role sync changes a tenant user's role, the OpenID strategy re-resolves the tenant appConfig and re-checks allowedDomains, so a token cannot complete login under the previous role's looser policy. - Skip role-sync-specific validation when disabled (P3): getOpenIdRoleSyncOptions returns disabled options before validating role-sync settings, so a stale or mistyped value no longer breaks OpenID login while the feature is off. Adds unit and strategy-level coverage for all three.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0188f41348
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tenantId = getTenantId(); | ||
| if (!tenantId || tenantId === SYSTEM_TENANT_ID) { | ||
| filter.tenantId = { $in: [null, undefined] }; | ||
| } | ||
| let query = Role.find(filter); |
There was a problem hiding this comment.
When OpenID role sync is enabled for a non-tenant/base user and TENANT_ISOLATION_STRICT=true, this path still calls Role.find(filter) with no ALS tenant context. The tenant-isolation plugin's pre('find') hook throws before honoring the manually added tenantId filter, so base OpenID/remote-agent auth fails with a 500 instead of validating the configured base roles. Wrap the base-role lookup in a system context (or otherwise bypass strict-mode tenant injection) while keeping the tenantId constraint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 83305da. The base-role lookup now runs inside runAsSystem (SYSTEM_TENANT_ID context), so the tenant-isolation pre('find') hook no longer throws in strict mode for a context-less query, while an explicit tenantId: { $in: [null, undefined] } filter still constrains the match to base roles. Added a strict-mode regression test asserting a base lookup succeeds rather than 500s.
GitNexus: 🚀 deployedThe |
Hi Danny - can confirm that works as expected - thanks for taking this!!! |
Follow-up to the base-role scoping fix (Codex P1). With TENANT_ISOLATION_STRICT=true,
the tenant-isolation pre('find') hook throws on a context-less query before the manual
tenantId filter is honored, so base OpenID/remote-agent auth would 500 instead of
validating base roles. findRolesByNames now runs the no-context lookup inside
runAsSystem (SYSTEM_TENANT_ID), bypassing strict-mode injection while still applying an
explicit base-role (tenantId unset) filter. Adds a strict-mode regression test.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
* Shared Role-Sync Core
* Environment Configuration
* Browser OpenID Wiring & improved shared component
* API Auth Wiring
* Improved Role Lookup
* added example for sync env
* small simplification
* protect existing manual assigned ADMIN Roles
* fix: Apply OpenID role-sync fallback for present-but-empty claims
Both role-sync call sites skipped on a falsy `openIdRoleValues`, treating an
empty claim string ('') the same as a missing claim and returning before
`selectOpenIdRole` could apply the configured fallback role. An IdP emitting
an empty roles claim for a user with no mapped groups left the stale local
role in place instead of the authoritative fallback.
Skip only when the helper returns `undefined` (missing/invalid), letting an
empty string flow through to fallback selection — consistent with how an
empty array is already handled. Adds regression coverage on both the OpenID
strategy and the remote-agent API auth paths.
* refactor: Address OpenID role-sync review feedback
- role.ts: reuse the shared escapeRegExp util instead of a local escapeRegex
duplicate, matching prompt/skill/user/userGroup methods (Copilot).
- openidStrategy.js / remoteAgentAuth.ts: make the tenantStorage.run callbacks
async so the documented ALS contract is satisfied and tenant context cannot
be lost during Mongoose execution; the wrapped lookups/updates are already
async, so behavior is unchanged (codex P2).
* fix: Harden OpenID role-sync claim and fallback handling
Addresses the second Codex review cycle (P2 findings):
- Apply fallback when the claim is absent: getOpenIdRolesForOpenIdSync now
returns an empty list (not undefined) when the token source exists but has
no usable claim value, so callers still run selection and assign the
configured fallback instead of leaving a stale elevated role. A truly
unavailable source still returns undefined and skips sync.
- Resolve group overage for access tokens too: the _claim_names/_claim_sources
overage path previously only ran for claimSource 'id'; Entra also moves an
oversized groups claim into access tokens, so 'access'+'groups' (the only
source supported by remote-agent API sync) now resolves overage as well.
- Allow system fallback roles for tenant users: getLibreChatRolesForOpenIdSync
treats SystemRoles (e.g. USER) as always-available canonical names, since
they are provisioned globally at startup and a tenant-scoped lookup may not
return them — preventing a spurious 'configured roles do not exist: USER'.
Adds unit and strategy-level coverage for all three.
* fix: Tighten OpenID role-sync tenant scoping and config validation
Addresses the third Codex review cycle:
- Constrain base-user role lookups to base roles (P2): findRolesByNames now
filters to roles with an unset tenantId when no tenant ALS context is active,
so a base user cannot match — and be assigned — a role that only exists within
a tenant. Tenant-scoped lookups remain controlled by the isolation plugin.
- Re-enforce tenant login policy after role sync (P2): when role sync changes a
tenant user's role, the OpenID strategy re-resolves the tenant appConfig and
re-checks allowedDomains, so a token cannot complete login under the previous
role's looser policy.
- Skip role-sync-specific validation when disabled (P3): getOpenIdRoleSyncOptions
returns disabled options before validating role-sync settings, so a stale or
mistyped value no longer breaks OpenID login while the feature is off.
Adds unit and strategy-level coverage for all three.
* fix: Run base role lookups under system context for strict isolation
Follow-up to the base-role scoping fix (Codex P1). With TENANT_ISOLATION_STRICT=true,
the tenant-isolation pre('find') hook throws on a context-less query before the manual
tenantId filter is honored, so base OpenID/remote-agent auth would 500 instead of
validating base roles. findRolesByNames now runs the no-context lookup inside
runAsSystem (SYSTEM_TENANT_ID), bypassing strict-mode injection while still applying an
explicit base-role (tenantId unset) filter. Adds a strict-mode regression test.
---------
Co-authored-by: Peter Rothlaender <peter.rothlaender@ginkgo.com>
Summary
I recreated #13317 on current
devto clear the merge conflicts and add generic OpenID role sync from configured OIDC claims.access,id, oruserinfo, configured role validation, and single-role selection by priority.OPENID_ROLE_SYNC_API_ENABLED=true.OPENID_ROLE_SYNC_*variables in.env.example.Change Type
Testing
npm cinpm run build:data-providernpm run build:data-schemasnpm run build:apinpm test -- --runInBand strategies/openidStrategy.spec.js --coverage=falsefromapinpm run test:ci -- --runInBand src/auth/openidRoleSync.spec.ts src/middleware/remoteAgentAuth.spec.ts --coverage=falsefrompackages/apinpm run test:ci -- --runInBand src/methods/role.methods.spec.ts --coverage=falsefrompackages/data-schemasTest Configuration:
Checklist