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

Check if the field exists before decryption #26

Merged
merged 1 commit into from
Oct 23, 2018
Merged

Check if the field exists before decryption #26

merged 1 commit into from
Oct 23, 2018

Conversation

eyp
Copy link
Contributor

@eyp eyp commented Oct 23, 2018

Sometimes the field could be marked to not be retrieved, in that case, the decrypt function will fail because because the field is undefined.

Sometimes the field could be marked to not be retrieved, in that case, the decrypt function will fail because because the field is undefined.
@coveralls
Copy link

coveralls commented Oct 23, 2018

Pull Request Test Coverage Report for Build 86

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.5%) to 96.154%

Totals Coverage Status
Change from base Build 84: -0.5%
Covered Lines: 130
Relevant Lines: 134

💛 - Coveralls

@wheresvic wheresvic merged commit 84fa711 into wheresvic:master Oct 23, 2018
@wheresvic
Copy link
Owner

Hi @eyp

Thanks a lot for taking the time to submit this fix, I will merge and make a patch bugfix release :)

@wheresvic
Copy link
Owner

@eyp Do you think you could help me in writing a test case for this patch? I tried writing one like the following:

it("should decrypt data on find() method call when only selected encrypted fields are retrieved", () => {
    // given
    let sut = getSut();

    // when
    return sut
      .save()
      .then(() => {
        expectEncryptionValues(sut);

        return NestedFieldEncryption.findOneAndUpdate(
          {
            _id: sut._id
          },
          {
            toEncryptString: "yaddayadda",
            toEncryptObject: { nested: "snoop" }
          }
        );
      })
      .then(() => {
        return NestedFieldEncryption.find({ _id: sut._id }, "__enc_toEncryptString toEncryptString");
      })
      .then(foundArray => {
        const found = foundArray[0];
        expect(found.toEncryptString).to.equal("yaddayadda");
      });
  });

But it works without the patch as well (which would have been my expectation too). Maybe I'm missing something from your use case.

Thanks!

@eyp
Copy link
Contributor Author

eyp commented Oct 24, 2018

Sure, I can try to write it, or revise yours.

@wheresvic
Copy link
Owner

wheresvic commented Oct 24, 2018 via email

@eyp
Copy link
Contributor Author

eyp commented Oct 24, 2018

I'm sorry, but are you talking about #23 or #26 ?
Anyway, I've written another test right now for the #26 (this one)

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.

3 participants