diff --git a/.env.example b/.env.example index dc2eafe7677..32179e02192 100644 --- a/.env.example +++ b/.env.example @@ -1156,6 +1156,11 @@ USE_ENTRA_ID_FOR_PEOPLE_SEARCH=false # When enabled, entra id groups owners will be considered as members of the group ENTRA_ID_INCLUDE_OWNERS_AS_MEMBERS=false +# When enabled, login sync only updates membership of Entra ID groups already in the database +# (e.g. created when a resource is shared with them) instead of creating every group the user is in +# permissions.syncOnlyExistingEntraGroups in librechat.yaml takes precedence +ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS=false + # Microsoft Graph API scopes needed for people/group search # Default scopes provide access to user profiles and group memberships OPENID_GRAPH_SCOPES=User.Read,People.Read,GroupMember.Read.All diff --git a/api/server/controllers/auth/oauth.js b/api/server/controllers/auth/oauth.js index e7df7661a28..ac9ecc28a21 100644 --- a/api/server/controllers/auth/oauth.js +++ b/api/server/controllers/auth/oauth.js @@ -5,9 +5,11 @@ const { getAdminPanelUrl, isAdminPanelRedirect, generateAdminExchangeCode, + getAppConfigOptionsFromUser, } = require('@librechat/api'); const { syncUserEntraGroupMemberships } = require('~/server/services/PermissionService'); const { setAuthTokens } = require('~/server/services/AuthService'); +const { getAppConfig } = require('~/server/services/Config'); const { sendOpenIDAuthResponse } = require('~/server/services/OpenIDRefreshRecovery'); const getLogStores = require('~/cache/getLogStores'); const { checkBan } = require('~/server/middleware'); @@ -76,7 +78,13 @@ function createOAuthHandler(redirectUri = domains.client) { req.user.provider == 'openid' && isEnabled(process.env.OPENID_REUSE_TOKENS) === true ) { - await syncUserEntraGroupMemberships(req.user, req.user.tokenset.access_token); + const appConfig = await getAppConfig(getAppConfigOptionsFromUser(req.user)); + await syncUserEntraGroupMemberships( + req.user, + req.user.tokenset.access_token, + null, + appConfig, + ); await sendOpenIDAuthResponse({ tokenset: req.user.tokenset, user: req.user, diff --git a/api/server/services/PermissionService.js b/api/server/services/PermissionService.js index c525c0a0b86..25fdebbecce 100644 --- a/api/server/services/PermissionService.js +++ b/api/server/services/PermissionService.js @@ -1,5 +1,10 @@ const mongoose = require('mongoose'); -const { AccessControlService, isEnabled, ensureDirectoryPrincipalUser } = require('@librechat/api'); +const { + isEnabled, + AccessControlService, + ensureDirectoryPrincipalUser, + syncsOnlyExistingEntraGroups, +} = require('@librechat/api'); const { tenantStorage, getTenantId, @@ -483,7 +488,8 @@ const ensureGroupPrincipalExists = async function (principal, authContext = null /** * Sync user's Entra ID group memberships with auto-creation of missing groups - * Optimized approach: + * Optimized approach (steps 4-6 are skipped when `permissions.syncOnlyExistingEntraGroups` + * or `ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS` is enabled, so only existing groups are synced): * 1. Get all group IDs user should be member of from Entra * 2. Try to add user to existing groups (fast, no Graph API calls) * 3. Query DB to identify which groups don't exist (indexed query, fast) @@ -498,12 +504,13 @@ const ensureGroupPrincipalExists = async function (principal, authContext = null * @param {string} user.provider - Authentication provider ('openid') * @param {string} accessToken - Access token for Graph API calls * @param {mongoose.ClientSession} [session] - Optional MongoDB session for transactions + * @param {AppConfig} [appConfig] - Resolved app config for the user * @returns {Promise} */ -const syncUserEntraGroupMemberships = async (user, accessToken, session = null) => { +const syncUserEntraGroupMemberships = async (user, accessToken, session = null, appConfig) => { const tenantId = user?.tenantId ? String(user.tenantId) : undefined; if (!tenantId || getTenantId() != null) { - return performEntraGroupMembershipSync(user, accessToken, session); + return performEntraGroupMembershipSync(user, accessToken, session, appConfig); } /** * The OAuth callback runs before `tenantContextMiddleware`, so establish the @@ -511,11 +518,11 @@ const syncUserEntraGroupMemberships = async (user, accessToken, session = null) * cache invalidation are then scoped exactly like authenticated reads. */ return tenantStorage.run({ tenantId, userId: user._id?.toString() }, async () => - performEntraGroupMembershipSync(user, accessToken, session), + performEntraGroupMembershipSync(user, accessToken, session, appConfig), ); }; -const performEntraGroupMembershipSync = async (user, accessToken, session = null) => { +const performEntraGroupMembershipSync = async (user, accessToken, session = null, appConfig) => { try { if (!entraIdPrincipalFeatureEnabled(user) || !accessToken || !user.idOnTheSource) { return; @@ -570,7 +577,16 @@ const performEntraGroupMembershipSync = async (user, accessToken, session = null const missingGroupIds = allGroupIds.filter((id) => !existingGroupIds.has(id)); - if (missingGroupIds.length > 0) { + const syncOnlyExisting = syncsOnlyExistingEntraGroups( + appConfig?.config?.permissions, + process.env.ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS, + ); + + if (missingGroupIds.length > 0 && syncOnlyExisting) { + logger.debug( + `[PermissionService.syncUserEntraGroupMemberships] Skipping ${missingGroupIds.length} groups not in database`, + ); + } else if (missingGroupIds.length > 0) { logger.info( `[PermissionService.syncUserEntraGroupMemberships] Found ${missingGroupIds.length} groups that don't exist, fetching details...`, ); diff --git a/api/server/services/PermissionService.spec.js b/api/server/services/PermissionService.spec.js index 604b03bfecf..34daa94a2f0 100644 --- a/api/server/services/PermissionService.spec.js +++ b/api/server/services/PermissionService.spec.js @@ -2536,4 +2536,76 @@ describe('syncUserEntraGroupMemberships - $pullAll on Group.memberIds', () => { // Reset mock getEntraGroupDetailsBatch.mockResolvedValue([]); }); + + describe('with ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS enabled', () => { + const { getEntraGroupDetailsBatch } = require('~/server/services/GraphApiService'); + + beforeEach(() => { + process.env.ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS = 'true'; + getEntraGroupDetailsBatch.mockClear(); + }); + + afterEach(() => { + delete process.env.ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS; + getEntraGroupDetailsBatch.mockResolvedValue([]); + }); + + it('does not create groups missing from the database', async () => { + await Group.create({ + name: 'Shared', + source: 'entra', + idOnTheSource: 'entra-shared', + memberIds: [], + }); + getUserEntraGroups.mockResolvedValue(['entra-shared', 'entra-unused']); + getEntraGroupDetailsBatch.mockResolvedValue([{ id: 'entra-unused', name: 'Unused' }]); + + await syncUserEntraGroupMemberships(user, 'fake-token'); + + expect(getEntraGroupDetailsBatch).not.toHaveBeenCalled(); + expect(await Group.countDocuments({ idOnTheSource: 'entra-unused' })).toBe(0); + const shared = await Group.findOne({ idOnTheSource: 'entra-shared' }).lean(); + expect(shared.memberIds).toContain(userEntraId); + }); + + it('still removes the user from existing groups they left', async () => { + await Group.create({ + name: 'Left', + source: 'entra', + idOnTheSource: 'entra-left', + memberIds: [userEntraId], + }); + getUserEntraGroups.mockResolvedValue(['entra-unused']); + getEntraGroupDetailsBatch.mockResolvedValue([{ id: 'entra-unused', name: 'Unused' }]); + + await syncUserEntraGroupMemberships(user, 'fake-token'); + + const left = await Group.findOne({ idOnTheSource: 'entra-left' }).lean(); + expect(left.memberIds).not.toContain(userEntraId); + expect(await Group.countDocuments({ idOnTheSource: 'entra-unused' })).toBe(0); + }); + + it('creates missing groups when librechat.yaml turns the setting off', async () => { + getUserEntraGroups.mockResolvedValue(['entra-unused']); + getEntraGroupDetailsBatch.mockResolvedValue([{ id: 'entra-unused', name: 'Unused' }]); + const appConfig = { config: { permissions: { syncOnlyExistingEntraGroups: false } } }; + + await syncUserEntraGroupMemberships(user, 'fake-token', null, appConfig); + + const created = await Group.findOne({ idOnTheSource: 'entra-unused' }).lean(); + expect(created.memberIds).toContain(userEntraId); + }); + }); + + it('skips missing groups when only librechat.yaml enables the setting', async () => { + const { getEntraGroupDetailsBatch } = require('~/server/services/GraphApiService'); + getUserEntraGroups.mockResolvedValue(['entra-unused']); + getEntraGroupDetailsBatch.mockResolvedValue([{ id: 'entra-unused', name: 'Unused' }]); + const appConfig = { config: { permissions: { syncOnlyExistingEntraGroups: true } } }; + + await syncUserEntraGroupMemberships(user, 'fake-token', null, appConfig); + + expect(await Group.countDocuments({ idOnTheSource: 'entra-unused' })).toBe(0); + getEntraGroupDetailsBatch.mockResolvedValue([]); + }); }); diff --git a/e2e/specs/mock/usage.spec.ts b/e2e/specs/mock/usage.spec.ts index c77448d2375..85193e1ecb7 100644 --- a/e2e/specs/mock/usage.spec.ts +++ b/e2e/specs/mock/usage.spec.ts @@ -283,8 +283,8 @@ test.describe('context usage gauge', () => { await page.setViewportSize({ width: viewport.width, height: viewport.height }); await expect .poll(() => - details.evaluate((element) => - getComputedStyle(element).gridTemplateColumns.split(' ').length, + details.evaluate( + (element) => getComputedStyle(element).gridTemplateColumns.split(' ').length, ), ) .toBe(viewport.columns); @@ -326,8 +326,8 @@ test.describe('context usage gauge', () => { await page.setViewportSize({ width: 375, height: 812 }); await expect .poll(() => - details.evaluate((element) => - getComputedStyle(element).gridTemplateColumns.split(' ').length, + details.evaluate( + (element) => getComputedStyle(element).gridTemplateColumns.split(' ').length, ), ) .toBe(1); diff --git a/librechat.example.yaml b/librechat.example.yaml index 15215eb6e85..fd68e2320c8 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -1799,3 +1799,6 @@ endpoints: # maxWriteAttempts: 3 # Integer 1-100, including the first attempt. # leaseTtlMs: 30000 # Integer 1000-600000. Sharing updates on deployments without transactions hold a per-resource lease this long between renewals. # leaseWaitMs: 3000 # Integer 0-60000. How long an update waits for a contended lease before answering 409. +# # Login sync updates only Entra ID groups already in the database instead of creating +# # every group the user is in. Unset defers to ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS. +# syncOnlyExistingEntraGroups: false diff --git a/packages/api/src/acl/principals.spec.ts b/packages/api/src/acl/principals.spec.ts index 4fe55c518e7..dd8c1ecdddc 100644 --- a/packages/api/src/acl/principals.spec.ts +++ b/packages/api/src/acl/principals.spec.ts @@ -1,4 +1,4 @@ -import { ensureDirectoryPrincipalUser } from './principals'; +import { ensureDirectoryPrincipalUser, syncsOnlyExistingEntraGroups } from './principals'; const createMethods = () => ({ findUserBySourceId: jest.fn().mockResolvedValue(null), @@ -52,3 +52,20 @@ describe('ensureDirectoryPrincipalUser', () => { expect(methods.findUserBySourceId).not.toHaveBeenCalled(); }); }); + +describe('syncsOnlyExistingEntraGroups', () => { + it('defaults to false when neither setting is present', () => { + expect(syncsOnlyExistingEntraGroups(undefined, undefined)).toBe(false); + }); + + it('falls back to the env flag when the yaml setting is unset', () => { + expect(syncsOnlyExistingEntraGroups({ maxWriteAttempts: 3 }, 'true')).toBe(true); + }); + + it('lets the yaml setting override the env flag', () => { + expect(syncsOnlyExistingEntraGroups({ syncOnlyExistingEntraGroups: false }, 'true')).toBe( + false, + ); + expect(syncsOnlyExistingEntraGroups({ syncOnlyExistingEntraGroups: true }, 'false')).toBe(true); + }); +}); diff --git a/packages/api/src/acl/principals.ts b/packages/api/src/acl/principals.ts index f91d651148e..d49eb70ad26 100644 --- a/packages/api/src/acl/principals.ts +++ b/packages/api/src/acl/principals.ts @@ -1,4 +1,5 @@ -import type { TPrincipal } from 'librechat-data-provider'; +import type { TCustomConfig, TPrincipal } from 'librechat-data-provider'; +import { isEnabled } from '~/utils'; export interface DirectoryPrincipalUser { id: string; @@ -46,3 +47,9 @@ export const ensureDirectoryPrincipalUser = async ( idOnTheSource: principal.idOnTheSource, }); }; + +/** `permissions.syncOnlyExistingEntraGroups` in `librechat.yaml` takes precedence over the env flag. */ +export const syncsOnlyExistingEntraGroups = ( + permissions: TCustomConfig['permissions'], + envValue: string | undefined, +): boolean => permissions?.syncOnlyExistingEntraGroups ?? isEnabled(envValue); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 642ebb37e9c..f601f0f8da1 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -4126,6 +4126,9 @@ export const configSchema = z.object({ maxWriteAttempts: permissionWriteAttemptsSchema, leaseTtlMs: permissionLeaseTtlSchema, leaseWaitMs: permissionLeaseWaitSchema, + /** Login sync updates only Entra ID groups already in the database instead of creating + * every group the user is in. Unset defers to `ENTRA_ID_SYNC_ONLY_EXISTING_GROUPS`. */ + syncOnlyExistingEntraGroups: z.boolean().optional(), }) .optional(), cache: z.boolean().default(true),