feat: Add Recaptcha attestation provider#94
Conversation
…Provider (#85) Signed-off-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com> Co-authored-by: Paul Beusterien <paulbeusterien@google.com> Co-authored-by: Nick Cooke <nickcooke@google.com> Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
…a provider - Change implicitly unwrapped optionals to standard optionals or constants to prevent runtime crashes. - Handle Recaptcha SDK dynamic lookups safely without force-unwrapping. - Standardize error handling to use GACAppCheckErrorUtil and AppCheckCoreErrorDomain. - Add missing AppCheckCore imports to fix scope build errors. - Remove outdated TODO from AppCheckCore.podspec.
…d environment guidelines - Add new Communication Guidelines section prioritizing categorized bullet points and visual indicators. - Add Environment & Troubleshooting section for dealing with sandboxes, Python paths, and Ruby version conflicts. - Standardize Git & Commits guidance to encourage frequent, scoped commits.
…e provider - Add `RecaptchaEnterpriseProviderUnit` test target to SPM package. - Verify provider initialization, dynamic SDK loading, and graceful error states. - Validate API token exchange payload formatting and responses, including limited-use configurations. - Ensure strict compatibility with Objective-C `FBLPromise` and `AppCheckCoreErrorCode` APIs. - Apply standard formatting via style.sh.
… guidelines - Document known compiler issues with `FBLPromise` generics in Swift bridging. - Add exact syntax workarounds for unlabeled Obj-C static methods and dynamic dispatch fallbacks. - Specify SPM target scoping requirements and `swift test --filter` syntax in TDD step.
- Deleted forwarding headers in `AppCheckCore/Sources/Core/...` - Updated imports in all Objective-C sources and tests to use `AppCheckCore/Sources/Public/AppCheckCore/...` - Verified build is successful via `swift test --list-tests`
- Added requirement for final report (summary + commit message) - Added mandate for updating agents.md with learnings - Documented fixture loading issue on macOS with SPM
- Added Step 5 for style application and Step 6 for doc formatting - Removed redundant style instructions from Quality Gates - Wrapped long lines in `agents.md` to 80 characters
|
/gemini review |
| import RecaptchaInterop | ||
|
|
||
| @objc(GACRecaptchaEnterpriseProvider) | ||
| public final class AppCheckCoreRecaptchaEnterpriseProvider: NSObject, AppCheckCoreProvider { |
There was a problem hiding this comment.
todo: Decide whether to add platform guards.
-
Option 1: Keep it as is (No availability guards)
The provider relies on reflection to load the reCAPTCHA SDK at runtime.- Pros: Allows the provider to be used in cross-platform code (e.g., shared between iOS and macOS) without wrapping it in #if os(iOS) or if #available. It compiles on all platforms and fails gracefully at runtime if the SDK is missing.
- Cons: Developers can attempt to use it on platforms where it will never work (like macOS), and they will only find out at runtime.
-
Option 2: Add availability guards (e.g.,
@available(iOS 15.0, *))
Mark the class or initializers with explicit OS version or platform constraints.- Pros: Prevents developers from using the provider on platforms or OS versions where it is not supported, catching mistakes at compile time.
- Cons: Forces developers to use availability checks (if #available(...)) or platform checks (#if os(iOS)) in their code if they are building for multiple platforms, even though the provider itself is safe to compile everywhere due to reflection.
There was a problem hiding this comment.
Curious what reviewers think here?
There was a problem hiding this comment.
todo: Update the doc comment pending discussion.
There was a problem hiding this comment.
I think my personal preference would be Option 1 with the addition of - (BOOL)isSupported; to GACAppCheckProvider (and bubbling it up to FIRAppCheckProvider) before release. It'd have to be in the @optional section in FIRAppCheckProvider to avoid breaking custom providers but it would allow accurate availability checks at runtime.
Our current documentation shows this code snippet
class YourAppCheckProviderFactory: NSObject, AppCheckProviderFactory {
func createProvider(with app: FirebaseApp) -> AppCheckProvider? {
if #available(iOS 14.0, *) {
return AppAttestProvider(app: app)
} else {
return DeviceCheckProvider(app: app)
}
}
}
which, even if updated to include other Apple platforms, would return an instance of AppAttestProvider on macOS, even though it doesn't work on the platform. Being able to check AppAttestProvider.isSupported (wrapping DCAppAttestService.shared.isSupported) and RecaptchaEnterpriseProvider.isSupported (or whatever names), would allow developers to create a functional chain of fallbacks.
Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com> Signed-off-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
There was a problem hiding this comment.
fyi: @weixifan, this is the networking layer.
|
|
||
| NSString *const kGACAppCheckMissingRecaptchaSDKMessage = | ||
| @"The reCAPTCHA Enterprise SDK is not linked. See " | ||
| @"https://cloud.google.com/recaptcha/docs/instrument-ios-apps#prepare-environment"; |
There was a problem hiding this comment.
This should be updated to Firebase documentation before release.
| import AppCheckCore | ||
| #endif | ||
| import Foundation | ||
| import Promises |
There was a problem hiding this comment.
Maybe worth a try to see if jetski can convert the Promises to async/await? and eliminate the Swift Promises dep?
Add ReCAPTCHA Enterprise Attestation Provider
Metadata
Description
This PR introduces the
RecaptchaEnterpriseProviderto the App Check SDK, enabling developers to use reCAPTCHA Enterprise for app attestation on iOS. It includes the core implementation in Swift, integration with the Objective-C core, and a comprehensive test suite.Integration Approach
The following is the approach for each package manager.
RecaptchaEnterpriseProvider). This ensures users (the firebase-ios-sdk package) only pull in theRecaptchaInteropdependency if they explicitly use this provider. Mainly, this prevents the need of creating a third target to selectively re-export both the Objective-C and Swift API.AppCheckCoretarget for iOS to maintain consistency and avoid the complexity of new pods or subspecs.Dependencies
interop-ios-for-google-sdks(specifically theRecaptchaInteropproduct) to support decoupled interaction with the reCAPTCHA Enterprise SDK.New APIs
AppCheckCoreRecaptchaEnterpriseProvider(Swift) /GACRecaptchaEnterpriseProvider(Obj-C)init(siteKey:resourceName:APIKey:requestHooks:)getToken(completion:)getLimitedUseToken(completion:)Error Handling & Edge Cases (CUJ)
GACAppCheckErrorCodeUnsupportedwith a specific, actionable message directing the developer to the setup documentation:kGACAppCheckMissingRecaptchaSDKMessage) and appliedNS_SWIFT_NAMEfor more idiomatic Swift usage.AppCheckCoreErrorCodeServerUnreachableto trigger the SDK's exponential backoff retry logic correctly.Headers Moved to Public
To allow the Swift implementation to interact with the Objective-C core, the following headers were moved to
AppCheckCore/Sources/Public/AppCheckCore:GACAppCheckAPIService.hGACAppCheckBackoffWrapper.hGACAppCheckErrorUtil.hGACURLSessionDataResponse.hTests Added
A new test target
RecaptchaEnterpriseProviderUnitwas added with 17 new tests covering:AppCheckCoreRecaptchaEnterpriseProviderTests: Verifies missing SDK handling, success flows, and limited-use token success flows.RecaptchaEnterpriseCoreAPIServiceTests: Verifies token exchange API requests and JSON body structure.RecaptchaEnterpriseCoreTokenGeneratorTests: Verifies token generation, backoff application, and error mapping.Other Changes
agents.mdto define workflow instructions for AI agents in this repo..github/workflows/spm.ymlto use theAppCheck-Packagetarget.cc: @weixifan @rlazo