-
Notifications
You must be signed in to change notification settings - Fork 194
Fix serialization for field level encryption. #1622
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
Fix serialization for field level encryption. #1622
Conversation
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; |
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.
The CryptoConverter now needs the ObjectMapper.
| TypeInformation<?> type = ClassTypeInformation.from(value.getClass()); | ||
| ((MappingCouchbaseConverter) this).writeInternalRoot(value, embeddedDoc, type, false, null); | ||
| ((MappingCouchbaseConverter) this).writeInternalRoot(value, embeddedDoc, type, false, null, true); | ||
| value = embeddedDoc; |
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.
writeInternalRoot needs a flag of whether or not to process PropertyValueConverters (specifically the CryptoConverter) so it doesn't recursively encrypt such an annotated property.
| import org.springframework.data.convert.ValueConversionContext; | ||
| import org.springframework.data.mapping.PersistentProperty; | ||
|
|
||
| import com.couchbase.client.core.encryption.CryptoManager; |
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.
Addition of ObjectMapper.
| return plaintextParser.readValueAs(beanPropertyTypeRef); | ||
| */ | ||
|
|
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.
The deleted comment is a bit misleading. The conversions do take place in context.getConverter.read() and getPotentiallyConvertedSimpleRead() below.
| CouchbaseDocument doc = new CouchbaseDocument(); | ||
| context.getConverter().writeInternalRoot(value, doc, property.getTypeInformation(), false, property, false); | ||
| plainText = JsonObject.from(doc.export()).toBytes(); | ||
| } |
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.
The old code relied on the entity (Airport, Address etc) produced by context.read(value) have a nice toString() method that gives the json representation of the object. It's a bad assumption.
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
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.
Add one of our @jsonvalue Enums to Address which gets used in the FLE tests and elsewhere.
| T10("\"10%"), T20("\"20%"), T30("\"30%"); | ||
|
|
||
| private final String code; | ||
|
|
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.
Add a quote to the content of the Enum serialization.
| @Encrypted public DateTime encDateTime; | ||
|
|
||
| @Encrypted public ETurbulenceCategory turbulence; | ||
| @Encrypted public Address encAddress = new Address(); |
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.
Add an encrypted @jsonvalue Enum.
| encString = "myString"; | ||
| encStrings = new String[] { "myString" }; | ||
| encString = "myS\"tring"; | ||
| encStrings = new String[] { "mySt\"ring" }; |
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.
make a tricky String and String array
| encDateTime = NOW_DateTime; | ||
|
|
||
| encAddress = new Address(); | ||
|
|
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.
let the caller (test case) initialize this.
The key changes are at lines 141 and 145 of CryptoConverter.
(a) use objectMapper to encode strings instead of just escaping and adding quotes; and
(b) use context.getConverter().writeInternalRoot() to serialize instead of
relying on context.read(value).toString()) which relies on the entity object having a toString() that produces json.
Closes #1621.