From 6c2d46aca3b9adc9791b3d3b9f92d981b0495a2c Mon Sep 17 00:00:00 2001 From: Doug Cain Date: Tue, 25 Aug 2026 17:20:04 +0100 Subject: [PATCH] fix: namespace the SAML request cache key by application samlRequestCacheName exists so the pending AuthnRequest IDs can live in a distributed cache when the application runs behind a load balancer, which the documentation asks for. Such a cache is shared by every application pointed at it, and CacheBox regions in separate applications resolve to one keyspace there - so two applications sharing a Redis region accepted each other's request IDs as pending. Not a way in on its own: the response still has to carry an assertion satisfying the receiving provider's expected issuer, audience and recipient, all of which are per-provider. It is the difference between "this application issued this AuthnRequest" and "some application sharing this cache did", which is the property tracking them is meant to establish. Keyed on the application name rather than a ColdBox setting because several applications commonly share one ColdBox configuration - the multi-tenant deployment that surfaced this serves identical code under a different this.name per tenant, so AppName is the same string for all of them. Found while testing 3.0.0 against Microsoft Entra on a three node Valkey cluster: the keys landed as boxlang-cbsso-saml-cbsso:saml-request:, carrying no discriminator between the tenants sharing that region. --- changelog.md | 8 +++++ models/SAMLRequestTracker.cfc | 29 ++++++++++++++++++- .../tests/specs/SAMLRequestTrackerSpec.cfc | 16 ++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index 282d78f..bbe3fc7 100644 --- a/changelog.md +++ b/changelog.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Namespace the SAML request cache key by application name. `samlRequestCacheName` exists so the pending + AuthnRequest IDs can live in a distributed cache behind a load balancer, but such a cache is shared by + every application pointed at it, so two applications on one region accepted each other's request IDs as + pending. Surfaced on a multi-tenant deployment serving the same code under a different `this.name` per + tenant. + ## [3.0.0] - 2026-08-25 ### Added diff --git a/models/SAMLRequestTracker.cfc b/models/SAMLRequestTracker.cfc index 5935c1b..9e9380a 100644 --- a/models/SAMLRequestTracker.cfc +++ b/models/SAMLRequestTracker.cfc @@ -7,7 +7,8 @@ component singleton { variables.cacheKeyPrefix = "cbsso:saml-request:"; public void function onDIComplete(){ - variables.requestCache = variables.cachebox.getCache( variables.cacheName ); + variables.requestCache = variables.cachebox.getCache( variables.cacheName ); + variables.cacheKeyPrefix = buildCacheKeyPrefix(); } public void function remember( required string requestId ){ @@ -35,4 +36,30 @@ component singleton { return variables.cacheKeyPrefix & arguments.requestId; } + /** + * Namespaced by application. A distributed cache - which is what `samlRequestCacheName` is for, and + * what the documentation asks for behind a load balancer - is shared by every application pointed at + * it, and CacheBox regions in separate applications resolve to one keyspace there. Two applications + * on one Redis region therefore accepted each other's request IDs as pending. + * + * That is not a way in on its own: the response still has to carry an assertion satisfying this + * provider's expected issuer, audience and recipient. It is the difference between "this application + * issued this AuthnRequest" and "some application sharing this cache did", which is the whole point + * of tracking them. + * + * The application name rather than a ColdBox setting, because several applications commonly share one + * ColdBox configuration - a multi-tenant deployment serving the same code under a different + * `this.name` per tenant is the case that surfaced this - and `AppName` would be identical across + * every one of them. + */ + private string function buildCacheKeyPrefix(){ + var applicationName = getApplicationMetadata().name ?: ""; + + if ( !len( trim( applicationName ) ) ) { + return "cbsso:saml-request:"; + } + + return "cbsso:" & trim( applicationName ) & ":saml-request:"; + } + } diff --git a/test-harness/tests/specs/SAMLRequestTrackerSpec.cfc b/test-harness/tests/specs/SAMLRequestTrackerSpec.cfc index e7e1d47..32393e3 100644 --- a/test-harness/tests/specs/SAMLRequestTrackerSpec.cfc +++ b/test-harness/tests/specs/SAMLRequestTrackerSpec.cfc @@ -21,6 +21,22 @@ component extends="coldbox.system.testing.BaseTestCase" { expect( variables.tracker.isPending( variables.requestId ) ).toBeTrue(); } ); + it( "namespaces the cache key by application", function(){ + // a distributed cache is shared by every application pointed at it, so an unnamespaced key + // lets one application's AuthnRequest ID read as pending in another + variables.tracker.remember( variables.requestId ); + + var applicationName = getApplicationMetadata().name; + var cache = getInstance( "cachebox" ).getCache( + getInstance( dsl = "coldbox:setting:samlRequestCacheName@cbsso" ) + ); + + expect( cache.lookupQuiet( "cbsso:#applicationName#:saml-request:#variables.requestId#" ) ).toBeTrue(); + expect( cache.lookupQuiet( "cbsso:saml-request:#variables.requestId#" ) ).toBeFalse( + "an unnamespaced key would collide with every other application on a shared cache" + ); + } ); + it( "consumes a request ID only once", function(){ variables.tracker.remember( variables.requestId );