search

tink-crypto / tink

Tink is a multi-language, cross-platform, open source library that provides cryptographic APIs that are secure, easy to use correctly, and hard(er) to misuse.
https://developers.google.com/tink
Apache License 2.0
13.5k stars 1.18k forks source link

Change HybridEncryptWrapper class visibility to public #563

Closed yoavamit closed 2 years ago

yoavamit commented 2 years ago

Looking at the implementation in other languages (go, python, c++) this class/object is exposed.

Without the HybridEncryptWrapper class being public, users who decide to implement a custom HybridEncrypt and HybridDecrypt primitives will have to manually register their primitives instead of using the more convenient HybridEncryptWrapper.register() and HybridDecryptWrapper.register() calls respectively.

Clarification: HybridDecryptWrapper is already a public class, it's just HybridEncryptWrapper class that needs to be exposed as well.

thaidn commented 2 years ago

@tholenst wdyt?

tholenst commented 2 years ago

@tholenst wdyt?

The class can be public, but a private constructor should be added so that users can only register, but not create the object.

yoavamit commented 2 years ago

@tholenst wdyt?

The class can be public, but a private constructor should be added so that users can only register, but not create the object.

I'll add a commit making the constructor package private (same as HybridDecryptWrapper)

yoavamit commented 2 years ago

@tholenst @thaidn I updated the PR to fix the constructor. Let me know if you think the changes look reasonable to you, or if there's anything else you think should be changed here 🙇

thaidn commented 2 years ago

This looks good to me. Will merge.