Biometrics lock (Lock app with FaceID/TouchID/iOS Password) #2398

bgoncal · 2023-08-17T19:55:37Z

Summary

Concept for locking the HA app using iOS biometrics. (FaceID wasn't working on my simulator, thats why the video shows password instead).

Pending:

Localised copy
Better error logging
Remove print statements

Screenshots

codecov · 2023-08-20T12:55:35Z

Codecov Report

Attention: 474 lines in your changes are missing coverage. Please review.

Comparison is base (8f7f0ec) 28.37% compared to head (eec44b0) 28.16%.
Report is 3 commits behind head on master.

❗ Current head eec44b0 differs from pull request most recent head 5dbb72e. Consider uploading reports for the commit 5dbb72e to get more accurate results

Files	Patch %	Lines
...dentialsSharing/ThreadCredentialsSharingView.swift	0.00%	117 Missing ⚠️
Sources/App/WebView/WebViewController.swift	0.00%	85 Missing ⚠️
...thentication/BiometricsAuthenticationService.swift	0.00%	82 Missing ⚠️
...alsSharing/ThreadCredentialsSharingViewModel.swift	38.46%	24 Missing ⚠️
...ces/App/Settings/DebugSettingsViewController.swift	0.00%	21 Missing ⚠️
...rces/Shared/DesignSystem/Components/CardView.swift	0.00%	21 Missing ⚠️
...App/Settings/Security/SecurityViewController.swift	0.00%	18 Missing ⚠️
...rces/Shared/DesignSystem/Components/HAButton.swift	0.00%	14 Missing ⚠️
...es/Shared/LocalAuthentication/BiometricsView.swift	0.00%	14 Missing ⚠️
Sources/Shared/Environment/MatterWrapper.swift	0.00%	12 Missing ⚠️
... and 14 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2398      +/-   ##
==========================================
- Coverage   28.37%   28.16%   -0.21%     
==========================================
  Files         273      290      +17     
  Lines       30284    30834     +550     
==========================================
+ Hits         8593     8685      +92     
- Misses      21691    22149     +458

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zacwest · 2023-08-20T22:18:19Z

Sources/App/WebView/LocalAuthentication/AuthenticationService.swift

+    }
+
+    private func authenticate(policy: LAPolicy) {
+        context.evaluatePolicy(policy, localizedReason: "Authentication required") { approved, error in


the hard part here is also blocking the ui while this should be happening, and e.g. in the app switcher. how would you imagine this to be implemented?

Usually most apps blur the UI using and overlay UIView, what do you think? I'd go for that, although at least in the simulator I wasn't able to dismiss iOS biometric/password screen (which is visible in the video), so maybe it is already doing that.

Something like this, I just added to the PR, but Im not 100% sure the App Switcher will show this overlay if the user has already authenticated.

Sources/App/WebView/WebViewController.swift

Sources/App/Settings/SettingsRootDataSource.swift

home-assistant · 2023-08-20T22:19:56Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

zacwest · 2023-11-16T03:59:13Z

Sources/App/Resources/en.lproj/Localizable.strings

@@ -410,6 +410,9 @@ Home Assistant is free and open source home automation software with a focus on
 "settings_details.general.visibility.options.dock_and_menu_bar" = "Dock and Menu Bar";
 "settings_details.general.visibility.options.menu_bar" = "Menu Bar";
 "settings_details.general.visibility.title" = "Show App In…";
+"settings_details.general.security.title" = "Security";
+"settings_details.general.security.action" = "Enable biometric lock";
+"settings_details.general.security.footer" = "You will be required to authenticate with biometrics everytime you open the app.";


every time

Sources/App/WebView/WebViewController.swift

zacwest · 2023-11-16T04:01:41Z

Sources/Shared/LocalAuthentication/AuthenticationService.swift

+            Current.Log
+                .error(
+                    [
+                        "Failed to authenticate with biometrics": "User context can't evaluate policy for device ownership",
+                    ]
+                )


what should we tell the user if they fail to do this? should we have on-screen ui that says an error?

If the user can't authenticate neither with biometrics or password then the user is locked out, I would say this is very edge case since it's credentials are the same as the iOS/macOS device. Any suggestion?

Sources/Shared/LocalAuthentication/AuthenticationService.swift

zacwest · 2023-11-16T04:04:34Z

Sources/App/Settings/SettingsRootDataSource.swift

@@ -95,6 +97,16 @@ enum SettingsRootDataSource {
        }
    }

+    private static func security() -> SettingsButtonRow {
+        SettingsButtonRow {


how does this work on catalyst? should we hide it or make it available -- if the latter, how do we deal with e.g. the settings scene?

In catalyst it was a bit trickier, because of the "enter background/foreground" thats happens all the time even with the touchID dialog, I treated it with an extra logic to only reauthenticate when the user has reply enter background.

When the user open settings scene, the main web view scene locks again until the user unlocks it.

Screen.Recording.2023-11-16.at.16.33.08.mov

RPReplay_Final1700149090.MP4

…r location

Sources/Shared/LocalAuthentication/BiometricsAuthenticationViewController.swift

Sources/WatchApp/Assets.xcassets/WatchIcon.appiconset/Contents.json

Sources/App/Settings/Security/SecurityViewController.swift

zacwest · 2023-11-16T17:43:36Z

Sources/App/WebView/WebViewWindowController.swift

+// MARK: - Biometrics lock
+
+extension WebViewWindowController {
+    private func listenForBiometricsLockRelatedEvents() {


what happens if the user gets signed out (e.g. remotely, due to token) while this is happening?

How can I easily reproduce that? I think the overlay will stay there, but it's nice to verify

go into your user profile & delete the token for your device in your profile. you can do it from the same machine or other.

if the overlay stays, it may be worth killing it off since there's nothing it's protecting (if it's the last account) and may block signing into something new. there's no way to get to settings during onboarding.

Tested. Indeed the "lock screen" was still being shown even with no servers. So I updated it in a way that the user needs to authenticate one last time after the token expires, and then the biometric lock will be disabled and only reenabled if the user sets up a new server and manually enables the toggle.

Sources/Shared/LocalAuthentication/BiometricsAuthenticationService.swift

…r location

…to Biometric-lock-concept

# Conflicts: # Sources/App/Resources/Assets.xcassets/Logo.imageset/Contents.json # Sources/WatchApp/Assets.xcassets/WatchIcon.appiconset/Contents.json # Sources/WatchApp/Assets.xcassets/WatchIcon.beta.appiconset/Contents.json # Sources/WatchApp/Assets.xcassets/WatchIcon.dev.appiconset/Contents.json

Sources/App/Settings/SettingsViewController.swift

zacwest · 2023-12-06T04:13:22Z

Sources/App/WebView/WebViewWindowController.swift

+// MARK: - Biometrics lock
+
+extension WebViewWindowController {
+    private func listenForBiometricsLockRelatedEvents() {


go into your user profile & delete the token for your device in your profile. you can do it from the same machine or other.

if the overlay stays, it may be worth killing it off since there's nothing it's protecting (if it's the last account) and may block signing into something new. there's no way to get to settings during onboarding.

Sources/Shared/DesignSystem/Constants/HASpacing.swift

Sources/Shared/LocalAuthentication/BiometricsAuthenticationService.swift

HomeAssistant.xcodeproj/project.pbxproj

Sources/App/Resources/Info.plist

zacwest

i think things seem good, but i am a little worried about the 'protect' part and how it works with multiple windows happening

zacwest · 2023-12-07T04:58:46Z

Sources/App/Resources/et.lproj/Localizable.strings

+"thread.credentials.network_name_title" = "Network Name";
+"thread.credentials.network_key_title" = "Network Key";
+"thread.credentials.border_agent_id_title" = "Border Agent ID";
+"thread.credentials.share_credentials_button_title" = "Share credential with Home Assistant";
+"thread.credentials.screen_title" = "Thread Credentials";


zacwest · 2023-12-07T05:01:14Z

Sources/Shared/LocalAuthentication/BiometricsAuthenticationService.swift

+        guard let window = controller.view.window,
+              previousRootViewController == nil else { return }
+
+        self.window = window


what happens if there are multiple windows open? e.g. multi-scene support on iPad.

i think you probably want to either collate all the windows during the lock phase, or make this chain off the WebViewWindowController itself, but synchronize the unlock status

I think I will start over with this implementation, I liked your idea of sync status. Another question... should we allow shortcut actions if the app is locked?

I think it becomes really hard to do locking if we disallow extensions to do work. Most of them do not support being able to prompt the user for permission.

We could either:

tell the user it won't lock extensions

disallow extensions entirely if lock is enabled (where we cannot prompt)

allow the user to choose which extensions should be allowed

Maybe we could add more control in app but I think the first one sounds easiest for now.

bgoncal · 2024-01-05T00:04:00Z

Closing PR, I will start over with a better foundation for locking the app

home-assistant bot added the cla-signed label Aug 17, 2023

zacwest requested changes Aug 20, 2023

View reviewed changes

home-assistant bot marked this pull request as draft August 20, 2023 22:19

bgoncal marked this pull request as ready for review August 21, 2023 20:51

home-assistant bot requested a review from zacwest August 21, 2023 20:51

bgoncal force-pushed the Biometric-lock-concept branch 2 times, most recently from cf3a0d3 to e300ba5 Compare November 7, 2023 13:18

zacwest requested changes Nov 16, 2023

View reviewed changes

home-assistant bot marked this pull request as draft November 16, 2023 04:02

zacwest reviewed Nov 16, 2023

View reviewed changes

bgoncal added 8 commits November 16, 2023 11:20

Biometrics lock concept

1afab5a

Improvements on authentication handler and protection overlay

a7564fb

Add localized placeholders

1d757ca

Lint autocorrect

c56f638

PR improvements and macOS taking in consideration

1335665

Linter autocorrect

f85e6ae

Add missing self reference

9fd70fa

Fix typo and move biometrics logic

5850f35

bgoncal force-pushed the Biometric-lock-concept branch from 3b3a134 to 5850f35 Compare November 16, 2023 10:21

Added new UI for the "lock" screen, moved some code around to a prope…

ce1f140

…r location

bgoncal marked this pull request as ready for review November 16, 2023 15:47

home-assistant bot requested a review from zacwest November 16, 2023 15:47

bgoncal changed the title ~~Biometrics lock concept (Lock app with FaceID/TouchID/iOS Password)~~ Biometrics lock (Lock app with FaceID/TouchID/iOS Password) Nov 16, 2023

zacwest requested changes Nov 16, 2023

View reviewed changes

home-assistant bot marked this pull request as draft November 16, 2023 17:46

bgoncal added 3 commits November 28, 2023 15:59

Biometrics lock concept

96cdfac

Improvements on authentication handler and protection overlay

fc15eac

Add localized placeholders

b728292

bgoncal added 4 commits November 28, 2023 15:59

Linter autocorrect

2a9ddda

Add missing self reference

1f0b692

Fix typo and move biometrics logic

e051dd4

Added new UI for the "lock" screen, moved some code around to a prope…

63c9c46

…r location

bgoncal force-pushed the Biometric-lock-concept branch from ce1f140 to 63c9c46 Compare November 28, 2023 14:59

Merge branch 'Biometric-lock-concept' of github.com:bgoncal/HA-iOS in…

8c656d3

…to Biometric-lock-concept

bgoncal force-pushed the Biometric-lock-concept branch 2 times, most recently from b1b369b to 8c656d3 Compare December 5, 2023 01:42

bgoncal added 5 commits December 4, 2023 23:42

Removed mac implementation and moved to SwiftUI for iOS

75982af

Present on top most controller

27cf8ac

Linter

97839c3

Present biometric lock on window level

77e5077

bgoncal marked this pull request as ready for review December 5, 2023 13:20

home-assistant bot requested a review from zacwest December 5, 2023 13:20

bgoncal self-assigned this Dec 5, 2023

zacwest requested changes Dec 6, 2023

View reviewed changes

home-assistant bot marked this pull request as draft December 6, 2023 04:18

bgoncal added 5 commits December 6, 2023 09:14

Remove unneeded check

27e2a63

Improve logic to hide security toggle for mac

2bc8f60

Use camel case for spacing values

072af4e

Auto disable biometric lock when delete all servers

6674456

Re-add hebrew and estonian languages

eec44b0

bgoncal marked this pull request as ready for review December 6, 2023 13:45

home-assistant bot requested a review from zacwest December 6, 2023 13:45

zacwest requested changes Dec 7, 2023

View reviewed changes

home-assistant bot marked this pull request as draft December 7, 2023 05:02

Merge branch 'master' into Biometric-lock-concept

5dbb72e

bgoncal closed this Jan 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Biometrics lock (Lock app with FaceID/TouchID/iOS Password) #2398

Biometrics lock (Lock app with FaceID/TouchID/iOS Password) #2398

bgoncal commented Aug 17, 2023 •

edited

Loading

codecov bot commented Aug 20, 2023 •

edited

Loading

zacwest Aug 20, 2023

bgoncal Aug 21, 2023

bgoncal Aug 21, 2023

home-assistant bot commented Aug 20, 2023

zacwest Nov 16, 2023

zacwest Nov 16, 2023

bgoncal Nov 16, 2023

zacwest Nov 16, 2023

bgoncal Nov 16, 2023

zacwest Nov 16, 2023

bgoncal Dec 5, 2023

zacwest Dec 6, 2023

bgoncal Dec 6, 2023

zacwest Dec 6, 2023

zacwest left a comment

zacwest Dec 7, 2023

zacwest Dec 7, 2023

bgoncal Dec 7, 2023

zacwest Dec 8, 2023

bgoncal commented Jan 5, 2024

Biometrics lock (Lock app with FaceID/TouchID/iOS Password) #2398

Biometrics lock (Lock app with FaceID/TouchID/iOS Password) #2398

Conversation

bgoncal commented Aug 17, 2023 • edited Loading

Summary

Screenshots

codecov bot commented Aug 20, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Aug 20, 2023

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

zacwest 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

bgoncal commented Jan 5, 2024

bgoncal commented Aug 17, 2023 •

edited

Loading

codecov bot commented Aug 20, 2023 •

edited

Loading