Skip to content
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.

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

[Auth] Phone Auth – Fallback to reCATCHA flow when "invalid app credential" error is thrown #13519

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Aug 19, 2024

This is one step towards fixing #13479.

There are two parts to fix for that issue:

  1. why invalid app credential is being returned
  2. how SDK responds to it

After testing 10.29.0, I can only reproduce the issue on 11.0.0. This PR addresses the second point– how the SDK responds to the error.

Based on the 11.0.0 code comment (see diff) and 10.29.0 code (see below snippet), reCAPTCHA should launch for a missing app token or invalid app credential.

BOOL isInvalidAppCredential =
error.code == FIRAuthErrorCodeInternalError &&
underlyingError.code == FIRAuthErrorCodeInvalidAppCredential;

The issue is that the v11 parsing logic was expecting the invalid app credential to be an internal error, but it is not wrapped as such, so the control flow never enters the conditional and calls into reCAPTCHA.

return error(code: SharedErrorCode.public(code), userInfo: userInfo)

Looking back at the 10.29.0 code, I think it's possible this was a bug there too where invalid app credential is also a public error. But, at least from my testing, I'm not seeing the server return "invalid app credential" on 10.29.0.

BOOL isPublic = (code & FIRAuthPublicErrorCodeFlag) == FIRAuthPublicErrorCodeFlag;

/** Indicates that an invalid APNS device token was used in the verifyClient request.
*/
FIRAuthInternalErrorCodeInvalidAppCredential = FIRAuthPublicErrorCodeFlag |
FIRAuthErrorCodeInvalidAppCredential,

This PR should add a fallback to reCAPTCHA to address behavior.

#no-changelog

@ncooke3 ncooke3 changed the title [Auth] Fix v11 phone auth regression that prevented fallback to reCATCHA flow [Auth] Fallback to reCATCHA flow when "invalid app credential" error is thrown Aug 19, 2024
@ncooke3 ncooke3 changed the title [Auth] Fallback to reCATCHA flow when "invalid app credential" error is thrown [Auth] Fall back to reCATCHA flow when "invalid app credential" error is thrown Aug 19, 2024
@ncooke3 ncooke3 changed the title [Auth] Fall back to reCATCHA flow when "invalid app credential" error is thrown [Auth] Phone Auth – Fallback to reCATCHA flow when "invalid app credential" error is thrown Aug 19, 2024
@ncooke3 ncooke3 marked this pull request as ready for review August 19, 2024 19:30
@ncooke3 ncooke3 merged commit c570e44 into main Aug 23, 2024
56 checks passed
@ncooke3 ncooke3 deleted the nc/fix-phone-auth branch August 23, 2024 22:49
mergify bot referenced this pull request in cgrindel/rules_swift_package_manager Sep 16, 2024
….2.0" (#1228)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[firebase/firebase-ios-sdk](https://github.com/firebase/firebase-ios-sdk)
| minor | `from: "11.1.0"` -> `from: "11.2.0"` |

---

### Release Notes

<details>
<summary>firebase/firebase-ios-sdk (firebase/firebase-ios-sdk)</summary>

