Wire up the "Forgot recovery key" button for the "Key storage out of sync" toast #29138
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
florianduros
requested changes
Jan 31, 2025
Comment on lines
56
to
66
| const recheckSetupRequired = useCallback(() => { | ||
| (async () => { | ||
| const crypto = matrixClient.getCrypto()!; | ||
| const isCrossSigningReady = await crypto.isCrossSigningReady(); | ||
| if (isCrossSigningReady) { | ||
| setState("main"); | ||
| } else { | ||
| setState("set_up_encryption"); | ||
| } | ||
| })(); | ||
| }, [matrixClient]); |
There was a problem hiding this comment.
I'm in favour to use a custom hook like useSetUpEncryptionRequired before. It helps the readability of the component imo
There was a problem hiding this comment.
Something like this? I find the custom hook a bit awkward in this scenario since you have to pass a lot of functions around, but I don't mind too much.
src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
60
to
64
| if (isCrossSigningReady) { | ||
| setState("main"); | ||
| } else { | ||
| setState("set_up_encryption"); | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (isCrossSigningReady) { | |
| setState("main"); | |
| } else { | |
| setState("set_up_encryption"); | |
| } | |
| if (isCrossSigningReady) setState("main"); | |
| else setState("set_up_encryption"); |
There was a problem hiding this comment.
I'd really rather not if at all possible: I find these line-ifs split over multiple lines very confusing, and I think our code style permits line ifs only if they're one single line.
src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsx
Outdated
Show resolved
Hide resolved
playwright/e2e/crypto/toasts.spec.ts
Outdated
| import { registerAccountMas } from "../oidc"; | ||
| import { deleteCachedSecrets } from "./utils"; | ||
|
|
||
| test.use(masHomeserver); |
playwright/e2e/crypto/toasts.spec.ts
Outdated
Comment on lines
20
to
44
| test.beforeEach(async ({ page, app, mailpitClient }) => { | ||
| await page.goto("/#/login"); | ||
| await page.getByRole("button", { name: "Continue" }).click(); | ||
| await registerAccountMas(page, mailpitClient, "alice", "[email protected]", "Pa$sW0rD!"); | ||
|
|
||
| await expect(page.getByRole("heading", { level: 1, name: "Welcome alice" })).toBeVisible(); | ||
|
|
||
| // We won't be prompted for crypto setup unless we have na e2e room, so make one | ||
| await page.getByRole("button", { name: "Add room" }).click(); | ||
| await page.getByRole("menuitem", { name: "New room" }).click(); | ||
| await page.getByRole("textbox", { name: "Name" }).fill("Test room"); | ||
| await page.getByRole("button", { name: "Create room" }).click(); | ||
|
|
||
| // Now set up recovery (otherwise we'll delete the only copy of the secret) | ||
| await page.getByLabel("User menu").click(); | ||
| await page.getByLabel("All settings").click(); | ||
| await page.getByRole("tab", { name: "Encryption" }).click(); | ||
| await page.getByRole("button", { name: "Set up recovery" }).click(); | ||
| await page.getByRole("button", { name: "Continue" }).click(); | ||
| recoveryKey = await page.getByTestId("recoveryKey").textContent(); | ||
| await page.getByRole("button", { name: "Continue" }).click(); | ||
| await page.getByRole("textbox", { name: "Enter recovery key" }).fill(recoveryKey); | ||
| await page.getByRole("button", { name: "Finish set up" }).click(); | ||
|
|
||
| await expect(page.getByRole("button", { name: "Change recovery key" })).toBeVisible(); |
There was a problem hiding this comment.
Can't we use crypto/utils#createBot to create a second session, bootstrap cross singing and create a recovery key?
crypto/utils#verifySession can be used to verify the current session too
There was a problem hiding this comment.
It took a bit but I think this is working with the bot client now.
uhoreg
approved these changes
Feb 1, 2025
Co-authored-by: Florian Duros <[email protected]>
…tofsync_forgotpassword
florianduros
approved these changes
Feb 4, 2025
dbkr
added
the
backport staging
RiotRobot
pushed a commit
that referenced
this pull request
Feb 4, 2025
…sync" toast (#29138) * Wire up the "Forgot recovery key" button for the "Key storage out of sync" toast * Unused import & fix test * Test 'forgot' variant * Fix dependencies * Add more toast tests * Unused import * Test initialState in Encryption Tab * Let's see if github has any more luck running this test than me * Working playwright test with screenshot * year * Convert playwright test to use the bot client * Disambiguate Co-authored-by: Florian Duros <[email protected]> * Add doc & do other part of rename * Split out into custom hook * Fix tests --------- Co-authored-by: Florian Duros <[email protected]> (cherry picked from commit 9657d39)
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.
Fixes #29118
Checklist
public/exportedsymbols have accurate TSDoc documentation.