this diff introduces a hook that will be passed to IdentityServiceClientWrapper. this hook will let us get the data required for DeviceKeyUpload when the login methods are called.
Details
- Reviewers
ashoat kamil tomek - Commits
- rCOMM88478898fc74: [web] hook to get device key upload data
tested in next diff
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
web/account/account-hooks.js | ||
---|---|---|
183–191 ↗ | (On Diff #36119) | there might be a performance impact, but no other downside, i think |
We need to think through prekey and one-time key lifecycle, and make sure the approach is consistent across platforms
web/account/account-hooks.js | ||
---|---|---|
190 ↗ | (On Diff #36133) | Is there a reason we wait until the above line completes before starting this one? |
217 ↗ | (On Diff #36133) | Noticed a discrepancy here. In D10455, you call this function from logInPasswordUser. On native, logInPasswordUser doesn't seem to result in new one-time keys being generated (let me know if I'm missing something). We should think a bit more carefully about when we want to generate new one-time keys. There is a cap to the number of one-time keys that Olm persists (I think around 100?). Generating new one-time keys can result in old, unused ones being deleted, without them being cleared on the identity service. Perhaps what we want here is to check the number of available one-time keys. If there are fewer than N, we can generate new ones. Also note that we currently have a (partially-built?) mechanism in Tunnelbroker that allows one identity client to ask another for new one-time keys. |
web/account/account-hooks.js | ||
---|---|---|
217 ↗ | (On Diff #36133) | Additionally – you mutate the Olm accounts here by generating new one-time keys, but I can't tell where you're persisting the updated Olm accounts |
web/account/account-hooks.js | ||
---|---|---|
190 ↗ | (On Diff #36133) | no |
217 ↗ | (On Diff #36133) |
Now using the same approach that we do on keyserver, which is what you described here |
217 ↗ | (On Diff #36133) | Now persisting the updated olm accounts using setCryptoStore |
web/account/account-hooks.js | ||
---|---|---|
231 ↗ | (On Diff #36344) | Makes sense, as I will be moving it to the database so we might as well handle it then. |
web/account/account-hooks.js | ||
---|---|---|
179–180 ↗ | (On Diff #36344) | Ah okay. I don't really care |
web/account/account-hooks.js | ||
---|---|---|
231 ↗ | (On Diff #36344) | should we create a task? |
web/account/account-hooks.js | ||
---|---|---|
231 ↗ | (On Diff #36344) | nvm i see the task was already created https://linear.app/comm/issue/ENG-6659/make-sure-we-no-longer-use-primaryaccount-name |