Next Release - #3782
Merged
Merged
Conversation
## 🎯 Goal Let integrators encrypt the offline database at rest. The offline cache stores channels, messages, members, drafts and reminders, and right now we write all of it as plaintext `SQLite`. It's opt-in. Apps that don't pass the new prop behave exactly as they do today. One thing to know up front, since it shapes the rest of the PR: `op-sqlite` accepts an `encryptionKey` on a build without `SQLCipher` and then ignores it. You get a plaintext database and no error at any layer. So part of this change is detecting that and refusing to open the database, instead of passing the key along and assuming it was used. Accompanying docs PR: GetStream/docs-content#1521 ## 🛠 Implementation details ### API `Chat` takes one new prop: ```tsx <Chat client={client} enableOfflineSupport getEncryptionKey={getEncryptionKey}> ``` `getEncryptionKey?: () => Promise<string | undefined>` runs once per database open, so once per launch and again after a sign-out. Its result is passed to `SQLCipher` through `op-sqlite`. `SqliteClientError` and `SqliteClientErrorCode` are exported too. ### We throw instead of recovering When the database can't be opened with the encryption that was asked for, `Chat` throws a `SqliteClientError` from render and the integrator's error boundary handles it. We don't fall back to plaintext, we don't switch offline support off, and we don't delete anything. The reason is that all of those recoveries have a security consequence and there's no default that's right for everyone. Falling back to plaintext defeats the point of the feature and nothing tells you it happened. Dropping the cache decides a compliance question for the integrator. Deleting the file throws away offline actions that are still queued. We also can't tell "the Keystore isn't unlocked yet, try again shortly" from "something is wrong here, sign this device out". So we detect the failure and classify it, and the app decides what to do about it. ### Scenarios | Scenario | What it means | What the SDK does | Recommended recovery | | ------------------------------------------------------- | ------------------------------------------------------------ | ---------------------------------------------- | ------------------------------------------------------------------ | | No `getEncryptionKey` passed | Encryption not requested | Opens plaintext, same as today | n/a | | Key supplied, fresh install | Nothing on disk yet | Creates the database encrypted with that key | n/a | | Key supplied, plaintext database already on disk | Integrator is turning encryption on for an existing install | Throws `OFFLINE_DB_UNREADABLE` | Delete the database, remount `Chat` | | Key differs from the one the database was written with | Key rotated, or read from the wrong place | Throws `OFFLINE_DB_UNREADABLE` | Delete the database, remount `Chat` | | `getEncryptionKey` removed, encrypted database on disk | Integrator is turning encryption off again | Throws `OFFLINE_DB_UNREADABLE` | Delete the database, remount `Chat` | | `getEncryptionKey` throws | Key isn't available yet, e.g. Keystore still locked | Throws `ENCRYPTION_KEY_UNAVAILABLE` | Remount to retry, e.g. on next app foreground | | `getEncryptionKey` resolves `undefined` | Same as above | Throws `ENCRYPTION_KEY_UNAVAILABLE` | Remount to retry, e.g. on next app foreground | | Key supplied, native build has no `SQLCipher` | The key would be ignored and the database left plaintext | Throws `SQLCIPHER_BUILD_MISSING`, doesn't open | Not fixable at runtime, remount with `enableOfflineSupport={false}` | | Database file corrupted | Nothing to do with encryption | Throws `OFFLINE_DB_UNREADABLE` | Delete the database, remount `Chat` | Two of those rows need a closer look in review. `OFFLINE_DB_UNREADABLE` is not gated on `getEncryptionKey` being set, and that's on purpose, because of the "turning encryption off again" row. If we only threw it when a key was supplied, an integrator removing the prop would get a blank screen instead of an error they can recover from. I hit that on device. The last row is why the boundary is useful even for apps that never use encryption. A corrupted database gives you the same code, so anything using `enableOfflineSupport` can end up there. ### Where the error comes from `AbstractOfflineDB.init` in the LLC catches whatever `initializeDB` throws and doesn't re-throw it, so a caller can't find out why initialisation failed. I left that alone, because changing it would tie this PR to an LLC release. `OfflineDB` stores the reason on the instance on the way out instead, and the new hook reads it back once `init` has settled. No LLC changes needed for this. - `SqliteClient` resolves the key, opens through `SQLCipher` and maps failures onto the codes above. It also gets `preflightEncryption()`, which runs before `setOfflineDBApi`. Without that ordering the client attaches a database that's already dead, and the unguarded `await this.offlineDb.upsertChannels(...)` inside `queryChannels` rejects. You end up on a loading screen that never resolves. - `useInitializeOfflineDb()` is new and does preflight, attach, init and raise, with the init options behind an `options` param. It's pulled out of `Chat`, which loses 72 lines. - `OfflineDB` records `initializationError` and re-throws, so `init` still marks the database uninitialised. ## 🎨 UI Changes ## 🧪 Testing <!-- Explain how this change can be tested (or why it can't be tested) --> ## ☑️ Checklist - [x] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [x] PR targets the `develop` branch - [x] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android
Contributor
SDK Size
|
Contributor
|
🎉 This PR is included in version 9.8.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Goal
Let integrators encrypt the offline database at rest. The offline cache stores channels, messages, members, drafts and reminders, and right now we write all of it as plaintext
SQLite.It's opt-in. Apps that don't pass the new prop behave exactly as they do today.
One thing to know up front, since it shapes the rest of the PR:
op-sqliteaccepts anencryptionKeyon a build withoutSQLCipherand then ignores it. You get a plaintext database and no error at any layer. So part of this change is detecting that and refusing to open the database, instead of passing the key along and assuming it was used.Accompanying docs PR:
https://github.com/GetStream/docs-content/pull/1521
🛠 Implementation details
API
Chattakes one new prop:getEncryptionKey?: () => Promise<string | undefined>runs once per database open, so once per launch and again after a sign-out. Its result is passed toSQLCipherthroughop-sqlite.SqliteClientErrorandSqliteClientErrorCodeare exported too.We throw instead of recovering
When the database can't be opened with the encryption that was asked for,
Chatthrows aSqliteClientErrorfrom render and the integrator's error boundary handles it. We don't fall back to plaintext, we don't switch offline support off, and we don't delete anything.The reason is that all of those recoveries have a security consequence and there's no default that's right for everyone. Falling back to plaintext defeats the point of the feature and nothing tells you it happened. Dropping the cache decides a compliance question for the integrator. Deleting the file throws away offline actions that are still queued. We also can't tell "the Keystore isn't unlocked yet, try again shortly" from "something is wrong here, sign this device out". So we detect the failure and classify it, and the app decides what to do about it.
Scenarios
| Scenario | What it means | What the SDK does | Recommended recovery | | ------------------------------------------------------- | ------------------------------------------------------------ | ---------------------------------------------- |
------------------------------------------------------------------ | | No
getEncryptionKeypassed | Encryption not requested | Opens plaintext, same as today | n/a || Key supplied, fresh install | Nothing on disk yet | Creates the database encrypted with that key | n/a |
| Key supplied, plaintext database already on disk | Integrator is turning encryption on for an existing install | Throws
OFFLINE_DB_UNREADABLE| Delete the database, remountChat| | Key differs from the one the database was written with | Key rotated, or read from the wrong place | ThrowsOFFLINE_DB_UNREADABLE| Delete the database, remountChat||
getEncryptionKeyremoved, encrypted database on disk | Integrator is turning encryption off again | ThrowsOFFLINE_DB_UNREADABLE| Delete the database, remountChat||
getEncryptionKeythrows | Key isn't available yet, e.g. Keystore still locked | ThrowsENCRYPTION_KEY_UNAVAILABLE| Remount to retry, e.g. on next app foreground ||
getEncryptionKeyresolvesundefined| Same as above | ThrowsENCRYPTION_KEY_UNAVAILABLE| Remount to retry, e.g. on next app foreground || Key supplied, native build has no
SQLCipher| The key would be ignored and the database left plaintext | ThrowsSQLCIPHER_BUILD_MISSING, doesn't open | Not fixable at runtime, remount withenableOfflineSupport={false}|| Database file corrupted | Nothing to do with encryption | Throws
OFFLINE_DB_UNREADABLE| Delete the database, remountChat|Two of those rows need a closer look in review.
OFFLINE_DB_UNREADABLEis not gated ongetEncryptionKeybeing set, and that's on purpose, because of the "turning encryption off again" row. If we only threw it when a key was supplied, an integrator removing the prop would get a blank screen instead of an error they can recover from. I hit that on device.The last row is why the boundary is useful even for apps that never use encryption. A corrupted database gives you the same code, so anything using
enableOfflineSupportcan end up there.Where the error comes from
AbstractOfflineDB.initin the LLC catches whateverinitializeDBthrows and doesn't re-throw it, so a caller can't find out why initialisation failed. I left that alone, because changing it would tie this PR to an LLC release.OfflineDBstores the reason on the instance on the way out instead, and the new hook reads it back onceinithas settled. No LLC changes needed for this.SqliteClientresolves the key, opens throughSQLCipherand maps failures onto the codes above. It also getspreflightEncryption(), which runs beforesetOfflineDBApi. Without that ordering the client attaches a database that's already dead, and the unguardedawait this.offlineDb.upsertChannels(...)insidequeryChannelsrejects. You end up on a loading screen that never resolves.useInitializeOfflineDb()is new and does preflight, attach, init and raise, with the init options behind anoptionsparam. It's pulled out ofChat, which loses 72 lines.OfflineDBrecordsinitializationErrorand re-throws, soinitstill marks the database uninitialised.🎨 UI Changes
🧪 Testing
☑️ Checklist
developbranch🎯 Goal
🛠 Implementation details
🎨 UI Changes
iOS
Android
🧪 Testing
☑️ Checklist
developbranch