-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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.
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
OAuth2AuthenticationExceptionMixin doesn't work in JDK 17 #11893
Comments
Hi, @EvgeniGordeev, I wonder if this is related to FasterXML/jackson-databind#3275. Have you already tried using the latest Jackson dependencies to see if that resolves the issue? It appears that the above issue was fixed in 2.13.4, though Boot 2.7.2 uses 2.13.3. |
@jzheaux Checked it in SB 2.7.4 - the issue is still there. |
@jzheaux here's a rudimentary test that shows off the issue with Spring Boot 2.7.4 and Jackson 2.13.4: https://github.com/chrisrhut/spring-security-11893 As mentioned by @EvgeniGordeev it can be fixed by adding the |
Since the exception is complaining about code controlled by Jackson, it seems to me this would need to be addressed in Jackson. Is there some way that Spring Security is using Jackson incorrectly? If not, please open a ticket with Jackson. Feel free to add here a link to that ticket. |
@jzheaux I do believe this is something Spring Security is doing incorrectly, but that the failure only manifests in newer JDK's (16 and higher), that have hardened security around reflection in the module system. In the @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE,
isGetterVisibility = JsonAutoDetect.Visibility.NONE)
@JsonIgnoreProperties(ignoreUnknown = true, value = { "cause", "stackTrace", "suppressedExceptions" })
abstract class OAuth2AuthenticationExceptionMixin {
@JsonCreator
OAuth2AuthenticationExceptionMixin(@JsonProperty("error") OAuth2Error error,
@JsonProperty("detailMessage") String message) {
}
} I think the Using The patch is simply this. I'm happy to make it into a PR: diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixin.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixin.java
index 9fa810c1ac..b52557d5aa 100644
--- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixin.java
+++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixin.java
@@ -42,7 +42,7 @@ abstract class OAuth2AuthenticationExceptionMixin {
@JsonCreator
OAuth2AuthenticationExceptionMixin(@JsonProperty("error") OAuth2Error error,
- @JsonProperty("detailMessage") String message) {
+ @JsonProperty("message") String message) {
}
} Thanks! |
@chrisrhut I had to add a similar mixin to get around it. |
workaround: Mixin import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import org.springframework.security.oauth2.core.OAuth2Error;
/**
* https://github.com/spring-projects/spring-security/issues/11893.
* See {@link org.springframework.security.oauth2.client.jackson2.OAuth2AuthenticationExceptionMixin}.
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
@JsonIgnoreProperties(ignoreUnknown = true, value = {"cause", "stackTrace", "suppressedExceptions", "detailMessage", "suppressed", "localizedMessage"})
abstract class OAuth2AuthenticationExceptionMixinJdk17Fix {
@JsonCreator
OAuth2AuthenticationExceptionMixinJdk17Fix(@JsonProperty("error") OAuth2Error error,
@JsonProperty("message") String message) {
}
} Custom Jackson Module: public class CustomModule extends SimpleModule {
public CustomModule() {
super(CustomModule.class.getName(), new Version(1, 0, 0, null, null, null));
}
public void setupModule(Module.SetupContext context) {
context.setMixInAnnotations(OAuth2AuthenticationException.class, OAuth2AuthenticationExceptionMixinJdk17Fix.class);
}
} Test: import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.security.jackson2.SecurityJackson2Modules;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
public class OAuth2AuthenticationExceptionMixinJdk17FixTest {
@Test
public void writeAndRead() throws JsonProcessingException {
ObjectMapper om = new ObjectMapper();
om.activateDefaultTyping(om.getPolymorphicTypeValidator(), ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY);
om.registerModules(SecurityJackson2Modules.getModules(this.getClass().getClassLoader()));
om.registerModule(new OidcSupportModule());
OAuth2AuthenticationException exception = new OAuth2AuthenticationException(new OAuth2Error(OAuth2ErrorCodes.INVALID_CLIENT), "errorMessage");
String deser = om.writeValueAsString(exception);
OAuth2AuthenticationException ser = om.readValue(deser, OAuth2AuthenticationException.class);
Assertions.assertEquals(exception.getError().getErrorCode(), ser.getError().getErrorCode());
Assertions.assertEquals(exception.getMessage(), ser.getMessage());
}
} |
Thank you, @chrisrhut for offering to make a PR out of #11893 (comment). That would be most welcome. It would be nice to merge this to 6.x as well as 5.x in which case, the mixin should still recognize old serialized values that use the |
(Edit see the update below.) @jzheaux I started to take a look at this over the holiday. I'm a little stymied because I can't reproduce the issue in the I thought it may be because the project declares the workaround JVM args in its JUnit test configuration (source): tasks.named('test', Test).configure {
onlyIf { !project.hasProperty("buildSrc.skipTests") }
useJUnitPlatform()
jvmArgs(
'--add-opens', 'java.base/java.lang=ALL-UNNAMED',
'--add-opens', 'java.base/java.util=ALL-UNNAMED'
)
} But even if I remove that, I cannot get the test to fail and thus I can't get into a valid starting position for the fix and associated tests. I'm not sure if you have any inkling what could be different, but I'd appreciate a second set of eyes. Here are a couple of other things I tried:
Thanks for any new insight! PS here's a patch to copy my test case into diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixinTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixinTests.java
index ab97544e2f..4df7f0a0e7 100644
--- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixinTests.java
+++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthenticationExceptionMixinTests.java
@@ -65,6 +65,14 @@ public class OAuth2AuthenticationExceptionMixinTests {
JSONAssert.assertEquals(expected, serializedJson, true);
}
+ @Test
+ public void demonstratesIssue11893() throws Exception {
+ Exception e = new OAuth2AuthenticationException(new OAuth2Error("invalid_nonce"));
+ byte[] bytes = mapper.writeValueAsBytes(e);
+
+ assertThat(bytes).isNotNull();
+ }
+
@Test EDIT: Update, the |
Hi, Caused by: java.io.UncheckedIOException: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid type definition for type Should i create a seperate ticket for it? |
|
Describe the bug
With Redis session enabled, GenericJackson2JsonRedisSerializer based on ObjectMapper with OAuth2ClientJackson2Module an exception is thrown in JDK 17 while serializing:
com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid type definition for type `org.springframework.security.oauth2.core.OAuth2AuthenticationException`: Failed to construct BeanSerializer for [simple type, class org.springframework.security.oauth2.core.OAuth2AuthenticationException]: (java.lang.IllegalArgumentException) Failed to call `setAccess()` on Field 'detailMessage' (of class `java.lang.Throwable`) due to `java.lang.reflect.InaccessibleObjectException`, problem: Unable to make field private java.lang.String java.lang.Throwable.detailMessage accessible: module java.base does not "opens java.lang" to unnamed module @5aebe890
To Reproduce
Spring boot 2.7.2:
Expected behavior
OAuth2AuthenticationException object is successfully serialized in JDK 17.
Workaround
VM option
--add-opens java.base/java.lang=ALL-UNNAMED
as usual.The text was updated successfully, but these errors were encountered: