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

Object returned by save or create is cyphered #23

Closed
eyp opened this issue Sep 14, 2018 · 12 comments · Fixed by #25
Closed

Object returned by save or create is cyphered #23

eyp opened this issue Sep 14, 2018 · 12 comments · Fixed by #25
Assignees
Labels

Comments

@eyp
Copy link
Contributor

eyp commented Sep 14, 2018

If I have a field name marked as cyphered and I do:

new User(myUser).save().then((savedUser) => {
savedUser.name is cyphered
}
Shouldn't it be like when I do a find? Or I must do a find afterwardsto be able to use the object?

@wheresvic
Copy link
Owner

wheresvic commented Sep 16, 2018

Hi @eyp ,

You can use the decrypt method as provided by the encryption plugin - see below for an example:

new User(myUser).save().then((savedUser) => {
   savedUser.decryptFieldsSync();
}

where user is mongoose.model("User", UserSchema) and the UserSchema has the field encryption plugin set.

@eyp
Copy link
Contributor Author

eyp commented Sep 16, 2018

Thanks. I'm experiencing the same issue doing a find({my criteria}).
Of course if I loop all the results and do a decryptFieldsSync() the fields are decrypted. Is this the right way to do it?

@wheresvic
Copy link
Owner

@eyp - yes, you can loop through and decrypt the fields.

We can also consider providing an option for the plugin that would automatically decrypt them - would this be something useful for you?

@wheresvic wheresvic self-assigned this Sep 17, 2018
@eyp
Copy link
Contributor Author

eyp commented Sep 17, 2018

Yes, I think it would be better to have an option. What I was looking for was something that made transparent the insertion of ciphered fields, but also the read from the database. So what I wanted only was to have some fields ciphered in the database, not outside it.

@vinczedani
Copy link
Contributor

The 'init' hook should decrypt your data,
How are you finding the documents?

@eyp
Copy link
Contributor Author

eyp commented Sep 17, 2018

Just after doing MySchema.find({some criteria}) I need to decrypt the fields manually.

@vinczedani
Copy link
Contributor

Dear @eyp
Sorry for the long wait, but I only had time now to check on your issue. You were right, the find({ query }) did not decrypt the data as expected. I created a new pull request and a new test case to cover this. The fix and the test is now an open PR.
@wheresvic
I think the expected behaviour is to have the data decrypted just like on findById and other find methods.
So in my opinion, even tho it is breaking the interface, it should not be a major version bump.
Cheers,
Daniel

@wheresvic
Copy link
Owner

@eyp please try with latest release (2.1.1) 😁

@eyp
Copy link
Contributor Author

eyp commented Oct 23, 2018

It's strange, now I'm in 2.1.1 and I still have to decrypt manually, doing:
myResults.forEach((myResult) => {
myResult.decryptFieldsSync()
})
After a regular find.

@eyp
Copy link
Contributor Author

eyp commented Oct 24, 2018

It's true that your test is working fine, so it should be working in my case as well. I'll revise my code to figure out what's happening.

@eyp
Copy link
Contributor Author

eyp commented Oct 24, 2018

So what's happening is that even if my version is 2.1.1 (updated with npm, of course), the file that is downloaded doesn't have the change you did for fixing this error (the line 207):

schema.pre("init", function(_next, _data) {

instead of

schema.post("init", function(_next, _data) {

@eyp
Copy link
Contributor Author

eyp commented Oct 24, 2018

Anyway, just realized you released 2.1.3 that has everything. So forget about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants