-
Notifications
You must be signed in to change notification settings - Fork 4
HTM-1654: Use JSON marshalling for session serialisation #1536
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,38 +6,129 @@ | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| package org.tailormap.api.configuration; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import java.io.InputStream; | ||||||||||||||||||||||||||||||||||||
| import java.io.ObjectInputStream; | ||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.annotation.JsonTypeInfo; | ||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.core.StreamReadFeature; | ||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.databind.SerializationFeature; | ||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; | ||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||
| import java.lang.invoke.MethodHandles; | ||||||||||||||||||||||||||||||||||||
| import java.nio.charset.StandardCharsets; | ||||||||||||||||||||||||||||||||||||
| import org.jspecify.annotations.NonNull; | ||||||||||||||||||||||||||||||||||||
| import org.slf4j.Logger; | ||||||||||||||||||||||||||||||||||||
| import org.slf4j.LoggerFactory; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.beans.factory.BeanClassLoaderAware; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.context.annotation.Bean; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.context.annotation.Configuration; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.core.convert.ConversionService; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.core.convert.support.GenericConversionService; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.core.serializer.Deserializer; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.core.serializer.support.DeserializingConverter; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.core.serializer.support.SerializingConverter; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.security.jackson2.SecurityJackson2Modules; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.session.config.SessionRepositoryCustomizer; | ||||||||||||||||||||||||||||||||||||
| import org.springframework.session.jdbc.JdbcIndexedSessionRepository; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Configuration(proxyBeanMethods = false) | ||||||||||||||||||||||||||||||||||||
| public class JdbcSessionConfiguration implements BeanClassLoaderAware { | ||||||||||||||||||||||||||||||||||||
| private static final Logger logger = | ||||||||||||||||||||||||||||||||||||
| LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private static final String CREATE_SESSION_ATTRIBUTE_QUERY = | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| INSERT INTO %TABLE_NAME%_ATTRIBUTES (SESSION_PRIMARY_ID, ATTRIBUTE_NAME, ATTRIBUTE_BYTES) | ||||||||||||||||||||||||||||||||||||
| VALUES (?, ?, convert_from(?, 'UTF8')::jsonb) | ||||||||||||||||||||||||||||||||||||
| """; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private static final String UPDATE_SESSION_ATTRIBUTE_QUERY = | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| UPDATE %TABLE_NAME%_ATTRIBUTES | ||||||||||||||||||||||||||||||||||||
| SET ATTRIBUTE_BYTES = encode(?, 'escape')::jsonb | ||||||||||||||||||||||||||||||||||||
| WHERE SESSION_PRIMARY_ID = ? | ||||||||||||||||||||||||||||||||||||
| AND ATTRIBUTE_NAME = ? | ||||||||||||||||||||||||||||||||||||
| """; | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+46
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private ClassLoader classLoader; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Bean | ||||||||||||||||||||||||||||||||||||
| SessionRepositoryCustomizer<JdbcIndexedSessionRepository> customizer() { | ||||||||||||||||||||||||||||||||||||
| return (sessionRepository) -> { | ||||||||||||||||||||||||||||||||||||
| sessionRepository.setCreateSessionAttributeQuery(CREATE_SESSION_ATTRIBUTE_QUERY); | ||||||||||||||||||||||||||||||||||||
| sessionRepository.setUpdateSessionAttributeQuery(UPDATE_SESSION_ATTRIBUTE_QUERY); | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||
| * Ignore deserialization exceptions, see <a | ||||||||||||||||||||||||||||||||||||
| * href="https://github.com/spring-projects/spring-session/issues/529#issuecomment-2761671945">here</a>. | ||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||
| @Configuration | ||||||||||||||||||||||||||||||||||||
| public class JdbcSessionConfiguration { | ||||||||||||||||||||||||||||||||||||
| @Bean("springSessionConversionService") | ||||||||||||||||||||||||||||||||||||
| public ConversionService springSessionConversionService() { | ||||||||||||||||||||||||||||||||||||
| GenericConversionService converter = new GenericConversionService(); | ||||||||||||||||||||||||||||||||||||
| converter.addConverter(Object.class, byte[].class, new SerializingConverter()); | ||||||||||||||||||||||||||||||||||||
| converter.addConverter(byte[].class, Object.class, new DeserializingConverter(new CustomDeserializer())); | ||||||||||||||||||||||||||||||||||||
| public ConversionService springSessionConversionService(ObjectMapper objectMapper) { | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| BasicPolymorphicTypeValidator ptv = BasicPolymorphicTypeValidator.builder() | ||||||||||||||||||||||||||||||||||||
| .allowIfSubType("org.tailormap.api.security.") | ||||||||||||||||||||||||||||||||||||
| .allowIfSubType("org.springframework.security.") | ||||||||||||||||||||||||||||||||||||
| .allowIfSubType("java.util.") | ||||||||||||||||||||||||||||||||||||
| .allowIfSubType(java.lang.Number.class) | ||||||||||||||||||||||||||||||||||||
| .allowIfSubType("java.time.") | ||||||||||||||||||||||||||||||||||||
| .allowIfBaseType(Object.class) | ||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ObjectMapper copy = objectMapper | ||||||||||||||||||||||||||||||||||||
| .copy() | ||||||||||||||||||||||||||||||||||||
| .configure( | ||||||||||||||||||||||||||||||||||||
| StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature(), | ||||||||||||||||||||||||||||||||||||
| (logger.isDebugEnabled() || logger.isTraceEnabled())) | ||||||||||||||||||||||||||||||||||||
| .configure(SerializationFeature.INDENT_OUTPUT, (logger.isDebugEnabled() || logger.isTraceEnabled())); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // register mixins early so Jackson picks up the @JsonCreator constructor for TailormapUserDetails | ||||||||||||||||||||||||||||||||||||
| // implementations | ||||||||||||||||||||||||||||||||||||
| copy.addMixIn( | ||||||||||||||||||||||||||||||||||||
| org.tailormap.api.security.TailormapUserDetailsImpl.class, | ||||||||||||||||||||||||||||||||||||
| org.tailormap.api.security.TailormapUserDetailsImplMixin.class); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| copy.addMixIn( | ||||||||||||||||||||||||||||||||||||
| org.tailormap.api.security.TailormapOidcUser.class, | ||||||||||||||||||||||||||||||||||||
| org.tailormap.api.security.TailormapOidcUserMixin.class); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| copy.registerModules(SecurityJackson2Modules.getModules(this.classLoader)) | ||||||||||||||||||||||||||||||||||||
| .activateDefaultTyping(ptv, ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| final GenericConversionService converter = new GenericConversionService(); | ||||||||||||||||||||||||||||||||||||
| // Object -> byte[] (serialize to JSON bytes) | ||||||||||||||||||||||||||||||||||||
| converter.addConverter(Object.class, byte[].class, source -> { | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| logger.debug("Serializing Spring Session: {}", source); | ||||||||||||||||||||||||||||||||||||
| return copy.writerFor(Object.class).writeValueAsBytes(source); | ||||||||||||||||||||||||||||||||||||
| } catch (IOException e) { | ||||||||||||||||||||||||||||||||||||
| logger.error("Error serializing Spring Session object: {}", source, e); | ||||||||||||||||||||||||||||||||||||
| throw new RuntimeException("Unable to serialize Spring Session.", e); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| throw new RuntimeException("Unable to serialize Spring Session.", e); | |
| String message = | |
| "Unable to serialize Spring Session attribute of type " | |
| + (source != null ? source.getClass().getName() : "null") | |
| + "."; | |
| throw new RuntimeException(message, e); |
mprins marked this conversation as resolved.
Show resolved
Hide resolved
Check warning on line 113 in src/main/java/org/tailormap/api/configuration/JdbcSessionConfiguration.java
Codecov / codecov/patch
src/main/java/org/tailormap/api/configuration/JdbcSessionConfiguration.java#L112-L113
Added lines #L112 - L113 were not covered by tests
Check warning on line 118 in src/main/java/org/tailormap/api/configuration/JdbcSessionConfiguration.java
Codecov / codecov/patch
src/main/java/org/tailormap/api/configuration/JdbcSessionConfiguration.java#L115-L118
Added lines #L115 - L118 were not covered by tests
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling for deserialization failures wraps exceptions in RuntimeException without providing specific context. When session deserialization fails, the generic message "Unable to deserialize Spring Session." does not provide enough information for debugging. Consider providing more detailed error messages that include information about the data being deserialized, such as the length or a truncated preview of the JSON content.
| throw new RuntimeException("Unable to deserialize Spring Session.", e); | |
| String preview; | |
| try { | |
| String content = new String(source, StandardCharsets.UTF_8); | |
| int maxLength = 256; | |
| preview = | |
| content.length() > maxLength | |
| ? content.substring(0, maxLength) + "..." | |
| : content; | |
| } catch (Exception ex) { | |
| // Fallback in case the bytes cannot be converted to a UTF-8 string | |
| preview = "<unavailable>"; | |
| } | |
| throw new RuntimeException( | |
| "Unable to deserialize Spring Session. Payload length=" + source.length | |
| + ", preview='" + preview + "'", | |
| e); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||
| /* | ||||||
| * Copyright (C) 2025 B3Partners B.V. | ||||||
| * | ||||||
| * SPDX-License-Identifier: MIT | ||||||
| */ | ||||||
| package org.tailormap.api.security; | ||||||
|
|
||||||
| import com.fasterxml.jackson.annotation.JsonAutoDetect; | ||||||
| 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 java.util.Collection; | ||||||
| import java.util.Map; | ||||||
| import org.springframework.security.core.GrantedAuthority; | ||||||
|
|
||||||
| @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) | ||||||
| @JsonAutoDetect( | ||||||
| fieldVisibility = JsonAutoDetect.Visibility.ANY, | ||||||
| getterVisibility = JsonAutoDetect.Visibility.NONE, | ||||||
| isGetterVisibility = JsonAutoDetect.Visibility.NONE) | ||||||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||||||
| public abstract class TailormapOidcUserMixin { | ||||||
|
|
||||||
| @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) | ||||||
| public TailormapOidcUserMixin( | ||||||
| @SuppressWarnings("unused") @JsonProperty("claims") Map<String, Object> claims, | ||||||
mprins marked this conversation as resolved.
Dismissed
Show dismissed
Hide dismissed
|
||||||
| @SuppressWarnings("unused") @JsonProperty("authorities") Collection<? extends GrantedAuthority> authorities, | ||||||
mprins marked this conversation as resolved.
Dismissed
Show dismissed
Hide dismissed
|
||||||
| @SuppressWarnings("unused") @JsonProperty("attributes") Map<String, Object> attributes, | ||||||
github-code-quality[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
|
||||||
| @SuppressWarnings("unused") @JsonProperty("nameAttributeKey") String nameAttributeKey, | ||||||
github-code-quality[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
|
||||||
| @SuppressWarnings("unused") @JsonProperty("oidcRegistrationName") String oidcRegistrationName, | ||||||
github-code-quality[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
|
||||||
| @SuppressWarnings("unused") @JsonProperty("additionalGroupProperties") | ||||||
| Collection<TailormapAdditionalProperty> additionalGroupProperties) { | ||||||
|
Comment on lines
+32
to
+33
|
||||||
| // mixin constructor only for Jackson 2; no implementation | ||||||
|
||||||
| // mixin constructor only for Jackson 2; no implementation | |
| // mixin constructor only for Jackson; no implementation |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TailormapOidcUserMixin specifies a @JsonCreator constructor with parameters claims, authorities, attributes, nameAttributeKey, oidcRegistrationName, and additionalGroupProperties. However, TailormapOidcUser does not have a constructor with this signature. The actual constructor has parameters: authorities, idToken, userInfo, nameAttributeKey, oidcRegistrationName, additionalGroupProperties. This mismatch will cause Jackson deserialization to fail because Jackson cannot find a matching constructor. You need to add a constructor that matches the mixin signature or adjust the mixin to match the actual constructor.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,11 +5,13 @@ | |||||||||||||||||||||
| */ | ||||||||||||||||||||||
| package org.tailormap.api.security; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||||||||||||||||||||||
| import java.io.Serial; | ||||||||||||||||||||||
| import java.time.ZoneId; | ||||||||||||||||||||||
| import java.time.ZonedDateTime; | ||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||||
| import java.util.HashSet; | ||||||||||||||||||||||
| import org.apache.commons.lang3.StringUtils; | ||||||||||||||||||||||
| import org.springframework.security.core.GrantedAuthority; | ||||||||||||||||||||||
|
|
@@ -19,8 +21,14 @@ | |||||||||||||||||||||
| import org.tailormap.api.persistence.json.AdminAdditionalProperty; | ||||||||||||||||||||||
| import org.tailormap.api.repository.GroupRepository; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
||||||||||||||||||||||
| /** | |
| * Internal {@link TailormapUserDetails} implementation. | |
| * | |
| * <p>Do not use this class directly; always depend on the {@link TailormapUserDetails} | |
| * interface instead. This class is {@code public} only to support JSON/session | |
| * deserialization (e.g. by Jackson / Spring Session). Changing its visibility back to | |
| * package-private would break that deserialization.</p> | |
| */ |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JavaDoc is incomplete. It lists 6 parameters but is missing documentation for the additionalProperties and additionalGroupProperties parameters that are specified in the mixin constructor. These parameters should be documented to match the mixin definition.
| * @param organisation the organisation | |
| * @param organisation the organisation | |
| * @param additionalProperties additional user-specific properties | |
| * @param additionalGroupProperties additional properties derived from the user's groups |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SQL query syntax is incorrect. The UPDATE query uses encode() function with 'escape' encoding to convert bytes to text, then casts to jsonb. However, the parameter is already UTF-8 JSON bytes and should use convert_from() like the INSERT query. The encode() function with 'escape' encoding is meant for binary data, not JSON. This will likely cause invalid JSON to be stored in the database.