0

Only call GetDeviceAttributeUpdatePermission for manual enrollment

GetDeviceAttributeUpdatePermission resolves the current user by OAuth
token and then checks if that user may change the device's attributes.
This is only sensible for manual enrollment where a user has actually
authenticated and we have an OAuth token for the user.
For enrollment modes without explicit user auth, such as
attestation-based enrollment or enrollment-token-based enrollment, the
enrolling user is not known so this check is not possible.

For now, assume that device attributes should not be changed in this,
as there is no way to find out if the enrolling user is permitted to
change them.

Bug: 942013
Change-Id: I5f7e08ca2af223d0dbcac3839c6eb2b2f45e62b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566820
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834188}
This commit is contained in:
Pavol Marko
2020-12-07 13:57:19 +00:00
committed by Chromium LUCI CQ
parent 75712bafd6
commit 31a0632196
3 changed files with 21 additions and 3 deletions
chrome/browser/chromeos/login/enrollment
components/policy/core/common/cloud

@ -608,6 +608,10 @@ IN_PROC_BROWSER_TEST_F(AutoEnrollmentLocalPolicyServer, DeviceDisabled) {
// Attestation enrollment.
IN_PROC_BROWSER_TEST_F(AutoEnrollmentLocalPolicyServer, Attestation) {
// Even though the server would allow device attributes update, Chrome OS will
// not attempt that for attestation enrollment.
policy_server_.SetUpdateDeviceAttributesPermission(true);
AllowlistSimpleChallengeSigningKey();
policy_server_.SetFakeAttestationFlow();
EXPECT_TRUE(policy_server_.SetDeviceStateRetrievalResponse(

@ -286,6 +286,19 @@ void EnterpriseEnrollmentHelperImpl::DoEnroll(
void EnterpriseEnrollmentHelperImpl::GetDeviceAttributeUpdatePermission() {
DCHECK(auth_data_);
if (!auth_data_->has_oauth_token()) {
// Checking whether the device attributes can be updated requires knowning
// which user is performing enterprise enrollment, because the permission is
// tied to a user.
// For enterprise enrollment authorized by attestation or an enrollment
// token, the current user is unknown.
// A possible follow-up (tracked in https://crbug.com/942013) will be to
// allow the first affiliated user that signs in and has the permission to
// edit device attributes.
OnDeviceAttributeUpdatePermission(/*granted=*/false);
return;
}
policy::BrowserPolicyConnectorChromeOS* connector =
g_browser_process->platform_part()->browser_policy_connector_chromeos();
// Don't update device attributes for Active Directory management.

@ -720,9 +720,10 @@ void CloudPolicyClient::GetDeviceAttributeUpdatePermission(
CloudPolicyClient::StatusCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(is_registered());
// This condition is wrong in case of Attestation enrollment
// (https://crbug.com/942013).
// DCHECK(auth->has_oauth_token() || auth->has_enrollment_token());
// This request only works with an OAuth token identifying a user, because
// DMServer will resolve that user and check if they have permissions to
// update the device's attributes.
DCHECK(auth->has_oauth_token());
const bool has_oauth_token = auth->has_oauth_token();
const std::string oauth_token =