Enabling biometry without password during sync (#17960)

* feat: added migration for the keychain hashed password

* feat: added sync biometry without password entry

* fix: biometry typo from develop

* ref: moved migration side-effects outside the event

* ref: some renaming for keychain migration

* ref: addressed @cammellos' review comments

* ref: removed unnecessary anon fn

* fix: addressed @ilmotta's review comments

* ref: removed theme from enable-biometrics

* ref: addressed J-Son89's review comments

* test: added tests for mask-data and hash-masked-password

* test: added schema to hash-masked-password and fixed test

* fix: forgot the threading

* ref: improved the masked data schema

* fix: no biometry error when canceled by user

* fix: biometry error wasn't propagated during login

* fix: alert dismiss button not passed properly

* fix: show biometrics NOT_ENROLLED error only once

* lint: removed unused require
This commit is contained in:
Lungu Cristian 2023-11-30 13:40:29 +02:00 committed by GitHub
parent d412fa3c63
commit ed270b2162
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 236 additions and 89 deletions

View File

@ -58,14 +58,16 @@
(defn save-credentials (defn save-credentials
"Stores the credentials for the address to the Keychain" "Stores the credentials for the address to the Keychain"
[server username password callback] ([server username password]
(save-credentials server username password identity))
([server username password callback]
(-> (.setInternetCredentials ^js react-native-keychain (-> (.setInternetCredentials ^js react-native-keychain
(string/lower-case server) (string/lower-case server)
username username
password password
keychain-secure-hardware keychain-secure-hardware
keychain-restricted-availability) keychain-restricted-availability)
(.then callback))) (.then callback))))
(defn get-credentials (defn get-credentials
"Gets the credentials for a specified server from the Keychain" "Gets the credentials for a specified server from the Keychain"

View File

@ -22,7 +22,7 @@
(vector (vector
action-button action-button
dismiss-button) dismiss-button)
dismiss-button) (vector dismiss-button))
(when on-dismiss (when on-dismiss
{:cancelable false}))))) {:cancelable false})))))

View File

