feat(firebase): tvOS support for firebase_app_check - #18
Conversation
Federated firebase_app_check_tvos — App Check for Apple TV, built on the Firebase Apple SDK. Re-exports the firebase_app_check Dart API and ships the native tvOS pluginClass; depends on firebase_core_tvos. DeviceCheck, App Attest, and Debug providers work on tvOS 15+. reCAPTCHA (RecaptchaProvider) is unavailable in the Firebase tvOS SDK — excluded, with its Pigeon case kept as an unsupported-platform stub so dispatch stays intact.
a56b760 to
c3ad863
Compare
DenisovAV
left a comment
There was a problem hiding this comment.
Review
Clean port, gate-green on the merged tree, and the question I went in expecting to sink it turned out fine. Two small notes below, neither blocking.
App Check on tvOS is real, not theatre
I set out to establish whether attestation actually works here, because App Check exists to gate backend access — a port where only the debug provider functions would be worse than no port, and the README would be claiming protection the device cannot provide. It is not the case. From the tvOS SDK on my machine (AppleTVOS26.5):
DCDevice: API_AVAILABLE(ios(11.0), macos(10.15), tvos(11.0), watchos(9.0))
DCAppAttestService: API_AVAILABLE(macos(11.0), ios(14.0), tvos(15.0), watchos(9.0))
Both providers exist, App Attest from tvOS 15 — which is exactly this podspec's floor. FirebaseAppCheck.podspec at 12.15.0 declares tvos_deployment_target = '15.0' and s.tvos.weak_framework = 'DeviceCheck', matching your podspec's weak-link comment. And when a provider genuinely cannot run, AppCheckCore raises unsupportedAttestationProvider: rather than handing back a token — so the failure is loud, which is what matters.
The one hand-edit that narrows reCAPTCHA to #if os(iOS) is not a shortcut either: FIRRecaptchaProvider.h is declared API_UNAVAILABLE(macos, tvos, watchos, macCatalyst), so the guard is required to compile.
Port faithfulness
Delta against firebase_app_check 0.4.5 as published is four hunks: the core import, os(tvOS) added to the messenger gate and to Messages.g.swift, the reCAPTCHA narrowing, and tvOS 14.0 added to the appAttestWithDeviceCheckFallback availability. Constants.swift identical. Nothing behavioural.
Two notes
1. The error-code mapping is off by one, and it lands worst on tvOS. createFlutterError (FirebaseAppCheckPlugin.swift:219-233) reads:
case 0: // FIRAppCheckErrorCodeServerUnreachable
case 1: // FIRAppCheckErrorCodeInvalidConfiguration
case 2: // FIRAppCheckErrorCodeKeychain
case 3: // FIRAppCheckErrorCodeUnsupportedFIRAppCheckErrors.h at 12.15.0 declares Unknown = 0, ServerUnreachable = 1, InvalidConfiguration = 2, Keychain = 3, Unsupported = 4. Every case is shifted. The consequence is specific to this platform: Unsupported — "App Attest is not available on this device", the most likely error on older Apple TV hardware — is code 4, falls to default, and reaches Dart as unknown; a keychain failure arrives labelled code-unsupported.
It is verbatim upstream, so I would not fix it here — diverging costs you on every rebase. But it is worth a line in PORTING_REPORT.md, so that whoever does the physical-hardware pass does not read an unknown and go looking in the wrong place.
2. The README's reCAPTCHA sentence promises more than the code guarantees. "Requesting it on tvOS leaves the provider unconfigured and getToken surfaces an explicit error" holds when activate() runs before any App Check instance exists. FlutterAppCheckProviderFactory.createProvider pre-configures a DeviceCheck wrapper, and the tvOS recaptcha branch leaves delegateProvider untouched — so if some Firebase product triggered App Check first, tokens keep flowing via DeviceCheck without the error. Either delegateProvider = nil in that branch, or soften the sentence. Upstream has the same shape on macOS, so this is a faithful port of an upstream wart; only the README makes a claim about it.
Nit: the tvOS 14.0 in the fallback #available is constant-true against a 15.0 floor, so the DeviceCheck fallback is unreachable on tvOS. 15.0 would be more honest.
LGTM once you decide on the two notes — neither needs to hold the merge.
Dismissing my own approval — it was the wrong state for this review. The body asks for a change (either delegateProvider = nil in the tvOS reCAPTCHA branch, or softening the README sentence that promises an explicit error) and for a note in PORTING_REPORT about the off-by-one error codes. A review that asks for changes should sit as a comment, not an approval; "LGTM once you decide on the two notes" is not something an approval can express. The findings themselves stand as written.
… 15 availability, error-code note)
DenisovAV
left a comment
There was a problem hiding this comment.
Re-review — both notes closed
I offered two ways to resolve the reCAPTCHA discrepancy, and the one you took is the right one. The README no longer promises an explicit error; it now says that when reCAPTCHA is requested, the tvOS build falls back to the default DeviceCheck provider. That is what the code does, rather than what one would like it to do.
The off-by-one error mapping is recorded in PORTING_REPORT.md, which is what I asked for — no divergence from upstream. Whoever runs the physical-hardware pass will no longer read an unknown and go looking in the wrong place when it actually means App Attest is unavailable.
The nit is fixed too: tvOS 14.0 became 15.0 in both places, so those availability checks are no longer constant-true against a 15.0 floor.
Gate clean, CI 18/18.
Retracting my earlier objection: I approved this while it still carried open requests, which was the wrong state for that review. It is the right state now.
LGTM.
|
0.4.6 pulls platform interface 0.4.2, which moved the pigeon contract in two ways the generated Swift here (built against ≤0.4.1) doesn't cover:
|
What does this PR do?
Adds federated
firebase_app_check_tvos— Firebase App Check for Apple TV, built on the Firebase Apple SDK. Re-exports thefirebase_app_checkDart API and ships the native tvOSpluginClass; depends onfirebase_core_tvos.DeviceCheck, App Attest, and Debug attestation providers work on tvOS 15+. reCAPTCHA is excluded —
RecaptchaProvideris not part of the Firebase tvOS SDK; its Pigeon"recaptcha"case is kept as anunsupported-platformstub so registration/dispatch stay intact.Package(s) touched:
firebase_app_check_tvos(new)How was it tested?
Native
FirebaseAppCheck 12.15.0initializes on tvOS. With the Debug provider,activate()issues a debug token andgetToken()exchanges it with the App Check backend (exchangeDebugToken), returning a valid App Check token.Platform.operatingSystem == "tvos"/Platform.isIOS == true.example/appgetTokenround-trip succeeds against a live projectdart analyzeis clean for the packageVersioning & changelog
version:set to0.0.1(new package)## 0.0.1entry at the top ofCHANGELOG.md0.x: initial0.0.1Checklist
firebase_app_check_tvosfiles are touchedTODO/debug leftREADME.mddocuments tvOS behaviour (reCAPTCHA excluded; DeviceCheck/App Attest need a paid account for real-device attestation; version-alignment note)Notes for reviewers
RecaptchaProvideris iOS-only in the Firebase SDK. The"recaptcha"handler is kept as anunsupported-platformstub (never removed), so Pigeon dispatch doesn't break._tvosleaf packages): the native Pigeon matches thefirebase_core 4.11.x/firebase_core_platform_interface 7.1.0train. Mixingfirebase_core_tvoswith a differentfirebase_corewhoseFirebaseOptionslist differs can crash inCoreFirebaseOptions.fromListat launch (Dartoptions:init).Relavant issue: #12