Implementation for chrome.certificateProvider.requestPin/stopPinRequest

chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc

 // Copyright 2015 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 <openssl/evp.h>
 #include <openssl/rsa.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdlib.h>
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
+#include "chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
 #include "components/policy/policy_constants.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_openssl_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/result_catcher.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "ui/views/controls/label.h"
+#include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/widget/widget.h"
 
 using testing::Return;
 using testing::_;
 
 namespace {
 
 void IgnoreResult(const base::Closure& callback, const base::Value* value) {
   callback.Run();
 }
 
@@ skipping 64 matching lines @@
 std::string JsUint8Array(const std::vector<uint8_t>& bytes) {
   std::string res = "new Uint8Array([";
   for (const uint8_t byte : bytes) {
     res += base::UintToString(byte);
     res += ", ";
   }
   res += "])";
   return res;
 }
 
+// Enters the code in the ShowPinDialog window and pushes the OK event.
+void EnterCode(chromeos::CertificateProviderService* service,
+               base::string16 code) {

[emaxx, 2016/09/19 14:01:44]

nit: Pass the strings by const-reference (here and in other places in this
file).

[igorcov, 2016/09/19 15:42:37]

Done.

+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+  view->textfield_for_testing()->SetText(code);
+  view->Accept();
+  base::RunLoop().RunUntilIdle();
+}
+
+// Enters the valid code for extensions from local example folders, in the
+// ShowPinDialog window and waits for the window to close.
+void EnterCorrectPin(chromeos::CertificateProviderService* service) {
+  views::Widget* active_window =
+      service->pin_dialog_manager()->active_window_for_testing();
+  EnterCode(service, base::ASCIIToUTF16("1234"));
+
+  // Wait for extension to send request to close the window.
+  while (!active_window->IsClosed()) {
+    base::RunLoop().RunUntilIdle();
+  }
+}
+
+// Enters an invalid code for extensions from local example folders, in the
+// ShowPinDialog window and waits for the window to update with the error.
+void EnterWrongPin(chromeos::CertificateProviderService* service) {
+  EnterCode(service, base::ASCIIToUTF16("123"));
+
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+  // Waiting for extension to open the dialog again with an error in red
+  // displayed.
+  while (view->error_label_for_testing()->enabled_color() != SK_ColorRED) {
+    base::RunLoop().RunUntilIdle();
+  }
+}
+
 class CertificateProviderApiTest : public ExtensionApiTest {
  public:
   CertificateProviderApiTest() {}
 
   void SetUpInProcessBrowserTestFixture() override {
     EXPECT_CALL(provider_, IsInitializationComplete(_))
         .WillRepeatedly(Return(true));
     policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
 
     ExtensionApiTest::SetUpInProcessBrowserTestFixture();
@@ skipping 11 matching lines @@
     policy::PolicyMap policy;
     policy.Set(policy::key::kAutoSelectCertificateForUrls,
                policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
                policy::POLICY_SOURCE_CLOUD, std::move(autoselect_policy),
                nullptr);
     provider_.UpdateChromePolicy(policy);
 
     content::RunAllPendingInMessageLoop();
   }
 
+  // Loads certificate_provider extension from folder (the page basic.html).
+  // Returns the CertificateProviderService object from browser context.
+  chromeos::CertificateProviderService* LoadExtensionFromFolder(

[emaxx, 2016/09/19 14:01:44]

Please change the existing tests in this file to use this helper function too.
Currently, it seems that there's some duplication of logic between the old code
and the new code.

+      std::string folder,
+      std::string file_name) {
+    content::WebContents* extension_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    chromeos::CertificateProviderService* service =
+        chromeos::CertificateProviderServiceFactory::GetForBrowserContext(
+            extension_contents->GetBrowserContext());
+    const base::FilePath extension_path =
+        test_data_dir_.AppendASCII("certificate_provider/" + folder);
+    const extensions::Extension* const extension =
+        LoadExtension(extension_path);
+    service->pin_dialog_manager()->AddSignRequestId(extension->id(), 123);

[emaxx, 2016/09/19 14:01:44]

nit: The test uses the same constant "123" for different purposes: first, as the
sign request id, and, second, as the entered PIN.
Please change this to make these two cases using different constants.

[igorcov, 2016/09/19 15:42:36]

Done.

+    ui_test_utils::NavigateToURL(browser(),
+                                 extension->GetResourceURL(file_name));
+    base::RunLoop().RunUntilIdle();
+    return service;
+  }
+
  protected:
   policy::MockConfigurationPolicyProvider provider_;
 };
 
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, Basic) {
   // Start an HTTPS test server that requests a client certificate.
   net::SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.request_client_certificate = true;
@@ skipping 95 matching lines @@
     base::RunLoop run_loop;
     const std::string code = "replyWithSignatureSecondTime();";
     bool result = false;
     extension_contents->GetMainFrame()->ExecuteJavaScriptForTests(
         base::ASCIIToUTF16(code),
         base::Bind(&StoreBool, &result, run_loop.QuitClosure()));
     run_loop.Run();
     EXPECT_TRUE(result);
   }
 }
