Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Recovery section in the new user settings Encryption tab #28673

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

florianduros
Copy link
Member

@florianduros florianduros commented Dec 6, 2024

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)

First of all, I'm sorry for the size of the PR.

Task #26468
Figma design
Add a new encryption tab with a new recovery section.

Implemented

  • Add the crypto team as code owner of this new encryption tab

The encryption tab has the following behaviour:

  • If the device is not verified, a button to launch the verification button is displayed. Only this button and his description is displayed. The user must verify his device before playing with the encryption settings.
  • Otherwise, display the Recovery section (the Key storage and Advanced section in the future)
  • If the user want to set up or change the recovery key when clicking on the corresponding button of the recovery section, the content of the encryption tab is replaced by a breadcrumb to do it.

The recovery section has the following behaviour:

  • If the key backup is not setup, we ask to setup a recovery key
  • If the secrets (cross-signing keys) are not cached locally, we ask to enter the recovery key
  • Otherwise, display the "change recovery key" button

Not implemented

This PR DON'T (because the PR is already massive and theses tasks should be done in other PRs):

  • Implement the "Key storage is out of sync" toast
  • Implement the new "Enter your recovery key" dialog and the "Forgot your recovery key? You’ll need to reset your identity." dialog
  • Add dot to the User settings tab

@florianduros florianduros force-pushed the florianduros/encryption-tab branch 3 times, most recently from 7ee9a42 to a90f51f Compare December 6, 2024 11:06
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from a90f51f to b3dc812 Compare December 6, 2024 14:19
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from b3dc812 to 98d86e0 Compare December 6, 2024 16:21
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from 98d86e0 to 2710462 Compare December 10, 2024 15:02
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from 2710462 to 5e31204 Compare December 11, 2024 14:30
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from 5e31204 to 54ede8b Compare December 11, 2024 14:49
@florianduros florianduros force-pushed the florianduros/encryption-tab branch from 54ede8b to 1811aa6 Compare December 11, 2024 17:22
@florianduros florianduros force-pushed the florianduros/encryption-tab branch 5 times, most recently from 97b9f95 to 9aecfcc Compare December 12, 2024 09:16
@florianduros florianduros marked this pull request as ready for review December 18, 2024 18:59
@florianduros florianduros requested a review from a team as a code owner December 18, 2024 18:59
@florianduros florianduros requested review from dbkr, t3chguy, a team and richvdh and removed request for a team December 18, 2024 18:59
padding: var(--cpd-space-10x);
border-radius: var(--cpd-space-4x);
/* From figma */
box-shadow: 0 1.2px 2.4px 0 rgba(27, 29, 34, 0.15);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be in Compound?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that compound hosts box-shadow tokens

src/components/views/settings/SettingsSubheader.tsx Outdated Show resolved Hide resolved
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

There's a lot here, but lgtm as far as I can see!

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Sorry, on further reading I have some questions.

playwright/e2e/crypto/utils.ts Outdated Show resolved Hide resolved
},
);

test("should setup the recovery key", { tag: "@screenshot" }, async ({ page, app, util }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this test be more along the lines of registering an account and then setting up recovery? Is the bot doing something here to get crypto into a certain state?

Copy link
Member Author

@florianduros florianduros Dec 23, 2024

Choose a reason for hiding this comment

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

You can if you delete your key backup. For example go to the Security & Privacy tab and click on "Delete backup". Even if we remove this section of Security & Privacy later, an another matrix client can remove it.

The e2e test call client.getCrypto()?.deleteKeyBackupVersion(backupVersion) on the last backup version to be in this state.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, I think I was confusing this with deleteCachedSecrets().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants