Skip to content

[Firestore] Fix crash fetching Auth and App Check tokens #15558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[Firestore] Fix crash fetching Auth and App Check tokens #15558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab40c25
[Firestore] Fix crash fetching Auth and App Check tokens
paulb777 Nov 29, 2025
a9193c5
build fix
paulb777 Nov 29, 2025
5bf8d12
update tests
paulb777 Nov 30, 2025
feb0af2
Restore deleted newlines
paulb777 Dec 1, 2025
272cefe
Remove inadvertent newline
paulb777 Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Firestore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Unreleased
- [fixed] Fix crash while fetching Auth and App Check tokens. (#15281)

# 12.4.0
- [fixed] Implemented an internal workaround to fix
[CVE-2025-0838](https://nvd.nist.gov/vuln/detail/CVE-2025-0838). (#15300)
Expand Down
3 changes: 2 additions & 1 deletion Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ namespace firestore {
namespace credentials {

class FirebaseAppCheckCredentialsProvider
: public CredentialsProvider<std::string, std::string> {
: public CredentialsProvider<std::string, std::string>,
public std::enable_shared_from_this<FirebaseAppCheckCredentialsProvider> {
public:
FirebaseAppCheckCredentialsProvider(FIRApp* app,
id<FIRAppCheckInterop> app_check);
Expand Down
13 changes: 8 additions & 5 deletions Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@

void FirebaseAppCheckCredentialsProvider::GetToken(
TokenListener<std::string> completion) {
std::weak_ptr<Contents> weak_contents = contents_;
if (contents_->app_check) {
// Capture self via a shared_ptr to ensure that the credentials provider
// is not deallocated while the getToken request is outstanding.
std::shared_ptr<FirebaseAppCheckCredentialsProvider> self =
shared_from_this();
if (self->contents_->app_check) {
void (^get_token_callback)(id<FIRAppCheckTokenResultInterop>) =
^(id<FIRAppCheckTokenResultInterop> result) {
if (result.error != nil) {
Expand All @@ -90,13 +93,13 @@

// Retrieve a cached or generate a new FAC Token. If forcingRefresh == YES
// always generates a new token and updates the cache.
[contents_->app_check getTokenForcingRefresh:force_refresh_
completion:get_token_callback];
[self->contents_->app_check getTokenForcingRefresh:self->force_refresh_
completion:get_token_callback];
} else {
// If there's no AppCheck provider, call back immediately with a nil token.
completion(std::string{""});
}
force_refresh_ = false;
self->force_refresh_ = false;
}

void FirebaseAppCheckCredentialsProvider::SetCredentialChangeListener(
Expand Down
3 changes: 2 additions & 1 deletion Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ namespace credentials {
* For non-Apple desktop build, this is right now just a stub.
*/
class FirebaseAuthCredentialsProvider
: public CredentialsProvider<AuthToken, User> {
: public CredentialsProvider<AuthToken, User>,
public std::enable_shared_from_this<FirebaseAuthCredentialsProvider> {
public:
// TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which
// deals all platforms. Right now, only works for FIRApp*.
Expand Down
19 changes: 8 additions & 11 deletions Firestore/core/src/credentials/firebase_auth_credentials_provider_apple.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,26 +81,23 @@
// fail if there is a token change while the request is outstanding.
int initial_token_counter = contents_->token_counter;

std::weak_ptr<Contents> weak_contents = contents_;
// Capture self via a shared_ptr to ensure that the credentials provider
// is not deallocated while the getToken request is outstanding.
std::shared_ptr<FirebaseAuthCredentialsProvider> self = shared_from_this();
void (^get_token_callback)(NSString*, NSError*) =
^(NSString* _Nullable token, NSError* _Nullable error) {
std::shared_ptr<Contents> contents = weak_contents.lock();
if (!contents) {
return;
}

std::unique_lock<std::mutex> lock(contents->mutex);
if (initial_token_counter != contents->token_counter) {
std::unique_lock<std::mutex> lock(self->contents_->mutex);
if (initial_token_counter != self->contents_->token_counter) {
// Cancel the request since the user changed while the request was
// outstanding so the response is likely for a previous user (which
// user, we can't be sure).
LOG_DEBUG("GetToken aborted due to token change.");
return GetToken(completion);
return self->GetToken(completion);
} else {
if (error == nil) {
if (token != nil) {
completion(
AuthToken{util::MakeString(token), contents->current_user});
completion(AuthToken{util::MakeString(token),
self->contents_->current_user});
} else {
completion(AuthToken::Unauthenticated());
}
Expand Down
39 changes: 23 additions & 16 deletions Firestore/core/test/unit/credentials/firebase_app_check_credentials_provider_test.mm
View file
Open in desktop
Copy link
Member

@ncooke3 ncooke3 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why the tests here are changing how the provider is created.

Do the added class members need to be public?

// Firestore/core/src/credentials/firebase_app_check_credentials_provider_apple.h
      public std::enable_shared_from_this<FirebaseAppCheckCredentialsProvider> {

Copy link
Member Author

@paulb777 paulb777 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's Gemini's explanation for why the tests needed changes:

When the production code is built, it uses std::make_shared. But when the unit tests are built,
they are likely creating the FirebaseAuthCredentialsProvider object directly on the stack, like this:

1 // From your search results in firebase_auth_credentials_provider_test.mm
2 FirebaseAuthCredentialsProvider credentials_provider(app, auth);

When credentials_provider.GetToken(...) is called in the test, it's being called on a stack-allocated object. The object isn't managed
by a shared_ptr, so the shared_from_this() call inside GetToken fails.

I need to modify the unit test file (firebase-ios-sdk/Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm)
to create the provider using std::make_shared, mirroring the production code. This will resolve the build error and ensure the tests are
more accurate.

Copy link
Member

@ncooke3 ncooke3 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that explanation sounds reasonable.

Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ - (nonnull NSString*)tokenDidChangeNotificationName {
auto token_promise = std::make_shared<std::promise<std::string>>();

FIRApp* app = testutil::AppForUnitTesting();
FirebaseAppCheckCredentialsProvider credentials_provider(app, nil);
credentials_provider.GetToken(
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, nil);
credentials_provider->GetToken(
[token_promise](util::StatusOr<std::string> result) {
EXPECT_TRUE(result.ok());
const std::string& token = result.ValueOrDie();
Expand All @@ -115,8 +116,9 @@ - (nonnull NSString*)tokenDidChangeNotificationName {
FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check =
[[FSTAppCheckFake alloc] initWithToken:@"fake token"];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
credentials_provider.GetToken([](util::StatusOr<std::string> result) {
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);
credentials_provider->GetToken([](util::StatusOr<std::string> result) {
EXPECT_TRUE(result.ok());
const std::string& token = result.ValueOrDie();
EXPECT_EQ("fake token", token);
Expand All @@ -127,20 +129,22 @@ - (nonnull NSString*)tokenDidChangeNotificationName {
FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check =
[[FSTAppCheckFake alloc] initWithToken:@"fake token"];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
credentials_provider.SetCredentialChangeListener(
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);
credentials_provider->SetCredentialChangeListener(
[](std::string token) { EXPECT_EQ("", token); });

credentials_provider.SetCredentialChangeListener(nullptr);
credentials_provider->SetCredentialChangeListener(nullptr);
}

TEST(FirebaseAppCheckCredentialsProviderTest, InvalidateToken) {
FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check =
[[FSTAppCheckFake alloc] initWithToken:@"fake token"];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
credentials_provider.InvalidateToken();
credentials_provider.GetToken(
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);
credentials_provider->InvalidateToken();
credentials_provider->GetToken(
[&app_check](util::StatusOr<std::string> result) {
EXPECT_TRUE(result.ok());
EXPECT_TRUE(app_check.forceRefreshTriggered);
Expand All @@ -154,9 +158,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName {

FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);

credentials_provider.SetCredentialChangeListener(
credentials_provider->SetCredentialChangeListener(
[token_promise](const std::string& result) {
if (result != "") {
token_promise->set_value(result);
Expand Down Expand Up @@ -186,9 +191,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName {

FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);

credentials_provider.SetCredentialChangeListener(
credentials_provider->SetCredentialChangeListener(
[token_promise](const std::string& result) {
if (result != "") {
token_promise->set_value(result);
Expand Down Expand Up @@ -227,9 +233,10 @@ - (nonnull NSString*)tokenDidChangeNotificationName {

FIRApp* app = testutil::AppForUnitTesting();
FSTAppCheckFake* app_check = [[FSTAppCheckFake alloc] initWithToken:@""];
FirebaseAppCheckCredentialsProvider credentials_provider(app, app_check);
auto credentials_provider =
std::make_shared<FirebaseAppCheckCredentialsProvider>(app, app_check);

credentials_provider.SetCredentialChangeListener(
credentials_provider->SetCredentialChangeListener(
[token_promise](const std::string& result) {
if (result != "") {
token_promise->set_value(result);
Expand Down
40 changes: 23 additions & 17 deletions Firestore/core/test/unit/credentials/firebase_auth_credentials_provider_test.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
auto token_promise = std::make_shared<std::promise<AuthToken>>();

FIRApp* app = testutil::AppForUnitTesting();
FirebaseAuthCredentialsProvider credentials_provider(app, nil);
credentials_provider.GetToken(
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, nil);
credentials_provider->GetToken(
[token_promise](util::StatusOr<AuthToken> result) {
EXPECT_TRUE(result.ok());
const AuthToken& token = result.ValueOrDie();
Expand All @@ -99,8 +100,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
TEST(FirebaseAuthCredentialsProviderTest, GetTokenUnauthenticated) {
FIRApp* app = testutil::AppForUnitTesting();
FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:nil uid:nil];
FirebaseAuthCredentialsProvider credentials_provider(app, auth);
credentials_provider.GetToken([](util::StatusOr<AuthToken> result) {
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, auth);
credentials_provider->GetToken([](util::StatusOr<AuthToken> result) {
EXPECT_TRUE(result.ok());
const AuthToken& token = result.ValueOrDie();
EXPECT_ANY_THROW(token.token());
Expand All @@ -114,8 +116,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
FIRApp* app = testutil::AppForUnitTesting();
FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid"
uid:@"fake uid"];
FirebaseAuthCredentialsProvider credentials_provider(app, auth);
credentials_provider.GetToken([](util::StatusOr<AuthToken> result) {
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, auth);
credentials_provider->GetToken([](util::StatusOr<AuthToken> result) {
EXPECT_TRUE(result.ok());
const AuthToken& token = result.ValueOrDie();
EXPECT_EQ("token for fake uid", token.token());
Expand All @@ -129,22 +132,24 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
FIRApp* app = testutil::AppForUnitTesting();
FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"default token"
uid:@"fake uid"];
FirebaseAuthCredentialsProvider credentials_provider(app, auth);
credentials_provider.SetCredentialChangeListener([](User user) {
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, auth);
credentials_provider->SetCredentialChangeListener([](User user) {
EXPECT_EQ("fake uid", user.uid());
EXPECT_TRUE(user.is_authenticated());
});

credentials_provider.SetCredentialChangeListener(nullptr);
credentials_provider->SetCredentialChangeListener(nullptr);
}

TEST(FirebaseAuthCredentialsProviderTest, InvalidateToken) {
FIRApp* app = testutil::AppForUnitTesting();
FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid"
uid:@"fake uid"];
FirebaseAuthCredentialsProvider credentials_provider(app, auth);
credentials_provider.InvalidateToken();
credentials_provider.GetToken([&auth](util::StatusOr<AuthToken> result) {
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, auth);
credentials_provider->InvalidateToken();
credentials_provider->GetToken([&auth](util::StatusOr<AuthToken> result) {
EXPECT_TRUE(result.ok());
EXPECT_TRUE(auth.forceRefreshTriggered);
const AuthToken& token = result.ValueOrDie();
Expand All @@ -160,14 +165,15 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
FIRApp* app = testutil::AppForUnitTesting();
FSTAuthFake* auth = [[FSTAuthFake alloc] initWithToken:@"token for fake uid"
uid:@"fake uid"];
FirebaseAuthCredentialsProvider credentials_provider(app, auth);
auto credentials_provider =
std::make_shared<FirebaseAuthCredentialsProvider>(app, auth);

std::thread thread1([&credentials_provider] {
credentials_provider.GetToken(
std::thread thread1([credentials_provider] {
credentials_provider->GetToken(
[](util::StatusOr<AuthToken> result) { EXPECT_TRUE(result.ok()); });
});
std::thread thread2([&credentials_provider] {
credentials_provider.GetToken(
std::thread thread2([credentials_provider] {
credentials_provider->GetToken(
[](util::StatusOr<AuthToken> result) { EXPECT_TRUE(result.ok()); });
});

Expand Down
Loading