New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 900574 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

"AllowedDomainsForApps" policy doesn't work as before.

Project Member Reported by marcore@chromium.org, Oct 31

Issue description

Chrome version: 70.0.3538.67 https://drive.google.com/open?id=1zGfhskWnn-hvBH2yb5iF25xrd8jfj-As
OS version: Windows 10
Case#: 17286775
 
Description: the GPO policy AllowedDomainsForApps is not working 

last known good: 69.0.3497.100
first bad: 70.0.3538.67
I'll try to bisect more.

Steps to reproduce: 
1) configure SSO on the domain
2) configure "Keep local data until you quit your browser"
3) configure AllowedDomainsForApps with your domain name (pushed via GPO)

Current Behavior / Reproduction: 
every time you login on the browser it show you "Verify that it's you"

Expected Behavior: 
don't show the "Verify that it's you"
as explained on this help article: https://gsuiteupdates.googleblog.com/2018/04/more-secure-sign-in-chrome.html

video not working (M70): https://drive.google.com/open?id=1SMSipSLFG-_AH-sIzmZReiv-CMIN2gt0
video working (M69): https://drive.google.com/open?id=1JMptDE25mWRMPIWONrY3W4ZsF1Vj3XK9
policies json: https://drive.google.com/open?id=1MlTUZ2Js8YVzrYxmon6i_Uz3qTpLiccn
policies pdf: https://drive.google.com/open?id=10nV46-QXUTvXj0rT6t7a3Nv-rVAfYIqb

 
Labels: -Pri-2 Restrict-View-Google Pri-1
HAR file chrome 69: https://drive.google.com/open?id=1A2Ehv-qfvtXGGltzdXfuu-XCyOxSMLlw
HAR file chrome 70: https://drive.google.com/open?id=1r0kYNJ2SYRBTPHpu1dsTwW8J9biWDYwA
Labels: -Restrict-View-Google -Pri-1 Pri-2
Comments about our findings in HAR files - it like the logic behind adding HTTP header x-googapps-allowed-domains has changed, for example - 

In 69 -
GET https://accounts.google.com/signin/chrome/sync/identifier... - header is added
GET https://accounts.google.com/accounts/static/_/js/... - header is added

In 70 - 
GET https://accounts.google.com/signin/chrome/sync/identifier... - header is NOT added
GET https://accounts.google.com/accounts/static/_/js/... - header is added

So it seems like previously header was set per domain, now it seem to check URL path as well.
And subdomains like play.google.com and ogs.google.com don't receive this header as well in v70 (expected behavior, I guess, but change nonetheless)
Cc: georgesak@chromium.org
Labels: FoundIn-70 OS-Windows
George, are you able to identify a resource that can help with this issue?  I was unable to find anything that would explain why this behavior would change with release 70.

Could it be somewhat related to the DICE sign-in changes?
Owner: jam@chromium.org
I've bisect more:
70.0.3509.0 Last GOOD
70.0.3510.0 first BAD
video of the bisect: https://drive.google.com/open?id=1mSu6LiXS7NRfNZjQEP9JrfXcSiIM43Ow
https://chromium.googlesource.com/chromium/src/+log/70.0.3509.0..70.0.3510.0?pretty=fuller&n=10000
it could be 
https://chromium.googlesource.com/chromium/src/+/b8dd98642173f26e20338e9da9eb74d31126198c

John, could you please check this issue ?
Cc: kcnair@google.com
Status: Assigned (was: Untriaged)
Cc: marchuk@google.com
++case 17425064 

One more observation, if users disable "Allow Chrome sign-in" in local settings, "Verify that it's you" is not appearing on v70.
Status: Started (was: Assigned)
Just saw this, looking.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c460cb60dd046f595ed2ead9762b95b61c7323a9

commit c460cb60dd046f595ed2ead9762b95b61c7323a9
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Nov 08 00:29:56 2018

Fix modified headers from URLLoaderThrottles not being sent over the network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

