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

Issue 839353 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature



Sign in to add a comment

Forced enrollment for pre-enrolled Chrome devices (impl)

Project Member Reported by pmarko@chromium.org, May 3 2018

Issue description

Implement forced enrollment for pre-provisioned Chrome devices.

Launch bug: bug 838527
DD: go/chromeos-oobe-forced-enrollment-dd
 
Labels: -Type-Bug Type-Feature
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, May 9 2018

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

commit 992d634f4029ad291cf82523e7456fa63f9ee139
Author: Pavol Marko <pmarko@chromium.org>
Date: Wed May 09 12:55:25 2018

Proto changes for forced enrollment on OOBE

To check for initial enrollment on OOBE, the system will first perform a
|DeviceInitialEnrollmentCheckRequest| exchange with the device
management server.
If the result of this exchange is that the device should be
force-enrolled, it will use |DeviceInitialEnrollmentStateRequest|
to query which domain it should enroll into.

Bug:  839353 
Test: compile. Actual test will be added in a follow-up CL that uses these protos.
Change-Id: Iaa83065f339f9c49cb7af25c9e1e34bbdd1ad176
Reviewed-on: https://chromium-review.googlesource.com/1042186
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557160}
[modify] https://crrev.com/992d634f4029ad291cf82523e7456fa63f9ee139/components/policy/proto/device_management_backend.proto

Project Member

Comment 4 by bugdroid1@chromium.org, May 10 2018

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

commit 055e83c372dc31752b3a64d450038644c9f2b7dd
Author: Pavol Marko <pmarko@chromium.org>
Date: Thu May 10 19:40:20 2018

Add support for forced initial enrollment check

Add support for the forced initial enrollment check in
AutoEnrollmentClient. This check uses a different identifier set, 8 byte
hashes and a different message for the state download.
The differences are abstracted by using two virtual classes with
different implementations for FRE/initial enrollment checks:
DeviceIdentifierProvider specifies the identifier set and hashes, and
StateDownloadMessageProcessor understands fills state download requests
and parses state download responses.

Bug:  839353 
Test: unit_tests --gtest_filter=*AutoEnrollmentClientTest*
Change-Id: I83d041f568d84c42b9b8cf00a1e44955f9d5abe3
Reviewed-on: https://chromium-review.googlesource.com/1049929
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557627}
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/chrome/browser/chromeos/policy/auto_enrollment_client.cc
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/chrome/browser/chromeos/policy/auto_enrollment_client.h
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/chrome/browser/chromeos/policy/auto_enrollment_client_unittest.cc
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/components/policy/core/common/cloud/cloud_policy_constants.cc
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/components/policy/core/common/cloud/cloud_policy_constants.h
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/components/policy/core/common/cloud/device_management_service.cc
[modify] https://crrev.com/055e83c372dc31752b3a64d450038644c9f2b7dd/components/policy/core/common/cloud/device_management_service.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 15 2018

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

commit a12b78ab3be655a8eb859e6a958f71bbbf535452
Author: Pavol Marko <pmarko@chromium.org>
Date: Tue May 15 19:08:37 2018

Perform initial enrollment check for Chrome OS

If FRE is not performed, and initial enrollment is enabled according to
command line flags and device state, performs initial enrollment check.
Specifically, the initial enrollment check is done if all of the
following conditions are true:
- the device was not enterprise enrolled or in consumer mode before
  (according to the check_enrollment VPD variable)
- the RLZ embargo date has passed
  (to ensure not to do initial enrollment exchanges on the factory floor)
- Initial enrollment is enabled usinga  command-line switch
- serial number and RLZ brand code are available.

Bug:  839353 
Change-Id: I259c06a5e5274f3bdef54ccc0161e6f642f9499f
Reviewed-on: https://chromium-review.googlesource.com/1052707
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558797}
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/login/enrollment/auto_enrollment_check_screen.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/policy/server_backed_state_keys_broker.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/policy/server_backed_state_keys_broker.h
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/chromeos/tpm_firmware_update.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chromeos/chromeos_switches.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/chromeos/chromeos_switches.h
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/rlz/chromeos/lib/rlz_value_store_chromeos.cc
[modify] https://crrev.com/a12b78ab3be655a8eb859e6a958f71bbbf535452/rlz/chromeos/lib/rlz_value_store_chromeos.h

Comment 6 by pmarko@chromium.org, May 16 2018

