Skip to content

Commit 39b898b

Browse files
M-ChrisM-Chris
committed
feat: Critical auth+autodiscovery fix and enhanced initModules() API
- CRITICAL: Fixed auth middleware bypass with auto-discovered modules - Added bulletproof initModules() API for explicit module loading - Ensures middleware order: auth runs before module routes - Works with disabled auto-discovery via forced loading - Synchronous interface: app.initModules() - no await required - Identical behavior to enabled auto-discovery - Comprehensive edge case handling and race condition protection Completes v1.5.17
1 parent c428133 commit 39b898b

4 files changed

Lines changed: 233 additions & 42 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,18 @@
1-
## [1.5.17] - 2025-09-28
1+
## [1.5.17] - 2025-09-29
2+
3+
### Critical Fix
4+
- **Auto-Discovery + Auth Middleware Compatibility**: Fixed critical issue where auto-discovery could bypass user middleware (like auth)
5+
- Module routes now properly respect the middleware chain order
6+
- Auth middleware is guaranteed to run before module route handlers
7+
- Comprehensive race condition protection and edge case handling
8+
9+
### Major Enhancement
10+
- **Enhanced `initModules()` API**: Added powerful public API for explicit module loading control
11+
- **Synchronous interface**: `app.initModules()` - no `await` required
12+
- **Works with disabled auto-discovery**: Forces module loading even when `autoDiscover: false`
13+
- **Custom configuration support**: `app.initModules({ paths: ['./my-modules'] })`
14+
- **Identical behavior**: Uses same discovery and loading mechanisms as enabled auto-discovery
15+
- **Perfect timing**: Ensures modules load before server starts, maintaining middleware order
216

317
### Fixed
418
- **JWT Error Handling**: Enhanced JWT error detection and handling to provide elegant user-friendly messages
@@ -15,6 +29,9 @@
1529
### Architectural Improvements
1630
- **Separation of Concerns**: HTTP server now focuses purely on HTTP protocol concerns, not application-specific authentication
1731
- **Cleaner Error Handling**: JWT errors are handled elegantly at the middleware level where they belong
32+
- **Robust Module Loading**: Two-phase module mounting ensures middleware order is preserved
33+
- **Race Condition Protection**: Auto-discovery initialization is idempotent and handles concurrent calls safely
34+
- **Unified Module Loading**: All module loading paths now use identical discovery and loading mechanisms
1835

1936
## [1.5.16] - 2025-09-28
2037

‎src/core/framework.ts‎

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -619,22 +619,61 @@ export class Moro extends EventEmitter {
619619
this.logger.debug(`Mounting router for basePath: ${basePath}`, 'Router');
620620

621621
// Enterprise-grade middleware integration with performance optimization
622+
// IMPORTANT: Module middleware runs AFTER user middleware (like auth) to ensure proper order
622623
this.httpServer.use(async (req: HttpRequest, res: HttpResponse, next: () => void) => {
623624
if (req.path.startsWith(basePath)) {
624625
this.logger.debug(`Module middleware handling: ${req.method} ${req.path}`, 'Middleware', {
625626
basePath,
626627
});
627628

629+
// Mark this request as being handled by a module
630+
(req as any).__moduleBasePath = basePath;
631+
(req as any).__moduleRouter = router;
632+
633+
// Continue to next middleware (including auth) first
634+
next();
635+
} else {
636+
next();
637+
}
638+
});
639+
640+
this.logger.info(`Router mounted for ${basePath}`, 'Router');
641+
}
642+
643+
private finalModuleHandlerSetup = false;
644+
645+
// Setup final module handler that runs after all user middleware
646+
setupFinalModuleHandler(): void {
647+
// Prevent duplicate setup
648+
if (this.finalModuleHandlerSetup) {
649+
this.logger.debug('Final module handler already set up, skipping', 'ModuleSystem');
650+
return;
651+
}
652+
this.finalModuleHandlerSetup = true;
653+
654+
this.logger.info(
655+
'Setting up final module handler to run after user middleware',
656+
'ModuleSystem'
657+
);
658+
659+
this.httpServer.use(async (req: HttpRequest, res: HttpResponse, next: () => void) => {
660+
// Check if this request was marked for module handling
661+
const moduleBasePath = (req as any).__moduleBasePath;
662+
const moduleRouter = (req as any).__moduleRouter;
663+
664+
if (moduleBasePath && moduleRouter && !res.headersSent) {
665+
this.logger.debug(`Final module handler processing: ${req.method} ${req.path}`, 'Router');
666+
628667
try {
629-
const handled = await router.handle(req, res, basePath);
630-
this.logger.debug(`Route handled: ${handled}`, 'Router');
668+
const handled = await moduleRouter.handle(req, res, moduleBasePath);
669+
this.logger.debug(`Route handled by module: ${handled}`, 'Router');
631670

632671
if (!handled) {
633672
next(); // Let other middleware handle it
634673
}
635674
// If handled, the router already sent the response, so don't call next()
636675
} catch (error) {
637-
this.logger.error('Router error', 'Router', {
676+
this.logger.error('Module router error', 'Router', {
638677
error: error instanceof Error ? error.message : String(error),
639678
});
640679
if (!res.headersSent) {
@@ -645,8 +684,6 @@ export class Moro extends EventEmitter {
645684
next();
646685
}
647686
});
648-
649-
this.logger.info(`Router mounted for ${basePath}`, 'Router');
650687
}
651688

652689
private async setupWebSocketHandlers(config: ModuleConfig): Promise<void> {

‎src/moro.ts‎

Lines changed: 163 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ export class Moro extends EventEmitter {
2727
private lazyModules = new Map<string, ModuleConfig>();
2828
private routeHandlers: Record<string, Function> = {};
2929
private moduleDiscovery?: any; // Store for cleanup
30+
private autoDiscoveryOptions: MoroOptions | null = null;
31+
private autoDiscoveryInitialized = false;
32+
private autoDiscoveryPromise: Promise<void> | null = null;
3033
// Enterprise event system integration
3134
private eventBus: MoroEventBus;
3235
// Application logger
@@ -118,15 +121,10 @@ export class Moro extends EventEmitter {
118121
...options,
119122
});
120123

121-
// Auto-discover modules if enabled
122-
if (options.autoDiscover !== false) {
123-
// Initialize auto-discovery asynchronously
124-
this.initializeAutoDiscovery(options).catch(error => {
125-
this.logger.error('Auto-discovery initialization failed', 'Framework', {
126-
error: error instanceof Error ? error.message : String(error),
127-
});
128-
});
129-
}
124+
// Store auto-discovery options for later initialization
125+
// IMPORTANT: Auto-discovery is deferred to ensure user middleware (like auth)
126+
// is registered before module middleware that might bypass it
127+
this.autoDiscoveryOptions = options.autoDiscover !== false ? options : null;
130128

131129
// Emit initialization event through enterprise event bus
132130
this.eventBus.emit('framework:initialized', {
@@ -355,6 +353,13 @@ export class Moro extends EventEmitter {
355353
version: moduleOrPath.version || '1.0.0',
356354
});
357355
}
356+
357+
// IMPORTANT: If modules are loaded manually after auto-discovery,
358+
// ensure the final module handler is set up to maintain middleware order
359+
if (this.autoDiscoveryInitialized) {
360+
this.coreFramework.setupFinalModuleHandler();
361+
}
362+
358363
return this;
359364
}
360365

@@ -496,35 +501,162 @@ export class Moro extends EventEmitter {
496501
this.registerDirectRoutes();
497502
}
498503

499-
const actualCallback = () => {
500-
const displayHost = host || 'localhost';
501-
this.logger.info('Moro Server Started', 'Server');
502-
this.logger.info(`Runtime: ${this.runtimeType}`, 'Server');
503-
this.logger.info(`HTTP API: http://${displayHost}:${port}`, 'Server');
504-
if (this.config.websocket.enabled) {
505-
this.logger.info(`WebSocket: ws://${displayHost}:${port}`, 'Server');
506-
}
507-
this.logger.info('Learn more at https://morojs.com', 'Server');
504+
const startServer = () => {
505+
const actualCallback = () => {
506+
const displayHost = host || 'localhost';
507+
this.logger.info('Moro Server Started', 'Server');
508+
this.logger.info(`Runtime: ${this.runtimeType}`, 'Server');
509+
this.logger.info(`HTTP API: http://${displayHost}:${port}`, 'Server');
510+
if (this.config.websocket.enabled) {
511+
this.logger.info(`WebSocket: ws://${displayHost}:${port}`, 'Server');
512+
}
513+
this.logger.info('Learn more at https://morojs.com', 'Server');
514+
515+
// Log intelligent routes info
516+
const intelligentRoutes = this.intelligentRouting.getIntelligentRoutes();
517+
if (intelligentRoutes.length > 0) {
518+
this.logger.info(`Intelligent Routes: ${intelligentRoutes.length} registered`, 'Server');
519+
}
520+
521+
this.eventBus.emit('server:started', { port, runtime: this.runtimeType });
522+
if (callback) callback();
523+
};
508524

509-
// Log intelligent routes info
510-
const intelligentRoutes = this.intelligentRouting.getIntelligentRoutes();
511-
if (intelligentRoutes.length > 0) {
512-
this.logger.info(`Intelligent Routes: ${intelligentRoutes.length} registered`, 'Server');
525+
if (host && typeof host === 'string') {
526+
this.coreFramework.listen(port, host, actualCallback);
527+
} else {
528+
this.coreFramework.listen(port, actualCallback);
513529
}
530+
};
514531

515-
this.eventBus.emit('server:started', { port, runtime: this.runtimeType });
516-
if (callback) callback();
532+
// Ensure auto-discovery is complete before starting server
533+
this.ensureAutoDiscoveryComplete()
534+
.then(() => {
535+
startServer();
536+
})
537+
.catch(error => {
538+
this.logger.error('Auto-discovery initialization failed during server start', 'Framework', {
539+
error: error instanceof Error ? error.message : String(error),
540+
});
541+
// Start server anyway - auto-discovery failure shouldn't prevent startup
542+
startServer();
543+
});
544+
}
545+
546+
// Public method to manually initialize auto-discovery
547+
// Useful for ensuring auth middleware is registered before auto-discovery
548+
async initializeAutoDiscoveryNow(): Promise<void> {
549+
return this.ensureAutoDiscoveryComplete();
550+
}
551+
552+
// Public API: Initialize modules explicitly after middleware setup
553+
// This provides users with explicit control over module loading timing
554+
// IMPORTANT: This forces module loading even if autoDiscovery.enabled is false
555+
// Usage: app.initModules() or app.initModules({ paths: ['./my-modules'] })
556+
initModules(options?: {
557+
paths?: string[];
558+
patterns?: string[];
559+
recursive?: boolean;
560+
loadingStrategy?: 'eager' | 'lazy' | 'conditional';
561+
watchForChanges?: boolean;
562+
ignorePatterns?: string[];
563+
loadOrder?: 'alphabetical' | 'dependency' | 'custom';
564+
failOnError?: boolean;
565+
maxDepth?: number;
566+
}): void {
567+
this.logger.info('User-requested module initialization', 'ModuleSystem');
568+
569+
// If already initialized, do nothing
570+
if (this.autoDiscoveryInitialized) {
571+
this.logger.debug('Auto-discovery already completed, skipping', 'ModuleSystem');
572+
return;
573+
}
574+
575+
// Store the options and mark that we want to force initialization
576+
this.autoDiscoveryOptions = {
577+
autoDiscover: {
578+
enabled: true, // Force enabled regardless of original config
579+
paths: options?.paths || ['./modules', './src/modules'],
580+
patterns: options?.patterns || [
581+
'**/*.module.{ts,js}',
582+
'**/index.{ts,js}',
583+
'**/*.config.{ts,js}',
584+
],
585+
recursive: options?.recursive ?? true,
586+
loadingStrategy: options?.loadingStrategy || ('eager' as const),
587+
watchForChanges: options?.watchForChanges ?? false,
588+
ignorePatterns: options?.ignorePatterns || [
589+
'**/*.test.{ts,js}',
590+
'**/*.spec.{ts,js}',
591+
'**/node_modules/**',
592+
],
593+
loadOrder: options?.loadOrder || ('dependency' as const),
594+
failOnError: options?.failOnError ?? false,
595+
maxDepth: options?.maxDepth ?? 5,
596+
},
517597
};
518598

519-
if (host && typeof host === 'string') {
520-
this.coreFramework.listen(port, host, actualCallback);
521-
} else {
522-
this.coreFramework.listen(port, actualCallback);
599+
this.logger.debug(
600+
'Module initialization options stored, will execute on next listen/getHandler call',
601+
'ModuleSystem'
602+
);
603+
}
604+
605+
// Robust method to ensure auto-discovery is complete, handling race conditions
606+
private async ensureAutoDiscoveryComplete(): Promise<void> {
607+
// If already initialized, nothing to do
608+
if (this.autoDiscoveryInitialized) {
609+
return;
610+
}
611+
612+
// If auto-discovery is disabled, mark as initialized
613+
if (!this.autoDiscoveryOptions) {
614+
this.autoDiscoveryInitialized = true;
615+
return;
616+
}
617+
618+
// If already in progress, wait for it to complete
619+
if (this.autoDiscoveryPromise) {
620+
return this.autoDiscoveryPromise;
621+
}
622+
623+
// Start auto-discovery
624+
this.autoDiscoveryPromise = this.performAutoDiscovery();
625+
626+
try {
627+
await this.autoDiscoveryPromise;
628+
this.autoDiscoveryInitialized = true;
629+
} catch (error) {
630+
// Reset promise on error so it can be retried
631+
this.autoDiscoveryPromise = null;
632+
throw error;
633+
} finally {
634+
this.autoDiscoveryOptions = null; // Clear after attempt
523635
}
524636
}
525637

638+
// Perform the actual auto-discovery work
639+
private async performAutoDiscovery(optionsOverride?: MoroOptions): Promise<void> {
640+
const optionsToUse = optionsOverride || this.autoDiscoveryOptions;
641+
if (!optionsToUse) return;
642+
643+
this.logger.debug('Starting auto-discovery initialization', 'AutoDiscovery');
644+
645+
await this.initializeAutoDiscovery(optionsToUse);
646+
647+
this.logger.debug('Auto-discovery initialization completed', 'AutoDiscovery');
648+
}
649+
526650
// Get handler for non-Node.js runtimes
527651
getHandler() {
652+
// Ensure auto-discovery is complete for non-Node.js runtimes
653+
// This handles the case where users call getHandler() immediately after createApp()
654+
this.ensureAutoDiscoveryComplete().catch(error => {
655+
this.logger.error('Auto-discovery initialization failed for runtime handler', 'Framework', {
656+
error: error instanceof Error ? error.message : String(error),
657+
});
658+
});
659+
528660
// Create a unified request handler that works with the runtime adapter
529661
const handler = async (req: HttpRequest, res: HttpResponse) => {
530662
// Add documentation middleware first (if enabled)
@@ -902,6 +1034,9 @@ export class Moro extends EventEmitter {
9021034
// Load modules based on strategy
9031035
await this.loadDiscoveredModules(modules, autoDiscoveryConfig);
9041036

1037+
// Setup final module handler to run after user middleware (like auth)
1038+
this.coreFramework.setupFinalModuleHandler();
1039+
9051040
// Setup file watching if enabled
9061041
if (autoDiscoveryConfig.watchForChanges) {
9071042
this.moduleDiscovery.watchModulesAdvanced(

‎tests/unit/core/moro-auto-discovery.test.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ describe('Moro Auto-Discovery Integration', () => {
113113
}
114114
});
115115

116-
// Wait for auto-discovery to complete
117-
await new Promise(resolve => setTimeout(resolve, 100));
116+
// Trigger auto-discovery manually (using the async method for tests)
117+
await app.initializeAutoDiscoveryNow();
118118

119119
// Check that modules are loaded
120120
expect((app as any).loadedModules.has('users')).toBe(true);
@@ -130,8 +130,8 @@ describe('Moro Auto-Discovery Integration', () => {
130130
}
131131
});
132132

133-
// Wait for auto-discovery to complete
134-
await new Promise(resolve => setTimeout(resolve, 100));
133+
// Trigger auto-discovery manually (using the async method for tests)
134+
await app.initializeAutoDiscoveryNow();
135135

136136
// Check that modules are registered for lazy loading
137137
expect((app as any).lazyModules.has('users')).toBe(true);
@@ -173,8 +173,8 @@ describe('Moro Auto-Discovery Integration', () => {
173173
}
174174
});
175175

176-
// Wait for auto-discovery to complete
177-
await new Promise(resolve => setTimeout(resolve, 100));
176+
// Trigger auto-discovery manually (using the async method for tests)
177+
await app.initializeAutoDiscoveryNow();
178178

179179
// Admin module should not be loaded in development
180180
expect((app as any).loadedModules.has('admin')).toBe(false);
@@ -346,7 +346,8 @@ describe('Moro Auto-Discovery Integration', () => {
346346

347347
const app = createApp({ autoDiscover: true });
348348

349-
await new Promise(resolve => setTimeout(resolve, 100));
349+
// Trigger auto-discovery manually (using the async method for tests)
350+
await app.initializeAutoDiscoveryNow();
350351

351352
expect((app as any).loadedModules.has('legacy')).toBe(true);
352353
});
@@ -360,7 +361,8 @@ describe('Moro Auto-Discovery Integration', () => {
360361

361362
const app = createApp({ modulesPath: './modules' });
362363

363-
await new Promise(resolve => setTimeout(resolve, 200));
364+
// Trigger auto-discovery manually (using the async method for tests)
365+
await app.initializeAutoDiscoveryNow();
364366

365367
expect((app as any).loadedModules.has('custom-path')).toBe(true);
366368
});

0 commit comments

Comments
 (0)