Skip to content
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

encrypt_properties #44

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

encrypt_properties #44

wants to merge 17 commits into from

Conversation

akash-gautam07
Copy link

@akash-gautam07 akash-gautam07 commented Jul 31, 2023

This PR creates a encrypt_properties case in the tsub-js, to provide functionality for encrypting the properties.

@saiprasannasastry
Copy link

add a description

src/transformers.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@silesky
Copy link

silesky commented Aug 3, 2023

@akash-gautam07 this needs tests ;-)

src/transformers.ts Outdated Show resolved Hide resolved
src/transformers.ts Outdated Show resolved Hide resolved
@saiprasannasastry
Copy link

@silesky updated the tests :)

Copy link

@silesky silesky left a comment

Choose a reason for hiding this comment

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

Looks good, just some small changes =S

@saiprasannasastry
Copy link

saiprasannasastry commented Aug 14, 2023

Looks good, just some small changes =S

was the interface the only comment? i'd like an approval if there are no more comments because of timezone difference, but there is an issue in code, with seed being passed i am not getting a stable cipher text.
If you have any understanding would love your review on that aspect as well

package.json Outdated Show resolved Hide resolved
package.json Outdated
"tiny-hashes": "^1.0.1"
},
"devDependencies": {
"@types/jest": "^27.4.0",
"@types/node": "^16.18.40",
Copy link

Choose a reason for hiding this comment

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

Do we actually need node types here? It's better to not have any node types in an isomorphic package, since it ensures we don't accidentally use node built-ins in our code (I sometimes add them for tests, but not ideal).

@saiprasannasastry
Copy link

@akash-gautam07 there's an assertion that's failed . we are not getting stable identifiers can you fix this

const labelBytes = forge.util.encodeUtf8(label)
let ciphertextBase64: string

if (seed) {
Copy link

Choose a reason for hiding this comment

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

Should this be !seed?

@akash-gautam07
Copy link
Author

@akash-gautam07 there's an assertion that's failed . we are not getting stable identifiers can you fix this

@saiprasannasastry Isn't it the purpose of utilizing RSA-OAEP is to ensure the generation of new ciphertexts each time? The tsub code also generates different encrypted ciphers each time.

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.

4 participants