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.

Sign up for GitHub

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

Jump to bottom

Global cookies settings are not retained when a site is added/removed from the shields down block #26288

Closed
GeetaSarvadnya opened this issue Oct 26, 2022 · 10 comments · Fixed by brave/brave-core#16067
Closed

Global cookies settings are not retained when a site is added/removed from the shields down block #26288

GeetaSarvadnya opened this issue Oct 26, 2022 · 10 comments · Fixed by brave/brave-core#16067
Assignees
@boocmp
Labels
bug feature/shields The overall Shields feature in Brave. OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Test-Plan-Specified QA/Yes release-notes/include
Milestone
1.48.x - Release

Comments

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Oct 26, 2022 •
edited
Loading

Description

Found while testing #12782
Cookies settings are not retained when a site is added/removed from the shields down block

Steps to Reproduce

Case 1:

  1. clean profile 1.46.x
  2. open nytimes.com site in an NTP
  3. add nytimes to shield down block in brave://settings/content/braveShields
  4. make sure shield is Down for nytimes.com
  5. click on the shield icon for nytimes.com and make the shields Up
  6. make sure shield is Up for nytimes.com
  7. expand the advanced controls shield settings and look for the cookies settings
  8. cookies settings is Allow all cookies but the global shield cookies settings is Block cross site cookies
  9. global cookies settings are not retained

Case 2:

  1. clean profile 1.46.x
  2. add nytimes.com in shields down block under brave://settings/content/braveShields
  3. open nytimes.com in an NTP and ensure that shield is down
  4. remove the blocked site nytimes.com from brave://settings/content/braveShields
  5. open nytimes.com tab and ensure shield is enabled and default shield settings are retained on nytimes.com
  6. change the global shield settings (Trackers and ads = Aggressive, FF= Strict, may break sites and cookies = Disabled)
  7. ensured that the updated global shield settings are retained for nytimes
  8. add nytimes to shields down block in content settings
  9. ensured nytimes shield is down
  10. enable the shield for nytimes.com manually
  11. ensured shield is enabled and default shield settings are retained on nytimes (steps 6 shield settings are retained)
  12. change the global shield settings (Trackers and ads = Standard, FF= Standard and cookies = All)
  13. open nytimes, the updated shield settings are not retained for cookies

Actual result:

Cookies settings are not retained when a site is added/removed from the shields down block

https://drive.google.com/file/d/1ZHYz488-D-nOKpITsRguIhwxNAJ4aSzk/view?usp=sharing

Expected result:

