[local_auth] Api to stop authentication #2111

karan-rawal · 2019-09-26T06:34:16Z

Description

This Api Allows the user to cancel the authentication. Helpful when there's in display fingerprint scanner.

Related Issues

No related issues. It's an enhancement.

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.

Breaking Change

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

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

karan-rawal · 2019-10-14T07:39:27Z

@jmagman @collinjackson @cyanglaz Can you please review this PR, I need it in my project. It's been a long time. :(

mehmetf · 2019-10-24T20:17:25Z

packages/local_auth/lib/local_auth.dart

+    if (_platform.isAndroid) {
+      return _channel.invokeMethod<bool>('stopAuthentication');
+    }
+    final Future<bool> future = Future<bool>(() => true);


You can replace these three lines with:

return Future.sync(() => true);

packages/local_auth/lib/local_auth.dart

packages/local_auth/pubspec.yaml

mehmetf · 2019-10-24T20:19:10Z

packages/local_auth/lib/local_auth.dart

+  /// Returns false if there was some error or no auth in progress.
+  ///
+  /// Returns [Future] bool true or false:
+  Future<bool> stopAuthentication() {


Out of curiosity, how would the Dart app decide to call this?

For devices with in-display fingerprint sensor(assuming that authentication is in progress), if user clicks on any other button on the screen, the fingerprint scanning will still be in progress. Hence, we needed an API to stop the authentication in such scenarios.

Thanks for the response!

If user clicks on any other button on the screen

This is the part that I don't get. Why would I allow user to click anything in my app if there's authentication in progress? The user is not authenticated so should not be interacting with the app at all. I was assuming that on-screen auth will show exactly the same dialog. The only difference would be where you touch.

Could you please point me to a video or an article of some sort which shows cancelAuthentication in action?

@mehmetf For in-display fingerprint scanners, we don't get any popup. We just get the fingerprint scanner icon.

http://tiny.cc/e345ez

Assuming current behaviour of the plugin
In the link, you can see that we only get the fingerprint scanner icon and no popup. The user can still type the passcode, and click done. When user clicks done, and we navigate to another screen the authentication is still going on. The plugin doesn't provide any mechanism to cancel this authentication.

So this PR should solve the above problem.

I hope I was clear in explaining. :)

Very interesting.

Something is broken in this flow though.

The image shows FP and a PIN and user can choose to authenticate either way.

If both of these are not appearing due to this plugin then who is displaying the PIN entry method? That seems wrong. So, I assume both of these are appearing due to the plugin calling biometric.authenticate.

So, assuming biometric API is showing the PIN entry, if user enters PIN instead of FP, why would biometric.authenticate not return "success"? Why would we still expect a fingerprint scan?

I am OK adding this API because it is supported on Android. However the use case you are describing seems very very wrong to me. The app should not be responsible for cancelling authentication if the user already authenticated.

If you agree with me, could you take some time to research how the Android API is supposed to be used? In particular why would PIN entry not run success/failure callbacks? (https://github.com/flutter/plugins/blob/master/packages/local_auth/android/src/main/java/io/flutter/plugins/localauth/AuthenticationHelper.java#L142)

@mehmetf Thanks for your quick response.

The screenshot is not from my app but explains the exact scenario we need in our app. The screenshot is from oneplus's app locker screen.

The PIN entry is not due to the plugin. It's the app's screen having PIN entry, and biometric.authenticate just overlays the fingerprint icon on top of it for scanning (The user can still interact with other part of the screen though).

Because the biometric API is not responsible for showing the PIN entry, the biometric.authenticate doesn't know anything about it. There's no relation between them. :)

I'm not sure if the use case is wrong, but I have seen this flow in a lot of apps.
Just to add, the biometric API does show fingerprint dialog for physical fingerprint sensors. But it just shows fingerprint icon overlay for in-screen fingerprint sensors.

I see!

OK that makes more sense. So basically you app is offering two auth methods simultaneously and you want to cancel one if the user chooses the other one.

@mehmetf Exactly. :)

mehmetf · 2019-10-24T20:24:42Z

packages/local_auth/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java

        result.error(
            "no_fragment_activity",
            "local_auth plugin requires activity to be a FragmentActivity.",
            null);


There's a bug here for authInProgress. We might set it to true and never actually call authenticate.

Could you chage line 41 to: if (authInProgress.get())
then add to line 62: authInProgress.set(true);.

