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

Safety Check is warning about Passwords even though we disable that feature #39212

Closed
1 of 6 tasks
fmarier opened this issue Jun 20, 2024 · 5 comments · Fixed by brave/brave-core#24703
Closed
1 of 6 tasks
Assignees
Labels
feature/password-manager needs-text-change This change requires some careful wording. OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Yes release-notes/include

Comments

@fmarier
Copy link
Member

fmarier commented Jun 20, 2024

Description

As reported in #12859 (comment), there is a very unhelpful message from Safety Check in brave://settings/privacy:
Screenshot from 2024-06-20 16-54-33

since the password block is hidden in brave://settings/safetyCheck.

I have confirmed that no network requests are triggered by this, so it may be a local check for bad passwords, I'm not sure.

Steps to reproduce

  1. Use a new browser profile.
  2. Go to brave://settings/privacy and scroll down to Safety Check.
  3. Confirm that it's not reporting anything.
  4. Go to brave://password-manager/passwords.
  5. Add a fake password for facebook.com: username = foo and password = bar (note: it appears to require a bad password to trigger).
  6. Go back to brave://settings/privacy and scroll down to Safety Check.

Actual result

Brave found some safety recommendations for your review
Passwords

Expected result

Brave regularly checks to make sure your browser has the safest settings. We'll let you know if anything needs your review.

Reproduces how often

Easily reproduced

Brave version (brave://version info)

Brave	1.68.92 Chromium: 126.0.6478.71 (Official Build) beta (64-bit)
Revision	393b9968e233540a48f04d74bc4601a05d3a0169
OS	Linux

Channel information

  • release (stable)
  • beta
  • nightly

Reproducibility

  • with Brave Shields disabled
  • with Brave Rewards disabled
  • in the latest version of Chrome

Miscellaneous information

No response

@fmarier fmarier added this to Untriaged Backlog in Security & Privacy via automation Jun 20, 2024
@rebron rebron added priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes labels Jun 21, 2024
@rebron
Copy link
Collaborator

rebron commented Jun 21, 2024

cc: @emerick

@rebron rebron added needs-text-change This change requires some careful wording. release-notes/include labels Jul 10, 2024
@mkarolin
Copy link
Contributor

@fmarier Do we want to always hide the specifics? For example, if we have

image

do we not want to see "Permissions" part either? So always hide the sub-text and just replace upstream text with ours?

@fmarier
Copy link
Member Author

fmarier commented Jul 10, 2024

If the Safety Check/Hub shows something useful when Permissions are flagged in the sub-text, then it's okay to leave it (and probably better UX too).

The problem is with recommendations related to features we disable (the password checker is the only one I can think of).

Removing the sub-text wouldn't completely fix the issue because if it says "Brave found some safety recommendations for your review" (omitting Passwords) and then nothing is shown in Safety Check/Hub, then that would be confusing to users.

@kjozwiak
Copy link
Member

The above requires 1.68.127 or higher for 1.68.x verification 👍

@GeetaSarvadnya GeetaSarvadnya added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jul 23, 2024
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jul 23, 2024

Verification PASSED on

Brave | 1.68.127 Chromium: 127.0.6533.57 (Official Build) (64-bit)
-- | --
Revision | 88b0d9010af274686b27d8be77edd728fcba04a5
OS | Windows 10 Version 22H2 (Build 19045.4651)

Reproduced the issue on 1.67.134 and seen the message Brave found some safety recommendations for your review Passwords

image

Upgraded the profile to 1.68.127 and ensured that the message Brave regularly checks to make sure your browser has the safest settings. We'll let you know if anything needs your review. is shown as expected
image

@GeetaSarvadnya GeetaSarvadnya added QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/password-manager needs-text-change This change requires some careful wording. OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Yes release-notes/include
Projects
Security & Privacy
  
Completed
Development

Successfully merging a pull request may close this issue.

7 participants