Skip to content

Commit 58c3374

Browse files
joaquinfelicijoaquinfelici
authored
fix: isolate CSRF cookie per physical-tenant scope (#458)
* fix: isolate CSRF cookie per physical-tenant scope * fix: scope CSRF cookie path to basePath on API chains * fix: derive CSRF cookie prefix from X_CSRF_TOKEN to mirror session naming * fix: also validate CSRF cookie name length in rejectCookieNameCollisions
1 parent 41a0902 commit 58c3374

8 files changed

Lines changed: 426 additions & 33 deletions

File tree

‎spring-boot-starter/src/main/java/io/camunda/security/spring/scope/ScopedApiSecurityChainBuilder.java‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
*/
88
package io.camunda.security.spring.scope;
99

10+
import static io.camunda.security.spring.security.CamundaSecurityFilterChainConstants.X_CSRF_TOKEN;
11+
1012
import io.camunda.security.api.model.config.AuthenticationConfiguration;
1113
import io.camunda.security.core.port.out.SecurityPathPort;
1214
import io.camunda.security.spring.CamundaSecurityLibraryProperties;
@@ -78,6 +80,25 @@ public SecurityFilterChain buildOidcApiChain(
7880
final JwtDecoder jwtDecoder,
7981
final SessionRepositoryFilter<?> sessionRepositoryFilter)
8082
throws Exception {
83+
return buildOidcApiChainWith(
84+
http,
85+
matchers,
86+
unprotectedMatchers,
87+
jwtDecoder,
88+
sessionRepositoryFilter,
89+
null,
90+
X_CSRF_TOKEN);
91+
}
92+
93+
private SecurityFilterChain buildOidcApiChainWith(
94+
final HttpSecurity http,
95+
final Collection<String> matchers,
96+
final Collection<String> unprotectedMatchers,
97+
final JwtDecoder jwtDecoder,
98+
final SessionRepositoryFilter<?> sessionRepositoryFilter,
99+
final String csrfCookiePath,
100+
final String csrfCookieName)
101+
throws Exception {
81102
Objects.requireNonNull(jwtDecoder, "jwtDecoder must not be null");
82103
LOG.debug(
83104
"Building OIDC API chain for matchers={}, unprotected={}", matchers, unprotectedMatchers);
@@ -112,7 +133,8 @@ public SecurityFilterChain buildOidcApiChain(
112133
.oauth2Login(AbstractHttpConfigurer::disable)
113134
.oidcLogout(AbstractHttpConfigurer::disable)
114135
.logout(AbstractHttpConfigurer::disable);
115-
SecurityFilterChainSupport.applyCsrfConfiguration(filterChainBuilder, properties, pathPort);
136+
SecurityFilterChainSupport.applyCsrfConfiguration(
137+
filterChainBuilder, properties, pathPort, csrfCookiePath, csrfCookieName);
116138
SecurityFilterChainSupport.setupSecureHeaders(filterChainBuilder, properties.getHttpHeaders());
117139

118140
return filterChainBuilder.build();
@@ -130,6 +152,18 @@ public SecurityFilterChain buildBasicApiChain(
130152
final Collection<String> unprotectedMatchers,
131153
final SessionRepositoryFilter<?> sessionRepositoryFilter)
132154
throws Exception {
155+
return buildBasicApiChainWith(
156+
http, matchers, unprotectedMatchers, sessionRepositoryFilter, null, X_CSRF_TOKEN);
157+
}
158+
159+
private SecurityFilterChain buildBasicApiChainWith(
160+
final HttpSecurity http,
161+
final Collection<String> matchers,
162+
final Collection<String> unprotectedMatchers,
163+
final SessionRepositoryFilter<?> sessionRepositoryFilter,
164+
final String csrfCookiePath,
165+
final String csrfCookieName)
166+
throws Exception {
133167
LOG.debug(
134168
"Building Basic API chain for matchers={}, unprotected={}", matchers, unprotectedMatchers);
135169
if (sessionRepositoryFilter != null) {
@@ -156,7 +190,8 @@ public SecurityFilterChain buildBasicApiChain(
156190
.sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.NEVER))
157191
.requestCache(cache -> cache.requestCache(new NullRequestCache()));
158192

159-
SecurityFilterChainSupport.applyCsrfConfiguration(filterChainBuilder, properties, pathPort);
193+
SecurityFilterChainSupport.applyCsrfConfiguration(
194+
filterChainBuilder, properties, pathPort, csrfCookiePath, csrfCookieName);
160195
SecurityFilterChainSupport.setupSecureHeaders(filterChainBuilder, properties.getHttpHeaders());
161196

162197
return filterChainBuilder.build();
@@ -200,14 +235,24 @@ public SecurityFilterChain buildScopedApiChain(
200235
}
201236
final var matchers = pathPort.apiPaths().stream().map(p -> prefix + p).toList();
202237
final var unprotected = pathPort.unprotectedApiPaths().stream().map(p -> prefix + p).toList();
238+
final var csrfCookieName = ScopedSecurityChainRegistrar.csrfCookieName(basePath);
203239
return switch (method) {
204240
case OIDC -> {
205241
final var decoder =
206242
Objects.requireNonNull(
207243
oidcDecoderSupplier.get(), "oidcDecoderSupplier must not return a null JwtDecoder");
208-
yield buildOidcApiChain(http, matchers, unprotected, decoder, sessionRepositoryFilter);
244+
yield buildOidcApiChainWith(
245+
http,
246+
matchers,
247+
unprotected,
248+
decoder,
249+
sessionRepositoryFilter,
250+
basePath,
251+
csrfCookieName);
209252
}
210-
case BASIC -> buildBasicApiChain(http, matchers, unprotected, sessionRepositoryFilter);
253+
case BASIC ->
254+
buildBasicApiChainWith(
255+
http, matchers, unprotected, sessionRepositoryFilter, basePath, csrfCookieName);
211256
default -> throw new IllegalStateException("Unsupported authentication method: " + method);
212257
};
213258
}
@@ -283,7 +328,12 @@ public SecurityFilterChain buildUnprotectedScopedApiChain(
283328
.formLogin(AbstractHttpConfigurer::disable)
284329
.anonymous(AbstractHttpConfigurer::disable);
285330

286-
SecurityFilterChainSupport.applyCsrfConfiguration(filterChainBuilder, properties, pathPort);
331+
SecurityFilterChainSupport.applyCsrfConfiguration(
332+
filterChainBuilder,
333+
properties,
334+
pathPort,
335+
basePath,
336+
ScopedSecurityChainRegistrar.csrfCookieName(basePath));
287337
SecurityFilterChainSupport.setupSecureHeaders(filterChainBuilder, properties.getHttpHeaders());
288338

289339
return filterChainBuilder.build();

‎spring-boot-starter/src/main/java/io/camunda/security/spring/scope/ScopedSecurityChainRegistrar.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package io.camunda.security.spring.scope;
99

1010
import static io.camunda.security.spring.security.CamundaSecurityFilterChainConstants.ORDER_WEBAPP_API;
11+
import static io.camunda.security.spring.security.CamundaSecurityFilterChainConstants.X_CSRF_TOKEN;
1112

1213
import io.camunda.security.api.context.CamundaSecurityScopeProvider;
1314
import io.camunda.security.api.model.config.ScopedSecurityDescriptor;
@@ -47,6 +48,7 @@
4748
final class ScopedSecurityChainRegistrar implements BeanDefinitionRegistryPostProcessor {
4849

4950
static final String SESSION_COOKIE_PREFIX = "camunda-session-";
51+
static final String CSRF_COOKIE_PREFIX = X_CSRF_TOKEN + "-";
5052
static final int MAX_COOKIE_NAME_LENGTH =
5153
200; // well under the RFC 6265 4096-byte name=value budget
5254

@@ -273,7 +275,8 @@ private OrderedSecurityFilterChainWrapper buildWebappChain(
273275
descriptor.basePath(),
274276
descriptor.authentication(),
275277
sessionFilter,
276-
sessionCookieName(descriptor.basePath()));
278+
sessionCookieName(descriptor.basePath()),
279+
csrfCookieName(descriptor.basePath()));
277280
return new OrderedSecurityFilterChainWrapper(chain, ORDER_WEBAPP_API);
278281
} catch (final IllegalStateException ex) {
279282
throw ex;
@@ -288,6 +291,11 @@ static String sessionCookieName(final String basePath) {
288291
return SESSION_COOKIE_PREFIX + sanitizeBasePath(basePath);
289292
}
290293

294+
/** The per-scope CSRF cookie name: {@code X-CSRF-TOKEN-<sanitize(basePath)>}. */
295+
static String csrfCookieName(final String basePath) {
296+
return CSRF_COOKIE_PREFIX + sanitizeBasePath(basePath);
297+
}
298+
291299
static void rejectCookieNameCollisions(final List<ScopedSecurityDescriptor> descriptors) {
292300
final var seen = new HashSet<String>();
293301
final var collisions = new LinkedHashSet<String>();
@@ -302,19 +310,21 @@ static void rejectCookieNameCollisions(final List<ScopedSecurityDescriptor> desc
302310
+ SESSION_COOKIE_PREFIX
303311
+ "'. Use a basePath containing alphanumerics.");
304312
}
305-
final var name = SESSION_COOKIE_PREFIX + suffix;
306-
if (name.length() > MAX_COOKIE_NAME_LENGTH) {
313+
final var sessionName = SESSION_COOKIE_PREFIX + suffix;
314+
final var csrfName = CSRF_COOKIE_PREFIX + suffix;
315+
if (sessionName.length() > MAX_COOKIE_NAME_LENGTH
316+
|| csrfName.length() > MAX_COOKIE_NAME_LENGTH) {
307317
throw new IllegalStateException(
308318
"Derived session cookie name for basePath="
309319
+ d.basePath()
310320
+ " exceeds the maximum length of "
311321
+ MAX_COOKIE_NAME_LENGTH
312322
+ " characters ("
313-
+ name.length()
323+
+ sessionName.length()
314324
+ "). Use a shorter basePath.");
315325
}
316-
if (!seen.add(name)) {
317-
collisions.add(name);
326+
if (!seen.add(sessionName)) {
327+
collisions.add(sessionName);
318328
}
319329
}
320330
if (!collisions.isEmpty()) {

‎spring-boot-starter/src/main/java/io/camunda/security/spring/security/ScopedWebappSecurityChainBuilder.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,8 @@ public SecurityFilterChain buildScopedWebappChain(
306306
final String basePath,
307307
final AuthenticationConfiguration authentication,
308308
final SessionRepositoryFilter<?> sessionRepositoryFilter,
309-
final String scopedSessionCookieName)
309+
final String scopedSessionCookieName,
310+
final String scopedCsrfCookieName)
310311
throws Exception {
311312
Objects.requireNonNull(http, "http must not be null");
312313
Objects.requireNonNull(basePath, "basePath must not be null");
@@ -316,6 +317,7 @@ public SecurityFilterChain buildScopedWebappChain(
316317
Objects.requireNonNull(authentication.getMethod(), "authentication.method must not be null");
317318
Objects.requireNonNull(sessionRepositoryFilter, "sessionRepositoryFilter must not be null");
318319
Objects.requireNonNull(scopedSessionCookieName, "scopedSessionCookieName must not be null");
320+
Objects.requireNonNull(scopedCsrfCookieName, "scopedCsrfCookieName must not be null");
319321
Objects.requireNonNull(
320322
authorizedClientManagerFactory, "authorizedClientManagerFactory must not be null");
321323
Objects.requireNonNull(
@@ -335,10 +337,15 @@ public SecurityFilterChain buildScopedWebappChain(
335337
return switch (authentication.getMethod()) {
336338
case OIDC ->
337339
buildOidcWebappChainInternal(
338-
http, prefix, authentication, sessionRepositoryFilter, scopedSessionCookieName);
340+
http,
341+
prefix,
342+
authentication,
343+
sessionRepositoryFilter,
344+
scopedSessionCookieName,
345+
scopedCsrfCookieName);
339346
case BASIC ->
340347
buildBasicWebappChainInternal(
341-
http, prefix, sessionRepositoryFilter, scopedSessionCookieName);
348+
http, prefix, sessionRepositoryFilter, scopedSessionCookieName, scopedCsrfCookieName);
342349
default ->
343350
throw new IllegalStateException(
344351
"Unsupported authentication method: " + authentication.getMethod());
@@ -404,7 +411,8 @@ private SecurityFilterChain buildOidcWebappChainInternal(
404411
final String prefix,
405412
final AuthenticationConfiguration authentication,
406413
final SessionRepositoryFilter<?> sessionRepositoryFilter,
407-
final String scopedSessionCookieName)
414+
final String scopedSessionCookieName,
415+
final String scopedCsrfCookieName)
408416
throws Exception {
409417

410418
final var matchers = pathPort.webappPaths().stream().map(p -> prefix + p).toList();
@@ -490,7 +498,7 @@ private SecurityFilterChain buildOidcWebappChainInternal(
490498
.addLogoutHandler(
491499
pathScopedCookieClearingLogoutHandler(scopedSessionCookieName, prefix))
492500
.addLogoutHandler(
493-
pathScopedCookieClearingLogoutHandler(X_CSRF_TOKEN, prefix));
501+
pathScopedCookieClearingLogoutHandler(scopedCsrfCookieName, prefix));
494502
logoutSuccessHandlerProvider.ifAvailable(logout::logoutSuccessHandler);
495503
});
496504

@@ -499,7 +507,7 @@ private SecurityFilterChain buildOidcWebappChainInternal(
499507
final var logoutHandler =
500508
new CompositeLogoutHandler(
501509
pathScopedCookieClearingLogoutHandler(scopedSessionCookieName, prefix),
502-
pathScopedCookieClearingLogoutHandler(X_CSRF_TOKEN, prefix),
510+
pathScopedCookieClearingLogoutHandler(scopedCsrfCookieName, prefix),
503511
new SecurityContextLogoutHandler());
504512
filterChainBuilder.addFilterAfter(
505513
new OAuth2RefreshTokenFilter(
@@ -515,7 +523,7 @@ private SecurityFilterChain buildOidcWebappChainInternal(
515523
}
516524

517525
SecurityFilterChainSupport.applyCsrfConfiguration(
518-
filterChainBuilder, properties, pathPort, prefix);
526+
filterChainBuilder, properties, pathPort, prefix, scopedCsrfCookieName);
519527
SecurityFilterChainSupport.setupSecureHeaders(filterChainBuilder, properties.getHttpHeaders());
520528

521529
// Install the multi-IdP login picker (GH-269): the custom entry point trips
@@ -532,7 +540,8 @@ private SecurityFilterChain buildBasicWebappChainInternal(
532540
final HttpSecurity http,
533541
final String prefix,
534542
final SessionRepositoryFilter<?> sessionRepositoryFilter,
535-
final String scopedSessionCookieName)
543+
final String scopedSessionCookieName,
544+
final String scopedCsrfCookieName)
536545
throws Exception {
537546

538547
final var matchers = pathPort.webappPaths().stream().map(p -> prefix + p).toList();
@@ -572,7 +581,7 @@ private SecurityFilterChain buildBasicWebappChainInternal(
572581
.addLogoutHandler(
573582
pathScopedCookieClearingLogoutHandler(scopedSessionCookieName, prefix))
574583
.addLogoutHandler(
575-
pathScopedCookieClearingLogoutHandler(X_CSRF_TOKEN, prefix)))
584+
pathScopedCookieClearingLogoutHandler(scopedCsrfCookieName, prefix)))
576585
.exceptionHandling(
577586
eh ->
578587
eh.authenticationEntryPoint(authFailureHandler)
@@ -590,7 +599,7 @@ private SecurityFilterChain buildBasicWebappChainInternal(
590599
}
591600

592601
SecurityFilterChainSupport.applyCsrfConfiguration(
593-
filterChainBuilder, properties, pathPort, prefix);
602+
filterChainBuilder, properties, pathPort, prefix, scopedCsrfCookieName);
594603
SecurityFilterChainSupport.setupSecureHeaders(filterChainBuilder, properties.getHttpHeaders());
595604

596605
return filterChainBuilder.build();

‎spring-boot-starter/src/main/java/io/camunda/security/spring/security/SecurityFilterChainSupport.java‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import jakarta.servlet.http.HttpServletResponse;
2424
import java.io.IOException;
2525
import java.util.HashSet;
26+
import java.util.Objects;
2627
import java.util.Set;
2728
import org.springframework.beans.factory.ObjectProvider;
2829
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
@@ -77,19 +78,32 @@ static Set<String> csrfAllowedPaths(
7778

7879
public static CookieCsrfTokenRepository cookieCsrfTokenRepository(
7980
final CamundaSecurityLibraryProperties properties) {
80-
return cookieCsrfTokenRepository(properties, null);
81+
return buildCookieCsrfTokenRepository(properties, null, X_CSRF_TOKEN);
8182
}
8283

8384
public static CookieCsrfTokenRepository cookieCsrfTokenRepository(
8485
final CamundaSecurityLibraryProperties properties, final String cookiePath) {
85-
return buildCookieCsrfTokenRepository(properties, resolveCookiePath(cookiePath));
86+
return buildCookieCsrfTokenRepository(properties, resolveCookiePath(cookiePath), X_CSRF_TOKEN);
87+
}
88+
89+
public static CookieCsrfTokenRepository cookieCsrfTokenRepository(
90+
final CamundaSecurityLibraryProperties properties,
91+
final String cookiePath,
92+
final String cookieName) {
93+
return buildCookieCsrfTokenRepository(properties, resolveCookiePath(cookiePath), cookieName);
8694
}
8795

8896
private static CookieCsrfTokenRepository buildCookieCsrfTokenRepository(
89-
final CamundaSecurityLibraryProperties properties, final String resolvedCookiePath) {
97+
final CamundaSecurityLibraryProperties properties,
98+
final String resolvedCookiePath,
99+
final String cookieName) {
100+
Objects.requireNonNull(cookieName, "cookieName must not be null");
101+
if (cookieName.isBlank()) {
102+
throw new IllegalArgumentException("cookieName must not be blank");
103+
}
90104
final CookieCsrfTokenRepository repository = new CookieCsrfTokenRepository();
91105
repository.setHeaderName(X_CSRF_TOKEN);
92-
repository.setCookieName(X_CSRF_TOKEN);
106+
repository.setCookieName(cookieName);
93107
final boolean httpOnly = properties.getCsrf().isCookieHttpOnly();
94108
repository.setCookieCustomizer(builder -> builder.httpOnly(httpOnly));
95109
if (resolvedCookiePath != null) {
@@ -122,7 +136,7 @@ public static void applyCsrfConfiguration(
122136
final CamundaSecurityLibraryProperties properties,
123137
final SecurityPathPort pathPort)
124138
throws Exception {
125-
applyCsrfConfiguration(http, properties, pathPort, null);
139+
applyCsrfConfiguration(http, properties, pathPort, null, X_CSRF_TOKEN);
126140
}
127141

128142
public static void applyCsrfConfiguration(
@@ -131,6 +145,16 @@ public static void applyCsrfConfiguration(
131145
final SecurityPathPort pathPort,
132146
final String cookiePath)
133147
throws Exception {
148+
applyCsrfConfiguration(http, properties, pathPort, cookiePath, X_CSRF_TOKEN);
149+
}
150+
151+
public static void applyCsrfConfiguration(
152+
final HttpSecurity http,
153+
final CamundaSecurityLibraryProperties properties,
154+
final SecurityPathPort pathPort,
155+
final String cookiePath,
156+
final String csrfCookieName)
157+
throws Exception {
134158
if (!properties.getCsrf().isEnabled()) {
135159
http.csrf(AbstractHttpConfigurer::disable);
136160
return;
@@ -140,7 +164,7 @@ public static void applyCsrfConfiguration(
140164

141165
final String resolvedCookiePath = resolveCookiePath(cookiePath);
142166
final CookieCsrfTokenRepository repo =
143-
buildCookieCsrfTokenRepository(properties, resolvedCookiePath);
167+
buildCookieCsrfTokenRepository(properties, resolvedCookiePath, csrfCookieName);
144168
final CsrfTokenRepository csrfTokenRepository =
145169
(resolvedCookiePath != null)
146170
? new ContextPathScopedCsrfTokenRepository(repo, resolvedCookiePath)

0 commit comments

Comments
 (0)