-
Notifications
You must be signed in to change notification settings - Fork 194
[backend] feat(catalog): encryption of pwd values in db (#4605) #4689
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: release/current
Are you sure you want to change the base?
Changes from all commits
8274d6c
66f22bd
5f2097a
078a872
ea72511
38d4612
d3b7cf1
62daec5
79e54d1
b2126f2
242d2c2
5707332
e44c03c
32d1c79
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 |
|---|---|---|
|
|
@@ -53,6 +53,8 @@ steps: | |
| OPENAEV_ADMIN_EMAIL: [email protected] | ||
| OPENAEV_ADMIN_PASSWORD: admin | ||
| OPENAEV_ADMIN_TOKEN: 0d17ce9a-f3a8-4c6d-9721-c98dc3dc023f | ||
| OPENAEV_ADMIN_ENCRYPTION_KEY: ThisIsMyUltraSecureEncryptionKey | ||
| OPENAEV_ADMIN_ENCRYPTION_SALT: ilikesaltyfoodnomnom | ||
| SPRING_PROFILES_ACTIVE: ci | ||
| OPENBAS_RABBITMQ_HOSTNAME: rabbitmq-e2e | ||
| commands: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,11 @@ | |
| import static io.openaev.database.model.CatalogConnectorConfiguration.ENCRYPTED_FORMATS; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import io.openaev.database.model.CatalogConnector; | ||
| import io.openaev.database.model.CatalogConnectorConfiguration; | ||
| import io.openaev.database.model.ConnectorInstanceConfiguration; | ||
| import io.openaev.database.model.ConnectorInstancePersisted; | ||
| import io.openaev.database.model.*; | ||
| import io.openaev.rest.exception.UnencryptableElementException; | ||
| import io.openaev.service.connector_instances.EncryptionService; | ||
| import io.openaev.utils.JsonUtils; | ||
| import io.openaev.utils.reflection.FieldUtils; | ||
| import jakarta.validation.constraints.NotNull; | ||
|
|
@@ -20,13 +20,17 @@ | |
| import java.util.stream.Collectors; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| @Slf4j | ||
| public class BaseIntegrationConfiguration { | ||
| private final ObjectMapper mapper = new ObjectMapper(); | ||
| @Getter @Setter private boolean enable = false; | ||
|
|
||
| public static <T extends BaseIntegrationConfiguration> T fromConnectorInstanceConfigurationSet( | ||
| @NotNull Set<ConnectorInstanceConfiguration> configurations, Class<T> targetClass) | ||
| @NotNull ConnectorInstance instance, | ||
| Class<T> targetClass, | ||
| EncryptionService encryptionService) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels odd to pass a service as a method argument.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I do agree that was my initial intention but I had cyclic dependencies. I did not think of moving part of the code to a dedicated utils, I'll try to change that :) |
||
| throws NoSuchMethodException, | ||
| InvocationTargetException, | ||
| InstantiationException, | ||
|
|
@@ -37,12 +41,25 @@ public static <T extends BaseIntegrationConfiguration> T fromConnectorInstanceCo | |
| FieldUtils.getAllDeclaredAnnotatedFields(targetClass, IntegrationConfigKey.class); | ||
| for (Field field : annotatedFields) { | ||
| Optional<ConnectorInstanceConfiguration> config = | ||
| configurations.stream() | ||
| instance.getConfigurations().stream() | ||
| .filter(c -> c.getKey().equals(field.getAnnotation(IntegrationConfigKey.class).key())) | ||
| .findFirst(); | ||
| if (config.isPresent()) { | ||
| FieldUtils.setField( | ||
| newObj, field, JsonUtils.fromJsonNode(config.get().getValue(), field.getType())); | ||
| // If the field is encrypted and can be decrypted | ||
Dimfacion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (config.get().isEncrypted() && encryptionService != null) { | ||
| // Decrypt the field and set it | ||
| FieldUtils.setField( | ||
| newObj, | ||
| field, | ||
| JsonUtils.fromJsonNode( | ||
| new ObjectMapper() | ||
| .valueToTree(encryptionService.decrypt(config.get().getValue().asText())), | ||
| field.getType())); | ||
| } else { | ||
| // Otherwise, we just set the field | ||
| FieldUtils.setField( | ||
| newObj, field, JsonUtils.fromJsonNode(config.get().getValue(), field.getType())); | ||
| } | ||
Dimfacion marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } else { | ||
| FieldUtils.setField(newObj, field, null); | ||
| } | ||
|
|
@@ -51,20 +68,41 @@ public static <T extends BaseIntegrationConfiguration> T fromConnectorInstanceCo | |
| } | ||
|
|
||
| public Set<ConnectorInstanceConfiguration> toInstanceConfigurationSet( | ||
| ConnectorInstancePersisted relatedInstance) { | ||
| ConnectorInstancePersisted relatedInstance, EncryptionService encryptionService) { | ||
| List<Field> annotatedFields = | ||
| FieldUtils.getAllDeclaredAnnotatedFields(this.getClass(), IntegrationConfigKey.class); | ||
| return annotatedFields.stream() | ||
| .map( | ||
| af -> | ||
| ConnectorInstanceConfiguration.builder() | ||
| .key(af.getAnnotation(IntegrationConfigKey.class).key()) | ||
| .value(mapper.valueToTree(FieldUtils.getField(this, af))) | ||
| .isEncrypted( | ||
| ENCRYPTED_FORMATS.contains( | ||
| af.getAnnotation(IntegrationConfigKey.class).valueFormat())) | ||
| .connectorInstance(relatedInstance) | ||
| .build()) | ||
| af -> { | ||
| JsonNode value = mapper.valueToTree(FieldUtils.getField(this, af)); | ||
| boolean isEncrypted = | ||
| ENCRYPTED_FORMATS.contains( | ||
| af.getAnnotation(IntegrationConfigKey.class).valueFormat()); | ||
| // If the field is encrypted | ||
| if (isEncrypted) { | ||
| // If the encryption service is not null, we use it | ||
| if (encryptionService != null) { | ||
| try { | ||
| value = mapper.valueToTree(encryptionService.encrypt(value.toString())); | ||
| } catch (Exception e) { | ||
| throw new UnencryptableElementException( | ||
| "Cannot encrypt the element : " + af.getName(), e); | ||
| } | ||
| } else { | ||
| // If the encryption service is null, there might be an issue with how the | ||
| // executor has been initialized | ||
| log.warn( | ||
| "A encrypted element cannot be decrypted due to the encryption service being null. You might want to look into that as this can cause issue."); | ||
| } | ||
| } | ||
|
|
||
| return ConnectorInstanceConfiguration.builder() | ||
| .key(af.getAnnotation(IntegrationConfigKey.class).key()) | ||
| .value(value) | ||
| .isEncrypted(isEncrypted) | ||
| .connectorInstance(relatedInstance) | ||
| .build(); | ||
| }) | ||
| .collect(Collectors.toSet()); | ||
| } | ||
|
|
||
|
|
||
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.
This method feels overly complex to me. Could we simplify it by passing the config and only decrypting the relevant keys?
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.
This method do not only do decrypting. It's used to create a Configuration java model from the database model. I guess I can simplify it a bit with another private method but it can't be used only for the relevant keys.