Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #7113, #7181: Ability to reveal password during wallet unlock, re…
Browse files Browse the repository at this point in the history
…store, and create (#7269)

* 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.

* Move Biometrics button below Unlock/Restore buttons when unlocking wallet. Don't focus keyboard when pre-filling password via biometrics.

* Disable autocapitalization in recovery phrase field
  • Loading branch information
StephenHeaps authored May 1, 2023
1 parent 87e7718 commit 6122a2d
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 82 deletions.
109 changes: 60 additions & 49 deletions Sources/BraveWallet/Crypto/Onboarding/CreateWalletView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -125,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),
Expand Down
37 changes: 31 additions & 6 deletions Sources/BraveWallet/Crypto/Onboarding/RestoreWalletView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -144,6 +145,7 @@ private struct RestoreWalletView: View {
}
}
.textFieldStyle(BraveValidatedTextFieldStyle(error: restoreError, when: .invalidPhrase))
.textInputAutocapitalization(.never)
if isShowingLegacyWalletToggle {
HStack {
Toggle(Strings.Wallet.restoreWalletImportFromLegacyBraveWallet, isOn: $isBraveLegacyWallet)
Expand Down Expand Up @@ -192,12 +194,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)
Expand All @@ -210,6 +221,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,
Expand Down
89 changes: 62 additions & 27 deletions Sources/BraveWallet/Crypto/UnlockWalletView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ 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
@FocusState private var isPasswordFieldFocused: Bool

private enum UnlockError: LocalizedError {
case incorrectPassword
Expand Down Expand Up @@ -45,6 +47,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()
}
Expand All @@ -66,54 +70,63 @@ 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: 46) {
VStack(spacing: isSmallScreen ? 38 : 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 {
SecureField(Strings.Wallet.passwordPlaceholder, text: $password, onCommit: unlock)
.textContentType(.password)
.font(.subheadline)
.introspectTextField(customize: { tf in
tf.becomeFirstResponder()
})
.textFieldStyle(BraveValidatedTextFieldStyle(error: unlockError))
if keyringStore.isKeychainPasswordStored, let icon = biometricsIcon {
Button(action: fillPasswordFromKeychain) {
icon
.imageScale(.large)
.font(.headline)
}
}
}
.padding(.top, isSmallScreen ? 0 : 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) {
VStack(spacing: isSmallScreen ? 20 : 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)
.font(.subheadline.weight(.medium))
}
.foregroundColor(Color(.braveLabel))
}
.padding(.top, isSmallScreen ? 5 : 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, 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))
Expand All @@ -125,6 +138,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
}
}
}
Expand All @@ -140,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)
}
}
Loading

0 comments on commit 6122a2d

Please sign in to comment.