Skip to content

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

Merged
franky47 merged 4 commits into
47ng:nextfrom
martijn-dev:feature/sanitize-hash
Jun 25, 2024
Merged

feat(normalize-hash): Add support for normalizing hashes#110
franky47 merged 4 commits into
47ng:nextfrom
martijn-dev:feature/sanitize-hash

Conversation

@martijn-dev

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

Copy link
Copy Markdown
Contributor

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

@franky47 franky47 left a comment

Copy link
Copy Markdown
Member

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.

Comment thread src/dmmf.ts Outdated
Comment thread src/hash.ts
Comment thread README.md
Comment thread src/dmmf.ts
Comment thread README.md Outdated
Comment thread README.md Outdated
@coveralls

coveralls commented Jun 25, 2024

Copy link
Copy Markdown

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

coveralls commented Jun 25, 2024

Copy link
Copy Markdown

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

@franky47 franky47 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@coveralls

coveralls commented Jun 25, 2024

Copy link
Copy Markdown

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

coveralls commented Jun 25, 2024

Copy link
Copy Markdown

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

coveralls commented Jun 25, 2024

Copy link
Copy Markdown

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
@franky47

Copy link
Copy Markdown
Member

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

@martijn-dev

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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