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

Issue 666720 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug



Sign in to add a comment

Policy fetch broken on ToT

Project Member Reported by tnagel@chromium.org, Nov 18 2016

Issue description

Policy fetch fails with "Invalid request or request parameters", apparently due to a new production version of DM server not ignoring unknown policy types anymore.

(Attaching a patch for a quick and dirty workaround to unblock developers.)
 
patch
930 bytes View Download

Comment 1 by emaxx@chromium.org, Nov 18 2016

Cc: dkalin@chromium.org atwilson@chromium.org dskaram@chromium.org
Status: Started (was: Assigned)
That's because of changes made in  issue 644304 , which were made in assumption that DMServer will just ignore requests with unknown policy types - which is apparently no longer true.

I'm preparing CL with a quick fix that will disable the new functionality on the client side.

Comment 2 by emaxx@chromium.org, Nov 18 2016

Labels: CrosExtendedLogin
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 18 2016

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

commit f14a5fdb16cda4b707b127d1c8cf62115a72f709
Author: emaxx <emaxx@chromium.org>
Date: Fri Nov 18 14:21:32 2016

Disable fetching of policy for login screen apps

This disables fetching of policies for login screen apps, which used the
new policy type "google/chromeos/signinextension", as DMServer started
to respond with errors to requests with unknown policy types.

