feat(auth): add phone MFA #9044

Lyokone · 2022-07-06T13:52:55Z

Description

see title

Related Issues

Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (melos run analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I signed the CLA.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change.
No, this is not a breaking change.

* chores: update format CI step to match local melos format * chores: update format CI step to match local melos format * feat(auth, android): mfa * feat(auth, android): add MFA to phone options * feat(auth, android): add phone MFA * feat(auth, android): add MFA signin * feat(auth): add documentation * feat(auth, android): fix typings * feat(auth, android): fix tests * feat(auth, android): add input for phone number to example app * feat(auth, android): add unenroll and getEnrolledFactors * feat(auth, android): fix formatting * feat(auth, android): fix formatting * feat(ui, android): use mocktail for mocking dependencies * feat(auth, android): fix analyze * feat(auth): fix melos generate command * Update packages/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_multi_factor.dart Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>

* chores: update format CI step to match local melos format * chores: update format CI step to match local melos format * feat(auth, android): mfa * feat(auth, android): add MFA to phone options * feat(auth, android): add phone MFA * feat(auth, android): add MFA signin * feat(auth, android): fix tests * feat(auth, android): add input for phone number to example app * feat(auth, android): add unenroll and getEnrolledFactors * feat(auth, android): fix formatting * feat(auth, android): fix formatting * feat(auth, android): fix analyze * feat(auth, ios): implement enroll * feat(auth, ios): implement MFA for signin * feat(auth, ios): add conditions to allow macos to build * feat(auth, ios): add conditions to allow macos to build * feat(auth, ios): fix ios build * Update packages/firebase_auth/firebase_auth/ios/Classes/FLTFirebaseAuthPlugin.m Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> * fix: fix rebase Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>

* feat(auth, android): add phone MFA to Android (#8998) * chores: update format CI step to match local melos format * chores: update format CI step to match local melos format * feat(auth, android): mfa * feat(auth, android): add MFA to phone options * feat(auth, android): add phone MFA * feat(auth, android): add MFA signin * feat(auth): add documentation * feat(auth, android): fix typings * feat(auth, android): fix tests * feat(auth, android): add input for phone number to example app * feat(auth, android): add unenroll and getEnrolledFactors * feat(auth, android): fix formatting * feat(auth, android): fix formatting * feat(ui, android): use mocktail for mocking dependencies * feat(auth, android): fix analyze * feat(auth): fix melos generate command * Update packages/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_multi_factor.dart Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> * feat(auth, ios): add phone MFA (#9008) * chores: update format CI step to match local melos format * chores: update format CI step to match local melos format * feat(auth, android): mfa * feat(auth, android): add MFA to phone options * feat(auth, android): add phone MFA * feat(auth, android): add MFA signin * feat(auth, android): fix tests * feat(auth, android): add input for phone number to example app * feat(auth, android): add unenroll and getEnrolledFactors * feat(auth, android): fix formatting * feat(auth, android): fix formatting * feat(auth, android): fix analyze * feat(auth, ios): implement enroll * feat(auth, ios): implement MFA for signin * feat(auth, ios): add conditions to allow macos to build * feat(auth, ios): add conditions to allow macos to build * feat(auth, ios): fix ios build * Update packages/firebase_auth/firebase_auth/ios/Classes/FLTFirebaseAuthPlugin.m Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> * fix: fix rebase Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com> * feat(auth): fix android merge * feat(auth): fix ios merge * feat(auth): add e2e tests * fix(auth): fix reintroduce tests * feat(auth): add documentation * feat(auth): add e2e tests Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>

* feat(auth): add multifactoruser * feat(auth, web): bridge * feat(auth, web): solve bridging errors * feat(auth, web): finish phone web mfa * feat(auth, web): finish phone web mfa * feat(auth, web): fix analyze * feat(auth, web): fix analyze * feat(auth, web): fix wrong argument

russellwheatley

some minor feedback but LGTM.

...e_auth/android/src/main/java/io/flutter/plugins/firebase/auth/FlutterFirebaseAuthPlugin.java

packages/firebase_auth/firebase_auth_platform_interface/lib/src/pigeon/messages.pigeon.dart

rrousselGit · 2022-07-19T11:23:47Z

melos.yaml

@@ -186,8 +186,9 @@ scripts:

  generate:pigeon:
    run: |
-      melos exec -- "flutter pub run pigeon --input ./pigeons/messages.dart"
-      melos run generate:pigeon:macos
+      melos exec -- "flutter pub run pigeon --input ./pigeons/messages.dart" && \


Is that &&\ needed? It wasn't there before

Without it, the second command would run even if the first one fails.
If there is an issue with the generation, it's better to not run the other commands IMO

rrousselGit · 2022-07-19T11:30:31Z

packages/firebase_auth/firebase_auth/lib/src/user.dart

@@ -421,6 +421,10 @@ class User {
    await _delegate.verifyBeforeUpdateEmail(newEmail, actionCodeSettings);
  }

+  MultiFactor get multiFactor {
+    return MultiFactor._(_delegate.multiFactor);


MultiFactor either should override == or this property should be cached.

At the moment this.multiFactor != this.multiFactor

Good catch, thanks

rrousselGit · 2022-07-19T11:32:31Z

...th/firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart

@@ -23,9 +24,12 @@ class MethodChannelFirebaseAuth extends FirebaseAuthPlatform {
  );

  static Map<String, MethodChannelFirebaseAuth>
-      _methodChannelFirebaseAuthInstances =
+      methodChannelFirebaseAuthInstances =


Is it expected for this to be public? It's public but undocumented

I've added documentation, needed to get the Auth instance in the error parsing

rrousselGit · 2022-07-19T11:33:59Z

...rebase_auth/firebase_auth_platform_interface/lib/src/method_channel/utils/pigeon_helper.dart

+  List<PigeonMultiFactorInfo?> pigeonMultiFactorInfo,
+) {
+  return pigeonMultiFactorInfo
+      .where((element) => element != null)


use package:collection and its whereNotNull

rrousselGit · 2022-07-19T11:35:05Z

...s/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/utils/exception.dart

+  final pigeonMultiFactorInfo =
+      (additionalData['multiFactorHints'] as List<Object?>)
+          .where((element) => element != null)
+          .cast<Object>()


rrousselGit · 2022-07-19T11:35:37Z

...s/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/utils/exception.dart

@@ -64,3 +72,46 @@ FirebaseException platformExceptionToFirebaseAuthException(
    credential: credential,
  );
 }
+
+FirebaseAuthMultiFactorException parseMultiFactorError(
+    Map<String, dynamic> details) {


Prefer Object? instead of dynamic

rrousselGit · 2022-07-19T11:36:56Z

...base_auth_platform_interface/lib/src/platform_interface/platform_interface_multi_factor.dart

+    required String uid,
+    required this.phoneNumber,
+  }) : super(
+          displayName: displayName,


Are we using Dart 2.17 already? If so, these should be using the new super syntax

nvm we aren't

Lyokone force-pushed the feat/mfa branch from d29f201 to eb29b91 Compare July 7, 2022 07:16

russellwheatley added the plugin: auth label Jul 11, 2022

Lyokone force-pushed the feat/mfa branch from 0af57e3 to cd679b7 Compare July 12, 2022 07:24

Lyokone marked this pull request as ready for review July 18, 2022 08:51

Lyokone requested a review from lesnitsky as a code owner July 18, 2022 08:51

Lyokone removed the request for review from lesnitsky July 18, 2022 09:08

Lyokone self-assigned this Jul 18, 2022

Lyokone force-pushed the feat/mfa branch from bfd2695 to 244f5f4 Compare July 18, 2022 11:13

Lyokone and others added 6 commits July 18, 2022 15:21

feat(auth): fix android merge

b9f5ff5

feat(auth): fix ios merge

8585df3

Lyokone force-pushed the feat/mfa branch from 244f5f4 to c6ddc18 Compare July 18, 2022 13:21

russellwheatley approved these changes Jul 19, 2022

View reviewed changes

...e_auth/android/src/main/java/io/flutter/plugins/firebase/auth/FlutterFirebaseAuthPlugin.java Outdated Show resolved Hide resolved

packages/firebase_auth/firebase_auth_platform_interface/lib/src/pigeon/messages.pigeon.dart Show resolved Hide resolved

feat(auth, android): add early return

d6a62d6

Lyokone force-pushed the feat/mfa branch from dc1c52f to d6a62d6 Compare July 19, 2022 11:22

rrousselGit reviewed Jul 19, 2022

View reviewed changes

rrousselGit approved these changes Jul 19, 2022

View reviewed changes

Lyokone force-pushed the feat/mfa branch 2 times, most recently from 0facb04 to fb1e99b Compare July 19, 2022 14:57

feat(auth, android): change dynamic to Object

097128b

Lyokone force-pushed the feat/mfa branch from fb1e99b to 097128b Compare July 19, 2022 15:15

Lyokone merged commit 1b85c8b into master Jul 20, 2022

Lyokone deleted the feat/mfa branch July 20, 2022 07:13

russellwheatley mentioned this pull request Jul 20, 2022

🐛 [firebase_auth] TypeError: Cannot read properties of undefined (reading 'settings') #9097

Closed

firebase locked and limited conversation to collaborators Aug 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auth): add phone MFA #9044

feat(auth): add phone MFA #9044

Lyokone commented Jul 6, 2022

russellwheatley left a comment

rrousselGit Jul 19, 2022

Lyokone Jul 19, 2022

rrousselGit Jul 19, 2022

Lyokone Jul 19, 2022

rrousselGit Jul 19, 2022

Lyokone Jul 19, 2022

rrousselGit Jul 19, 2022

Lyokone Jul 19, 2022

rrousselGit Jul 19, 2022

rrousselGit Jul 19, 2022

rrousselGit Jul 19, 2022

rrousselGit Jul 19, 2022

feat(auth): add phone MFA #9044

feat(auth): add phone MFA #9044

Conversation

Lyokone commented Jul 6, 2022

Description

Related Issues

Checklist

Breaking Change

russellwheatley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment