#184 Change CSRF implementation to default to using SessionStorage instead of CacheStorage to remove 'page expired' errors and make extensible.#238
Conversation
… of CacheStorage to remove 'page expired' errors and make extensible.
There was a problem hiding this comment.
Pull request overview
This PR refactors CBWIRE's CSRF protection to use a custom, extensible implementation based on cbstorages instead of relying on the cbcsrf module. The change addresses the "Page Expired" errors that occurred with cbcsrf's cache-based approach by defaulting to session-based storage, while maintaining flexibility for distributed deployments. The implementation introduces an interface-based design that allows users to choose between session storage (default, OWASP-recommended) or cache storage (for clusters), or even create custom implementations.
Key Changes:
- Introduced
ICSRFStorageinterface and two built-in implementations:SessionCSRFStorage(default) andCacheCSRFStorage - Refactored
TokenServiceto use lazy-loaded storage implementations via the new interface - Added
csrfEnabledandcsrfServicesettings to ModuleConfig for user configuration - Replaced
cbcsrfdependency withcbstoragesin box.json
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| models/interfaces/ICSRFStorage.cfc | Defines the contract for CSRF storage implementations with 5 core methods (set, get, exists, delete, clear) |
| models/services/csrf/SessionCSRFStorage.cfc | Session-based storage implementation using cbstorages sessionStorage - the new default |
| models/services/csrf/CacheCSRFStorage.cfc | Cache-based storage implementation for distributed systems that don't require sessions |
| models/services/TokenService.cfc | Refactored to delegate storage to configurable ICSRFStorage implementations via lazy loading |
| models/CBWIREController.cfc | Updated to use TokenService instead of cbcsrf, with conditional CSRF verification based on csrfEnabled setting |
| ModuleConfig.cfc | Added csrfEnabled and csrfService configuration settings with detailed documentation |
| box.json | Updated dependencies from cbcsrf to cbstorages, including install path changes |
| test-harness/tests/specs/unit/services/csrf/SessionCSRFStorageSpec.cfc | Comprehensive test suite for SessionCSRFStorage (6 tests) |
| test-harness/tests/specs/unit/services/csrf/CacheCSRFStorageSpec.cfc | Comprehensive test suite for CacheCSRFStorage (6 tests) |
| test-harness/tests/specs/unit/services/TokenServiceSpec.cfc | Enhanced test suite for TokenService with 7 tests covering generation, verification, and rotation |
| test-harness/layouts/Main.cfm | Minor cleanup removing version number from header text |
| .claude/settings.local.json | Added Claude AI configuration file for development tooling permissions |
|
@grantcopley I've opened a new pull request, #250, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@grantcopley I've opened a new pull request, #251, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Add test coverage for CSRF-disabled scenarios
Use getApplicationMetadata() for cross-platform session detection in TokenService
Files Created
- Defines the contract for CSRF storage implementations
- 5 methods: set(), get(), exists(), delete(), clear()
models/services/csrf/SessionCSRFStorage.cfc
- Default OWASP-recommended storage
- Uses sessionStorage@cbstorages
- Requires sessions enabled
models/services/csrf/CacheCSRFStorage.cfc
- For distributed/clustered systems
- Uses cacheStorage@cbstorages
- No session requirement (uses cookie/URL fallback)
- test-harness/tests/specs/unit/services/csrf/SessionCSRFS
torageSpec.cfc
- test-harness/tests/specs/unit/services/csrf/CacheCSRFSto
rageSpec.cfc
Files Modified
- Added csrfEnabled: true setting
- Added csrfService: "SessionCSRFStorage@cbwire" setting
- Refactored to use lazy-loaded ICSRFStorage
implementation
- Maintains same public API (backward compatible)
- Conditional CSRF verification based on csrfEnabled
- generateCSRFToken() returns empty string when disabled
- Added cleanup before each test
- Complete documentation of new architecture
- Configuration examples
- Custom implementation guide
Test Results
✅ All 305 tests passing
Key Features
✅ Interface-based design - Clean, extensible architecture
✅ Two built-in implementations - Session (default) and
Cache (distributed)
✅ User-configurable - Simple module settings
✅ User-extensible - Can implement custom storage (Redis,
DynamoDB, etc.)
✅ OWASP-compliant only - No insecure cookie-based storage
✅ Backward compatible - No breaking changes
✅ Zero-config default - Works out-of-box with
SessionCSRFStorage
✅ Comprehensive documentation - Full guide with examples
Configuration Example
moduleSettings = {
"cbwire": {
"csrfEnabled": true,
"csrfService": "SessionCSRFStorage@cbwire" // or
CacheCSRFStorage@cbwire
}
};
The implementation solves the "Page Expired" issue while providing flexibility for different deployment scenarios!