BUG= 666720 , 644304 
TEST=manual (run chromeos=1 build with an enrolled profile, navigate to chrome://policy, press "Reload policies", check that OK is displayed for the device policy)

Review-Url: https://codereview.chromium.org/2513903003
Cr-Commit-Position: refs/heads/master@{#433190}

[modify] https://crrev.com/f14a5fdb16cda4b707b127d1c8cf62115a72f709/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/f14a5fdb16cda4b707b127d1c8cf62115a72f709/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h

Comment 4 by emaxx@chromium.org, Nov 18 2016

Status: Fixed (was: Started)
This should be fixed now.

Thiemo, thanks for triaging this!

Comment 5 by emaxx@chromium.org, Nov 18 2016

And I'll request a merge once the fix is confirmed to work on ToT.

Comment 6 by emaxx@chromium.org, Nov 18 2016

Labels: Merge-Request-56
This fix is required for enrolled enterprise devices.

Note that the fix disables some functionality (fetching of admin policies for Chrome OS login screen apps), but this was anyway not expected to be fully functional in M56.

Comment 7 by emaxx@chromium.org, Nov 18 2016

Labels: ReleaseBlock-Dev

Comment 8 by dimu@chromium.org, Nov 18 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 18 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee89d810be7379fc0c713cf4f806efe5c38a548d

commit ee89d810be7379fc0c713cf4f806efe5c38a548d
Author: Maksim Ivanov <emaxx@chromium.org>
Date: Fri Nov 18 17:50:09 2016

Disable fetching of policy for login screen apps

This disables fetching of policies for login screen apps, which used the
new policy type "google/chromeos/signinextension", as DMServer started
to respond with errors to requests with unknown policy types.

BUG= 666720 , 644304 
TEST=manual (run chromeos=1 build with an enrolled profile, navigate to chrome://policy, press "Reload policies", check that OK is displayed for the device policy)

Review-Url: https://codereview.chromium.org/2513903003
Cr-Commit-Position: refs/heads/master@{#433190}
(cherry picked from commit f14a5fdb16cda4b707b127d1c8cf62115a72f709)

Review URL: https://codereview.chromium.org/2518613002 .

Cr-Commit-Position: refs/branch-heads/2924@{#3}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/ee89d810be7379fc0c713cf4f806efe5c38a548d/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/ee89d810be7379fc0c713cf4f806efe5c38a548d/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h

Labels: -ReleaseBlock-Dev
Note that device policy issues usually don't warrant blocking the dev release.  (I've only learned that recently.)

https://docs.google.com/document/d/1SVFeHFp6bnpFA6LseShcAPXrAFRaOFjGVeUlx8oo7rk/edit
Labels: M-56
#c10, As you've mentioned, don't think this be added to the dev-blocker qual list. 
But this does block feature testing or Sanity testing efforts. So Pri:0 would be a good label to signify urgency. 

Cc: monachow@chromium.org
Status: Verified (was: Fixed)
M	ChromeOS	Chrome	ARC	Type	Channel
56	9000.15.0	56.0.2924.12	3535779	release	dev


I have been testing Enterprise functionality using M56 builds (dev channel) since this Monday, Nov. 28 and doing a lot of auto update to different builds, unplug and plug Ethernet, power off and power on devices.  chrome://policy page is always reloaded timely with "Policy cache OK".

Comment 14 by roy...@google.com, Dec 8 2016

Issue 668329 has been merged into this issue.
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 30 2016

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

commit 5eb4901df37717dd05fd071a807a0ebaf563f0f3
Author: emaxx <emaxx@chromium.org>
Date: Fri Dec 30 03:29:57 2016

Re-enable fetching component policies for login screen apps

This CL relands enabling fetching of policies for Chrome OS login screen
apps.

The originally landed code was temporarily disabled due to DMServer
responding with errors when an unknown policy type was requested. Now
this can be enabled back, after the DMServer got fixed to at least
ignore the new policy type (until it starts to actually provide policies
with this type).

BUG= 644304 , 666720 
TEST=re-enabled browser tests;
     manual test: sign into an enrolled device, go to chrome://policy
     and check that the device policies were fetched successfully

Review-Url: https://codereview.chromium.org/2603463003
Cr-Commit-Position: refs/heads/master@{#441015}

[modify] https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h
[modify] https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 12 2017

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

commit 3cce5ba611580090dbbd55686e0372a2a4955bbe
Author: tnagel <tnagel@chromium.org>
Date: Thu Jan 12 19:47:10 2017

Revert of Re-enable fetching component policies for login screen apps (patchset #3 id:40001 of https://codereview.chromium.org/2603463003/ )

Reason for revert:
I'm very sorry I have to revert this since it's breaking device policy updates.

https://bugs.chromium.org/p/chromium/issues/detail?id=679956#c7

Original issue's description:
> Re-enable fetching component policies for login screen apps
>
> This CL relands enabling fetching of policies for Chrome OS login screen
> apps.
>
> The originally landed code was temporarily disabled due to DMServer
> responding with errors when an unknown policy type was requested. Now
> this can be enabled back, after the DMServer got fixed to at least
> ignore the new policy type (until it starts to actually provide policies
> with this type).
>
> BUG= 644304 , 666720 
> TEST=re-enabled browser tests;
>      manual test: sign into an enrolled device, go to chrome://policy
>      and check that the device policies were fetched successfully
>
> Committed: https://crrev.com/5eb4901df37717dd05fd071a807a0ebaf563f0f3
> Cr-Commit-Position: refs/heads/master@{#441015}

TBR=emaxx@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 644304 , 666720 

Review-Url: https://codereview.chromium.org/2621413007
Cr-Commit-Position: refs/heads/master@{#443328}

[modify] https://crrev.com/3cce5ba611580090dbbd55686e0372a2a4955bbe/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/3cce5ba611580090dbbd55686e0372a2a4955bbe/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/3cce5ba611580090dbbd55686e0372a2a4955bbe/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h
[modify] https://crrev.com/3cce5ba611580090dbbd55686e0372a2a4955bbe/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Feb 9 2017

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

commit 94c4e6b642d971a31680fbb19858f56e1a4d8d7b
Author: emaxx <emaxx@chromium.org>
Date: Thu Feb 09 00:19:32 2017

Reland Re-enable fetching component policies for login screen apps

This CL relands enabling fetching of policies for Chrome OS login screen
apps.

The originally landed code was temporarily disabled due to DMServer
responding with errors when an unknown policy type was requested. Now
this can be enabled back, after the DMServer got fixed to at least
ignore the new policy type (until it starts to actually provide policies
with this type).

TBR=tnagel@chromium.org

BUG= 644304 , 666720 
TEST=re-enabled browser tests;
     manual test: sign into an enrolled device, go to chrome://policy
     and check that the device policies were fetched successfully

Review-Url: https://codereview.chromium.org/2685893004
Cr-Commit-Position: refs/heads/master@{#449157}

[modify] https://crrev.com/94c4e6b642d971a31680fbb19858f56e1a4d8d7b/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/94c4e6b642d971a31680fbb19858f56e1a4d8d7b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/94c4e6b642d971a31680fbb19858f56e1a4d8d7b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h
[modify] https://crrev.com/94c4e6b642d971a31680fbb19858f56e1a4d8d7b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc

Sign in to add a comment