Skip to content

Fix serialization for field level encryption. #1622

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 1 commit into from
Dec 16, 2022
Merged

Fix serialization for field level encryption. #1622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 11 additions & 9 deletions src/main/java/org/springframework/data/couchbase/config/AbstractCouchbaseConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
import java.util.Map;
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

The CryptoConverter now needs the ObjectMapper.

import java.util.Set;

import com.couchbase.client.java.encryption.annotation.Encrypted;
import com.fasterxml.jackson.annotation.JsonValue;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider;
Expand Down Expand Up @@ -77,11 +75,13 @@
import com.couchbase.client.core.error.CouchbaseException;
import com.couchbase.client.java.Cluster;
import com.couchbase.client.java.codec.JacksonJsonSerializer;
import com.couchbase.client.java.encryption.annotation.Encrypted;
import com.couchbase.client.java.encryption.databind.jackson.EncryptionModule;
import com.couchbase.client.java.env.ClusterEnvironment;
import com.couchbase.client.java.json.JacksonTransformers;
import com.couchbase.client.java.json.JsonValueModule;
import com.couchbase.client.java.query.QueryScanConsistency;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;

Expand Down Expand Up @@ -312,7 +312,7 @@ public CouchbaseMappingContext couchbaseMappingContext(CustomConversions customC
}

