Implements UIs for the "create account" and "sign in" flows in `brave://settings` #26741

szilardszaloki · 2024-11-25T22:56:43Z

create_account_and_sign_in.mp4

Note: the tooltip and the corresponding regular expression-based password score implementation in the "create account" dialog have been removed in favor of getting the password strength from zxcvbn.

Known issues:

The recommended approach to a Lit element's HTML template is to have it defined in an .html.ts file that's directly checked in to the repository, that is, no .html files are going through the html_to_wrapper target in the WebUI build pipeline. As such, the HTML template definition is lacking the  and  tags, without which the C++ templatizer does not do any text replacements — hence those tags are currently added manually.

TODOs:

I suggest that we export noChange from third_party/lit/v3_0/lit.ts (which has been recently removed in Chromium due to deleting their only reference to it) and use it instead of the workaround that addresses a visual artifact in brave_account_create_dialog.html.ts.

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

—

github-actions · 2024-11-25T23:01:15Z

The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "password" and so security team members have been added as reviewers to take a look.

No need to request a full security review at this stage, the security team will take a look shortly and either clear the label or request more information/changes.

Notifications have already been sent, but if this is blocking your merge feel free to reach out directly to the security team on Slack so that we can expedite this check.

brave-builds · 2024-11-26T00:01:44Z

A Storybook has been deployed to preview UI for the latest push

fallaciousreasoning

Hey this looks great - I had a dig into _html_template_start_ - man, this is why we can't have nice things 😆

It's such an ugly way of solving it, it makes me sad 😆 Thanks upstream.

My only real comment is that we should use a mojo interface instead of a handler

browser/ui/webui/settings/brave_account_handler.h

browser/resources/settings/getting_started_page/getting_started.html

browser/resources/settings/getting_started_page/getting_started.ts

browser/resources/settings/getting_started_page/brave_account_browser_proxy.ts

browser/resources/settings/getting_started_page/brave_account_common.ts

browser/resources/settings/getting_started_page/brave_account_create_dialog.ts

fallaciousreasoning · 2024-11-26T20:32:09Z

browser/resources/settings/getting_started_page/brave_account_create_dialog.ts

+    this.isCheckboxChecked = detail.checked
+  }
+
+  // TODO(sszaloki): we should consider exporting `noChange`


out of interest, why would we need noChange here?

Basically, when there's a matching password in the password confirmation field, we show the checkmark and the text Passwords match. Now, if the user presses Cmd + Backspace on the text in the password confirmation field, we should just hide the dropdown and should not recalculate which icon or text to show — since the password confirmation field became empty, and there's nothing in there we should try to match against the field above. That's an edge-case here, but if we don't do anything, there's a visual artifact: the icon changes to the red triangle, while the text changes to Passwords don't match, and then the dropdown disappears — which looks rookie. 🙂

The proper way to handle that would be via signaling with noChange that we don't need a change in the DOM if this.passwordConfirmation.length === 0. Alternatively, there is this hack we have in brave_account_create_dialog.ts, but it's far from ideal, as the TS file is required to know about style-related stuff (i.e. name of icons).

browser/resources/settings/getting_started_page/brave_account_dialog.html.ts

browser/resources/settings/getting_started_page/brave_account_forgot_password_dialog.html.ts

browser/resources/settings/getting_started_page/brave_account_row.html.ts

claucece

This is looking awesome! Thank you @szilardszaloki !

fmarier · 2024-12-07T01:32:45Z

app/brave_settings_strings.grdp

+    Sign in or create a Brave account
+  </message>
+  <message name="IDS_SETTINGS_BRAVE_ACCOUNT_ROW_DESCRIPTION" translateable="false" desc="Description of the Brave Account row in the Get Started section">
+    A Brave account will allow you to easily sync your settings, passwords, bookmarks, etc. across all your devices and access our full range of premium products.


We'll need to update that since we are shipping accounts without support for Sync (for the time being).

fmarier · 2024-12-07T01:33:27Z

app/brave_settings_strings.grdp

+  <message name="IDS_SETTINGS_BRAVE_ACCOUNT_ALREADY_HAVE_ACCOUNT_SIGN_IN_BUTTON_LABEL" translateable="false" desc="Label for the button that takes the user to the 'Sign In' dialog">
+    Already have an account? Sign in
+  </message>
+  <message name="IDS_SETTINGS_BRAVE_ACCOUNT_SELF_CUSTODY_DESCRIPTION" translateable="false" desc="Description of the self-custody option which we provide for advanced users">


The self-custody option should not be there until we are ready to integrate with Sync.

app/brave_settings_strings.grdp

browser/resources/settings/getting_started_page/brave_account_common.ts

browser/ui/webui/settings/brave_account_handler.cc

browser/ui/webui/settings/brave_settings_localized_strings_provider.cc

