From aa522dfd00727fa61cfd501719d9e8d1c8620b32 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 17:45:07 +0000 Subject: [PATCH 1/2] fix: explicit cbcsrf module settings win over the cbsecurity csrf key cbsecurity copied its csrf settings, defaults included, over the cbcsrf module settings, so settings set on cbcsrf itself were silently lost. The precedence is now explicit cbcsrf settings, then explicit cbsecurity.csrf, then the defaults. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01E8yy4oK5vnaVucmYLg4UtY --- changelog.md | 4 + models/CBSecurity.cfc | 93 +++++++++++++- .../tests/specs/unit/CsrfSettingsSpec.cfc | 119 ++++++++++++++++++ 3 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 test-harness/tests/specs/unit/CsrfSettingsSpec.cfc diff --git a/changelog.md b/changelog.md index 3f14102..69b88a9 100644 --- a/changelog.md +++ b/changelog.md @@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Handler/action annotation security now shares `processInvalidAccess()` with route middleware. Behavior is unchanged. +### Fixed + +- Settings you set on the `cbcsrf` module itself were silently overwritten by the `csrf` key of the cbsecurity settings, even when you never set that key, because cbsecurity copied its defaults over them. The precedence is now: settings you explicitly set in the `cbcsrf` module settings, then settings you explicitly set in `cbsecurity.csrf`, then the defaults. `cbsecurity.csrf` now shows the values that are in effect. + ## [3.8.0] - 2026-09-21 ### Changed diff --git a/models/CBSecurity.cfc b/models/CBSecurity.cfc index b8ed8ca..ba63a24 100644 --- a/models/CBSecurity.cfc +++ b/models/CBSecurity.cfc @@ -157,6 +157,12 @@ component threadsafe singleton accessors="true" { } function onDIComplete(){ + // What the user explicitly set in cbsecurity.csrf, captured before the defaults fill the gaps + var explicitCsrf = {}; + if ( structKeyExists( variables.settings, "csrf" ) && isStruct( variables.settings.csrf ) ) { + explicitCsrf = duplicate( variables.settings.csrf ); + } + // Default level-1 settings variables.settings.append( variables.DEFAULT_SETTINGS, false ); // Default level-2 settings @@ -197,14 +203,97 @@ component threadsafe singleton accessors="true" { log.info( "+ Basic Auth Validator Detected -> User Service set to BasicAuthUserService" ); } - // cbcsrf settings incorporation - variables.moduleSettings.cbcsrf.settings.append( variables.settings.csrf, true ); + // cbcsrf settings incorporation. Precedence, highest first: + // 1. Settings explicitly set in the cbcsrf module settings + // 2. Settings explicitly set in cbsecurity.csrf + // 3. The defaults, which are the same in both modules + var cbcsrfSettings = variables.moduleSettings.cbcsrf.settings; + cbcsrfSettings.append( + resolveCsrfSettings( + cbsecurityCsrf = explicitCsrf, + cbcsrfSettings = cbcsrfSettings, + cbcsrfExplicit = getExplicitCbcsrfKeys() + ), + true + ); + // Make cbsecurity.csrf show the values that are really in effect + for ( var csrfKey in variables.DEFAULT_SETTINGS.csrf ) { + if ( structKeyExists( cbcsrfSettings, csrfKey ) ) { + variables.settings.csrf[ csrfKey ] = cbcsrfSettings[ csrfKey ]; + } + } // DBLogger Configuration variables.dbLogger.configure(); // Log it log.info( "√ CBSecurity Services started and configured." ); } + /** + * Work out the settings for the cbcsrf module. The cbcsrf module settings win when the user set them + * explicitly, then the keys the user set in cbsecurity.csrf, then the defaults. + * + * A cbcsrf key counts as set by the user when it is in the module settings of the app configuration, or when + * its current value differs from the default, which also covers a `config/modules/cbcsrf.cfc` override. + * + * @cbsecurityCsrf The keys the user explicitly set in cbsecurity.csrf + * @cbcsrfSettings The current settings of the cbcsrf module + * @cbcsrfExplicit The cbcsrf keys the user set in the module settings of the app configuration + * + * @return The cbcsrf settings to use + */ + struct function resolveCsrfSettings( + required struct cbsecurityCsrf, + required struct cbcsrfSettings, + array cbcsrfExplicit = [] + ){ + var resolved = duplicate( arguments.cbcsrfSettings ); + + for ( var key in arguments.cbsecurityCsrf ) { + // The user configured this key on the cbcsrf module itself, so it wins + if ( arrayFindNoCase( arguments.cbcsrfExplicit, key ) ) { + continue; + } + // The value is not the default, so someone overrode it on the cbcsrf module, so it wins + if ( + structKeyExists( arguments.cbcsrfSettings, key ) && + structKeyExists( variables.DEFAULT_SETTINGS.csrf, key ) && + !sameValue( arguments.cbcsrfSettings[ key ], variables.DEFAULT_SETTINGS.csrf[ key ] ) + ) { + continue; + } + resolved[ key ] = arguments.cbsecurityCsrf[ key ]; + } + + return resolved; + } + + /** + * The keys the user explicitly set for the cbcsrf module in the moduleSettings of the app configuration. + * Returns an empty array if they cannot be read. + */ + private array function getExplicitCbcsrfKeys(){ + try { + var appSettings = variables.wirebox.getInstance( dsl = "coldbox" ).getConfigSettings(); + var moduleSettings = appSettings.coldBoxConfig.getPropertyMixin( "moduleSettings", "variables", {} ); + if ( structKeyExists( moduleSettings, "cbcsrf" ) && isStruct( moduleSettings.cbcsrf ) ) { + return structKeyArray( moduleSettings.cbcsrf ); + } + } catch ( any e ) { + log.warn( "Could not read the cbcsrf module settings from the app configuration: #e.message#" ); + } + return []; + } + + /** + * Compare two setting values, simple or complex + */ + private boolean function sameValue( required any a, required any b ){ + if ( isSimpleValue( arguments.a ) && isSimpleValue( arguments.b ) ) { + return compare( arguments.a, arguments.b ) == 0; + } + return serializeJSON( arguments.a ) == serializeJSON( arguments.b ); + } + /** * Get the default rule settings structure */ diff --git a/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc b/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc new file mode 100644 index 0000000..30c96f5 --- /dev/null +++ b/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc @@ -0,0 +1,119 @@ +/** + * Which settings win between the cbsecurity `csrf` key and the cbcsrf module settings + */ +component extends="coldbox.system.testing.BaseModelTest" model="cbsecurity.models.CBSecurity" { + + function beforeAll(){ + super.beforeAll(); + // The defaults that both modules share + variables.defaults = { + enableAutoVerifier : false, + verifyExcludes : [], + rotationTimeout : 30, + enableEndpoint : false, + cacheStorage : "CacheStorage@cbstorages", + enableAuthTokenRotator : true + }; + } + + function run(){ + describe( "cbcsrf settings precedence", function(){ + beforeEach( function(){ + setup(); + model.$property( + "DEFAULT_SETTINGS", + "variables", + { csrf : duplicate( defaults ) } + ); + } ); + + it( "leaves the cbcsrf settings alone when cbsecurity.csrf sets nothing", function(){ + var cbcsrf = duplicate( defaults ); + cbcsrf.rotationTimeout = 99; + cbcsrf.enableEndpoint = true; + + var resolved = model.resolveCsrfSettings( {}, cbcsrf ); + + expect( resolved.rotationTimeout ).toBe( 99 ); + expect( resolved.enableEndpoint ).toBeTrue(); + } ); + + it( "applies the keys the user set in cbsecurity.csrf", function(){ + var resolved = model.resolveCsrfSettings( { rotationTimeout : 77 }, duplicate( defaults ) ); + + expect( resolved.rotationTimeout ).toBe( 77 ); + expect( resolved.enableEndpoint ).toBeFalse(); + } ); + + it( "lets an explicit cbcsrf setting win over cbsecurity.csrf", function(){ + var cbcsrf = duplicate( defaults ); + cbcsrf.rotationTimeout = 99; + + var resolved = model.resolveCsrfSettings( + { rotationTimeout : 77 }, + cbcsrf, + [ "rotationTimeout" ] + ); + + expect( resolved.rotationTimeout ).toBe( 99 ); + } ); + + it( "matches the explicit cbcsrf keys without regard to case", function(){ + var cbcsrf = duplicate( defaults ); + cbcsrf.rotationTimeout = 99; + + var resolved = model.resolveCsrfSettings( + { rotationTimeout : 77 }, + cbcsrf, + [ "ROTATIONTIMEOUT" ] + ); + + expect( resolved.rotationTimeout ).toBe( 99 ); + } ); + + it( "lets a cbcsrf value that is not the default win even if it was set outside the app config", function(){ + // For example from config/modules/cbcsrf.cfc + var cbcsrf = duplicate( defaults ); + cbcsrf.rotationTimeout = 99; + + var resolved = model.resolveCsrfSettings( { rotationTimeout : 77 }, cbcsrf ); + + expect( resolved.rotationTimeout ).toBe( 99 ); + } ); + + it( "mixes the two modules key by key", function(){ + var cbcsrf = duplicate( defaults ); + cbcsrf.rotationTimeout = 99; + + var resolved = model.resolveCsrfSettings( + { rotationTimeout : 77, enableEndpoint : true }, + cbcsrf, + [ "rotationTimeout" ] + ); + + // cbcsrf wins where it was set, cbsecurity fills in the rest + expect( resolved.rotationTimeout ).toBe( 99 ); + expect( resolved.enableEndpoint ).toBeTrue(); + } ); + + it( "compares complex values to find out if they are still the default", function(){ + var cbcsrf = duplicate( defaults ); + cbcsrf.verifyExcludes = [ "stripe" ]; + + var resolved = model.resolveCsrfSettings( { verifyExcludes : [ "other" ] }, cbcsrf ); + expect( resolved.verifyExcludes ).toBe( [ "stripe" ] ); + + // An untouched empty array is still the default, so cbsecurity can set it + var fresh = model.resolveCsrfSettings( { verifyExcludes : [ "other" ] }, duplicate( defaults ) ); + expect( fresh.verifyExcludes ).toBe( [ "other" ] ); + } ); + + it( "does not change the settings it is given", function(){ + var cbcsrf = duplicate( defaults ); + model.resolveCsrfSettings( { rotationTimeout : 77 }, cbcsrf ); + expect( cbcsrf.rotationTimeout ).toBe( 30 ); + } ); + } ); + } + +} From d3802ded9f4e58d886dad4c24cedb6d600711071 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 18:07:57 +0000 Subject: [PATCH 2/2] fix: explicit cbsecurity.csrf keys win over cbcsrf module settings Only keys the user set in cbsecurity.csrf are applied on top of the cbcsrf module settings. Everything else keeps the cbcsrf value, so cbsecurity defaults never overwrite a cbcsrf override. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01E8yy4oK5vnaVucmYLg4UtY --- changelog.md | 2 +- models/CBSecurity.cfc | 71 +++---------------- .../tests/specs/unit/CsrfSettingsSpec.cfc | 57 +++------------ 3 files changed, 18 insertions(+), 112 deletions(-) diff --git a/changelog.md b/changelog.md index 69b88a9..6df2aab 100644 --- a/changelog.md +++ b/changelog.md @@ -23,7 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Settings you set on the `cbcsrf` module itself were silently overwritten by the `csrf` key of the cbsecurity settings, even when you never set that key, because cbsecurity copied its defaults over them. The precedence is now: settings you explicitly set in the `cbcsrf` module settings, then settings you explicitly set in `cbsecurity.csrf`, then the defaults. `cbsecurity.csrf` now shows the values that are in effect. +- Settings you set on the `cbcsrf` module itself were silently overwritten by the `csrf` key of the cbsecurity settings, even when you never set that key, because cbsecurity copied its defaults over them. Now only the keys you explicitly set in `cbsecurity.csrf` are applied, and they win over the `cbcsrf` module settings. Every other key keeps the `cbcsrf` value. `cbsecurity.csrf` now shows the values that are in effect. ## [3.8.0] - 2026-09-21 diff --git a/models/CBSecurity.cfc b/models/CBSecurity.cfc index ba63a24..26ae053 100644 --- a/models/CBSecurity.cfc +++ b/models/CBSecurity.cfc @@ -204,16 +204,11 @@ component threadsafe singleton accessors="true" { } // cbcsrf settings incorporation. Precedence, highest first: - // 1. Settings explicitly set in the cbcsrf module settings - // 2. Settings explicitly set in cbsecurity.csrf - // 3. The defaults, which are the same in both modules + // 1. Settings explicitly set in cbsecurity.csrf + // 2. Settings of the cbcsrf module (its own overrides or its defaults) var cbcsrfSettings = variables.moduleSettings.cbcsrf.settings; cbcsrfSettings.append( - resolveCsrfSettings( - cbsecurityCsrf = explicitCsrf, - cbcsrfSettings = cbcsrfSettings, - cbcsrfExplicit = getExplicitCbcsrfKeys() - ), + resolveCsrfSettings( cbsecurityCsrf = explicitCsrf, cbcsrfSettings = cbcsrfSettings ), true ); // Make cbsecurity.csrf show the values that are really in effect @@ -229,71 +224,21 @@ component threadsafe singleton accessors="true" { } /** - * Work out the settings for the cbcsrf module. The cbcsrf module settings win when the user set them - * explicitly, then the keys the user set in cbsecurity.csrf, then the defaults. - * - * A cbcsrf key counts as set by the user when it is in the module settings of the app configuration, or when - * its current value differs from the default, which also covers a `config/modules/cbcsrf.cfc` override. + * Work out the settings for the cbcsrf module. The keys the user explicitly set in cbsecurity.csrf win, every + * other key keeps the value the cbcsrf module already has, so its own overrides are never replaced by + * cbsecurity defaults. * * @cbsecurityCsrf The keys the user explicitly set in cbsecurity.csrf * @cbcsrfSettings The current settings of the cbcsrf module - * @cbcsrfExplicit The cbcsrf keys the user set in the module settings of the app configuration * * @return The cbcsrf settings to use */ - struct function resolveCsrfSettings( - required struct cbsecurityCsrf, - required struct cbcsrfSettings, - array cbcsrfExplicit = [] - ){ + struct function resolveCsrfSettings( required struct cbsecurityCsrf, required struct cbcsrfSettings ){ var resolved = duplicate( arguments.cbcsrfSettings ); - - for ( var key in arguments.cbsecurityCsrf ) { - // The user configured this key on the cbcsrf module itself, so it wins - if ( arrayFindNoCase( arguments.cbcsrfExplicit, key ) ) { - continue; - } - // The value is not the default, so someone overrode it on the cbcsrf module, so it wins - if ( - structKeyExists( arguments.cbcsrfSettings, key ) && - structKeyExists( variables.DEFAULT_SETTINGS.csrf, key ) && - !sameValue( arguments.cbcsrfSettings[ key ], variables.DEFAULT_SETTINGS.csrf[ key ] ) - ) { - continue; - } - resolved[ key ] = arguments.cbsecurityCsrf[ key ]; - } - + resolved.append( arguments.cbsecurityCsrf, true ); return resolved; } - /** - * The keys the user explicitly set for the cbcsrf module in the moduleSettings of the app configuration. - * Returns an empty array if they cannot be read. - */ - private array function getExplicitCbcsrfKeys(){ - try { - var appSettings = variables.wirebox.getInstance( dsl = "coldbox" ).getConfigSettings(); - var moduleSettings = appSettings.coldBoxConfig.getPropertyMixin( "moduleSettings", "variables", {} ); - if ( structKeyExists( moduleSettings, "cbcsrf" ) && isStruct( moduleSettings.cbcsrf ) ) { - return structKeyArray( moduleSettings.cbcsrf ); - } - } catch ( any e ) { - log.warn( "Could not read the cbcsrf module settings from the app configuration: #e.message#" ); - } - return []; - } - - /** - * Compare two setting values, simple or complex - */ - private boolean function sameValue( required any a, required any b ){ - if ( isSimpleValue( arguments.a ) && isSimpleValue( arguments.b ) ) { - return compare( arguments.a, arguments.b ) == 0; - } - return serializeJSON( arguments.a ) == serializeJSON( arguments.b ); - } - /** * Get the default rule settings structure */ diff --git a/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc b/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc index 30c96f5..6ace689 100644 --- a/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc +++ b/test-harness/tests/specs/unit/CsrfSettingsSpec.cfc @@ -20,11 +20,6 @@ component extends="coldbox.system.testing.BaseModelTest" model="cbsecurity.model describe( "cbcsrf settings precedence", function(){ beforeEach( function(){ setup(); - model.$property( - "DEFAULT_SETTINGS", - "variables", - { csrf : duplicate( defaults ) } - ); } ); it( "leaves the cbcsrf settings alone when cbsecurity.csrf sets nothing", function(){ @@ -38,74 +33,40 @@ component extends="coldbox.system.testing.BaseModelTest" model="cbsecurity.model expect( resolved.enableEndpoint ).toBeTrue(); } ); - it( "applies the keys the user set in cbsecurity.csrf", function(){ + it( "applies the keys the user set in cbsecurity.csrf over the defaults", function(){ var resolved = model.resolveCsrfSettings( { rotationTimeout : 77 }, duplicate( defaults ) ); expect( resolved.rotationTimeout ).toBe( 77 ); expect( resolved.enableEndpoint ).toBeFalse(); } ); - it( "lets an explicit cbcsrf setting win over cbsecurity.csrf", function(){ - var cbcsrf = duplicate( defaults ); - cbcsrf.rotationTimeout = 99; - - var resolved = model.resolveCsrfSettings( - { rotationTimeout : 77 }, - cbcsrf, - [ "rotationTimeout" ] - ); - - expect( resolved.rotationTimeout ).toBe( 99 ); - } ); - - it( "matches the explicit cbcsrf keys without regard to case", function(){ - var cbcsrf = duplicate( defaults ); - cbcsrf.rotationTimeout = 99; - - var resolved = model.resolveCsrfSettings( - { rotationTimeout : 77 }, - cbcsrf, - [ "ROTATIONTIMEOUT" ] - ); - - expect( resolved.rotationTimeout ).toBe( 99 ); - } ); - - it( "lets a cbcsrf value that is not the default win even if it was set outside the app config", function(){ - // For example from config/modules/cbcsrf.cfc + it( "lets an explicit cbsecurity.csrf key win over a cbcsrf override", function(){ var cbcsrf = duplicate( defaults ); cbcsrf.rotationTimeout = 99; var resolved = model.resolveCsrfSettings( { rotationTimeout : 77 }, cbcsrf ); - expect( resolved.rotationTimeout ).toBe( 99 ); + expect( resolved.rotationTimeout ).toBe( 77 ); } ); - it( "mixes the two modules key by key", function(){ + it( "keeps cbcsrf overrides for keys cbsecurity did not set", function(){ var cbcsrf = duplicate( defaults ); cbcsrf.rotationTimeout = 99; + cbcsrf.enableEndpoint = true; - var resolved = model.resolveCsrfSettings( - { rotationTimeout : 77, enableEndpoint : true }, - cbcsrf, - [ "rotationTimeout" ] - ); + var resolved = model.resolveCsrfSettings( { rotationTimeout : 77 }, cbcsrf ); - // cbcsrf wins where it was set, cbsecurity fills in the rest - expect( resolved.rotationTimeout ).toBe( 99 ); + expect( resolved.rotationTimeout ).toBe( 77 ); expect( resolved.enableEndpoint ).toBeTrue(); } ); - it( "compares complex values to find out if they are still the default", function(){ + it( "applies complex values from cbsecurity.csrf", function(){ var cbcsrf = duplicate( defaults ); cbcsrf.verifyExcludes = [ "stripe" ]; var resolved = model.resolveCsrfSettings( { verifyExcludes : [ "other" ] }, cbcsrf ); - expect( resolved.verifyExcludes ).toBe( [ "stripe" ] ); - // An untouched empty array is still the default, so cbsecurity can set it - var fresh = model.resolveCsrfSettings( { verifyExcludes : [ "other" ] }, duplicate( defaults ) ); - expect( fresh.verifyExcludes ).toBe( [ "other" ] ); + expect( resolved.verifyExcludes ).toBe( [ "other" ] ); } ); it( "does not change the settings it is given", function(){