@ -2,9 +2,11 @@
(:require (:require
[native-module.core :as native-module] [native-module.core :as native-module]
[re-frame.core :as re-frame] [re-frame.core :as re-frame]
[react-native.async-storage :as async-storage]
[react-native.platform :as platform] [react-native.platform :as platform]
[react-native.touch-id :as touch-id] [react-native.touch-id :as touch-id]
[status-im2.common.keychain.events :as keychain] [status-im2.common.keychain.events :as keychain]
[taoensso.timbre :as log]
[utils.i18n :as i18n] [utils.i18n :as i18n]
[utils.re-frame :as rf])) [utils.re-frame :as rf]))
@ -41,16 +43,60 @@
{:db (assoc db :biometric/supported-type supported-type)}) {:db (assoc db :biometric/supported-type supported-type)})
(rf/defn show-message (rf/defn show-message
{:events [:biometric/show-message]}
[_ code] [_ code]
(let [content (if (#{"NOT_AVAILABLE" "NOT_ENROLLED"} code) (let [handle-error? (and code
(not (contains? #{"USER_CANCELED" "USER_FALLBACK"} code)))
content (if (#{"NOT_AVAILABLE" "NOT_ENROLLED"} code)
(i18n/label :t/grant-face-id-permissions) (i18n/label :t/grant-face-id-permissions)
(when-not (or (= code "USER_CANCELED") (= code "USER_FALLBACK")) (i18n/label :t/biometric-auth-error {:code code}))]
(i18n/label :t/biometric-auth-error {:code code})))] (when handle-error?
(when content
{:utils/show-popup {:utils/show-popup
{:title (i18n/label :t/biometric-auth-login-error-title) {:title (i18n/label :t/biometric-auth-login-error-title)
:content content}}))) :content content}})))
(defn- supress-biometry-error-key
[key-uid]
(keyword (str "biometric/supress-not-enrolled-error-" key-uid)))
;; NOTE: if the account had biometrics registered, but it's not enrolled at the moment,
;; we should show the error message only once and supress further "NOT_ENROLLED" errors
;; until biometry is enrolled again. Note that we can only know that when :biometric/authenticate
;; is dispatched and fails with "NOT_ENROLLED", since :biometric/get-supported-biometric-type
;; only tells us what kind of biometric is available on the device, but it doesn't know of its
;; enrollment status.
(re-frame/reg-fx
:biometric/supress-not-enrolled-error
(fn [[key-uid dispatch-event]]
(let [storage-key (supress-biometry-error-key key-uid)]
(-> (async-storage/get-item storage-key identity)
(.then (fn [item]
(when (not item)
(rf/dispatch dispatch-event)
(async-storage/set-item! storage-key true))))
(.catch (fn [err]
(log/error "Couldn't supress biometry NOT_ENROLLED error"
{:key-uid key-uid
:event :biometric/supress-not-enrolled-error
:error err})))))))
;; NOTE: when biometrics is re-enrolled, we erase the flag in async-storage to assure
;; the "NOT_ENROLLED" error message will be shown again if biometrics is un-enrolled
;; in the future.
(re-frame/reg-fx
:biometric/reset-not-enrolled-error
(fn [key-uid]
(let [storage-key (supress-biometry-error-key key-uid)]
(-> (async-storage/get-item storage-key identity)
(.then (fn [supress?]
(when supress?
(async-storage/set-item! storage-key nil))))
(.catch (fn [err]
(log/error "Couldn't reset supressing biometry NOT_ENROLLED error"
{:key-uid key-uid
:event :biometric/reset-not-enrolled-error
:error err})))))))
(re-frame/reg-fx (re-frame/reg-fx
:biometric/authenticate :biometric/authenticate
(fn [options] (fn [options]

View File

@ -49,16 +49,15 @@
(defn save-auth-method! (defn save-auth-method!
[key-uid method] [key-uid method]
(keychain/save-credentials (-> (keychain/save-credentials
(str key-uid "-auth") (str key-uid "-auth")
key-uid key-uid
method method)
#(when-not % (.catch (fn [err]
(log/error (log/error "Failed to save auth method in the keychain"
(str "Error while saving auth method." {:error err
" " :key-uid key-uid
"The app will continue to work normally, " :auth-method method})))))
"but you will have to login again next time you launch it.")))))
(re-frame/reg-fx (re-frame/reg-fx
:keychain/save-auth-method :keychain/save-auth-method
@ -79,27 +78,59 @@
(if can-save? (if can-save?
(keychain/get-credentials (keychain/get-credentials
(str key-uid "-auth") (str key-uid "-auth")
#(callback (if % (oops/oget % "password") auth-method-none))) (fn [value]
(callback (if value (oops/oget value "password") auth-method-none))))
(callback nil)))))) (callback nil))))))
(defn save-user-password! (defn save-user-password!
[key-uid password] [key-uid password]
(keychain/save-credentials key-uid key-uid (security/safe-unmask-data password) #())) (keychain/save-credentials key-uid key-uid (security/safe-unmask-data password) #()))
(defn get-user-password!
[key-uid callback]
(keychain/get-credentials key-uid
(fn [value]
(callback (when value
(-> value
(oops/oget "password")
(security/mask-data)))))))
(re-frame/reg-fx (re-frame/reg-fx
:keychain/get-user-password :keychain/get-user-password
(fn [[key-uid callback]] (fn [[key-uid callback]]
(keychain/get-credentials (get-user-password! key-uid callback)))
key-uid
#(if % (callback (security/mask-data (oops/oget % "password"))) (callback nil)))))
(rf/defn get-user-password (rf/defn get-user-password
[_ key-uid callback] [_ key-uid callback]
{:keychain/get-user-password [key-uid callback]}) {:keychain/get-user-password [key-uid callback]})
(defn- password-migration-key-name
[key-uid]
(str key-uid "-password-migration"))
(defn save-password-migration!
[key-uid]
(-> (keychain/save-credentials
(password-migration-key-name key-uid)
key-uid
;; NOTE: using the key-uid as the password, but we don't really care about the
;; value, we only care that it's there
key-uid)
(.catch (fn [error]
(log/error "Failed to get the keychain password migration flag"
{:error error
:key-uid key-uid})))))
(defn get-password-migration!
[key-uid callback]
(keychain/get-credentials
(password-migration-key-name key-uid)
(comp callback boolean)))
(re-frame/reg-fx (re-frame/reg-fx
:keychain/clear-user-password :keychain/clear-user-password
(fn [key-uid] (fn [key-uid]
(keychain/reset-credentials (password-migration-key-name key-uid))
(keychain/reset-credentials key-uid))) (keychain/reset-credentials key-uid)))
(re-frame/reg-fx (re-frame/reg-fx
@ -107,5 +138,28 @@
(fn [{:keys [key-uid masked-password on-success on-error]}] (fn [{:keys [key-uid masked-password on-success on-error]}]
(-> (save-user-password! key-uid masked-password) (-> (save-user-password! key-uid masked-password)
(.then #(save-auth-method! key-uid auth-method-biometric)) (.then #(save-auth-method! key-uid auth-method-biometric))
(.then #(save-password-migration! key-uid))
(.then #(when on-success (on-success))) (.then #(when on-success (on-success)))
(.catch #(when on-error (on-error %)))))) (.catch #(when on-error (on-error %))))))
;; NOTE: migrating the plaintext password in the keychain
;; with the hashed one. Added due to the sync onboarding
;; flow, where the password arrives already hashed.
(re-frame/reg-fx
:keychain/password-hash-migration
(fn [{:keys [key-uid callback]
:or {callback identity}}]
(-> (get-password-migration! key-uid identity)
(.then (fn [migrated?]
(if migrated?
(callback)
(-> (get-user-password! key-uid identity)
(.then security/hash-masked-password)
(.then #(save-user-password! key-uid %))
(.then #(save-password-migration! key-uid))
(.then callback)))))
(.catch (fn [err]
(log/error "Failed to migrate the keychain password"
{:error err
:key-uid key-uid
:event :keychain/password-hash-migration}))))))

View File

@ -1,7 +1,6 @@
(ns status-im2.contexts.onboarding.enable-biometrics.view (ns status-im2.contexts.onboarding.enable-biometrics.view
(:require (:require
[quo.core :as quo] [quo.core :as quo]
[quo.theme :as quo.theme]
[react-native.core :as rn] [react-native.core :as rn]
[react-native.safe-area :as safe-area] [react-native.safe-area :as safe-area]
[status-im2.common.biometric.events :as biometric] [status-im2.common.biometric.events :as biometric]
@ -12,8 +11,7 @@
[status-im2.contexts.onboarding.enable-biometrics.style :as style] [status-im2.contexts.onboarding.enable-biometrics.style :as style]
[status-im2.navigation.state :as state] [status-im2.navigation.state :as state]
[utils.i18n :as i18n] [utils.i18n :as i18n]
[utils.re-frame :as rf] [utils.re-frame :as rf]))
[utils.security.core :as security]))
(defn page-title (defn page-title
@ -26,7 +24,7 @@
:description-accessibility-label :enable-biometrics-sub-title}]) :description-accessibility-label :enable-biometrics-sub-title}])
(defn enable-biometrics-buttons (defn enable-biometrics-buttons
[insets theme] [insets]
(let [supported-biometric-type (rf/sub [:biometric/supported-type]) (let [supported-biometric-type (rf/sub [:biometric/supported-type])
bio-type-label (biometric/get-label-by-type supported-biometric-type) bio-type-label (biometric/get-label-by-type supported-biometric-type)
profile-color (or (:color (rf/sub [:onboarding-2/profile])) profile-color (or (:color (rf/sub [:onboarding-2/profile]))
@ -34,23 +32,12 @@
syncing-results? (= :syncing-results @state/root-id)] syncing-results? (= :syncing-results @state/root-id)]
[rn/view {:style (style/buttons insets)} [rn/view {:style (style/buttons insets)}
[standard-auth/button [standard-auth/button
(merge
{:size 40 {:size 40
:accessibility-label :enable-biometrics-button :accessibility-label :enable-biometrics-button
:icon-left :i/face-id :icon-left :i/face-id
:customization-color profile-color :customization-color profile-color
:button-label (i18n/label :t/biometric-enable-button {:bio-type-label bio-type-label})} :on-press #(rf/dispatch [:onboarding-2/enable-biometrics])
(if syncing-results? :button-label (i18n/label :t/biometric-enable-button {:bio-type-label bio-type-label})}]
{:theme theme
:blur? true
:on-enter-password (fn [entered-password]
(rf/dispatch
[:onboarding-2/authenticate-enable-biometrics
(security/safe-unmask-data
entered-password)])
(rf/dispatch [:hide-bottom-sheet]))
:auth-button-label (i18n/label :t/confirm)}
{:on-press #(rf/dispatch [:onboarding-2/enable-biometrics])}))]
[quo/button [quo/button
{:accessibility-label :maybe-later-button {:accessibility-label :maybe-later-button
:background :blur :background :blur
@ -78,18 +65,16 @@
:source (resources/get-image :biometrics)}])) :source (resources/get-image :biometrics)}]))
(defn f-enable-biometrics (defn f-enable-biometrics
[{:keys [theme]}] []
(let [insets (safe-area/get-insets)] (let [insets (safe-area/get-insets)]
[rn/view {:style (style/page-container insets)} [rn/view {:style (style/page-container insets)}
[page-title] [page-title]
(if whitelist/whitelisted? (if whitelist/whitelisted?
[enable-biometrics-parallax] [enable-biometrics-parallax]
[enable-biometrics-simple]) [enable-biometrics-simple])
[enable-biometrics-buttons insets theme]])) [enable-biometrics-buttons insets]]))
(defn view
[]
[:f> f-enable-biometrics])
(defn- internale-enable-biometrics
[params]
[:f> f-enable-biometrics params])
(def view (quo.theme/with-theme internale-enable-biometrics))

View File

@ -35,15 +35,6 @@
{:biometric/authenticate {:on-success #(rf/dispatch [:onboarding-2/biometrics-done]) {:biometric/authenticate {:on-success #(rf/dispatch [:onboarding-2/biometrics-done])
:on-fail #(rf/dispatch [:onboarding-2/biometrics-fail %])}}) :on-fail #(rf/dispatch [:onboarding-2/biometrics-fail %])}})
(rf/defn authenticate-enable-biometrics
{:events [:onboarding-2/authenticate-enable-biometrics]}
[{:keys [db]} password]
{:db (-> db
(assoc-in [:onboarding-2/profile :password] password)
(assoc-in [:onboarding-2/profile :syncing?] true))
:biometric/authenticate {:on-success #(rf/dispatch [:onboarding-2/biometrics-done])
:on-fail #(rf/dispatch [:onboarding-2/biometrics-fail %])}})
(rf/defn navigate-to-enable-notifications (rf/defn navigate-to-enable-notifications
{:events [:onboarding-2/navigate-to-enable-notifications]} {:events [:onboarding-2/navigate-to-enable-notifications]}
[{:keys [db]}] [{:keys [db]}]
@ -154,7 +145,9 @@
biometric-enabled? biometric-enabled?
(assoc :keychain/save-password-and-auth-method (assoc :keychain/save-password-and-auth-method
{:key-uid key-uid {:key-uid key-uid
:masked-password masked-password :masked-password (if syncing?
masked-password
(security/hash-masked-password masked-password))
:on-success (fn [] :on-success (fn []
(if syncing? (if syncing?
(rf/dispatch [:onboarding-2/navigate-to-enable-notifications]) (rf/dispatch [:onboarding-2/navigate-to-enable-notifications])

View File

@ -11,7 +11,6 @@
[status-im.group-chats.core :as group-chats] [status-im.group-chats.core :as group-chats]
[status-im.mobile-sync-settings.core :as mobile-network] [status-im.mobile-sync-settings.core :as mobile-network]
[status-im.transport.core :as transport] [status-im.transport.core :as transport]
[status-im2.common.biometric.events :as biometric]
[status-im2.common.keychain.events :as keychain] [status-im2.common.keychain.events :as keychain]
[status-im2.common.log :as logging] [status-im2.common.log :as logging]
[status-im2.config :as config] [status-im2.config :as config]
@ -42,11 +41,22 @@
{:db (assoc-in db [:profile/login :processing] true) {:db (assoc-in db [:profile/login :processing] true)
::login [key-uid (native-module/sha3 (security/safe-unmask-data password))]})) ::login [key-uid (native-module/sha3 (security/safe-unmask-data password))]}))
(rf/defn biometrics-login
{:events [:profile.login/biometrics-login]}
[{:keys [db]}]
(let [{:keys [key-uid password]} (:profile/login db)]
{:db (assoc-in db [:profile/login :processing] true)
::login [key-uid (security/safe-unmask-data password)]}))
(rf/defn login-local-paired-user (rf/defn login-local-paired-user
{:events [:profile.login/local-paired-user]} {:events [:profile.login/local-paired-user]}
[{:keys [db]}] [{:keys [db]}]
(let [{:keys [key-uid password]} (get-in db [:syncing :profile])] (let [{:keys [key-uid password]} (get-in db [:syncing :profile])
{::login [key-uid password]})) masked-password (security/mask-data password)]
{:db (-> db
(assoc-in [:onboarding-2/profile :password] masked-password)
(assoc-in [:onboarding-2/profile :syncing?] true))
::login [key-uid password]}))
(rf/defn redirect-to-root (rf/defn redirect-to-root
[{:keys [db] :as cofx}] [{:keys [db] :as cofx}]
@ -151,20 +161,16 @@
(rf/defn get-auth-method-success (rf/defn get-auth-method-success
{:events [:profile.login/get-auth-method-success]} {:events [:profile.login/get-auth-method-success]}
[{:keys [db]} auth-method] [{:keys [db]} auth-method key-uid]
(merge {:db (assoc db :auth-method auth-method)} (merge {:db (assoc db :auth-method auth-method)}
(when (= auth-method keychain/auth-method-biometric) (when (= auth-method keychain/auth-method-biometric)
{:biometric/authenticate {:keychain/password-hash-migration
{:key-uid key-uid
:callback (fn []
(rf/dispatch [:biometric/authenticate
{:on-success #(rf/dispatch [:profile.login/biometric-success]) {:on-success #(rf/dispatch [:profile.login/biometric-success])
:on-faile #(rf/dispatch [:profile.login/biometric-auth-fail])}}))) :on-fail #(rf/dispatch
[:profile.login/biometric-auth-fail %])}]))}})))
(rf/defn biometric-auth-success
{:events [:profile.login/biometric-success]}
[{:keys [db] :as cofx}]
(let [key-uid (get-in db [:profile/login :key-uid])]
(keychain/get-user-password cofx
key-uid
#(rf/dispatch [:profile.login/get-user-password-success %]))))
;; result of :keychain/get-auth-method above ;; result of :keychain/get-auth-method above
(rf/defn get-user-password-success (rf/defn get-user-password-success
@ -175,14 +181,29 @@
cofx cofx
{:db (assoc-in db [:profile/login :password] password)} {:db (assoc-in db [:profile/login :password] password)}
(navigation/init-root :progress) (navigation/init-root :progress)
(login)))) (biometrics-login))))
(rf/reg-event-fx
:profile.login/biometric-success
(fn [{:keys [db]}]
(let [key-uid (get-in db [:profile/login :key-uid])]
{:db db
:fx [[:biometric/reset-not-enrolled-error key-uid]
[:keychain/get-user-password
[key-uid #(rf/dispatch [:profile.login/get-user-password-success %])]]]})))
(rf/reg-event-fx
:profile.login/biometric-auth-fail
(fn [{:keys [db]} [code]]
(let [key-uid (get-in db [:profile/login :key-uid])]
{:db db
:fx [[:dispatch [:init-root :profiles]]
(if (= code "NOT_ENROLLED")
[:biometric/supress-not-enrolled-error
[key-uid
[:biometric/show-message code]]]
[:dispatch [:biometric/show-message code]])]})))
(rf/defn biometric-auth-fail
{:events [:profile.login/biometric-auth-fail]}
[{:keys [db] :as cofx} code]
(rf/merge cofx
(navigation/init-root :profiles)
(biometric/show-message code)))
(rf/defn verify-database-password (rf/defn verify-database-password
{:events [:profile.login/verify-database-password]} {:events [:profile.login/verify-database-password]}
@ -197,7 +218,7 @@
(rf/defn verify-database-password-success (rf/defn verify-database-password-success
{:events [:profile.login/verified-database-password]} {:events [:profile.login/verified-database-password]}
[{:keys [db] :as cofx} valid? callback] [{:keys [db]} valid? callback]
(if valid? (if valid?
(do (do
(when (fn? callback) (when (fn? callback)

View File

@ -1,5 +1,7 @@
(ns utils.security.core (ns utils.security.core
(:require (:require
[native-module.core :as native-module]
[schema.core :as schema]
[utils.security.security-html :as h])) [utils.security.security-html :as h]))
(defprotocol Unmaskable (defprotocol Unmaskable
@ -58,3 +60,23 @@
and does not contain an rtlo character, which might mean that the url is spoofed" and does not contain an rtlo character, which might mean that the url is spoofed"
[text] [text]
(not (re-matches rtlo-link-regex text))) (not (re-matches rtlo-link-regex text)))
(defn hash-masked-password
[masked-password]
(-> masked-password
safe-unmask-data
native-module/sha3
mask-data))
(defn masked-data-instance?
[value]
(instance? MaskedData value))
(def ?masked-password
[:fn {:error/message "should be an instance of utils.security.core/MaskedData"}
masked-data-instance?])
(schema/=> hash-masked-password
[:=>
[:cat ?masked-password]
?masked-password])

View File

@ -1,6 +1,7 @@
(ns utils.security.security-test (ns utils.security.security-test
(:require (:require
[cljs.test :refer-macros [deftest is testing]] [cljs.test :refer-macros [deftest is testing]]
[native-module.core :as native-module]
[utils.security.core :as security])) [utils.security.core :as security]))
(def rtlo-link "http://google.com") (def rtlo-link "http://google.com")
@ -29,3 +30,26 @@
(deftest safe-link-text-test-exceptions (deftest safe-link-text-test-exceptions
(testing "rtlo links" (testing "rtlo links"
(is (not (security/safe-link-text? rtlo-link-text))))) (is (not (security/safe-link-text? rtlo-link-text)))))
(deftest mask-data-test
(testing "returns an instance of MaskedData"
(is (instance? security/MaskedData (security/mask-data "test"))))
(testing "hides the original value"
(is (= "******" (str (security/mask-data "test")))))
(testing "succeeds the equality check between same MaskedData instances"
(is (= (security/mask-data "value") (security/mask-data "value"))))
(testing "fails the equality check between different MaskedData instances"
(is (not (= (security/mask-data "value-A") (security/mask-data "value-B")))))
(testing "fails the equality check with non-MaskedData instances"
(is (not (= (security/mask-data "value") "value"))))
(testing "counts the masked data correctly"
(is (= (count "test") (count (security/mask-data "test")))))
(testing "unmasks the data correctly"
(is (= "test" (-> "test" security/mask-data security/safe-unmask-data)))))
(deftest hash-masked-password-test
(testing "returns an instance of MaskedData with the hashed content"
(is (= (-> "test" native-module/sha3 security/mask-data)
(-> "test" security/mask-data security/hash-masked-password))))
(testing "throws a schema exception if the argument is not an instance of MaskedData"
(is (thrown? js/Error (security/hash-masked-password "test")))))