47ng / prisma-field-encryption

Transparent field-level encryption at rest for Prisma
https://github.com/franky47/prisma-field-encryption-sandbox
MIT License
223 stars 27 forks source link

prisma.$use() deprecated #63

Closed ikbenignace closed 11 months ago

ikbenignace commented 1 year ago

Since 4.16.0, released today, prisma.$use() is deprecated so we should change the way we add the middleware.

franky47 commented 1 year ago

Thanks for the heads-up, looks like query extension components are the way to go.

If anyone wishes to try their hand at a PR, I'll be happy to give them a hand.

ikbenignace commented 1 year ago

I'm trying to figure it out, but it looks way different

ikbenignace commented 1 year ago

I think I will stick to 4.15.0 for a while

franky47 commented 1 year ago

From what I read in the release notes, middlewares with $use are deprecated but still supported for a handful of versions, so it should work fine with 4.16.0 onwards.

That being said, migrating to the extensions mechanism will be required at some point.

ikbenignace commented 1 year ago

I do not know if it has something to do with the $use on 4.16, but I can not do a where call anymore on email, even though I have a emailHash.

Do you have a discord server for discussions?

franky47 commented 1 year ago

That's odd, the integration test suite passes fine with 4.16.0.

Do you have a discord server for discussions?

I don't. I prefer discussions to remain public, localised, and searchable by others that may encounter the same problem.

ikbenignace commented 1 year ago

I understand. I think the middleware works for me, atleast I get the data as decrypted data, but the where function doens't really work anymore, don't know yet what I've changed, but this is off topic.

franky47 commented 1 year ago

If you get to the bottom of it and find something odd that isn't covered by the test suite, please let me know.

ikbenignace commented 1 year ago

Your test case seems fine, I'm just wondering whats wrong with my implementation since every emailHash is different altough it is the same email address

franky47 commented 1 year ago

That sounds like a double application of the middleware, are you using Next.js by any chance? We've had issues with hot module reloading in the past, see https://github.com/47ng/prisma-field-encryption/issues/61.

ikbenignace commented 1 year ago

I'm using Nestjs and I found the issue because #61, you only should use the middleware once. I did this in every service, but since we use dependency injection, it was applied multiple times, which doubles every process. You only have to add the middleware on your PrismaService

ikbenignace commented 1 year ago

Okay forgot this above statement, the data is now unencrypted :p

ikbenignace commented 1 year ago

I don't really know how to use it, because I want to use it on initialization, but for some reason, it will not encrypt anything in the services of each controller, don't know how else or where I should use the middleware

franky47 commented 1 year ago

Usually with other backend frameworks (Next.js, Fastify, Express), I create the Prisma client in a dedicated file where I connect the middlewares before exporting the configured client. Then route handlers / controllers can import the client from there (or use whatever dependency injection mechanism is built into the framework) to perform queries.

See https://github.com/47ng/prisma-field-encryption/blob/next/src/tests/prismaClient.ts

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 1.5.0-beta.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

franky47 commented 1 year ago

I've published an extension interface in 1.5.0-beta.1, could you give it a try with Prisma 4.16.0 please?

Upgrade path:

- import { fieldEncryptionMiddleware } from 'prisma-field-encryption'
+ import { fieldEncryptionExtension } from 'prisma-field-encryption'

- export const prismaClient = new PrismaClient()
+ const globalClient = new PrismaClient()

- prismaClient.$use(
-   fieldEncryptionMiddleware({ /* configuration */ })
- )
+ export const prismaClient = globalClient.$extends(
+   fieldEncryptionExtension({ /* configuration */ })
+ )
ikbenignace commented 1 year ago

It seems to work fine :) Thank you

ikbenignace commented 1 year ago

For some weird reason I'm getting:

Exported variable 'prismaClient' has or is using name 'PrismaPromise_2' from external module "c:/Users/mella/source/repos/api.trackngrow.eu/node_modules/@prisma/client/runtime/index" but cannot be named.ts(4023)
The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.ts(7056)

afbeelding

Seems this is the issue: https://github.com/prisma/prisma/issues/19866 and there is a fix for

Jolg42 commented 1 year ago

@ikbenignace indeed this already has a fix, stay tuned for a Prisma 4.16.1 patch release very soon 🙌🏼

franky47 commented 1 year ago

