Skip to content

Build modules using Java 8 #10817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 2, 2022
Merged

Build modules using Java 8 #10817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71a6b74
Revert "Make source code compatible with JDK 8"
marcusdacoregio Feb 2, 2022
9a72461
Fix member variable using Java 9+ feature
marcusdacoregio Feb 2, 2022
63dc3de
Use Java 8 Toolchain for modules
marcusdacoregio Feb 2, 2022
61794ba
Use Java 11 Toolchain for OpenSaml4 compile
marcusdacoregio Feb 2, 2022
85d9359
Use Reflection to instantiate OpenSAML4 classes
marcusdacoregio Feb 3, 2022
4fbf9dd
Run SAML 2.0 tests in an exclusive task
marcusdacoregio Feb 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ subprojects {
tasks.withType(JavaCompile) {
options.encoding = "UTF-8"
options.compilerArgs.add("-parameters")
options.release = 8
}
}

Expand Down Expand Up @@ -139,6 +138,13 @@ allprojects {
}
}
}

tasks.withType(JavaCompile).configureEach {
javaCompiler = javaToolchains.compilerFor {
languageVersion = JavaLanguageVersion.of(8)
}
}

}

if (hasProperty('buildScan')) {
Expand Down
31 changes: 30 additions & 1 deletion config/spring-security-config.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ dependencies {
testRuntimeOnly 'org.hsqldb:hsqldb'
}


rncToXsd {
rncDir = file('src/main/resources/org/springframework/security/config/')
xsdDir = rncDir
Expand All @@ -130,3 +129,33 @@ tasks.withType(KotlinCompile).configureEach {
}

build.dependsOn rncToXsd

compileTestJava {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it true that we don't want to run these tests against JDK 8? It seems to me that we want to run the tests twice: Once for JDK 8 and once for JDK 11.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to run they using JDK 11 only because they are using OpenSAML4 classes

exclude "org/springframework/security/config/annotation/web/configurers/saml2/**", "org/springframework/security/config/http/Saml2*"
}

task compileSaml2TestJava(type: JavaCompile) {
javaCompiler = javaToolchains.compilerFor {
languageVersion = JavaLanguageVersion.of(11)
}
source = sourceSets.test.java.srcDirs
include "org/springframework/security/config/annotation/web/configurers/saml2/**", "org/springframework/security/config/http/Saml2*"
classpath = sourceSets.test.compileClasspath
destinationDirectory = new File("${buildDir}/classes/java/test")
options.sourcepath = sourceSets.test.java.getSourceDirectories()
}

task saml2Tests(type: Test) {
javaLauncher = javaToolchains.launcherFor {
languageVersion = JavaLanguageVersion.of(11)
}
filter {
includeTestsMatching "org.springframework.security.config.annotation.web.configurers.saml2.*"
}
useJUnitPlatform()
dependsOn compileSaml2TestJava
}

tasks.named('test') {
finalizedBy 'saml2Tests'
}
63 changes: 48 additions & 15 deletions ...pringframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer;
import org.springframework.security.core.Authentication;
import org.springframework.security.saml2.provider.service.authentication.AbstractSaml2AuthenticationRequest;
import org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider;
import org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationRequestFactory;
import org.springframework.security.saml2.provider.service.authentication.OpenSamlAuthenticationProvider;
import org.springframework.security.saml2.provider.service.authentication.OpenSamlAuthenticationRequestFactory;
import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationRequestFactory;
Expand All @@ -55,6 +53,7 @@
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -112,6 +111,8 @@
public final class Saml2LoginConfigurer<B extends HttpSecurityBuilder<B>>
extends AbstractAuthenticationFilterConfigurer<B, Saml2LoginConfigurer<B>, Saml2WebSsoAuthenticationFilter> {

private static final String OPEN_SAML_4_VERSION = "4";

private String loginPage;

private String authenticationRequestUri = "/saml2/authenticate/{registrationId}";
Expand Down Expand Up @@ -320,11 +321,9 @@ private Saml2AuthenticationRequestFactory getAuthenticationRequestFactory(B http
return resolver;
}
if (version().startsWith("4")) {
return new OpenSaml4AuthenticationRequestFactory();
}
else {
return new OpenSamlAuthenticationRequestFactory();
return OpenSaml4LoginSupportFactory.getAuthenticationRequestFactory();
}
return new OpenSamlAuthenticationRequestFactory();
}

private Saml2AuthenticationRequestContextResolver getAuthenticationRequestContextResolver(B http) {
Expand Down Expand Up @@ -354,17 +353,9 @@ private AuthenticationConverter getAuthenticationConverter(B http) {
return authenticationConverterBean;
}

private String version() {
String version = Version.getVersion();
if (version != null) {
return version;
}
return Version.getVersion();
}

private void registerDefaultAuthenticationProvider(B http) {
if (version().startsWith("4")) {
http.authenticationProvider(postProcess(new OpenSaml4AuthenticationProvider()));
http.authenticationProvider(postProcess(OpenSaml4LoginSupportFactory.getAuthenticationProvider()));
}
else {
http.authenticationProvider(postProcess(new OpenSamlAuthenticationProvider()));
Expand Down Expand Up @@ -414,6 +405,19 @@ private Saml2AuthenticationRequestRepository<AbstractSaml2AuthenticationRequest>
return repository;
}

private String version() {
String version = Version.getVersion();
if (StringUtils.hasText(version)) {
return version;
}
boolean openSaml4ClassPresent = ClassUtils
.isPresent("org.opensaml.core.xml.persist.impl.PassthroughSourceStrategy", null);
if (openSaml4ClassPresent) {
return OPEN_SAML_4_VERSION;
}
throw new IllegalStateException("cannot determine OpenSAML version");
}

private <C> C getSharedOrBean(B http, Class<C> clazz) {
C shared = http.getSharedObject(clazz);
if (shared != null) {
Expand Down Expand Up @@ -441,4 +445,33 @@ private <C> void setSharedObject(B http, Class<C> clazz, C object) {
}
}

private static class OpenSaml4LoginSupportFactory {

private static Saml2AuthenticationRequestFactory getAuthenticationRequestFactory() {
try {
Class<?> authenticationRequestFactory = ClassUtils.forName(
"org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationRequestFactory",
OpenSaml4LoginSupportFactory.class.getClassLoader());
return (Saml2AuthenticationRequestFactory) authenticationRequestFactory.getDeclaredConstructor()
.newInstance();
}
catch (ReflectiveOperationException ex) {
throw new IllegalStateException("Could not instantiate OpenSaml4AuthenticationRequestFactory", ex);
}
}

private static AuthenticationProvider getAuthenticationProvider() {
try {
Class<?> authenticationProvider = ClassUtils.forName(
"org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider",
OpenSaml4LoginSupportFactory.class.getClassLoader());
return (AuthenticationProvider) authenticationProvider.getDeclaredConstructor().newInstance();
}
catch (ReflectiveOperationException ex) {
throw new IllegalStateException("Could not instantiate OpenSaml4AuthenticationProvider", ex);
}
}

}

}
77 changes: 59 additions & 18 deletions ...ringframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 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.
Expand Down Expand Up @@ -47,8 +47,6 @@
import org.springframework.security.saml2.provider.service.web.authentication.logout.HttpSessionLogoutRequestRepository;
import org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml3LogoutRequestResolver;
import org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml3LogoutResponseResolver;
import org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutRequestResolver;
import org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutResponseResolver;
import org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestFilter;
import org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestRepository;
import org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestResolver;
Expand All @@ -67,6 +65,8 @@
import org.springframework.security.web.util.matcher.AndRequestMatcher;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

/**
* Adds SAML 2.0 logout support.
Expand Down Expand Up @@ -113,6 +113,8 @@
public final class Saml2LogoutConfigurer<H extends HttpSecurityBuilder<H>>
extends AbstractHttpConfigurer<Saml2LogoutConfigurer<H>, H> {

private static final String OPEN_SAML_4_VERSION = "4";

private ApplicationContext context;

private RelyingPartyRegistrationRepository relyingPartyRegistrationRepository;
Expand Down Expand Up @@ -304,6 +306,19 @@ private Saml2LogoutResponseResolver createSaml2LogoutResponseResolver(
return this.logoutResponseConfigurer.logoutResponseResolver(relyingPartyRegistrationResolver);
}

private String version() {
String version = Version.getVersion();
if (StringUtils.hasText(version)) {
return version;
}
boolean openSaml4ClassPresent = ClassUtils
.isPresent("org.opensaml.core.xml.persist.impl.PassthroughSourceStrategy", null);
if (openSaml4ClassPresent) {
return OPEN_SAML_4_VERSION;
}
throw new IllegalStateException("cannot determine OpenSAML version");
}

private <C> C getBeanOrNull(Class<C> clazz) {
if (this.context == null) {
return null;
Expand All @@ -314,14 +329,6 @@ private <C> C getBeanOrNull(Class<C> clazz) {
return this.context.getBean(clazz);
}

private String version() {
String version = Version.getVersion();
if (version != null) {
return version;
}
return Version.getVersion();
}

/**
* A configurer for SAML 2.0 LogoutRequest components
*/
Expand Down Expand Up @@ -402,7 +409,7 @@ private Saml2LogoutRequestResolver logoutRequestResolver(
return this.logoutRequestResolver;
}
if (version().startsWith("4")) {
return new OpenSaml4LogoutRequestResolver(relyingPartyRegistrationResolver);
return OpenSaml4LogoutSupportFactory.getLogoutRequestResolver(relyingPartyRegistrationResolver);
}
return new OpenSaml3LogoutRequestResolver(relyingPartyRegistrationResolver);
}
Expand Down Expand Up @@ -470,13 +477,13 @@ private Saml2LogoutResponseValidator logoutResponseValidator() {

private Saml2LogoutResponseResolver logoutResponseResolver(
RelyingPartyRegistrationResolver relyingPartyRegistrationResolver) {
if (this.logoutResponseResolver == null) {
if (version().startsWith("4")) {
return new OpenSaml4LogoutResponseResolver(relyingPartyRegistrationResolver);
}
return new OpenSaml3LogoutResponseResolver(relyingPartyRegistrationResolver);
if (this.logoutResponseResolver != null) {
return this.logoutResponseResolver;
}
return this.logoutResponseResolver;
if (version().startsWith("4")) {
return OpenSaml4LogoutSupportFactory.getLogoutResponseResolver(relyingPartyRegistrationResolver);
}
return new OpenSaml3LogoutResponseResolver(relyingPartyRegistrationResolver);
}

}
Expand Down Expand Up @@ -519,4 +526,38 @@ public void logout(HttpServletRequest request, HttpServletResponse response, Aut

}

private static class OpenSaml4LogoutSupportFactory {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this class still necessary? I think it was necessary when it was acting as a classpath guard. However, since you are using reflection now, I don't think this class offers this same benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those classes make the code cleaner by moving the try-catch from the reflective operations to another method


private static Saml2LogoutResponseResolver getLogoutResponseResolver(
RelyingPartyRegistrationResolver relyingPartyRegistrationResolver) {
try {
Class<?> logoutResponseResolver = ClassUtils.forName(
"org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutResponseResolver",
OpenSaml4LogoutSupportFactory.class.getClassLoader());
return (Saml2LogoutResponseResolver) logoutResponseResolver
.getDeclaredConstructor(RelyingPartyRegistrationResolver.class)
.newInstance(relyingPartyRegistrationResolver);
}
catch (ReflectiveOperationException ex) {
throw new IllegalStateException("Could not instantiate OpenSaml4LogoutResponseResolver", ex);
}
}

private static Saml2LogoutRequestResolver getLogoutRequestResolver(
RelyingPartyRegistrationResolver relyingPartyRegistrationResolver) {
try {
Class<?> logoutRequestResolver = ClassUtils.forName(
"org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutRequestResolver",
OpenSaml4LogoutSupportFactory.class.getClassLoader());
return (Saml2LogoutRequestResolver) logoutRequestResolver
.getDeclaredConstructor(RelyingPartyRegistrationResolver.class)
.newInstance(relyingPartyRegistrationResolver);
}
catch (ReflectiveOperationException ex) {
throw new IllegalStateException("Could not instantiate OpenSaml4LogoutRequestResolver", ex);
}
}

}

}
12 changes: 7 additions & 5 deletions ...rg/springframework/security/config/annotation/web/configurers/NamespaceHttpX509Tests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;

import javax.security.auth.x500.X500Principal;
import javax.servlet.http.HttpServletRequest;

import org.bouncycastle.asn1.x500.X500Name;
import org.bouncycastle.asn1.x500.style.BCStyle;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import sun.security.x509.X500Name;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify how these changes pertain to the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those changes came from the revert that I did to the old commit, where we weren't using toolchain to build the modules


import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -242,8 +240,12 @@ protected void configure(HttpSecurity http) throws Exception {
}

private String extractCommonName(X509Certificate certificate) {
X500Principal principal = certificate.getSubjectX500Principal();
return new X500Name(principal.getName()).getRDNs(BCStyle.CN)[0].getFirst().getValue().toString();
try {
return ((X500Name) certificate.getSubjectDN()).getCommonName();
}
catch (Exception ex) {
throw new IllegalArgumentException(ex);
}
}

}
Expand Down
18 changes: 4 additions & 14 deletions core/src/test/java/org/springframework/security/core/SpringSecurityCoreVersionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -61,24 +60,15 @@ public class SpringSecurityCoreVersionTests {

@BeforeEach
public void setup() throws Exception {
setFinalStaticField(SpringSecurityCoreVersion.class, "logger", this.logger);
Field logger = ReflectionUtils.findField(SpringSecurityCoreVersion.class, "logger");
StaticFinalReflectionUtils.setField(logger, this.logger);
}

@AfterEach
public void cleanup() throws Exception {
System.clearProperty(getDisableChecksProperty());
setFinalStaticField(SpringSecurityCoreVersion.class, "logger",
LogFactory.getLog(SpringSecurityCoreVersion.class));
}

private static void setFinalStaticField(Class<?> clazz, String fieldName, Object value)
throws ReflectiveOperationException {
Field field = clazz.getDeclaredField(fieldName);
field.setAccessible(true);
Field modifiers = Field.class.getDeclaredField("modifiers");
modifiers.setAccessible(true);
modifiers.setInt(field, field.getModifiers() & ~Modifier.FINAL);
field.set(null, value);
Field logger = ReflectionUtils.findField(SpringSecurityCoreVersion.class, "logger");
StaticFinalReflectionUtils.setField(logger, LogFactory.getLog(SpringSecurityCoreVersion.class));
}

@Test
Expand Down
Loading