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

Figma design

TheAdvanced section in theEncryption tab of the user settings should not be displayed when the key storage is out of sync.
The secrets verification is moved to the encryption tab level instead to be at the recovery section level in order to only display the "key storage out of sync" section.

The PR can be reviewed by commit.

florianduros added2 commits

January 29, 2025 16:29

fix(encryption tab): hide the advanced section when the secrets are n…

4450809

…ot cached locallyThe secret verification is now made at the level of `EncryptionUserSettingsTab` instead at the `RecoveryPanel` level. In the `EncryptionUserSettingsTab`, we decide to only display `RecoveryPanelOutOfSync` in case of uncached secrets.`RecoveryPanelOutOfSync` is simplified version of `RecoveryPanel` handling only the `secrets_not_cached` case.

refactor(encryption tab): simplify theRecoveryPanel without having…

37c9fb8

… to handle the missing secrets

florianduros added the T-Defect label

Jan 29, 2025

github-actionsbotdeployed toNetlify

January 29, 2025 15:46

View deployment

github-actionsbotdeployed toEndToEndTests

January 29, 2025 15:54

View deployment

test(encryption tab): move test about cached secrets in `EncryptionUs…

ca2f881

…erSettingsTab-test.tsx`

github-actionsbotdeployed toNetlify

January 29, 2025 16:01

View deployment

github-actionsbotdeployed toEndToEndTests

January 29, 2025 16:09

View deployment

github-actionsbotdeployed toNetlify

January 29, 2025 16:29

View deployment

test(encryption tab): move e2e test which are testing all the encrypt…

22988e7

…ion tab in `encryption-tab.spec.ts

florianduros force-pushed theflorianduros/secret-not-cache-advanced-section branch from85141ee to22988e7Compare

January 29, 2025 16:33

github-actionsbotdeployed toNetlify

January 29, 2025 16:35

View deployment

github-actionsbotdeployed toEndToEndTests

January 29, 2025 16:44

View deployment

florianduros marked this pull request as ready for review

January 29, 2025 16:56

florianduros requested review froma team ascode owners

January 29, 2025 16:56

florianduros requested review fromrichvdh anddbkr

January 29, 2025 16:56

dbkr approved these changes

Jan 30, 2025

View reviewed changes

richvdh requested changes

Jan 30, 2025

View reviewed changes

Copy link

Member

richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

looks sensible in general. A few comments.

src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsx OutdatedShow resolvedHide resolved

src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsxShow resolvedHide resolved

src/components/views/settings/encryption/RecoveryPanel.tsx OutdatedShow resolvedHide resolved

playwright/e2e/settings/encryption-user-tab/encryption-tab.spec.ts OutdatedShow resolvedHide resolved

playwright/e2e/settings/encryption-user-tab/encryption-tab.spec.tsShow resolvedHide resolved

playwright/e2e/settings/encryption-user-tab/encryption-tab.spec.ts OutdatedShow resolvedHide resolved

playwright/e2e/settings/encryption-user-tab/encryption-tab.spec.ts Outdated

		await expect(content).toMatchScreenshot("verify-device-encryption-tab.png");
		await verifyButton.click();

		await util.verifyDevice(recoveryKey);

Copy link

Member

richvdhJan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

[aside: the fact that some utilities are standalone functions, and others have to be enabled via a fixture, is confusing to me. IMHO we should use fixtures less.]

playwright/e2e/settings/encryption-user-tab/recovery.spec.tsShow resolvedHide resolved

refactor(encryption tab): moveRecoveryPanelOutOfSync in its own file

259d4d4

- fix typos- call onFinish after accessSecretStorage- onFinish doesn't need to be asynchronous

github-actionsbotdeployed toNetlify

January 30, 2025 13:59

View deployment

github-actionsbotdeployed toNetlify

January 30, 2025 14:08

View deployment

github-actionsbotdeployed toNetlify

January 30, 2025 14:15

View deployment

florianduros added2 commits

January 30, 2025 15:20

doc(encryption tab): improve documentation when the secrets are not c…

5c4a795

…ached locally

test(encryption tab): improve test documentation and naming

bbb2b01

florianduros force-pushed theflorianduros/secret-not-cache-advanced-section branch from79c2c4b tobbb2b01Compare

January 30, 2025 14:20

github-actionsbotdeployed toNetlify

January 30, 2025 14:22

View deployment

github-actionsbotdeployed toEndToEndTests

January 30, 2025 14:30

View deployment

florianduros requested a review fromrichvdh

January 30, 2025 14:51

richvdh reviewed

Jan 30, 2025

View reviewed changes

src/components/views/settings/encryption/RecoveryPanelOutOfSync.tsx OutdatedShow resolvedHide resolved

src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsx


		// Check if the secrets are cached
		const cachedSecrets = (await crypto.getCrossSigningStatus()).privateKeysCachedLocally;
		const secretsOk = cachedSecrets.masterKey && cachedSecrets.selfSigningKey && cachedSecrets.userSigningKey;

Copy link

Member

richvdhJan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Don't we also need to check the key backup decryption key here? What happens if that is in 4S but not in the local client?

Copy link

MemberAuthor

floriandurosJan 30, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Hmm, good point. I'm doing another PR with this change#29150 (wip)

src/components/views/settings/tabs/user/EncryptionUserSettingsTab.tsx


		// Check if the secrets are cached
		const cachedSecrets = (await crypto.getCrossSigningStatus()).privateKeysCachedLocally;
		const secretsOk = cachedSecrets.masterKey && cachedSecrets.selfSigningKey && cachedSecrets.userSigningKey;

Copy link

Member

richvdhJan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

At some point (maybe not right now), we need to move this out of here and into a utility function somewhere (possibly in the js-sdk), otherwise, we have 150 different places to remember to update every time we introduce a new secret. Particularly, this is conflicting with the device dehydration work.