Investigate biometric prompt

brampitoyo commented 6 years ago

Why/User Benefit/User Problem

I want to make Focus even more private by restricting its access only to me, even as I share my phone to another person.

What / Requirements

A “Lock” feature with a native biometric prompt that opens and unlocks Firefox Focus, for users who choose to have it and run compatible Android devices.

Acceptance Criteria (how do I know when I’m done?)

Potentially a Settings menu item
Potentially a UI Tour prompt to enable “touch to unlock” (only for compatible devices)
Further behaviour (lock on exit, auto-lock over a period of time) to be discussed
Does not cover PIN code creation. Just biometric prompt.

brampitoyo commented 6 years ago

cc @ekager

brampitoyo commented 6 years ago

After some thinking, this feature won’t really be useful when Firefox Focus is newly open. There’s nothing to show, so there’s nothing to hide. Instead, it’s relevant only when switching back into Firefox Focus with one or more tabs already open.

With that in mind, the Settings menu item should be organised under Privacy & Security > Switching Apps.

To be consistent with Face/TouchID support on iOS (https://github.com/mozilla-mobile/focus-ios/issues/141), this item should be an on/off toggle, and titled:

Use fingerprint to unlock app Your fingerprint can unlock Firefox Focus if a URL is already opened in the app.

That’s the start.

If we want a finer control, it’s possible to either:

Make Firefox Focus lock itself after x period of time (maybe this is redundant, because your phone already does it?)
Or, make it so that fingerprint is not always required (so it’s not annoying).

Require fingerprint After [x period of time]

Where the options can simply show a time picker, or a list of possible times (1 minute, 15 minutes, 1 hour, etc.)

But be aware that this complicates the feature, and the most basic authentication may be enough to make Firefox Focus even more private than before.

Sdaswani commented 6 years ago

@bbinto @ekager suggested this is something good for @sblatz to work on for 7.0. It's a great addition to the privacy story while not being a release blocker for 7.0 (though knowing Sawyer, it will 100% get done :) ). We also have the edge cases from iOS to learn from. Thoughts?

bbinto commented 6 years ago

I'm down with that, so let's make it a P2/P3 --- effort/t-shirt size, @Sdaswani ?

Sdaswani commented 6 years ago

@sblatz probably needs some time to brush up on Android before he can comment on a T-shirt.

Sdaswani commented 6 years ago

@brampitoyo also needs to spend some time UXing it. @brampitoyo any ideas how long it will take you? Can it be done in next two weeks while @sblatz is learning Android?

brampitoyo commented 6 years ago

@Sdaswani I am hoping that the UX component for this project will be very minimal, because we will be using the native (Android system) biometric prompt.

If there’s any copy, we’ll need to come up with ones for the dialogue’s title, subtitle, description, and negative button.

And if there’s any new Settings menu item, it will be as discussed above:

Use fingerprint to unlock app Your fingerprint can unlock Firefox Focus if a URL is already opened in the app.

@BrianNJones expertise will help us a lot in coming up with these strings.

Sdaswani commented 6 years ago

Awesome, thanks @brampitoyo . Do we just need strings from @BrianNJones and then we can go ahead and start implementing?

bbinto commented 6 years ago

good feature for being a good Android citizen (@lwa-moz)

pocmo commented 6 years ago

@rpappalax @npark-mozilla Maybe you can investigate if we can test the fingerprint feature in automation somehow. That's a feature we could easily break without noticing.

nojunpark commented 6 years ago

So UI Test is possible on emulator I believe, it's just that we need to register fingerprint in the emulator beforehand. (in theory this is doable, but it involves opening settings, setting pin, and registering fake fingerprint) I hope google releases a better way to test fingerprint soon though.

brampitoyo commented 6 years ago

@Sdaswani That’s correct.

@BrianNJones, here’s our context:

In the upcoming Android P, apps can take advantage of a new API to show an authentication dialog that can responds to various types of biometric authentication: fingerprint, face and iris.

This dialogue contains 4 different elements that we can customise with our own strings, although we don’t have to show 2 of them.

Title (required). This doesn’t have to be the app name and can instead be a command.
- Example: Unlock Firefox Focus, Sign in, Sign into Firefox Focus.
- Example from elsewhere in Android: Verify purchase
Subtitle (optional)
Description (optional)
- Example: Confirm fingerprint/face/iris to continue, Tap your finger on your device’s fingerprint sensor to authenticate, Touch this fingerprint icon, Place your fingertip on the Home button to verify your identity, Please place your fingertip on the scanner to verify your identity
Negative button (required)
- Example: if it’s a shopping app, the negative button might say “Cancel”. If the app will terminate upon cancelling, then we might say something like “Quit” or “Close”.

If I were to come up with an authentication dialogue, I might come up with something like this:

Title: Unlock Firefox Focus Subtltle: none Description: none Negative button: Quit App

There are error scenarios that will result in a message, as well, but I’m not sure whether the system will supply us with those, or if we need to come up with our own.

sblatz commented 6 years ago

@brampitoyo under which header in the settings menu should this option reside? On iOS we created a new header called "SECURITY." Should we do the same here? If we did create this Security header, perhaps we could consolidate the "secure" mode used when app switching into the same section.

Also, should we be using "Use fingerprint to unlock app" as the title text if we're hooking into an API that can use other forms of biometrics (i.e. face unlock or iris unlock)?

brampitoyo commented 6 years ago

under which header in the settings menu should this option reside? On iOS we created a new header called "SECURITY."

It should belong together with “Stealth”, under the “Switching apps” section.

On iOS Settings, primary categories don’t have their own page. As a result, I had to resort to using the heading “Privacy & Security”. When we have more “Switching Apps” options on iOS, we’ll create that sub-page.

sblatz commented 6 years ago

In order to use the new API which allows for any biometrics to be used (iris, face, fingerprint), we need to bump the API version we're using (currently 27, needs to be 28). I tried doing this, but it leads to compilation errors. Perhaps this feature should be implemented as just fingerprint scanning for now, as this is exposed on API 27, unless fixing these compilation errors is trivial.

@boek @ekager would it take long to fix these errors? cc: @Sdaswani

Sdaswani commented 6 years ago

@boek @ekager did we have a plan to upgrade to 27?

pocmo commented 6 years ago

@boek @ekager did we have a plan to upgrade to 27?

We already support 27 (Android 8.1). The SDK for API 28 (Android 9) was just released some days ago and Android 9 / P hasn't been released yet. :)

colintheshots commented 6 years ago

There's a BiometricPrompt Compat library coming according to the official Android developer blog. All the blog post says is Oreo and lower is supported. It doesn't list a minimum API level. I'd build to that whenever it's available.

https://android-developers.googleblog.com/2018/06/better-biometrics-in-android-p.html

sblatz commented 6 years ago

@colintheshots is that the same as the API found here? This one explicitly calls out API 28 as being the required version. Is there something I'm missing?

BrianNJones commented 6 years ago

@brampitoyo @Sdaswani , thanks for all the good info/context. (Digging in after being out last week.) Bram, couple questions:

Are the Login screenshots you provided the screenshots a user would see at setup of the biometric login, or are they what a user would see each time Focus is opened? (Asking mostly b/c I'm confused by the use of "authorise" here...seems to indicate a first step, not an action that's repeated.)
If it's the former, do we have examples of what initial biometric setup looks like? Thanks much!

colintheshots commented 6 years ago

@sblatz No, it would likely be named BiometricPromptCompat or be a different class in the new support libraries. These things are so new, they haven't released the compat version yet. The image seems to imply they want you to use BiometricPrompt if API version is >=28 or the unreleased BiometricPromptCompat if not.

sblatz commented 6 years ago

@ekager mentioned that she would investigate the workload of bumping to version 28. If it ends up being a lot of work, we can just use the FingerPrintManager API (version >=23, but deprecated in 28) until the BiometricPromptCompat releases at some unknown future date.

Per my discussion with @colintheshots, it seems like trying to implement BiometricPrompt right now would be premature anyway as Android P will not reach most users for a long time; because of this, my view is that we should move forward with Fingerprint API to hit the most amount of users right now.

@Sdaswani thoughts?

Sdaswani commented 6 years ago

Sounds like FingerPrintAPI is the way to go @sblatz - I do want @colintheshots to confirm that when we move to API Level 28 the transition to the BiometricPrompt is not a heavy lift though.

brampitoyo commented 6 years ago

@BrianNJones, answering your questions below.

Are the Login screenshots you provided the screenshots a user would see at setup of the biometric login, or are they what a user would see each time Focus is opened

It’s the latter. The login screenshot is what the user would see before Focus.app is opened.

This behaviour is annoying if the user doesn’t understand the privacy–convenience trade-off, but can be very useful if required. Like FaceID on iOS, we will turn it off by default.

To set it up, you’d go to Settings → Privacy & Security → Switching Apps (subheading). Then, simply toggle ON an item.

Like FaceID on iOS, this item can say something similar to:

What do you think?

BrianNJones commented 6 years ago

Thanks Bram. Let me try this again (we've had connection hiccups all week and I'm not sure what's actually sent and what hasn't.)

I think we'd need: Title: Unlock Firefox Focus Desc: Touch the fingerprint sensor to continue. Negative button: Cancel

On Tue, Jul 10, 2018 at 7:27 PM, Bram Pitoyo notifications@github.com wrote:

@BrianNJones https://github.com/BrianNJones, answering your questions below.

Are the Login screenshots you provided the screenshots a user would see at setup of the biometric login, or are they what a user would see each time Focus is opened

It’s the latter. The login screenshot is what the user would see before Focus.app is opened.

This behaviour is annoying if the user doesn’t understand the privacy–convenience trade-off, but can be very useful if required. Like FaceID on iOS, we will turn it off by default.

To set it up, you’d go to Settings → Privacy & Security → Switching Apps (subheading). Then, simply toggle ON an item.

Like FaceID on iOS, this item can say something similar to:

https://user-images.githubusercontent.com/51007/42544262-88894a58-8504-11e8-9cbe-5e6a1dd9ec62.png

What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/focus-android/issues/2712#issuecomment-404006876, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad38l9FVtENk7iW_dLq_vDmCQ0sYENudks5uFUaLgaJpZM4Udjxy .

brampitoyo commented 6 years ago

Thanks, @BrianNJones. With the string written, this issue is now ready to implement.

sblatz commented 6 years ago

@brampitoyo @BrianNJones On Focus iOS the "cancel" button is actually a "New Session" button, as if the user fails to authenticate (or doesn't care to), they can simply begin a new session. I included an example of this below. Should we use "New Session" here as well, or should I move forward with it being "Cancel"?

BrianNJones commented 6 years ago

@brampitoyo @sblatz , I believe the intent here (Android) is that the button act as a negative. So: Cancel is appropriate here.

brampitoyo commented 6 years ago

On Focus iOS the "cancel" button is actually a "New Session" button, as if the user fails to authenticate (or doesn't care to), they can simply begin a new session

@sblatz I assume that pressing the negative button will not erase the session, but will simply close the dialogue and return user to the homescreen, is this correct?

This is the least destructive action. The user’s session is always preserved when the negative button is pressed, unless – let’s say – the user has failed authentication too many times.

On iOS, I also recommend the same behaviour. When Face ID fails, we don’t need to show any button in the dialogue. Simply ask the user to try again (the UI already does this).

So, the only way to force a new session is by failing authentication too many times. It’s similar to how many security systems work: fail too many times, and your session is erase. But there’s no way to manually erase a session.

What do you think?

oliviabrown9 commented 6 years ago

But there’s no way to manually erase a session.

@brampitoyo For iOS, I think that we're setting a precedent in other places in the upcoming release that erase is not a protected feature (users don't have to authenticate to erase in the quick action from the home screen or from Siri). I personally think it's fair to have an equivalent erase option with the new session option. I'm also not sure I understand where "cancel" would take users.

sblatz commented 6 years ago

How should we handle back button presses when the authentication prompt is displayed? Should that be ignored or should we just clear the session and put them on the home screen? @brampitoyo

sblatz commented 6 years ago

Per meeting with @brampitoyo, the back button should just send the user back to the home screen.

@oliviabrown9 to ask for updated "new session" string and follow up with ticket for Android and iOS Focus.

brampitoyo commented 6 years ago

@BrianNJones Interestingly, it turned out that “Cancel” isn’t a desired action for us in this specific situation.

When you’re about to authenticate a purchase, tapping “Cancel” should revoke that purchase.

But when you’re about to unlock Focus and see the word “Cancel”, you’ll assume that you’d be returned to the home screen.

But if you return to the home screen, then that doesn’t make sense:

You can reopen the app and try to re-authenticate. But you can just keep re-trying on the initial dialogue, without needing to press “Cancel” and then reopen the app.
Or, you can erase your browsing history by using the notification panel and action menu. When your browsing history is erased, no more authentication is needed (because there’s no tab to show).

So, unlike in the purchase scenario, the “Cancel” action isn’t useful. What we should do instead is “Erase”, so that the user can just restart the browsing session if the authentication fails, without needing to go to the notification panel or action menu.

And so, the negative button copy that we need to come up with, is probably something that will inform the user “If you tap this, your data is erased and you start afresh”.

In other words, maybe the “Cancel” button should be called “Erase”?

I hope I have explained my point clearly! Maybe @oliviabrown9 can help summarise? I’d also be happy to schedule a meeting to talk about this issue, and how it applies to iOS.

BrianNJones commented 6 years ago

@brampitoyo @sblatz , thanks again for the amazing context. Bram, do you think "Erase" is predictive enough (user will understand what happens after tapping)? I'm not sure; it seems more "final" than "fresh start." Does "Start New Session" or "New Session" do the same thing, you think?

sblatz commented 6 years ago

@brampitoyo Since Focus should obscure content until the user authenticates, should we automatically turn on Stealth mode when the user enables fingerprint auth? Otherwise, the authentication doesn't really do much since you can still see it in multitasking before you authenticate.

Not sure if this requires some explanation to the user if we decide to lock the toggle to the "on" position while biometric authentication is enabled.

cc: @Sdaswani, @bbinto

sblatz commented 6 years ago

Here is a (crude) demo of the current functionality. I apologize for the off-device recording, since it's in "secure mode" screen capture doesn't work!

Demo

@brampitoyo what are your thoughts on this UX/UI? This is the UI recommended from the system design documents for devices before Android P. In Android P I believe we should be able to easily display a built in prompt similar to the one you attached at the top of this post, but until that API exists, does this work?

brampitoyo commented 6 years ago

@BrianNJones

…do you think "Erase" is predictive enough (user will understand what happens after tapping)? I'm not sure; it seems more "final" than "fresh start." Does "Start New Session" or "New Session" do the same thing…

I think that “Start New Session” or “New Session” is more understandable in this particular scenario. My only worry is in having a different word to convey a concept that’s practically similar to “Erase” (or “Erase and Open”), if that makes sense?

Would something along the lines of “Erase and Start New Session)” suffice, or would it be too long and hard to localise?

Anyway, the wording that @sblatz proposed, “New Session”, is perfectly functional. It’s also already deployed on the demo, and I found it understandable. If you have no objection, let’s go with that.

@sblatz

Since Focus should obscure content until the user authenticates, should we automatically turn on Stealth mode when the user enables fingerprint auth? Otherwise, the authentication doesn't really do much…

I agree. Stealth mode should be enabled when biometrics is turned on.

Actually, I also wonder if we should enable Stealth mode by default on all future versions? We have two good reasons to do so:

Stealth mode is already enabled by default on Focus for iOS, with no way to turn it off
It keeps our users more secure

We can also consider taking out the Stealth mode settings menu item entirely, but I worry that some users do want to see their currently opened tabs, especially if they use Firefox Focus as a default browser. But you can argue that this goes against our privacy-first design philosophy.

Maybe this is a discussion for another issue. Thoughts?

Otherwise, the demo video you posted looks good, and contains exactly the behaviour I’ve expected out of this feature. Thanks very much!

sblatz commented 6 years ago

@brampitoyo definitely in agreement about your thoughts on stealth mode as far as it being on by default. I don't see the harm in allowing users to disable it, though, as it could be helpful for those who have it as a default browser!

pocmo commented 6 years ago

Actually, I also wonder if we should enable Stealth mode by default on all future versions? We have two good reasons to do so:

FYI: When we launched Focus it was ON by default. This changed later in #1104 (See also #718). One of the reasons was that this system feature also disables the ability to take screenshots.

BrianNJones commented 6 years ago

@brampitoyo @sblatz , agree! "New Session" is reasonably clear/unambiguous and has the advantage of already being "in market."

sblatz commented 6 years ago

@pocmo @bram I forgot that we wouldn't be able to take screenshots if stealth mode is on. Instead, what if we emulated what iOS does, and just create a grey view the obscures the webview when the app is backgrounded when fingerprint auth is enabled and have "Secure Mode" as a separate option?

brampitoyo commented 6 years ago

@sblatz @pocmo

[…] what if we emulated what iOS does, and just create a grey view the obscures the webview when the app is backgrounded when fingerprint auth is enabled.

I like this idea.

We know that Stealth mode controls two things:

Obscuring webview
Blocking screenshots from being taken

If the API supports biometrics, then we show the biometrics setting menu item.

If biometrics is ON, then webview should always be obscured. In this scenario, Stealth mode will only control screenshots blocking.

If biometrics is OFF, or if the phone doesn’t support biometrics, then Stealth mode will control both webview and screenshots blocking.

With this new behaviour, it makes sense that we should describe stealth mode a little differently.

When stealth controls webview and screenshots, it should be described as:

Stealth Hide webpages when switching apps, block taking screenshots of webpages

When stealth only controls screenshots, it should be described like this:

Stealth Block taking screenshots of webpages

But I worry that changing menu item descriptions on the fly will cause layout problems and confusion (“why does this setting sometimes mean A, and sometimes mean B?”).

Thoughts?

pocmo commented 6 years ago

what if we emulated what iOS does, and just create a grey view the obscures the webview when the app is backgrounded when fingerprint auth is enabled and have "Secure Mode" as a separate option?

This unfortunately doesn't work because Android takes the screenshot before the app gets backgrounded (and we know about it). That's something we tried too back then! :)