Cookies settings should be retained when a site is added/removed from the shields down block

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 1.46.80 Chromium: 107.0.5304.68 (Official Build) beta (64-bit)
Revision a4e93e89d3b3df1be22214603fba846ad0183ca5-refs/branch-heads/5304@{#991}
OS Windows 10 Version 21H2 (Build 19044.2130)

Version/Channel Information:

  • Can you reproduce this issue with the current release? NA
  • Can you reproduce this issue with the beta channel? Yes
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? NA
  • Does the issue resolve itself when disabling Brave Rewards? NA
  • Is the issue reproducible on the latest version of Chrome? NA

Miscellaneous Information:

cc: @brave/qa-team @rebron @spylogsster
@GeetaSarvadnya GeetaSarvadnya mentioned this issue Oct 26, 2022
Closed
@GeetaSarvadnya GeetaSarvadnya changed the title Cookies settings are not retained when a site is added/removed from the shields down block Global cookies settings are not retained when a site is added/removed from the shields down block Oct 27, 2022
@btlechowski
Copy link

Reproduced on Linux

Brave 1.46.117 Chromium: 107.0.5304.110 (Official Build) beta (64-bit)
Revision 2a558545ab7e6fb8177002bf44d4fc1717cb2998-refs/branch-heads/5304@{#1202}
OS Ubuntu 18.04 LTS

@LaurenWags LaurenWags added this to the 1.46.x - Beta milestone Nov 18, 2022
@rebron rebron added the priority/P2 A bad problem. We might uplift this to the next planned release. label Nov 18, 2022
@spylogsster spylogsster self-assigned this Nov 18, 2022
@bridiver
Copy link
Contributor

bridiver commented Nov 21, 2022 •
edited
Loading

@GeetaSarvadnya please use shields up/down terminology because "block" is confusing in this context. Also I assume "enable the shield manually" - means in the shields panel? Can you please update it to reflect that?

@bridiver
Copy link
Contributor

@GeetaSarvadnya if you enable/disable with the shields panel, it will be www.nytimes.com, but in this ticket you specific nytimes.com in brave://settings/content/braveShields

@bridiver
Copy link
Contributor

bridiver commented Nov 21, 2022 •
edited
Loading

If I do [*.]nytimes.com in brave://settings/content/braveShields then I get shields down, but that is a different setting than what the shields panel controls

If I then re-enable nytimes in the shields panel, it shows shields up (because the more specific match for www.nytimes.com is shields up), but cookies are still (incorrectly) applied based on the [*.]nytimes.com entry

So… there are two bugs here:

  1. We’re still showing the effective setting in the UI and that can be broken in all kinds of different ways. That change was supposed to be reverted
  2. We’re applying [*.]nytimes.com to the effective cookies setting even though there’s a more specific setting www.nytimes.com that should take precedence

@bridiver
Copy link
Contributor

bridiver commented Nov 21, 2022 •
edited
Loading

if I just enter nytimes.com in brave://settings/content/braveShields then it has no effect on nytimes as expected because it's actually www.nytimes.com (chromium hides the www. by default when you're not editing the url)

@spylogsster spylogsster removed their assignment Nov 21, 2022
@bridiver
Copy link
Contributor

based on the actual problem I think @boocmp would be the right person for this.

@GeetaSarvadnya GeetaSarvadnya mentioned this issue Nov 22, 2022
Closed
@GeetaSarvadnya
Copy link
Author

@GeetaSarvadnya please use shields up/down terminology because "block" is confusing in this context. Also I assume "enable the shield manually" - means in the shields panel? Can you please update it to reflect that?

@bridiver Yes, in the shields panel, I will update the steps. Thanks!

@spylogsster spylogsster mentioned this issue Nov 23, 2022
Merged
25 tasks
@rebron rebron removed this from the 1.46.x - Release milestone Nov 29, 2022
@brave-builds brave-builds added this to the 1.48.x - Nightly milestone Dec 7, 2022
@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 13, 2023
@MadhaviSeelam
Copy link

MadhaviSeelam commented Jan 13, 2023 •
edited
Loading

Verification PASSED using

Brave | 1.48.125 Chromium: 109.0.5414.87 (Official Build) beta (64-bit)
-- | --
Revision | 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS | Windows 11 Version 21H2 (Build 22000.1455)

Case 1: Block wildcard entry for Shields - (Do not update Shields settings) - PASSED

  1. Install 1.48.125
  2. launch Brave
  3. visit nytimes.com site in a new-tab page
  4. open brave://settings/content/braveShields
  5. click Add button in the Shields Down section under Sites listed below...
  6. add [*]nytimes.com in the dialog
  7. confirmed Wildcards are not allowed for Brave Shields. message displayed
  8. click Cancel and add www.nytimes.com (enter www in order to disable Shields)
  9. return to nytimes.com tab
  10. confirmed Shields are Down in the Shields panel
  11. enable Shields in the Shields panel
  12. expanded the Advanced controls in the Shields panel
  13. confirmed Block cross-site cookies settings shown
  14. return to brave://settings/content/braveShields
  15. confirmed www.nytimes.com was removed from the Shields Down section
  16. open brave://settings/shields
  17. confirmed Block cookies settings show Only cross-site as expected

Confirmed wildcard entries are blocked for Shields in brave://settings/content/braveShields

Confirmed cookie settings per site and Global settings show cross-site

step 3 step 6-7 step 8 step 10 step 11 step 13 step 15 step 18
image image image image image image image image

Case 2: Block wildcard entry for Shields - (Update Global Shields settings) - PASSED

  1. new profile
  2. visit nytimes.com in a new-tab page
  3. open brave://settings/content/braveShields
  4. click Add button in the Shields Down section under Sites listed below...
  5. enter www.nytimes.com (enter www in order to disable Shields)
  6. returned to nytimes.com tab
  7. confirmed Shields Down in the Shields panel
  8. return to brave://settings/content/braveShields
  9. click three dots to remove the entry (nytimes.com)
  10. returned to nytimes.com tab
  11. confirmed Shields is enabled and default shield settings are shown
  12. open brave://settings/shields in a new-tab
  13. change settings for following:
    • Trackers &ads blocking: Aggressive
    • Block fingerprinting: Strict, may break sites
    • Block cookies = Disabled
  14. returned to nytimes tab and verified that the updated global shield settings are shown
  15. returned to brave://settings/content/braveShields
  16. add www.nytimes.com to Shields Down section
  17. returned to nytimes tab and confirmed Shields is disabled
  18. enabled Shields for nytimes.com in the Shields panel
  19. confirmed updated Shield settings are retained (from Step 13)
  20. return to brave://settings/shields
  21. change Global shield settings as below:
    • Trackers and ads: Standard,
    • Block fingerprinting: Standard
    • Block cookies:All
  22. return to nytimes tab and click Shields panel

Confirmed Shields panel shows updated Shields settings after adding and removing entry in brave://settings/shields

step 2 step 4 step 5 step 7 step 9 step 11 step 13 step 15 step 16 step 18 step 19 step 21 step 22
image image image image image image image image image image image image image

Case 3: Allow wildcard entry for non Shields content settings - PASSED

  1. new profile
  2. visit reddit.com
  3. confirmed permissions expiration modal is displayed
  4. open brave://settings/content/notifications
  5. click Add button in the Not allowed to send notifications
  6. add [*.]reddit.com in the dialog
  7. confirmed wildcard entry is allowed as expected
  8. return and refresh reddit tab and click the lock
  9. confirmed Notification permissions are blocked
  10. return to brave://settings/content/notifications
  11. click three dots to remove the entry (reddit.com)
  12. confirmed Notifications are allowed

Confirmed wildcard entries are allowed for non Shields content setting - brave://settings/content/notifications

step 2 step 5 step 6 step 8 step 10 step11 step 12
image image image image image image image

@MadhaviSeelam MadhaviSeelam added QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jan 17, 2023
@stephendonner stephendonner added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 19, 2023
@stephendonner
Copy link

stephendonner commented Jan 19, 2023 •
edited
Loading

Verification PASSED using

Brave 1.48.131 Chromium: 109.0.5414.87 (Official Build) beta (x86_64)
Revision 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS macOS Version 11.7.2 (Build 20G1020)

Case 1: Block wildcard entry for Shields - (Do not update Shields settings) - PASSED

  1. installed 1.48.131
  2. launched Brave
  3. visited nytimes.com site in a new-tab page
  4. opened brave://settings/content/braveShields
  5. clicked Add button in the Shields Down section under Sites listed below...
  6. added [*]nytimes.com in the dialog
  7. confirmed Wildcards are not allowed for Brave Shields. message displayed
  8. clicked Cancel and added www.nytimes.com (enter www in order to disable Shields)
  9. returned to nytimes.com tab
  10. confirmed Shields are Down in the Shields panel
  11. enabled Shields in the Shields panel
  12. expanded the Advanced controls in the Shields panel
  13. confirmed Block cross-site cookies settings shown
  14. returned to brave://settings/content/braveShields
  15. confirmed www.nytimes.com was removed from the Shields Down section
  16. open brave://settings/shields
  17. confirmed Block cookies settings show Only cross-site as expected

Confirmed wildcard entries are blocked for Shields in brave://settings/content/braveShields

Confirmed cookie settings per site and Global settings show cross-site

step 3 steps 6-7 step 8 step 10 step 11 step 13 step 15 step 17
Screen Shot 2023-01-18 at 4 46 55 PM Screen Shot 2023-01-18 at 4 48 18 PM Screen Shot 2023-01-18 at 4 48 50 PM Screen Shot 2023-01-18 at 4 54 42 PM Screen Shot 2023-01-18 at 4 49 20 PM Screen Shot 2023-01-18 at 4 49 30 PM Screen Shot 2023-01-18 at 4 49 40 PM Screen Shot 2023-01-18 at 4 51 17 PM

Case 2: Block wildcard entry for Shields - (Update Global Shields settings) - PASSED

  1. new profile
  2. visit nytimes.com in a new-tab page
  3. open brave://settings/content/braveShields
  4. click Add button in the Shields Down section under Sites listed below...
  5. enter www.nytimes.com (enter www in order to disable Shields)
  6. returned to nytimes.com tab
  7. confirmed Shields Down in the Shields panel
  8. return to brave://settings/content/braveShields
  9. click three dots to remove the entry (nytimes.com)
  10. returned to nytimes.com tab
  11. confirmed Shields is enabled and default shield settings are shown
  12. open brave://settings/shields in a new-tab
  13. change settings for following:
    • Trackers &ads blocking: Aggressive
    • Block fingerprinting: Strict, may break sites
    • Block cookies = Disabled
  14. returned to nytimes tab and verified that the updated global shield settings are shown
  15. returned to brave://settings/content/braveShields
  16. add www.nytimes.com to Shields Down section
  17. returned to nytimes tab and confirmed Shields is disabled
  18. enabled Shields for nytimes.com in the Shields panel
  19. confirmed updated Shield settings are retained (from Step 13)
  20. return to brave://settings/shields
  21. change Global shield settings as below:
    • Trackers and ads: Standard,
    • Block fingerprinting: Standard
    • Block cookies:All
  22. return to nytimes tab and click Shields panel

Confirmed Shields panel shows updated Shields settings after adding and removing entry in brave://settings/shields

step 2 step 4 step 5 step 7 step 9 step 11 step 13 step 14 step 16 steps 18-19 step 21 step 22
Screen Shot 2023-01-18 at 5 18 35 PM Screen Shot 2023-01-18 at 5 21 32 PM Screen Shot 2023-01-18 at 5 21 36 PM Screen Shot 2023-01-18 at 5 25 23 PM Screen Shot 2023-01-18 at 5 28 48 PM Screen Shot 2023-01-18 at 5 29 46 PM Screen Shot 2023-01-18 at 5 31 16 PM Screen Shot 2023-01-18 at 5 34 10 PM Screen Shot 2023-01-18 at 5 35 29 PM Screen Shot 2023-01-18 at 5 36 02 PM Screen Shot 2023-01-18 at 5 37 58 PM Screen Shot 2023-01-18 at 5 38 41 PM

Case 3: Allow wildcard entry for non-Shields content settings - PASSED

  1. new profile
  2. loaded reddit.com
  3. confirmed permissions expiration modal is displayed
  4. open brave://settings/content/notifications
  5. click Add button in the Not allowed to send notifications
  6. add [*.]reddit.com in the dialog
  7. confirmed wildcard entry is allowed as expected
  8. return and refresh reddit tab and click the lock
  9. confirmed Notification permissions are blocked
  10. return to brave://settings/content/notifications
  11. click three dots to remove the entry (reddit.com)
  12. confirmed Notifications are allowed

Confirmed wildcard entries are allowed for non-Shields content setting - brave://settings/content/notifications

step 2 step 5 step 6 step 8 step 10 step11 step 12
Screen Shot 2023-01-18 at 5 03 20 PM Screen Shot 2023-01-18 at 5 05 02 PM Screen Shot 2023-01-18 at 5 05 19 PM Screen Shot 2023-01-18 at 5 07 28 PM Screen Shot 2023-01-18 at 5 08 22 PM Screen Shot 2023-01-18 at 5 08 26 PM Screen Shot 2023-01-18 at 5 08 59 PM

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jan 19, 2023
@MadhaviSeelam
Copy link

MadhaviSeelam commented Jan 25, 2023 •
edited
Loading

Verification PASSED using

Brave | 1.48.140 Chromium: 109.0.5414.119 (Official Build) beta (64-bit)
-- | --
Revision | 772095164c7d5d4e73160f858efed3b5e87eca83-refs/branch-heads/5414@{#1458}
OS | Linux

Case 1: Block wildcard entry for Shields - (Do not update Shields settings) - PASSED

  1. Install 1.48.140
  2. launch Brave
  3. visit nytimes.com site in a new-tab page
  4. open brave://settings/content/braveShields
  5. click Add button in the Shields Down section under Sites listed below...
  6. add [*]nytimes.com in the dialog
  7. confirmed Wildcards are not allowed for Brave Shields. message displayed
  8. click Cancel and add www.nytimes.com (enter www in order to disable Shields)
  9. return to nytimes.com tab
  10. confirmed Shields are Down in the Shields panel
  11. enable Shields in the Shields panel
  12. expanded the Advanced controls in the Shields panel
  13. confirmed Block cross-site cookies settings shown
  14. return to brave://settings/content/braveShields
  15. confirmed www.nytimes.com was removed from the Shields Down section
  16. open brave://settings/shields
  17. confirmed Block cookies settings show Only cross-site as expected

Confirmed wildcard entries are blocked for Shields in brave://settings/content/braveShields

Confirmed cookie settings per site and Global settings show cross-site

step 3 step 6-7 step 8 step 10 step 11-13 step 15 step 18
image image image image image image image

Case 2: Block wildcard entry for Shields - (Update Global Shields settings) - PASSED

  1. new profile
  2. visit nytimes.com in a new-tab page
  3. open brave://settings/content/braveShields
  4. click Add button in the Shields Down section under Sites listed below...
  5. enter www.nytimes.com (enter www in order to disable Shields)
  6. returned to nytimes.com tab
  7. confirmed Shields Down in the Shields panel
  8. return to brave://settings/content/braveShields
  9. click three dots to remove the entry (nytimes.com)
  10. returned to nytimes.com tab
  11. confirmed Shields is enabled and default shield settings are shown
  12. open brave://settings/shields in a new-tab
  13. change settings for following:
    • Trackers &ads blocking: Aggressive
    • Block fingerprinting: Strict, may break sites
    • Block cookies = Disabled
  14. returned to nytimes tab and verified that the updated global shield settings are shown
  15. returned to brave://settings/content/braveShields
  16. add www.nytimes.com to Shields Down section
  17. returned to nytimes tab and confirmed Shields is disabled
  18. enabled Shields for nytimes.com in the Shields panel
  19. confirmed updated Shield settings are retained (from Step 13)
  20. return to brave://settings/shields
  21. change Global shield settings as below:
    • Trackers and ads: Standard,
    • Block fingerprinting: Standard
    • Block cookies:All
  22. return to nytimes tab and click Shields panel

Confirmed Shields panel shows updated Shields settings after adding and removing entry in brave://settings/shields

step 2 step 4 step 5 step 7 step 9 step 11 step 13 step 15 step 16 step 18 step 19 step 21 step 22
image image image image image image image image image image image image image

Case 3: Allow wildcard entry for non Shields content settings - PASSED

  1. new profile
  2. visit reddit.com
  3. confirmed permissions expiration modal is displayed
  4. open brave://settings/content/notifications
  5. click Add button in the Not allowed to send notifications
  6. add [*.]reddit.com in the dialog
  7. confirmed wildcard entry is allowed as expected
  8. return and refresh reddit tab and click the lock
  9. confirmed Notification permissions are blocked
  10. return to brave://settings/content/notifications
  11. click three dots to remove the entry (reddit.com)
  12. confirmed Notifications are allowed

Confirmed wildcard entries are allowed for non Shields content setting - brave://settings/content/notifications

step 2 step 6 step 7 step 9 step 10 step11 step 12
image image image image image image image

@LaurenWags LaurenWags mentioned this issue Feb 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@boocmp boocmp

Labels
bug feature/shields The overall Shields feature in Brave. OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Test-Plan-Specified QA/Yes release-notes/include
Projects
None yet
Milestone
1.48.x - Release
Development

Successfully merging a pull request may close this issue.

Block wildcards for brave shields brave/brave-core
10 participants
@bridiver @stephendonner @spylogsster @rebron @LaurenWags @btlechowski @GeetaSarvadnya @brave-builds @boocmp @MadhaviSeelam