Create new helper function for checking if users already exist

-New public method for checking users in UsersPrivate API

-Add a new private method just for checking if a user is part of the
user list since two methods need the same functionality.

-New test includes a negative test for not finding a user + a positive
test after a user is added and searched for.

Bug: 768798
Change-Id: I8ec2ca524549fbd8dd9a0813a7bf7777fe7ed2d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1786862
Commit-Queue: Gavin Williams <gavinwill@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699586}

chrome/browser/chromeos/extensions/users_private/users_private_api.cc

@@ -61,6 +61,11 @@ bool CanModifyWhitelistedUsers(Profile* profile) {
   return !IsEnterpriseManaged() && IsOwnerProfile(profile) && !IsChild(profile);
 }
 
+bool IsExistingWhitelistedUser(const std::string& username) {
+  return chromeos::CrosSettings::Get()->FindEmailInList(
+      chromeos::kAccountsPrefUsers, username, /*wildcard_match=*/nullptr);
+}
+
 // Creates User object for the exising user_manager::User .
 api::users_private::User CreateApiUser(const std::string& email,
                                        const user_manager::User& user) {
@@ -87,27 +92,13 @@ api::users_private::User CreateUnknownApiUser(const std::string& email) {
   return api_user;
 }
 
-}  // anonymous namespace
-
-////////////////////////////////////////////////////////////////////////////////
-// UsersPrivateGetWhitelistedUsersFunction
-
-UsersPrivateGetWhitelistedUsersFunction::
-    UsersPrivateGetWhitelistedUsersFunction()
-    : chrome_details_(this) {
-}
-
-UsersPrivateGetWhitelistedUsersFunction::
-    ~UsersPrivateGetWhitelistedUsersFunction() {
-}
-
-ExtensionFunction::ResponseAction
-UsersPrivateGetWhitelistedUsersFunction::Run() {
-  Profile* profile = chrome_details_.GetProfile();
+std::unique_ptr<base::ListValue> GetUsersList(Profile* profile,
+                                      content::BrowserContext* browser_context)
+{
   std::unique_ptr<base::ListValue> user_list(new base::ListValue);
 
   if (!CanModifyWhitelistedUsers(profile))
-    return RespondNow(OneArgument(std::move(user_list)));
+    return user_list;
 
   // Create one list to set. This is needed because user white list update is
   // asynchronous and sequential. Before previous write comes back, cached list
@@ -115,7 +106,7 @@ UsersPrivateGetWhitelistedUsersFunction::Run() {
   std::unique_ptr<base::ListValue> email_list;
 
   UsersPrivateDelegate* delegate =
-      UsersPrivateDelegateFactory::GetForBrowserContext(browser_context());
+      UsersPrivateDelegateFactory::GetForBrowserContext(browser_context);
   PrefsUtil* prefs_util = delegate->GetPrefsUtil();
 
   std::unique_ptr<api::settings_private::PrefObject> users_pref_object =
@@ -167,7 +158,52 @@ UsersPrivateGetWhitelistedUsersFunction::Run() {
             .ToValue());
   }
 
-  return RespondNow(OneArgument(std::move(user_list)));
+  return user_list;
+}
+
+}  // anonymous namespace
+
+////////////////////////////////////////////////////////////////////////////////
+// UsersPrivateGetWhitelistedUsersFunction
+
+UsersPrivateGetWhitelistedUsersFunction::
+    UsersPrivateGetWhitelistedUsersFunction()
+    : chrome_details_(this) {
+}
+
+UsersPrivateGetWhitelistedUsersFunction::
+    ~UsersPrivateGetWhitelistedUsersFunction() {
+}
+
+ExtensionFunction::ResponseAction
+UsersPrivateGetWhitelistedUsersFunction::Run() {
+  Profile* profile = chrome_details_.GetProfile();
+  return RespondNow(OneArgument(GetUsersList(profile, browser_context())));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// UsersPrivateIsWhitelistedUserFunction
+
+UsersPrivateIsWhitelistedUserFunction::UsersPrivateIsWhitelistedUserFunction()
+    : chrome_details_(this) {
+}
+
+UsersPrivateIsWhitelistedUserFunction::
+    ~UsersPrivateIsWhitelistedUserFunction() {}
+
+ExtensionFunction::ResponseAction UsersPrivateIsWhitelistedUserFunction::Run() {
+  std::unique_ptr<api::users_private::IsWhitelistedUser::Params> parameters =
+      api::users_private::IsWhitelistedUser::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(parameters.get());
+
+  // GetUsersList called to clear the stale user name cache
+  GetUsersList(chrome_details_.GetProfile(), browser_context());
+
+  std::string username = gaia::CanonicalizeEmail(parameters->email);
+  if (IsExistingWhitelistedUser(username)) {
+    return RespondNow(OneArgument(std::make_unique<base::Value>(true)));
+  }
+  return RespondNow(OneArgument(std::make_unique<base::Value>(false)));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -193,8 +229,7 @@ UsersPrivateAddWhitelistedUserFunction::Run() {
   }
 
   std::string username = gaia::CanonicalizeEmail(parameters->email);
-  if (chromeos::CrosSettings::Get()->FindEmailInList(
-          chromeos::kAccountsPrefUsers, username, NULL)) {
+  if (IsExistingWhitelistedUser(username)) {
     return RespondNow(OneArgument(std::make_unique<base::Value>(false)));
   }
 

chrome/browser/chromeos/extensions/users_private/users_private_api.h

@@ -33,6 +33,25 @@ class UsersPrivateGetWhitelistedUsersFunction : public ExtensionFunction {
   DISALLOW_COPY_AND_ASSIGN(UsersPrivateGetWhitelistedUsersFunction);
 };
 
+// Implements the chrome.usersPrivate.isWhitelistedUser method.
+class UsersPrivateIsWhitelistedUserFunction : public ExtensionFunction {
+ public:
+  UsersPrivateIsWhitelistedUserFunction();
+  DECLARE_EXTENSION_FUNCTION("usersPrivate.isWhitelistedUser",
+                             USERSPRIVATE_ISWHITELISTEDUSER)
+
+ protected:
+  ~UsersPrivateIsWhitelistedUserFunction() override;
+
+  // ExtensionFunction overrides.
+  ResponseAction Run() override;
+
+ private:
+  ChromeExtensionFunctionDetails chrome_details_;
+
+  DISALLOW_COPY_AND_ASSIGN(UsersPrivateIsWhitelistedUserFunction);
+};
+
 // Implements the chrome.usersPrivate.addWhitelistedUser method.
 class UsersPrivateAddWhitelistedUserFunction : public ExtensionFunction {
  public:

chrome/browser/chromeos/extensions/users_private/users_private_apitest.cc

@@ -221,6 +221,10 @@ IN_PROC_BROWSER_TEST_F(UsersPrivateApiTest, AddAndRemoveUsers) {
   EXPECT_TRUE(RunSubtest("addAndRemoveUsers")) << message_;
 }
 
+IN_PROC_BROWSER_TEST_F(UsersPrivateApiTest, IsWhitelistedUser) {
+  EXPECT_TRUE(RunSubtest("isWhitelistedUser")) << message_;
+}
+
 IN_PROC_BROWSER_TEST_F(UsersPrivateApiTest, IsOwner) {
   EXPECT_TRUE(RunSubtest("isOwner")) << message_;
 }

chrome/common/extensions/api/users_private.idl

@@ -41,11 +41,16 @@ namespace usersPrivate {
   callback ManagedCallback = void (boolean managed);
   callback UserCallback = void (User user);
   callback LoginStatusCallback = void (LoginStatusDict status);
+  callback IsWhiteListedUserCallback = void (boolean found);
 
   interface Functions {
     // Gets a list of the currently whitelisted users.
     static void getWhitelistedUsers(UsersCallback callback);
 
+    // Checks to see if the user is already present as a whitelisted user.
+    static void isWhitelistedUser(DOMString email,
+                                  IsWhiteListedUserCallback callback);
+
     // Adds a new user with the given email to the whitelist.
     // The callback is called with true if the user was added succesfully, or
     // with false if not (e.g. because the user was already present, or the

chrome/test/data/extensions/api_test/users_private/test.js

@@ -67,6 +67,36 @@ var availableTests = [
 
   },
 
+  function isWhitelistedUser() {
+    chrome.usersPrivate.isWhitelistedUser(
+      kEmail1,
+      function(result) {
+        chrome.test.assertFalse(result);
+
+        chrome.usersPrivate.addWhitelistedUser(
+          kEmail2,
+          function(result) {
+            callbackResult(result);
+
+            // We never added kEmail1 so this should return false.
+            chrome.usersPrivate.isWhitelistedUser(
+              kEmail1,
+              function(result) {
+                chrome.test.assertFalse(result);
+
+                chrome.usersPrivate.isWhitelistedUser(
+                  kEmail2,
+                  function(user) {
+                    chrome.test.assertTrue(user);
+                    chrome.test.succeed();
+                  });
+              });
+          });
+      });
+
+
+  },
+
   function isOwner() {
     chrome.usersPrivate.getCurrentUser(function(user) {
       // Since we are testing with --stub-cros-settings this should be true.

extensions/browser/extension_function_histogram_value.h

@@ -1445,6 +1445,7 @@ enum HistogramValue {
   AUTOTESTPRIVATE_SETOVERVIEWMODESTATE = 1382,
   AUTOTESTPRIVATE_TAKESCREENSHOTFORDISPLAY = 1383,
   AUTOFILLPRIVATE_SETCREDITCARDFIDOAUTHENABLEDSTATE = 1384,
+  USERSPRIVATE_ISWHITELISTEDUSER = 1385,
   // Last entry: Add new entries above, then run:
   // python tools/metrics/histograms/update_extension_histograms.py
   ENUM_BOUNDARY

third_party/closure_compiler/externs/users_private.js

@@ -42,6 +42,13 @@ chrome.usersPrivate.LoginStatusDict;
  */
 chrome.usersPrivate.getWhitelistedUsers = function(callback) {};
 
+/**
+ * Checks to see if the user is already present as a whitelisted user.
+ * @param {string} email
+ * @param {function(boolean):void} callback
+ */
+chrome.usersPrivate.isWhitelistedUser = function(email, callback) {};
+
 /**
  * Adds a new user with the given email to the whitelist. The callback is called
  * with true if the user was added succesfully, or with false if not (e.g.

tools/metrics/histograms/enums.xml

@@ -20671,6 +20671,7 @@ Called by update_net_error_codes.py.-->
   <int value="1382" label="AUTOTESTPRIVATE_SETOVERVIEWMODESTATE"/>
   <int value="1383" label="AUTOTESTPRIVATE_TAKESCREENSHOTFORDISPLAY"/>
   <int value="1384" label="AUTOFILLPRIVATE_SETCREDITCARDFIDOAUTHENABLEDSTATE"/>
+  <int value="1385" label="USERSPRIVATE_ISWHITELISTEDUSER"/>
 </enum>
 
 <enum name="ExtensionIconState">