Entity Listener injecting secret key

Question

I have a system based on the following repository:
https://github.com/damienbeaufils/spring-data-jpa-encryption-example
To encrypt/decrypt a field on the database and it works.

But the way KeyProperty raises issues with tools like SonarQube

@Component
public class KeyProperty {

    public static String DATABASE_ENCRYPTION_KEY;

    @Value("${example.database.encryption.key}")
    public void setDatabase(String databaseEncryptionKey) {
        DATABASE_ENCRYPTION_KEY = databaseEncryptionKey;
    }

}   

To overcome this I've tried injecting the KeyProperty on the EntityListener but to no success due to how EntityListeners are created.

I've tried to search for alternatives but every single one seems to eventually relay to write to static field from an instance method.

Edit:
Sonarqube Issue sample:
Make this "public static DATABASE_ENCRYPTION_KEY" field final
Type: Vulnerability Severity: Critical

Answer 1

But I'd say it is not mandatory that DATABASE_ENCRYPTION_KEY is static field...

Can you change it to member variable and use getKey() in AbstractCryptoConverter? At a first look I do not see the reason, why it has to be static...

---

I have no idea what your "it doesn't work". Spring is able to inject to abstract class, that's not an issue.

I downloaded the project, converted it to Maven (I'm not Gradle guy) using https://stackoverflow.com/a/13396776/384674. Altered pom.xml is on GitHugGist if someone is interested.

What I did as a first step

@Component
public class KeyProperty {

    private static String DATABASE_ENCRYPTION_KEY;

    @Value("${example.database.encryption.key}")
    public void setDatabase(String databaseEncryptionKey) {
        DATABASE_ENCRYPTION_KEY = databaseEncryptionKey;
    }

    public String getKey() {
        return DATABASE_ENCRYPTION_KEY;
    }

}

Additionally I had to exchange reference to DATABASE_ENCRYPTION_KEY with keyProperty.getKey(), while I added @Autowired KeyProperty keyProperty in AbstractCryptoConverter also;

I'm using static DATABASE_ENCRYPTION_KEY just to easier testing, but while it is private it cannot be used elsewhere.

The "problem" is, that tests broke. It's because author is using this static variable everywhere (and that's probably the reason why it was used).

Those tests can be done properly too. I'm not going to fix whole project (at least not today), but this is a desciption how to fix StringCryptoConverterTest working with KeyProperty above.

Highlevel idea is to replace all assignments like

KeyProperty.DATABASE_ENCRYPTION_KEY = "MySuperSecretKey";

with (because Mockito is used)

when(keyProperty.getKey()).thenReturn("MySuperSecretKey");

additionally in setUp() I added (just because it's not Spring managed, so we have to do DI ourselves, I believe there is better Mockito way, but...)

stringCryptoConverter.keyProperty  = keyProperty;
spiedStringCryptoConverter.keyProperty = keyProperty;

and that's it, it works again...

I didn't want to paste the whole code here, so you can navigate to GitHubGist. I left all old usages of DATABASE_ENCRYPTION_KEY in source commented, to have clear visibility, what was changed and how if not clear from description above. I did something similar for LocalDateCryptoConverterTest, but I realized later, you might be more interested in StringCryptoConverterTest, so see LocalDateCryptoConverterTest on GitHubGist also if interested.

---

Now, when I shown it works well, the proper Spring way is:

@Component
public class KeyProperty {

    @Value("${example.database.encryption.key}")
    String key;

    public String getKey() {
        return key;
    }

}

and those 2 tests didn't break still. One can get rid of KeyProperty completely, but it would need more effort to fix tests...