[Nearby] Recreate private certificates if local device metadata changes

Because certificates hold data such as the device name, full name, and
icon URL, recreate certificates whenever any of those values change for
the local device.

Fixed: b/168137953
Change-Id: Ic827476c4070b348ffe1f7c1248bc45adad08132
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2406602
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#806463}

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.cc

@@ -21,7 +21,6 @@
 #include "chrome/browser/nearby_sharing/client/nearby_share_client.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_http_result.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
-#include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager.h"
 #include "chrome/browser/nearby_sharing/logging/logging.h"
 #include "chrome/browser/nearby_sharing/proto/certificate_rpc.pb.h"
 #include "chrome/browser/nearby_sharing/proto/encrypted_metadata.pb.h"
@@ -234,10 +233,12 @@ NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl(
                                   /*page_number=*/1,
                                   /*certificate_count=*/0),
               clock_)) {
+  local_device_data_manager_->AddObserver(this);
   contact_manager_->AddObserver(this);
 }
 
 NearbyShareCertificateManagerImpl::~NearbyShareCertificateManagerImpl() {
+  local_device_data_manager_->RemoveObserver(this);
   contact_manager_->RemoveObserver(this);
 }
 
@@ -303,7 +304,7 @@ void NearbyShareCertificateManagerImpl::OnAllowlistChanged(
     return;
 
   certificate_storage_->ClearPrivateCertificates();
-  private_certificate_expiration_scheduler_->Reschedule();
+  private_certificate_expiration_scheduler_->MakeImmediateRequest();
 }
 
 void NearbyShareCertificateManagerImpl::OnContactsDownloaded(
@@ -316,7 +317,18 @@ void NearbyShareCertificateManagerImpl::OnContactsUploaded(
     return;
 
   certificate_storage_->ClearPrivateCertificates();
-  private_certificate_expiration_scheduler_->Reschedule();
+  private_certificate_expiration_scheduler_->MakeImmediateRequest();
+}
+
+void NearbyShareCertificateManagerImpl::OnLocalDeviceDataChanged(
+    bool did_device_name_change,
+    bool did_full_name_change,
+    bool did_icon_url_change) {
+  if (!did_device_name_change && !did_full_name_change && !did_icon_url_change)
+    return;
+
+  certificate_storage_->ClearPrivateCertificates();
+  private_certificate_expiration_scheduler_->MakeImmediateRequest();
 }
 
 base::Optional<base::Time>

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h

@@ -22,6 +22,7 @@
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_http_result.h"
 #include "chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager.h"
+#include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager.h"
 #include "chrome/browser/nearby_sharing/proto/rpc_resources.pb.h"
 #include "chrome/browser/ui/webui/nearby_share/public/mojom/nearby_share_settings.mojom.h"
 
@@ -64,7 +65,8 @@ class ListPublicCertificatesResponse;
 // interface for performing cryptographic operations related to certificates."
 class NearbyShareCertificateManagerImpl
     : public NearbyShareCertificateManager,
-      public NearbyShareContactManager::Observer {
+      public NearbyShareContactManager::Observer,
+      public NearbyShareLocalDeviceDataManager::Observer {
  public:
   class Factory {
    public:
@@ -126,6 +128,11 @@ class NearbyShareCertificateManagerImpl
       const std::vector<nearbyshare::proto::ContactRecord>& contacts) override;
   void OnContactsUploaded(bool did_contacts_change_since_last_upload) override;
 
+  // NearbyShareLocalDeviceDataManager::Observer:
+  void OnLocalDeviceDataChanged(bool did_device_name_change,
+                                bool did_full_name_change,
+                                bool did_icon_url_change) override;
+
   // Used by the private certificate expiration scheduler to determine the next
   // private certificate expiration time. Returns base::Time::Min() if
   // certificates are missing. This function never returns base::nullopt.

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl_unittest.cc

@@ -68,6 +68,15 @@ class NearbyShareCertificateManagerImplTest
     NearbyShareCertificateStorageImpl::Factory::SetFactoryForTesting(
         &cert_store_factory_);
 
+    // Set default device data.
+    local_device_data_manager_->SetDeviceName(
+        GetNearbyShareTestMetadata().device_name());
+    local_device_data_manager_->SetFullName(
+        GetNearbyShareTestMetadata().full_name());
+    local_device_data_manager_->SetIconUrl(
+        GetNearbyShareTestMetadata().icon_url());
+    SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
+
     cert_manager_ = NearbyShareCertificateManagerImpl::Factory::Create(
         local_device_data_manager_.get(), contact_manager_.get(),
         pref_service_.get(),
@@ -497,14 +506,6 @@ TEST_F(NearbyShareCertificateManagerImplTest,
        RefreshPrivateCertificates_ValidCertificates) {
   cert_store_->SetPrivateCertificates(private_certificates_);
 
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
-
   cert_manager_->Start();
   HandlePrivateCertificateRefresh(/*expect_private_cert_refresh=*/false,
                                   /*expected_success=*/true);
@@ -516,14 +517,6 @@ TEST_F(NearbyShareCertificateManagerImplTest,
   cert_store_->SetPrivateCertificates(
       std::vector<NearbySharePrivateCertificate>());
 
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
-
   cert_manager_->Start();
   HandlePrivateCertificateRefresh(/*expect_private_cert_refresh=*/true,
                                   /*expected_success=*/true);
@@ -536,14 +529,6 @@ TEST_F(NearbyShareCertificateManagerImplTest,
   cert_store_->SetPrivateCertificates(
       std::vector<NearbySharePrivateCertificate>());
 
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
-
   cert_manager_->Start();
   HandlePrivateCertificateRefresh(/*expect_private_cert_refresh=*/true,
                                   /*expected_success=*/true);
@@ -555,8 +540,8 @@ TEST_F(NearbyShareCertificateManagerImplTest,
        RevokePrivateCertificates_OnAllowlistChanged) {
   cert_manager_->Start();
 
-  // Destroy and recreate private certificates if and only if contacts were
-  // removed from the user's list of selected contacts.
+  // Destroy and recreate private certificates if contacts were removed from the
+  // user's list of selected contacts.
   size_t num_expected_calls = 0;
   for (bool were_contacts_added_to_allowlist : {true, false}) {
     for (bool were_contacts_removed_from_allowlist : {true, false}) {
@@ -569,7 +554,7 @@ TEST_F(NearbyShareCertificateManagerImplTest,
       EXPECT_EQ(num_expected_calls,
                 cert_store_->num_clear_private_certificates_calls());
       EXPECT_EQ(num_expected_calls,
-                private_cert_exp_scheduler_->num_reschedule_calls());
+                private_cert_exp_scheduler_->num_immediate_requests());
     }
   }
 }
@@ -578,8 +563,8 @@ TEST_F(NearbyShareCertificateManagerImplTest,
        RevokePrivateCertificates_OnContactsUploaded) {
   cert_manager_->Start();
 
-  // Destroy and recreate private certificates if and only if the user's contact
-  // list has changed since the last upload.
+  // Destroy and recreate private certificates if the user's contact list has
+  // changed since the last upload.
   size_t num_expected_calls = 0;
   for (bool did_contacts_change_since_last_upload : {true, false}) {
     contact_manager_->NotifyContactsUploaded(
@@ -590,7 +575,33 @@ TEST_F(NearbyShareCertificateManagerImplTest,
     EXPECT_EQ(num_expected_calls,
               cert_store_->num_clear_private_certificates_calls());
     EXPECT_EQ(num_expected_calls,
-              private_cert_exp_scheduler_->num_reschedule_calls());
+              private_cert_exp_scheduler_->num_immediate_requests());
+  }
+}
+
+TEST_F(NearbyShareCertificateManagerImplTest,
+       RefreshPrivateCertificates_OnLocalDeviceMetadataChanged) {
+  cert_manager_->Start();
+
+  // Destroy and recreate private certificates if any metadata fields change.
+  size_t num_expected_calls = 0;
+  for (bool did_device_name_change : {true, false}) {
+    for (bool did_full_name_change : {true, false}) {
+      for (bool did_icon_url_change : {true, false}) {
+        local_device_data_manager_->NotifyLocalDeviceDataChanged(
+            did_device_name_change, did_full_name_change, did_icon_url_change);
+
+        if (did_device_name_change || did_full_name_change ||
+            did_icon_url_change) {
+          ++num_expected_calls;
+        }
+
+        EXPECT_EQ(num_expected_calls,
+                  cert_store_->num_clear_private_certificates_calls());
+        EXPECT_EQ(num_expected_calls,
+                  private_cert_exp_scheduler_->num_immediate_requests());
+      }
+    }
   }
 }
 
@@ -600,14 +611,6 @@ TEST_F(NearbyShareCertificateManagerImplTest,
   FastForward(kNearbyShareCertificateValidityPeriod * 1.5);
   cert_store_->SetPrivateCertificates(private_certificates_);
 
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
-
   cert_manager_->Start();
   HandlePrivateCertificateRefresh(/*expect_private_cert_refresh=*/true,
                                   /*expected_success=*/true);
@@ -621,11 +624,7 @@ TEST_F(NearbyShareCertificateManagerImplTest,
       std::vector<NearbySharePrivateCertificate>());
 
   // Device name is missing in local device data manager.
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
+  local_device_data_manager_->SetDeviceName(std::string());
 
   cert_manager_->Start();
 
@@ -640,12 +639,6 @@ TEST_F(NearbyShareCertificateManagerImplTest,
       std::vector<NearbySharePrivateCertificate>());
 
   // The bluetooth adapter returns an invalid Bluetooth MAC address.
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  local_device_data_manager_->SetFullName(
-      GetNearbyShareTestMetadata().full_name());
-  local_device_data_manager_->SetIconUrl(
-      GetNearbyShareTestMetadata().icon_url());
   SetBluetoothMacAddress("invalid_mac_address");
 
   cert_manager_->Start();
@@ -666,9 +659,8 @@ TEST_F(NearbyShareCertificateManagerImplTest,
       std::vector<NearbySharePrivateCertificate>());
 
   // Full name and icon URL are missing in local device data manager.
-  local_device_data_manager_->SetDeviceName(
-      GetNearbyShareTestMetadata().device_name());
-  SetBluetoothMacAddress(kTestUnparsedBluetoothMacAddress);
+  local_device_data_manager_->SetFullName(base::nullopt);
+  local_device_data_manager_->SetIconUrl(base::nullopt);
 
   cert_manager_->Start();
   HandlePrivateCertificateRefresh(/*expect_private_cert_refresh=*/true,

chrome/browser/nearby_sharing/local_device_data/fake_nearby_share_local_device_data_manager.h

@@ -90,6 +90,10 @@ class FakeNearbyShareLocalDeviceDataManager
       std::vector<nearbyshare::proto::PublicCertificate> certificates,
       UploadCompleteCallback callback) override;
 
+  // Make protected observer-notification methods from base class public in this
+  // fake class.
+  using NearbyShareLocalDeviceDataManager::NotifyLocalDeviceDataChanged;
+
   void SetId(const std::string& id) { id_ = id; }
   void SetFullName(const base::Optional<std::string>& full_name);
   void SetIconUrl(const base::Optional<std::string>& icon_url);