+
+// User closes the dialog three times, and the extension should be blocked from

[emaxx, 2016/09/19 14:01:44]

nit: Replace the "three times" text with a more generic text, because the exact
number is actually determined by the externally defined
kMaxClosedDialogsPer10Mins constant.

+// showing it again.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogClose) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  views::Widget* window =
+      service->pin_dialog_manager()->active_window_for_testing();
+  for (int i = 0;
+       i < extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins;
+       i++) {
+    window->Close();
+    // Waiting for the new window to pop up.
+    while (window == service->pin_dialog_manager()->active_window_for_testing())

[emaxx, 2016/09/19 14:01:44]

Not sure whether I understand the code correctly. Are you assuming that after
destroying the old Widget* window, the newly constructed window will have a
different address in the memory? This may be true only by occasion, but you
shouldn't rely on this in the test code.

[igorcov, 2016/09/19 15:42:36]

You are right. I've changed the code to wait for the view to update.

+      base::RunLoop().RunUntilIdle();
+
+    window = service->pin_dialog_manager()->active_window_for_testing();
+  }
+
+  // This time when closed, the window should not be able to pop up again.
+  window->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}
+
+// User enters the correct PIN.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogAccept) {

[emaxx, 2016/09/19 14:01:44]

nit: Please move this test to be the first one among the PIN dialog tests -
because this is actually the most basic scenario.

[igorcov, 2016/09/19 15:42:37]

Done.

+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  EnterCorrectPin(service);
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(view, nullptr);
+}
+
+// User enters a wrong PIN first and a correct PIN on the second try.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogWrongPin) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  EnterWrongPin(service);
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The window should be active.
+  EXPECT_EQ(
+      service->pin_dialog_manager()->active_window_for_testing()->IsVisible(),
+      true);
+  view = service->pin_dialog_manager()->active_view_for_testing();
+  EXPECT_NE(view, nullptr);
+
+  // Enter the valid PIN.
+  EnterCorrectPin(service);
+  view = service->pin_dialog_manager()->active_view_for_testing();
+
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(view, nullptr);
+}
+
+// User enters wrong PIN three times.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest,
+                       ShowPinDialogWrongPinThreeTimes) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+  for (int i = 0; i < 3; i++) {
+    EnterWrongPin(service);
+  }
+
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The textfield has to be disabled, as extension does not allow input now.
+  EXPECT_EQ(view->textfield_for_testing()->enabled(), false);
+
+  // Close the dialog.
+  service->pin_dialog_manager()->active_window_for_testing()->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}
+
+// User closes the dialog while the extension is processing the request.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest,
+                       ShowPinDialogCloseWhileProcessing) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic_lock.html");
+
+  EnterCode(service, base::ASCIIToUTF16("123"));
+  service->pin_dialog_manager()->active_window_for_testing()->Close();
+  base::RunLoop().RunUntilIdle();
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}