Skip to content

Commit c086ece

Browse files
AB-xdevAB-xdev
committed
Rewrite serializer to use Jackson v3
1 parent 9acb53c commit c086ece

2 files changed

Lines changed: 80 additions & 79 deletions

File tree

‎oauth2-oidc-remember-me/src/main/java/software/xdev/sse/oauth2/rememberme/serializer/DefaultOAuth2CookieRememberMeAuthSerializer.java‎

Lines changed: 38 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Objects;
2626
import java.util.Optional;
2727
import java.util.Set;
28+
import java.util.function.Consumer;
2829
import java.util.function.Function;
2930
import java.util.function.Supplier;
3031
import java.util.stream.Collectors;
@@ -46,15 +47,13 @@
4647
import com.fasterxml.jackson.annotation.JsonInclude;
4748
import com.fasterxml.jackson.annotation.JsonProperty;
4849
import com.fasterxml.jackson.annotation.JsonTypeInfo;
49-
import com.fasterxml.jackson.core.JsonProcessingException;
50-
import com.fasterxml.jackson.databind.ObjectMapper;
51-
import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
52-
import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
53-
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
5450
import com.nimbusds.jwt.JWTClaimNames;
5551
import com.nimbusds.jwt.JWTParser;
5652

5753
import software.xdev.sse.oauth2.util.OAuth2UserNameAttributeKeyExtractor;
54+
import tools.jackson.databind.json.JsonMapper;
55+
import tools.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
56+
import tools.jackson.databind.jsontype.PolymorphicTypeValidator;
5857

5958

