Decryption doesn't work for records created with Model.findOrCreate method

[ahmedrad]

Sails version: 1.0.0-46
Node version: 8.6.0
NPM version: 5.6.0
DB adapter name: sails-postgresql
DB adapter version: 1.0.0-12
Operating system: OS X sierra 10.12.6

---

I've spent quite some time on this and I've come to the conclusion that I can't correctly decrypt any records created with the findOrCreate method called on any model while records created with the create method decrypt fine. There's really not much else to add. Here's a repo to reproduce the issue:

https://github.com/ahmedrad/sails-bug

[sailsbot]

Hi @ahmedrad! It looks like you missed a step or two when you created your issue. Please edit your comment (use the pencil icon at the top-right corner of the comment box) and fix the following:

• Verify "I am experiencing a concrete technical issue (aka a bug) with Sails (ideas and feature proposals should follow the guide for proposing features and enhancements (http://bit.ly/sails-feature-guide), which involves making a pull request). If you're not 100% certain whether it's a bug or not, that's okay--you may continue. The worst that can happen is that the issue will be closed and we'll point you in the right direction."
• Verify "I am not asking a question about how to use Sails or about whether or not Sails has a certain feature (please refer to the documentation(http://sailsjs.com), or post on http://stackoverflow.com, our Google Group (http://bit.ly/sails-google-group) or our live chat (https://gitter.im/balderdashy/sails)."
• Verify "I have already searched for related issues, and found none open (if you found a related closed issue, please link to it in your post)."
• Verify "My issue title is concise, on-topic and polite ("jst.js being removed from layout.ejs on lift" is good; "templates dont work" or "why is sails dumb" are not so good)."
• Verify "I have tried all the following (if relevant) and my issue remains:"
• Verify "I can provide steps to reproduce this issue that others can follow."

As soon as those items are rectified, post a new comment (e.g. “Ok, fixed!”) below and we'll take a look. Thanks!

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@sailsjs.com

[ahmedrad]

“Ok, fixed!

[sailsbot]

Sorry to be a hassle, but it looks like your issue is still missing some required info. Please double-check your initial comment and try again.

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@sailsjs.com

[ahmedrad]

ok this time fixed for real!

[sailsbot]

@ahmedrad Thanks for posting, we'll take a look as soon as possible.

---

For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

[sgress454]

Thanks @ahmedrad -- this is definitely a possible bug, encrypt/decrypt is a new feature. We'll check this out, but in the meantime the easy workaround (especially w/ Node 8) is:

let record = await Model.findOne({ ... });
if (!record) { record = await Model.create({...}).fetch(); }

It should be equally performant as this is basically what Waterline is doing behind the scenes.

[mikermcneil]

https://trello.com/c/6MOd5YxZ

[mikermcneil]

@ahmedrad thanks, reproduced and looking into it

[mikermcneil]

Looks like the issue is that it's inserted incorrectly in that case (works fine for looking up an already-existing record-- but when it creates, it does it wrong)

[mikermcneil]

@ahmedrad this is a tough one to solve without making the implementation a lot messier. I'm tempted to just add an error message that prevents you from using .findOrCreate() with .decrypt(). Open to ideas why this wouldn't be a good idea. Got to run to a mtg but will think on this more

[sgress454]

@mikermcneil

I'm tempted to just add an error message that prevents you from using .findOrCreate() with .decrypt()

It sounds like you're saying that records created using .findOrCreate() will never be able to be decrypted, right? Because they're not being encrypted properly? In which case we'll have to just disallow findOrCreate entirely when default encryption is turned on for an attribute, not just if decrypt is chained...

[ahmedrad]

I already implemented the workaround thanks @sgress454!

@mikermcneil my personal opinion is that an error message is not sufficient as creating the record and retrieving it are not gonna always happen in the same place. In my case I was creating an Oauth record that will be used later on for API calls and could not retrieve the actual Oauth keys when the api calls were needed as I could not decrypt them.

If there's no short term solution for the bug then I'd go with disabling findOrCreate on any model with an encrypted attribute. Not that it's a great solution either as people might add an encrypted attribute to a model they've been using for a while and suddenly find themselves having to go remove all these findOrCreate calls :(

[mikermcneil]

@ahmedrad @sgress454 thanks for your help! After a fresh look, it wasn't as bad to fix as I thought. The real issue is that F2SQ wasn't actually idempotent (because it does encryption now, in some cases), but we were treating it as such in the case of findOrCreate(). Simplest solution was to provide a private way of disabling encryption (see linked commit). That new flag should not be relied upon by userland other than for experimental use (it could change at any time).

This is resolved and published in waterline 0.13.1.

@ahmedrad Would you mind verifying that everything looks good from your end? I'll leave it to you to close this issue. Thanks!

[ahmedrad]

looks like it's fixed! thanks for the support.