From ca547df4d76c26355a0d675ce4b1bae886208b18 Mon Sep 17 00:00:00 2001 From: Janusz Sobczak Date: Tue, 17 Jan 2023 15:05:16 -0800 Subject: [PATCH] Subscribe for public credentials update Nearby Presence scanning sessions may be very long. We need a way to notify the scanner that new credentials were added during scanning. `SubscribeForPublicCredentials()` allows us to subscribe for such updates. PiperOrigin-RevId: 502699345 --- .../implementation/credential_callbacks.h | 11 + presence/implementation/BUILD | 2 + .../implementation/broadcast_manager_test.cc | 4 +- presence/implementation/credential_manager.h | 16 ++ .../implementation/credential_manager_impl.cc | 179 +++++++++++- .../implementation/credential_manager_impl.h | 82 +++++- .../credential_manager_impl_test.cc | 260 ++++++++++++++---- presence/implementation/scan_manager_test.cc | 9 +- .../implementation/service_controller_impl.h | 13 +- 9 files changed, 495 insertions(+), 81 deletions(-) diff --git a/internal/platform/implementation/credential_callbacks.h b/internal/platform/implementation/credential_callbacks.h index ec26bb45..7be3c555 100644 --- a/internal/platform/implementation/credential_callbacks.h +++ b/internal/platform/implementation/credential_callbacks.h @@ -43,6 +43,17 @@ struct CredentialSelector { credential_selector.account_name, static_cast(credential_selector.identity_type)); } + template + friend H AbslHashValue(H h, const CredentialSelector& selector) { + return H::combine(std::move(h), selector.manager_app_id, + selector.account_name, selector.identity_type); + } + friend bool operator==(const CredentialSelector& a, + const CredentialSelector& b) { + return a.manager_app_id == b.manager_app_id && + a.account_name == b.account_name && + a.identity_type == b.identity_type; + } }; enum class PublicCredentialType { diff --git a/presence/implementation/BUILD b/presence/implementation/BUILD index 323c1a72..6dd4a82b 100644 --- a/presence/implementation/BUILD +++ b/presence/implementation/BUILD @@ -75,6 +75,7 @@ cc_library( "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/hash", "@com_google_absl//absl/log:die_if_null", "@com_google_absl//absl/random", "@com_google_absl//absl/random:distributions", @@ -247,6 +248,7 @@ cc_test( deps = [ ":internal", "//internal/platform:comm", + "//internal/platform:test_util", "//internal/platform:types", "//internal/platform/implementation:types", "//internal/platform/implementation/g3", # build_cleaner: keep diff --git a/presence/implementation/broadcast_manager_test.cc b/presence/implementation/broadcast_manager_test.cc index 6925770c..ca578bc9 100644 --- a/presence/implementation/broadcast_manager_test.cc +++ b/presence/implementation/broadcast_manager_test.cc @@ -98,8 +98,8 @@ class BroadcastManagerTest : public testing::TestWithParam { start_broadcast_status_.Set(status); }}; Mediums mediums_; - CredentialManagerImpl credential_manager_; - nearby::SingleThreadExecutor executor_; + SingleThreadExecutor executor_; + CredentialManagerImpl credential_manager_{&executor_}; BroadcastManager broadcast_manager_{mediums_, credential_manager_, executor_}; }; diff --git a/presence/implementation/credential_manager.h b/presence/implementation/credential_manager.h index ffd9fb89..e5798d8d 100644 --- a/presence/implementation/credential_manager.h +++ b/presence/implementation/credential_manager.h @@ -27,6 +27,8 @@ namespace nearby { namespace presence { +using SubscriberId = uint64_t; + /* * The instance of CredentialManager is owned by {@code ServiceControllerImpl}. * Helping service controller to manage local credentials and coordinate with @@ -68,6 +70,20 @@ class CredentialManager { PublicCredentialType public_credential_type, GetPublicCredentialsResultCallback callback) = 0; + // Subscribes for public credentials updates. The `callback` is triggered when + // the public credentials are fetched initially, and then every time the + // credentials change. + virtual SubscriberId SubscribeForPublicCredentials( + const CredentialSelector& credential_selector, + PublicCredentialType public_credential_type, + GetPublicCredentialsResultCallback callback) = 0; + + // Unsubscribes from public credentials updates. No new callbacks will be + // triggered after this function returns. If there is a callback already + // running, that callback may continue after + // `UnsubscribeFromPublicCredentials()` return. + virtual void UnsubscribeFromPublicCredentials(SubscriberId id) = 0; + // Decrypts the device metadata from a public credential. // Returns an empty string if decryption fails. virtual std::string DecryptDeviceMetadata( diff --git a/presence/implementation/credential_manager_impl.cc b/presence/implementation/credential_manager_impl.cc index c51d9eed..fb92ef6d 100644 --- a/presence/implementation/credential_manager_impl.cc +++ b/presence/implementation/credential_manager_impl.cc @@ -14,6 +14,7 @@ #include "presence/implementation/credential_manager_impl.h" +#include #include #include #include @@ -25,6 +26,7 @@ #include "internal/crypto/aead.h" #include "internal/crypto/ec_private_key.h" #include "internal/crypto/hkdf.h" +#include "internal/crypto/random.h" #include "internal/platform/base64_utils.h" #include "internal/platform/future.h" #include "internal/platform/implementation/credential_callbacks.h" @@ -87,16 +89,27 @@ void CredentialManagerImpl::GenerateCredentials( public_credentials, PublicCredentialType::kLocalPublicCredential, SaveCredentialsResultCallback{ .credentials_saved_cb = - [callback = std::move(credentials_generated_cb), + [this, manager_app_id = std::string(manager_app_id), + account_name = device_metadata.account_name(), + callback = std::move(credentials_generated_cb), public_credentials](absl::Status status) mutable { - if (status.ok()) { - std::move(callback.credentials_generated_cb)( - std::move(public_credentials)); - } else { + if (!status.ok()) { NEARBY_LOGS(WARNING) << "Save credentials failed with: " << status; std::move(callback.credentials_generated_cb)(status); + return; } + std::move(callback.credentials_generated_cb)( + std::move(public_credentials)); + RunOnServiceControllerThread( + "local-creds-changed", + [this, manager_app_id = std::string(manager_app_id), + account_name = std::string(account_name)]() + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) { + OnCredentialsChanged( + manager_app_id, account_name, + PublicCredentialType::kLocalPublicCredential); + }); }}); } @@ -108,15 +121,27 @@ void CredentialManagerImpl::UpdateRemotePublicCredentials( manager_app_id, account_name, /* private_credentials */ {}, remote_public_creds, PublicCredentialType::kRemotePublicCredential, SaveCredentialsResultCallback{ - .credentials_saved_cb = [callback = - std::move(credentials_updated_cb)]( - absl::Status status) mutable { - if (!status.ok()) { - NEARBY_LOGS(WARNING) - << "Update remote credentials failed with: " << status; - } - std::move(callback.credentials_updated_cb)(status); - }}); + .credentials_saved_cb = + [this, manager_app_id = std::string(manager_app_id), + account_name = std::string(account_name), + callback = std::move(credentials_updated_cb)]( + absl::Status status) mutable { + if (!status.ok()) { + NEARBY_LOGS(WARNING) + << "Update remote credentials failed with: " << status; + } else { + RunOnServiceControllerThread( + "remote-creds-changed", + [this, manager_app_id = std::string(manager_app_id), + account_name = std::string(account_name)]() + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) { + OnCredentialsChanged( + manager_app_id, account_name, + PublicCredentialType::kRemotePublicCredential); + }); + } + std::move(callback.credentials_updated_cb)(status); + }}); } std::pair @@ -316,5 +341,131 @@ CredentialManagerImpl::GetPublicCredentialsSync( return result.Get(timeout); } +SubscriberId CredentialManagerImpl::SubscribeForPublicCredentials( + const CredentialSelector& credential_selector, + PublicCredentialType public_credential_type, + GetPublicCredentialsResultCallback callback) { + SubscriberId id = ::crypto::RandData(); + RunOnServiceControllerThread( + "add-subscriber", + [this, key = SubscriberKey{credential_selector, public_credential_type}, + id, callback = std::move(callback)]() + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) mutable { + AddSubscriber(key, id, std::move(callback)); + }); + GetPublicCredentials(credential_selector, public_credential_type, + CreateNotifySubscribersCallback( + {credential_selector, public_credential_type})); + return id; +} + +void CredentialManagerImpl::UnsubscribeFromPublicCredentials(SubscriberId id) { + RunOnServiceControllerThread("remove-subscriber", + [this, id]() ABSL_EXCLUSIVE_LOCKS_REQUIRED( + *executor_) { RemoveSubscriber(id); }); +} + +void CredentialManagerImpl::AddSubscriber( + SubscriberKey key, SubscriberId id, + GetPublicCredentialsResultCallback callback) { + subscribers_[key].push_back(Subscriber(id, std::move(callback))); +} + +void CredentialManagerImpl::RemoveSubscriber(SubscriberId id) { + for (auto& entry : subscribers_) { + auto it = std::find_if( + entry.second.begin(), entry.second.end(), + [&](Subscriber& subscriber) { return subscriber.GetId() == id; }); + if (it != entry.second.end()) { + entry.second.erase(it); + if (subscribers_[entry.first].empty()) { + subscribers_.erase(entry.first); + } + return; + } + } +} + +absl::flat_hash_set +CredentialManagerImpl::GetSubscribedIdentities( + absl::string_view manager_app_id, absl::string_view account_name, + PublicCredentialType credential_type) const { + absl::flat_hash_set identities; + for (auto& entry : subscribers_) { + const SubscriberKey& key = entry.first; + if (key.public_credential_type == credential_type && + key.credential_selector.manager_app_id == manager_app_id && + key.credential_selector.account_name == account_name) { + identities.insert(key.credential_selector.identity_type); + } + } + return identities; +} + +void CredentialManagerImpl::OnCredentialsChanged( + absl::string_view manager_app_id, absl::string_view account_name, + PublicCredentialType credential_type) { + NEARBY_LOGS(INFO) << "OnCredentialsChanged for app " << manager_app_id + << ", account " << account_name; + for (IdentityType identity_type : + GetSubscribedIdentities(manager_app_id, account_name, credential_type)) { + CredentialSelector credential_selector = { + .manager_app_id = std::string(manager_app_id), + .account_name = std::string(account_name), + .identity_type = identity_type}; + GetPublicCredentials(credential_selector, credential_type, + CreateNotifySubscribersCallback( + {credential_selector, credential_type})); + } +} + +GetPublicCredentialsResultCallback +CredentialManagerImpl::CreateNotifySubscribersCallback(SubscriberKey key) { + return GetPublicCredentialsResultCallback{ + .credentials_fetched_cb = + [this, key]( + absl::StatusOr> + credentials) { + if (!credentials.ok()) { + NEARBY_LOGS(WARNING) + << "Failed to get public credentials: error code: " + << credentials.status(); + return; + } + RunOnServiceControllerThread( + "notify-subscribers", + [this, key, credentials = std::move(*credentials)]() + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) { + NotifySubscribers(key, credentials); + }); + }}; +} + +void CredentialManagerImpl::NotifySubscribers( + const SubscriberKey& key, + std::vector<::nearby::internal::PublicCredential> credentials) { + // We are on `executor_` thread, so we can iterate over `subscribers_` + // without locking. + auto it = subscribers_.find(key); + if (it == subscribers_.end()) { + NEARBY_LOGS(WARNING) + << "No subscribers for (app: " << key.credential_selector.manager_app_id + << ", account: " << key.credential_selector.account_name + << ", identity type: " + << static_cast(key.credential_selector.identity_type) + << ", credential type: " << static_cast(key.public_credential_type) + << ")"; + return; + } + for (auto& subscriber : it->second) { + subscriber.NotifyCredentialsFetched(credentials); + } +} + +void CredentialManagerImpl::Subscriber::NotifyCredentialsFetched( + std::vector<::nearby::internal::PublicCredential>& credentials) { + callback_.credentials_fetched_cb(credentials); +} + } // namespace presence } // namespace nearby diff --git a/presence/implementation/credential_manager_impl.h b/presence/implementation/credential_manager_impl.h index 32303909..f7454d81 100644 --- a/presence/implementation/credential_manager_impl.h +++ b/presence/implementation/credential_manager_impl.h @@ -15,16 +15,21 @@ #ifndef THIRD_PARTY_NEARBY_PRESENCE_IMPLEMENTATION_CREDENTIAL_MANAGER_IMPL_H_ #define THIRD_PARTY_NEARBY_PRESENCE_IMPLEMENTATION_CREDENTIAL_MANAGER_IMPL_H_ +#include #include #include #include #include +#include "absl/base/thread_annotations.h" +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "internal/platform/credential_storage_impl.h" #include "internal/platform/implementation/credential_callbacks.h" +#include "internal/platform/runnable.h" +#include "internal/platform/single_thread_executor.h" #include "internal/proto/credential.pb.h" #include "presence/implementation/credential_manager.h" @@ -33,14 +38,19 @@ namespace presence { class CredentialManagerImpl : public CredentialManager { public: - CredentialManagerImpl() { + using IdentityType = ::nearby::internal::IdentityType; + + explicit CredentialManagerImpl(SingleThreadExecutor* executor) + : executor_(ABSL_DIE_IF_NULL(executor)) { credential_storage_ptr_ = std::make_unique(); } // Test purpose only. - explicit CredentialManagerImpl( + CredentialManagerImpl( + SingleThreadExecutor* executor, std::unique_ptr credential_storage_ptr) - : credential_storage_ptr_(std::move(credential_storage_ptr)) {} + : executor_(ABSL_DIE_IF_NULL(executor)), + credential_storage_ptr_(std::move(credential_storage_ptr)) {} // AES only supports key sizes of 16, 24 or 32 bytes. static constexpr int kAuthenticityKeyByteSize = 16; @@ -85,6 +95,13 @@ class CredentialManagerImpl : public CredentialManager { PublicCredentialType public_credential_type, absl::Duration timeout); + SubscriberId SubscribeForPublicCredentials( + const CredentialSelector& credential_selector, + PublicCredentialType public_credential_type, + GetPublicCredentialsResultCallback callback) override; + + void UnsubscribeFromPublicCredentials(SubscriberId id) override; + std::string DecryptDeviceMetadata( absl::string_view device_metadata_encryption_key, absl::string_view authenticity_key, @@ -94,8 +111,7 @@ class CredentialManagerImpl : public CredentialManager { nearby::internal::PublicCredential> CreatePrivateCredential( const nearby::internal::DeviceMetadata& device_metadata, - nearby::internal::IdentityType identity_type, uint64_t start_time_ms, - uint64_t end_time_ms); + IdentityType identity_type, uint64_t start_time_ms, uint64_t end_time_ms); nearby::internal::PublicCredential CreatePublicCredential( const nearby::internal::PrivateCredential& private_credential, @@ -111,6 +127,62 @@ class CredentialManagerImpl : public CredentialManager { absl::string_view device_metadata_encryption_key); private: + struct SubscriberKey { + CredentialSelector credential_selector; + PublicCredentialType public_credential_type; + template + friend H AbslHashValue(H h, const SubscriberKey& key) { + return H::combine(std::move(h), key.credential_selector, + key.public_credential_type); + } + friend bool operator==(const SubscriberKey& a, const SubscriberKey& b) { + return a.public_credential_type == b.public_credential_type && + a.credential_selector == b.credential_selector; + } + }; + class Subscriber { + public: + Subscriber(SubscriberId id, GetPublicCredentialsResultCallback callback) + : callback_(std::move(callback)), id_(id) {} + + SubscriberId GetId() const { return id_; } + + // Notifies the subscriber about fetched credentials. + void NotifyCredentialsFetched( + std::vector<::nearby::internal::PublicCredential>& credentials); + + private: + GetPublicCredentialsResultCallback callback_; + SubscriberId id_; + }; + + void RunOnServiceControllerThread(absl::string_view name, + Runnable&& runnable) { + executor_->Execute(std::string(name), std::move(runnable)); + } + void OnCredentialsChanged(absl::string_view manager_app_id, + absl::string_view account_name, + PublicCredentialType credential_type) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_); + void NotifySubscribers( + const SubscriberKey& key, + std::vector<::nearby::internal::PublicCredential> credentials) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_); + void AddSubscriber(SubscriberKey key, SubscriberId id, + GetPublicCredentialsResultCallback callback) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_); + void RemoveSubscriber(SubscriberId id) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_); + absl::flat_hash_set GetSubscribedIdentities( + absl::string_view manager_app_id, absl::string_view account_name, + PublicCredentialType credential_type) const + ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_); + GetPublicCredentialsResultCallback CreateNotifySubscribersCallback( + SubscriberKey key); + + absl::flat_hash_map> subscribers_ + ABSL_GUARDED_BY(*executor_); + SingleThreadExecutor* executor_; std::unique_ptr credential_storage_ptr_; }; diff --git a/presence/implementation/credential_manager_impl_test.cc b/presence/implementation/credential_manager_impl_test.cc index 2601cb73..b61b4727 100644 --- a/presence/implementation/credential_manager_impl_test.cc +++ b/presence/implementation/credential_manager_impl_test.cc @@ -27,12 +27,17 @@ #include "internal/platform/count_down_latch.h" #include "internal/platform/credential_storage_impl.h" #include "internal/platform/implementation/crypto.h" +#include "internal/platform/logging.h" +#include "internal/platform/medium_environment.h" #include "internal/proto/credential.pb.h" +#include "internal/proto/credential.proto.h" namespace nearby { namespace presence { namespace { +using ::nearby::CountDownLatch; using ::nearby::Crypto; +using ::nearby::MediumEnvironment; using ::nearby::internal::DeviceMetadata; using ::nearby::internal::IdentityType; using ::nearby::internal::PrivateCredential; @@ -42,10 +47,14 @@ using ::proto2::contrib::parse_proto::ParseTestProto; using ::protobuf_matchers::EqualsProto; using ::testing::status::StatusIs; -DeviceMetadata CreateTestDeviceMetadata() { +constexpr absl::string_view kManagerAppId = "TEST_MANAGER_APP"; +constexpr absl::string_view kAccountName = "test account"; + +DeviceMetadata CreateTestDeviceMetadata( + absl::string_view account_name = kAccountName) { DeviceMetadata device_metadata; device_metadata.set_stable_device_id("test_device_id"); - device_metadata.set_account_name("test_account"); + device_metadata.set_account_name(account_name); device_metadata.set_device_name("NP test device"); device_metadata.set_icon_url("test_image.test.com"); device_metadata.set_bluetooth_mac_address("FF:FF:FF:FF:FF:FF"); @@ -55,8 +64,8 @@ DeviceMetadata CreateTestDeviceMetadata() { CredentialSelector BuildDefaultCredentialSelector() { CredentialSelector credential_selector; - credential_selector.manager_app_id = "TEST_MANAGER_APP"; - credential_selector.account_name = "test_account"; + credential_selector.manager_app_id = std::string(kManagerAppId); + credential_selector.account_name = std::string(kAccountName); credential_selector.identity_type = IDENTITY_TYPE_PRIVATE; return credential_selector; } @@ -85,6 +94,8 @@ class CredentialManagerImplTest : public ::testing::Test { class MockCredentialManager : public CredentialManagerImpl { public: + explicit MockCredentialManager(SingleThreadExecutor* executor) + : CredentialManagerImpl(executor) {} MOCK_METHOD(std::string, EncryptDeviceMetadata, (absl::string_view device_metadata_encryption_key, absl::string_view authenticity_key, @@ -92,21 +103,45 @@ class CredentialManagerImplTest : public ::testing::Test { (override)); }; - CredentialManagerImplTest() { - mock_credential_storage_ptr_ = std::make_unique(); - mock_credential_manager_ptr_ = std::make_unique(); + ~CredentialManagerImplTest() override { executor_.Shutdown(); } + + // Waits for active tasks in the background thread to complete. + void Fence() { + // A runnable on medium environment thread can add a task on "our" executor, + // and vice-versa. We need to wait for tasks on both threads in a loop a few + // times to make sure that all tasks have finished. + for (int i = 0; i < 3; i++) { + MediumEnvironment::Instance().Sync(); + CountDownLatch latch(1); + executor_.Execute([&]() { latch.CountDown(); }); + latch.Await(); + } + } + + void AddLocalIdentity(absl::string_view manager_app_id, + absl::string_view account_name, + IdentityType identity_type) { + DeviceMetadata device_metadata = CreateTestDeviceMetadata(account_name); + + credential_manager_.GenerateCredentials( + device_metadata, manager_app_id, {identity_type}, + /*credential_life_cycle_days=*/1, + /*contigous_copy_of_credentials=*/1, + {[](absl::StatusOr> credentials) { + EXPECT_OK(credentials); + }}); } protected: - std::unique_ptr mock_credential_storage_ptr_; - std::unique_ptr mock_credential_manager_ptr_; + SingleThreadExecutor executor_; + CredentialManagerImpl credential_manager_{&executor_}; + MockCredentialManager mock_credential_manager_{&executor_}; }; -TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { +TEST_F(CredentialManagerImplTest, CreateOneCredentialSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - CredentialManagerImpl credential_manager; - auto credentials = credential_manager.CreatePrivateCredential( + auto credentials = credential_manager_.CreatePrivateCredential( device_metadata, IDENTITY_TYPE_PRIVATE, /* start_time_ms= */ 0, /* end_time_ms= */ 1000); @@ -141,7 +176,7 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { // Decrypt the device metadata - auto decrypted_device_metadata = credential_manager.DecryptDeviceMetadata( + auto decrypted_device_metadata = credential_manager_.DecryptDeviceMetadata( private_credential.metadata_encryption_key(), public_credential.authenticity_key(), public_credential.encrypted_metadata_bytes()); @@ -150,16 +185,14 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { decrypted_device_metadata); } -TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { +TEST_F(CredentialManagerImplTest, GenerateCredentialsSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - CredentialManagerImpl credential_manager; absl::StatusOr> public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; - credential_manager.GenerateCredentials( - device_metadata, - /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, + credential_manager_.GenerateCredentials( + device_metadata, kManagerAppId, identityTypes, 1, 2, {.credentials_generated_cb = [&](absl::StatusOr> credentials) { @@ -178,7 +211,89 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { } } -TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { +TEST_F(CredentialManagerImplTest, + SubscribeCallsCallbackWithExistingCredentials) { + absl::StatusOr> public_credentials1; + absl::StatusOr> public_credentials2; + AddLocalIdentity(kManagerAppId, kAccountName, IDENTITY_TYPE_PRIVATE); + + SubscriberId id1 = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = IDENTITY_TYPE_PRIVATE}, + PublicCredentialType::kLocalPublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + public_credentials1 = std::move(credentials); + }}); + SubscriberId id2 = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = IDENTITY_TYPE_PRIVATE}, + PublicCredentialType::kLocalPublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + public_credentials2 = std::move(credentials); + }}); + + Fence(); + EXPECT_OK(public_credentials1); + EXPECT_OK(public_credentials2); + EXPECT_EQ(public_credentials1->size(), 1); + EXPECT_EQ(public_credentials2->size(), 1); + // Cleanup + credential_manager_.UnsubscribeFromPublicCredentials(id1); + credential_manager_.UnsubscribeFromPublicCredentials(id2); + Fence(); +} + +TEST_F(CredentialManagerImplTest, + SubscribeCallsCallbackWithUpdatedCredentials) { + absl::StatusOr> public_credentials; + + SubscriberId id = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = IDENTITY_TYPE_PRIVATE}, + PublicCredentialType::kLocalPublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + public_credentials = std::move(credentials); + }}); + Fence(); + EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kUnknown)); + + AddLocalIdentity(kManagerAppId, kAccountName, IDENTITY_TYPE_PRIVATE); + + Fence(); + ASSERT_OK(public_credentials); + EXPECT_EQ(public_credentials->size(), 1); + // Cleanup + credential_manager_.UnsubscribeFromPublicCredentials(id); + Fence(); +} + +TEST_F(CredentialManagerImplTest, NoCallbacksAfterUnsubscribe) { + absl::StatusOr> public_credentials; + SubscriberId id = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = IDENTITY_TYPE_PRIVATE}, + PublicCredentialType::kLocalPublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + public_credentials = std::move(credentials); + }}); + + credential_manager_.UnsubscribeFromPublicCredentials(id); + AddLocalIdentity(kManagerAppId, kAccountName, IDENTITY_TYPE_PRIVATE); + + Fence(); + EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kUnknown)); +} + +TEST_F(CredentialManagerImplTest, + GenerateCredentialsSuccessfullyButStoreFailed) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); auto credential_storage_ptr = std::make_unique(); @@ -192,14 +307,14 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { callback.credentials_saved_cb( absl::FailedPreconditionError("Expected failure")); })); - CredentialManagerImpl credential_manager(std::move(credential_storage_ptr)); + credential_manager_ = + CredentialManagerImpl(&executor_, std::move(credential_storage_ptr)); absl::StatusOr> public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; - credential_manager.GenerateCredentials( - device_metadata, - /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, + credential_manager_.GenerateCredentials( + device_metadata, kManagerAppId, identityTypes, 1, 2, {.credentials_generated_cb = [&](absl::StatusOr> credentials) { @@ -209,11 +324,11 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { StatusIs(absl::StatusCode::kFailedPrecondition)); } -TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { +TEST_F(CredentialManagerImplTest, UpdateRemotePublicCredentialsSuccessfully) { nearby::internal::PublicCredential public_credential_for_test; public_credential_for_test.set_identity_type( nearby::internal::IdentityType::IDENTITY_TYPE_TRUSTED); - std::vector publicCredentials{ + std::vector public_credentials{ {public_credential_for_test}}; nearby::CountDownLatch updated_latch(1); @@ -226,25 +341,68 @@ TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { }, }; - CredentialManagerImpl credential_manager; - - credential_manager.UpdateRemotePublicCredentials( - /* manager_app_id= */ "TEST_MANAGER_APP", - /* account_name= */ "test_account", publicCredentials, + credential_manager_.UpdateRemotePublicCredentials( + kManagerAppId, kAccountName, public_credentials, std::move(update_credentials_cb)); EXPECT_TRUE(updated_latch.Await().Ok()); } -TEST(CredentialManagerImpl, GetPrivateCredentialsFailed) { - absl::StatusOr> private_credentials; - CredentialSelector credential_selector; - credential_selector.manager_app_id = "TEST_MANAGER_APP"; - credential_selector.account_name = "test_account"; - credential_selector.identity_type = IDENTITY_TYPE_PRIVATE; - CredentialManagerImpl credential_manager; +TEST_F(CredentialManagerImplTest, + UpdateRemotePublicCredentialsNotifiesSubscribers) { + absl::StatusOr> subscribed_credentials; + nearby::internal::PublicCredential public_credential_for_test; + public_credential_for_test.set_identity_type( + nearby::internal::IdentityType::IDENTITY_TYPE_PRIVATE); + std::vector public_credentials{ + {public_credential_for_test}}; + nearby::CountDownLatch updated_latch(1); + UpdateRemotePublicCredentialsCallback update_credentials_cb{ + .credentials_updated_cb = + [&updated_latch](absl::Status status) { + if (status.ok()) { + updated_latch.CountDown(); + } + }, + }; + SubscriberId id1 = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = internal::IDENTITY_TYPE_PRIVATE}, + PublicCredentialType::kRemotePublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + subscribed_credentials = std::move(credentials); + }}); + SubscriberId id2 = credential_manager_.SubscribeForPublicCredentials( + CredentialSelector{.manager_app_id = std::string(kManagerAppId), + .account_name = std::string(kAccountName), + .identity_type = internal::IDENTITY_TYPE_TRUSTED}, + PublicCredentialType::kRemotePublicCredential, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + // This callback should not be called because there are no Trusted + // credentials in this test. + GTEST_FAIL(); + }}); - credential_manager.GetPrivateCredentials( + credential_manager_.UpdateRemotePublicCredentials( + kManagerAppId, kAccountName, public_credentials, + std::move(update_credentials_cb)); + + EXPECT_TRUE(updated_latch.Await().Ok()); + Fence(); + EXPECT_OK(subscribed_credentials); + EXPECT_EQ(subscribed_credentials->size(), 1); + credential_manager_.UnsubscribeFromPublicCredentials(id1); + credential_manager_.UnsubscribeFromPublicCredentials(id2); +} + +TEST_F(CredentialManagerImplTest, GetPrivateCredentialsFailed) { + absl::StatusOr> private_credentials; + CredentialSelector credential_selector = BuildDefaultCredentialSelector(); + + credential_manager_.GetPrivateCredentials( credential_selector, {.credentials_fetched_cb = [&](absl::StatusOr> credentials) { @@ -254,15 +412,11 @@ TEST(CredentialManagerImpl, GetPrivateCredentialsFailed) { EXPECT_THAT(private_credentials, StatusIs(absl::StatusCode::kNotFound)); } -TEST(CredentialManagerImpl, GetPublicCredentialsFailed) { +TEST_F(CredentialManagerImplTest, GetPublicCredentialsFailed) { absl::StatusOr> public_credentials; - CredentialSelector credential_selector; - credential_selector.manager_app_id = "TEST_MANAGER_APP"; - credential_selector.account_name = "test_account"; - credential_selector.identity_type = IDENTITY_TYPE_PRIVATE; - CredentialManagerImpl credential_manager; + CredentialSelector credential_selector = BuildDefaultCredentialSelector(); - credential_manager.GetPublicCredentials( + credential_manager_.GetPublicCredentials( credential_selector, PublicCredentialType::kLocalPublicCredential, {.credentials_fetched_cb = [&](absl::StatusOr> credentials) { @@ -272,23 +426,22 @@ TEST(CredentialManagerImpl, GetPublicCredentialsFailed) { EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kNotFound)); } -TEST(CredentialManagerImpl, GetCredentialsSuccessfully) { +TEST_F(CredentialManagerImplTest, GetCredentialsSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); absl::StatusOr> public_credentials; - CredentialManagerImpl credential_manager; std::vector identity_types{IDENTITY_TYPE_PRIVATE}; absl::StatusOr> private_credentials; CredentialSelector credential_selector = BuildDefaultCredentialSelector(); - credential_manager.GenerateCredentials( - device_metadata, "TEST_MANAGER_APP", identity_types, 1, 1, + credential_manager_.GenerateCredentials( + device_metadata, kManagerAppId, identity_types, 1, 1, {.credentials_generated_cb = [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); }}); - credential_manager.GetPrivateCredentials( + credential_manager_.GetPrivateCredentials( credential_selector, {.credentials_fetched_cb = [&](absl::StatusOr> credentials) { @@ -301,12 +454,13 @@ TEST(CredentialManagerImpl, GetCredentialsSuccessfully) { EXPECT_FALSE(private_credentials->empty()); } -TEST(CredentialManagerImpl, PublicCredentialsFailEncryption) { +TEST_F(CredentialManagerImplTest, PublicCredentialsFailEncryption) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); absl::StatusOr> public_credentials; auto credential_manager_ptr = - std::make_unique(); + std::make_unique( + &executor_); EXPECT_CALL(*credential_manager_ptr, EncryptDeviceMetadata) .WillOnce(::testing::Invoke( [](absl::string_view device_metadata_encryption_key, @@ -315,7 +469,7 @@ TEST(CredentialManagerImpl, PublicCredentialsFailEncryption) { std::vector identity_types{IDENTITY_TYPE_PRIVATE}; credential_manager_ptr->GenerateCredentials( - device_metadata, "TEST_MANAGER_APP", identity_types, 1, 1, + device_metadata, kManagerAppId, identity_types, 1, 1, {.credentials_generated_cb = [&](absl::StatusOr> credentials) { diff --git a/presence/implementation/scan_manager_test.cc b/presence/implementation/scan_manager_test.cc index 2945a0d0..67facd21 100644 --- a/presence/implementation/scan_manager_test.cc +++ b/presence/implementation/scan_manager_test.cc @@ -51,7 +51,10 @@ using CountDownLatch = ::nearby::CountDownLatch; class ScanManagerTest : public testing::Test { protected: void SetUp() override { env_.Start(); } - void TearDown() override { env_.Stop(); } + void TearDown() override { + executor_.Shutdown(); + env_.Stop(); + } std::unique_ptr StartAdvertisingOn(Ble& ble) { PresenceBroadcast::BroadcastSection section = { @@ -109,11 +112,11 @@ class ScanManagerTest : public testing::Test { std::vector MakeDefaultExtendedProperties() { return {DataElement(ActionBit::kPresenceManagerAction)}; } - CredentialManagerImpl credential_manager_; + SingleThreadExecutor executor_; + CredentialManagerImpl credential_manager_{&executor_}; nearby::MediumEnvironment& env_ = {nearby::MediumEnvironment::Instance()}; CountDownLatch start_latch_{1}; CountDownLatch found_latch_{1}; - SingleThreadExecutor executor_; }; TEST_F(ScanManagerTest, CanStartThenStopScanning) { diff --git a/presence/implementation/service_controller_impl.h b/presence/implementation/service_controller_impl.h index 0021f085..e36ff749 100644 --- a/presence/implementation/service_controller_impl.h +++ b/presence/implementation/service_controller_impl.h @@ -38,7 +38,6 @@ class ServiceControllerImpl : public ServiceController { public: using SingleThreadExecutor = ::nearby::SingleThreadExecutor; - ServiceControllerImpl() = default; ~ServiceControllerImpl() override { executor_.Shutdown(); } absl::StatusOr StartScan(ScanRequest scan_request, @@ -55,9 +54,15 @@ class ServiceControllerImpl : public ServiceController { private: SingleThreadExecutor executor_; - Mediums mediums_; - CredentialManagerImpl credential_manager_; - ScanManager scan_manager_{mediums_, credential_manager_, executor_}; + void NotifyStartCallbackStatus(BroadcastSessionId id, absl::Status status); + void RunOnServiceControllerThread(absl::string_view name, Runnable runnable) { + executor_.Execute(std::string(name), std::move(runnable)); + } + Mediums mediums_; // NOLINT: further impl will use it. + CredentialManagerImpl credential_manager_{ + &executor_}; // NOLINT: further impl will use it. + ScanManager scan_manager_{mediums_, credential_manager_, + executor_}; // NOLINT: further impl will use it. BroadcastManager broadcast_manager_{mediums_, credential_manager_, executor_}; };