From 4f599a0d3e82cd2875d9e311ac9f4ea59fa96f7c Mon Sep 17 00:00:00 2001 From: Xin He Date: Wed, 7 Sep 2022 11:48:38 -0700 Subject: [PATCH] Save generated credentials and make the callback PiperOrigin-RevId: 472776321 --- Package.swift | 2 +- internal/platform/BUILD | 5 +- internal/platform/credential_storage.h | 41 +++--- ..._storage.cc => credential_storage_impl.cc} | 8 +- internal/platform/credential_storage_impl.h | 70 ++++++++++ ...est.cc => credential_storage_impl_test.cc} | 14 +- presence/implementation/BUILD | 2 + .../implementation/credential_manager_impl.cc | 41 ++++-- .../implementation/credential_manager_impl.h | 20 ++- .../credential_manager_impl_test.cc | 132 ++++++++++++++++-- 10 files changed, 266 insertions(+), 69 deletions(-) rename internal/platform/{credential_storage.cc => credential_storage_impl.cc} (89%) create mode 100644 internal/platform/credential_storage_impl.h rename internal/platform/{credential_storage_test.cc => credential_storage_impl_test.cc} (94%) diff --git a/Package.swift b/Package.swift index 56ac6b89..71793c3b 100644 --- a/Package.swift +++ b/Package.swift @@ -472,7 +472,7 @@ let package = Package( "internal/platform/crypto_test.cc", "internal/platform/byte_array_test.cc", "internal/platform/bluetooth_utils_test.cc", - "internal/platform/credential_storage_test.cc", + "internal/platform/credential_storage_impl_test.cc", "internal/platform/single_thread_executor_test.cc", "internal/platform/scheduled_executor_test.cc", "internal/platform/count_down_latch_test.cc", diff --git a/internal/platform/BUILD b/internal/platform/BUILD index 919ec756..619ffe75 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -335,7 +335,7 @@ cc_library( "ble.cc", "ble_v2.cc", "bluetooth_classic.cc", - "credential_storage.cc", + "credential_storage_impl.cc", "file.cc", "wifi_hotspot.cc", "wifi_lan.cc", @@ -347,6 +347,7 @@ cc_library( "bluetooth_adapter.h", "bluetooth_classic.h", "credential_storage.h", + "credential_storage_impl.h", "wifi.h", "wifi_hotspot.h", "wifi_lan.h", @@ -390,7 +391,7 @@ cc_test( "cancelable_alarm_test.cc", "condition_variable_test.cc", "count_down_latch_test.cc", - "credential_storage_test.cc", + "credential_storage_impl_test.cc", "crypto_test.cc", "future_test.cc", "logging_test.cc", diff --git a/internal/platform/credential_storage.h b/internal/platform/credential_storage.h index d778fcdb..30d13136 100644 --- a/internal/platform/credential_storage.h +++ b/internal/platform/credential_storage.h @@ -32,37 +32,30 @@ namespace nearby { */ class CredentialStorage { public: - explicit CredentialStorage() - : impl_(api::ImplementationPlatform::CreateCredentialStorage()) {} - ~CredentialStorage() = default; + virtual ~CredentialStorage() = default; - // CredentialStorage class is movable but not copyable. - CredentialStorage(CredentialStorage&& other) = default; - CredentialStorage& operator=(CredentialStorage&& other) = default; - - void SaveCredentials(absl::string_view manager_app_id, - absl::string_view account_name, - const std::vector<::nearby::internal::PrivateCredential>& - private_credentials, - const std::vector<::nearby::internal::PublicCredential>& - public_credentials, - api::PublicCredentialType public_credential_type, - api::SaveCredentialsResultCallback callback); + virtual void SaveCredentials( + absl::string_view manager_app_id, absl::string_view account_name, + const std::vector<::nearby::internal::PrivateCredential>& + private_credentials, + const std::vector<::nearby::internal::PublicCredential>& + public_credentials, + api::PublicCredentialType public_credential_type, + api::SaveCredentialsResultCallback callback) = 0; // Used to fetch private creds when broadcasting. - void GetPrivateCredentials(const api::CredentialSelector& credential_selector, - api::GetPrivateCredentialsResultCallback callback); + virtual void GetPrivateCredentials( + const api::CredentialSelector& credential_selector, + api::GetPrivateCredentialsResultCallback callback) = 0; // Used to fetch remote public creds when scanning. - void GetPublicCredentials(const api::CredentialSelector& credential_selector, - api::PublicCredentialType public_credential_type, - api::GetPublicCredentialsResultCallback callback); - - private: - std::unique_ptr impl_; + virtual void GetPublicCredentials( + const api::CredentialSelector& credential_selector, + api::PublicCredentialType public_credential_type, + api::GetPublicCredentialsResultCallback callback) = 0; }; } // namespace nearby } // namespace location -#endif // THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_CREDENTIAL_STORAGE_H_ +#endif // THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_CREDENTIAL_STORAGE_H diff --git a/internal/platform/credential_storage.cc b/internal/platform/credential_storage_impl.cc similarity index 89% rename from internal/platform/credential_storage.cc rename to internal/platform/credential_storage_impl.cc index 0cfdc3ad..1cf9daa1 100644 --- a/internal/platform/credential_storage.cc +++ b/internal/platform/credential_storage_impl.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "internal/platform/credential_storage.h" +#include "internal/platform/credential_storage_impl.h" namespace location { namespace nearby { @@ -20,7 +20,7 @@ namespace nearby { using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; -void CredentialStorage::SaveCredentials( +void CredentialStorageImpl::SaveCredentials( absl::string_view manager_app_id, absl::string_view account_name, const std::vector& private_credentials, const std::vector& public_credentials, @@ -31,13 +31,13 @@ void CredentialStorage::SaveCredentials( public_credential_type, callback); } -void CredentialStorage::GetPrivateCredentials( +void CredentialStorageImpl::GetPrivateCredentials( const api::CredentialSelector& credential_selector, api::GetPrivateCredentialsResultCallback callback) { return impl_->GetPrivateCredentials(credential_selector, callback); } -void CredentialStorage::GetPublicCredentials( +void CredentialStorageImpl::GetPublicCredentials( const api::CredentialSelector& credential_selector, api::PublicCredentialType public_credential_type, api::GetPublicCredentialsResultCallback callback) { diff --git a/internal/platform/credential_storage_impl.h b/internal/platform/credential_storage_impl.h new file mode 100644 index 00000000..892e0bfa --- /dev/null +++ b/internal/platform/credential_storage_impl.h @@ -0,0 +1,70 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_CREDENTIAL_STORAGE_IMPL_H_ +#define THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_CREDENTIAL_STORAGE_IMPL_H_ + +#include +#include + +#include "absl/strings/string_view.h" +#include "internal/platform/credential_storage.h" +#include "internal/platform/implementation/credential_storage.h" +#include "internal/platform/implementation/platform.h" + +namespace location { +namespace nearby { + +/* + * The instance of CredentialStorageImpl is owned by {@code CredentialManager}. + * It's a wrapper on top of implementation/credential_storage.h to providing + * credential storage operations for Nearby logic layer to invoke. + */ +class CredentialStorageImpl : public CredentialStorage { + public: + explicit CredentialStorageImpl() + : impl_(api::ImplementationPlatform::CreateCredentialStorage()) {} + + // CredentialStorageImpl class is movable but not copyable. + CredentialStorageImpl(CredentialStorageImpl&& other) = default; + CredentialStorageImpl& operator=(CredentialStorageImpl&& other) = default; + + void SaveCredentials(absl::string_view manager_app_id, + absl::string_view account_name, + const std::vector<::nearby::internal::PrivateCredential>& + private_credentials, + const std::vector<::nearby::internal::PublicCredential>& + public_credentials, + api::PublicCredentialType public_credential_type, + api::SaveCredentialsResultCallback callback) override; + + // Used to fetch private creds when broadcasting. + void GetPrivateCredentials( + const api::CredentialSelector& credential_selector, + api::GetPrivateCredentialsResultCallback callback) override; + + // Used to fetch remote public creds when scanning. + void GetPublicCredentials( + const api::CredentialSelector& credential_selector, + api::PublicCredentialType public_credential_type, + api::GetPublicCredentialsResultCallback callback) override; + + private: + std::unique_ptr impl_; +}; + +} // namespace nearby +} // namespace location + +#endif // THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_CREDENTIAL_STORAGE_IMPL_H_ diff --git a/internal/platform/credential_storage_test.cc b/internal/platform/credential_storage_impl_test.cc similarity index 94% rename from internal/platform/credential_storage_test.cc rename to internal/platform/credential_storage_impl_test.cc index 0fb8565f..c5531b90 100644 --- a/internal/platform/credential_storage_test.cc +++ b/internal/platform/credential_storage_impl_test.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "internal/platform/credential_storage.h" +#include "internal/platform/credential_storage_impl.h" #include #include @@ -29,7 +29,7 @@ namespace nearby { using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; -TEST(CredentialStorageTest, CanSaveAndGetPrivateCredentials) { +TEST(CredentialStorageImplTest, CanSaveAndGetPrivateCredentials) { // Define mock parameters std::string manager_app_id{"0192"}; std::string account_name{"test_account"}; @@ -79,9 +79,9 @@ TEST(CredentialStorageTest, CanSaveAndGetPrivateCredentials) { credentials_fetched_lambda; get_private_creds_callback.get_credentials_failed_cb = get_credentials_failed_lambda; - // Create CredentialStorage object to test SaveCredentials & + // Create CredentialStorageImpl object to test SaveCredentials & // GetPrivateCredentials - CredentialStorage creds_storage; + CredentialStorageImpl creds_storage; creds_storage.GetPrivateCredentials(credential_selector, get_private_creds_callback); EXPECT_FALSE(get_private_cred_succeeded); @@ -94,7 +94,7 @@ TEST(CredentialStorageTest, CanSaveAndGetPrivateCredentials) { EXPECT_TRUE(get_private_cred_succeeded); } -TEST(CredentialStorageTest, CanSaveAndGetPublicCredentials) { +TEST(CredentialStorageImplTest, CanSaveAndGetPublicCredentials) { // Define mock parameters std::string manager_app_id{"0192"}; std::string account_name{"test_account"}; @@ -143,9 +143,9 @@ TEST(CredentialStorageTest, CanSaveAndGetPublicCredentials) { get_public_creds_callback.credentials_fetched_cb = credentials_fetched_lambda; get_public_creds_callback.get_credentials_failed_cb = get_credentials_failed_lambda; - // Create CredentialStorage object to test SaveCredentials & + // Create CredentialStorageImpl object to test SaveCredentials & // GetPublicCredentials - CredentialStorage creds_storage; + CredentialStorageImpl creds_storage; creds_storage.GetPublicCredentials( credential_selector, public_credential_type, get_public_creds_callback); EXPECT_FALSE(get_public_cred_succeeded); diff --git a/presence/implementation/BUILD b/presence/implementation/BUILD index dda94bc6..2b6b9ee3 100644 --- a/presence/implementation/BUILD +++ b/presence/implementation/BUILD @@ -149,6 +149,8 @@ cc_test( deps = [ ":internal", "//net/proto2/contrib/parse_proto:testing", + "//internal/platform:comm", + "//internal/platform/implementation:comm", "//internal/platform/implementation:types", "//internal/platform/implementation/g3", # build_cleaner: keep "//internal/proto:credential_cc_proto", diff --git a/presence/implementation/credential_manager_impl.cc b/presence/implementation/credential_manager_impl.cc index b9c9379a..636caef9 100644 --- a/presence/implementation/credential_manager_impl.cc +++ b/presence/implementation/credential_manager_impl.cc @@ -32,6 +32,9 @@ namespace presence { namespace { using ::location::nearby::Base64Utils; using ::location::nearby::Crypto; +using ::location::nearby::api::CredentialOperationStatus; +using ::location::nearby::api::PublicCredentialType; +using ::location::nearby::api::SaveCredentialsResultCallback; using ::nearby::internal::DeviceMetadata; using ::nearby::internal::IdentityType; using ::nearby::internal::PrivateCredential; @@ -67,9 +70,26 @@ void CredentialManagerImpl::GenerateCredentials( end_time_millis += gap_millis; } } - // TODO(b/241488275) Store all the public and private credentials and call the - // callback to inform client. - credentials_generated_cb.credentials_generated_cb(public_credentials); + + auto save_creds_lambda = [&public_credentials, &credentials_generated_cb]( + CredentialOperationStatus status) { + if (status == CredentialOperationStatus::kSucceeded) { + credentials_generated_cb.credentials_generated_cb(public_credentials); + } else { + NEARBY_LOGS(ERROR) << "Fails to save generated credentials"; + credentials_generated_cb.credentials_generated_cb( + std::vector()); + } + }; + + SaveCredentialsResultCallback save_creds_cb; + save_creds_cb.credentials_saved_cb = save_creds_lambda; + + // Create credential_storage object and invoke SaveCredentials. + credential_storage_ptr_->SaveCredentials( + manager_app_id, device_metadata.account_name(), private_credentials, + public_credentials, PublicCredentialType::kLocalPublicCredential, + save_creds_cb); } std::pair, std::unique_ptr> @@ -90,15 +110,10 @@ CredentialManagerImpl::CreatePrivateCredential(DeviceMetadata device_metadata, // Uses SHA-256 algorithm to generate the credential ID from the authenticity // key auto secret_id = Crypto::Sha256(secret_key); - if (secret_id.Empty()) { - NEARBY_LOG(ERROR, - "Failed to create private credential because it failed to " - "create a secret id."); - return std::pair, - std::unique_ptr>( - std::unique_ptr(nullptr), - std::unique_ptr(nullptr)); - } + // Does not expect to fail here since Crypto::Sha256 should not return empty + // ByteArray. + CHECK(!secret_id.Empty()) << "Crypto::Sha256 failed!"; + private_credential_ptr->set_secret_id(secret_id.AsStringView()); std::string alias = Base64Utils::Encode(secret_id); @@ -158,7 +173,7 @@ std::unique_ptr CredentialManagerImpl::CreatePublicCredential( private_credential->device_metadata().SerializeAsString()); if (encrypted_meta_data.empty()) { - NEARBY_LOG(ERROR, "Fails to encrypt the device metadata."); + NEARBY_LOGS(ERROR) << "Fails to encrypt the device metadata."; return std::unique_ptr(nullptr); } diff --git a/presence/implementation/credential_manager_impl.h b/presence/implementation/credential_manager_impl.h index 92761382..c63aae9c 100644 --- a/presence/implementation/credential_manager_impl.h +++ b/presence/implementation/credential_manager_impl.h @@ -23,7 +23,7 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "internal/platform/credential_storage.h" +#include "internal/platform/credential_storage_impl.h" #include "internal/proto/credential.pb.h" #include "presence/implementation/credential_manager.h" @@ -32,7 +32,16 @@ namespace presence { class CredentialManagerImpl : public CredentialManager { public: - CredentialManagerImpl() = default; + CredentialManagerImpl() { + credential_storage_ptr_ = + std::make_unique(); + } + + // Test purpose only. + explicit CredentialManagerImpl( + std::unique_ptr + credential_storage_ptr) + : credential_storage_ptr_(std::move(credential_storage_ptr)) {} // AES only supports key sizes of 16, 24 or 32 bytes. static constexpr int kAuthenticityKeyByteSize = 16; @@ -89,9 +98,6 @@ class CredentialManagerImpl : public CredentialManager { return absl::UnimplementedError("EncryptDataElements unimplemented"); } - private: - FRIEND_TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully); - std::pair, std::unique_ptr> CreatePrivateCredential(nearby::internal::DeviceMetadata device_metadata, @@ -109,6 +115,10 @@ class CredentialManagerImpl : public CredentialManager { // Extend the key from 16 bytes to 32 bytes. std::vector ExtendMetadataEncryptionKey( std::string device_metadata_encryption_key); + + private: + std::unique_ptr + credential_storage_ptr_; }; } // namespace presence diff --git a/presence/implementation/credential_manager_impl_test.cc b/presence/implementation/credential_manager_impl_test.cc index d522003e..43ebd359 100644 --- a/presence/implementation/credential_manager_impl_test.cc +++ b/presence/implementation/credential_manager_impl_test.cc @@ -14,20 +14,25 @@ #include "presence/implementation/credential_manager_impl.h" -#include +#include +#include +#include #include "net/proto2/contrib/parse_proto/testing.h" #include "gmock/gmock.h" #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "internal/platform/credential_storage_impl.h" +#include "internal/platform/implementation/credential_storage.h" #include "internal/platform/implementation/crypto.h" #include "internal/proto/credential.pb.h" -#include "presence/implementation/encryption.h" namespace nearby { namespace presence { +namespace { using ::location::nearby::Crypto; - +using ::location::nearby::api::PublicCredentialType; +using ::location::nearby::api::SaveCredentialsResultCallback; using ::nearby::internal::DeviceMetadata; using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; @@ -35,17 +40,44 @@ using ::nearby::internal::IdentityType::IDENTITY_TYPE_PRIVATE; using ::proto2::contrib::parse_proto::ParseTestProto; using ::protobuf_matchers::EqualsProto; +DeviceMetadata CreateTestDeviceMetadata() { + DeviceMetadata device_metadata; + device_metadata.set_stable_device_id("test_device_id"); + device_metadata.set_account_name("test_account"); + 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"); + device_metadata.set_device_type(internal::DeviceMetadata::PHONE); + return device_metadata; +} + +class CredentialManagerImplTest : public ::testing::Test { + public: + class MockCredentialStorage : public location::nearby::CredentialStorageImpl { + public: + MOCK_METHOD(void, SaveCredentials, + (absl::string_view manager_app_id, + absl::string_view account_name, + const std::vector<::nearby::internal::PrivateCredential>& + private_credentials, + const std::vector<::nearby::internal::PublicCredential>& + public_credentials, + PublicCredentialType public_credential_type, + SaveCredentialsResultCallback callback), + (override)); + }; + + CredentialManagerImplTest() { + mock_credential_storage_ptr_ = std::make_unique(); + } + + protected: + std::unique_ptr mock_credential_storage_ptr_; +}; + // TODO(b/241926454): Make sure CredentialManager builds with Github. TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { - DeviceMetadata device_metadata = ParseTestProto(R"pb( - stable_device_id: "test_device_id" - ; - account_name: "test_account"; - device_name: "NP test device"; - icon_url: "test_image.test.com" - bluetooth_mac_address: "FF:FF:FF:FF:FF:FF"; - device_type: PHONE; - )pb"); + DeviceMetadata device_metadata = CreateTestDeviceMetadata(); CredentialManagerImpl credential_manager; auto credentials = credential_manager.CreatePrivateCredential( @@ -53,7 +85,6 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { /* end_time_ms= */ 1000); PrivateCredential* private_credential = credentials.first.get(); - PublicCredential* public_credential = credentials.second.get(); // Verify the private credential. EXPECT_THAT(private_credential->device_metadata(), @@ -68,6 +99,7 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { EXPECT_EQ(private_credential->metadata_encryption_key().size(), CredentialManagerImpl::kAuthenticityKeyByteSize); + PublicCredential* public_credential = credentials.second.get(); // Verify the public credential. EXPECT_EQ(public_credential->identity_type(), IDENTITY_TYPE_PRIVATE); EXPECT_FALSE(public_credential->secret_id().empty()); @@ -91,5 +123,79 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { EXPECT_EQ(private_credential->device_metadata().SerializeAsString(), decrypted_device_metadata); } + +TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { + DeviceMetadata device_metadata = CreateTestDeviceMetadata(); + + CredentialManagerImpl credential_manager; + + GenerateCredentialsCallback credentials_generated_cb; + + std::vector publicCredentials; + + auto create_creds_callback_lambda = + [&publicCredentials]( + std::vector credentials) { + publicCredentials = credentials; + }; + + credentials_generated_cb.credentials_generated_cb = + create_creds_callback_lambda; + std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; + + credential_manager.GenerateCredentials( + device_metadata, + /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, + credentials_generated_cb); + + EXPECT_EQ(publicCredentials.size(), 2); + for (auto& public_credential : publicCredentials) { + EXPECT_EQ(public_credential.identity_type(), IDENTITY_TYPE_PRIVATE); + EXPECT_FALSE(public_credential.secret_id().empty()); + EXPECT_EQ(public_credential.end_time_millis() - + public_credential.start_time_millis(), + 1 * 24 * 3600 * 1000); + EXPECT_FALSE(public_credential.encrypted_metadata_bytes().empty()); + } +} + +TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { + DeviceMetadata device_metadata = CreateTestDeviceMetadata(); + + auto credential_storage_ptr = + std::make_unique(); + + EXPECT_CALL(*credential_storage_ptr, SaveCredentials) + .WillOnce(::testing::Invoke( + [](absl::string_view manager_app_id, absl::string_view account_name, + const std::vector& private_credentials, + const std::vector& public_credentials, + PublicCredentialType public_credential_type, + SaveCredentialsResultCallback callback) { + callback.credentials_saved_cb( + location::nearby::api::CredentialOperationStatus::kFailed); + })); + CredentialManagerImpl credential_manager(std::move(credential_storage_ptr)); + + GenerateCredentialsCallback credentials_generated_cb; + std::vector publicCredentials; + auto create_creds_callback_lambda = + [&publicCredentials]( + std::vector credentials) { + publicCredentials = credentials; + }; + + credentials_generated_cb.credentials_generated_cb = + create_creds_callback_lambda; + std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; + + credential_manager.GenerateCredentials( + device_metadata, + /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, + credentials_generated_cb); + EXPECT_TRUE(publicCredentials.empty()); +} +} // namespace + } // namespace presence } // namespace nearby