Fix code expecting "has_request_token() => is enterprise managed" |
||||||
Issue descriptionA few instances in code expect that the existence of a request token (DM token) in enterprise_management::PolicyData implies that the device is enterprise managed. This is currently not true for Chromad, where the device is AD enterprise managed, but there's no request token. Can we just assign a fake request token in authpolicyd? https://cs.chromium.org/search/?q=%5Cbhas_request_token%5Cb&type=cs https://cs.corp.google.com/search/?q=has_request_token+package:%5Echromeos_public$&m=100&type=cs
,
May 16 2017
Presence of the request token is a legacy way of checking whether a device or user is managed or not. The new way is the |management_mode| field. Note that you'll have to take into account backwards compatibility, i.e. if management_mode is not present, fall back to the token presence check. However, for chromad you can probably make sure that management_mode is set adequately (maybe you even want to add another enum value for this case?).
,
May 22 2017
From reading the Chrome InstallAttributes code [1], in case "enterprise.mode" isn't present, it's possible to fallback to "enterprise.owned". [1] https://cs.chromium.org/chromium/src/chrome/browser/chromeos/settings/install_attributes.cc?rcl=863dc7c7934fb53d57d29173a5c99a47f3a7bac6&l=509
,
May 22 2017
FTR: "enterprise.mode" was added in M19, CL [1] "enterprise.owned" was added in M12, CL [2] Discounting mario, alex and zbg which are already EOL, there's only pre-19 FSIs for lumpy and stumpy. Thus we need to wait for lumpy/stumpy EOL before "enterprise.owned" can be removed. [1] https://chromiumcodereview.appspot.com/9403010 [2] https://codereview.chromium.org/6821075
,
May 24 2017
Moved enterprise.owned deprecation to a separate bug: issue 725918. Also moving cc:mnissler there.
,
May 24 2017
Probably we should go through the PolicyFetchResponse and PolicyData protos and update the docs wherever AD behaviour differs from cloud management. FTR: authpolicy assembles these protos in AuthPolicy::StorePolicy() and leaves almost all of the fields unset.
,
May 24 2017
In the Chrome codebase, I could only find one use of has_request_token() that seemed problematic. (The others either looked legit or seemed confined to cloud-only code parts.) CL: https://codereview.chromium.org/2902183002/
,
May 24 2017
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c3667249b09c5407d026720f84dcbbae0784b93c commit c3667249b09c5407d026720f84dcbbae0784b93c Author: Thiemo Nagel <tnagel@chromium.org> Date: Tue May 30 23:29:03 2017 login: Fix calculation of management state Use management_mode to determine whether device is managed and only fall back to DM token in case the former doesn't exist. This is required to properly support Chromad, but should have no effect on other types of management (including local ownership). BUG= chromium:722799 TEST=unit test added Change-Id: I8f081c6779cf9a5c38de0e11e59524d074738419 Reviewed-on: https://chromium-review.googlesource.com/518015 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/c3667249b09c5407d026720f84dcbbae0784b93c/login_manager/device_policy_service.cc [modify] https://crrev.com/c3667249b09c5407d026720f84dcbbae0784b93c/login_manager/device_policy_service.h [modify] https://crrev.com/c3667249b09c5407d026720f84dcbbae0784b93c/login_manager/device_policy_service_unittest.cc
,
May 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/external/libbrillo/+/d710b3bbaf7a8253830cbe9d2e720ac8bde16251 commit d710b3bbaf7a8253830cbe9d2e720ac8bde16251 Author: Thiemo Nagel <tnagel@chromium.org> Date: Wed May 31 19:38:39 2017 libbrillo: Add DevicePolicy::IsEnterpriseManaged() Use management_mode to determine whether device is managed and only fall back to DM token in case the former doesn't exist. This is required to properly support Chromad, but should have no effect on other types of management (including local ownership). BUG= chromium:722799 TEST=unit tests added Change-Id: Ifa8bc6bf616dc23eb767c97934fe4cdb77a3451a Reviewed-on: https://chromium-review.googlesource.com/518045 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [add] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/policy/tests/device_policy_impl_unittest.cc [modify] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/policy/mock_device_policy.h [modify] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/policy/device_policy_impl.h [modify] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/libbrillo.gypi [modify] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/policy/device_policy.h [modify] https://crrev.com/d710b3bbaf7a8253830cbe9d2e720ac8bde16251/policy/device_policy_impl.cc
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92db6b2c14f28d8339590ea4c6a0186d9bbf08c5 commit 92db6b2c14f28d8339590ea4c6a0186d9bbf08c5 Author: tnagel <tnagel@chromium.org> Date: Thu Jun 01 09:47:34 2017 Improve determination of managed state in DeviceSettingsProvider Use PolicyData::management_mode to determine whether the device has a local owner and only fall back to DM token in case management_mode is unset. That way, the correct determination is guaranteed for both cloud and Active Directory management. (Currently, the code is not broken because AD policy doesn't include a user name, but that might change in the future.) Also update the PolicyData::management_mode documentation to include Active Directory. BUG= 722799 Review-Url: https://codereview.chromium.org/2902183002 Cr-Commit-Position: refs/heads/master@{#476239} [modify] https://crrev.com/92db6b2c14f28d8339590ea4c6a0186d9bbf08c5/chrome/browser/chromeos/settings/device_settings_provider.cc [modify] https://crrev.com/92db6b2c14f28d8339590ea4c6a0186d9bbf08c5/components/policy/proto/device_management_backend.proto
,
Jun 1 2017
,
Jun 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/c7b81397e440e6f0e6e9748ace1033f8fc2df205 commit c7b81397e440e6f0e6e9748ace1033f8fc2df205 Author: Thiemo Nagel <tnagel@chromium.org> Date: Fri Jun 02 12:06:31 2017 login: Source code cosmetics Reorder methods to have the two static ones together. No functional change. BUG= chromium:722799 TEST=existing tests Change-Id: I6a8bf223ac636e2ef78318d630f8440e3f797e61 Reviewed-on: https://chromium-review.googlesource.com/519325 Commit-Ready: Thiemo Nagel <tnagel@chromium.org> Tested-by: Thiemo Nagel <tnagel@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> [modify] https://crrev.com/c7b81397e440e6f0e6e9748ace1033f8fc2df205/login_manager/device_policy_service.cc [modify] https://crrev.com/c7b81397e440e6f0e6e9748ace1033f8fc2df205/login_manager/device_policy_service.h
,
Jul 6 2017
bulk Verify of older or not-user-facing Chromad bugs |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ljusten@chromium.org
, May 16 2017