Thanks @Jolg42 for a quick patch, and thanks @ikbenignace for the feedback!

I got away with an as PrismaClient after the call to $extends for the time being.

gokulkrishh commented 12 months ago

@franky47 any plans to make prisma-field-encryption compatiable with prima@5.0.0?

franky47 commented 12 months ago

From what I can read in their release notes, nothing should be breaking.

The fieldReference feature will not work on encrypted fields however, as it offloads comparisons to the database engine and requires cleartext data.

gokulkrishh commented 12 months ago

@franky47 Cool, then can you update the peer dependencies for this package to latest prisma?

franky47 commented 12 months ago

Done in #66, will update when it gets merged.

I'm still looking at type errors when extending the client before being able to merge it though. Maybe @Jolg42 could chime in on this one:

The main issue is to type the client extension so that the resulting client type is the same as the original, but without using a Prisma import in the library (like Prisma.defineExtension). This is because of people who have use-cases for clients with custom locations (#20).

gokulkrishh commented 12 months ago

@franky47 Awesome looking forward to it. Thanks for doing the version upgrade compatible.

millsp commented 12 months ago

Hey @franky47 we have some documentation that shows how an extension can be published as a separate package. Extensions should always work regardless of the custom location or not when packaged in the standard way. Let me know if this helps. https://www.prisma.io/docs/concepts/components/prisma-client/client-extensions/shared-extensions

I am not sure where you're at in the implementation and what still needs to be resolved, so don't hesitate to throw questions at me, happy to help.

franky47 commented 12 months ago

My main issue with types at the moment has to do with how to test both the middleware implementation and the extension, for which the resulting client type doesn't seem compatible.

Call sites are fine with either individually, but doing a conditional export to test both cases in CI doesn't work without an explicit cast as PrismaClient, because of the intersection of both client types.

A weird TypeScript edge case, not sure how to proceed without duplicating the test suite (which I'd rather avoid). For now, an explicit cast seems like the least of two evils.

millsp commented 11 months ago

Yes, do not duplicate your test suite for this. Extended clients do differ in types because they do not allow for $use or $on to be called, so their types will be incompatible. That is the only difference, and a simple cast should be enough. You'd get a runtime error anyways if you called either $use or $on on an extended client.

franky47 commented 11 months ago

@millsp Thanks for the details.

FYI, somehow this type incompatibility goes wider than $use or $on: any model method (eg: user.create) ends up erroring when the union of extended client | global client is used:

CleanShot 2023-07-19 at 07 45 01@2x

franky47 commented 11 months ago

@gokulkrishh I've released 1.5.0-beta.2 with Prisma 5 as a peer dependency, could you give it a try and let me know if everything works for you?

gokulkrishh commented 11 months ago

@franky47 It seems like everything is work fine for me. Better get an another opinion as well.

franky47 commented 11 months ago

One issue I just found is related to the data migrations, where the as PrismaClient type cast is required (using the import from the generated client location if not the default) to pass to the migration iterators.

@millsp: how would one type a function that takes in an extended Prisma client? From what I can tell, the imported type definitions contains global methods like $on and $use, and even omitting those still causes type errors on call sites.

See https://github.com/franky47/prisma-field-encryption-sandbox/blob/da772d72584aaedcd178fd02ade3b55279339ef0/migrate.ts#L7

millsp commented 11 months ago

Thanks for the report. I just gave it a try, and can confirm what you said. My understanding is that the types are functionally equivalent (between regular client and extended client, as we've tested it), but somehow are reported to be structurally different probably because of some elaborate conditional types applied to generic types.

The reason for that is that extended clients actually mimic the generated client types, and are fully dynamic. It is a brand new approach to how we expose and construct the client types. I expect that at some point in the future we will refactor and leverage the extension mechanisms and fully unify the type logic for extended client vs. regular client, and thus resolve your issue.

Hopefully that makes sense. Also, you're welcome to open an issue on our end.

franky47 commented 11 months ago

@millsp Ok, thanks for looking into it. I've cc'd you into https://github.com/prisma/prisma/issues/20326 to discuss about extended client types there.

github-actions[bot] commented 11 months ago

:tada: This issue has been resolved in version 1.5.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

gokulkrishh commented 11 months ago

Expense.fyi is now using the latest version. @franky47 Thanks.