pocmo commented 6 years ago

With this new behaviour, it makes sense that we should describe stealth mode a little differently.

I think we went through some iterations (although I do not remember the details anymore). Afaik an early iteration mentioned screenshots too and the setting ended up to be complicated to be understood by users.

sblatz commented 6 years ago

@pocmo Thanks for giving some background on this @brampitoyo It appears that there is no way to accomplish this on Android. Chrome, 1Password, etc. all have the exact same behavior (in order to hide on the app switcher, you lose the ability to screenshot). I think the current implementation of forcing the stealth mode flag will have to suffice until the Android API allows us to implement a different solution.

(Of note: it seems like there was a huge blow-back from users when Chrome implemented this for incognito mode; we should be careful with whatever decision we make).

cc: @Sdaswani

Sdaswani commented 6 years ago

Yes, let's land this PR so QA can hack at it.

brampitoyo commented 6 years ago

@sblatz

It appears that there is no way to accomplish this on Android […] in order to hide on the app switcher, you lose the ability to screenshot […] it seems like there was a huge blow-back from users when Chrome implemented this for incognito mode…

If this is the case, then let’s clarify the biometrics string:

Use [fingerprint/face] to unlock app Your [fingerprint/face] can unlock Focus if a URL is already open in the app. Stealth will be turned on.

Biometrics should appear under the Switching Apps subcategory under Privacy & Security. It will appear above Stealth. This positioning will help clarify the relationship between the two.

We should also clarify the stealth string, to explain that screenshots will be blocked:

Stealth Hide webpages when switching apps and block taking screenshots

Lastly, when Stealth is auto-enabled (ie. cannot be turned off unless the user turns biometrics off), then it should be greyed out (inactive).

When everything is brought together, the Settings page looks like this:

What do you think?

ekager commented 6 years ago

Closed by #2981

mozilla-mobile / focus-android