-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[mob] Add crypto abstraction layer (ente_crypto_api) #8628
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
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fefd6306df
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| import "dart:io"; | ||
|
|
||
| import "package:ente_crypto_dart/ente_crypto_dart.dart"; | ||
| import "package:ente_crypto_api/ente_crypto_api.dart"; |
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.
Replace cryptoPwHash/sodium usage after API switch
After switching this file to ente_crypto_api, it still calls the old top-level cryptoPwHash and sodium symbols inside confirmPinAuth. Those symbols are not exported by the new package, so the lock_screen package will fail to compile with undefined identifiers unless the call is migrated to CryptoUtil.cryptoPwHash and the CryptoUtil.pwhash* getters.
Useful? React with 👍 / 👎.
| import 'package:dio/dio.dart'; | ||
| import 'package:ente_accounts/services/user_service.dart'; | ||
| import 'package:ente_crypto_dart/ente_crypto_dart.dart'; | ||
| import 'package:ente_crypto_api/ente_crypto_api.dart'; |
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.
Rename encryptFileWithMD5 call to new API
This file now imports ente_crypto_api, but later invokes CryptoUtil.encryptFileWithMD5. The new API only exposes encryptFileWithMd5, so the locker app will not compile after this change; rename the call (or add a compatibility wrapper) to keep uploads building.
Useful? React with 👍 / 👎.
Introduce a crypto abstraction to allow swapping implementations without changing consumer code. This enables future support for different crypto backends (Rust FFI, platform plugins, etc). - Add ente_crypto_api package with CryptoApi interface and CryptoUtil façade - Add ente_crypto_dart_adapter wrapping existing ente_crypto_dart - Update all packages/apps to use the new abstraction - Register adapter at app startup before any crypto operations
fefd630 to
656152f
Compare
|
Found a couple of issues that need fixing: 1. 2.
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5ea90862b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| import 'package:ente_crypto_dart/src/models/derived_key_result.dart' | ||
| as dart_models; | ||
| import 'package:ente_crypto_dart/src/models/encryption_result.dart' | ||
| as dart_models; |
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.
Use distinct prefixes for dart_models imports
Dart does not allow the same import prefix to be assigned to multiple libraries. Here both derived_key_result.dart and encryption_result.dart are imported as dart_models, which triggers a compile-time error (“prefix 'dart_models' is already defined”). As a result, any app depending on ente_crypto_dart_adapter (e.g., auth/locker) will fail to build. Rename one prefix or drop the unused import.
Useful? React with 👍 / 👎.
Head branch was pushed to by a user without write access
Summary
Introduce a crypto abstraction layer to allow swapping implementations without changing consumer code. This enables future support for different crypto backends (Rust FFI, platform plugins, etc).
Changes
ente_crypto_apipackage withCryptoApiinterface andCryptoUtilfaçadeente_crypto_dart_adapterwrapping existingente_crypto_dartArchitecture
Adding a New Adapter
To add a new adapter (e.g., Rust FFI):
CryptoApimain.dartto register the new adapter