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

Third party cookies are still blocked even if per site shield is set to allow all cookies #2095

Closed
Liunkae opened this Issue Nov 10, 2018 · 8 comments

Comments

@Liunkae
Copy link
Contributor

Liunkae commented Nov 10, 2018

Description

Even after adjusting the per site cookie shield setting to All Cookies Allowed, some cookies are still blocked.

Steps to Reproduce

  1. Start with a fresh profile.
  2. Go to https://www.draw.io/
  3. Click the shields button.
  4. Change the cookie setting to All Cookies Allowed.
  5. Refresh the page.
  6. Click the lock icon and click Cookies.
  7. Click the Blocked tab.

Actual result:

Some of the cookies are blocked.

Expected result:

There should be no blocked cookies.

Reproduces how often:

Always.

Brave version (brave://version info)

Brave | 0.57.6 Chromium: 71.0.3578.31 (Official Build) beta(64-bit)
Revision | c88fdf2a4ce19a713615ca4fbde7a0d0b5fe2363-refs/branch-heads/3578@{#427}
OS | Mac OS X

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds? Yes, the latest beta build.

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields? No.
  • Is the issue reproducible on the latest version of Chrome? Probably not because there's no shield setting.

Additional Information

@srirambv srirambv added this to the 1.x Backlog milestone Nov 11, 2018

@rebron rebron added this to Untriaged Backlog in General Nov 12, 2018

@rebron rebron added the priority/P2 label Nov 13, 2018

@rebron rebron added this to Untriaged / Incoming in Shields via automation Nov 13, 2018

@rebron rebron removed this from Untriaged Backlog in General Nov 13, 2018

@iefremov

This comment has been minimized.

Copy link

iefremov commented Nov 19, 2018

TLDR: Even with enabled "All cookie allowed" or even "Shields DOWN for this site" we will block 3rdparty cookies for requests initiated from frames with origin different from the top-level one.

Example: on draw.io we block cookies for subframes accounts.google.com and content.googleapis.com

Reason:

  1. code that block cookies [1] uses site_for_cookies() from URLRequest [2] instead of the actual tab origin (top-level document origin).
  2. site_for_cookies() is empty for some frames [3]
  3. content settings checking util returns the default value for an empty url, which is BLOCK [4]

[1] https://github.com/brave/brave-core/blob/c532659bd23252ae70c94f842457f6f32ab09eef/browser/net/brave_network_delegate_base.cc#L186
[2] https://github.com/brave/brave-core/blob/68bbcf0af7b34e91c9866dbc54d1c0b151c5f726/browser/net/url_context.cc#L26
[3] https://cs.chromium.org/chromium/src/net/url_request/url_request.h?q=site_for_cookies&sq=package:chromium&dr=CSs&l=283
[4] https://github.com/brave/brave-core/blob/68bbcf0af7b34e91c9866dbc54d1c0b151c5f726/components/brave_shields/browser/brave_shields_util.cc#L80

@iefremov iefremov self-assigned this Nov 19, 2018

@bbondy bbondy moved this from Untriaged / Incoming to Design in Shields Nov 21, 2018

@bbondy bbondy moved this from Design to Front-end in Shields Nov 21, 2018

@bbondy bbondy moved this from Front-end to Priority tasks in Shields Nov 21, 2018

@bbondy bbondy added priority/P1 and removed priority/P2 labels Nov 21, 2018

@Bryanx

This comment has been minimized.

Copy link

Bryanx commented Nov 22, 2018

I have the same issue on my university website where Powerpoints are blocked. Is there a work-around to allow all cookies regardless?
image

iefremov added a commit to brave/brave-core that referenced this issue Nov 23, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

iefremov added a commit to brave/brave-core that referenced this issue Nov 23, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.
@iefremov

This comment has been minimized.

Copy link

iefremov commented Nov 23, 2018

Filed also #2223

@Liunkae

This comment has been minimized.

Copy link
Contributor Author

Liunkae commented Nov 23, 2018

@Bryanx As a workaround, change the default cookie control in brave://settings/ to "Allow all cookies".

iefremov added a commit to brave/brave-core that referenced this issue Nov 26, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

@iefremov iefremov referenced this issue Nov 27, 2018

Merged

Issue 2095: use the proper tab origin. #942

7 of 18 tasks complete

iefremov added a commit to brave/brave-core that referenced this issue Nov 27, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

iefremov added a commit to brave/brave-core that referenced this issue Nov 27, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

iefremov added a commit to brave/brave-core that referenced this issue Nov 27, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

bbondy added a commit to brave/brave-core that referenced this issue Nov 27, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

iefremov added a commit to brave/brave-core that referenced this issue Nov 28, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

bbondy added a commit to brave/brave-core that referenced this issue Nov 28, 2018

Issue 2095: use the proper tab origin.
brave/brave-browser#2095

As per URLRequest documentation and the corresponding RFC,
site_for_cookies cannot be used as a top-level document origin since it
can be empty in certain cases. We need to replace it with something more
reliable.

Shields automation moved this from Priority tasks to Completed Nov 28, 2018

@bbondy bbondy modified the milestones: 1.x Backlog, 0.57.x - Beta Nov 28, 2018

@iefremov

This comment has been minimized.

Copy link

iefremov commented Nov 29, 2018

Note: we will need to rework the whole thing once NetworkService is enabled in Chromium.

@LaurenWags

This comment has been minimized.

Copy link
Collaborator

LaurenWags commented Dec 4, 2018

Verified passed with

Brave 0.57.12 Chromium: 71.0.3578.75 (Official Build) (64-bit)
Revision 06ef00b5279f93f8e0c1e73acedd49d7dcc09767-refs/branch-heads/3578@{#836}
OS Mac OS X
  • Verified STR from description

Verification Passed on

Brave 0.57.17 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Windows
  • Verified the STR from description

Verification Passed on

Brave 0.57.17 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Linux

image

@Bnjmn83

This comment has been minimized.

Copy link

Bnjmn83 commented Jan 7, 2019

Experiencing the very same issue with Version 0.58.18 Chromium: 71.0.3578.98 (Official Build) (64-bit)

Selecting Allow all cookies in the global shield settings resolved this for me.

Is that behavior correct?

@iefremov

This comment has been minimized.

Copy link

iefremov commented Jan 7, 2019

Experiencing the very same issue with Version 0.58.18 Chromium: 71.0.3578.98 (Official Build) (64-bit)

Selecting Allow all cookies in the global shield settings resolved this for me.

Is that behavior correct?

Could you please specify the exact URL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.