Enable key upload to backups where we have the decryption key #4677
Conversation
github-actions
bot
added
the
Z-Community-PR
There was a problem hiding this comment.
Putting this on hold while we try to understand what bug it is trying to fix, per #4676 (comment).
In any case, we'll need to see an integration test which demonstrates the problem.
richvdh
changed the title
There was a problem hiding this comment.
ok, the fix looks like it makes sense, but the main thing this is missing is a regression test.
Please could you take a look at matrix-js-sdk/spec/integ/crypto/megolm-backup.spec.ts. In there there are some existing tests for checkKeyBackupAndEnable: please could you take a look at them and see if you can add another which covers your usecase.
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
| // We are lacking a way to signal that the secret has been received, so we wait a bit.. | ||
| jest.useRealTimers(); | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, 500); | ||
| }); | ||
| jest.useFakeTimers({ doNotFake: ["queueMicrotask"] }); | ||
|
|
||
| // the backup secret should not be cached | ||
| const cachedKey = await aliceClient.getCrypto()!.getSessionBackupPrivateKey(); | ||
| expect(cachedKey).toBeNull(); |
There was a problem hiding this comment.
It might be nice to factor the 5 copies of this code out to a helper function, but I'll not insist
| @@ -1338,7 +1383,7 @@ describe("verification", () => { | |||
| async function sendBackupGossipAndExpectVersion( | |||
| requestId: string, | |||
| secret: string, | |||
| expectBackup: KeyBackupInfo, | |||
| expectBackup?: KeyBackupInfo | { errcode?: string; error?: string }, | |||
There was a problem hiding this comment.
could you document the behavior of expectBackup? What does it mean if it is undefined, what if it has an errcode, etc?
Co-authored-by: Richard van der Hoff <[email protected]>
| errcode: "M_NOT_FOUND", | ||
| error: "No backup found", | ||
| }, | ||
| 401, |
| new MatrixError( | ||
| { | ||
| errcode: "M_NOT_FOUND", | ||
| error: "No backup found", | ||
| }, | ||
| 401, | ||
| ), |
There was a problem hiding this comment.
this could be more neatly written:
| new MatrixError( | |
| { | |
| errcode: "M_NOT_FOUND", | |
| error: "No backup found", | |
| }, | |
| 401, | |
| ), | |
| new MatrixError({ errcode: "M_NOT_FOUND", error: "No backup found" }, 404), |
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Fixes #4676
Signed-off-by: Ajay Bura [email protected]
Checklist
public/exportedsymbols have accurate TSDoc documentation.