Move Olm account to IndexedDB #579
Merged
Conversation
This file contains 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
Wraps all access to the account in a transaction so any updates done to the account must be done in the same transaction, making the update atomic between tabs. Doesn't do any migration from localstorage yet.
dbkr
changed the title
richvdh
reviewed
Nov 21, 2017
src/crypto/OlmDevice.js
Outdated
| account.unpickle(this._pickleKey, pickledAccount); | ||
| return func(account); | ||
| const accountTxn = await this._cryptoStore.endToEndAccountTransaction(); | ||
| // Olm has a limited stack size so we must tightly control the number of |
Rather than a promise which relies on the caller's promise handler code being run in the same tick which is not guaranteed.
dbkr
changed the title
To avoid throwing away all the data for anyone running firefox in one of the modes where indexedDB is broken.
dbkr
changed the title
richvdh
reviewed
Nov 22, 2017
src/crypto/OlmDevice.js
Outdated
| } | ||
| async function _initialise_account(sessionStore, cryptoStore, pickleKey, account) { | ||
| let removeFromSessionStore = false; | ||
| await cryptoStore.endToEndAccountTransaction((accountData, save) => { |
There was a problem hiding this comment.
can we s/accountData/pickledAccount/ or somesuch? I'm finding myself a bit confused.
This was entirely unnecessary and hopefully make things a bit simpler to understand and has fewer asyncs flying around.
richvdh
reviewed
Nov 22, 2017
src/crypto/OlmDevice.js
Outdated
| @@ -182,9 +213,9 @@ OlmDevice.prototype._getAccount = function(func) { | |||
| * @param {OlmAccount} account | |||
| * @private | |||
| */ | |||
| OlmDevice.prototype._saveAccount = function(account) { | |||
| OlmDevice.prototype._saveAccount = async function(account) { | |||
There was a problem hiding this comment.
this is presumably no longer used?
src/crypto/OlmDevice.js
Outdated
| * extract our OlmAccount from the session store and call the given function | ||
| * extract our OlmAccount from the crypto store and call the given function | ||
| * with the account object and a 'save' function which returns a promise. | ||
| * The function will not be awaited upon and the save function must be |
There was a problem hiding this comment.
this is maybe a bit misleading. The function will be awaited upon, by virtue of any promise it returns being returned by this async function - so the promise returned by func must resolve before the promise returned by _getAccount will resolve.
It would probably be clearer to say "The account will be freed as soon as func returns - even if func returns a promise".
| /** | ||
| * Load the end to end account for the logged-in user. Once the account | ||
| * is retrieved, the given function is executed and passed the base64 | ||
| * encoded account string and a method for saving the account string |
There was a problem hiding this comment.
s/base64 encoded/pickled/? And s/account string/pickle/?
Might be more consistent. just a thought.
| * encoded account string and a method for saving the account string | ||
| * back to the database. This allows the account to be read and writen | ||
| * atomically. | ||
| * @param {func} func Function called with the account data and a save function |
There was a problem hiding this comment.
@param {function(string, function())} func ....
would be the correct annotation here I think
| * back to the database. This allows the account to be read and writen | ||
| * atomically. | ||
| * @param {func} func Function called with the account data and a save function | ||
| * @return {Promise} Resolves with the return value of the function once |
| * @param {func} func Function called with the account data and a save function | ||
| * @return {Promise} Resolves with the return value of the function once | ||
| * the transaction is complete (ie. once data is written back if the | ||
| * save function is called. |
richvdh
reviewed
Nov 23, 2017
| * @param {func} func Function called with the account data and a save function | ||
| * @return {Promise} Resolves with the return value of the function once | ||
| * @param {function(string, function())} func Function called with the | ||
| * account data and a save function |
krombel
added a commit
to krombel/matrix-js-sdk
that referenced
this pull request
Mar 21, 2018
[Full Changelog](matrix-org/matrix-js-sdk@v0.9.2...v0.10.0-rc.1) * Fix duplicated state events in timeline from peek [\matrix-org#630](matrix-org#630) * Create indexeddb worker when starting the store [\matrix-org#627](matrix-org#627) * Fix indexeddb logging [\matrix-org#626](matrix-org#626) * Don't do /keys/changes on incremental sync [\matrix-org#625](matrix-org#625) * Don't mark devicelist dirty unnecessarily [\matrix-org#623](matrix-org#623) * Cache the joined member count for a room state [\matrix-org#619](matrix-org#619) * Fix JS doc [\matrix-org#618](matrix-org#618) * Precompute push actions for state events [\matrix-org#617](matrix-org#617) * Fix bug where global "Never send to unverified..." is ignored [\matrix-org#616](matrix-org#616) * Intern legacy top-level 'membership' field [\matrix-org#615](matrix-org#615) * Don't synthesize RR for m.room.redaction as causes the RR to go missing. [\matrix-org#598](matrix-org#598) * Make Events create Dates on demand [\matrix-org#613](matrix-org#613) * Stop cloning events when adding to state [\matrix-org#612](matrix-org#612) * De-dup code: use the initialiseState function [\matrix-org#611](matrix-org#611) * Create sentinel members on-demand [\matrix-org#610](matrix-org#610) * Some more doc on how sentinels work [\matrix-org#609](matrix-org#609) * Migrate room encryption store to crypto store [\matrix-org#597](matrix-org#597) * add parameter to getIdentityServerUrl to strip the protocol for invites [\matrix-org#600](matrix-org#600) * Move Device Tracking Data to Crypto Store [\matrix-org#594](matrix-org#594) * Optimise pushprocessor [\matrix-org#591](matrix-org#591) * Set event error before emitting [\matrix-org#592](matrix-org#592) * Add event type for stickers [WIP] [\matrix-org#590](matrix-org#590) * Migrate inbound sessions to cryptostore [\matrix-org#587](matrix-org#587) * Disambiguate names if they contain an mxid [\matrix-org#588](matrix-org#588) * Check for sessions in indexeddb before migrating [\matrix-org#585](matrix-org#585) * Emit an event for crypto store migration [\matrix-org#586](matrix-org#586) * Supporting fixes For making UnknownDeviceDialog not pop up automatically [\matrix-org#575](matrix-org#575) * Move sessions to the crypto store [\matrix-org#584](matrix-org#584) * Change crypto store transaction API [\matrix-org#582](matrix-org#582) * Add some missed copyright notices [\matrix-org#581](matrix-org#581) * Move Olm account to IndexedDB [\matrix-org#579](matrix-org#579) * Fix logging of DecryptionErrors to be more useful [\matrix-org#580](matrix-org#580) * [BREAKING] Change the behaviour of the unverfied devices blacklist flag [\matrix-org#568](matrix-org#568) * Support set_presence=offline for syncing [\matrix-org#557](matrix-org#557) * Consider cases where the sender may not redact their own event [\matrix-org#556](matrix-org#556)
krombel
added a commit
to krombel/matrix-js-sdk
that referenced
this pull request
Apr 18, 2018
[Full Changelog](matrix-org/matrix-js-sdk@v0.9.2...v0.10.0-rc.1) * Fix duplicated state events in timeline from peek [\matrix-org#630](matrix-org#630) * Create indexeddb worker when starting the store [\matrix-org#627](matrix-org#627) * Fix indexeddb logging [\matrix-org#626](matrix-org#626) * Don't do /keys/changes on incremental sync [\matrix-org#625](matrix-org#625) * Don't mark devicelist dirty unnecessarily [\matrix-org#623](matrix-org#623) * Cache the joined member count for a room state [\matrix-org#619](matrix-org#619) * Fix JS doc [\matrix-org#618](matrix-org#618) * Precompute push actions for state events [\matrix-org#617](matrix-org#617) * Fix bug where global "Never send to unverified..." is ignored [\matrix-org#616](matrix-org#616) * Intern legacy top-level 'membership' field [\matrix-org#615](matrix-org#615) * Don't synthesize RR for m.room.redaction as causes the RR to go missing. [\matrix-org#598](matrix-org#598) * Make Events create Dates on demand [\matrix-org#613](matrix-org#613) * Stop cloning events when adding to state [\matrix-org#612](matrix-org#612) * De-dup code: use the initialiseState function [\matrix-org#611](matrix-org#611) * Create sentinel members on-demand [\matrix-org#610](matrix-org#610) * Some more doc on how sentinels work [\matrix-org#609](matrix-org#609) * Migrate room encryption store to crypto store [\matrix-org#597](matrix-org#597) * add parameter to getIdentityServerUrl to strip the protocol for invites [\matrix-org#600](matrix-org#600) * Move Device Tracking Data to Crypto Store [\matrix-org#594](matrix-org#594) * Optimise pushprocessor [\matrix-org#591](matrix-org#591) * Set event error before emitting [\matrix-org#592](matrix-org#592) * Add event type for stickers [WIP] [\matrix-org#590](matrix-org#590) * Migrate inbound sessions to cryptostore [\matrix-org#587](matrix-org#587) * Disambiguate names if they contain an mxid [\matrix-org#588](matrix-org#588) * Check for sessions in indexeddb before migrating [\matrix-org#585](matrix-org#585) * Emit an event for crypto store migration [\matrix-org#586](matrix-org#586) * Supporting fixes For making UnknownDeviceDialog not pop up automatically [\matrix-org#575](matrix-org#575) * Move sessions to the crypto store [\matrix-org#584](matrix-org#584) * Change crypto store transaction API [\matrix-org#582](matrix-org#582) * Add some missed copyright notices [\matrix-org#581](matrix-org#581) * Move Olm account to IndexedDB [\matrix-org#579](matrix-org#579) * Fix logging of DecryptionErrors to be more useful [\matrix-org#580](matrix-org#580) * [BREAKING] Change the behaviour of the unverfied devices blacklist flag [\matrix-org#568](matrix-org#568) * Support set_presence=offline for syncing [\matrix-org#557](matrix-org#557) * Consider cases where the sender may not redact their own event [\matrix-org#556](matrix-org#556)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Migrates the Olm account storage from the session store to the crypto store, backed by indexedDB. This allows us to lock access to it in a transaction, so multiple tabs can't overwrite each other's data.
Also adds a localstorage-backed crypto store to prevent people using Firefox in ones of its modes where IndexedDB support is broken from losing all their e2e data. It still uses the in-memory store for key requests as before, but saves the Olm account to localstorage. We may want to kick the app to warn the user that they're running in a degraded mode?
Olm accounts in the old session store are migrated into the crypto store seamlessly.