From 41eb0e2a8e4328a27cd430808a97a33be4117561 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 18:56:42 +0000 Subject: [PATCH 1/3] Implement UseUserAccessGroup for Firebase Auth C++ SDK This commit adds the UseUserAccessGroup method to the Firebase Auth C++ SDK. - On iOS, this method calls the native [FIRAuth useUserAccessGroup:error:] method to specify a user access group for iCloud keychain access. - On other platforms (desktop, Android), this method is a no-op and returns kAuthErrorNone as it's an iOS-only feature. Public API has been added to firebase::auth::Auth in auth.h, with implementations in auth_ios.mm (iOS) and auth_desktop.cc (stub for other platforms). --- auth/src/desktop/auth_desktop.cc | 5 +++++ auth/src/include/firebase/auth.h | 15 +++++++++++++++ auth/src/ios/auth_ios.mm | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/auth/src/desktop/auth_desktop.cc b/auth/src/desktop/auth_desktop.cc index dc9aca2950..e934013c64 100644 --- a/auth/src/desktop/auth_desktop.cc +++ b/auth/src/desktop/auth_desktop.cc @@ -575,6 +575,11 @@ void Auth::UseEmulator(std::string host, uint32_t port) { auth_impl->assigned_emulator_url.append(std::to_string(port)); } +AuthError Auth::UseUserAccessGroup(const char* access_group) { + // This is an iOS-only feature. No-op on other platforms. + return kAuthErrorNone; +} + void InitializeTokenRefresher(AuthData* auth_data) { auto auth_impl = static_cast(auth_data->auth_impl); auth_impl->token_refresh_thread.Initialize(auth_data); diff --git a/auth/src/include/firebase/auth.h b/auth/src/include/firebase/auth.h index f6809c4a57..7ad013e4c7 100644 --- a/auth/src/include/firebase/auth.h +++ b/auth/src/include/firebase/auth.h @@ -517,6 +517,21 @@ class Auth { /// not available on the current device. static Auth* GetAuth(App* app, InitResult* init_result_out = nullptr); + /// @brief Specifies a user access group for iCloud keychain access. + /// + /// This method is only functional on iOS. On other platforms, it is a no-op + /// and will always return `kAuthErrorNone`. + /// + /// If you are using iCloud keychain synchronization, you will need to call + /// this method to set the user access group. + /// + /// @param[in] access_group The user access group to use. Set to `nullptr` or + /// an empty string to use the default access group. + /// + /// @return `kAuthErrorNone` on success, or an AuthError code if an error + /// occurred. + AuthError UseUserAccessGroup(const char* access_group); + private: /// @cond FIREBASE_APP_INTERNAL friend class ::firebase::App; diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index a0292ba3b8..f449735670 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -608,5 +608,23 @@ void DisableTokenAutoRefresh(AuthData *auth_data) {} void InitializeTokenRefresher(AuthData *auth_data) {} void DestroyTokenRefresher(AuthData *auth_data) {} +AuthError Auth::UseUserAccessGroup(const char* access_group_str) { + if (!auth_data_) { + return kAuthErrorUninitialized; + } + NSString* access_group_ns_str = nil; + if (access_group_str != nullptr && strlen(access_group_str) > 0) { + access_group_ns_str = [NSString stringWithUTF8String:access_group_str]; + } + + NSError* error = nil; + BOOL success = [AuthImpl(auth_data_) useUserAccessGroup:access_group_ns_str error:&error]; + if (success) { + return kAuthErrorNone; + } else { + return AuthErrorFromNSError(error); + } +} + } // namespace auth } // namespace firebase From c8f87a37bced752f5616d59c62fbb6680f4b482b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 19:28:58 +0000 Subject: [PATCH 2/3] Address review comments for UseUserAccessGroup implementation - Added Android stub for UseUserAccessGroup. - Updated Doxygen comments in auth.h for clarity and accuracy regarding empty string handling. - Modified iOS implementation to pass empty strings verbatim to the native SDK. - Corrected error code used in iOS implementation when auth_data_ is null (kAuthErrorFailure instead of kAuthErrorUninitialized). - Added a basic smoke test to integration_test.cc for iOS to ensure UseUserAccessGroup can be called. --- auth/integration_test/src/integration_test.cc | 14 ++++++++++++++ auth/src/android/auth_android.cc | 5 +++++ auth/src/include/firebase/auth.h | 8 +++----- auth/src/ios/auth_ios.mm | 4 ++-- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index eef1301adc..3b9cce85b2 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -1513,4 +1513,18 @@ TEST_F(FirebaseAuthTest, TestLinkFederatedProviderBadProviderIdFails) { #endif // defined(ENABLE_OAUTH_TESTS) +#if TARGET_OS_IPHONE +TEST_F(FirebaseAuthTest, TestUseUserAccessGroup) { + // This is a simple smoke test to ensure the method can be called + // without crashing on iOS and returns the expected default success. + // Deeper testing of keychain access group functionality would require + // more complex setup and is typically done manually or with UI tests. + EXPECT_EQ(auth_->UseUserAccessGroup(nullptr), + firebase::auth::kAuthErrorNone); + EXPECT_EQ(auth_->UseUserAccessGroup("test-group"), + firebase::auth::kAuthErrorNone); + EXPECT_EQ(auth_->UseUserAccessGroup(""), firebase::auth::kAuthErrorNone); +} +#endif // TARGET_OS_IPHONE + } // namespace firebase_testapp_automated diff --git a/auth/src/android/auth_android.cc b/auth/src/android/auth_android.cc index e0a9a669cb..335a9e5173 100644 --- a/auth/src/android/auth_android.cc +++ b/auth/src/android/auth_android.cc @@ -676,5 +676,10 @@ void DisableTokenAutoRefresh(AuthData* auth_data) {} void InitializeTokenRefresher(AuthData* auth_data) {} void DestroyTokenRefresher(AuthData* auth_data) {} +AuthError Auth::UseUserAccessGroup(const char* access_group) { + // This is an iOS-only feature. No-op on other platforms. + return kAuthErrorNone; +} + } // namespace auth } // namespace firebase diff --git a/auth/src/include/firebase/auth.h b/auth/src/include/firebase/auth.h index 7ad013e4c7..93f5b023d4 100644 --- a/auth/src/include/firebase/auth.h +++ b/auth/src/include/firebase/auth.h @@ -522,11 +522,9 @@ class Auth { /// This method is only functional on iOS. On other platforms, it is a no-op /// and will always return `kAuthErrorNone`. /// - /// If you are using iCloud keychain synchronization, you will need to call - /// this method to set the user access group. - /// - /// @param[in] access_group The user access group to use. Set to `nullptr` or - /// an empty string to use the default access group. + /// @param[in] access_group The user access group to use. Set to `nullptr` + /// to use the default access group. An empty string will be passed as an + /// empty string. /// /// @return `kAuthErrorNone` on success, or an AuthError code if an error /// occurred. diff --git a/auth/src/ios/auth_ios.mm b/auth/src/ios/auth_ios.mm index f449735670..785b8e4c45 100644 --- a/auth/src/ios/auth_ios.mm +++ b/auth/src/ios/auth_ios.mm @@ -610,10 +610,10 @@ void DestroyTokenRefresher(AuthData *auth_data) {} AuthError Auth::UseUserAccessGroup(const char* access_group_str) { if (!auth_data_) { - return kAuthErrorUninitialized; + return kAuthErrorFailure; } NSString* access_group_ns_str = nil; - if (access_group_str != nullptr && strlen(access_group_str) > 0) { + if (access_group_str != nullptr) { access_group_ns_str = [NSString stringWithUTF8String:access_group_str]; } From c380f0257829006ea1d87ebbb9ae8915cc38f578 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 27 Jun 2025 19:47:19 +0000 Subject: [PATCH 3/3] Adjust UseUserAccessGroup integration test per review The test now calls UseUserAccessGroup without checking its return value. This is to prevent failures in environments where keychain sharing might not be configured, as the primary goal of this test is to ensure the method call itself doesn't crash on iOS. --- auth/integration_test/src/integration_test.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/auth/integration_test/src/integration_test.cc b/auth/integration_test/src/integration_test.cc index 3b9cce85b2..6bd8d47a43 100644 --- a/auth/integration_test/src/integration_test.cc +++ b/auth/integration_test/src/integration_test.cc @@ -1516,14 +1516,14 @@ TEST_F(FirebaseAuthTest, TestLinkFederatedProviderBadProviderIdFails) { #if TARGET_OS_IPHONE TEST_F(FirebaseAuthTest, TestUseUserAccessGroup) { // This is a simple smoke test to ensure the method can be called - // without crashing on iOS and returns the expected default success. + // without crashing on iOS. // Deeper testing of keychain access group functionality would require // more complex setup and is typically done manually or with UI tests. - EXPECT_EQ(auth_->UseUserAccessGroup(nullptr), - firebase::auth::kAuthErrorNone); - EXPECT_EQ(auth_->UseUserAccessGroup("test-group"), - firebase::auth::kAuthErrorNone); - EXPECT_EQ(auth_->UseUserAccessGroup(""), firebase::auth::kAuthErrorNone); + // We don't check the return value as keychain sharing may not be configured, + // leading to legitimate errors. + auth_->UseUserAccessGroup(nullptr); + auth_->UseUserAccessGroup("test-group"); + auth_->UseUserAccessGroup(""); } #endif // TARGET_OS_IPHONE