diff --git a/.github/workflows/update-antora-ui-spring.yml b/.github/workflows/update-antora-ui-spring.yml index 3863b100571..f1309ed3012 100644 --- a/.github/workflows/update-antora-ui-spring.yml +++ b/.github/workflows/update-antora-ui-spring.yml @@ -18,7 +18,7 @@ jobs: matrix: branch: [ '5.8.x', '6.2.x', '6.3.x', 'main' ] steps: - - uses: spring-io/spring-doc-actions/update-antora-spring-ui@852920ba3fb1f28b35a2f13201133bc00ef33677 + - uses: spring-io/spring-doc-actions/update-antora-spring-ui@e28269199d1d27975cf7f65e16d6095c555b3cd0 name: Update with: docs-branch: ${{ matrix.branch }} @@ -28,7 +28,7 @@ jobs: runs-on: ubuntu-latest name: Update on docs-build steps: - - uses: spring-io/spring-doc-actions/update-antora-spring-ui@852920ba3fb1f28b35a2f13201133bc00ef33677 + - uses: spring-io/spring-doc-actions/update-antora-spring-ui@e28269199d1d27975cf7f65e16d6095c555b3cd0 name: Update with: docs-branch: 'docs-build' diff --git a/build.gradle b/build.gradle index 25f41aa7800..aee73839861 100644 --- a/build.gradle +++ b/build.gradle @@ -124,7 +124,7 @@ wrapperUpgrade { gradle { 'spring-security' { repo = 'spring-projects/spring-security' - baseBranch = '6.2.x' // runs only on 6.2.x and the update is merged forward to main + baseBranch = '6.3.x' // runs only on 6.3.x and the update is merged forward to main } } } diff --git a/config/src/main/java/org/springframework/security/config/http/CsrfBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/CsrfBeanDefinitionParser.java index e8cbad916d9..6b1123ed370 100644 --- a/config/src/main/java/org/springframework/security/config/http/CsrfBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/CsrfBeanDefinitionParser.java @@ -183,6 +183,9 @@ BeanDefinition getCsrfAuthenticationStrategy() { BeanDefinitionBuilder csrfAuthenticationStrategy = BeanDefinitionBuilder .rootBeanDefinition(CsrfAuthenticationStrategy.class); csrfAuthenticationStrategy.addConstructorArgReference(this.csrfRepositoryRef); + if (StringUtils.hasText(this.requestHandlerRef)) { + csrfAuthenticationStrategy.addPropertyReference("requestHandler", this.requestHandlerRef); + } return csrfAuthenticationStrategy.getBeanDefinition(); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index 17d8f8a3a92..7117b8c81bd 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,6 +56,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Role; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.AnnotationConfigurationException; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.PermissionEvaluator; @@ -1103,6 +1104,21 @@ public void jsr250MethodWhenExcludeAuthorizationObservationsThenUnobserved() { verifyNoInteractions(handler); } + // gh-16819 + @Test + void autowireWhenDefaultsThenAdvisorAnnotationsAreSorted() { + this.spring.register(MethodSecurityServiceConfig.class).autowire(); + AuthorizationAdvisorProxyFactory proxyFactory = this.spring.getContext() + .getBean(AuthorizationAdvisorProxyFactory.class); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + private static Consumer disallowBeanOverriding() { return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java index c247a6d7fed..6b263c7048d 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/resource/OAuth2ResourceServerConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1560,12 +1560,15 @@ static class JwkSetUriConfig { @Bean SecurityFilterChain filterChain(HttpSecurity http) throws Exception { // @formatter:off + DefaultBearerTokenResolver defaultBearerTokenResolver = new DefaultBearerTokenResolver(); + defaultBearerTokenResolver.setAllowUriQueryParameter(true); http .authorizeRequests() .requestMatchers("/requires-read-scope").access("hasAuthority('SCOPE_message:read')") .anyRequest().authenticated() .and() .oauth2ResourceServer() + .bearerTokenResolver(defaultBearerTokenResolver) .jwt() .jwkSetUri(this.jwkSetUri); return http.build(); diff --git a/config/src/test/java/org/springframework/security/config/http/CsrfConfigTests.java b/config/src/test/java/org/springframework/security/config/http/CsrfConfigTests.java index 901945e73aa..311c513763e 100644 --- a/config/src/test/java/org/springframework/security/config/http/CsrfConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/CsrfConfigTests.java @@ -336,6 +336,43 @@ public void postWhenUsingCsrfAndXorCsrfTokenRequestAttributeHandlerWithRawTokenT // @formatter:on } + @Test + public void postWhenUsingCsrfAndXorCsrfTokenRequestAttributeHandlerThenCsrfAuthenticationStrategyUses() + throws Exception { + this.spring.configLocations(this.xml("WithXorCsrfTokenRequestAttributeHandler"), this.xml("shared-controllers")) + .autowire(); + // @formatter:off + MvcResult mvcResult1 = this.mvc.perform(get("/csrf")) + .andExpect(status().isOk()) + .andReturn(); + // @formatter:on + MockHttpServletRequest request1 = mvcResult1.getRequest(); + MockHttpSession session = (MockHttpSession) request1.getSession(); + CsrfTokenRepository repository = WebTestUtils.getCsrfTokenRepository(request1); + // @formatter:off + MockHttpServletRequestBuilder login = post("/login") + .param("username", "user") + .param("password", "password") + .session(session) + .with(csrf()); + this.mvc.perform(login) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/")); + // @formatter:on + assertThat(repository.loadToken(request1)).isNull(); + // @formatter:off + MvcResult mvcResult2 = this.mvc.perform(get("/csrf").session(session)) + .andExpect(status().isOk()) + .andReturn(); + // @formatter:on + MockHttpServletRequest request2 = mvcResult2.getRequest(); + CsrfToken csrfToken = repository.loadToken(request2); + CsrfToken csrfTokenAttribute = (CsrfToken) request2.getAttribute(CsrfToken.class.getName()); + assertThat(csrfTokenAttribute).isNotNull(); + assertThat(csrfTokenAttribute.getToken()).isNotBlank(); + assertThat(csrfTokenAttribute.getToken()).isNotEqualTo(csrfToken.getToken()); + } + @Test public void postWhenHasCsrfTokenButSessionExpiresThenRequestIsCancelledAfterSuccessfulAuthentication() throws Exception { diff --git a/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml b/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml index 3f81363d270..aac12989e91 100644 --- a/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml +++ b/config/src/test/resources/org/springframework/security/config/http/OAuth2ResourceServerBeanDefinitionParserTests-JwkSetUri.xml @@ -25,10 +25,15 @@ + + + + - + diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java index 5cf36b5fa21..3b18466743c 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizationAdvisorProxyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,6 +47,7 @@ import org.springframework.aop.Pointcut; import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.aop.framework.ProxyFactory; +import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.lang.NonNull; import org.springframework.security.authorization.AuthorizationProxyFactory; @@ -79,8 +80,8 @@ * @author Josh Cummings * @since 6.3 */ -public final class AuthorizationAdvisorProxyFactory - implements AuthorizationProxyFactory, Iterable, AopInfrastructureBean { +public final class AuthorizationAdvisorProxyFactory implements AuthorizationProxyFactory, + Iterable, AopInfrastructureBean, SmartInitializingSingleton { private static final boolean isReactivePresent = ClassUtils.isPresent("reactor.core.publisher.Mono", null); @@ -125,6 +126,7 @@ public static AuthorizationAdvisorProxyFactory withDefaults() { advisors.add(new PostFilterAuthorizationMethodInterceptor()); AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + AnnotationAwareOrderComparator.sort(proxyFactory.advisors); return proxyFactory; } @@ -142,9 +144,15 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() { advisors.add(new PostFilterAuthorizationReactiveMethodInterceptor()); AuthorizationAdvisorProxyFactory proxyFactory = new AuthorizationAdvisorProxyFactory(advisors); proxyFactory.addAdvisor(new AuthorizeReturnObjectMethodInterceptor(proxyFactory)); + AnnotationAwareOrderComparator.sort(proxyFactory.advisors); return proxyFactory; } + @Override + public void afterSingletonsInstantiated() { + AnnotationAwareOrderComparator.sort(this.advisors); + } + /** * Proxy an object to enforce authorization advice. * @@ -165,7 +173,6 @@ public static AuthorizationAdvisorProxyFactory withReactiveDefaults() { */ @Override public Object proxy(Object target) { - AnnotationAwareOrderComparator.sort(this.advisors); if (target == null) { return null; } @@ -178,9 +185,9 @@ public Object proxy(Object target) { } ProxyFactory factory = new ProxyFactory(target); factory.addAdvisors(this.authorizationProxy); - for (Advisor advisor : this.advisors) { - factory.addAdvisors(advisor); - } + List advisors = new ArrayList<>(this.advisors); + AnnotationAwareOrderComparator.sort(advisors); + factory.addAdvisors(advisors); factory.addInterface(AuthorizationProxy.class); factory.setOpaque(true); factory.setProxyTargetClass(!Modifier.isFinal(target.getClass().getModifiers())); diff --git a/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java index cb06a59785b..5308b8d251f 100644 --- a/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java +++ b/core/src/main/java/org/springframework/security/authorization/method/AuthorizeReturnObjectMethodInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -48,7 +48,7 @@ public final class AuthorizeReturnObjectMethodInterceptor implements Authorizati private int order = AuthorizationInterceptorsOrder.SECURE_RESULT.getOrder(); public AuthorizeReturnObjectMethodInterceptor(AuthorizationProxyFactory authorizationProxyFactory) { - Assert.notNull(authorizationProxyFactory, "authorizationManager cannot be null"); + Assert.notNull(authorizationProxyFactory, "authorizationProxyFactory cannot be null"); this.authorizationProxyFactory = authorizationProxyFactory; } diff --git a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java index aa5a2499638..93d7ee1520c 100644 --- a/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java +++ b/core/src/test/java/org/springframework/security/authorization/AuthorizationAdvisorProxyFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,6 +40,7 @@ import org.junit.jupiter.api.Test; import org.springframework.aop.Pointcut; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authentication.TestAuthentication; @@ -360,6 +361,32 @@ public void proxyWhenDefaultsThenInstanceOfAuthorizationProxy() { assertThat(target).isSameAs(this.flight); } + // gh-16819 + @Test + void advisorsWhenWithDefaultsThenAreSorted() { + AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withDefaults(); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + + // gh-16819 + @Test + void advisorsWhenWithReactiveDefaultsThenAreSorted() { + AuthorizationAdvisorProxyFactory proxyFactory = AuthorizationAdvisorProxyFactory.withReactiveDefaults(); + AnnotationAwareOrderComparator comparator = AnnotationAwareOrderComparator.INSTANCE; + AuthorizationAdvisor previous = null; + for (AuthorizationAdvisor advisor : proxyFactory) { + boolean ordered = previous == null || comparator.compare(previous, advisor) < 0; + assertThat(ordered).isTrue(); + previous = advisor; + } + } + private Authentication authenticated(String user, String... authorities) { return TestAuthentication.authenticated(TestAuthentication.withUsername(user).authorities(authorities).build()); } diff --git a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java index 172928401e8..4f19f52de0f 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java +++ b/crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCrypt.java @@ -611,7 +611,8 @@ private static String hashpw(byte passwordb[], String salt, boolean for_check) { int rounds, off; StringBuilder rs = new StringBuilder(); - if (passwordb.length > 72) { + // Enforce max length for new passwords only + if (!for_check && passwordb.length > 72) { throw new IllegalArgumentException("password cannot be more than 72 bytes"); } if (salt == null) { diff --git a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java index df14ebe9064..f2921064fa8 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java @@ -223,13 +223,34 @@ public void checkWhenNoRoundsThenTrue() { } @Test - public void enforcePasswordLength() { + public void encodeWhenPasswordOverMaxLengthThenThrowIllegalArgumentException() { BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + String password72chars = "123456789012345678901234567890123456789012345678901234567890123456789012"; - assertThat(encoder.matches(password72chars, encoder.encode(password72chars))).isTrue(); - String password73chars = password72chars.concat("a"); - assertThatIllegalArgumentException() - .isThrownBy(() -> encoder.matches(password73chars, encoder.encode(password73chars))); + encoder.encode(password72chars); + + String password73chars = password72chars + "3"; + assertThatIllegalArgumentException().isThrownBy(() -> encoder.encode(password73chars)); + } + + @Test + public void matchesWhenPasswordOverMaxLengthThenAllowToMatch() { + BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + + String password71chars = "12345678901234567890123456789012345678901234567890123456789012345678901"; + String encodedPassword71chars = "$2a$10$jx3x2FaF.iX5QZ9i3O424Os2Ou5P5JrnedmWYHuDyX8JKA4Unp4xq"; + assertThat(encoder.matches(password71chars, encodedPassword71chars)).isTrue(); + + String password72chars = password71chars + "2"; + String encodedPassword72chars = "$2a$10$oXYO6/UvbsH5rQEraBkl6uheccBqdB3n.RaWbrimog9hS2GX4lo/O"; + assertThat(encoder.matches(password72chars, encodedPassword72chars)).isTrue(); + + // Max length is 72 bytes, however, we need to ensure backwards compatibility + // for previously encoded passwords that are greater than 72 bytes and allow the + // match to be performed. + String password73chars = password72chars + "3"; + String encodedPassword73chars = "$2a$10$1l9.kvQTsqNLiCYFqmKtQOHkp.BrgIrwsnTzWo9jdbQRbuBYQ/AVK"; + assertThat(encoder.matches(password73chars, encodedPassword73chars)).isTrue(); } } diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc index e877ebc10c4..07813e737a0 100644 --- a/docs/modules/ROOT/nav.adoc +++ b/docs/modules/ROOT/nav.adoc @@ -132,7 +132,7 @@ *** xref:servlet/appendix/faq.adoc[FAQ] * xref:reactive/index.adoc[Reactive Applications] ** xref:reactive/getting-started.adoc[Getting Started] -** Authentication +** xref:reactive/authentication/index.adoc[Authentication] *** xref:reactive/authentication/x509.adoc[X.509 Authentication] *** xref:reactive/authentication/logout.adoc[Logout] *** Session Management diff --git a/docs/modules/ROOT/pages/features/authentication/index.adoc b/docs/modules/ROOT/pages/features/authentication/index.adoc index 6d02574e501..d542fed5355 100644 --- a/docs/modules/ROOT/pages/features/authentication/index.adoc +++ b/docs/modules/ROOT/pages/features/authentication/index.adoc @@ -8,4 +8,4 @@ Once authentication is performed we know the identity and can perform authorizat Spring Security provides built-in support for authenticating users. This section is dedicated to generic authentication support that applies in both Servlet and WebFlux environments. -Refer to the sections on authentication for xref:servlet/authentication/index.adoc#servlet-authentication[Servlet] and xref:servlet/authentication/index.adoc[WebFlux] for details on what is supported for each stack. +Refer to the sections on authentication for xref:servlet/authentication/index.adoc[Servlet] and xref:reactive/authentication/index.adoc[WebFlux] for details on what is supported for each stack. diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc index e25586929b9..0210ebdc814 100644 --- a/docs/modules/ROOT/pages/index.adoc +++ b/docs/modules/ROOT/pages/index.adoc @@ -1,5 +1,10 @@ = Spring Security +[NOTE] +==== +Spring Security's documentation can be https://docs.spring.io/spring-security/reference/spring-security-docs.zip[downloaded] as a zip file. +==== + Spring Security is a framework that provides xref:features/authentication/index.adoc[authentication], xref:features/authorization/index.adoc[authorization], and xref:features/exploits/index.adoc[protection against common attacks]. With first class support for securing both xref:servlet/index.adoc[imperative] and xref:reactive/index.adoc[reactive] applications, it is the de-facto standard for securing Spring-based applications. diff --git a/docs/modules/ROOT/pages/reactive/authentication/index.adoc b/docs/modules/ROOT/pages/reactive/authentication/index.adoc new file mode 100644 index 00000000000..a8c7f92f758 --- /dev/null +++ b/docs/modules/ROOT/pages/reactive/authentication/index.adoc @@ -0,0 +1,3 @@ +[[webflux-authentication]] += Authentication +:page-section-summary-toc: 1 \ No newline at end of file diff --git a/docs/modules/ROOT/pages/servlet/appendix/namespace/http.adoc b/docs/modules/ROOT/pages/servlet/appendix/namespace/http.adoc index 59e48e0986d..1a137e8c97c 100644 --- a/docs/modules/ROOT/pages/servlet/appendix/namespace/http.adoc +++ b/docs/modules/ROOT/pages/servlet/appendix/namespace/http.adoc @@ -34,7 +34,7 @@ The attributes on the `` element control some of the properties on the cor Use AuthorizationManager API instead of SecurityMetadataSource (defaults to true) [[nsa-http-authorization-manager-ref]] -* **access-decision-manager-ref** +* **use-authorization-manager** Use this AuthorizationManager instead of deriving one from elements [[nsa-http-access-decision-manager-ref]] diff --git a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc index 4eaf5f3d5ee..debf2340922 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -1048,8 +1048,8 @@ public class SecurityConfig { http .securityMatcher("/api/**") <1> .authorizeHttpRequests(authorize -> authorize - .requestMatchers("/user/**").hasRole("USER") <2> - .requestMatchers("/admin/**").hasRole("ADMIN") <3> + .requestMatchers("/api/user/**").hasRole("USER") <2> + .requestMatchers("/api/admin/**").hasRole("ADMIN") <3> .anyRequest().authenticated() <4> ) .formLogin(withDefaults()); @@ -1071,8 +1071,8 @@ open class SecurityConfig { http { securityMatcher("/api/**") <1> authorizeHttpRequests { - authorize("/user/**", hasRole("USER")) <2> - authorize("/admin/**", hasRole("ADMIN")) <3> + authorize("/api/user/**", hasRole("USER")) <2> + authorize("/api/admin/**", hasRole("ADMIN")) <3> authorize(anyRequest, authenticated) <4> } } @@ -1084,8 +1084,8 @@ open class SecurityConfig { ====== <1> Configure `HttpSecurity` to only be applied to URLs that start with `/api/` -<2> Allow access to URLs that start with `/user/` to users with the `USER` role -<3> Allow access to URLs that start with `/admin/` to users with the `ADMIN` role +<2> Allow access to URLs that start with `/api/user/` to users with the `USER` role +<3> Allow access to URLs that start with `/api/admin/` to users with the `ADMIN` role <4> Any other request that doesn't match the rules above, will require authentication The `securityMatcher(s)` and `requestMatcher(s)` methods will decide which `RequestMatcher` implementation fits best for your application: If {spring-framework-reference-url}web.html#spring-web[Spring MVC] is in the classpath, then javadoc:org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher[] will be used, otherwise, javadoc:org.springframework.security.web.util.matcher.AntPathRequestMatcher[] will be used. @@ -1111,8 +1111,8 @@ public class SecurityConfig { http .securityMatcher(antMatcher("/api/**")) <2> .authorizeHttpRequests(authorize -> authorize - .requestMatchers(antMatcher("/user/**")).hasRole("USER") <3> - .requestMatchers(regexMatcher("/admin/.*")).hasRole("ADMIN") <4> + .requestMatchers(antMatcher("/api/user/**")).hasRole("USER") <3> + .requestMatchers(regexMatcher("/api/admin/.*")).hasRole("ADMIN") <4> .requestMatchers(new MyCustomRequestMatcher()).hasRole("SUPERVISOR") <5> .anyRequest().authenticated() ) @@ -1146,8 +1146,8 @@ open class SecurityConfig { http { securityMatcher(antMatcher("/api/**")) <2> authorizeHttpRequests { - authorize(antMatcher("/user/**"), hasRole("USER")) <3> - authorize(regexMatcher("/admin/**"), hasRole("ADMIN")) <4> + authorize(antMatcher("/api/user/**"), hasRole("USER")) <3> + authorize(regexMatcher("/api/admin/**"), hasRole("ADMIN")) <4> authorize(MyCustomRequestMatcher(), hasRole("SUPERVISOR")) <5> authorize(anyRequest, authenticated) } @@ -1161,8 +1161,8 @@ open class SecurityConfig { <1> Import the static factory methods from `AntPathRequestMatcher` and `RegexRequestMatcher` to create `RequestMatcher` instances. <2> Configure `HttpSecurity` to only be applied to URLs that start with `/api/`, using `AntPathRequestMatcher` -<3> Allow access to URLs that start with `/user/` to users with the `USER` role, using `AntPathRequestMatcher` -<4> Allow access to URLs that start with `/admin/` to users with the `ADMIN` role, using `RegexRequestMatcher` +<3> Allow access to URLs that start with `/api/user/` to users with the `USER` role, using `AntPathRequestMatcher` +<4> Allow access to URLs that start with `/api/admin/` to users with the `ADMIN` role, using `RegexRequestMatcher` <5> Allow access to URLs that match the `MyCustomRequestMatcher` to users with the `SUPERVISOR` role, using a custom `RequestMatcher` == Further Reading diff --git a/docs/modules/ROOT/pages/servlet/test/mockmvc/index.adoc b/docs/modules/ROOT/pages/servlet/test/mockmvc/index.adoc index 49d97c46a6f..67450d9b5da 100644 --- a/docs/modules/ROOT/pages/servlet/test/mockmvc/index.adoc +++ b/docs/modules/ROOT/pages/servlet/test/mockmvc/index.adoc @@ -2,4 +2,4 @@ = Spring MVC Test Integration :page-section-summary-toc: 1 -Spring Security provides comprehensive integration with https://docs.spring.io/spring/docs/current/spring-framework-reference/html/testing.html#spring-mvc-test-framework[Spring MVC Test] +Spring Security provides comprehensive integration with {spring-framework-reference-url}testing/mockmvc.html[Spring MVC Test] diff --git a/gradle.properties b/gradle.properties index 8b9f4b9fb39..6070f29c787 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,7 +14,7 @@ # limitations under the License. # springBootVersion=3.3.3 -version=6.4.4 +version=6.4.5 samplesBranch=main org.gradle.jvmargs=-Xmx3g -XX:+HeapDumpOnOutOfMemoryError org.gradle.parallel=true diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6d3d1adaf41..1bf6c5e9f42 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,18 +6,18 @@ io-spring-nohttp = "0.0.11" jakarta-websocket = "2.2.0" org-apache-directory-server = "1.5.5" org-apache-maven-resolver = "1.9.22" -org-aspectj = "1.9.22.1" +org-aspectj = "1.9.24" org-bouncycastle = "1.79" -org-eclipse-jetty = "11.0.24" +org-eclipse-jetty = "11.0.25" org-jetbrains-kotlin = "1.9.25" org-jetbrains-kotlinx = "1.9.0" org-mockito = "5.14.2" org-opensaml = "4.3.2" org-opensaml5 = "5.1.2" -org-springframework = "6.2.4" +org-springframework = "6.2.6" [libraries] -ch-qos-logback-logback-classic = "ch.qos.logback:logback-classic:1.5.17" +ch-qos-logback-logback-classic = "ch.qos.logback:logback-classic:1.5.18" com-fasterxml-jackson-jackson-bom = "com.fasterxml.jackson:jackson-bom:2.18.3" com-google-inject-guice = "com.google.inject:guice:3.0" com-netflix-nebula-nebula-project-plugin = "com.netflix.nebula:nebula-project-plugin:8.2.0" @@ -28,15 +28,15 @@ com-squareup-okhttp3-okhttp = { module = "com.squareup.okhttp3:okhttp", version. com-unboundid-unboundid-ldapsdk = "com.unboundid:unboundid-ldapsdk:6.0.11" com-unboundid-unboundid-ldapsdk7 = "com.unboundid:unboundid-ldapsdk:7.0.1" commons-collections = "commons-collections:commons-collections:3.2.2" -io-micrometer-micrometer-observation = "io.micrometer:micrometer-observation:1.14.5" +io-micrometer-micrometer-observation = "io.micrometer:micrometer-observation:1.14.6" io-mockk = "io.mockk:mockk:1.13.17" -io-projectreactor-reactor-bom = "io.projectreactor:reactor-bom:2023.0.16" +io-projectreactor-reactor-bom = "io.projectreactor:reactor-bom:2023.0.17" io-rsocket-rsocket-bom = { module = "io.rsocket:rsocket-bom", version.ref = "io-rsocket" } io-spring-javaformat-spring-javaformat-checkstyle = { module = "io.spring.javaformat:spring-javaformat-checkstyle", version.ref = "io-spring-javaformat" } io-spring-javaformat-spring-javaformat-gradle-plugin = { module = "io.spring.javaformat:spring-javaformat-gradle-plugin", version.ref = "io-spring-javaformat" } io-spring-nohttp-nohttp-checkstyle = { module = "io.spring.nohttp:nohttp-checkstyle", version.ref = "io-spring-nohttp" } io-spring-nohttp-nohttp-gradle = { module = "io.spring.nohttp:nohttp-gradle", version.ref = "io-spring-nohttp" } -io-spring-security-release-plugin = "io.spring.gradle:spring-security-release-plugin:1.0.3" +io-spring-security-release-plugin = "io.spring.gradle:spring-security-release-plugin:1.0.4" jakarta-annotation-jakarta-annotation-api = "jakarta.annotation:jakarta.annotation-api:2.1.1" jakarta-inject-jakarta-inject-api = "jakarta.inject:jakarta.inject-api:2.0.1" jakarta-persistence-jakarta-persistence-api = "jakarta.persistence:jakarta.persistence-api:3.1.0" @@ -70,7 +70,7 @@ org-bouncycastle-bcprov-jdk15on = { module = "org.bouncycastle:bcprov-jdk18on", org-eclipse-jetty-jetty-server = { module = "org.eclipse.jetty:jetty-server", version.ref = "org-eclipse-jetty" } org-eclipse-jetty-jetty-servlet = { module = "org.eclipse.jetty:jetty-servlet", version.ref = "org-eclipse-jetty" } org-hamcrest = "org.hamcrest:hamcrest:2.2" -org-hibernate-orm-hibernate-core = "org.hibernate.orm:hibernate-core:6.6.11.Final" +org-hibernate-orm-hibernate-core = "org.hibernate.orm:hibernate-core:6.6.13.Final" org-hsqldb = "org.hsqldb:hsqldb:2.7.4" org-jetbrains-kotlin-kotlin-bom = { module = "org.jetbrains.kotlin:kotlin-bom", version.ref = "org-jetbrains-kotlin" } org-jetbrains-kotlin-kotlin-gradle-plugin = "org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.25" @@ -89,7 +89,7 @@ org-skyscreamer-jsonassert = "org.skyscreamer:jsonassert:1.5.3" org-slf4j-log4j-over-slf4j = "org.slf4j:log4j-over-slf4j:1.7.36" org-slf4j-slf4j-api = "org.slf4j:slf4j-api:2.0.17" org-springframework-data-spring-data-bom = "org.springframework.data:spring-data-bom:2024.1.4" -org-springframework-ldap-spring-ldap-core = "org.springframework.ldap:spring-ldap-core:3.2.11" +org-springframework-ldap-spring-ldap-core = "org.springframework.ldap:spring-ldap-core:3.2.12" org-springframework-spring-framework-bom = { module = "org.springframework:spring-framework-bom", version.ref = "org-springframework" } org-synchronoss-cloud-nio-multipart-parser = "org.synchronoss.cloud:nio-multipart-parser:1.1.0" diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index a4b76b9530d..9bbc975c742 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index d71047787f8..36e4933e1da 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,7 +1,7 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionSha256Sum=8d97a97984f6cbd2b85fe4c60a743440a347544bf18818048e611f5288d46c94 -distributionUrl=https\://services.gradle.org/distributions/gradle-8.12.1-bin.zip +distributionSha256Sum=20f1b1176237254a6fc204d8434196fa11a4cfb387567519c61556e8710aed78 +distributionUrl=https\://services.gradle.org/distributions/gradle-8.13-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/gradlew b/gradlew index f5feea6d6b1..faf93008b77 100755 --- a/gradlew +++ b/gradlew @@ -86,8 +86,7 @@ done # shellcheck disable=SC2034 APP_BASE_NAME=${0##*/} # Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036) -APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s -' "$PWD" ) || exit +APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit # Use the maximum available, or set MAX_FD != -1 to use that value. MAX_FD=maximum @@ -206,7 +205,7 @@ fi DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' # Collect all arguments for the java command: -# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments, +# * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments, # and any embedded shellness will be escaped. # * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be # treated as '${Hostname}' itself on the command line. diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServerOAuth2AuthorizedClientExchangeFilterFunction.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServerOAuth2AuthorizedClientExchangeFilterFunction.java index 0736c4ac250..ca20a6d7cfa 100644 --- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServerOAuth2AuthorizedClientExchangeFilterFunction.java +++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/reactive/function/client/ServerOAuth2AuthorizedClientExchangeFilterFunction.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -245,7 +245,7 @@ private static ReactiveOAuth2AuthorizedClientManager createDefaultAuthorizedClie * be used to create an Authentication for saving. * * @param authorizedClient the {@link OAuth2AuthorizedClient} to use. - * @return the {@link Consumer} to populate the + * @return the {@link Consumer} to populate the attributes */ public static Consumer> oauth2AuthorizedClient(OAuth2AuthorizedClient authorizedClient) { return (attributes) -> attributes.put(OAUTH2_AUTHORIZED_CLIENT_ATTR_NAME, authorizedClient); diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java index d238e870178..f00305843a1 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolver.java @@ -52,26 +52,77 @@ public final class DefaultBearerTokenResolver implements BearerTokenResolver { @Override public String resolve(final HttpServletRequest request) { - final String authorizationHeaderToken = resolveFromAuthorizationHeader(request); - final String parameterToken = isParameterTokenSupportedForRequest(request) - ? resolveFromRequestParameters(request) : null; - if (authorizationHeaderToken != null) { - if (parameterToken != null) { + // @formatter:off + return resolveToken( + resolveFromAuthorizationHeader(request), + resolveAccessTokenFromQueryString(request), + resolveAccessTokenFromBody(request) + ); + // @formatter:on + } + + private static String resolveToken(String... accessTokens) { + if (accessTokens == null || accessTokens.length == 0) { + return null; + } + + String accessToken = null; + for (String token : accessTokens) { + if (accessToken == null) { + accessToken = token; + } + else if (token != null) { BearerTokenError error = BearerTokenErrors .invalidRequest("Found multiple bearer tokens in the request"); throw new OAuth2AuthenticationException(error); } - return authorizationHeaderToken; } - if (parameterToken != null && isParameterTokenEnabledForRequest(request)) { - if (!StringUtils.hasText(parameterToken)) { - BearerTokenError error = BearerTokenErrors - .invalidRequest("The requested token parameter is an empty string"); - throw new OAuth2AuthenticationException(error); - } - return parameterToken; + + if (accessToken != null && accessToken.isBlank()) { + BearerTokenError error = BearerTokenErrors + .invalidRequest("The requested token parameter is an empty string"); + throw new OAuth2AuthenticationException(error); + } + + return accessToken; + } + + private String resolveFromAuthorizationHeader(HttpServletRequest request) { + String authorization = request.getHeader(this.bearerTokenHeaderName); + if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) { + return null; + } + + Matcher matcher = authorizationPattern.matcher(authorization); + if (!matcher.matches()) { + BearerTokenError error = BearerTokenErrors.invalidToken("Bearer token is malformed"); + throw new OAuth2AuthenticationException(error); + } + + return matcher.group("token"); + } + + private String resolveAccessTokenFromQueryString(HttpServletRequest request) { + if (!this.allowUriQueryParameter || !HttpMethod.GET.name().equals(request.getMethod())) { + return null; + } + + return resolveToken(request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME)); + } + + private String resolveAccessTokenFromBody(HttpServletRequest request) { + if (!this.allowFormEncodedBodyParameter + || !MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()) + || HttpMethod.GET.name().equals(request.getMethod())) { + return null; + } + + String queryString = request.getQueryString(); + if (queryString != null && queryString.contains(ACCESS_TOKEN_PARAMETER_NAME)) { + return null; } - return null; + + return resolveToken(request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME)); } /** @@ -109,50 +160,4 @@ public void setBearerTokenHeaderName(String bearerTokenHeaderName) { this.bearerTokenHeaderName = bearerTokenHeaderName; } - private String resolveFromAuthorizationHeader(HttpServletRequest request) { - String authorization = request.getHeader(this.bearerTokenHeaderName); - if (!StringUtils.startsWithIgnoreCase(authorization, "bearer")) { - return null; - } - Matcher matcher = authorizationPattern.matcher(authorization); - if (!matcher.matches()) { - BearerTokenError error = BearerTokenErrors.invalidToken("Bearer token is malformed"); - throw new OAuth2AuthenticationException(error); - } - return matcher.group("token"); - } - - private static String resolveFromRequestParameters(HttpServletRequest request) { - String[] values = request.getParameterValues(ACCESS_TOKEN_PARAMETER_NAME); - if (values == null || values.length == 0) { - return null; - } - if (values.length == 1) { - return values[0]; - } - BearerTokenError error = BearerTokenErrors.invalidRequest("Found multiple bearer tokens in the request"); - throw new OAuth2AuthenticationException(error); - } - - private boolean isParameterTokenSupportedForRequest(final HttpServletRequest request) { - return isFormEncodedRequest(request) || isGetRequest(request); - } - - private static boolean isGetRequest(HttpServletRequest request) { - return HttpMethod.GET.name().equals(request.getMethod()); - } - - private static boolean isFormEncodedRequest(HttpServletRequest request) { - return MediaType.APPLICATION_FORM_URLENCODED_VALUE.equals(request.getContentType()); - } - - private static boolean hasAccessTokenInQueryString(HttpServletRequest request) { - return (request.getQueryString() != null) && request.getQueryString().contains(ACCESS_TOKEN_PARAMETER_NAME); - } - - private boolean isParameterTokenEnabledForRequest(HttpServletRequest request) { - return ((this.allowFormEncodedBodyParameter && isFormEncodedRequest(request) && !isGetRequest(request) - && !hasAccessTokenInQueryString(request)) || (this.allowUriQueryParameter && isGetRequest(request))); - } - } diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java index bd07c59b747..c3c7699f8e7 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -77,18 +77,18 @@ private String token(ServerHttpRequest request) { } return authorizationHeaderToken; } - if (parameterToken != null && isParameterTokenSupportedForRequest(request)) { - if (!StringUtils.hasText(parameterToken)) { - BearerTokenError error = BearerTokenErrors - .invalidRequest("The requested token parameter is an empty string"); - throw new OAuth2AuthenticationException(error); - } - return parameterToken; + if (parameterToken != null && !StringUtils.hasText(parameterToken)) { + BearerTokenError error = BearerTokenErrors + .invalidRequest("The requested token parameter is an empty string"); + throw new OAuth2AuthenticationException(error); } - return null; + return parameterToken; } - private static String resolveAccessTokenFromRequest(ServerHttpRequest request) { + private String resolveAccessTokenFromRequest(ServerHttpRequest request) { + if (!isParameterTokenSupportedForRequest(request)) { + return null; + } List parameterTokens = request.getQueryParams().get("access_token"); if (CollectionUtils.isEmpty(parameterTokens)) { return null; diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java index f04fb69a3df..db8c6292bd7 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/DefaultBearerTokenResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -110,6 +110,7 @@ public void resolveWhenHeaderWithInvalidCharactersIsPresentThenAuthenticationExc @Test public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowFormEncodedBodyParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.setMethod("POST"); @@ -121,6 +122,7 @@ public void resolveWhenValidHeaderIsPresentTogetherWithFormParameterThenAuthenti @Test public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowUriQueryParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.addHeader("Authorization", "Bearer " + TEST_TOKEN); request.setMethod("GET"); @@ -133,6 +135,7 @@ public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthent // gh-10326 @Test public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowUriQueryParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); request.addParameter("access_token", "token1", "token2"); @@ -143,6 +146,7 @@ public void resolveWhenRequestContainsTwoAccessTokenQueryParametersThenAuthentic // gh-10326 @Test public void resolveWhenRequestContainsTwoAccessTokenFormParametersThenAuthenticationExceptionIsThrown() { + this.resolver.setAllowFormEncodedBodyParameter(true); MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("POST"); request.setContentType("application/x-www-form-urlencoded"); @@ -233,6 +237,19 @@ public void resolveWhenPostAndFormParameterIsSupportedAndQueryParameterIsPresent assertThat(this.resolver.resolve(request)).isNull(); } + @Test + public void resolveWhenPostAndQueryParameterIsSupportedAndFormParameterIsPresentThenTokenIsNotResolved() { + this.resolver.setAllowUriQueryParameter(true); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContentType("application/x-www-form-urlencoded"); + request.setQueryString("access_token=" + TEST_TOKEN); + request.addParameter("access_token", TEST_TOKEN); + + assertThat(this.resolver.resolve(request)).isNull(); + } + @Test public void resolveWhenFormParameterIsPresentAndNotSupportedThenTokenIsNotResolved() { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -261,6 +278,25 @@ public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResol assertThat(this.resolver.resolve(request)).isNull(); } + // gh-16038 + @Test + public void resolveWhenRequestContainsTwoAccessTokenFormParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setContentType("application/x-www-form-urlencoded"); + request.addParameter("access_token", "token1", "token2"); + assertThat(this.resolver.resolve(request)).isNull(); + } + + // gh-16038 + @Test + public void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + request.addParameter("access_token", "token1", "token2"); + assertThat(this.resolver.resolve(request)).isNull(); + } + @Test public void resolveWhenQueryParameterIsPresentAndEmptyStringThenTokenIsNotResolved() { this.resolver.setAllowUriQueryParameter(true); diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java index 1f4b17697fd..32349f121d9 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/server/authentication/ServerBearerTokenAuthenticationConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -157,6 +157,7 @@ public void resolveWhenHeaderWithInvalidCharactersIsPresentAndNotSubscribedThenN @Test public void resolveWhenValidHeaderIsPresentTogetherWithQueryParameterThenAuthenticationExceptionIsThrown() { // @formatter:off + this.converter.setAllowUriQueryParameter(true); MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") .queryParam("access_token", TEST_TOKEN) .header(HttpHeaders.AUTHORIZATION, "Bearer " + TEST_TOKEN); @@ -205,6 +206,7 @@ public void resolveWhenQueryParameterIsPresentAndNotSupportedThenTokenIsNotResol @Test void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationException() { + this.converter.setAllowUriQueryParameter(true); MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") .queryParam("access_token", TEST_TOKEN, TEST_TOKEN); assertThatExceptionOfType(OAuth2AuthenticationException.class).isThrownBy(() -> convertToToken(request)) @@ -217,6 +219,14 @@ void resolveWhenQueryParameterHasMultipleAccessTokensThenOAuth2AuthenticationExc } + // gh-16038 + @Test + void resolveWhenRequestContainsTwoAccessTokenQueryParametersAndSupportIsDisabledThenTokenIsNotResolved() { + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/") + .queryParam("access_token", TEST_TOKEN, TEST_TOKEN); + assertThat(convertToToken(request)).isNull(); + } + private BearerTokenAuthenticationToken convertToToken(MockServerHttpRequest.BaseBuilder request) { return convertToToken(request.build()); } diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/internal/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/internal/OpenSaml4Template.java index 002c0862e1f..c8df5a36494 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/internal/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/internal/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4Template.java index f529b8b4a06..4892b7593ab 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4Template.java index 5344d080dc5..f177502cc4b 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml4Template.java index 8cd40194fe7..dd1197f95c7 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4Template.java index a56bcf81000..bb6201b4230 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml4Template.java index b2ca1e11114..db7810cf06b 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml4Template.java index 9ca12533791..62949ae27a8 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4Template.java b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4Template.java index eee4fc8242f..399072fc2cb 100644 --- a/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4Template.java +++ b/saml2/saml2-service-provider/src/opensaml4Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml4Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java index ef459854198..cdccc13c137 100644 --- a/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java +++ b/saml2/saml2-service-provider/src/opensaml4Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -386,6 +386,24 @@ public void authenticateWhenEncryptedAssertionWithSignatureThenItSucceeds() { this.provider.authenticate(token); } + // gh-16367 + @Test + public void authenticateWhenEncryptedAssertionWithSignatureThenEncryptedAssertionStillAvailable() { + Response response = response(); + Assertion assertion = TestOpenSamlObjects.signed(assertion(), + TestSaml2X509Credentials.assertingPartySigningCredential(), RELYING_PARTY_ENTITY_ID); + EncryptedAssertion encryptedAssertion = TestOpenSamlObjects.encrypted(assertion, + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + response.getEncryptedAssertions().add(encryptedAssertion); + Saml2AuthenticationToken token = token(signed(response), decrypting(verifying(registration()))); + OpenSaml4AuthenticationProvider provider = new OpenSaml4AuthenticationProvider(); + provider.setResponseValidator((t) -> { + assertThat(t.getResponse().getEncryptedAssertions()).isNotEmpty(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenEncryptedAssertionWithResponseSignatureThenItSucceeds() { Response response = response(); @@ -410,6 +428,26 @@ public void authenticateWhenEncryptedNameIdWithSignatureThenItSucceeds() { this.provider.authenticate(token); } + // gh-16367 + @Test + public void authenticateWhenEncryptedNameIdWithSignatureThenEncryptedNameIdStillAvailable() { + Response response = response(); + Assertion assertion = assertion(); + NameID nameId = assertion.getSubject().getNameID(); + EncryptedID encryptedID = TestOpenSamlObjects.encrypted(nameId, + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + assertion.getSubject().setNameID(null); + assertion.getSubject().setEncryptedID(encryptedID); + response.getAssertions().add(signed(assertion)); + Saml2AuthenticationToken token = token(response, decrypting(verifying(registration()))); + OpenSaml4AuthenticationProvider provider = new OpenSaml4AuthenticationProvider(); + provider.setAssertionValidator((t) -> { + assertThat(t.getAssertion().getSubject().getEncryptedID()).isNotNull(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenEncryptedAttributeThenDecrypts() { Response response = response(); @@ -426,6 +464,26 @@ public void authenticateWhenEncryptedAttributeThenDecrypts() { assertThat(principal.getAttribute("name")).containsExactly("value"); } + // gh-16367 + @Test + public void authenticateWhenEncryptedAttributeThenEncryptedAttributesStillAvailable() { + Response response = response(); + Assertion assertion = assertion(); + EncryptedAttribute attribute = TestOpenSamlObjects.encrypted("name", "value", + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + AttributeStatement statement = build(AttributeStatement.DEFAULT_ELEMENT_NAME); + statement.getEncryptedAttributes().add(attribute); + assertion.getAttributeStatements().add(statement); + response.getAssertions().add(assertion); + Saml2AuthenticationToken token = token(signed(response), decrypting(verifying(registration()))); + OpenSaml4AuthenticationProvider provider = new OpenSaml4AuthenticationProvider(); + provider.setAssertionValidator((t) -> { + assertThat(t.getAssertion().getAttributeStatements().get(0).getEncryptedAttributes()).isNotEmpty(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenDecryptionKeysAreMissingThenThrowAuthenticationException() { Response response = response(); diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/internal/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/internal/OpenSaml5Template.java index ef74e61c970..fdeffc915ad 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/internal/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/internal/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5Template.java index b2003af461a..c5b6ff98ae4 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5Template.java index d0add4c54c6..576bee21dcd 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/authentication/logout/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml5Template.java index e5a39122a35..99b18df8aea 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/metadata/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5Template.java index 50f60e5a0d4..399fedf5774 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/registration/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml5Template.java index 73b3d9f391a..62da91197a2 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml5Template.java index db40f084ee0..305dd60440b 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml5Template.java b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml5Template.java index 2d095694f5d..c7abef6236d 100644 --- a/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml5Template.java +++ b/saml2/saml2-service-provider/src/opensaml5Main/java/org/springframework/security/saml2/provider/service/web/authentication/logout/OpenSaml5Template.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -482,7 +482,6 @@ public void decrypt(XMLObject object) { private void decryptResponse(Response response) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); int count = 0; int size = response.getEncryptedAssertions().size(); @@ -492,7 +491,6 @@ private void decryptResponse(Response response) { try { Assertion decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } count++; @@ -502,7 +500,6 @@ private void decryptResponse(Response response) { } } - response.getEncryptedAssertions().removeAll(encrypteds); response.getAssertions().addAll(decrypteds); // Re-marshall the response so that any ID attributes within the decrypted @@ -534,7 +531,6 @@ private void decryptAssertion(Assertion assertion) { NameID decrypted = (NameID) this.decrypter.decrypt(d.getEncryptedID()); if (decrypted != null) { d.setNameID(decrypted); - d.setEncryptedID(null); } } catch (DecryptionException ex) { @@ -548,12 +544,10 @@ private void decryptAssertion(Assertion assertion) { private void decryptAttributes(AttributeStatement statement) { Collection decrypteds = new ArrayList<>(); - Collection encrypteds = new ArrayList<>(); for (EncryptedAttribute encrypted : statement.getEncryptedAttributes()) { try { Attribute decrypted = this.decrypter.decrypt(encrypted); if (decrypted != null) { - encrypteds.add(encrypted); decrypteds.add(decrypted); } } @@ -561,7 +555,6 @@ private void decryptAttributes(AttributeStatement statement) { throw new Saml2Exception(ex); } } - statement.getEncryptedAttributes().removeAll(encrypteds); statement.getAttributes().addAll(decrypteds); } @@ -572,7 +565,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(subject.getEncryptedID()); if (decrypted != null) { subject.setNameID(decrypted); - subject.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -586,7 +578,6 @@ private void decryptSubject(Subject subject) { NameID decrypted = (NameID) this.decrypter.decrypt(sc.getEncryptedID()); if (decrypted != null) { sc.setNameID(decrypted); - sc.setEncryptedID(null); } } catch (final DecryptionException ex) { @@ -603,7 +594,6 @@ private void decryptLogoutRequest(LogoutRequest request) { NameID decrypted = (NameID) this.decrypter.decrypt(request.getEncryptedID()); if (decrypted != null) { request.setNameID(decrypted); - request.setEncryptedID(null); } } catch (DecryptionException ex) { diff --git a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java index 22ed0e89b6a..284c7a90c36 100644 --- a/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java +++ b/saml2/saml2-service-provider/src/opensaml5Test/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml5AuthenticationProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -386,6 +386,24 @@ public void authenticateWhenEncryptedAssertionWithSignatureThenItSucceeds() { this.provider.authenticate(token); } + // gh-16367 + @Test + public void authenticateWhenEncryptedAssertionWithSignatureThenEncryptedAssertionStillAvailable() { + Response response = response(); + Assertion assertion = TestOpenSamlObjects.signed(assertion(), + TestSaml2X509Credentials.assertingPartySigningCredential(), RELYING_PARTY_ENTITY_ID); + EncryptedAssertion encryptedAssertion = TestOpenSamlObjects.encrypted(assertion, + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + response.getEncryptedAssertions().add(encryptedAssertion); + Saml2AuthenticationToken token = token(signed(response), decrypting(verifying(registration()))); + OpenSaml5AuthenticationProvider provider = new OpenSaml5AuthenticationProvider(); + provider.setResponseValidator((t) -> { + assertThat(t.getResponse().getEncryptedAssertions()).isNotEmpty(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenEncryptedAssertionWithResponseSignatureThenItSucceeds() { Response response = response(); @@ -410,6 +428,26 @@ public void authenticateWhenEncryptedNameIdWithSignatureThenItSucceeds() { this.provider.authenticate(token); } + // gh-16367 + @Test + public void authenticateWhenEncryptedNameIdWithSignatureThenEncryptedNameIdStillAvailable() { + Response response = response(); + Assertion assertion = assertion(); + NameID nameId = assertion.getSubject().getNameID(); + EncryptedID encryptedID = TestOpenSamlObjects.encrypted(nameId, + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + assertion.getSubject().setNameID(null); + assertion.getSubject().setEncryptedID(encryptedID); + response.getAssertions().add(signed(assertion)); + Saml2AuthenticationToken token = token(response, decrypting(verifying(registration()))); + OpenSaml5AuthenticationProvider provider = new OpenSaml5AuthenticationProvider(); + provider.setAssertionValidator((t) -> { + assertThat(t.getAssertion().getSubject().getEncryptedID()).isNotNull(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenEncryptedAttributeThenDecrypts() { Response response = response(); @@ -426,6 +464,26 @@ public void authenticateWhenEncryptedAttributeThenDecrypts() { assertThat(principal.getAttribute("name")).containsExactly("value"); } + // gh-16367 + @Test + public void authenticateWhenEncryptedAttributeThenEncryptedAttributesStillAvailable() { + Response response = response(); + Assertion assertion = assertion(); + EncryptedAttribute attribute = TestOpenSamlObjects.encrypted("name", "value", + TestSaml2X509Credentials.assertingPartyEncryptingCredential()); + AttributeStatement statement = build(AttributeStatement.DEFAULT_ELEMENT_NAME); + statement.getEncryptedAttributes().add(attribute); + assertion.getAttributeStatements().add(statement); + response.getAssertions().add(assertion); + Saml2AuthenticationToken token = token(signed(response), decrypting(verifying(registration()))); + OpenSaml5AuthenticationProvider provider = new OpenSaml5AuthenticationProvider(); + provider.setAssertionValidator((t) -> { + assertThat(t.getAssertion().getAttributeStatements().get(0).getEncryptedAttributes()).isNotEmpty(); + return Saml2ResponseValidatorResult.success(); + }); + provider.authenticate(token); + } + @Test public void authenticateWhenDecryptionKeysAreMissingThenThrowAuthenticationException() { Response response = response(); diff --git a/web/src/main/java/org/springframework/security/web/access/WebInvocationPrivilegeEvaluator.java b/web/src/main/java/org/springframework/security/web/access/WebInvocationPrivilegeEvaluator.java index 521346ef2ff..131cb7d147e 100644 --- a/web/src/main/java/org/springframework/security/web/access/WebInvocationPrivilegeEvaluator.java +++ b/web/src/main/java/org/springframework/security/web/access/WebInvocationPrivilegeEvaluator.java @@ -29,6 +29,9 @@ public interface WebInvocationPrivilegeEvaluator { /** * Determines whether the user represented by the supplied Authentication * object is allowed to invoke the supplied URI. + *

+ * Note this will only match authorization rules that don't require a certain + * {@code HttpMethod}. * @param uri the URI excluding the context path (a default context path setting will * be used) */ @@ -36,13 +39,18 @@ public interface WebInvocationPrivilegeEvaluator { /** * Determines whether the user represented by the supplied Authentication - * object is allowed to invoke the supplied URI, with the given . + * object is allowed to invoke the supplied URI, with the given parameters. *

- * Note the default implementation of FilterInvocationSecurityMetadataSource + * Note: + *

    + *
  • The default implementation of FilterInvocationSecurityMetadataSource * disregards the contextPath when evaluating which secure object * metadata applies to a given request URI, so generally the contextPath * is unimportant unless you are using a custom - * FilterInvocationSecurityMetadataSource. + * FilterInvocationSecurityMetadataSource.
    • + *
  • this will only match authorization rules that don't require a certain + * {@code HttpMethod}.
    • + *
* @param uri the URI excluding the context path * @param contextPath the context path (may be null). * @param method the HTTP method (or null, for any method) diff --git a/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java index 4aafd032223..ce39dc63e10 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/ui/DefaultLoginPageGeneratingFilter.java @@ -452,7 +452,7 @@ private boolean matches(HttpServletRequest request, String url) { """; private static final String LOGIN_PAGE_TEMPLATE = """ diff --git a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java index 40301e5de7f..37659f81332 100644 --- a/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -84,7 +84,7 @@ public void setCookieCustomizer(Consumer c */ public static CookieServerCsrfTokenRepository withHttpOnlyFalse() { CookieServerCsrfTokenRepository result = new CookieServerCsrfTokenRepository(); - result.setCookieCustomizer((cookie) -> cookie.httpOnly(false)); + result.cookieHttpOnly = false; return result; } diff --git a/web/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java b/web/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java index 4dc7efc5a8d..59d07292880 100644 --- a/web/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java +++ b/web/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -333,9 +333,7 @@ private static Set convertTransports( public PublicKeyCredentialRequestOptions createCredentialRequestOptions( PublicKeyCredentialRequestOptionsRequest request) { Authentication authentication = request.getAuthentication(); - // FIXME: do not load credentialRecords if anonymous - PublicKeyCredentialUserEntity userEntity = findUserEntityOrCreateAndSave(authentication.getName()); - List credentialRecords = this.userCredentials.findByUserId(userEntity.getId()); + List credentialRecords = findCredentialRecords(authentication); return PublicKeyCredentialRequestOptions.builder() .allowCredentials(credentialDescriptors(credentialRecords)) .challenge(Bytes.random()) @@ -346,6 +344,17 @@ public PublicKeyCredentialRequestOptions createCredentialRequestOptions( .build(); } + private List findCredentialRecords(Authentication authentication) { + if (!this.trustResolver.isAuthenticated(authentication)) { + return Collections.emptyList(); + } + PublicKeyCredentialUserEntity userEntity = this.userEntities.findByUsername(authentication.getName()); + if (userEntity == null) { + return Collections.emptyList(); + } + return this.userCredentials.findByUserId(userEntity.getId()); + } + @Override public PublicKeyCredentialUserEntity authenticate(RelyingPartyAuthenticationRequest request) { PublicKeyCredentialRequestOptions requestOptions = request.getRequestOptions(); diff --git a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java index 1aa89f21a8c..a6c290fd886 100644 --- a/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/server/csrf/CookieServerCsrfTokenRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -290,6 +290,21 @@ void loadTokenWhenCookieExistsWithNullValue() { loadAndAssertExpectedValues(); } + // gh-16820 + @Test + void withHttpOnlyFalseWhenCookieCustomizerThenStillDefaultsToFalse() { + CookieServerCsrfTokenRepository repository = CookieServerCsrfTokenRepository.withHttpOnlyFalse(); + repository.setCookieCustomizer((customizer) -> customizer.maxAge(1000)); + MockServerHttpRequest.BaseBuilder request = MockServerHttpRequest.get("/dummy"); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + CsrfToken csrfToken = repository.generateToken(exchange).block(); + repository.saveToken(exchange, csrfToken).block(); + ResponseCookie cookie = exchange.getResponse().getCookies().getFirst("XSRF-TOKEN"); + assertThat(cookie).isNotNull(); + assertThat(cookie.getMaxAge().getSeconds()).isEqualTo(1000); + assertThat(cookie.isHttpOnly()).isEqualTo(Boolean.FALSE); + } + private void setExpectedHeaderName(String expectedHeaderName) { this.csrfTokenRepository.setHeaderName(expectedHeaderName); this.expectedHeaderName = expectedHeaderName; diff --git a/web/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java b/web/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java index 4061746e884..733d7ec98b5 100644 --- a/web/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java +++ b/web/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,6 +42,8 @@ import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.userdetails.PasswordEncodedUser; +import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.web.webauthn.api.AuthenticatorAttestationResponse; import org.springframework.security.web.webauthn.api.AuthenticatorAttestationResponse.AuthenticatorAttestationResponseBuilder; import org.springframework.security.web.webauthn.api.AuthenticatorSelectionCriteria; @@ -66,6 +68,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verifyNoInteractions; @ExtendWith(MockitoExtension.class) class Webauthn4jRelyingPartyOperationsTests { @@ -536,6 +539,50 @@ void createCredentialRequestOptionsThenUserVerificationSameAsCreation() { .isEqualTo(creationOptions.getAuthenticatorSelection().getUserVerification()); } + @Test + void createCredentialRequestOptionsWhenAnonymousAuthentication() { + AnonymousAuthenticationToken authentication = new AnonymousAuthenticationToken("key", "anonymousUser", + Set.of(() -> "ROLE_ANONYMOUS")); + PublicKeyCredentialRequestOptionsRequest createRequest = new ImmutablePublicKeyCredentialRequestOptionsRequest( + authentication); + PublicKeyCredentialRequestOptions credentialRequestOptions = this.rpOperations + .createCredentialRequestOptions(createRequest); + + assertThat(credentialRequestOptions.getAllowCredentials()).isEmpty(); + // verify anonymous user not saved + verifyNoInteractions(this.userEntities); + } + + @Test + void createCredentialRequestOptionsWhenNullAuthentication() { + PublicKeyCredentialRequestOptionsRequest createRequest = new ImmutablePublicKeyCredentialRequestOptionsRequest( + null); + PublicKeyCredentialRequestOptions credentialRequestOptions = this.rpOperations + .createCredentialRequestOptions(createRequest); + + assertThat(credentialRequestOptions.getAllowCredentials()).isEmpty(); + // verify anonymous user not saved + verifyNoInteractions(this.userEntities); + } + + @Test + void createCredentialRequestOptionsWhenAuthenticated() { + UserDetails user = PasswordEncodedUser.user(); + UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken(user, null, + user.getAuthorities()); + PublicKeyCredentialUserEntity userEntity = TestPublicKeyCredentialUserEntity.userEntity().build(); + CredentialRecord credentialRecord = TestCredentialRecord.userCredential().build(); + given(this.userEntities.findByUsername(user.getUsername())).willReturn(userEntity); + given(this.userCredentials.findByUserId(userEntity.getId())).willReturn(Arrays.asList(credentialRecord)); + PublicKeyCredentialRequestOptionsRequest createRequest = new ImmutablePublicKeyCredentialRequestOptionsRequest( + auth); + PublicKeyCredentialRequestOptions credentialRequestOptions = this.rpOperations + .createCredentialRequestOptions(createRequest); + + assertThat(credentialRequestOptions.getAllowCredentials()).extracting(PublicKeyCredentialDescriptor::getId) + .containsExactly(credentialRecord.getCredentialId()); + } + private static AuthenticatorAttestationResponse setFlag(byte... flags) throws Exception { AuthenticatorAttestationResponseBuilder authAttResponseBldr = TestAuthenticatorAttestationResponse .createAuthenticatorAttestationResponse();