Disable/enable echo for enterprise device.

chrome/browser/chromeos/extensions/echo_private_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/extensions/echo_private_api.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
 
 namespace {
 
 // For a given registration code type, returns the code value from the
 // underlying system. Caller owns the returned pointer.
 base::Value* GetValueForRegistrationCodeType(std::string& type) {
   // Possible ECHO code type and corresponding key name in StatisticsProvider.
   const std::string kCouponType = "COUPON_CODE";
   const std::string kCouponCodeKey = "ubind_attribute";
   const std::string kGroupType = "GROUP_CODE";
   const std::string kGroupCodeKey = "gbind_attribute";
 
   chromeos::system::StatisticsProvider* provider =
       chromeos::system::StatisticsProvider::GetInstance();
   std::string result;
   if (!chromeos::KioskModeSettings::Get()->IsKioskModeEnabled()) {

[Mattias Nissler (ping if slow), 2013/02/04 09:06:19]

You should check the policy value here too - we want some confidence that
there's no way to retrieve coupon codes when the policy says echo is disabled.

     // In Kiosk mode, we effectively disable the registration API
     // by always returning an empty code.
     if (type == kCouponType)
       provider->GetMachineStatistic(kCouponCodeKey, &result);
     else if (type == kGroupType)
       provider->GetMachineStatistic(kGroupCodeKey, &result);
   }
   return new base::StringValue(result);
 }
 
@@ skipping 43 matching lines @@
     base::Time::Exploded ctime;
     fileInfo.creation_time.UTCExplode(&ctime);
     timestamp += base::StringPrintf("%u-%u-%u",
                                     ctime.year,
                                     ctime.month,
                                     ctime.day_of_month);
   }
   SetResult(new base::StringValue(timestamp));
   return true;
 }
+
+IsEchoDisabledFunction::IsEchoDisabledFunction() {
+}
+
+IsEchoDisabledFunction::~IsEchoDisabledFunction() {
+}
+
+// Check the enterprise policy kChromeOsRegistrationEnabled flag value.
+// This policy is used to enable/disable Echo for enterprise device.
+bool IsEchoDisabledFunction::RunImpl() {
+  bool isEnabled = true;
+  // If the policy is not provided, isEnabled will not be modified by
+  // GetBoolean.

[oscarpan, 2013/02/02 01:50:56]

I assume this comment is correct and seems so from my manual test, but reviewers
please convince me. Thanks a lot!

[xiyuan, 2013/02/02 07:49:27]

I think the comment is correct. If not confortable, you can use return value of
GetBoolean to set the fallback value.

[Mattias Nissler (ping if slow), 2013/02/04 09:06:19]

It should be you convincing the reviewers ;) In these cases, please make the
effort to check the source whether the assumption is justified.

+  chromeos::CrosSettings::Get()->GetBoolean(
+    chromeos::kChromeOsRegistrationEnabled, &isEnabled);

[xiyuan, 2013/02/02 07:49:27]

nit: 4-space indent

[xiyuan, 2013/02/02 07:49:27]

Do we need to worry that this value might not be trusted?

[oscarpan, 2013/02/02 21:52:35]

I'm not sure when this value is unreliable. Is there any way a user can modify
the policy value? This reminds me another thing, right now I create this
separate method to check whether Echo is disabled. But if a user can modify the
extension code, he can still bypass this check and claim offers using Echo. Is
that even possible to modify private extension? If so, it seems safer to remove
this new api and add the check inside GetRegistrationCodeFunction, and never
return the registration code if Echo is disabled.

[xiyuan, 2013/02/02 22:44:16]

I am not worrying about extension code being changed. It's more about the value
returned from GetBoolean call. I have an impression that the settings values are
cached locally somewhere to speed things up. The cached values are not safe
because user might temper with it. If this is a problem, then you need to call
CrsoSettings::PrepareTrustedValues to ensure GetBoolean will return a trusted
value (value that passed signature check etc). 

It's quite simple to use. You simple add a PrepareTrustedValues call before
you call GetBoolean and put your function as the callback when trusted values
are available. Of course, if you do this, you might need to change it to be an
AsyncExtensionFunction.

One example could be found here:
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/polic...

[oscarpan, 2013/02/05 00:17:47]

Done.

[oscarpan, 2013/02/05 00:17:47]

How about we postpone this feature to later cls.

+  SetResult(new base::FundamentalValue(!isEnabled));
+  return true;
+}