-
-
Notifications
You must be signed in to change notification settings - Fork 29
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: Add support for filtering encrypted field by list of values #120
base: next
Are you sure you want to change the base?
feat: Add support for filtering encrypted field by list of values #120
Conversation
Pull Request Test Coverage Report for Build 10309403851Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this feature!
A couple of things could do with some changes (field naming and checking for arrays), otherwise it looks good.
@@ -67,7 +67,7 @@ export function encryptOnWrite<Models extends string, Actions extends string>( | |||
models, | |||
function encryptFieldValue({ | |||
fieldConfig, | |||
value: clearText, | |||
value: unHashedValue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Keep the original clearText
name, as the value is not only used for hashing, but also for encryption in most (writing) cases.
src/encryption.ts
Outdated
if (typeof unHashedValue === 'object') { | ||
hash = (unHashedValue as string[]).map((value) => hashString(value, fieldConfigHash)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Use the Array.isArray
function, to make the intent clearer. It also removes the need for the as string[]
type casting.
src/encryption.ts
Outdated
|
||
// Encrypting list values is not yet supported | ||
if (typeof unHashedValue !== 'string') { | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Could we add a log point to this code path, for debugging? It's unlikely we'll hit it with Prisma's current API AFAIK, but it would make it easier to catch issues in the future.
( | ||
// Used for where: { field: in: []} queries | ||
typeof (node as any)?.[specialSubField] === 'string' || | ||
typeof (node as any)?.[specialSubField] === 'object' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Same test here, using Array.isArray
.
- Apply Prettier formatting
Took a stab at implementing a fix for #119.