Skip to content

[Auth] Fix handling of AppCheck placeholder tokens #15555

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

Closed
wants to merge 4 commits into from
+64 −10
Closed

[Auth] Fix handling of AppCheck placeholder tokens #15555

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d61ab3
[Auth] Fix handling of AppCheck placeholder tokens
paulb777 Nov 27, 2025
250ad96
fix flake in new test
paulb777 Nov 28, 2025
0d983df
fix flake
paulb777 Nov 28, 2025
6627247
Merge branch 'main' into pb-auth-appcheck-bug
paulb777 Dec 1, 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
1 change: 1 addition & 0 deletions FirebaseAuth/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased
- [fixed] Add a mechanism to prevent concurrent token refreshes. (#15474)
- [fixed] Fix handling of AppCheck placeholder tokens. (#15372)

# 12.2.0
- [added] Added TOTP support for macOS.
Expand Down
4 changes: 2 additions & 2 deletions FirebaseAuth/Sources/Swift/AuthProvider/OAuthProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,8 @@ import Foundation
let tokenResult = await appCheck.getToken(forcingRefresh: false)
if let error = tokenResult.error {
AuthLog.logWarning(code: "I-AUT000018",
message: "Error getting App Check token; using placeholder " +
"token instead. Error: \(error)")
message: "Error getting App Check token. Error: \(error)")
throw error
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.

Reviewers: Do you agree with Gemini's answer to this question?

Is this fix ok for apps that include App Check, but aren't enforcing it?

✦ That's an excellent question, and it highlights a key nuance of this fix.

No, this fix is not ideal for apps that include App Check but do not have enforcement turned on.

Here's the breakdown of the behavior change:

  • Previous Behavior: If getting the App Check token failed, the SDK would log a warning, proceed with a "dummy" token, and send the request to the Auth backend. If you were
    not enforcing App Check, the backend would ignore the invalid token and the authentication would succeed.
  • New Behavior (With This Fix): If getting the App Check token fails, the SDK now immediately throws an error, and the authentication flow stops on the client. The request
    is never sent to the Auth backend.

Impact on Your Scenario:

For the ~1% of users experiencing a token generation failure on the first launch, their sign-up will now fail immediately, even with enforcement turned off. Previously, it
would have succeeded.

Why This Fix is Still Correct (From an SDK Perspective):

The new behavior is more explicit and predictable. Silently sending invalid "dummy" tokens is a bug because it masks an underlying problem—the token generation failure. The
"fail-fast" approach is generally safer because it makes the SDK's behavior consistent, regardless of your backend settings.

Recommended Solution for Your Use Case:

The best practice to handle these transient, first-launch errors is to implement a retry mechanism in your application code.

When you call auth.createUser(...) or provider.verifyPhoneNumber(...), your error handling logic should catch the App Check-related error and prompt the user to try again. A
simple "An error occurred, please try again" message with a retry button is often sufficient. The token generation process is highly likely to have succeeded by the time the
user initiates the second attempt.

This approach provides a much better user experience and makes your app resilient to these initial, temporary failures.

Copy link
Contributor

@andrewheard andrewheard Dec 1, 2025

Choose a reason for hiding this comment

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

This definitely concerns me because it seems to defeat the purpose of the difference between the public FirebaseAppCheck API and the FirebaseAppCheckInterop APIs. Let's run this by @weixifan before merging.

}
let appCheckTokenFragment = "fac=\(tokenResult.token)"
components?.fragment = appCheckTokenFragment
Expand Down
4 changes: 2 additions & 2 deletions FirebaseAuth/Sources/Swift/AuthProvider/PhoneAuthProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,8 @@ import Foundation
let tokenResult = await appCheck.getToken(forcingRefresh: false)
if let error = tokenResult.error {
AuthLog.logWarning(code: "I-AUT000018",
message: "Error getting App Check token; using placeholder " +
"token instead. Error: \(error)")
message: "Error getting App Check token. Error: \(error)")
throw error
}
let appCheckTokenFragment = "fac=\(tokenResult.token)"
components?.fragment = appCheckTokenFragment
Expand Down
6 changes: 4 additions & 2 deletions FirebaseAuth/Tests/Unit/Fakes/FakeAppCheck.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import Foundation

class FakeAppCheck: NSObject, AppCheckInterop {
let fakeAppCheckToken = "fakeAppCheckToken"
var error: Error?

func getToken(forcingRefresh: Bool, completion: @escaping AppCheckTokenHandlerInterop) {
completion(FakeAppCheckResult(token: "fakeAppCheckToken"))
completion(FakeAppCheckResult(token: "fakeAppCheckToken", error: error))
}

func tokenDidChangeNotificationName() -> String {
Expand All @@ -40,7 +41,8 @@ class FakeAppCheckResult: NSObject, FIRAppCheckTokenResultInterop {
var token: String
var error: Error?

init(token: String) {
init(token: String, error: Error? = nil) {
self.token = token
self.error = error
}
}
32 changes: 32 additions & 0 deletions FirebaseAuth/Tests/Unit/OAuthProviderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,38 @@ import FirebaseCore
OAuthProviderTests.testAppCheck = false
}

func testGetCredentialWithUIDelegateWithAppCheckError() throws {
let expectation = self.expectation(description: "App Check error propagated")
let fakeAppCheck = FakeAppCheck()
let expectedError = NSError(domain: "appCheckError", code: -1)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 28, 2025

Choose a reason for hiding this comment

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

low

For improved maintainability and to avoid magic strings, consider defining "appCheckError" and -1 as private constants at the top of the OAuthProviderTests class. This would align with the existing practice for other test constants in this file.

fakeAppCheck.error = expectedError
initApp(#function, useAppID: true)
let auth = try XCTUnwrap(OAuthProviderTests.auth)
auth.requestConfiguration.appCheck = fakeAppCheck
let provider = OAuthProvider(providerID: kFakeProviderID, auth: auth)

let projectConfigExpectation = self.expectation(description: "projectConfiguration")
rpcIssuer.projectConfigRequester = { request in
projectConfigExpectation.fulfill()
do {
return try self.rpcIssuer.respond(withJSON: ["authorizedDomains": [
Self.kFakeAuthorizedWebDomain,
Self.kFakeAuthorizedDomain,
]])
} catch {
XCTFail("Failure sending response: \(error)")
return (nil, nil)
}
}

provider.getCredentialWith(nil) { credential, error in
XCTAssertNil(credential)
XCTAssertEqual(error as? NSError, expectedError)
expectation.fulfill()
}
waitForExpectations(timeout: 2.0)
}

/** @fn testOAuthCredentialCoding
@brief Tests successful archiving and unarchiving of @c GoogleAuthCredential.
*/
Expand Down
29 changes: 24 additions & 5 deletions FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,10 +762,12 @@
bothClientAndAppID: Bool = false,
reCAPTCHAfallback: Bool = false,
forwardingNotification: Bool = true,
presenterError: Error? = nil) async throws {
presenterError: Error? = nil,
fakeAppCheck: FakeAppCheck? = nil) async throws {
initApp(function, useClientID: useClientID, bothClientAndAppID: bothClientAndAppID,
testMode: testMode,
forwardingNotification: forwardingNotification)
forwardingNotification: forwardingNotification,
fakeAppCheck: fakeAppCheck)
let auth = try XCTUnwrap(PhoneAuthProviderTests.auth)
let provider = PhoneAuthProvider.provider(auth: auth)
var expectations: [XCTestExpectation] = []
Expand Down Expand Up @@ -807,7 +809,7 @@
presenterError: presenterError
)
}
if errorURLString == nil, presenterError == nil {
if errorURLString == nil, presenterError == nil, fakeAppCheck?.error == nil {
let requestExpectation = expectation(description: "verifyRequester")
expectations.append(requestExpectation)
rpcIssuer.verifyRequester = { request in
Expand All @@ -821,7 +823,7 @@
XCTAssertTrue(reCAPTCHAfallback)
XCTAssertEqual(token, self.kFakeReCAPTCHAToken)
case .empty:
XCTAssertTrue(testMode)
XCTFail("Should not be empty")
}
requestExpectation.fulfill()
do {
Expand Down Expand Up @@ -861,7 +863,8 @@
bothClientAndAppID: Bool = false,
testMode: Bool = false,
forwardingNotification: Bool = true,
fakeToken: Bool = false) {
fakeToken: Bool = false,
fakeAppCheck: FakeAppCheck? = nil) {
let options = FirebaseOptions(googleAppID: "0:0000000000000:ios:0000000000000000",
gcmSenderID: "00000000000000000-00000000000-000000000")
options.apiKey = PhoneAuthProviderTests.kFakeAPIKey
Expand All @@ -884,6 +887,9 @@
kAuthGlobalWorkQueue.sync {
// Wait for Auth protectedDataInitialization to finish.
PhoneAuthProviderTests.auth = auth
if let fakeAppCheck = fakeAppCheck {
auth.requestConfiguration.appCheck = fakeAppCheck
}
if testMode {
// Disable app verification.
let settings = AuthSettings()
Expand Down Expand Up @@ -1095,5 +1101,18 @@
"://firebaseauth/" +
"link?deep_link_id=https%3A%2F%2Fexample.firebaseapp.com%2F__%2Fauth%2Fcallback%3FauthType%" +
"3DverifyApp%26recaptchaToken%3DfakeReCAPTCHAToken"

func testVerifyPhoneNumberWithAppCheckError() async throws {
let fakeAppCheck = FakeAppCheck()
let expectedError = NSError(domain: "appCheckError", code: -1)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 28, 2025

Choose a reason for hiding this comment

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

low

For improved maintainability and to avoid magic strings, consider defining "appCheckError" and -1 as private constants at the top of the PhoneAuthProviderTests class. This makes the test code cleaner and easier to manage if these values are reused.

fakeAppCheck.error = expectedError
try await internalTestVerify(
errorCode: expectedError.code,
function: #function,
reCAPTCHAfallback: true,
presenterError: nil,
fakeAppCheck: fakeAppCheck
)
}
}
#endif
Loading