Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Biometrics button on login screen #18106

Merged
merged 14 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,11 @@
{:margin-top 8
:flex-direction :row
:align-items :center})

(def input-container
{:flex-direction :row
:align-items :flex-end})

(def input
{:flex 1
:margin-right 12})
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
[quo.foundations.colors :as colors]
[quo.theme :as quo.theme]
[react-native.core :as rn]
[status-im2.common.keychain.events :as keychain]
[status-im2.common.standard-authentication.forgot-password-doc.view :as forgot-password-doc]
[status-im2.common.standard-authentication.password-input.style :as style]
[utils.i18n :as i18n]
Expand All @@ -29,19 +30,30 @@
(defn- view-internal
[{:keys [default-password theme shell?]}]
(let [{:keys [error processing]} (rf/sub [:profile/login])
auth-method (rf/sub [:auth-method])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my pr I was passing this in as a prop 🤔 No strong preferences, we could keep this input purer however and pass everything in as a prop 🤔 https://github.com/status-im/status-mobile/pull/17992/files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say unless it's used higher in the tree, mught be better to use the sub directly where it's used. I see very often 'subscription' maps passed as props all over the place, even when a sub is needed in one single child somewhere down the tree. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it's fine either way 👍

error-message (get-error-message error)
error? (boolean (seq error-message))]
[:<>
[quo/input
{:type :password
:blur? true
:disabled? processing
:placeholder (i18n/label :t/type-your-password)
:auto-focus true
:error? error?
:label (i18n/label :t/profile-password)
:on-change-text on-change-password
:default-value (security/safe-unmask-data default-password)}]
[rn/view {:style style/input-container}
[quo/input
{:type :password
:blur? true
:container-style style/input
:disabled? processing
:placeholder (i18n/label :t/type-your-password)
:auto-focus true
:error? error?
:label (i18n/label :t/profile-password)
:on-change-text on-change-password
:default-value (security/safe-unmask-data default-password)}]
(when (= auth-method keychain/auth-method-biometric)
[quo/button
{:type :outline
:icon-only? true
:on-press #(rf/dispatch [:profile.login/biometric-auth])
:background :blur
:size 40}
:i/face-id])]
(when error?
[rn/view {:style style/error-message}
[quo/info-message
Expand Down
7 changes: 7 additions & 0 deletions src/status_im2/contexts/profile/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@
{:db (assoc db :profile/profiles-overview profiles)}
(profile-selected key-uid)))

(rf/reg-event-fx
:profile/switch-profile
(fn [{:keys [db]} [key-uid]]
{:db db
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:db db is redundant

:fx [[:dispatch [:profile/profile-selected key-uid]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, its better just dispatch in the view no need in the :profile/switch-profile event

[:dispatch [:profile.login/login-with-biometric-if-available key-uid]]]}))

(defn reduce-profiles
[profiles]
(reduce
Expand Down
17 changes: 10 additions & 7 deletions src/status_im2/contexts/profile/login/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,22 @@
{:keychain/get-auth-method [key-uid
#(rf/dispatch [:profile.login/get-auth-method-success % key-uid])]})

(rf/reg-event-fx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you elaborate? this looks strange

Copy link
Member Author

@clauxx clauxx Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When logging in, we need :biometric/authenticate with the same success/fail callbacks in two different places, so the dispatch was moved in a separate event so it can be re-used. Removed it in favor of using :biometric/authenticate directly in the view.

:profile.login/biometric-auth
(fn []
{:dispatch [:biometric/authenticate
{:on-success #(rf/dispatch [:profile.login/biometric-success])
:on-fail #(rf/dispatch
[:profile.login/biometric-auth-fail %])}]}))

(rf/defn get-auth-method-success
{:events [:profile.login/get-auth-method-success]}
[{:keys [db]} auth-method key-uid]
(merge {:db (assoc db :auth-method auth-method)}
(when (= auth-method keychain/auth-method-biometric)
{:keychain/password-hash-migration
{:key-uid key-uid
:callback (fn []
(rf/dispatch [:biometric/authenticate
{:on-success #(rf/dispatch [:profile.login/biometric-success])
:on-fail #(rf/dispatch
[:profile.login/biometric-auth-fail %])}]))}})))
:callback #(rf/dispatch [:profile.login/biometric-auth])}})))

;; result of :keychain/get-auth-method above
(rf/defn get-user-password-success
Expand All @@ -218,8 +222,7 @@
(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")
:fx [(if (= code "NOT_ENROLLED")
clauxx marked this conversation as resolved.
Show resolved Hide resolved
[:biometric/supress-not-enrolled-error
[key-uid
[:biometric/show-message code]]]
Expand Down
2 changes: 1 addition & 1 deletion src/status_im2/contexts/profile/profiles/view.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
:profile-picture profile-picture})
:on-card-press (fn []
(rf/dispatch
[:profile/profile-selected key-uid])
[:profile/switch-profile key-uid])
(when-not keycard-pairing (set-hide-profiles)))}]))

(defn- f-profiles-section
Expand Down