Since this method always runs in the UIThread, we technically don't need to use AtomicBoolean. You can also optionally change all of this to a normal boolean primitive.

Resolved. Not changing it to boolean primitive. Since, I don't want things to break. :)

mehmetf · 2019-10-24T20:27:19Z

packages/local_auth/android/src/main/java/io/flutter/plugins/localauth/LocalAuthPlugin.java

+  private void stopAuthentication(Result result) {
+    try {
+      if (authenticationHelper != null && authInProgress.get()) {
+        authenticationHelper.stopAuthentication();


Doing this means .authenticate method will never return? Will one of the AuthCompletionHandler methods be called when you stop authentication?

@mehmetf

The .authenticate will return false when we stop authentication.

Yes, the onFailure method will be called.

karan-rawal · 2019-10-30T12:34:36Z

@mehmetf Could you please merge this PR? I mean it's been a while. :|

mehmetf · 2019-10-30T13:59:21Z

I normally don't merge PRs :-) However, I don't see anyone else reviewing this so I think it is safe to say that it has been approved. Thanks for the contribution!

karan-rawal · 2019-10-31T06:51:37Z

I normally don't merge PRs :-) However, I don't see anyone else reviewing this so I think it is safe to say that it has been approved. Thanks for the contribution!

Thanks @mehmetf :) Any idea about when it'll be on pub?
I will definitely contribute more to flutter. :)

mehmetf · 2019-10-31T16:57:00Z

@collinjackson I can't publish this package for whatever reason. Could you please do the honors?

karan-rawal and others added 5 commits September 3, 2019 18:06

[local_auth] Added api to cancel android fingerprint authentication.

eeaa591

[local_auth] Updated readme.

a8a84f9

Merge branch 'master' into ApiToStopAuthentication

bf5c75c

Fixed Platform.isAndroid issue.

ca88f26

Sync with remote

ce85ba6

googlebot added the cla: yes label Sep 26, 2019

karan-rawal added 2 commits October 3, 2019 15:15

Sync with master

93fab07

Merge branch 'master' into ApiToStopAuthentication

aed3a2b

collinjackson requested a review from mehmetf October 24, 2019 19:39

mehmetf reviewed Oct 24, 2019

View reviewed changes

karan-rawal force-pushed the ApiToStopAuthentication branch 3 times, most recently from fca86c7 to b0ed6cd Compare October 25, 2019 07:40

karan-rawal requested a review from mehmetf October 25, 2019 07:42

[local_auth] Fixed auth PR reviews.

2ed420b

karan-rawal force-pushed the ApiToStopAuthentication branch from b0ed6cd to 2ed420b Compare October 25, 2019 07:45

mehmetf approved these changes Oct 25, 2019

View reviewed changes

Merge branch 'master' into ApiToStopAuthentication

0c31915

mehmetf merged commit ffb7903 into flutter:master Oct 30, 2019

matthewrice345 mentioned this pull request Nov 5, 2019

local_auth no cancel possibility for onscreen fingerprint flutter/flutter#35616

Closed

mormih pushed a commit to mormih/plugins that referenced this pull request Nov 17, 2019

[local_auth] Api to stop authentication (flutter#2111)

c5a9668

sungmin-park pushed a commit to sungmin-park/flutter-plugins that referenced this pull request Dec 17, 2019

[local_auth] Api to stop authentication (flutter#2111)

6c47947

[local_auth] Api to stop authentication #2111

[local_auth] Api to stop authentication #2111

Uh oh!

Conversation

karan-rawal commented Sep 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Checklist

Breaking Change

Uh oh!

karan-rawal commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karan-rawal Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karan-rawal Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karan-rawal Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karan-rawal Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karan-rawal commented Oct 30, 2019

Uh oh!

mehmetf commented Oct 30, 2019

Uh oh!

karan-rawal commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mehmetf commented Oct 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

karan-rawal commented Sep 26, 2019 •

edited

Loading

karan-rawal commented Oct 14, 2019 •

edited

Loading

karan-rawal Oct 25, 2019 •

edited

Loading

karan-rawal Oct 25, 2019 •

edited

Loading

karan-rawal Oct 25, 2019 •

edited

Loading

karan-rawal Oct 25, 2019 •

edited

Loading

karan-rawal commented Oct 31, 2019 •

edited

Loading