Wire up the "Forgot recovery key" button for the "Key storage out of sync" toast by dbkr · Pull Request #29138 · element-hq/element-web

dbkr · 2025-01-29T18:46:53Z

Fixes #29118 by making the 'forgot recovery key' button take you into settings to the flow to reset your recovery key.

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
I have licensed the changes to Element by completing the Contributor License Agreement (CLA)

…sync" toast

florianduros · 2025-01-31T13:13:06Z

+    const recheckSetupRequired = useCallback(() => {
+        (async () => {
+            const crypto = matrixClient.getCrypto()!;
+            const isCrossSigningReady = await crypto.isCrossSigningReady();
+            if (isCrossSigningReady) {
+                setState("main");
+            } else {
+                setState("set_up_encryption");
+            }
+        })();
+    }, [matrixClient]);


I'm in favour to use a custom hook like useSetUpEncryptionRequired before. It helps the readability of the component imo

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.

florianduros · 2025-01-31T13:13:51Z

+            if (isCrossSigningReady) {
+                setState("main");
+            } else {
+                setState("set_up_encryption");
+            }


Suggested change

if (isCrossSigningReady) {

setState("main");

} else {

setState("set_up_encryption");

}

if (isCrossSigningReady) setState("main");

else setState("set_up_encryption");

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.

florianduros · 2025-01-31T13:25:31Z

+import { registerAccountMas } from "../oidc";
+import { deleteCachedSecrets } from "./utils";
+
+test.use(masHomeserver);


The test needs mas?

florianduros · 2025-01-31T13:28:52Z

+    test.beforeEach(async ({ page, app, mailpitClient }) => {
+        await page.goto("/#/login");
+        await page.getByRole("button", { name: "Continue" }).click();
+        await registerAccountMas(page, mailpitClient, "alice", "alice@email.com", "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();


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

It took a bit but I think this is working with the bot client now.

uhoreg

From a crypto point of view, looks OK

Co-authored-by: Florian Duros <florianduros@element.io>

…tofsync_forgotpassword

…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 <florianduros@element.io> * Add doc & do other part of rename * Split out into custom hook * Fix tests --------- Co-authored-by: Florian Duros <florianduros@element.io> (cherry picked from commit 9657d39)

richvdh · 2025-02-05T11:43:07Z

"Wire up", but what actually happens? can we get some screenshots or something?

richvdh · 2025-02-05T12:01:18Z

+import { test, expect } from "../../element-web-test";
+import { createBot, deleteCachedSecrets, logIntoElement } from "./utils";
+
+test.describe("Key storage out of sync toast", () => {


(don't we already have some tests for this toast somewhere else?)

I don't think so - maybe the backup failure screen is what you're thinking of?

…Key storage out of sync" toast (#29190) * Wire up the "Forgot recovery key" button for the "Key storage out of 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 <florianduros@element.io> * Add doc & do other part of rename * Split out into custom hook * Fix tests --------- Co-authored-by: Florian Duros <florianduros@element.io> (cherry picked from commit 9657d39) * Update fetchdep.sh to understand merge queues --------- Co-authored-by: David Baker <dbkr@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>

dbkr · 2025-03-14T17:31:00Z

Have opened #29503 to address the feedback that came in afterwards.

* Rename props & fix typo * Docs * Better docs * Add comment * Fix typo * Paragraphs in tsdoc * Add comment * Hopefully clearer comment * Really fix typo Co-authored-by: Will Hunt <will@half-shot.uk> * Stray word Co-authored-by: Andy Balaam <andy.balaam@matrix.org> * Hopefully clearer comment * Typo * Formatting & clarity Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --------- Co-authored-by: Will Hunt <will@half-shot.uk> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Wire up the "Forgot recovery key" button for the "Key storage out of …

b62aebb

…sync" toast

dbkr added the T-Defect label Jan 29, 2025

github-actions Bot deployed to Netlify January 29, 2025 18:49 View deployment

github-actions Bot deployed to EndToEndTests January 29, 2025 18:56 View deployment

dbkr added 2 commits January 30, 2025 13:01

Unused import & fix test

8b2aaa8

Test 'forgot' variant

2f0c5d2

github-actions Bot deployed to Netlify January 30, 2025 13:06 View deployment

Fix dependencies

33abac3

github-actions Bot deployed to Netlify January 30, 2025 13:12 View deployment

github-actions Bot deployed to EndToEndTests January 30, 2025 13:20 View deployment

Add more toast tests

d08b013

github-actions Bot deployed to Netlify January 30, 2025 13:50 View deployment

Unused import

4d99bef

github-actions Bot deployed to Netlify January 30, 2025 13:54 View deployment

github-actions Bot deployed to EndToEndTests January 30, 2025 14:02 View deployment

Test initialState in Encryption Tab

bc48eac

github-actions Bot deployed to Netlify January 30, 2025 14:29 View deployment

github-actions Bot deployed to EndToEndTests January 30, 2025 14:35 View deployment

dbkr added 2 commits January 31, 2025 12:26

Let's see if github has any more luck running this test than me

7ec1f01

Working playwright test with screenshot

823a05e

github-actions Bot deployed to Netlify January 31, 2025 12:36 View deployment

github-actions Bot deployed to EndToEndTests January 31, 2025 12:44 View deployment

year

6df7df9

dbkr marked this pull request as ready for review January 31, 2025 13:05

dbkr requested review from a team as code owners January 31, 2025 13:05

dbkr requested review from florianduros, t3chguy and uhoreg January 31, 2025 13:05

github-actions Bot deployed to Netlify January 31, 2025 13:07 View deployment

github-actions Bot deployed to EndToEndTests January 31, 2025 13:14 View deployment

florianduros requested changes Jan 31, 2025

View reviewed changes

uhoreg approved these changes Feb 1, 2025

View reviewed changes

Comment thread playwright/e2e/crypto/toasts.spec.ts Outdated

dbkr and others added 5 commits February 4, 2025 14:36

Convert playwright test to use the bot client

5508c97

Disambiguate

a612305

Co-authored-by: Florian Duros <florianduros@element.io>

Add doc & do other part of rename

939939a

Split out into custom hook

711e4f7

Merge remote-tracking branch 'origin/develop' into dbkr/keystorage_ou…

6681772

…tofsync_forgotpassword

github-actions Bot deployed to Netlify February 4, 2025 16:15 View deployment

florianduros approved these changes Feb 4, 2025

View reviewed changes

github-actions Bot deployed to EndToEndTests February 4, 2025 16:29 View deployment

Fix tests

60d1fa1

github-actions Bot deployed to Netlify February 4, 2025 17:13 View deployment

github-actions Bot deployed to EndToEndTests February 4, 2025 17:20 View deployment

dbkr added this pull request to the merge queue Feb 4, 2025

Merged via the queue into develop with commit 9657d39 Feb 4, 2025

dbkr deleted the dbkr/keystorage_outofsync_forgotpassword branch February 4, 2025 17:52

dbkr added the backport staging Label to automatically backport PR to staging branch label Feb 4, 2025

RiotRobot mentioned this pull request Feb 4, 2025

[Backport staging] Wire up the "Forgot recovery key" button for the "Key storage out of sync" toast #29190

Merged

richvdh reviewed Feb 5, 2025

View reviewed changes

dbkr mentioned this pull request Mar 14, 2025

Doc improvements from #29138 #29503

Merged

4 tasks

Uh oh!

Conversation

dbkr commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uhoreg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

richvdh commented Feb 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dbkr commented Mar 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dbkr commented Jan 29, 2025 •

edited

Loading