###
[`v11.2.0`](https://github.com/firebase/firebase-ios-sdk/releases/tag/11.2.0):
Firebase Apple 11.2.0

[Compare
Source](https://github.com/firebase/firebase-ios-sdk/compare/11.1.0...11.2.0)

The Firebase Apple SDK (11.2.0) is now available. For more details, see
the [Firebase Apple SDK release
notes.](https://firebase.google.com/support/release-notes/ios#11.2.0)

To install this SDK, see [Add Firebase to your
project](https://firebase.google.com/docs/ios/setup).

#### What's Changed

- \[Auth] Phone Auth – Fallback to reCATCHA flow when "invalid app
credential" error is thrown by
[@&#8203;ncooke3](https://github.com/ncooke3) in
[https://github.com/firebase/firebase-ios-sdk/pull/13519](https://github.com/firebase/firebase-ios-sdk/pull/13519)
- \[Auth] Fix Xcode 16 continuation crashes by
[@&#8203;paulb777](https://github.com/paulb777) in
[https://github.com/firebase/firebase-ios-sdk/pull/13521](https://github.com/firebase/firebase-ios-sdk/pull/13521)
- \[Auth] Fix Phone Auth via APNS for Sandbox Tokens and update Sample's
Firebase app by [@&#8203;paulb777](https://github.com/paulb777)
in
[https://github.com/firebase/firebase-ios-sdk/pull/13539](https://github.com/firebase/firebase-ios-sdk/pull/13539)
- \[Auth] Add background modes capability to plist by
[@&#8203;ncooke3](https://github.com/ncooke3) in
[https://github.com/firebase/firebase-ios-sdk/pull/13548](https://github.com/firebase/firebase-ios-sdk/pull/13548)
- \[Auth] When swizzling is disabled, open URLs via SceneDelegate by
[@&#8203;ncooke3](https://github.com/ncooke3) in
[https://github.com/firebase/firebase-ios-sdk/pull/13557](https://github.com/firebase/firebase-ios-sdk/pull/13557)
- \[Auth] Fix unexpected nil in fetchSignInMethods success case by
[@&#8203;ncooke3](https://github.com/ncooke3) in
[https://github.com/firebase/firebase-ios-sdk/pull/13561](https://github.com/firebase/firebase-ios-sdk/pull/13561)
- \[Auth] Fix user session persistence in multi tenant projects by
[@&#8203;paulb777](https://github.com/paulb777) in
[https://github.com/firebase/firebase-ios-sdk/pull/13567](https://github.com/firebase/firebase-ios-sdk/pull/13567)
- \[Crashlytics] Fix Firebase/Crashlytics min iOS version by
[@&#8203;paulb777](https://github.com/paulb777) in
[https://github.com/firebase/firebase-ios-sdk/pull/13580](https://github.com/firebase/firebase-ios-sdk/pull/13580)
- \[Database] Fix temporary disconnect when app goes inactive by
[@&#8203;paulb777](https://github.com/paulb777) in
[https://github.com/firebase/firebase-ios-sdk/pull/13564](https://github.com/firebase/firebase-ios-sdk/pull/13564)
- \[Firestore] Mark readonly public classes as Sendable by
[@&#8203;paulb777](https://github.com/paulb777) in
[https://github.com/firebase/firebase-ios-sdk/pull/13453](https://github.com/firebase/firebase-ios-sdk/pull/13453)
- \[Firestore] Support hex strings to prevent broken log lines by
[@&#8203;ls-todd-lunter](https://github.com/ls-todd-lunter) in
[https://github.com/firebase/firebase-ios-sdk/pull/13128](https://github.com/firebase/firebase-ios-sdk/pull/13128)
- \[Functions] `FunctionsContext` Updates by
[@&#8203;yakovmanshin](https://github.com/yakovmanshin) in
[https://github.com/firebase/firebase-ios-sdk/pull/13531](https://github.com/firebase/firebase-ios-sdk/pull/13531)
- \[Functions] Updated Functions Errors by
[@&#8203;yakovmanshin](https://github.com/yakovmanshin) in
[https://github.com/firebase/firebase-ios-sdk/pull/13535](https://github.com/firebase/firebase-ios-sdk/pull/13535)
- \[Testing] Update OCMock dependency to v3.9.4 by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13536](https://github.com/firebase/firebase-ios-sdk/pull/13536)
- \[Vertex AI] Make `uri` optional in `Citation` and add `title` field
by [@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13520](https://github.com/firebase/firebase-ios-sdk/pull/13520)
- \[Vertex AI] Add `Sendable` conformance to types by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13560](https://github.com/firebase/firebase-ios-sdk/pull/13560)
- \[Vertex AI] Make `Logger` properties constants by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13570](https://github.com/firebase/firebase-ios-sdk/pull/13570)
- \[Vertex AI] Make `GenerativeModel` and `Chat` into Swift actors by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13545](https://github.com/firebase/firebase-ios-sdk/pull/13545)
- \[Vertex AI] Make generateContentStream/sendMessageStream throws by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13573](https://github.com/firebase/firebase-ios-sdk/pull/13573)
- \[Vertex AI] Add `SourceImage` enum to `ImageConversionError` by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13575](https://github.com/firebase/firebase-ios-sdk/pull/13575)
- \[Vertex AI] Add `responseSchema` to `GenerationConfig` by
[@&#8203;andrewheard](https://github.com/andrewheard) in
[https://github.com/firebase/firebase-ios-sdk/pull/13576](https://github.com/firebase/firebase-ios-sdk/pull/13576)

#### New Contributors

- [@&#8203;ls-todd-lunter](https://github.com/ls-todd-lunter)
made their first contribution in
[https://github.com/firebase/firebase-ios-sdk/pull/13128](https://github.com/firebase/firebase-ios-sdk/pull/13128)

**Full Changelog**:
firebase/firebase-ios-sdk@11.1.0...11.2.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config
help](https://github.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: cgrindel-self-hosted-renovate[bot] <139595543+cgrindel-self-hosted-renovate[bot]@users.noreply.github.com>
Co-authored-by: Chuck Grindel <chuck.grindel@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@firebase firebase locked and limited conversation to collaborators Sep 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants