Skip to content

FLE implementation with property value converter. #1554

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

Conversation

mikereiche
Copy link
Collaborator

Closes #763

  • You have read the Spring Data contribution guidelines.
  • There is a ticket in the bug tracker for the project in our JIRA.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

@mikereiche mikereiche requested review from daschl and dnault September 12, 2022 21:32
}

/* TODO needed later
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The 'diff' gets confusing here. I removed a commented-out convertToCouchbaseType() method here and added a new convertForWriteIfNeeded() (above) that has three args vs. the existing one that had only one arg (below).


boolean wasString = false;
List<Exception> exceptionList = new LinkedList<>();
String decryptedString = new String(decrypted);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not really happy with the following code that takes the decrypted string and turns it into a simple type. But it seems to do the job.

Class<?> targetType = conversions.getCustomWriteTarget(context.getProperty().getType()).orElse(null);

value = ctx.getConverter().getPotentiallyConvertedSimpleWrite(property, ctx.getAccessor(), false);
if (conversions.isSimpleType(sourceType)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar comment to above - I'm not really happy with this code that converts the value into a string for decryptiing. But it seems to work.

@@ -204,7 +205,7 @@ public enum NumberToDateTimeConverter implements Converter<Number, DateTime> {

@Override
public DateTime convert(Number source) {
return source == null ? null : new DateTime(source.longValue());
return source == null ? null : new DateTime(source.longValue(), DateTimeZone.UTC);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is to make the Couchbase deserialization compatible with the SDK (via jackson-jodatime) which is in UTC (vs. local time zone).

}

@Override
protected CryptoManager cryptoManager() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the method the user will need to override.

}

@Version protected long version;
@Encrypted public String encryptedField;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Normally spring-data would use an @ValueConverter annotation for this. But my CouchbaseValueConverterFactory also recognizes Couchbase @Encrypted

@mikereiche mikereiche force-pushed the datacouch_763_fle_implementation_with_property_value_converter branch from 13f0e30 to 270b720 Compare September 13, 2022 20:56
Copy link
Contributor

@dnault dnault left a comment

Choose a reason for hiding this comment

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

+2, after the improvements we chatted about today

@mikereiche mikereiche force-pushed the datacouch_763_fle_implementation_with_property_value_converter branch from 270b720 to fe2eeae Compare October 7, 2022 22:18
@mikereiche mikereiche force-pushed the datacouch_763_fle_implementation_with_property_value_converter branch from fe2eeae to ea3e0b6 Compare October 7, 2022 23:01
@mikereiche mikereiche merged commit deca246 into main Oct 7, 2022
@mikereiche mikereiche deleted the datacouch_763_fle_implementation_with_property_value_converter branch February 27, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Field Level Encryption [DATACOUCH-455]
2 participants