From c543452eacd0c056d526144cb2f12177d76c121b Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Mon, 17 Apr 2023 10:18:53 -0400 Subject: [PATCH 1/6] Add `RevealableSecureField` password field for Unlock, Create and Restoring wallet. Hide password (if revealed) before populating unlock wallet password field with stored password. Hide recovery phrase (if revealed) before revealing wallet password when restoring wallet from recovery phrase. --- .../Crypto/Onboarding/CreateWalletView.swift | 23 ++++-- .../Crypto/Onboarding/RestoreWalletView.swift | 36 ++++++++-- .../BraveWallet/Crypto/UnlockWalletView.swift | 6 +- .../BraveWallet/RevealableSecureField.swift | 72 +++++++++++++++++++ 4 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 Sources/BraveWallet/RevealableSecureField.swift diff --git a/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift b/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift index 1f35f3b4e70..e99540c389e 100644 --- a/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift +++ b/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift @@ -47,6 +47,7 @@ private struct CreateWalletView: View { @State private var password: String = "" @State private var repeatedPassword: String = "" + @State private var isPasswordRevealed: Bool = false @State private var validationError: ValidationError? @State private var isShowingBiometricsPrompt: Bool = false @State private var isSkippingBiometricsPrompt: Bool = false @@ -111,12 +112,22 @@ private struct CreateWalletView: View { .multilineTextAlignment(.center) .fixedSize(horizontal: false, vertical: true) VStack { - SecureField(Strings.Wallet.passwordPlaceholder, text: $password) - .textContentType(.newPassword) - .textFieldStyle(BraveValidatedTextFieldStyle(error: validationError, when: .requirementsNotMet)) - SecureField(Strings.Wallet.repeatedPasswordPlaceholder, text: $repeatedPassword, onCommit: createWallet) - .textContentType(.newPassword) - .textFieldStyle(BraveValidatedTextFieldStyle(error: validationError, when: .inputsDontMatch)) + RevealableSecureField( + Strings.Wallet.passwordPlaceholder, + text: $password, + isRevealed: $isPasswordRevealed + ) + .textContentType(.newPassword) + .textFieldStyle(BraveValidatedTextFieldStyle(error: validationError, when: .requirementsNotMet)) + RevealableSecureField( + Strings.Wallet.repeatedPasswordPlaceholder, + showsRevealButton: false, + text: $repeatedPassword, + isRevealed: $isPasswordRevealed + ) + .textContentType(.newPassword) + .textFieldStyle(BraveValidatedTextFieldStyle(error: validationError, when: .inputsDontMatch)) + .onSubmit(createWallet) } .font(.subheadline) .padding(.horizontal, 48) diff --git a/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift b/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift index c934c54e62a..6f471b1419d 100644 --- a/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift +++ b/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift @@ -50,6 +50,7 @@ private struct RestoreWalletView: View { @State private var password: String = "" @State private var repeatedPassword: String = "" + @State private var isPasswordRevealed: Bool = false @State private var phrase: String = "" @State private var isEditingPhrase: Bool = false @State private var showingRecoveryPhase: Bool = false @@ -192,12 +193,21 @@ private struct RestoreWalletView: View { VStack { Text(Strings.Wallet.restoreWalletNewPasswordTitle) .font(.subheadline.weight(.medium)) - SecureField(Strings.Wallet.passwordPlaceholder, text: $password) - .textContentType(.newPassword) - .textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .requirementsNotMet)) - SecureField(Strings.Wallet.repeatedPasswordPlaceholder, text: $repeatedPassword) - .textContentType(.newPassword) - .textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .inputsDontMatch)) + RevealableSecureField( + Strings.Wallet.passwordPlaceholder, + text: $password, + isRevealed: $isPasswordRevealed + ) + .textContentType(.newPassword) + .textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .requirementsNotMet)) + RevealableSecureField( + Strings.Wallet.repeatedPasswordPlaceholder, + showsRevealButton: false, + text: $repeatedPassword, + isRevealed: $isPasswordRevealed + ) + .textContentType(.newPassword) + .textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .inputsDontMatch)) } .font(.subheadline) .padding(.horizontal, 48) @@ -210,6 +220,20 @@ private struct RestoreWalletView: View { .onChange(of: phrase, perform: handlePhraseChanged) .onChange(of: password, perform: handlePasswordChanged) .onChange(of: repeatedPassword, perform: handleRepeatedPasswordChanged) + .onChange(of: isPasswordRevealed) { isPasswordRevealed in + // only reveal password or recovery phrase, not both. Used to prevent + // 3rd-party keyboard usage on this view by keeping a SecureField visible + if isPasswordRevealed && showingRecoveryPhase { + showingRecoveryPhase = false + } + } + .onChange(of: showingRecoveryPhase) { showingRecoveryPhase in + // only reveal password or recovery phrase, not both. Used to prevent + // 3rd-party keyboard usage on this view by keeping a SecureField visible + if showingRecoveryPhase && isPasswordRevealed { + isPasswordRevealed = false + } + } .background( WalletPromptView( isPresented: $keyringStore.isRestoreFromUnlockBiometricsPromptVisible, diff --git a/Sources/BraveWallet/Crypto/UnlockWalletView.swift b/Sources/BraveWallet/Crypto/UnlockWalletView.swift index be4987d2f22..70f8ad687bd 100644 --- a/Sources/BraveWallet/Crypto/UnlockWalletView.swift +++ b/Sources/BraveWallet/Crypto/UnlockWalletView.swift @@ -12,6 +12,7 @@ struct UnlockWalletView: View { @ObservedObject var keyringStore: KeyringStore @State private var password: String = "" + @State private var isPasswordRevealed: Bool = false @State private var unlockError: UnlockError? @State private var attemptedBiometricsUnlock: Bool = false @@ -45,6 +46,8 @@ struct UnlockWalletView: View { private func fillPasswordFromKeychain() { if let password = keyringStore.retrievePasswordFromKeychain() { + // hide password (if revealed) before populating field with stored password + isPasswordRevealed = false self.password = password unlock() } @@ -80,13 +83,14 @@ struct UnlockWalletView: View { .multilineTextAlignment(.center) .fixedSize(horizontal: false, vertical: true) HStack { - SecureField(Strings.Wallet.passwordPlaceholder, text: $password, onCommit: unlock) + RevealableSecureField(Strings.Wallet.passwordPlaceholder, text: $password, isRevealed: $isPasswordRevealed) .textContentType(.password) .font(.subheadline) .introspectTextField(customize: { tf in tf.becomeFirstResponder() }) .textFieldStyle(BraveValidatedTextFieldStyle(error: unlockError)) + .onSubmit(unlock) if keyringStore.isKeychainPasswordStored, let icon = biometricsIcon { Button(action: fillPasswordFromKeychain) { icon diff --git a/Sources/BraveWallet/RevealableSecureField.swift b/Sources/BraveWallet/RevealableSecureField.swift new file mode 100644 index 00000000000..92b662927ef --- /dev/null +++ b/Sources/BraveWallet/RevealableSecureField.swift @@ -0,0 +1,72 @@ +// Copyright 2023 The Brave Authors. All rights reserved. +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +import SwiftUI + +struct RevealableSecureField: View { + + private enum Focus: Hashable { + case secure + case visible + } + + let placeholder: String + let showsRevealButton: Bool + @Binding var text: String + @Binding var isRevealed: Bool + @FocusState private var focus: Focus? + + init( + _ placeholder: String, + showsRevealButton: Bool = true, + text: Binding, + isRevealed: Binding + ) { + self.placeholder = placeholder + self.showsRevealButton = showsRevealButton + self._text = text + self._isRevealed = isRevealed + } + + var body: some View { + HStack(alignment: .firstTextBaseline) { + Group { + if isRevealed { + TextField(placeholder, text: $text) + .focused($focus, equals: .visible) + } else { + SecureField(placeholder, text: $text) + .focused($focus, equals: .secure) + } + } + .keyboardType(.asciiCapable) + .autocorrectionDisabled(true) + .autocapitalization(.none) + .introspectTextField { + $0.autocorrectionType = .no + $0.autocapitalizationType = .none + $0.spellCheckingType = .no + } + if showsRevealButton { + Button(action: { + if let focus { // if user has a field focused + // focus the new field automatically + if focus == .secure { + self.focus = .visible + } else { + self.focus = .secure + } + } + self.isRevealed.toggle() + }) { + Image(braveSystemName: isRevealed ? "brave.eye.slash" : "brave.eye") + .contentShape(Rectangle()) + .transition(.identity) + .animation(nil, value: isRevealed) + } + } + } + } +} From fd9f19c10526dc3007b2169161cd3e7096491e1b Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Mon, 17 Apr 2023 15:06:44 -0400 Subject: [PATCH 2/6] Move Biometrics button below Unlock/Restore buttons when unlocking wallet. Don't focus keyboard when pre-filling password via biometrics. --- .../BraveWallet/Crypto/UnlockWalletView.swift | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/Sources/BraveWallet/Crypto/UnlockWalletView.swift b/Sources/BraveWallet/Crypto/UnlockWalletView.swift index 70f8ad687bd..1ffeabd6dd2 100644 --- a/Sources/BraveWallet/Crypto/UnlockWalletView.swift +++ b/Sources/BraveWallet/Crypto/UnlockWalletView.swift @@ -15,6 +15,7 @@ struct UnlockWalletView: View { @State private var isPasswordRevealed: Bool = false @State private var unlockError: UnlockError? @State private var attemptedBiometricsUnlock: Bool = false + @FocusState private var isPasswordFieldFocused: Bool private enum UnlockError: LocalizedError { case incorrectPassword @@ -72,40 +73,26 @@ struct UnlockWalletView: View { var body: some View { ScrollView(.vertical) { - VStack(spacing: 46) { + VStack(spacing: 42) { Image("graphic-lock", bundle: .module) - .padding(.bottom) .accessibilityHidden(true) - VStack { - Text(Strings.Wallet.unlockWalletTitle) - .font(.headline) - .padding(.bottom) - .multilineTextAlignment(.center) - .fixedSize(horizontal: false, vertical: true) - HStack { - RevealableSecureField(Strings.Wallet.passwordPlaceholder, text: $password, isRevealed: $isPasswordRevealed) - .textContentType(.password) - .font(.subheadline) - .introspectTextField(customize: { tf in - tf.becomeFirstResponder() - }) - .textFieldStyle(BraveValidatedTextFieldStyle(error: unlockError)) - .onSubmit(unlock) - if keyringStore.isKeychainPasswordStored, let icon = biometricsIcon { - Button(action: fillPasswordFromKeychain) { - icon - .imageScale(.large) - .font(.headline) - } - } - } + .padding(.top, 20) + Text(Strings.Wallet.unlockWalletTitle) + .font(.headline) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + RevealableSecureField(Strings.Wallet.passwordPlaceholder, text: $password, isRevealed: $isPasswordRevealed) + .textContentType(.password) + .focused($isPasswordFieldFocused) + .font(.subheadline) + .textFieldStyle(BraveValidatedTextFieldStyle(error: unlockError)) + .onSubmit(unlock) .padding(.horizontal, 48) - } VStack(spacing: 30) { Button(action: unlock) { Text(Strings.Wallet.unlockWalletButtonTitle) } - .buttonStyle(BraveFilledButtonStyle(size: .normal)) + .buttonStyle(BraveFilledButtonStyle(size: .large)) .disabled(!isPasswordValid) NavigationLink(destination: RestoreWalletContainerView(keyringStore: keyringStore)) { Text(Strings.Wallet.restoreWalletButtonTitle) @@ -113,6 +100,19 @@ struct UnlockWalletView: View { } .foregroundColor(Color(.braveLabel)) } + .padding(.top, 10) + + if keyringStore.isKeychainPasswordStored, let icon = biometricsIcon { + Button(action: fillPasswordFromKeychain) { + icon + .resizable() + .aspectRatio(contentMode: .fit) + .imageScale(.large) + .font(.headline) + .frame(width: 26, height: 26) + } + .padding(.top, 18) + } } .frame(maxHeight: .infinity, alignment: .top) .padding() @@ -129,6 +129,9 @@ struct UnlockWalletView: View { if !keyringStore.lockedManually && !attemptedBiometricsUnlock && keyringStore.defaultKeyring.isLocked && UIApplication.shared.isProtectedDataAvailable { attemptedBiometricsUnlock = true fillPasswordFromKeychain() + } else { + // only focus field if not auto-filling via biometrics, and user did not manually lock + isPasswordFieldFocused = !keyringStore.lockedManually } } } From 2667b2efd3e01d1f4f33e529ea25ba3bac21a1e2 Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Mon, 17 Apr 2023 15:07:07 -0400 Subject: [PATCH 3/6] Smaller screen layout tweaks. --- .../BraveWallet/Crypto/UnlockWalletView.swift | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/Sources/BraveWallet/Crypto/UnlockWalletView.swift b/Sources/BraveWallet/Crypto/UnlockWalletView.swift index 1ffeabd6dd2..90a33ffe7fb 100644 --- a/Sources/BraveWallet/Crypto/UnlockWalletView.swift +++ b/Sources/BraveWallet/Crypto/UnlockWalletView.swift @@ -70,13 +70,19 @@ struct UnlockWalletView: View { } return nil } + + @State private var viewSize: CGSize = .zero + + private var isSmallScreen: Bool { + viewSize.height <= 667 + } var body: some View { ScrollView(.vertical) { - VStack(spacing: 42) { + VStack(spacing: isSmallScreen ? 38 : 42) { Image("graphic-lock", bundle: .module) .accessibilityHidden(true) - .padding(.top, 20) + .padding(.top, isSmallScreen ? 0 : 20) Text(Strings.Wallet.unlockWalletTitle) .font(.headline) .multilineTextAlignment(.center) @@ -88,7 +94,7 @@ struct UnlockWalletView: View { .textFieldStyle(BraveValidatedTextFieldStyle(error: unlockError)) .onSubmit(unlock) .padding(.horizontal, 48) - VStack(spacing: 30) { + VStack(spacing: isSmallScreen ? 20 : 30) { Button(action: unlock) { Text(Strings.Wallet.unlockWalletButtonTitle) } @@ -100,7 +106,7 @@ struct UnlockWalletView: View { } .foregroundColor(Color(.braveLabel)) } - .padding(.top, 10) + .padding(.top, isSmallScreen ? 5 : 10) if keyringStore.isKeychainPasswordStored, let icon = biometricsIcon { Button(action: fillPasswordFromKeychain) { @@ -111,13 +117,16 @@ struct UnlockWalletView: View { .font(.headline) .frame(width: 26, height: 26) } - .padding(.top, 18) + .padding(.top, isSmallScreen ? 12 : 18) } } .frame(maxHeight: .infinity, alignment: .top) .padding() .padding(.vertical) } + .readSize(onChange: { size in + self.viewSize = size + }) .navigationTitle(Strings.Wallet.cryptoTitle) .navigationBarTitleDisplayMode(.inline) .background(Color(.braveBackground).edgesIgnoringSafeArea(.all)) @@ -147,3 +156,22 @@ struct CryptoUnlockView_Previews: PreviewProvider { } } #endif + +private struct SizePreferenceKey: PreferenceKey { + static var defaultValue: CGSize = .zero + static func reduce(value: inout CGSize, nextValue: () -> CGSize) {} +} + +private extension View { + /// Determines the size of the view and calls the `onChange` when the size changes with the new size. + /// https://www.fivestars.blog/articles/swiftui-share-layout-information/ + func readSize(onChange: @escaping (CGSize) -> Void) -> some View { + background( + GeometryReader { geometryProxy in + Color.clear + .preference(key: SizePreferenceKey.self, value: geometryProxy.size) + } + ) + .onPreferenceChange(SizePreferenceKey.self, perform: onChange) + } +} From b33f56a832fd957163d8cb73ea742e6ac7f4fb0f Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Wed, 26 Apr 2023 11:00:00 -0400 Subject: [PATCH 4/6] Fix eye image after leo design update --- Sources/BraveWallet/RevealableSecureField.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/BraveWallet/RevealableSecureField.swift b/Sources/BraveWallet/RevealableSecureField.swift index 92b662927ef..cc9c3c36f2f 100644 --- a/Sources/BraveWallet/RevealableSecureField.swift +++ b/Sources/BraveWallet/RevealableSecureField.swift @@ -61,7 +61,7 @@ struct RevealableSecureField: View { } self.isRevealed.toggle() }) { - Image(braveSystemName: isRevealed ? "brave.eye.slash" : "brave.eye") + Image(braveSystemName: isRevealed ? "leo.eye.off" : "leo.eye.on") .contentShape(Rectangle()) .transition(.identity) .animation(nil, value: isRevealed) From 20904ffe7f9c8a7fbad3239a67be2846b5a421f7 Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Wed, 26 Apr 2023 11:31:01 -0400 Subject: [PATCH 5/6] Add padding to eye/reveal button. Fix for CreateWalletView reveal button interaction when text is inputted in the field (SwiftUI quirk with the WalletPromptView) --- .../Crypto/Onboarding/CreateWalletView.swift | 86 +++++++++---------- .../BraveWallet/RevealableSecureField.swift | 4 +- 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift b/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift index e99540c389e..6719213ea86 100644 --- a/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift +++ b/Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift @@ -136,53 +136,53 @@ private struct CreateWalletView: View { Text(Strings.Wallet.continueButtonTitle) } .buttonStyle(BraveFilledButtonStyle(size: .normal)) + .background( + WalletPromptView( + isPresented: $isShowingBiometricsPrompt, + buttonTitle: Strings.Wallet.biometricsSetupEnableButtonTitle, + action: { enabled, navController in + // Store password in keychain + if enabled, case let status = keyringStore.storePasswordInKeychain(password), + status != errSecSuccess { + let isPublic = AppConstants.buildChannel.isPublic + let alert = UIAlertController( + title: Strings.Wallet.biometricsSetupErrorTitle, + message: Strings.Wallet.biometricsSetupErrorMessage + (isPublic ? "" : " (\(status))"), + preferredStyle: .alert + ) + alert.addAction(.init(title: Strings.OKString, style: .default, handler: nil)) + navController?.presentedViewController?.present(alert, animated: true) + return false + } + let controller = UIHostingController( + rootView: BackupWalletView( + password: password, + keyringStore: keyringStore + ) + ) + navController?.pushViewController(controller, animated: true) + return true + }, + content: { + VStack { + Image(sharedName: "pin-migration-graphic") + .resizable() + .aspectRatio(contentMode: .fit) + .frame(maxWidth: 250) + .padding() + Text(Strings.Wallet.biometricsSetupTitle) + .font(.headline) + .fixedSize(horizontal: false, vertical: true) + .multilineTextAlignment(.center) + .padding(.bottom) + } + } + ) + ) } .frame(maxHeight: .infinity, alignment: .top) .padding() .padding(.vertical) - .background( - WalletPromptView( - isPresented: $isShowingBiometricsPrompt, - buttonTitle: Strings.Wallet.biometricsSetupEnableButtonTitle, - action: { enabled, navController in - // Store password in keychain - if enabled, case let status = keyringStore.storePasswordInKeychain(password), - status != errSecSuccess { - let isPublic = AppConstants.buildChannel.isPublic - let alert = UIAlertController( - title: Strings.Wallet.biometricsSetupErrorTitle, - message: Strings.Wallet.biometricsSetupErrorMessage + (isPublic ? "" : " (\(status))"), - preferredStyle: .alert - ) - alert.addAction(.init(title: Strings.OKString, style: .default, handler: nil)) - navController?.presentedViewController?.present(alert, animated: true) - return false - } - let controller = UIHostingController( - rootView: BackupWalletView( - password: password, - keyringStore: keyringStore - ) - ) - navController?.pushViewController(controller, animated: true) - return true - }, - content: { - VStack { - Image(sharedName: "pin-migration-graphic") - .resizable() - .aspectRatio(contentMode: .fit) - .frame(maxWidth: 250) - .padding() - Text(Strings.Wallet.biometricsSetupTitle) - .font(.headline) - .fixedSize(horizontal: false, vertical: true) - .multilineTextAlignment(.center) - .padding(.bottom) - } - } - ) - ) .background( NavigationLink( destination: BackupWalletView(password: password, keyringStore: keyringStore), diff --git a/Sources/BraveWallet/RevealableSecureField.swift b/Sources/BraveWallet/RevealableSecureField.swift index cc9c3c36f2f..e06f5ef489c 100644 --- a/Sources/BraveWallet/RevealableSecureField.swift +++ b/Sources/BraveWallet/RevealableSecureField.swift @@ -31,7 +31,7 @@ struct RevealableSecureField: View { } var body: some View { - HStack(alignment: .firstTextBaseline) { + HStack(alignment: .firstTextBaseline, spacing: 4) { Group { if isRevealed { TextField(placeholder, text: $text) @@ -62,6 +62,8 @@ struct RevealableSecureField: View { self.isRevealed.toggle() }) { Image(braveSystemName: isRevealed ? "leo.eye.off" : "leo.eye.on") + .padding(.vertical, 6) + .padding(.horizontal, 4) .contentShape(Rectangle()) .transition(.identity) .animation(nil, value: isRevealed) From 06d5ccf960c29c18980ea50b71230a40e370439e Mon Sep 17 00:00:00 2001 From: Stephen Heaps Date: Wed, 26 Apr 2023 12:09:07 -0400 Subject: [PATCH 6/6] Disable autocapitalization in recovery phrase field --- Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift b/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift index 6f471b1419d..9eb2a7bddd1 100644 --- a/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift +++ b/Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift @@ -145,6 +145,7 @@ private struct RestoreWalletView: View { } } .textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .invalidPhrase)) + .textInputAutocapitalization(.never) if isShowingLegacyWalletToggle { HStack { Toggle(Strings.Wallet.restoreWalletImportFromLegacyBraveWallet, isOn: $isBraveLegacyWallet)