Cleanup usage of optional in Certificate management.

PiperOrigin-RevId: 794755803

sharing/certificates/BUILD

@@ -120,7 +120,6 @@ cc_test(
     deps = [
         ":certificates",
         ":test_support",
-        "//internal/flags:nearby_flags",
         "//internal/platform/implementation:account_manager",
         "//internal/platform/implementation/g3",  # fixdeps: keep
         "//internal/test",
@@ -129,8 +128,8 @@ cc_test(
         "//sharing/common",
         "//sharing/common:enum",
         "//sharing/contacts:test_support",
-        "//sharing/flags/generated:generated_flags",
         "//sharing/internal/api:mock_sharing_platform",
+        "//sharing/internal/api:platform",
         "//sharing/internal/public:logging",
         "//sharing/internal/test:nearby_test",
         "//sharing/local_device_data:test_support",

sharing/certificates/fake_nearby_share_certificate_manager.cc

@@ -82,13 +82,6 @@ FakeNearbyShareCertificateManager::FakeNearbyShareCertificateManager()
 FakeNearbyShareCertificateManager::~FakeNearbyShareCertificateManager() =
     default;
 
-std::vector<nearby::sharing::proto::PublicCertificate>
-FakeNearbyShareCertificateManager::GetPrivateCertificatesAsPublicCertificates(
-    DeviceVisibility visibility) {
-  ++num_get_private_certificates_as_public_certificates_calls_;
-  return GetNearbyShareTestPublicCertificateList(visibility);
-}
-
 void FakeNearbyShareCertificateManager::GetDecryptedPublicCertificate(
     NearbyShareEncryptedMetadataKey encrypted_metadata_key,
     CertDecryptedCallback callback) {

sharing/certificates/fake_nearby_share_certificate_manager.h

@@ -91,9 +91,6 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
   ~FakeNearbyShareCertificateManager() override;
 
   // NearbyShareCertificateManager:
-  std::vector<nearby::sharing::proto::PublicCertificate>
-  GetPrivateCertificatesAsPublicCertificates(
-      proto::DeviceVisibility visibility) override;
   void GetDecryptedPublicCertificate(
       NearbyShareEncryptedMetadataKey encrypted_metadata_key,
       CertDecryptedCallback callback) override;

sharing/certificates/fake_nearby_share_certificate_storage.cc

@@ -109,8 +109,8 @@ void FakeNearbyShareCertificateStorage::GetPublicCertificate(
   get_public_certificate_callback_ = std::move(callback);
 }
 
-std::optional<std::vector<NearbySharePrivateCertificate>>
-FakeNearbyShareCertificateStorage::GetPrivateCertificates() const {
+std::vector<NearbySharePrivateCertificate>
+FakeNearbyShareCertificateStorage::GetPrivateCertificates() {
   return private_certificates_;
 }
 

sharing/certificates/fake_nearby_share_certificate_storage.h

@@ -108,8 +108,7 @@ class FakeNearbyShareCertificateStorage : public NearbyShareCertificateStorage {
       std::function<void(
           bool, std::unique_ptr<nearby::sharing::proto::PublicCertificate>)>
           callback) override;
-  std::optional<std::vector<NearbySharePrivateCertificate>>
-  GetPrivateCertificates() const override;
+  std::vector<NearbySharePrivateCertificate> GetPrivateCertificates() override;
   std::optional<absl::Time> NextPublicCertificateExpirationTime()
       const override;
   void ReplacePrivateCertificates(
@@ -156,8 +155,7 @@ class FakeNearbyShareCertificateStorage : public NearbyShareCertificateStorage {
  private:
   std::optional<absl::Time> next_public_certificate_expiration_time_;
   std::vector<std::string> public_certificate_ids_;
-  std::optional<std::vector<NearbySharePrivateCertificate>>
-      private_certificates_;
+  std::vector<NearbySharePrivateCertificate> private_certificates_;
   std::vector<PublicCertificateCallback> get_public_certificates_callbacks_;
   std::function<void(
       bool, std::unique_ptr<nearby::sharing::proto::PublicCertificate>)>

sharing/certificates/nearby_share_certificate_manager.h

@@ -95,16 +95,6 @@ class NearbyShareCertificateManager {
       proto::DeviceVisibility visibility,
       absl::Span<const uint8_t> authentication_token) const;
 
-  // Returns all local device private certificates of |visibility| converted to
-  // public certificates. The public certificates' for_selected_contacts fields
-  // will be set to reflect the |visibility|. NOTE: Only certificates with the
-  // requested visibility will be returned; if selected-contacts visibility is
-  // passed in, the all-contacts visibility certificates will *not* be returned
-  // as well.
-  virtual std::vector<nearby::sharing::proto::PublicCertificate>
-  GetPrivateCertificatesAsPublicCertificates(
-      proto::DeviceVisibility visibility) = 0;
-
   // Returns in |callback| the public certificate that is able to be decrypted
   // using |encrypted_metadata_key|, and returns absl::nullopt if no such public
   // certificate exists.

sharing/certificates/nearby_share_certificate_manager_impl.cc

@@ -270,6 +270,7 @@ NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl(
                 executor_->PostTask([this]() {
                   force_contacts_update_scheduler_->HandleResult(
                       UploadDeviceCertificatesInExecutor(
+                          certificate_storage_->GetPrivateCertificates(),
                           /*force_update_contacts=*/true));
                 });
               })),
@@ -427,6 +428,7 @@ void NearbyShareCertificateManagerImpl::RegeneratePrivateCertificates() {
 }
 
 bool NearbyShareCertificateManagerImpl::UploadDeviceCertificatesInExecutor(
+    const std::vector<NearbySharePrivateCertificate>& private_certs,
     bool force_update_contacts) {
   LOG(INFO) << "Start to upload local device certificates in executor.";
 
@@ -442,16 +444,14 @@ bool NearbyShareCertificateManagerImpl::UploadDeviceCertificatesInExecutor(
     return true;
   }
 
-  std::optional<std::vector<NearbySharePrivateCertificate>> private_certs =
-      certificate_storage_->GetPrivateCertificates();
-  if (!private_certs.has_value() || private_certs->empty()) {
+  if (private_certs.empty()) {
     LOG(WARNING) << "Ignore local device certificates upload, no private "
                     "certificates found.";
     return false;
   }
   std::vector<PublicCertificate> public_certs;
-  public_certs.reserve(private_certs->size());
-  for (const NearbySharePrivateCertificate& private_cert : *private_certs) {
+  public_certs.reserve(private_certs.size());
+  for (const NearbySharePrivateCertificate& private_cert : private_certs) {
     std::optional<PublicCertificate> public_cert =
         private_cert.ToPublicCertificate();
     if (!public_cert.has_value()) {
@@ -497,12 +497,6 @@ bool NearbyShareCertificateManagerImpl::UploadDeviceCertificatesInExecutor(
   return true;
 }
 
-std::vector<PublicCertificate>
-NearbyShareCertificateManagerImpl::GetPrivateCertificatesAsPublicCertificates(
-    DeviceVisibility visibility) {
-  return std::vector<PublicCertificate>();
-}
-
 void NearbyShareCertificateManagerImpl::GetDecryptedPublicCertificate(
     NearbyShareEncryptedMetadataKey encrypted_metadata_key,
     CertDecryptedCallback callback) {
@@ -550,9 +544,9 @@ NearbyShareCertificateManagerImpl::GetValidPrivateCertificate(
     visibility = DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS;
   }
 
-  std::optional<std::vector<NearbySharePrivateCertificate>> certs =
-      *certificate_storage_->GetPrivateCertificates();
-  for (auto& cert : *certs) {
+  std::vector<NearbySharePrivateCertificate> certs =
+      certificate_storage_->GetPrivateCertificates();
+  for (auto& cert : certs) {
     if (IsNearbyShareCertificateWithinValidityPeriod(
             context_->GetClock()->Now(), cert.not_before(), cert.not_after(),
             /*use_public_certificate_tolerance=*/false) &&
@@ -613,16 +607,16 @@ std::string NearbyShareCertificateManagerImpl::Dump() const {
   sstream << std::endl;
 
   sstream << "Private Certificates" << std::endl;
-  std::optional<std::vector<NearbySharePrivateCertificate>> private_certs =
+  std::vector<NearbySharePrivateCertificate> private_certs =
       certificate_storage_->GetPrivateCertificates();
-  if (private_certs.has_value()) {
-    sstream << "  Total count:" << private_certs->size() << std::endl;
-    for (const auto& cert : *private_certs) {
+  if (private_certs.empty()) {
+    sstream << "  Total count: 0" << std::endl;
+  } else {
+    sstream << "  Total count:" << private_certs.size() << std::endl;
+    for (const auto& cert : private_certs) {
       std::string id(cert.id().begin(), cert.id().end());
       DumpCertificateId(sstream, id, false);
     }
-  } else {
-    sstream << "  Total count: 0" << std::endl;
   }
 
   return sstream.str();
@@ -637,22 +631,8 @@ NearbyShareCertificateManagerImpl::NextPrivateCertificateExpirationTime() {
   if (!account.has_value()) {
     return std::nullopt;
   }
-  // We enforce that a fixed number--kNearbyShareNumPrivateCertificates for each
-  // visibility--of private certificates be present when user is logged in.
-  // This might not be true the first time the user enables Nearby Share or
-  // after certificates are revoked. For simplicity, consider the case of
-  // missing certificates an "expired" state. Return the minimum time to
-  // immediately trigger the private certificate creation flow.
-  if (certificate_storage_->GetPrivateCertificates()->size() <
-      NumExpectedPrivateCertificates()) {
-    return absl::InfinitePast();
-  }
-
-  std::optional<absl::Time> expiration_time =
-      certificate_storage_->NextPrivateCertificateExpirationTime();
-  DCHECK(expiration_time);
-
-  return *expiration_time;
+  return certificate_storage_->NextPrivateCertificateExpirationTime(
+      NumExpectedPrivateCertificates());
 }
 
 bool NearbyShareCertificateManagerImpl::RefreshPrivateCertificatesInExecutor(
@@ -670,7 +650,7 @@ bool NearbyShareCertificateManagerImpl::RefreshPrivateCertificatesInExecutor(
   }
 
   std::vector<NearbySharePrivateCertificate> certs =
-      *certificate_storage_->GetPrivateCertificates();
+      certificate_storage_->GetPrivateCertificates();
   if (certs.size() == NumExpectedPrivateCertificates()) {
     LOG(INFO) << "All private certificates are still valid. ";
     if (force_upload) {
@@ -753,7 +733,7 @@ bool NearbyShareCertificateManagerImpl::RefreshPrivateCertificatesInExecutor(
   if (force_upload) {
     force_contacts_update_scheduler_->MakeImmediateRequest();
   } else {
-    UploadDeviceCertificatesInExecutor(/*force_update_contacts=*/false);
+    UploadDeviceCertificatesInExecutor(certs, /*force_update_contacts=*/false);
   }
   return true;
 }

sharing/certificates/nearby_share_certificate_manager_impl.h

@@ -86,7 +86,14 @@ class NearbyShareCertificateManagerImpl
 
   ~NearbyShareCertificateManagerImpl() override;
 
+  void GetDecryptedPublicCertificate(
+      NearbyShareEncryptedMetadataKey encrypted_metadata_key,
+      CertDecryptedCallback callback) override;
+  void DownloadPublicCertificates() override;
+  void ForceUploadPrivateCertificates() override;
+  void ClearPublicCertificates(std::function<void(bool)> callback) override;
   void SetVendorId(int32_t vendor_id) override;
+  std::string Dump() const override;
 
  private:
   // Class for maintaining a single instance of public certificate download
@@ -138,15 +145,6 @@ class NearbyShareCertificateManagerImpl
        nearby::sharing::api::SharingRpcClientFactory* client_factory);
 
   // NearbyShareCertificateManager:
-  std::vector<nearby::sharing::proto::PublicCertificate>
-  GetPrivateCertificatesAsPublicCertificates(
-      proto::DeviceVisibility visibility) override;
-  void GetDecryptedPublicCertificate(
-      NearbyShareEncryptedMetadataKey encrypted_metadata_key,
-      CertDecryptedCallback callback) override;
-  void DownloadPublicCertificates() override;
-  void ForceUploadPrivateCertificates() override;
-  void ClearPublicCertificates(std::function<void(bool)> callback) override;
   void OnStart() override;
   void OnStop() override;
   std::optional<NearbySharePrivateCertificate> GetValidPrivateCertificate(
@@ -159,9 +157,6 @@ class NearbyShareCertificateManagerImpl
                                 bool did_full_name_change,
                                 bool did_icon_change) override;
 
-  // Dump certs information.
-  std::string Dump() const 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 absl::nullopt.
@@ -179,7 +174,9 @@ class NearbyShareCertificateManagerImpl
   // Certificate operations that run on the executor.
   // Returns true if the operation was successful.
   bool RefreshPrivateCertificatesInExecutor(bool force_upload);
-  bool UploadDeviceCertificatesInExecutor(bool force_update_contacts);
+  bool UploadDeviceCertificatesInExecutor(
+      const std::vector<NearbySharePrivateCertificate>& private_certs,
+      bool force_update_contacts);
   bool DownloadPublicCertificatesInExecutor();
   bool RemoveExpiredPublicCertificatesInExecutor();
 

sharing/certificates/nearby_share_certificate_manager_impl_test.cc

@@ -262,7 +262,7 @@ class NearbyShareCertificateManagerImplTest
       const nearby::sharing::proto::EncryptedMetadata& expected_metadata) {
     // Expect a full set of certificates for all-contacts and self-share
     std::vector<NearbySharePrivateCertificate> certs =
-        *cert_store_->GetPrivateCertificates();
+        cert_store_->GetPrivateCertificates();
     EXPECT_EQ(2 * kNearbyShareNumPrivateCertificates, certs.size());
 
     absl::Time min_not_before_all_contacts = absl::InfiniteFuture();
@@ -495,9 +495,9 @@ TEST_F(NearbyShareCertificateManagerImplTest,
               kNearbyShareCertificateValidityPeriod * 0.5 - Now());
 
   // Sanity check that the cert storage is as expected.
-  std::optional<std::vector<NearbySharePrivateCertificate>> stored_certs =
+  std::vector<NearbySharePrivateCertificate> stored_certs =
       cert_store_->GetPrivateCertificates();
-  EXPECT_EQ(stored_certs->at(0).ToCertificateData(),
+  EXPECT_EQ(stored_certs.at(0).ToCertificateData(),
             private_certificate.ToCertificateData());
 
   std::optional<NearbyShareEncryptedMetadataKey> encrypted_metadata_key =
@@ -509,7 +509,7 @@ TEST_F(NearbyShareCertificateManagerImplTest,
             encrypted_metadata_key->salt());
 
   // Verify that storage is updated when salts are consumed during encryption.
-  EXPECT_NE(cert_store_->GetPrivateCertificates()->at(0).ToCertificateData(),
+  EXPECT_NE(cert_store_->GetPrivateCertificates().at(0).ToCertificateData(),
             private_certificate.ToCertificateData());
 
   // Set up valid all-contacts visibility certificate. Then test with everyone
@@ -733,8 +733,8 @@ TEST_F(NearbyShareCertificateManagerImplTest,
   Sync();
 
   EXPECT_EQ(0, upload_scheduler_->num_immediate_requests());
-  EXPECT_EQ(cert_store_->GetPrivateCertificates()->size(), 0);
-  EXPECT_EQ(local_device_data_manager_->publish_device_calls().size(), 0);
+  EXPECT_TRUE(cert_store_->GetPrivateCertificates().empty());
+  EXPECT_TRUE(local_device_data_manager_->publish_device_calls().empty());
 }
 
 TEST_F(NearbyShareCertificateManagerImplTest,
@@ -752,7 +752,7 @@ TEST_F(NearbyShareCertificateManagerImplTest,
         Sync();
 
         std::vector<NearbySharePrivateCertificate> certs =
-            *cert_store_->GetPrivateCertificates();
+            cert_store_->GetPrivateCertificates();
         std::vector<std::string> cert_ids;
         for (const auto& cert : certs) {
           cert_ids.push_back(std::string(cert.id().begin(), cert.id().end()));
@@ -784,7 +784,33 @@ TEST_F(NearbyShareCertificateManagerImplTest,
 
   Sync();
   std::vector<NearbySharePrivateCertificate> certs =
-      *cert_store_->GetPrivateCertificates();
+      cert_store_->GetPrivateCertificates();
+  std::vector<std::string> cert_ids;
+  for (const auto& cert : certs) {
+    cert_ids.push_back(std::string(cert.id().begin(), cert.id().end()));
+  }
+  // New certificates should be generated.
+  EXPECT_EQ(private_certificate_ids_.size(), cert_ids.size());
+  EXPECT_THAT(cert_ids,
+              Not(UnorderedElementsAreArray(private_certificate_ids_)));
+
+  auto metadata = GetNearbyShareTestMetadata();
+  metadata.set_vendor_id(12345);
+  VerifyPrivateCertificates(/*expected_metadata=*/metadata);
+}
+
+TEST_F(NearbyShareCertificateManagerImplTest,
+       SetVendorId_WhenNoPrivateCertificates) {
+  Initialize();
+  cert_store_->ReplacePrivateCertificates({});
+  cert_manager_->Start();
+
+  cert_manager_->SetVendorId(12345);
+
+
+  Sync();
+  std::vector<NearbySharePrivateCertificate> certs =
+      cert_store_->GetPrivateCertificates();
   std::vector<std::string> cert_ids;
   for (const auto& cert : certs) {
     cert_ids.push_back(std::string(cert.id().begin(), cert.id().end()));

sharing/certificates/nearby_share_certificate_storage.cc

@@ -18,14 +18,11 @@
 #include <stdint.h>
 
 #include <algorithm>
-#include <optional>
-#include <ostream>
 #include <vector>
 
 #include "absl/time/time.h"
 #include "sharing/certificates/common.h"
 #include "sharing/certificates/nearby_share_private_certificate.h"
-#include "sharing/common/nearby_share_enums.h"
 #include "sharing/internal/base/encode.h"
 #include "sharing/internal/public/logging.h"
 #include "sharing/proto/enums.pb.h"
@@ -35,34 +32,30 @@ namespace sharing {
 
 using ::nearby::sharing::proto::DeviceVisibility;
 
-std::optional<absl::Time>
-NearbyShareCertificateStorage::NextPrivateCertificateExpirationTime() {
-  std::optional<std::vector<NearbySharePrivateCertificate>> certs =
-      GetPrivateCertificates();
-  if (!certs || certs->empty()) return std::nullopt;
+absl::Time NearbyShareCertificateStorage::NextPrivateCertificateExpirationTime(
+    int min_certs) {
+  std::vector<NearbySharePrivateCertificate> certs = GetPrivateCertificates();
+  if (certs.empty() || certs.size() < min_certs) {
+    return absl::InfinitePast();
+  }
 
   absl::Time min_time = absl::InfiniteFuture();
-  for (const NearbySharePrivateCertificate& cert : *certs)
+  for (const NearbySharePrivateCertificate& cert : certs) {
     min_time = std::min(min_time, cert.not_after());
-
+  }
   return min_time;
 }
 
 void NearbyShareCertificateStorage::UpdatePrivateCertificate(
     const NearbySharePrivateCertificate& private_certificate) {
-  std::optional<std::vector<NearbySharePrivateCertificate>> certs =
-      GetPrivateCertificates();
-  if (!certs) {
-    LOG(WARNING) << __func__ << ": No private certificates to update.";
-    return;
-  }
+  std::vector<NearbySharePrivateCertificate> certs = GetPrivateCertificates();
 
   auto it = std::find_if(
-      certs->begin(), certs->end(),
+      certs.begin(), certs.end(),
       [&private_certificate](const NearbySharePrivateCertificate& cert) {
         return cert.id() == private_certificate.id();
       });
-  if (it == certs->end()) {
+  if (it == certs.end()) {
     VLOG(1) << __func__ << ": No private certificate with id="
                << nearby::utils::HexEncode(private_certificate.id());
     return;
@@ -71,17 +64,16 @@ void NearbyShareCertificateStorage::UpdatePrivateCertificate(
   VLOG(1) << __func__ << ": Updating private certificate id="
              << nearby::utils::HexEncode(private_certificate.id());
   *it = private_certificate;
-  ReplacePrivateCertificates(*certs);
+  ReplacePrivateCertificates(certs);
 }
 
 void NearbyShareCertificateStorage::RemoveExpiredPrivateCertificates(
     absl::Time now) {
-  std::optional<std::vector<NearbySharePrivateCertificate>> certs =
-      GetPrivateCertificates();
-  if (!certs) return;
+  std::vector<NearbySharePrivateCertificate> certs = GetPrivateCertificates();
+  if (certs.empty()) return;
 
   std::vector<NearbySharePrivateCertificate> unexpired_certs;
-  for (const NearbySharePrivateCertificate& cert : *certs) {
+  for (const NearbySharePrivateCertificate& cert : certs) {
     if (!IsNearbyShareCertificateExpired(
             now, cert.not_after(),
             /*use_public_certificate_tolerance=*/false)) {
@@ -89,7 +81,7 @@ void NearbyShareCertificateStorage::RemoveExpiredPrivateCertificates(
     }
   }
 
-  size_t num_removed = certs->size() - unexpired_certs.size();
+  size_t num_removed = certs.size() - unexpired_certs.size();
   if (num_removed == 0) return;
 
   VLOG(1) << __func__ << ": Removing " << num_removed
@@ -104,13 +96,12 @@ void NearbyShareCertificateStorage::ClearPrivateCertificates() {
 
 void NearbyShareCertificateStorage::ClearPrivateCertificatesOfVisibility(
     DeviceVisibility visibility) {
-  std::optional<std::vector<NearbySharePrivateCertificate>> certs =
-      GetPrivateCertificates();
-  if (!certs) return;
+  std::vector<NearbySharePrivateCertificate> certs = GetPrivateCertificates();
+  if (certs.empty()) return;
 
   bool were_certs_removed = false;
   std::vector<NearbySharePrivateCertificate> new_certs;
-  for (const NearbySharePrivateCertificate& cert : *certs) {
+  for (const NearbySharePrivateCertificate& cert : certs) {
     if (cert.visibility() == visibility) {
       were_certs_removed = true;
     } else {

sharing/certificates/nearby_share_certificate_storage.h

@@ -57,15 +57,13 @@ class NearbyShareCertificateStorage {
           bool, std::unique_ptr<nearby::sharing::proto::PublicCertificate>)>
           callback) = 0;
 
-  // Returns all private certificates currently in storage. Will return
-  // absl::nullopt if deserialization from prefs fails -- not expected to happen
-  // under normal circumstances.
-  virtual std::optional<std::vector<NearbySharePrivateCertificate>>
-  GetPrivateCertificates() const = 0;
+  // Returns all valid private certificates currently in storage.
+  virtual std::vector<NearbySharePrivateCertificate>
+  GetPrivateCertificates() = 0;
 
-  // Returns the next time a certificate expires or absl::nullopt if no
-  // certificates are present.
-  std::optional<absl::Time> NextPrivateCertificateExpirationTime();
+  // Returns the next time a certificate expires or absl::InfinitePast() if
+  // there are fewer than `min_certs` present.
+  absl::Time NextPrivateCertificateExpirationTime(int min_certs);
   virtual std::optional<absl::Time> NextPublicCertificateExpirationTime()
       const = 0;
 

sharing/certificates/nearby_share_certificate_storage_impl.cc

@@ -352,8 +352,8 @@ void NearbyShareCertificateStorageImpl::GetPublicCertificate(
   public_certificate_database_->LoadCertificate(id, std::move(callback));
 }
 
-std::optional<std::vector<NearbySharePrivateCertificate>>
-NearbyShareCertificateStorageImpl::GetPrivateCertificates() const {
+std::vector<NearbySharePrivateCertificate>
+NearbyShareCertificateStorageImpl::GetPrivateCertificates() {
   std::vector<PrivateCertificateData> list =
       preference_manager_.GetPrivateCertificateArray(
           prefs::kNearbySharingPrivateCertificateListName);
@@ -362,8 +362,13 @@ NearbyShareCertificateStorageImpl::GetPrivateCertificates() const {
   for (const PrivateCertificateData& cert_data : list) {
     std::optional<NearbySharePrivateCertificate> cert(
         NearbySharePrivateCertificate::FromCertificateData(cert_data));
+    // If any certificates in preference manager are corrupted, we need to
+    // delete all certificates and regenerate them.
     if (!cert) {
-      return std::nullopt;
+      LOG(ERROR) << "Certificate data corrupted, cleaning up.";
+      ClearPrivateCertificates();
+      // TODO: ftsui - Look into regenerating certificates when this happens.
+      return {};
     }
     // Skip selected contacts visibility certificates.  They are obsolete.
     if (cert->visibility() ==

sharing/certificates/nearby_share_certificate_storage_impl.h

@@ -26,6 +26,7 @@
 #include <vector>
 
 #include "absl/container/flat_hash_set.h"
+#include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "absl/types/span.h"
 #include "sharing/certificates/nearby_share_certificate_storage.h"
@@ -78,8 +79,7 @@ class NearbyShareCertificateStorageImpl : public NearbyShareCertificateStorage,
       std::function<void(
           bool, std::unique_ptr<nearby::sharing::proto::PublicCertificate>)>
           callback) override;
-  std::optional<std::vector<NearbySharePrivateCertificate>>
-  GetPrivateCertificates() const override;
+  std::vector<NearbySharePrivateCertificate> GetPrivateCertificates() override;
   std::optional<absl::Time> NextPublicCertificateExpirationTime()
       const override;
   void ReplacePrivateCertificates(

sharing/certificates/nearby_share_certificate_storage_impl_test.cc

@@ -40,6 +40,7 @@
 #include "sharing/certificates/test_util.h"
 #include "sharing/common/nearby_share_prefs.h"
 #include "sharing/internal/api/mock_public_certificate_db.h"
+#include "sharing/internal/api/private_certificate_data.h"
 #include "sharing/internal/test/fake_preference_manager.h"
 #include "sharing/internal/test/fake_public_certificate_db.h"
 #include "sharing/proto/enums.pb.h"
@@ -557,7 +558,7 @@ TEST_F(NearbyShareCertificateStorageImplTest,
 
   cert_store->RemoveExpiredPrivateCertificates(now);
 
-  certs = *cert_store->GetPrivateCertificates();
+  certs = cert_store->GetPrivateCertificates();
   ASSERT_EQ(1u, certs.size());
   for (const NearbySharePrivateCertificate& cert : certs) {
     EXPECT_LE(now, cert.not_after());
@@ -615,13 +616,13 @@ TEST_F(NearbyShareCertificateStorageImplTest, ReplaceGetPrivateCertificates) {
   auto certs_before = CreatePrivateCertificates(
       3, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
   cert_store->ReplacePrivateCertificates(certs_before);
-  auto certs_after = cert_store->GetPrivateCertificates();
+  std::vector<NearbySharePrivateCertificate> certs_after =
+      cert_store->GetPrivateCertificates();
 
-  ASSERT_TRUE(certs_after.has_value());
-  ASSERT_EQ(certs_before.size(), certs_after->size());
+  ASSERT_EQ(certs_before.size(), certs_after.size());
   for (size_t i = 0; i < certs_before.size(); ++i) {
     EXPECT_EQ(certs_before[i].ToCertificateData(),
-              (*certs_after)[i].ToCertificateData());
+              certs_after[i].ToCertificateData());
   }
 
   certs_before = CreatePrivateCertificates(
@@ -629,11 +630,10 @@ TEST_F(NearbyShareCertificateStorageImplTest, ReplaceGetPrivateCertificates) {
   cert_store->ReplacePrivateCertificates(certs_before);
   certs_after = cert_store->GetPrivateCertificates();
 
-  ASSERT_TRUE(certs_after.has_value());
-  ASSERT_EQ(certs_before.size(), certs_after->size());
+  ASSERT_EQ(certs_before.size(), certs_after.size());
   for (size_t i = 0; i < certs_before.size(); ++i) {
     EXPECT_EQ(certs_before[i].ToCertificateData(),
-              (*certs_after)[i].ToCertificateData());
+              certs_after[i].ToCertificateData());
   }
   EXPECT_THAT(cert_store.use_count(), Eq(1));
 }
@@ -662,7 +662,7 @@ TEST_F(NearbyShareCertificateStorageImplTest, UpdatePrivateCertificates) {
   cert_store->UpdatePrivateCertificate(cert_to_update);
 
   std::vector<NearbySharePrivateCertificate> new_certs =
-      *cert_store->GetPrivateCertificates();
+      cert_store->GetPrivateCertificates();
   EXPECT_EQ(initial_certs.size(), new_certs.size());
   for (size_t i = 0; i < new_certs.size(); ++i) {
     NearbySharePrivateCertificate expected_cert =
@@ -683,22 +683,40 @@ TEST_F(NearbyShareCertificateStorageImplTest,
       preference_manager_, std::move(db));
   fake_db->InvokeInitStatusCallback(FakePublicCertificateDb::InitStatus::kOk);
 
+  static constexpr int kNumCerts = 3;
   auto certs = CreatePrivateCertificates(
-      3, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
+      kNumCerts, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
   cert_store->ReplacePrivateCertificates(certs);
-  std::optional<absl::Time> next_expiration =
-      cert_store->NextPrivateCertificateExpirationTime();
+  absl::Time next_expiration =
+      cert_store->NextPrivateCertificateExpirationTime(kNumCerts);
 
-  ASSERT_TRUE(next_expiration.has_value());
   bool found = false;
   for (auto& cert : certs) {
-    EXPECT_GE(cert.not_after(), *next_expiration);
-    if (cert.not_after() == *next_expiration) found = true;
+    EXPECT_GE(cert.not_after(), next_expiration);
+    if (cert.not_after() == next_expiration) found = true;
   }
   EXPECT_TRUE(found);
   EXPECT_THAT(cert_store.use_count(), Eq(1));
 }
 
+TEST_F(NearbyShareCertificateStorageImplTest,
+       NextPrivateCertificateExpirationTime_NotEnoughCerts) {
+  auto db = std::make_unique<nearby::FakePublicCertificateDb>(
+      PrepopulatePublicCertificates());
+  nearby::FakePublicCertificateDb* fake_db = db.get();
+
+  auto cert_store = NearbyShareCertificateStorageImpl::Factory::Create(
+      preference_manager_, std::move(db));
+  fake_db->InvokeInitStatusCallback(FakePublicCertificateDb::InitStatus::kOk);
+
+  static constexpr int kNumCerts = 3;
+  cert_store->ReplacePrivateCertificates({});
+  absl::Time next_expiration =
+      cert_store->NextPrivateCertificateExpirationTime(kNumCerts);
+
+  EXPECT_EQ(next_expiration, absl::InfinitePast());
+}
+
 TEST_F(NearbyShareCertificateStorageImplTest,
        NextPublicCertificateExpirationTime) {
   auto db = std::make_unique<nearby::FakePublicCertificateDb>(
@@ -737,10 +755,10 @@ TEST_F(NearbyShareCertificateStorageImplTest, ClearPrivateCertificates) {
           3, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
   cert_store->ReplacePrivateCertificates(certs_before);
   cert_store->ClearPrivateCertificates();
-  auto certs_after = cert_store->GetPrivateCertificates();
+  std::vector<NearbySharePrivateCertificate> certs_after =
+      cert_store->GetPrivateCertificates();
 
-  ASSERT_TRUE(certs_after.has_value());
-  EXPECT_EQ(0u, certs_after->size());
+  EXPECT_TRUE(certs_after.empty());
   EXPECT_THAT(cert_store.use_count(), Eq(1));
 }
 
@@ -771,19 +789,18 @@ TEST_F(NearbyShareCertificateStorageImplTest,
     cert_store->ReplacePrivateCertificates(all_certs);
     cert_store->ClearPrivateCertificatesOfVisibility(
         DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
-    auto certs_after = cert_store->GetPrivateCertificates();
-    ASSERT_TRUE(certs_after.has_value());
-    ASSERT_EQ(certs_self.size(), certs_after->size());
+    std::vector<NearbySharePrivateCertificate> certs_after =
+        cert_store->GetPrivateCertificates();
+    ASSERT_EQ(certs_self.size(), certs_after.size());
     for (size_t i = 0; i < certs_self.size(); ++i) {
       EXPECT_EQ(certs_self[i].ToCertificateData(),
-                (*certs_after)[i].ToCertificateData());
+                certs_after[i].ToCertificateData());
     }
 
     cert_store->ClearPrivateCertificatesOfVisibility(
         DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE);
     certs_after = cert_store->GetPrivateCertificates();
-    ASSERT_TRUE(certs_after.has_value());
-    EXPECT_EQ(certs_after->size(), 0u);
+    EXPECT_TRUE(certs_after.empty());
   }
 
   // Remove self certs then remove all-contacts certs.
@@ -791,21 +808,55 @@ TEST_F(NearbyShareCertificateStorageImplTest,
     cert_store->ReplacePrivateCertificates(all_certs);
     cert_store->ClearPrivateCertificatesOfVisibility(
         DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE);
-    auto certs_after = cert_store->GetPrivateCertificates();
-    ASSERT_TRUE(certs_after.has_value());
-    ASSERT_EQ(certs_all_contacts.size(), certs_after->size());
+    std::vector<NearbySharePrivateCertificate> certs_after =
+        cert_store->GetPrivateCertificates();
+    ASSERT_EQ(certs_all_contacts.size(), certs_after.size());
     for (size_t i = 0; i < certs_all_contacts.size(); ++i) {
       EXPECT_EQ(certs_all_contacts[i].ToCertificateData(),
-                (*certs_after)[i].ToCertificateData());
+                certs_after[i].ToCertificateData());
     }
 
     cert_store->ClearPrivateCertificatesOfVisibility(
         DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
     certs_after = cert_store->GetPrivateCertificates();
-    ASSERT_TRUE(certs_after.has_value());
-    EXPECT_EQ(certs_after->size(), 0u);
+    EXPECT_TRUE(certs_after.empty());
   }
   EXPECT_THAT(cert_store.use_count(), Eq(1));
 }
 
+TEST_F(NearbyShareCertificateStorageImplTest,
+       GetPrivateCertificates_WithCorruptedCerts) {
+  auto db = std::make_unique<nearby::FakePublicCertificateDb>(
+      PrepopulatePublicCertificates());
+  nearby::FakePublicCertificateDb* fake_db = db.get();
+
+  auto cert_store = NearbyShareCertificateStorageImpl::Factory::Create(
+      preference_manager_, std::move(db));
+  fake_db->InvokeInitStatusCallback(FakePublicCertificateDb::InitStatus::kOk);
+
+  std::vector<NearbySharePrivateCertificate> certs_all_contacts =
+      CreatePrivateCertificates(
+          3, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS);
+  cert_store->ReplacePrivateCertificates(certs_all_contacts);
+
+  std::vector<api::PrivateCertificateData> private_cert_data =
+      preference_manager_.GetPrivateCertificateArray(
+          prefs::kNearbySharingPrivateCertificateListName);
+  ASSERT_EQ(private_cert_data.size(), 3u);
+  // Set to invalid base64 encoded string.
+  private_cert_data[0].key_pair = "::..\\|@#";
+  preference_manager_.SetPrivateCertificateArray(
+      prefs::kNearbySharingPrivateCertificateListName, private_cert_data);
+
+  std::vector<NearbySharePrivateCertificate> certs =
+      cert_store->GetPrivateCertificates();
+
+  // Verify corrupted cert has been removed.
+  EXPECT_TRUE(certs.empty());
+  private_cert_data =
+      preference_manager_.GetPrivateCertificateArray(
+          prefs::kNearbySharingPrivateCertificateListName);
+  EXPECT_TRUE(private_cert_data.empty());
+}
+
 }  // namespace nearby::sharing