From f1612bee6619588e0f52b5b87f3bc7d918d6ebe0 Mon Sep 17 00:00:00 2001 From: Peter Hoffmann <954078+p-hoffmann@users.noreply.github.com> Date: Tue, 15 Sep 2026 03:35:40 +0800 Subject: [PATCH] Decrypt source credentials even when the converter is not injected EncryptedStringConverter received its encryptor only through an @Autowired setter, relying on Hibernate resolving the converter from Spring. When that injection does not happen, as in the GraalVM native image, the encryptor stays null and EncryptorUtils.decrypt returns the value unchanged, so the JDBC driver is handed the literal ENC(...) username and password and every source query fails with "password authentication failed". The encryptor is now held statically and set directly by the defaultStringEncryptor bean, so every converter instance sees it however it was created. A converter used without an encryptor now fails instead of silently reading ENC(...) values or writing plaintext. --- .../org/ohdsi/webapi/DataAccessConfig.java | 2 + .../source/EncryptedStringConverter.java | 31 +++++++--- .../source/EncryptedStringConverterTest.java | 61 +++++++++++++++++++ 3 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/ohdsi/webapi/source/EncryptedStringConverterTest.java diff --git a/src/main/java/org/ohdsi/webapi/DataAccessConfig.java b/src/main/java/org/ohdsi/webapi/DataAccessConfig.java index 87a1b3a15..0990333fd 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")); + } +}