Skip to content

Commit

Permalink
Add feature flag for forcing DMToken deletion.
Browse files Browse the repository at this point in the history
When this feature is enabled, the DMToken will be deleted regardless of
whether DMServer expects deletion or invalidation.

This is intended for testing purposes while server-side work is being
completed.

Bug: 1318153, b/165633106
Change-Id: I0b1edb71cd081ec7c672fe60f03295989b2057ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689881
Commit-Queue: Igor Ruvinov <igorruvinov@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013172}
  • Loading branch information
Igor Ruvinov authored and Chromium LUCI CQ committed Jun 10, 2022
1 parent ef68631 commit 44fed9f
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 5 deletions.
11 changes: 10 additions & 1 deletion chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3191,6 +3191,13 @@ const FeatureEntry::FeatureVariation kDesktopSharePreviewVariations[] = {
};
#endif // !BUILDFLAG(IS_ANDROID)

#if !BUILDFLAG(IS_CHROMEOS)
const FeatureEntry::FeatureParam kDmTokenDeletionParam[] = {{"forced", "true"}};
const FeatureEntry::FeatureVariation kDmTokenDeletionVariation[] = {
{"(Forced)", kDmTokenDeletionParam, std::size(kDmTokenDeletionParam),
nullptr}};
#endif // !BUILDFLAG(IS_CHROMEOS)

// RECORDING USER METRICS FOR FLAGS:
// -----------------------------------------------------------------------------
// The first line of the entry is the internal name.
Expand Down Expand Up @@ -8777,7 +8784,9 @@ const FeatureEntry kFeatureEntries[] = {
#if !BUILDFLAG(IS_CHROMEOS)
{"dm-token-deletion", flag_descriptions::kDmTokenDeletionName,
flag_descriptions::kDmTokenDeletionDescription, kOsAll,
FEATURE_VALUE_TYPE(policy::features::kDmTokenDeletion)},
FEATURE_WITH_PARAMS_VALUE_TYPE(policy::features::kDmTokenDeletion,
kDmTokenDeletionVariation,
"DmTokenDeletion")},
#endif // !BUILDFLAG(IS_CHROMEOS)

// NOTE: Adding a new flag requires adding a corresponding entry to enum
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2582,7 +2582,7 @@ TEST_F(CloudPolicyClientTest, PolicyReregistrationAfterDMTokenDeletion) {
EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
}

TEST_F(CloudPolicyClientTest, PolicyFetchDMTokenDeletion_FeatureDisabled) {
TEST_F(CloudPolicyClientTest, PolicyFetchDMTokenDeletion_Disabled) {
RegisterClient();
EXPECT_TRUE(client_->is_registered());
EXPECT_FALSE(client_->requires_reregistration());
Expand All @@ -2605,6 +2605,32 @@ TEST_F(CloudPolicyClientTest, PolicyFetchDMTokenDeletion_FeatureDisabled) {
// deletion error detail still results in the "not found" DM status.
EXPECT_EQ(DM_STATUS_SERVICE_DEVICE_NOT_FOUND, client_->status());
}

TEST_F(CloudPolicyClientTest, PolicyFetchDMTokenDeletion_Forced) {
// Enable the forced DMToken deletion feature.
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(features::kDmTokenDeletion,
{{"forced", "true"}});

RegisterClient();
EXPECT_TRUE(client_->is_registered());
EXPECT_FALSE(client_->requires_reregistration());

DeviceManagementService::JobConfiguration::JobType upload_type;
EXPECT_CALL(job_creation_handler_, OnJobCreation)
.WillOnce(DoAll(service_.CaptureJobType(&upload_type),
service_.SendJobResponseAsync(
net::OK, DeviceManagementService::kDeviceNotFound)));
EXPECT_CALL(observer_, OnRegistrationStateChanged);
EXPECT_CALL(observer_, OnClientError);

client_->FetchPolicy();
base::RunLoop().RunUntilIdle();

// The final DM status signals for DMToken deletion even though the
// corresponding error detail was never added to the response.
EXPECT_EQ(DM_STATUS_SERVICE_DEVICE_NEEDS_RESET, client_->status());
}
#endif // !BUILDFLAG(IS_CHROMEOS)

TEST_F(CloudPolicyClientTest, RequestFetchRobotAuthCodes) {
Expand Down
15 changes: 13 additions & 2 deletions components/policy/core/common/cloud/dmserver_job_configurations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,22 @@ DMServerJobConfiguration::MapNetErrorAndResponseToDMStatus(
return DM_STATUS_TEMPORARY_UNAVAILABLE;
case DeviceManagementService::kDeviceNotFound: {
#if !BUILDFLAG(IS_CHROMEOS)
if (!base::FeatureList::IsEnabled(features::kDmTokenDeletion))
return DM_STATUS_SERVICE_DEVICE_NOT_FOUND;

// If the DMToken deletion feature is enabled and forced, the DM status
// corresponding to DMToken deletion will be returned regardless of
// whether DMServer signals for deletion or invalidation. This is a
// temporary feature intended for testing purposes only.
if (base::GetFieldTrialParamByFeatureAsBool(features::kDmTokenDeletion,
/*param_name=*/"forced",
/*default_value=*/false))
return DM_STATUS_SERVICE_DEVICE_NEEDS_RESET;

// The `kDeviceNotFound` response code can correspond to different DM
// statuses depending on the contents of the response body.
em::DeviceManagementResponse response;
if (base::FeatureList::IsEnabled(features::kDmTokenDeletion) &&
response.ParseFromString(response_body) &&
if (response.ParseFromString(response_body) &&
std::find(response.error_detail().begin(),
response.error_detail().end(),
em::CBCM_DELETION_POLICY_PREFERENCE_DELETE_TOKEN) !=
Expand Down
9 changes: 8 additions & 1 deletion ios/chrome/browser/flags/about_flags.mm
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,11 @@
{"(Bookmark This Page)", kPopupMenuBookmarkStringBookmarkThisPage,
std::size(kPopupMenuBookmarkStringBookmarkThisPage), nullptr}};

const FeatureEntry::FeatureParam kDmTokenDeletionParam[] = {{"forced", "true"}};
const FeatureEntry::FeatureVariation kDmTokenDeletionVariation[] = {
{"(Forced)", kDmTokenDeletionParam, std::size(kDmTokenDeletionParam),
nullptr}};

// To add a new entry, add to the end of kFeatureEntries. There are four
// distinct types of entries:
// . ENABLE_DISABLE_VALUE: entry is either enabled, disabled, or uses the
Expand Down Expand Up @@ -972,7 +977,9 @@
"BookmarkString")},
{"dm-token-deletion", flag_descriptions::kDmTokenDeletionName,
flag_descriptions::kDmTokenDeletionDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(policy::features::kDmTokenDeletion)},
FEATURE_WITH_PARAMS_VALUE_TYPE(policy::features::kDmTokenDeletion,
kDmTokenDeletionVariation,
"DmTokenDeletion")},
};

bool SkipConditionalFeatureEntry(const flags_ui::FeatureEntry& entry) {
Expand Down

0 comments on commit 44fed9f

Please sign in to comment.