Dev-Tested initial enrollment functionality with YAPS using:

rm -rf /tmp/cpfe1 && \
  mkdir /tmp/cpfe1 && \
  echo "\"serial_number\"=\"SN1234\"" > /tmp/cpfe1/stub_machine-info && \
  echo "\"rlz_brand_code\"=\"ABCD\"" >> /tmp/cpfe1/stub_machine-info && \
  ~/chromium/src/out/Default/chrome \
      --login-manager \
      --user-data-dir=/tmp/cpfe1 \
      --enterprise-enable-initial-enrollment=always \
      --vmodule=*auto_enrollment*=1 \
      --device-management-url=http://localhost:8080/d/pmarko
      --enable-logging=stderr
Project Member

Comment 7 by bugdroid1@chromium.org, May 17 2018

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

commit f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef
Author: Pavol Marko <pmarko@chromium.org>
Date: Thu May 17 06:47:10 2018

Send brand code on enterprise enrollment

Sends the brand code in the DeviceRegisterRequest.
The server may remove initial enrollment device state from its database
when the device successfully enrolls. As initial enrollment device state
is keyed by the serial number / brand code tuple, brand code is now also
transferred on enrollment.

Bug:  839353 
Test: unit_tests --gtest_filter=CloudPolicyClientTest*
Change-Id: I57205c2eb3b219ab7c2793a47e935e376216e703
Reviewed-on: https://chromium-review.googlesource.com/1057675
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559439}
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/chromeos/login/existing_user_controller.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/extensions/api/enterprise_reporting_private/enterprise_reporting_private_api.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/chrome/browser/policy/cloud/machine_level_user_cloud_policy_helper.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/cloud/cloud_policy_client.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/cloud/cloud_policy_client.h
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/cloud/cloud_policy_client_unittest.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/cloud/mock_cloud_policy_client.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/cloud/user_cloud_policy_manager.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc
[modify] https://crrev.com/f6523e08a20d9e3825ed82f2b3c63b26c8d5bcef/components/policy/proto/device_management_backend.proto

Project Member

Comment 8 by bugdroid1@chromium.org, May 19 2018

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

commit ac1f561499f58993736ccacf0de3cdd7067b07ff
Author: Pavol Marko <pmarko@chromium.org>
Date: Sat May 19 21:44:59 2018

Extract factory ping embargo check

Moves the factory ping embargo check from rlz_lib::RlzValueStoreChromeOS
to the function chromeos::system::GetFactoryPingEmbargoState.
This avoids a dependency from chromeos to rlz_lib.

Bug:  839353 
Test: chromeos_unittests && rlz_unittests
Change-Id: Ia20c9a268d1030a77b1e138632005866c24d0788
Reviewed-on: https://chromium-review.googlesource.com/1059409
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560170}
[modify] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/chromeos/BUILD.gn
[add] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/chromeos/system/factory_ping_embargo_check.cc
[add] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/chromeos/system/factory_ping_embargo_check.h
[add] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/chromeos/system/factory_ping_embargo_check_unittest.cc
[modify] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/rlz/chromeos/lib/rlz_value_store_chromeos.cc
[modify] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/rlz/chromeos/lib/rlz_value_store_chromeos.h
[modify] https://crrev.com/ac1f561499f58993736ccacf0de3cdd7067b07ff/rlz/lib/financial_ping_test.cc

Project Member

Comment 9 by bugdroid1@chromium.org, May 23 2018

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

commit 085ec0d9d2481a1cf295a4dc4b880fe329fdd648
Author: Pavol Marko <pmarko@chromium.org>
Date: Wed May 23 11:49:22 2018

Add histograms for Initial Enrollment

Defines the ".ForcedReenrollment" and ".InitialEnrollment" suffixes for
auto enrollment histograms.
AutoEnrollmentClient now supports setting the histogram suffix.

Also adds a new histogram
Enterprise.AutoEnrollmentBucketDownloadTime{.ForcedReenrollment,.InitialEnrollment),
which measures the time required for the
DeviceAutoEnrollment{Request,Response} exchange which actually transfers
the hash bucket.

Bug:  839353 
Change-Id: I22c5a4df50a3e760e2301fc1763962cfa646b552
Reviewed-on: https://chromium-review.googlesource.com/1065879
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561039}
[modify] https://crrev.com/085ec0d9d2481a1cf295a4dc4b880fe329fdd648/chrome/browser/chromeos/policy/auto_enrollment_client.cc
[modify] https://crrev.com/085ec0d9d2481a1cf295a4dc4b880fe329fdd648/chrome/browser/chromeos/policy/auto_enrollment_client.h
[modify] https://crrev.com/085ec0d9d2481a1cf295a4dc4b880fe329fdd648/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, May 23 2018

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

commit 35bcf1a3e21f3add1c89587238ff5c13d8f5879e
Author: Pavol Marko <pmarko@chromium.org>
Date: Wed May 23 15:33:07 2018

Show Network Error Screen on Server Errors in Initial Enrollment

If a Server Error (e.g. HTTP 500) is encountered during the Initial
Enrollment exchange, show a network error screen instead of proceeding
without enrollment.
Also, refactor AutoEnrollmentClient to support a fake implementation
(FakeAutoEnrollmentClient). The test can make AutoEnrollmentController
use the fake by setting a factory for testing.

BUG= 839353 
Test=browser_tests --gtest_filter=WizardControllerDeviceState*

Change-Id: I6ef18f093ba26a1ccbb91221a4d504aac8a6f286
Reviewed-on: https://chromium-review.googlesource.com/1065821
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561098}
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/login/enrollment/auto_enrollment_check_screen.cc
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/login/enrollment/auto_enrollment_check_screen.h
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/auto_enrollment_client.h
[rename] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc
[add] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h
[rename] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
[add] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/fake_auto_enrollment_client.cc
[add] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/chromeos/policy/fake_auto_enrollment_client.h
[modify] https://crrev.com/35bcf1a3e21f3add1c89587238ff5c13d8f5879e/chrome/browser/prefs/browser_prefs.cc

Status: Fixed (was: Started)
Histograms / improved server error handling now also landed; marking this as Fixed.

This feature is still behind a command-line switch and can be enabled using
--enterprise-enable-initial-enrollment=always
Project Member

Comment 12 by bugdroid1@chromium.org, May 27 2018

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

commit 83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98
Author: Pavol Marko <pmarko@chromium.org>
Date: Sun May 27 06:31:26 2018

Enable Initial Enrollment for official builds and detect outdated server

This enables Initial Enrollment by default for official builds.
Also, this adds functionality to detect a server which does not support
Initial Enrollment by comparing against a maximal expected modulus
value.

Bug:  839353 
Test: unit_tests --gtest_filter=AutoEnrollmentClient*
Change-Id: I1b64ab2875c7317dc92c74276173e27b1c58bbe2
Reviewed-on: https://chromium-review.googlesource.com/1073309
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562143}
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/auto_enrollment_client.h
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/fake_auto_enrollment_client.cc
[modify] https://crrev.com/83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98/chrome/browser/chromeos/policy/fake_auto_enrollment_client.h

Labels: Merge-Request-68
Requesting merge of the commit in Comment #12 to M-68 to enable the feature by default for the test runs.
Project Member

Comment 14 by sheriffbot@chromium.org, May 28 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 15 by bugdroid1@chromium.org, May 28 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c948279723d2c34c7a91c9934921bde0d0c63e6

commit 1c948279723d2c34c7a91c9934921bde0d0c63e6
Author: Pavol Marko <pmarko@chromium.org>
Date: Mon May 28 14:36:31 2018

[Merge to M-68] Enable Initial Enrollment for official builds and detect outdated server

This enables Initial Enrollment by default for official builds.
Also, this adds functionality to detect a server which does not support
Initial Enrollment by comparing against a maximal expected modulus
value.

TBR=pmarko@chromium.org

(cherry picked from commit 83a6b5720768a47a1b6fa5c7fd80ad6e750b3c98)

Bug:  839353 
Test: unit_tests --gtest_filter=AutoEnrollmentClient*
Change-Id: I1b64ab2875c7317dc92c74276173e27b1c58bbe2
Reviewed-on: https://chromium-review.googlesource.com/1073309
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#562143}
Reviewed-on: https://chromium-review.googlesource.com/1075272
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#12}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/auto_enrollment_client.h
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/auto_enrollment_client_impl.h
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/fake_auto_enrollment_client.cc
[modify] https://crrev.com/1c948279723d2c34c7a91c9934921bde0d0c63e6/chrome/browser/chromeos/policy/fake_auto_enrollment_client.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 4 2018

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

commit 7cc7cf59ea126a1b07f4e3d0cb2d2d37e041e5ca
Author: Pavol Marko <pmarko@chromium.org>
Date: Mon Jun 04 07:53:40 2018

Add UMA histogram for factory ping embargo end date validity

Adds UMA histogram "FactoryPingEmbargo.EndDateValidity" which is used to
track how often the VPD value representing the factory ping embargo end
date is malformed, invalid or valid.
The histogram is emited to when the value is accessed, which could be
multiple times in the same chrome session.
If the value is not present in VPD, nothing is emited to the histogram.

Bug:  839353 
Test: manual
Change-Id: I63ece5e84d66bae89b231b466eb1d0eac1829b3b
Reviewed-on: https://chromium-review.googlesource.com/1069135
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564010}
[modify] https://crrev.com/7cc7cf59ea126a1b07f4e3d0cb2d2d37e041e5ca/chromeos/system/factory_ping_embargo_check.cc
[modify] https://crrev.com/7cc7cf59ea126a1b07f4e3d0cb2d2d37e041e5ca/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/7cc7cf59ea126a1b07f4e3d0cb2d2d37e041e5ca/tools/metrics/histograms/histograms.xml

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 6 2018

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

commit 9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9
Author: Pavol Marko <pmarko@chromium.org>
Date: Wed Jun 06 12:07:19 2018

Attempt system clock sync if in factory ping embargo period

If the device is in factory ping embargo period, or the factory ping
embargo end date is invalid (too far in the future), attempt to
synchronize the system clock with the network before continuing,
re-checking if the device is still in the factory ping embargo period
after the clock has been synchronized.
The system clock sync has a timeout of 15 seconds, if no sync could be
reached by then, assume that no Initial Enrollment should be done.

Also add an UMA histogram to track if Initial Enrollment was required,
and if not, why not.

Bug:  839353 
Test: browser_tests --gtest_filter=WizardControllerDeviceState*
Change-Id: I9094dd19f141e8a4dbdb8dcdcd16ef90d1ec2af5
Reviewed-on: https://chromium-review.googlesource.com/1076233
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564861}
[modify] https://crrev.com/9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h
[modify] https://crrev.com/9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-68
Requesting merge to M-68 for the commits in Comment #16 and Comment #17 (Chrome OS only).

Comment #16 does not change behavior - it introduces a UMA histogram for better monitoring of the feature.

Comment #17 addresses a concern raised recently which could make the feature ineffective on new devices that have lost battery and their system clock has reset. It introduces a up to 15 seconds wait time until the system clock has been synchronized before continuing if the initial enrollment check would have been skipped due to the system clock time.
In addition to the browsertests, I've performed manual verification steps of the behavior of CL https://chromium-review.googlesource.com/1076233 (the commit in Comment #17):

== Common preparation for all test cases ==
- Ensured that the device is not enrolled and that the files "/home/chronos/Local State" and /home/chronos/.oobe_completed do not exist.
- Ensured that the device SN/rlz_brand_code are registered for enrollment in YAPS
- Ensured that I'm running an official build or have --enterprise-enable-initial-enrollment=always
- Ensured that I'm using YAPS (--device-management-url)
- VPD preparation (using update_rw_vpd):
-- removed "ActivateDate"
-- removed "check_enrollment"
-- set "rlz_embargo_end_date" to a date that is AFTER tlsdated's build time and BEFORE now. In my case it was "2018-06-05".
- stop ui
- stop tlsdated
- Manually set the date somewhere before tlsdate's build date:
  date +%Y%m%d -s "20180401"

== Test case 1 (no tlsdated service) ==
- Run common preparation (see above)
- start ui
=> observed that "Determining device configuration" is displayed for ~15 seconds before continuing
observed message in /var/log/chrome/chrome:
[8310:8310:0401/000535.671333:WARNING:auto_enrollment_controller.cc(503)] Skip Initial Enrollment Check due to invalid embargo date (system clock sync failed).

