diff --git a/src/main/java/org/ohdsi/webapi/DataAccessConfig.java b/src/main/java/org/ohdsi/webapi/DataAccessConfig.java index 601399621..9d638131b 100644 --- a/src/main/java/org/ohdsi/webapi/DataAccessConfig.java +++ b/src/main/java/org/ohdsi/webapi/DataAccessConfig.java @@ -3,6 +3,7 @@ import com.cosium.spring.data.jpa.entity.graph.repository.support.EntityGraphJpaRepositoryFactoryBean; import org.ohdsi.webapi.arachne.encryption.EncryptorUtils; import org.ohdsi.webapi.arachne.encryption.NotEncrypted; +import org.ohdsi.webapi.source.EncryptedStringConverter; import org.jasypt.encryption.pbe.PBEStringEncryptor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.orm.hibernate5.SpringBeanContainer; @@ -134,6 +135,7 @@ public PBEStringEncryptor defaultStringEncryptor(){ PBEStringEncryptor stringEncryptor = encryptorEnabled ? EncryptorUtils.buildStringEncryptor(env) : new NotEncrypted(); + EncryptedStringConverter.setDefaultEncryptor(stringEncryptor); return stringEncryptor; } diff --git a/src/main/java/org/ohdsi/webapi/source/EncryptedStringConverter.java b/src/main/java/org/ohdsi/webapi/source/EncryptedStringConverter.java index 2b364867e..2166ef5b2 100644 --- a/src/main/java/org/ohdsi/webapi/source/EncryptedStringConverter.java +++ b/src/main/java/org/ohdsi/webapi/source/EncryptedStringConverter.java @@ -15,20 +15,27 @@ * * Spring/Hibernate integration note: * By default, Hibernate instantiates @Converter classes via reflection (newInstance()), - * bypassing Spring dependency injection entirely. To solve this, DataAccessConfig registers - * a SpringBeanContainer via the "hibernate.resource.beans.container" property on the - * EntityManagerFactory. This tells Hibernate to resolve managed beans (including this - * converter) from the Spring ApplicationContext, so @Autowired injection works naturally. + * bypassing Spring dependency injection entirely. DataAccessConfig registers a + * SpringBeanContainer so Hibernate resolves the converter from the ApplicationContext, but + * that is not guaranteed to run @Autowired either: in the GraalVM native image the setter + * is never invoked and the encryptor stayed null, so encrypted ENC(...) credentials were + * handed to the JDBC driver as-is. The encryptor is therefore held statically and set + * directly by the defaultStringEncryptor bean, so every instance sees it however it was + * created. A missing encryptor fails loudly instead of silently skipping encryption. */ @Component @Converter public class EncryptedStringConverter implements AttributeConverter { - private PBEStringEncryptor encryptor; + private static volatile PBEStringEncryptor encryptor; + + public static void setDefaultEncryptor(PBEStringEncryptor defaultStringEncryptor) { + encryptor = defaultStringEncryptor; + } @Autowired public void setEncryptor(PBEStringEncryptor defaultStringEncryptor) { - this.encryptor = defaultStringEncryptor; + setDefaultEncryptor(defaultStringEncryptor); } @Override @@ -36,7 +43,7 @@ public String convertToDatabaseColumn(String attribute) { if (attribute == null) { return null; } - return EncryptorUtils.encrypt(encryptor, attribute); + return EncryptorUtils.encrypt(requireEncryptor(), attribute); } @Override @@ -44,6 +51,14 @@ public String convertToEntityAttribute(String dbData) { if (dbData == null) { return null; } - return EncryptorUtils.decrypt(encryptor, dbData); + return EncryptorUtils.decrypt(requireEncryptor(), dbData); + } + + private static PBEStringEncryptor requireEncryptor() { + PBEStringEncryptor current = encryptor; + if (current == null) { + throw new IllegalStateException("EncryptedStringConverter has no encryptor: the defaultStringEncryptor bean has not been created"); + } + return current; } } diff --git a/src/test/java/org/ohdsi/webapi/source/EncryptedStringConverterTest.java b/src/test/java/org/ohdsi/webapi/source/EncryptedStringConverterTest.java new file mode 100644 index 000000000..38f9b5b92 --- /dev/null +++ b/src/test/java/org/ohdsi/webapi/source/EncryptedStringConverterTest.java @@ -0,0 +1,61 @@ +package org.ohdsi.webapi.source; + +import org.jasypt.encryption.pbe.StandardPBEStringEncryptor; +import org.junit.After; +import org.junit.Test; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertThrows; + +public class EncryptedStringConverterTest { + + @After + public void resetEncryptor() { + EncryptedStringConverter.setDefaultEncryptor(null); + } + + private static StandardPBEStringEncryptor encryptor() { + StandardPBEStringEncryptor encryptor = new StandardPBEStringEncryptor(); + encryptor.setPassword("test-password"); + return encryptor; + } + + @Test + public void converterCreatedWithoutInjectionDecrypts() { + EncryptedStringConverter.setDefaultEncryptor(encryptor()); + + // Hibernate may instantiate the converter itself, so @Autowired never runs on it. + EncryptedStringConverter converter = new EncryptedStringConverter(); + String stored = converter.convertToDatabaseColumn("secret"); + + assertThat(stored, startsWith("ENC(")); + assertThat(new EncryptedStringConverter().convertToEntityAttribute(stored), is("secret")); + } + + @Test + public void encryptedValueIsNotPassedThroughWithoutEncryptor() { + String stored = "ENC(" + encryptor().encrypt("secret") + ")"; + + assertThrows(IllegalStateException.class, + () -> new EncryptedStringConverter().convertToEntityAttribute(stored)); + } + + @Test + public void plaintextIsNotStoredWithoutEncryptor() { + assertThrows(IllegalStateException.class, + () -> new EncryptedStringConverter().convertToDatabaseColumn("secret")); + } + + @Test + public void autowiredSetterSharesEncryptorWithOtherInstances() { + new EncryptedStringConverter().setEncryptor(encryptor()); + + String stored = new EncryptedStringConverter().convertToDatabaseColumn("secret"); + + assertThat(stored, not(is("secret"))); + assertThat(new EncryptedStringConverter().convertToEntityAttribute(stored), is("secret")); + } +}