6059
/**
@@ -66,62 +65,62 @@
6665
@SuppressWarnings("java:S4544") // Handled by PolymorphicTypeValidator (see below)
6766
public class DefaultOAuth2CookieRememberMeAuthSerializer implements OAuth2CookieRememberMeAuthSerializer
6867
{
69-
protected final ObjectMapper mapper;
68+
protected final JsonMapper mapper;
7069

7170
public DefaultOAuth2CookieRememberMeAuthSerializer()
7271
{
73-
this(true);
72+
this(true, null);
7473
}
7574

7675
// Only here for tests
77-
protected DefaultOAuth2CookieRememberMeAuthSerializer(final boolean withPolymorphicTypeValidator)
76+
protected DefaultOAuth2CookieRememberMeAuthSerializer(
77+
final boolean withPolymorphicTypeValidator,
78+
final Consumer<JsonMapper.Builder> customizer)
7879
{
79-
this(withPolymorphicTypeValidator
80-
// https://cowtowncoder.medium.com/jackson-2-10-safe-default-typing-2d018f0ce2ba
81-
? BasicPolymorphicTypeValidator.builder()
82-
.allowIfSubType(Instant.class)
83-
.allowIfSubType(java.net.URL.class)
84-
.allowIfSubType(ArrayList.class)
85-
.build()
86-
: null);
80+
this(
81+
withPolymorphicTypeValidator
82+
// https://cowtowncoder.medium.com/jackson-2-10-safe-default-typing-2d018f0ce2ba
83+
? BasicPolymorphicTypeValidator.builder()
84+
.allowIfSubType(Instant.class)
85+
.allowIfSubType(java.net.URL.class)
86+
.allowIfSubType(ArrayList.class)
87+
.build()
88+
: null,
89+
customizer);
8790
}
8891

89-
protected DefaultOAuth2CookieRememberMeAuthSerializer(final PolymorphicTypeValidator polymorphicTypeValidator)
92+
protected DefaultOAuth2CookieRememberMeAuthSerializer(
93+
final PolymorphicTypeValidator polymorphicTypeValidator,
94+
final Consumer<JsonMapper.Builder> customizer)
9095
{
91-
this.mapper = new ObjectMapper()
92-
.registerModule(new JavaTimeModule())
93-
.setSerializationInclusion(JsonInclude.Include.NON_NULL);
94-
Optional.ofNullable(polymorphicTypeValidator)
95-
.ifPresent(this.mapper::setPolymorphicTypeValidator);
96+
final JsonMapper.Builder builder = JsonMapper.builder()
97+
.changeDefaultPropertyInclusion(v -> v.withValueInclusion(JsonInclude.Include.NON_NULL));
98+
99+
if(polymorphicTypeValidator != null)
100+
{
101+
builder.polymorphicTypeValidator(polymorphicTypeValidator);
102+
}
103+
if(customizer != null)
104+
{
105+
customizer.accept(builder);
106+
}
107+
108+
this.mapper = builder.build();
96109
}
97110

98111
@Override
99112
public String serialize(final OAuth2AuthenticationToken token, final OAuth2AuthorizedClient client)
100113
{
101-
try
102-
{
103-
return this.mapper.writeValueAsString(new SOAuth2AuthContainer(token, client));
104-
}
105-
catch(final JsonProcessingException e)
106-
{
107-
throw new IllegalStateException("Unable to serialize", e);
108-
}
114+
return this.mapper.writeValueAsString(new SOAuth2AuthContainer(token, client));
109115
}
110116

111117
@Override
112118
public OAuth2AuthContainer deserialize(
113119
final String json,
114120
final Function<String, ClientRegistration> clientRegistrationResolver)
115121
{
116-
try
117-
{
118-
return this.mapper.readValue(json, SOAuth2AuthContainer.class)
119-
.toOriginal(clientRegistrationResolver);
120-
}
121-
catch(final JsonProcessingException e)
122-
{
123-
throw new IllegalStateException("Unable to deserialize", e);
124-
}
122+
return this.mapper.readValue(json, SOAuth2AuthContainer.class)
123+
.toOriginal(clientRegistrationResolver);
125124
}
126125

127126
public record DefaultOAuth2AuthContainer(

‎oauth2-oidc-remember-me/src/test/java/software/xdev/sse/oauth2/rememberme/serializer/DefaultOAuth2CookieRememberMeAuthSerializerTest.java‎

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@
2828
import java.util.List;
2929
import java.util.Map;
3030
import java.util.Set;
31-
import java.util.function.Predicate;
31+
import java.util.function.Consumer;
3232

33-
import org.apache.commons.lang3.exception.ExceptionUtils;
3433
import org.junit.jupiter.api.Assertions;
3534
import org.junit.jupiter.api.Test;
3635
import org.springframework.security.core.authority.SimpleGrantedAuthority;
@@ -48,7 +47,17 @@
4847
import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority;
4948
import org.springframework.security.web.authentication.WebAuthenticationDetails;
5049

50+
import tools.jackson.core.StreamReadFeature;
51+
import tools.jackson.databind.exc.InvalidDefinitionException;
52+
import tools.jackson.databind.exc.InvalidTypeIdException;
53+
import tools.jackson.databind.json.JsonMapper;
5154

55+
56+
// NOTE: Originally designed for Jackson v2
57+
// Was improved in v3 so that for example Object objects are no longer deserialized into
58+
// potentially dangerous classes.
59+
// Please note that Jackson v3 does not cover all possible options (as it depends on the available classes)
60+
// and therefore this test is still present
5261
class DefaultOAuth2CookieRememberMeAuthSerializerTest
5362
{
5463
private static final String ACCESS_TOKEN = "dummy";
@@ -70,70 +79,63 @@ class DefaultOAuth2CookieRememberMeAuthSerializerTest
7079
private static final String NAME = "A B";
7180
private static final String EMAIL = "a.b@xdev-software.de";
7281

82+
private static final Consumer<JsonMapper.Builder> SERIALIZER_JSON_MAPPER_CUSTOMIZER =
83+
b -> b.enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION);
84+
85+
static DefaultOAuth2CookieRememberMeAuthSerializer createSerializer(final boolean safe)
86+
{
87+
return new DefaultOAuth2CookieRememberMeAuthSerializer(safe, SERIALIZER_JSON_MAPPER_CUSTOMIZER);
88+
}
89+
7390
@Test
7491
void defaultSerializationWorks()
7592
{
7693
Assertions.assertDoesNotThrow(() -> this.serializeAndDeserialize(
77-
new DefaultOAuth2CookieRememberMeAuthSerializer(),
94+
createSerializer(true),
7895
Map.of()));
7996
}
8097

8198
@SuppressWarnings("checkstyle:VisibilityModifier")
8299
static Set<String> attackSuccessIds = new HashSet<>();
83100

84101
@Test
85-
void performAttackWithoutProtectionSuccess()
102+
void performAttackWithoutDedicatedProtection()
86103
{
87-
final String id = "success";
88-
this.performAttack(
89-
new DefaultOAuth2CookieRememberMeAuthSerializer(false),
90-
id,
91-
List.of(
92-
"Unable to deserialize"::equals,
93-
s -> s.contains(AttackPerformer.SUCCESS_INDICATOR),
94-
AttackPerformer.SUCCESS_INDICATOR::equals)
104+
final String id = "default";
105+
final var serializer = createSerializer(false);
106+
final InvalidDefinitionException ex = Assertions.assertThrows(
107+
InvalidDefinitionException.class,
108+
() -> this.performAttack(serializer, id)
95109
);
96-
Assertions.assertTrue(attackSuccessIds.contains(id));
110+
Assertions.assertTrue(ex.getMessage().startsWith("Configured `PolymorphicTypeValidator`")
111+
&& ex.getMessage().contains("denies resolution of all subtypes of base type "
112+
+ "`java.lang.Object` as using too generic base type "
113+
+ "can open a security hole without checks on subtype: "
114+
+ "please configure a custom `PolymorphicTypeValidator` for this use case"));
115+
Assertions.assertFalse(attackSuccessIds.contains(id));
97116
}
98117

99118
@Test
100119
void performAttackFails()
101120
{
102121
final String id = "fail";
103-
this.performAttack(
104-
new DefaultOAuth2CookieRememberMeAuthSerializer(),
105-
id,
106-
List.of(
107-
"Unable to deserialize"::equals,
108-
s -> s.startsWith("Could not resolve type id")
109-
&& s.contains("$AttackPerformer' as a subtype of `java.lang.Object`: "
110-
+ "Configured `PolymorphicTypeValidator`")));
122+
final var serializer = createSerializer(true);
123+
final InvalidTypeIdException ex = Assertions.assertThrows(
124+
InvalidTypeIdException.class,
125+
() -> this.performAttack(serializer, id)
126+
);
127+
Assertions.assertTrue(ex.getMessage().startsWith("Could not resolve type id")
128+
&& ex.getMessage().contains(
129+
"$AttackPerformer' as a subtype of `java.lang.Object`: Configured `PolymorphicTypeValidator`"));
111130
Assertions.assertFalse(attackSuccessIds.contains(id));
112131
}
113132

114133
void performAttack(
115134
final DefaultOAuth2CookieRememberMeAuthSerializer serializer,
116-
final String id,
117-
final List<Predicate<String>> exceptionCauseMessageChecks)
135+
final String id)
118136
{
119137
final Map<String, Object> data = Map.of("test", new AttackPerformer(id));
120-
final IllegalStateException ex = Assertions.assertThrows(
121-
IllegalStateException.class,
122-
() -> this.serializeAndDeserialize(serializer, data));
123-
Throwable current = ex;
124-
125-
int i = 0;
126-
while(current != null)
127-
{
128-
Assertions.assertTrue(
129-
i < exceptionCauseMessageChecks.size()
130-
&& exceptionCauseMessageChecks.get(i).test(current.getMessage()),
131-
"Invalid exception message at nested=" + i + ": " + current.getMessage()
132-
+ "\nSOURCE EXCEPTION:\n"
133-
+ ExceptionUtils.getStackTrace(ex));
134-
current = current.getCause();
135-
i++;
136-
}
138+
this.serializeAndDeserialize(serializer, data);
137139
}
138140

139141
public static class AttackPerformer

0 commit comments

Comments
 (0)