Yubico / yubikit-android

Yubico Mobile Android SDK - YubiKit
Apache License 2.0
103 stars 38 forks source link

How to detect permission rejected? #8

Closed bjvetter closed 4 years ago

bjvetter commented 4 years ago

In beta 4, there is no longer an onSessionError-like function and the onSessionReceived now has a new parameter "hasPermissions" that indicates whether the user has granted permissions.

With this change, I typically see onSessionReceived() with hasPermissions set to false when the key is first inserted. The user then sees the permissions dialog. If the user taps to grant the permission, I then see onSessionReceived() with hasPermissions set to true. All is good.

If by chance the user rejects the permission, I see another call to onSessionReceived with hasPermission set to false. At this point, I have no idea what to do. How do I know that they rejected the permission as opposed to the first callback where the user has yet to grant permission? Am I supposed to track various previous calls and somehow deduce that they said no to the permission request?

In beta 3, I receive the error callback on the session with a error message that said the permission was rejected. At least at that point, I could tell them to pull out the key and re-insert it to grant the permission. In this beta 4, I can't even know what message or error to show the user.

Is there some special API or other means to track the rejection of the permission? Or is there some other way I can handle the permission request myself instead of relying on the UsbSession and UsbSessionListener interface?

imakhalova commented 4 years ago

Hello @bjvetter , thank you so much for your feedback. When user rejects permissions you still going to have onSessionReceived with flag hasPermissions false. That way you know that device doesn't have accepted credentials and can provide user UI message that notifies about it. BTW if user doesn't accept permissions you can suggest him to retry - then just startUsbDiscovery again and it will prompt for permissions again. Search for permissionSnackBar in demo code. From user standpoint it's looks the same - whether user was prompted for permissions once or not yet. Let me know if this works for you.

If not I can provide another callback method something like onUsbPermissionsReceived(UsbSession session, boolean isGranted). That what will be invoked on permissions request results.

bjvetter commented 4 years ago

That is what I am kind of doing right now. If I get an onSessionReceived() call and hasPermission is false, I can put up a Snackbar that says "Hey dude{tte}, you need to accept the permission". But sometimes, that call back happens when the permission system dialog is present and the other time when it is "spent". In the first case, I can say "Tap on OK" or you can't authenticate. The second time, I could say - hey, you need to remove your key and reinsert it to restart this process.

I guess the problem is that I don't really have a single message that works for these two cases. I can try to cobble one up but so far have failed to come up with one that people like that is short enough to fit into a snackbar.

I have found a hacky workaround. I am using some Android lifecycle callbacks to deduce if permissions dialog is present and use a different message. It isn't 100% but it usually works.

That said, a callback that lets the app know that the user rejected (or accepted) the grant would have been much more correct. Also, is there a way to force the presentation of the permissions prompt again without removing/reinserting the key? Does shutting down the usbSession and restarting the discovery process result in the permission dialog? Or does it remember the previous answer?

imakhalova commented 4 years ago

I'm actually willing to provide you better API that won't need you to workaround it. I'm looking for that golden solution that will work for any scenario. What do you think about my proposal: have onSessionReceived() before asking for permissions and onPermissionsReceived() after response on permissions request? Something like that: https://github.com/Yubico/yubikit-android/pull/6/commits/8a960fcf82737e4859fc91c1b2dba7a696093c32

bjvetter commented 4 years ago

I believe that the onRequestPermissionsResult () interface should work for me. I believe the flow would be:

When onPermissionResult() with isGranted false, I can put up a dialog/snackbar that if the user wants to retry, I can re-establish the usbsession and start the process over again (onSessionReceived hasPermission will be false, they will see another permission dialog, ...). If they don't want to retry, I can just cancel out of my auth.

If so, I think this meets my needs.

imakhalova commented 4 years ago

Yes, that interface does make sense to me. Thank you for confirming. I added this change.

If you willing to give it a try before release I deployed this version to maven but under snapshots for now. Within your project you can try version 1.0.0-beta04-SNAPSHOT foryubikit module.

Using snapshots from maven central requires also to add this in your app build.gradle before naming dependencies

repositories {
    ....
    maven {
        url "https://oss.sonatype.org/content/repositories/snapshots"
    }
    ...
}

I'll prepare beta-05 next month

bjvetter commented 4 years ago

Great, I'll give it a try

bjvetter commented 4 years ago

Update/feature works - verified it with your snapshot build.