Bug: 899268, 900574 
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606247}
[modify] https://crrev.com/c460cb60dd046f595ed2ead9762b95b61c7323a9/content/browser/loader/loader_browsertest.cc
[modify] https://crrev.com/c460cb60dd046f595ed2ead9762b95b61c7323a9/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/c460cb60dd046f595ed2ead9762b95b61c7323a9/content/browser/service_worker/service_worker_browsertest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 8

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd8fd0c3fb80e38108a7f562d0cedb0da5380d85

commit cd8fd0c3fb80e38108a7f562d0cedb0da5380d85
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Nov 08 18:44:19 2018

Fix modified headers from URLLoaderThrottles not being sent over the network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

TBR=jam@chromium.org

(cherry picked from commit c460cb60dd046f595ed2ead9762b95b61c7323a9)

Bug: 899268, 900574 
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606247}
Reviewed-on: https://chromium-review.googlesource.com/c/1326961
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1076}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/cd8fd0c3fb80e38108a7f562d0cedb0da5380d85/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/cd8fd0c3fb80e38108a7f562d0cedb0da5380d85/content/browser/service_worker/service_worker_browsertest.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/cd8fd0c3fb80e38108a7f562d0cedb0da5380d85

Commit: cd8fd0c3fb80e38108a7f562d0cedb0da5380d85
Author: jam@chromium.org
Commiter: jam@chromium.org
Date: 2018-11-08 18:44:19 +0000 UTC

Fix modified headers from URLLoaderThrottles not being sent over the network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

TBR=jam@chromium.org

(cherry picked from commit c460cb60dd046f595ed2ead9762b95b61c7323a9)

Bug: 899268, 900574 
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606247}
Reviewed-on: https://chromium-review.googlesource.com/c/1326961
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#1076}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 8

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f155fb3bc6f3d79b25b0feff4217fb22deb04ee0

commit f155fb3bc6f3d79b25b0feff4217fb22deb04ee0
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Nov 08 19:36:02 2018

Fix modified headers from URLLoaderThrottles not being sent over the network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

TBR=jam@chromium.org

(cherry picked from commit c460cb60dd046f595ed2ead9762b95b61c7323a9)

Bug: 899268, 900574 
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606247}
Reviewed-on: https://chromium-review.googlesource.com/c/1326803
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#584}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/f155fb3bc6f3d79b25b0feff4217fb22deb04ee0/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/f155fb3bc6f3d79b25b0feff4217fb22deb04ee0/content/browser/service_worker/service_worker_browsertest.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/f155fb3bc6f3d79b25b0feff4217fb22deb04ee0

Commit: f155fb3bc6f3d79b25b0feff4217fb22deb04ee0
Author: jam@chromium.org
Commiter: jam@chromium.org
Date: 2018-11-08 19:36:02 +0000 UTC

Fix modified headers from URLLoaderThrottles not being sent over the network.

r605518 fixed a modified URL from URLLoaderThrottles not being sent over the network. However it didn't fix the headers case, which is used by other policies. This fixes the ForceYouTubeRestrict and AllowedDomainsForApps policies.

TBR=jam@chromium.org

(cherry picked from commit c460cb60dd046f595ed2ead9762b95b61c7323a9)

Bug: 899268, 900574 
Change-Id: I5d9666a7497c2c7e5642e5c74d5bc65dc0407ea9
Reviewed-on: https://chromium-review.googlesource.com/c/1324932
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#606247}
Reviewed-on: https://chromium-review.googlesource.com/c/1326803
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#584}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Labels: TE-Verified-70.0.3538.102 TE-Verified-M70
Verified this issue on Windows 10 with chrome #70.0.3538.102 as steps mentioned in the comment #0 and observed the policy "AllowedDomainsForApps" is working as intended. Hence adding the verified the labels

Attaching the screen-cast for reference.
900574.mp4
2.4 MB View Download
Status: Fixed (was: Started)

Sign in to add a comment