[Nearby] Use fake cert manager in service tests

Bug: 1113850
Change-Id: I9d82f7a8074b0a0d2fe918bff8125f046e120ad8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353876
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Himanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798066}

chrome/browser/nearby_sharing/certificates/fake_nearby_share_certificate_manager.h

@@ -21,7 +21,8 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
   // Factory that creates FakeNearbyShareCertificateManager instances. Use in
   // NearbyShareCertificateManagerImpl::Factor::SetFactoryForTesting() in unit
   // tests.
-  class Factory : NearbyShareCertificateManagerImpl::Factory {
+  class Factory : public NearbyShareCertificateManagerImpl::Factory {
+   public:
     Factory();
     ~Factory() override;
 
@@ -75,6 +76,14 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
   using NearbyShareCertificateManager::NotifyPrivateCertificatesChanged;
   using NearbyShareCertificateManager::NotifyPublicCertificatesDownloaded;
 
+  size_t num_get_valid_private_certificate_calls() {
+    return num_get_valid_private_certificate_calls_;
+  }
+
+  size_t num_download_public_certificates_calls() {
+    return num_download_public_certificates_calls_;
+  }
+
   std::vector<GetDecryptedPublicCertificateCall>&
   get_decrypted_public_certificate_calls() {
     return get_decrypted_public_certificate_calls_;

chrome/browser/nearby_sharing/certificates/test_util.cc

@@ -278,3 +278,12 @@ GetNearbyShareTestPublicCertificate() {
       }());
   return *cert;
 }
+
+const NearbyShareDecryptedPublicCertificate&
+GetNearbyShareTestDecryptedPublicCertificate() {
+  static const base::NoDestructor<NearbyShareDecryptedPublicCertificate> cert(
+      *NearbyShareDecryptedPublicCertificate::DecryptPublicCertificate(
+          GetNearbyShareTestPublicCertificate(),
+          GetNearbyShareTestEncryptedMetadataKey()));
+  return *cert;
+}

chrome/browser/nearby_sharing/certificates/test_util.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/time/time.h"
+#include "chrome/browser/nearby_sharing/certificates/nearby_share_decrypted_public_certificate.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 #include "chrome/browser/nearby_sharing/proto/encrypted_metadata.pb.h"
@@ -46,4 +47,7 @@ NearbySharePrivateCertificate GetNearbyShareTestPrivateCertificate(
 const nearbyshare::proto::PublicCertificate&
 GetNearbyShareTestPublicCertificate();
 
+const NearbyShareDecryptedPublicCertificate&
+GetNearbyShareTestDecryptedPublicCertificate();
+
 #endif  // CHROME_BROWSER_NEARBY_SHARING_CERTIFICATES_TEST_UTIL_H_

chrome/browser/nearby_sharing/contacts/fake_nearby_share_contact_manager.h

@@ -24,7 +24,8 @@ class FakeNearbyShareContactManager : public NearbyShareContactManager {
   // Factory that creates FakeNearbyShareContactManager instances. Use in
   // NearbyShareContactManagerImpl::Factor::SetFactoryForTesting() in unit
   // tests.
-  class Factory : NearbyShareContactManagerImpl::Factory {
+  class Factory : public NearbyShareContactManagerImpl::Factory {
+   public:
     Factory();
     ~Factory() override;
 

chrome/browser/nearby_sharing/nearby_sharing_service_factory.cc

@@ -10,7 +10,6 @@
 #include "base/memory/singleton.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_features.h"
-#include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
 #include "chrome/browser/nearby_sharing/logging/logging.h"
 #include "chrome/browser/nearby_sharing/nearby_connections_manager.h"
@@ -74,8 +73,7 @@ KeyedService* NearbySharingServiceFactory::BuildServiceInstanceFor(
   NS_LOG(VERBOSE) << __func__ << ": creating NearbySharingService.";
   return new NearbySharingServiceImpl(
       pref_service, notification_display_service, profile,
-      std::move(nearby_connections_manager), &process_manager,
-      NearbyShareCertificateManagerImpl::Factory::Create());
+      std::move(nearby_connections_manager), &process_manager);
 }
 
 content::BrowserContext* NearbySharingServiceFactory::GetBrowserContextToUse(

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -161,8 +161,7 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
     NotificationDisplayService* notification_display_service,
     Profile* profile,
     std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager,
-    NearbyProcessManager* process_manager,
-    std::unique_ptr<NearbyShareCertificateManager> certificate_manager)
+    NearbyProcessManager* process_manager)
     : profile_(profile),
       settings_(prefs),
       nearby_connections_manager_(std::move(nearby_connections_manager)),
@@ -176,10 +175,10 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
               prefs,
               http_client_factory_.get())),
       contact_manager_(NearbyShareContactManagerImpl::Factory::Create()),
-      certificate_manager_(std::move(certificate_manager)) {
+      certificate_manager_(
+          NearbyShareCertificateManagerImpl::Factory::Create()) {
   DCHECK(profile_);
   DCHECK(nearby_connections_manager_);
-  DCHECK(certificate_manager_);
 
   nearby_process_observer_.Add(process_manager_);
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl.h

@@ -63,8 +63,7 @@ class NearbySharingServiceImpl
       NotificationDisplayService* notification_display_service,
       Profile* profile,
       std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager,
-      NearbyProcessManager* process_manager,
-      std::unique_ptr<NearbyShareCertificateManager> certificate_manager);
+      NearbyProcessManager* process_manager);
   ~NearbySharingServiceImpl() override;
 
   // NearbySharingService:
@@ -187,10 +186,9 @@ class NearbySharingServiceImpl
       base::Optional<NearbyShareDecryptedPublicCertificate> certificate);
   void ReceiveIntroduction(ShareTarget share_target,
                            base::Optional<std::string> token);
