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

feat(normalize-hash): Add support for normalizing hashes #110

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

martijn-dev
Copy link
Contributor

@martijn-dev martijn-dev commented Jun 25, 2024

What's changed:

  • I've added support for a ?normalize flag on the hash tag in the Prisma model.
  • Currently lowercase, uppercase, trim, spaces and diacritics sanitizing are supported. More could be added quite easy.
  • Normalizing takes place in the hashing function. This means that new values, as well as values that you use to query will be sanitized.
  • Normalize options can be combined.
  • Integration tests have been added.

Potential Risks / Breaking changes

  • The feature is an addition. There are no breaking changes involved.
  • As discussed normalizing hashes could give conflict in combination with unique. I've added a disclaimer in the README.

Text from the README:

You can normalize a hash before creation and querying. This might be useful in case you would like to find a User with the name of François with a query input of francois.

There are several normalize options:

/// @encryption:hash(email)?normalize=lowercase  <- lowercase hash
/// @encryption:hash(email)?normalize=uppercase  <- uppercase hash
/// @encryption:hash(email)?normalize=trim       <- trim start and end of hash
/// @encryption:hash(email)?normalize=spaces     <- remove spaces in hash
/// @encryption:hash(email)?normalize=diacritics <- remove diacritics like ç or é in hash

You can also combine the normalize options:

/// @encryption:hash(email)?normalize=lowercase&normalize=trim&normalize=trim&normalize=diacritics

Be aware: Using the normalize hash feature in combination with unique could cause conflicts. Example: Users with the name François and francois result in the same hash which could result in a database conflict.

Questions?

Let me know what you think!
Always open to discussion. I hope you see this feature as a catalyst to expand support for searching in encrypted db values.
Cheers,
Martijn


Fixes #109

@martijn-dev martijn-dev changed the title Add support for sanitizing hashes feat(sanitize-hash): Add support for sanitizing hashes Jun 25, 2024
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

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

Thanks @martijn-dev, this is great!

I left a couple of comments suggesting changes, the only required one would be the leftover console.log in dmmf.ts.

src/dmmf.ts Outdated Show resolved Hide resolved
src/hash.ts Show resolved Hide resolved
README.md Show resolved Hide resolved
src/dmmf.ts Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9660461861

Details

  • 27 of 31 (87.1%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.6%) to 80.1%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dmmf.ts 6 7 85.71%
src/errors.ts 1 2 50.0%
src/hash.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 7807229309: -0.6%
Covered Lines: 269
Relevant Lines: 318

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9660494463

Details

  • 27 of 31 (87.1%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.6%) to 80.1%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dmmf.ts 6 7 85.71%
src/errors.ts 1 2 50.0%
src/hash.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 7807229309: -0.6%
Covered Lines: 269
Relevant Lines: 318

💛 - Coveralls

@martijn-dev martijn-dev requested a review from franky47 June 25, 2024 12:22
@martijn-dev martijn-dev changed the title feat(sanitize-hash): Add support for sanitizing hashes feat(normalize-hash): Add support for normalizing hashes Jun 25, 2024
Copy link
Member

@franky47 franky47 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9662361328

Details

  • 27 of 33 (81.82%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 79.242%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dmmf.ts 6 8 75.0%
src/errors.ts 1 3 33.33%
src/hash.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 7807229309: -1.5%
Covered Lines: 269
Relevant Lines: 320

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9662382333

Details

  • 27 of 33 (81.82%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 79.242%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dmmf.ts 6 8 75.0%
src/errors.ts 1 3 33.33%
src/hash.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 7807229309: -1.5%
Covered Lines: 269
Relevant Lines: 320

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9662247468

Details

  • 27 of 33 (81.82%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 79.242%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/dmmf.ts 6 8 75.0%
src/errors.ts 1 3 33.33%
src/hash.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 7807229309: -1.5%
Covered Lines: 269
Relevant Lines: 320

💛 - Coveralls

@franky47 franky47 merged commit e10b822 into 47ng:next Jun 25, 2024
3 checks passed
@franky47
Copy link
Member

This has been released in 1.6.0-beta.1, if you want to give it a spin.

@martijn-dev
Copy link
Contributor Author

FYI, we've been using this feature for over a month in production now and we haven't faced any issues so far!

@franky47
Copy link
Member

Thanks for this feedback, I really appreciate it! I'll release a stable version along with #120 once it's merged.

@franky47
Copy link
Member

FYI this PR was just shipped in 1.6.0.

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.

Feature request: Add support to sanitize hashes
3 participants