perf(auth): lazily load auth drivers to reduce memory usage - #28146
Open
Vansh1811 wants to merge 6 commits into
Open
perf(auth): lazily load auth drivers to reduce memory usage#28146Vansh1811 wants to merge 6 commits into
Vansh1811 wants to merge 6 commits into
Conversation
Refactor auth provider registration to use async/await for dynamic imports and provider instance creAuth drivers for LDAP, OAuth2, OpenID, and SAML pull in heavyweight dependencies (ldapjs, openid-client, @node-saml/node-saml, etc.) that were previously imported eagerly at the top of api/src/auth.ts. This meant every one of these packages was loaded into memory on every server boot, even when only the local provider was configured. This change replaces the static imports with dynamic import() calls inside getProviderInstance, so only the drivers that are actually referenced by AUTH_PROVIDERS get loaded. Fixes directus#24334
Reduced memory usage on startup by lazily loading LDAP, OAuth2, OpenID, and SAML auth drivers via dynamic imports, ensuring dependencies are loaded only when configured via AUTH_PROVIDERS.
Add tests for dynamic loading of authentication providers.
ComfortablyCoding
requested changes
Aug 27, 2026
ComfortablyCoding
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR! Please see my comments below.
In addition, please ensure the PR template is followed. As this is a performance-related change, please provide stats demonstrating that it provides a meaningful improvement.
Comment on lines
+75
to
+82
| /** | ||
| * Lazily imports and instantiates the auth driver matching the given type. | ||
| * | ||
| * Auth drivers (LDAP, OAuth2, OpenID, SAML) pull in heavyweight dependencies | ||
| * (ldapjs, openid-client, etc). Dynamically importing them here ensures that | ||
| * only the drivers actually configured via AUTH_PROVIDERS get loaded into | ||
| * memory, instead of all drivers being loaded eagerly on every boot. | ||
| */ |
Member
There was a problem hiding this comment.
Let's simplify this
Suggested change
| /** | |
| * Lazily imports and instantiates the auth driver matching the given type. | |
| * | |
| * Auth drivers (LDAP, OAuth2, OpenID, SAML) pull in heavyweight dependencies | |
| * (ldapjs, openid-client, etc). Dynamically importing them here ensures that | |
| * only the drivers actually configured via AUTH_PROVIDERS get loaded into | |
| * memory, instead of all drivers being loaded eagerly on every boot. | |
| */ | |
| /** | |
| * Lazily loads the auth driver for the given type. | |
| */ |
Comment on lines
-5
to
-11
| import { | ||
| LDAPAuthDriver, | ||
| LocalAuthDriver, | ||
| OAuth2AuthDriver, | ||
| OpenIDAuthDriver, | ||
| SAMLAuthDriver, | ||
| } from './auth/drivers/index.js'; |
Member
There was a problem hiding this comment.
While this handles lazy instantiation, the drivers are still statically imported in the controller
directus/api/src/controllers/auth.ts
Line 7 in e29e467
|
|
||
| // Always register default provider | ||
| const defaultProvider = getProviderInstance('local', options)!; | ||
| const defaultProvider = await getProviderInstance('local', options)!; |
Member
There was a problem hiding this comment.
The ! is asserting the promise, not the value, is still seen as possibly undefined by TS
| expect(samlDriverCtor).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('registerAuthProviders dynamically imports the oauth2 driver when configured', async () => { |
Member
There was a problem hiding this comment.
The first test, this one and the next 2 essentially all test the same flow, let's condense them down to one.
Comment on lines
+70
to
+94
| test('registerAuthProviders only dynamically imports drivers that are configured', async () => { | ||
| mockEnv['AUTH_PROVIDERS'] = 'ldap_provider'; | ||
|
|
||
| const { registerAuthProviders, getAuthProvider } = await import('./auth.js'); | ||
|
|
||
| await registerAuthProviders(); | ||
|
|
||
| expect(ldapDriverCtor).toHaveBeenCalledTimes(1); | ||
| expect(oauth2DriverCtor).not.toHaveBeenCalled(); | ||
| expect(openidDriverCtor).not.toHaveBeenCalled(); | ||
| expect(samlDriverCtor).not.toHaveBeenCalled(); | ||
| expect(getAuthProvider('ldap_provider')).toEqual({ type: 'ldap' }); | ||
| expect(getAuthProvider('default')).toEqual({ type: 'local' }); | ||
| }); | ||
|
|
||
| test('registerAuthProviders does not import unused drivers when AUTH_PROVIDERS is unset', async () => { | ||
| const { registerAuthProviders } = await import('./auth.js'); | ||
|
|
||
| await registerAuthProviders(); | ||
|
|
||
| expect(ldapDriverCtor).not.toHaveBeenCalled(); | ||
| expect(oauth2DriverCtor).not.toHaveBeenCalled(); | ||
| expect(openidDriverCtor).not.toHaveBeenCalled(); | ||
| expect(samlDriverCtor).not.toHaveBeenCalled(); | ||
| }); |
Member
There was a problem hiding this comment.
These (and subsequent) tests do not assert lazy loading is taking place, only that the constructor is not instantiating. Any possible regression of the lazy load will therefor not be caught.
|
|
||
| // Register configured providers | ||
| providerNames.forEach((name: string) => { | ||
| for (let name of providerNames) { |
Member
There was a problem hiding this comment.
This will load them sequentially, no need in this case, let's load them in parallel.
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
2 participants
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.
Auth drivers for LDAP, OAuth2, OpenID, and SAML pull in heavyweight dependencies (ldapjs, openid-client, @node-saml/node-saml, etc.) that were previously imported eagerly at the top of api/src/auth.ts. This meant every one of these packages was loaded into memory on every server boot, even when only the local provider was configured.
This change replaces the static imports with dynamic import() calls inside getProviderInstance, so only the drivers that are actually referenced by AUTH_PROVIDERS get loaded.
Fixes #24334
What's Changed
Tested Scenarios