-  void OnReceivedIntroduction(
-      ShareTarget share_target,
-      base::Optional<std::string> token,
-      base::Optional<sharing::mojom::V1FramePtr> frame);
+  void OnReceivedIntroduction(ShareTarget share_target,
+                              base::Optional<std::string> token,
+                              base::Optional<sharing::mojom::V1FramePtr> frame);
   void OnFrameRead(ShareTarget share_target,
                    base::Optional<sharing::mojom::V1FramePtr> frame);
   void HandleCertificateInfoFrame(

chrome/browser/nearby_sharing/paired_key_verification_runner_unittest.cc

@@ -13,7 +13,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/time/time.h"
-#include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager.h"
+#include "chrome/browser/nearby_sharing/certificates/fake_nearby_share_certificate_manager.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_decrypted_public_certificate.h"
 #include "chrome/browser/nearby_sharing/certificates/test_util.h"
 #include "chrome/browser/nearby_sharing/fake_nearby_connection.h"
@@ -66,25 +66,6 @@ class MockIncomingFramesReader : public IncomingFramesReader {
       (override));
 };
 
-class MockNearbyShareCertificateManager : public NearbyShareCertificateManager {
- public:
-  MOCK_METHOD(NearbySharePrivateCertificate,
-              GetValidPrivateCertificate,
-              (NearbyShareVisibility visibility),
-              (override));
-  MOCK_METHOD(void,
-              GetDecryptedPublicCertificate,
-              (base::span<const uint8_t> encrypted_metadata_key,
-               base::span<const uint8_t> salt,
-               CertDecryptedCallback callback),
-              (override));
-  MOCK_METHOD(void, DownloadPublicCertificates, (), (override));
-
- protected:
-  MOCK_METHOD(void, OnStart, (), (override));
-  MOCK_METHOD(void, OnStop, (), (override));
-};
-
 PairedKeyVerificationRunner::PairedKeyVerificationResult Merge(
     PairedKeyVerificationRunner::PairedKeyVerificationResult local_result,
     sharing::mojom::PairedKeyResultFrame::Status remote_result) {
@@ -120,12 +101,36 @@ class PairedKeyVerificationRunnerTest : public testing::Test {
       : frames_reader_(&mock_nearby_process_manager_, &profile_, &connection_) {
   }
 
-  void SetUp() override {
-    share_target.is_incoming = true;
-
-    EXPECT_CALL(certificate_manager_, GetValidPrivateCertificate(testing::_))
-        .WillRepeatedly(testing::Return(GetNearbyShareTestPrivateCertificate(
-            NearbyShareVisibility::kAllContacts)));
+  void SetUp() override { share_target_.is_incoming = true; }
+
+  void RunVerification(bool use_valid_public_certificate,
+                       bool restricted_to_contacts,
+                       PairedKeyVerificationRunner::PairedKeyVerificationResult
+                           expected_result) {
+    base::Optional<NearbyShareDecryptedPublicCertificate> public_certificate =
+        use_valid_public_certificate
+            ? base::make_optional<NearbyShareDecryptedPublicCertificate>(
+                  GetNearbyShareTestDecryptedPublicCertificate())
+            : base::nullopt;
+
+    PairedKeyVerificationRunner runner(
+        share_target_, kEndpointId, kAuthToken, &connection_,
+        std::move(public_certificate), &certificate_manager_,
+        nearby_share::mojom::Visibility::kAllContacts, restricted_to_contacts,
+        &frames_reader_, kTimeout);
+
+    base::RunLoop run_loop;
+    runner.Run(base::BindLambdaForTesting(
+        [&](PairedKeyVerificationRunner::PairedKeyVerificationResult result) {
+          EXPECT_EQ(expected_result, result);
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+
+    // The private certificate is at least always immediately retrieved in order
+    // to create the signature for the sent PairedKeyEncryptionFrame.
+    EXPECT_GE(certificate_manager_.num_get_valid_private_certificate_calls(),
+              1u);
   }
 
   void SetUpPairedKeyEncryptionFrame(ReturnFrameType frame_type) {
@@ -135,11 +140,20 @@ class PairedKeyVerificationRunnerTest : public testing::Test {
             testing::Eq(sharing::mojom::V1Frame::Tag::PAIRED_KEY_ENCRYPTION),
             testing::_, testing::Eq(kTimeout)))
         .WillOnce(testing::WithArg<1>(testing::Invoke(
-            [frame_type](
-                base::OnceCallback<void(
-                    base::Optional<sharing::mojom::V1FramePtr>)> callback) {
+            [frame_type,
+             this](base::OnceCallback<void(
+                       base::Optional<sharing::mojom::V1FramePtr>)> callback) {
+              // A private certificate retrieval will only be necessary if we
+              // receive a frame that needs verification.
+              size_t initial_num_private_cert_gets =
+                  certificate_manager_
+                      .num_get_valid_private_certificate_calls();
+
               if (frame_type == ReturnFrameType::kNull) {
                 std::move(callback).Run(base::nullopt);
+                EXPECT_EQ(initial_num_private_cert_gets,
+                          certificate_manager_
+                              .num_get_valid_private_certificate_calls());
                 return;
               }
 
@@ -157,6 +171,9 @@ class PairedKeyVerificationRunnerTest : public testing::Test {
               }
 
               std::move(callback).Run(std::move(mojo_v1frame));
+              EXPECT_EQ(initial_num_private_cert_gets + 1,
+                        certificate_manager_
+                            .num_get_valid_private_certificate_calls());
             })));
   }
 
@@ -218,59 +235,37 @@ class PairedKeyVerificationRunnerTest : public testing::Test {
   testing::NiceMock<MockNearbyProcessManager> mock_nearby_process_manager_;
   TestingProfile profile_;
   FakeNearbyConnection connection_;
-  testing::NiceMock<MockNearbyShareCertificateManager> certificate_manager_;
+  FakeNearbyShareCertificateManager certificate_manager_;
   testing::NiceMock<MockIncomingFramesReader> frames_reader_;
-  ShareTarget share_target;
+  ShareTarget share_target_;
 };
 
 TEST_F(PairedKeyVerificationRunnerTest,
        NullCertificate_InvalidPairedKeyEncryptionFrame_RestrictToContacts) {
-  PairedKeyVerificationRunner runner(
-      share_target, kEndpointId, kAuthToken, &connection_,
-      /*certificate=*/base::nullopt, &certificate_manager_,
-      nearby_share::mojom::Visibility::kAllContacts,
-      /*restrict_to_contacts=*/true, &frames_reader_, kTimeout);
-
   // Empty key encryption frame fails the certificate verification.
   SetUpPairedKeyEncryptionFrame(ReturnFrameType::kEmpty);
 
-  base::RunLoop run_loop;
-  runner.Run(base::BindLambdaForTesting(
-      [&](PairedKeyVerificationRunner::PairedKeyVerificationResult result) {
-        EXPECT_EQ(
-            PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail,
-            result);
-        run_loop.Quit();
-      }));
-  run_loop.Run();
+  RunVerification(
+      /*use_valid_public_certificate=*/false,
+      /*restricted_to_contacts=*/true,
+      /*expected_result=*/
+      PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail);
 
   ExpectPairedKeyEncryptionFrameSent();
 }
 
 TEST_F(PairedKeyVerificationRunnerTest,
        ValidPairedKeyEncryptionFrame_ResultFrameTimedOut) {
-  PairedKeyVerificationRunner runner(
-      share_target, kEndpointId, kAuthToken, &connection_,
-      NearbyShareDecryptedPublicCertificate::DecryptPublicCertificate(
-          GetNearbyShareTestPublicCertificate(),
-          GetNearbyShareTestEncryptedMetadataKey()),
-      &certificate_manager_, nearby_share::mojom::Visibility::kAllContacts,
-      /*restrict_to_contacts=*/false, &frames_reader_, kTimeout);
-
   SetUpPairedKeyEncryptionFrame(ReturnFrameType::kValid);
 
   // Null result frame fails the certificate verification process.
   SetUpPairedKeyResultFrame(ReturnFrameType::kNull);
 
-  base::RunLoop run_loop;
-  runner.Run(base::BindLambdaForTesting(
-      [&](PairedKeyVerificationRunner::PairedKeyVerificationResult result) {
-        EXPECT_EQ(
-            PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail,
-            result);
-        run_loop.Quit();
-      }));
-  run_loop.Run();
+  RunVerification(
+      /*use_valid_public_certificate=*/true,
+      /*restricted_to_contacts=*/false,
+      /*expected_result=*/
+      PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail);
 
   ExpectPairedKeyEncryptionFrameSent();
   ExpectPairedKeyResultFrameSent(sharing::nearby::PairedKeyResultFrame::UNABLE);
@@ -308,32 +303,15 @@ TEST_P(ParameterisedPairedKeyVerificationRunnerTest,
   PairedKeyVerificationRunner::PairedKeyVerificationResult expected_result =
       Merge(params.result, status);
 
-  share_target.is_known = params.is_target_known;
-  base::Optional<NearbyShareDecryptedPublicCertificate> certificate;
-  if (params.is_valid_certificate) {
-    certificate =
-        NearbyShareDecryptedPublicCertificate::DecryptPublicCertificate(
-            GetNearbyShareTestPublicCertificate(),
-            GetNearbyShareTestEncryptedMetadataKey());
-  }
-
-  PairedKeyVerificationRunner runner(
-      share_target, kEndpointId, kAuthToken, &connection_,
-      std::move(certificate), &certificate_manager_,
-      nearby_share::mojom::Visibility::kAllContacts,
-      /*restricted_to_contacts=*/false, &frames_reader_, kTimeout);
+  share_target_.is_known = params.is_target_known;
 
   SetUpPairedKeyEncryptionFrame(params.encryption_frame_type);
   SetUpPairedKeyResultFrame(
       PairedKeyVerificationRunnerTest::ReturnFrameType::kValid, status);
 
-  base::RunLoop run_loop;
-  runner.Run(base::BindLambdaForTesting(
-      [&](PairedKeyVerificationRunner::PairedKeyVerificationResult result) {
-        EXPECT_EQ(expected_result, result);
-        run_loop.Quit();
-      }));
-  run_loop.Run();
+  RunVerification(
+      /*use_valid_public_certificate=*/params.is_valid_certificate,
+      /*restricted_to_contacts=*/false, expected_result);
 
   ExpectPairedKeyEncryptionFrameSent();
   if (params.encryption_frame_type ==

chrome/test/BUILD.gn

@@ -4423,8 +4423,11 @@ test("unit_tests") {
     }
 
     deps += [
+      "//chrome/browser/nearby_sharing/certificates:test_support",
       "//chrome/browser/nearby_sharing/certificates:unit_tests",
       "//chrome/browser/nearby_sharing/client:unit_tests",
+      "//chrome/browser/nearby_sharing/contacts:test_support",
+      "//chrome/browser/nearby_sharing/local_device_data:test_support",
       "//chrome/browser/nearby_sharing/local_device_data:unit_tests",
       "//chrome/browser/nearby_sharing/logging:unit_tests",
       "//chrome/browser/nearby_sharing/proto",