== Test case 2 (tlsdated runs but can't sync) ==
- run common preparatoin (see above)
- Ensure that you have a network where tlsdated servers are not reachable (I use an ethernet network without connectivity for this).
- start tlsdated
- start ui

=> Observed
[9685:9685:0604/121835.780171:WARNING:auto_enrollment_controller.cc(513)] Skip Initial Enrollment Check because the device is in the embargo period  (system clock sync failed).
in the logs.

== Test case 3 (tlsdated can sync) ==
- run common preparation (see above)
- Ensure that you have a wifi network available where tlsdated servers are reachable.
- start tlsdated
- start ui
- Connect to the network in OOBE

=> observed that the "Enterprise Enrollment" screen is shown.

Summary:
=> All three test cases were OK.

Project Member

Comment 20 by sheriffbot@chromium.org, Jun 7 2018

Labels: -Merge-Request-68 Merge-Reject-68 Hotlist-Merge-Reject
The bug is marked as P3 or Feature. It should not be merged as M68 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: bhthompson@chromium.org
Actually, it is important that the two CLs mentioned make it to M-68 as many devices will start with M-68 FSI and the feature needs to be complete in the FSI to work.

The first of those CLs is UMA only for monitoring.
The second one improves the behavior I'm a certain combination of conditions.
Labels: -Hotlist-Merge-Reject -Merge-Reject-68 Merge-Approved-68
If these are just adding UMA stats and fixing a bug this is ok, to make this easier in the future we may want to file a bug for what this is fixing up separate from the feature. 
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 8 2018

Labels: -merge-approved-68
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3616cc35cbe4f5cc5c27a4584397ba762519a1fe

commit 3616cc35cbe4f5cc5c27a4584397ba762519a1fe
Author: Pavol Marko <pmarko@chromium.org>
Date: Fri Jun 08 09:24:02 2018

[Merge to M68] Add UMA histogram for factory ping embargo end date validity

Adds UMA histogram "FactoryPingEmbargo.EndDateValidity" which is used to
track how often the VPD value representing the factory ping embargo end
date is malformed, invalid or valid.
The histogram is emited to when the value is accessed, which could be
multiple times in the same chrome session.
If the value is not present in VPD, nothing is emited to the histogram.

TBR=pmarko@chromium.org

(cherry picked from commit 7cc7cf59ea126a1b07f4e3d0cb2d2d37e041e5ca)

Bug:  839353 
Test: manual
Change-Id: I63ece5e84d66bae89b231b466eb1d0eac1829b3b
Reviewed-on: https://chromium-review.googlesource.com/1069135
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564010}
Reviewed-on: https://chromium-review.googlesource.com/1091750
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#257}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/3616cc35cbe4f5cc5c27a4584397ba762519a1fe/chromeos/system/factory_ping_embargo_check.cc
[modify] https://crrev.com/3616cc35cbe4f5cc5c27a4584397ba762519a1fe/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/3616cc35cbe4f5cc5c27a4584397ba762519a1fe/tools/metrics/histograms/histograms.xml

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 8 2018

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

commit 484db84619d051c07a5701a6a790088b1d4686f6
Author: Pavol Marko <pmarko@chromium.org>
Date: Fri Jun 08 09:24:24 2018

[Merge to M68] Attempt system clock sync if in factory ping embargo period

If the device is in factory ping embargo period, or the factory ping
embargo end date is invalid (too far in the future), attempt to
synchronize the system clock with the network before continuing,
re-checking if the device is still in the factory ping embargo period
after the clock has been synchronized.
The system clock sync has a timeout of 15 seconds, if no sync could be
reached by then, assume that no Initial Enrollment should be done.

Also add an UMA histogram to track if Initial Enrollment was required,
and if not, why not.

TBR=pmarko@chromium.org

(cherry picked from commit 9f7f4ee45a060f1059fb9c28b42fdb5fb8c2dce9)

Bug:  839353 
Test: browser_tests --gtest_filter=WizardControllerDeviceState*
Change-Id: I9094dd19f141e8a4dbdb8dcdcd16ef90d1ec2af5
Reviewed-on: https://chromium-review.googlesource.com/1076233
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564861}
Reviewed-on: https://chromium-review.googlesource.com/1091751
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#258}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/484db84619d051c07a5701a6a790088b1d4686f6/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.cc
[modify] https://crrev.com/484db84619d051c07a5701a6a790088b1d4686f6/chrome/browser/chromeos/login/enrollment/auto_enrollment_controller.h
[modify] https://crrev.com/484db84619d051c07a5701a6a790088b1d4686f6/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/484db84619d051c07a5701a6a790088b1d4686f6/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/484db84619d051c07a5701a6a790088b1d4686f6/tools/metrics/histograms/histograms.xml

Status: Verified (was: Fixed)
verified as part of launch bug testing
Cc: apronin@chromium.org

Sign in to add a comment