final public ObjectMapper getObjectMapper() {
if(objectMapper == null) {
if (objectMapper == null) {
synchronized (this) {
if (objectMapper == null) {
objectMapper = couchbaseObjectMapper();
Expand Down Expand Up @@ -403,7 +403,7 @@ protected boolean autoIndexCreation() {
*/
@Bean(name = BeanNames.COUCHBASE_CUSTOM_CONVERSIONS)
public CustomConversions customConversions() {
return customConversions(getCryptoManager());
return customConversions(getCryptoManager(), getObjectMapper());
}

/**
Expand All @@ -414,11 +414,12 @@ public CustomConversions customConversions() {
* @param cryptoManager
* @return must not be {@literal null}.
*/
public CustomConversions customConversions(CryptoManager cryptoManager) {
public CustomConversions customConversions(CryptoManager cryptoManager, ObjectMapper objectMapper) {
List<GenericConverter> newConverters = new ArrayList();
CustomConversions customConversions = CouchbaseCustomConversions.create(configurationAdapter -> {
SimplePropertyValueConversions valueConversions = new SimplePropertyValueConversions();
valueConversions.setConverterFactory(new CouchbasePropertyValueConverterFactory(cryptoManager, annotationToConverterMap()));
valueConversions.setConverterFactory(
new CouchbasePropertyValueConverterFactory(cryptoManager, annotationToConverterMap(), objectMapper));
valueConversions.setValueConverterRegistry(new PropertyValueConverterRegistrar().buildRegistry());
valueConversions.afterPropertiesSet(); // wraps the CouchbasePropertyValueConverterFactory with CachingPVCFactory
configurationAdapter.setPropertyValueConversions(valueConversions);
Expand All @@ -431,17 +432,18 @@ public CustomConversions customConversions(CryptoManager cryptoManager) {
return customConversions;
}

Map<Class<? extends Annotation>,Class<?>> annotationToConverterMap(){
Map<Class<? extends Annotation>,Class<?>> map= new HashMap();
Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap() {
Map<Class<? extends Annotation>, Class<?>> map = new HashMap();
map.put(Encrypted.class, CryptoConverter.class);
map.put(JsonValue.class, JsonValueConverter.class);
return map;
}

/**
* cryptoManager can be null, so it cannot be a bean and then used as an arg for bean methods
*/
private CryptoManager getCryptoManager() {
if(cryptoManager == null) {
if (cryptoManager == null) {
synchronized (this) {
if (cryptoManager == null) {
cryptoManager = cryptoManager();
Expand Down
5 changes: 3 additions & 2 deletions ...main/java/org/springframework/data/couchbase/core/convert/AbstractCouchbaseConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,16 @@ public Object convertForWriteIfNeeded(Object inValue) {
Class<?> elementType = value.getClass();

if (elementType == null || conversions.isSimpleType(elementType)) {
// superseded by EnumCvtrs value = Enum.class.isAssignableFrom(value.getClass()) ? ((Enum<?>) value).name() : value;
// superseded by EnumCvtrs value = Enum.class.isAssignableFrom(value.getClass()) ? ((Enum<?>) value).name() :
// value;
} else if (value instanceof Collection || elementType.isArray()) {
TypeInformation<?> type = ClassTypeInformation.from(value.getClass());
value = ((MappingCouchbaseConverter) this).writeCollectionInternal(MappingCouchbaseConverter.asCollection(value),
new CouchbaseList(conversions.getSimpleTypeHolder()), type, null, null);
} else {
CouchbaseDocument embeddedDoc = new CouchbaseDocument();
TypeInformation<?> type = ClassTypeInformation.from(value.getClass());
((MappingCouchbaseConverter) this).writeInternalRoot(value, embeddedDoc, type, false, null);
((MappingCouchbaseConverter) this).writeInternalRoot(value, embeddedDoc, type, false, null, true);
value = embeddedDoc;
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

writeInternalRoot needs a flag of whether or not to process PropertyValueConverters (specifically the CryptoConverter) so it doesn't recursively encrypt such an annotated property.

}
return value;
Expand Down
12 changes: 7 additions & 5 deletions ...g/springframework/data/couchbase/core/convert/CouchbasePropertyValueConverterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.couchbase.client.core.encryption.CryptoManager;
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

Addition of ObjectMapper.

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
import org.springframework.util.Assert;
import com.fasterxml.jackson.databind.ObjectMapper;

/**
* Accept the Couchbase @Encrypted and @JsonValue annotations in addition to @ValueConverter annotation.<br>
Expand All @@ -48,14 +48,16 @@
*/
public class CouchbasePropertyValueConverterFactory implements PropertyValueConverterFactory {

final CryptoManager cryptoManager;
final Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap;
private final CryptoManager cryptoManager;
private final ObjectMapper objectMapper;
private final Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap;
static protected final Map<Class<?>, Optional<PropertyValueConverter<?, ?, ?>>> converterCacheForType = new ConcurrentHashMap<>();

public CouchbasePropertyValueConverterFactory(CryptoManager cryptoManager,
Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap) {
Map<Class<? extends Annotation>, Class<?>> annotationToConverterMap, ObjectMapper objectMapper) {
this.cryptoManager = cryptoManager;
this.annotationToConverterMap = annotationToConverterMap;
this.objectMapper = objectMapper;
}

/**
Expand Down Expand Up @@ -155,7 +157,7 @@ public <DV, SV, P extends ValueConversionContext<?>> PropertyValueConverter<DV,

// CryptoConverter takes a cryptoManager argument
if (CryptoConverter.class.isAssignableFrom(converterType)) {
return (PropertyValueConverter<DV, SV, P>) new CryptoConverter(cryptoManager);
return (PropertyValueConverter<DV, SV, P>) new CryptoConverter(cryptoManager, objectMapper);
} else if (property != null) { // try constructor that takes PersistentProperty
try {
Constructor<?> constructor = converterType.getConstructor(PersistentProperty.class);
Expand Down
39 changes: 17 additions & 22 deletions src/main/java/org/springframework/data/couchbase/core/convert/CryptoConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,12 @@
import java.util.Map;
import java.util.Optional;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.springframework.core.convert.ConversionFailedException;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.convert.PropertyValueConverter;
import org.springframework.data.convert.ValueConversionContext;
import org.springframework.data.couchbase.core.convert.translation.JacksonTranslationService;
import org.springframework.data.couchbase.core.convert.translation.TranslationService;
import org.springframework.data.couchbase.core.mapping.CouchbaseDocument;
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
import org.springframework.data.mapping.PersistentProperty;
Expand All @@ -41,20 +38,24 @@
import com.couchbase.client.java.json.JsonArray;
import com.couchbase.client.java.json.JsonObject;
import com.couchbase.client.java.json.JsonValue;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

/**
* Encrypt/Decrypted properties annotated. This is registered in
* {@link org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration#customConversions(CryptoManager)}.
* {@link org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration#customConversions(CryptoManager, ObjectMapper)}.
*
* @author Michael Reiche
*/
public class CryptoConverter implements
PropertyValueConverter<Object, CouchbaseDocument, ValueConversionContext<? extends PersistentProperty<?>>> {

CryptoManager cryptoManager;
private final CryptoManager cryptoManager;
private final ObjectMapper objectMapper;

public CryptoConverter(CryptoManager cryptoManager) {
public CryptoConverter(CryptoManager cryptoManager, ObjectMapper objectMapper) {
this.cryptoManager = cryptoManager;
this.objectMapper = objectMapper;
}

@Override
Expand All @@ -81,22 +82,12 @@ private Object coerceToValueRead(byte[] decrypted, CouchbaseConversionContext co
CouchbasePersistentProperty property = context.getProperty();

CustomConversions cnvs = context.getConverter().getConversions();
ConversionService svc = context.getConverter().getConversionService();
Class<?> type = property.getType();

String decryptedString = new String(decrypted);
if ("null".equals(decryptedString)) {
return null;
}
// TODO - as-is, this never gets ran through ObjectMapper() -
// TODO - i.e. @JsonValue etc will not be processed by ObjectMapper

/* this what we would do if we could use a JsonParser with a beanPropertyTypeRef
final JsonParser plaintextParser = p.getCodec().getFactory().createParser(plaintext);
plaintextParser.setCodec(p.getCodec());

return plaintextParser.readValueAs(beanPropertyTypeRef);
*/

Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

The deleted comment is a bit misleading. The conversions do take place in context.getConverter.read() and getPotentiallyConvertedSimpleRead() below.

if (!cnvs.isSimpleType(type) && !type.isArray()) {
JsonObject jo = JsonObject.fromJson(decryptedString);
Expand Down Expand Up @@ -133,16 +124,20 @@ private byte[] coerceToBytesWrite(CouchbasePersistentProperty property, Converti
}
plainText = ja.toBytes();
} else if (cnvs.isSimpleType(sourceType)) { // simpleType
String plainString = value != null ? value.toString() : null; // TODO - this will ignore @JsonValue
String plainString = value != null ? value.toString() : null;
if ((sourceType == String.class || targetType == String.class) || sourceType == Character.class
|| sourceType == char.class || Enum.class.isAssignableFrom(sourceType)
|| Locale.class.isAssignableFrom(sourceType)) {
// TODO use jackson serializer here
plainString = "\"" + plainString.replaceAll("\"", "\\\"") + "\"";
|| sourceType == char.class || sourceType.isEnum() || Locale.class.isAssignableFrom(sourceType)) {
try {
plainString = objectMapper.writeValueAsString(plainString);// put quotes around strings
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
}
plainText = plainString.getBytes(StandardCharsets.UTF_8);
} else { // an entity
plainText = JsonObject.fromJson(context.read(value).toString().getBytes(StandardCharsets.UTF_8)).toBytes();
CouchbaseDocument doc = new CouchbaseDocument();
context.getConverter().writeInternalRoot(value, doc, property.getTypeInformation(), false, property, false);
plainText = JsonObject.from(doc.export()).toBytes();
}
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

The old code relied on the entity (Airport, Address etc) produced by context.read(value) have a nice toString() method that gives the json representation of the object. It's a bad assumption.

return plainText;
}
Expand Down
21 changes: 11 additions & 10 deletions src/main/java/org/springframework/data/couchbase/core/convert/MappingCouchbaseConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ public void write(final Object source, final CouchbaseDocument target) {
typeMapper.writeType(type, target);
}

writeInternalRoot(source, target, type, true, null);
writeInternalRoot(source, target, type, true, null, true);
if (target.getId() == null) {
throw new MappingException("An ID property is needed, but not found/could not be generated on this entity.");
}
Expand All @@ -459,7 +459,7 @@ public void write(final Object source, final CouchbaseDocument target) {
*/
@SuppressWarnings("unchecked")
public void writeInternalRoot(final Object source, CouchbaseDocument target, TypeInformation<?> typeHint,
boolean withId, CouchbasePersistentProperty property) {
boolean withId, CouchbasePersistentProperty property, boolean processValueConverter) {
if (source == null) {
return;
}
Expand All @@ -480,7 +480,7 @@ public void writeInternalRoot(final Object source, CouchbaseDocument target, Typ
}

CouchbasePersistentEntity<?> entity = mappingContext.getPersistentEntity(source.getClass());
writeInternalEntity(source, target, entity, withId, property);
writeInternalEntity(source, target, entity, withId, property, processValueConverter);
addCustomTypeKeyIfNecessary(typeHint, source, target);
}

Expand Down Expand Up @@ -517,7 +517,8 @@ private String convertToString(Object propertyObj) {
* @param withId one of the top-level properties is the id for the document
*/
protected void writeInternalEntity(final Object source, final CouchbaseDocument target,
final CouchbasePersistentEntity<?> entity, boolean withId, CouchbasePersistentProperty prop) {
final CouchbasePersistentEntity<?> entity, boolean withId, CouchbasePersistentProperty prop,
boolean processValueConverter) {
if (source == null) {
return;
}
Expand Down Expand Up @@ -566,7 +567,7 @@ public void doWithAssociation(final Association<CouchbasePersistentProperty> ass
}
});

if (prop != null && conversions.hasValueConverter(prop)) { // whole entity is encrypted
if (prop != null && processValueConverter && conversions.hasValueConverter(prop)) { // whole entity is encrypted
Map<String, Object> propertyConverted = (Map<String, Object>) conversions.getPropertyValueConversions()
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

This will prevent recursively encrypting an annotated property.

.getValueConverter(prop).write(source, new CouchbaseConversionContext(prop, this, accessor));
target.setContent(JsonObject.from(propertyConverted));
Expand Down Expand Up @@ -677,7 +678,7 @@ protected void writePropertyInternal(final Object source, final CouchbaseDocumen
CouchbasePersistentEntity<?> entity = isSubtype(prop.getType(), source.getClass())
? mappingContext.getRequiredPersistentEntity(source.getClass())
: mappingContext.getRequiredPersistentEntity(prop);
writeInternalEntity(source, propertyDoc, entity, false, prop);
writeInternalEntity(source, propertyDoc, entity, false, prop, true);
target.put(maybeMangle(prop), propertyDoc);
}

Expand Down Expand Up @@ -728,7 +729,7 @@ private CouchbaseDocument writeMapInternal(final Map<? extends Object, Object> s
prop.getTypeInformation(), prop, getPropertyAccessor(val)));
} else {
CouchbaseDocument embeddedDoc = new CouchbaseDocument();
writeInternalRoot(val, embeddedDoc, prop.getTypeInformation(), false, prop);
writeInternalRoot(val, embeddedDoc, prop.getTypeInformation(), false, prop, true);
target.put(simpleKey, embeddedDoc);
}
} else {
Expand Down Expand Up @@ -772,7 +773,7 @@ public CouchbaseList writeCollectionInternal(final Collection<?> source, final C
} else {
CouchbaseDocument embeddedDoc = new CouchbaseDocument();
writeInternalRoot(element, embeddedDoc,
prop != null ? prop.getTypeInformation() : TypeInformation.of(elementType), false, prop);
prop != null ? prop.getTypeInformation() : TypeInformation.of(elementType), false, prop, true);
target.put(embeddedDoc);
}

Expand Down Expand Up @@ -873,9 +874,9 @@ public Object getPotentiallyConvertedSimpleWrite(final CouchbasePersistentProper
* @param processValueConverter
* @return
*/
public Object getPotentiallyConvertedSimpleWrite(final CouchbasePersistentProperty value,
public Object getPotentiallyConvertedSimpleWrite(final CouchbasePersistentProperty property,
ConvertingPropertyAccessor<Object> accessor, boolean processValueConverter) {
return convertForWriteIfNeeded(value, accessor, processValueConverter); // can access annotations
return convertForWriteIfNeeded(property, accessor, processValueConverter); // can access annotations
}

/**
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/org/springframework/data/couchbase/core/convert/OtherConverters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.List;
import java.util.UUID;

import com.fasterxml.jackson.databind.ObjectWriter;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.ReadingConverter;
import org.springframework.data.convert.WritingConverter;
Expand Down Expand Up @@ -200,7 +199,7 @@ public Class<?> convert(String source) {

/**
* Writing converter for Enums. This is registered in
* {@link org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration#customConversions( CryptoManager)}.
* {@link org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration#customConversions( CryptoManager, ObjectMapper)}.
* The corresponding reading converters are in {@link IntegerToEnumConverterFactory} and
* {@link StringToEnumConverterFactory}
*/
Expand All @@ -220,12 +219,12 @@ public Object convert(Enum<?> source) {
objectMapper.writeValue(generator, source);
String s = writer.toString();
if (s != null && s.startsWith("\"")) {
return objectMapper.readValue(s,String.class);
return objectMapper.readValue(s, String.class);
}
if ("true".equals(s) || "false".equals(s)) {
return objectMapper.readValue(s,Boolean.class);
return objectMapper.readValue(s, Boolean.class);
}
return objectMapper.readValue(s,Number.class);
return objectMapper.readValue(s, Number.class);
} catch (IOException e) {
throw new RuntimeException(e);
}
Expand Down
9 changes: 8 additions & 1 deletion src/test/java/org/springframework/data/couchbase/domain/Address.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

Add one of our @jsonvalue Enums to Address which gets used in the FLE tests and elsewhere.

package org.springframework.data.couchbase.domain;

import com.couchbase.client.java.encryption.annotation.Encrypted;
import org.springframework.data.couchbase.core.mapping.Document;

@Document
Expand All @@ -26,6 +25,7 @@ public class Address extends ComparableEntity {
// for N1qlJoin
private String id;
private String parentId;
private ETurbulenceCategory turbulence;

public Address() {}

Expand All @@ -45,6 +45,13 @@ public void setCity(String city) {
this.city = city;
}

public ETurbulenceCategory getTurbulence() {
return turbulence;
}

public void setTurbulence(ETurbulenceCategory turbulence) {
this.turbulence = turbulence;
}
public String getParentId() {
return parentId;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/springframework/data/couchbase/domain/ETurbulenceCategory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
* @author Michael Reiche
*/
public enum ETurbulenceCategory {
T10("10%"), T20("20%"), T30("30%");
T10("\"10%"), T20("\"20%"), T30("\"30%");

private final String code;

Copy link
Collaborator Author

@mikereiche mikereiche Dec 16, 2022

Choose a reason for hiding this comment

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

Add a quote to the content of the Enum serialization.

Expand Down
Loading