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

Add encrypted SQLCipher WatermelonDB JSI #1635

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ororsatti
Copy link

@ororsatti ororsatti commented Jul 31, 2023

ideally i'd like to avoid adding to the non-JSI version since its going to be less and less supported as we progress with JSI in RN.

TODO:

  • Add SQLCipher to Android
  • Add an option to select sqlite build (sqlite3/sqlCipher - this seem to be a requirement on the last PR trying to add sqlcipher) on Android
  • ADD SQLCipher to iOS
  • Add an option to select sqlite build (sqlite3/sqlCipher - this seem to be a requirement on the last PR trying to add sqlcipher) iOS
  • Change API to accept passphase.
  • Add sqlcipher amalgamation to nozbe workspace (same method as sqlite and simdjson) @radex
  • Fix iOS tests

Changes:
- add a flag if using encrypted DB
- install openssl if using encrypted db
- add pathes to cipher amalgamation based on the flags.
- add openSSL lib
@ororsatti ororsatti changed the title Add sqlcipher to jsi Add encrypted SQLite to jsi Aug 1, 2023
@ororsatti ororsatti changed the title Add encrypted SQLite to jsi Add encrypted SQLCipher WatermelonDB JSI Aug 2, 2023
@ororsatti
Copy link
Author

@radex I can not seem to run the ios/android tests properly, any chance you help me with it?

@killerchip
Copy link

killerchip commented Jan 17, 2024

Hey guys,

Thanks for your great work on this, and thanks for the great library

Is this PR still alive? Is there still interest in Nozbe in adding encryption to the DB?
We are also looking into using WatermelonDB with encryption on native (Android mainly). But C++ is currently out of my league. Do you know how I can help?

I was able to make something work based on this PR #907, but of course, it does not support JSI.

@ororsatti, does your PR work? I just did a test, but it seems that the DB is not getting encrypted (Android).

@ororsatti
Copy link
Author

Hey guys,

Thanks for your great work on this, and thanks for the great library

Is this PR still alive? Is there still interest in Nozbe in adding encryption to the DB?
We are also looking into using WatermelonDB with encryption on native (Android mainly). But C++ is currently out of my league. Do you know how I can help?

I was able to make something work based on this PR #907, but of course, it does not support JSI.

@ororsatti, does your PR work? I just did a test, but it seems that the DB is not getting encrypted (Android).

I'm using it in several production apps, so yep.
I might be able to help talk to me this evening:
[email protected]

@killerchip
Copy link

I'm using it in several production apps, so yep. I might be able to help talk to me this evening: [email protected]

@ororsatti thank you very much for your valuable help on this. Indeed your PR works as expected.
As we discussed it would be awesome if this could turn into a separate WMDB adapter.

@radex would it be preferable to make it into a separate adapter? Or just as an option to the base SQLite adapter as is with this PR?

@ororsatti
Copy link
Author

I'm using it in several production apps, so yep. I might be able to help talk to me this evening: [email protected]

@ororsatti thank you very much for your valuable help on this. Indeed your PR works as expected.
As we discussed it would be awesome if this could turn into a separate WMDB adapter.

@radex would it be preferable to make it into a separate adapter? Or just as an option to the base SQLite adapter as is with this PR?

I don't think making it an adapter is the right decision.
An adapter is the middle layer, what stands between your app, and the underlying SQLite.
Since I am not changing the platform, nor the api, I don't see a reason to change the adapter here.
It should be as it is, a flag on which you raise if you want to use it, and pass password if you desire.

@ororsatti
Copy link
Author

I'm using it in several production apps, so yep. I might be able to help talk to me this evening: [email protected]

@ororsatti thank you very much for your valuable help on this. Indeed your PR works as expected.
As we discussed it would be awesome if this could turn into a separate WMDB adapter.

@radex would it be preferable to make it into a separate adapter? Or just as an option to the base SQLite adapter as is with this PR?

You can think about it like it's a car.
If you change the engine, but as long as it has the same gear, i.e the same API, you don't have to learn manual.
Same here, the api is the same, so no need to change the adapter.

@dlogvin
Copy link

dlogvin commented Oct 26, 2024

Hey! just wanted to check if this PR is still alive

@ororsatti
Copy link
Author

Hey! just wanted to check if this PR is still alive

im still maintaining it from time to time, but i don't think the lib owner merges community PRs...

@dlogvin
Copy link

dlogvin commented Nov 6, 2024

Hey! just wanted to check if this PR is still alive

im still maintaining it from time to time, but i don't think the lib owner merges community PRs...

That's quite sad. Adding encryption definitely is important, if not a requirement nowadays. Then I guess we have to fork 🫡

When I was looking at this library, I was hoping it had some sort of encryption capabilities, but I quickly left it as it came to my understanding that such a thing is probably not coming in the near future for this lib.

@ororsatti
Copy link
Author

ororsatti commented Nov 7, 2024

Hey! just wanted to check if this PR is still alive

im still maintaining it from time to time, but i don't think the lib owner merges community PRs...

That's quite sad. Adding encryption definitely is important, if not a requirement nowadays. Then I guess we have to fork 🫡

When I was looking at this library, I was hoping it had some sort of encryption capabilities, but I quickly left it as it came to my understanding that such a thing is probably not coming in the near future for this lib.

well this pr works, this pr in particular was used by me in production multiple times, and by a few more of the community... you can just use it. the conflicts doesn't seem that bad tbh

@kmye
Copy link

kmye commented Nov 9, 2024

i have tried to use the codes in this PR but there are build errors

e.g.

/mobile/ios/Pods/SQLCipher/sqlite3.h:1767:8: error: redefinition of 'sqlite3_mem_methods'
1767 | struct sqlite3_mem_methods {
|        ^
    In file included from /node_modules/@nozbe/watermelondb/native/ios/WatermelonDB/DatabasePlatformIOS.mm:1:
    In file included from /node_modules/@nozbe/watermelondb/native/shared/DatabasePlatform.h:5:
    In file included from /node_modules/@nozbe/watermelondb/native/shared/Database.h:7:

this seems to be caused by conflicting sqlite pod in my app, my app also include other libraries which includes sqlite and these could hgave caused the redefinition error. anyone has a solution this issue?

@rtripplanningbiz
Copy link

I'm using it in several production apps, so yep. I might be able to help talk to me this evening: [email protected]

@ororsatti thank you very much for your valuable help on this. Indeed your PR works as expected. As we discussed it would be awesome if this could turn into a separate WMDB adapter.

@radex would it be preferable to make it into a separate adapter? Or just as an option to the base SQLite adapter as is with this PR?

Hi, how did you get it to encrypt in android? I haven't tried on IOS since I don't need it there. Android I am able to build but the database is not encrypted. Thanks!

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.

5 participants