fmarier · 2024-12-07T01:48:16Z

browser/ui/webui/settings/brave_settings_localized_strings_provider.cc

@@ -48,6 +48,13 @@ namespace settings {

 namespace {

+constexpr char16_t kBraveAccountSelfCustodyLearnMoreURL[] =


We don't need self-custody this for now.

fmarier · 2024-12-07T23:22:04Z

I don't think I saw it in this PR: does this include the gmail.con spellchecker or is that for a future PR?

browser/resources/settings/sources.gni

browser/ui/BUILD.gn

bridiver

patch and browser/ui/BUILD.gn changes ok

szilardszaloki · 2024-12-11T02:15:23Z

I don't think I saw it in this PR: does this include the gmail.con spellchecker or is that for a future PR?

@fmarier Adding it to the current dropdown implementation introduces a visual artifact similar to the one being discussed here. Filed here for now — we'll likely want to export noChange from third_party/lit/v3_0/lit.ts to make our lives easier.

browser/resources/settings/getting_started_page/brave_account_common.ts

browser/resources/settings/getting_started_page/brave_account_create_dialog.ts

browser/resources/settings/getting_started_page/brave_account_common.ts

bsclifton

string reviewers ++

NOTE: I defer the content of the strings to @fmarier who has already left a comment 😄
But LGTM!

fmarier · 2024-12-12T00:28:20Z

I defer the content of the strings to @fmarier who has already left a comment 😄

Contents of the strings will need to change (as per my earlier comments) but since this is not enabled by default (and not translated) we can do that later as we get closer to enabling. So it's fine for now.

github-actions · 2024-12-12T00:43:41Z

[puLL-Merge] - brave/brave-core@26741

Description

This PR introduces a new Brave Account feature in the browser settings. It adds several new dialogs for account creation, sign-in, and password reset, as well as a new row in the "Get started" section of settings for managing the Brave account.

Changes

Changes

app/brave_settings_strings.grdp:
- Added new localized strings for Brave Account UI elements.
browser/resources/settings/BUILD.gn:
- Updated build configuration to include new Brave Account files.
browser/resources/settings/sources.gni:
- Added new Brave Account related files to the build process.
browser/resources/settings/getting_started_page/:
- Added several new files for Brave Account functionality:
  - brave_account_browser_proxy.ts
  - brave_account_common.ts
  - brave_account_create_dialog.ts
  - brave_account_dialog.ts
  - brave_account_entry_dialog.ts
  - brave_account_forgot_password_dialog.ts
  - brave_account_row.ts
  - brave_account_sign_in_dialog.ts
- Added corresponding CSS files for styling.
browser/ui/BUILD.gn:
- Added brave_account_handler.cc and brave_account_handler.h to the build.
browser/ui/webui/brave_settings_ui.cc:
- Added BraveAccountHandler to the WebUI message handlers.
browser/ui/webui/settings/brave_account_handler.cc and brave_account_handler.h:
- Implemented a new handler for Brave Account related WebUI messages.
browser/ui/webui/settings/brave_settings_localized_strings_provider.cc:
- Added new localized strings for Brave Account UI elements.
ui/webui/resources/leo/web_components.ts:
- Imported additional Leo web components (alert, icon, input).

sequenceDiagram
    participant User
    participant BraveSettingsUI
    participant BraveAccountHandler
    participant BraveAccountRow
    participant BraveAccountDialog

    User->>BraveSettingsUI: Open Settings
    BraveSettingsUI->>BraveAccountRow: Display Account Row
    User->>BraveAccountRow: Click "Get started"
    BraveAccountRow->>BraveAccountDialog: Show Entry Dialog
    User->>BraveAccountDialog: Choose Create Account
    BraveAccountDialog->>BraveAccountDialog: Show Create Account Dialog
    User->>BraveAccountDialog: Enter Account Details
    BraveAccountDialog->>BraveAccountHandler: Get Password Strength
    BraveAccountHandler-->>BraveAccountDialog: Return Password Strength
    User->>BraveAccountDialog: Submit Account Creation
    Note over BraveAccountDialog: Account Creation Logic (Not Implemented)
    BraveAccountDialog-->>BraveAccountRow: Update Account Status

Possible Issues

The implementation of actual account creation, sign-in, and password reset functionality is not included in this PR. These will need to be implemented separately.
There might be potential security considerations when handling user credentials that are not addressed in this PR.

Security Hotspots

brave_account_handler.cc: The GetPasswordStrength function handles password data. Ensure that this is done securely and that the password is not logged or stored inappropriately.

brave-builds · 2024-12-12T06:54:38Z

Released in v1.75.93

szilardszaloki requested review from fmarier, bsclifton, fallaciousreasoning and claucece November 25, 2024 22:56

szilardszaloki requested review from a team and bridiver as code owners November 25, 2024 22:56

github-actions bot assigned szilardszaloki Nov 25, 2024

github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build puLL-Merge labels Nov 25, 2024

github-actions bot added the needs-security-review label Nov 25, 2024

github-actions bot assigned kdenhartog and thypon Nov 25, 2024

fallaciousreasoning reviewed Nov 26, 2024

View reviewed changes

claucece approved these changes Dec 4, 2024

View reviewed changes

fmarier requested changes Dec 7, 2024

View reviewed changes

bridiver reviewed Dec 9, 2024

View reviewed changes

browser/resources/settings/sources.gni Show resolved Hide resolved

bridiver reviewed Dec 9, 2024

View reviewed changes

browser/ui/BUILD.gn Show resolved Hide resolved

bridiver approved these changes Dec 9, 2024

View reviewed changes

szilardszaloki force-pushed the szilard/brave-account-settings-ui branch 3 times, most recently from 9e189ec to 24e1608 Compare December 11, 2024 02:06

szilardszaloki force-pushed the szilard/brave-account-settings-ui branch 2 times, most recently from 06236ec to bbbbbed Compare December 11, 2024 15:31

fmarier approved these changes Dec 11, 2024

View reviewed changes

browser/resources/settings/getting_started_page/brave_account_common.ts Show resolved Hide resolved

browser/resources/settings/getting_started_page/brave_account_create_dialog.ts Outdated Show resolved Hide resolved

fmarier removed the needs-security-review label Dec 11, 2024

fallaciousreasoning reviewed Dec 11, 2024

View reviewed changes

browser/resources/settings/getting_started_page/brave_account_common.ts Outdated Show resolved Hide resolved

fallaciousreasoning approved these changes Dec 11, 2024

View reviewed changes

bsclifton approved these changes Dec 11, 2024

View reviewed changes

szilardszaloki added 2 commits December 11, 2024 17:42

Brave Account brave://settings UI.

588f333

Removes the "Account name" input field.

9f6e7b5

szilardszaloki force-pushed the szilard/brave-account-settings-ui branch from bbbbbed to 9f6e7b5 Compare December 12, 2024 00:42

szilardszaloki enabled auto-merge December 12, 2024 00:44

szilardszaloki merged commit 750933f into master Dec 12, 2024
18 checks passed

szilardszaloki deleted the szilard/brave-account-settings-ui branch December 12, 2024 02:35

github-actions bot added this to the 1.75.x - Nightly milestone Dec 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements UIs for the "create account" and "sign in" flows in `brave://settings` #26741

Implements UIs for the "create account" and "sign in" flows in `brave://settings` #26741

szilardszaloki commented Nov 25, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024

brave-builds commented Nov 26, 2024

fallaciousreasoning left a comment

fallaciousreasoning Nov 26, 2024

szilardszaloki Dec 10, 2024

claucece left a comment

fmarier Dec 7, 2024

fmarier Dec 7, 2024

fmarier Dec 7, 2024

fmarier commented Dec 7, 2024 •

edited

Loading

bridiver left a comment

szilardszaloki commented Dec 11, 2024

bsclifton left a comment

fmarier commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

Changes

brave-builds commented Dec 12, 2024

		@@ -48,6 +48,13 @@ namespace settings {

		namespace {

		constexpr char16_t kBraveAccountSelfCustodyLearnMoreURL[] =

Implements UIs for the "create account" and "sign in" flows in brave://settings #26741

Implements UIs for the "create account" and "sign in" flows in brave://settings #26741

Conversation

szilardszaloki commented Nov 25, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

github-actions bot commented Nov 25, 2024

brave-builds commented Nov 26, 2024

fallaciousreasoning left a comment

Choose a reason for hiding this comment

fallaciousreasoning Nov 26, 2024

Choose a reason for hiding this comment

szilardszaloki Dec 10, 2024

Choose a reason for hiding this comment

claucece left a comment

Choose a reason for hiding this comment

fmarier Dec 7, 2024

Choose a reason for hiding this comment

fmarier Dec 7, 2024

Choose a reason for hiding this comment

fmarier Dec 7, 2024

Choose a reason for hiding this comment

fmarier commented Dec 7, 2024 • edited Loading

bridiver left a comment

Choose a reason for hiding this comment

szilardszaloki commented Dec 11, 2024

bsclifton left a comment

Choose a reason for hiding this comment

fmarier commented Dec 12, 2024

github-actions bot commented Dec 12, 2024

Description

Changes

Possible Issues

Security Hotspots

brave-builds commented Dec 12, 2024

Implements UIs for the "create account" and "sign in" flows in `brave://settings` #26741

Implements UIs for the "create account" and "sign in" flows in `brave://settings` #26741

szilardszaloki commented Nov 25, 2024 •

edited

Loading

fmarier commented Dec 7, 2024 •

edited

Loading