Enable Firebase AppCheck with Sample + isolate AppCheckCore for Google SignIn #107
Conversation
- Introduced CI workflow for build and package validation using GitHub Actions. - Added detailed building instructions for local development in `docs/BUILDING.md`. - Created `AGENTS.md` to outline agent instructions and best practices for contributions. - Updated `Readme.md` to include CI and publishing information. - Enhanced `CONTRIBUTING.md` with guidelines for new contributors and repository orientation.
…n for fork contributors
…s and troubleshooting
- Integrate Google.AppCheckCore v11.2.0 support (pr/repair-firebase-app-check) - Upgrade Google.SignIn to v9.0.0 (origin/main) - Update AppAuth to v2.0.0 and GTMAppAuth to v5.0.0 (origin/main) - Keep separate GOOGLE_APP_CHECK_CORE_ARTIFACT PodSpec definition
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bce1c1fa4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…thApp to use NSObject instead of interfaces
|
Hey @AdamEssenmacher , sorry I went too fast, the other modes actually didn't work. Now this is fixed and validated in Playground sample app 👍
|
|
Looking good. The only 'miss' I'm noticing is that AppCheckCore should have PackageReferences on PromisesObjC and GoogleUtilities I think. I also have a question about the changes in |
…s for PromisesObjC and GoogleUtilities to AppCheckCore.csproj
|
Hey @AdamEssenmacher , as usual, thanks for the (fast!) review. Please find below my comments. AppCheckCore missing PackageReferencesGood catch, I fixed it by adding ApiDefinition.cs changesActually, most of my changes were driven by a runtime I'm not an ObjC binding expert, so I worked through it incrementally:
If you have a better approach for (3), I'm happy to try it. Thank you. |
|
Thanks for the answer. Believe it or not, I'm not an iOS bindings expert either 😓 The thing that jumped out at me was the NSObject parameter. That's not really right... but it's also going to work as long as the right object is passed into it. I suppose the likelihood of this ever being misused is pretty low, as it's kind of one of those things that gets set once at startup and you never have to think about it again. It sounds like you've tested it pretty thoroughly, and provided samples that others can follow. So, it's better than what we have now (which is nothing, lol). If the binding contract is less than ideal, I guess I really don't care that much. It can always be improved later. I'll get this merged in and do a light test in my own app before publishing a new package. Your work here is going to make a lot of people happy! |
|
One other thing @Kapusch , I can't add you as a collaborator at Plugin.Firebase (only Tobi can do that right now), but I'll ask him to after reviewing the AppCheck PR over there. I'd like you to add you as a collaborator here if you'd like. Either way, feel free to hit me up on Discord. I just joined the MAUIverse server today, so I should be easy to find there. |
I hear you. My primary goal was to make it work, but I understand it is not right and we should improve it.
If you think there is a true risk of instability, I would prefer to improve it now. Otherwise, agree it can wait.
As I said, this was all new to me, so I didn't feel legitimate initially, but since you proposed and as I'm convinced it's best to be a group of maintainers on this, I'd be happy to join you as collaborator here as well! Thank you 😊 |
|
I finally made the fix in Also, I took the opportunity to improve the sample app to let the tester defining the AppCheck mode at first launch, and reset it anytime. |
|
@codex do one final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ee57a36ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
This pull request introduces several improvements and fixes to the build, CI, and packaging process, as well as documentation updates and the addition of new artifacts. The most significant changes include enabling and updating the Firebase AppCheck and Google AppCheckCore components, improving CI reliability, and clarifying versioning and troubleshooting for contributors.
Artifact and Dependency Updates:
Firebase.AppCheckartifact to version12.5.0.4, added its podspec, samples, and set dependencies (includingGoogle.AppCheckCore). [1] [2] [3] [4] [5]Google.AppCheckCoreartifact (11.2.0.0) with its podspec and dependencies, and updatedGoogle.SignInto depend on it. [1] [2] [3] [4]Build and CI Improvements:
macos-15runners, added toolchain version checks, and configured GitHub Packages as a NuGet source for publishing. [1] [2].csprojdirectly (instead of using solution-level targets), improving reliability and fixing issues with solution target builds. [1] [2] [3]Versioning and Packaging:
FileVersionis always set, andPackageVersionis only overwritten if it doesn't already have a pre-release suffix—enabling forked builds to publish custom versions without modifying shared version lines.Documentation and Troubleshooting:
Other Build Script Fixes: