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

Commit

Permalink
Fix #5967: Password Protection for Secondary Account Removal (#6149)
Browse files Browse the repository at this point in the history
* Refactor `BiometricsPasscodeEntryView` to extract out password entry into `PasswordEntryView` so it can be re-used.

* Add `RemoveAccountConfirmationView`, integrate to replace the confirmation alert with password entry when removing a secondary account

* Separate password entry field with biometrics into `PasswordEntryField` so it can be re-used for private key password protection

* Add cancel button to modal, cleanup unneeded modifiers

* Address PR comments, fix bug where auto-lock on `RemoveAccountConfirmationView` would cause invalid wallet state where wallet is locked but displaying unlocked state (`CryptoView` was updating prior to `defaultKeyring` update).
  • Loading branch information
StephenHeaps authored Oct 17, 2022
1 parent ad28750 commit 7f89d78
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ struct AccountActivityView: View {
guard let info = keyringStore.allAccounts.first(where: { $0.address == activityStore.account.address }) else {
// The account has been removed... User should technically never see this state because
// `AccountsViewController` pops this view off the stack when the account is removed
presentationMode.dismiss()
return activityStore.account
}
return info
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ struct AccountDetailsView: View {

@Environment(\.presentationMode) @Binding private var presentationMode

private func removeAccount(password: String) {
keyringStore.removeSecondaryAccount(for: account, password: password)
}

private func renameAccountAndDismiss() {
if name.isEmpty {
// Show error?
Expand Down Expand Up @@ -76,17 +72,12 @@ struct AccountDetailsView: View {
.multilineTextAlignment(.center)
.frame(maxWidth: .infinity)
}
.alert(isPresented: $isPresentingRemoveConfirmation) {
Alert(
title: Text(Strings.Wallet.accountRemoveAlertConfirmation),
message: Text(Strings.Wallet.warningAlertConfirmation),
primaryButton: .destructive(Text(Strings.yes), action: {
// TODO: Issue #5967 - Add password protection to view
removeAccount(password: "")
}),
secondaryButton: .cancel(Text(Strings.no))
.sheet(isPresented: $isPresentingRemoveConfirmation, content: {
RemoveAccountConfirmationView(
account: account,
keyringStore: keyringStore
)
}
})
.listRowBackground(Color(.secondaryBraveGroupedBackground))
}
}
Expand Down
7 changes: 5 additions & 2 deletions Sources/BraveWallet/Crypto/Stores/KeyringStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,12 @@ public class KeyringStore: ObservableObject {
Task { @MainActor in // fetch all KeyringInfo for all coin types
let selectedCoin = await walletService.selectedCoin()
let selectedAccountAddress = await keyringService.selectedAccount(selectedCoin)
self.allKeyrings = await keyringService.keyrings(for: WalletConstants.supportedCoinTypes)
let allKeyrings = await keyringService.keyrings(for: WalletConstants.supportedCoinTypes)
self.defaultAccounts = await keyringService.defaultAccounts(for: WalletConstants.supportedCoinTypes)
if let defaultKeyring = allKeyrings.first(where: { $0.id == BraveWallet.DefaultKeyringId }) {
self.defaultKeyring = defaultKeyring
}
self.allKeyrings = allKeyrings
if let selectedAccountKeyring = allKeyrings.first(where: { $0.coin == selectedCoin }) {
if self.selectedAccount.address != selectedAccountAddress {
if let selectedAccount = selectedAccountKeyring.accountInfos.first(where: { $0.address == selectedAccountAddress }) {
Expand Down Expand Up @@ -290,8 +291,10 @@ public class KeyringStore: ObservableObject {

func removeSecondaryAccount(for account: BraveWallet.AccountInfo, password: String, completion: ((Bool) -> Void)? = nil) {
keyringService.removeImportedAccount(account.address, password: password, coin: account.coin) { success in
self.updateKeyringInfo()
completion?(success)
if success {
self.updateKeyringInfo()
}
}
}

Expand Down
205 changes: 205 additions & 0 deletions Sources/BraveWallet/PasswordEntryView.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
// Copyright 2022 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 DesignSystem
import Strings
import SwiftUI
import BraveCore
import LocalAuthentication

struct PasswordEntryError: LocalizedError, Equatable {
let message: String
var errorDescription: String? { message }

static let incorrectPassword = Self(message: Strings.Wallet.incorrectPasswordErrorMessage)
}

/// Field for entering wallet password, with optional biometrics support
struct PasswordEntryField: View {

/// The password being entered
@Binding var password: String
/// The error displayed under the password field
@Binding var error: PasswordEntryError?
/// If we should show the biometrics icon to allow biometics unlock (if available & password stored in keychain)
let shouldShowBiometrics: Bool
let keyringStore: KeyringStore
let onCommit: () -> Void

@State private var attemptedBiometricsUnlock: Bool = false

init(
password: Binding<String>,
error: Binding<PasswordEntryError?>,
shouldShowBiometrics: Bool,
keyringStore: KeyringStore,
onCommit: @escaping () -> Void
) {
self._password = password
self._error = error
self.shouldShowBiometrics = shouldShowBiometrics
self.onCommit = onCommit
self.keyringStore = keyringStore
}

private var biometricsIcon: Image? {
let context = LAContext()
if context.canEvaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, error: nil) {
switch context.biometryType {
case .faceID:
return Image(systemName: "faceid")
case .touchID:
return Image(systemName: "touchid")
case .none:
return nil
@unknown default:
return nil
}
}
return nil
}

private func fillPasswordFromKeychain() {
if let password = keyringStore.retrievePasswordFromKeychain() {
self.password = password
onCommit()
}
}

var body: some View {
HStack {
SecureField(Strings.Wallet.passwordPlaceholder, text: $password, onCommit: onCommit)
.textContentType(.password)
.font(.subheadline)
.introspectTextField(customize: { tf in
tf.becomeFirstResponder()
})
.textFieldStyle(BraveValidatedTextFieldStyle(error: error))
if shouldShowBiometrics, keyringStore.isKeychainPasswordStored, let icon = biometricsIcon {
Button(action: fillPasswordFromKeychain) {
icon
.imageScale(.large)
.font(.headline)
}
}
}
}
}

#if DEBUG
struct PasswordEntryField_Previews: PreviewProvider {
static var previews: some View {
PasswordEntryField(
password: Binding(get: { "" }, set: { _ in }),
error: Binding(get: { nil }, set: { _ in }),
shouldShowBiometrics: false,
keyringStore: .previewStore,
onCommit: {})
}
}
#endif

/// View for entering a password with an title and message displayed, and optional biometrics
struct PasswordEntryView: View {

let keyringStore: KeyringStore
let title: String
let message: String
let shouldShowBiometrics: Bool
let action: (_ password: String, _ completion: @escaping (PasswordEntryError?) -> Void) -> Void

init(
keyringStore: KeyringStore,
title: String = Strings.Wallet.confirmPasswordTitle,
message: String,
shouldShowBiometrics: Bool = true,
action: @escaping (_ password: String, _ completion: @escaping (PasswordEntryError?) -> Void) -> Void
) {
self.keyringStore = keyringStore
self.title = title
self.message = message
self.shouldShowBiometrics = shouldShowBiometrics
self.action = action
}

@State private var password = ""
@State private var error: PasswordEntryError?
@Environment(\.presentationMode) @Binding private var presentationMode

private var isPasswordValid: Bool {
!password.isEmpty
}

private func validate() {
action(password) { entryError in
DispatchQueue.main.async {
if let entryError = entryError {
self.error = entryError
UIImpactFeedbackGenerator(style: .medium).bzzt()
} else {
presentationMode.dismiss()
}
}
}
}

var body: some View {
NavigationView {
ScrollView(.vertical) {
VStack(spacing: 36) {
Image("graphic-lock", bundle: .module)
.accessibilityHidden(true)
VStack {
Text(message)
.font(.headline)
.padding(.bottom)
.multilineTextAlignment(.center)
.fixedSize(horizontal: false, vertical: true)
PasswordEntryField(
password: $password,
error: $error,
shouldShowBiometrics: shouldShowBiometrics,
keyringStore: keyringStore,
onCommit: validate
)
.padding(.horizontal, 48)
}
Button(action: validate) {
Text(Strings.Wallet.confirm)
}
.buttonStyle(BraveFilledButtonStyle(size: .normal))
.disabled(!isPasswordValid)
}
.padding()
.padding(.vertical)
.navigationBarTitleDisplayMode(.inline)
.navigationTitle(title)
.toolbar {
ToolbarItemGroup(placement: .cancellationAction) {
Button(action: { presentationMode.dismiss() }) {
Text(Strings.cancelButtonTitle)
.foregroundColor(Color(.braveOrange))
}
}
}
}
}
.navigationViewStyle(.stack)
}
}

#if DEBUG
struct PasswordEntryView_Previews: PreviewProvider {
static var previews: some View {
PasswordEntryView(
keyringStore: .previewStore,
message: String.localizedStringWithFormat(
Strings.Wallet.removeAccountConfirmationMessage,
"Account 1"
),
action: { _, _ in })
}
}
#endif
41 changes: 41 additions & 0 deletions Sources/BraveWallet/RemoveAccountConfirmationView.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2022 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 DesignSystem
import Strings
import SwiftUI
import BraveCore

struct RemoveAccountConfirmationView: View {

let account: BraveWallet.AccountInfo
var keyringStore: KeyringStore

var body: some View {
PasswordEntryView(
keyringStore: keyringStore,
message: String.localizedStringWithFormat(Strings.Wallet.removeAccountConfirmationMessage, account.name),
action: { password, completion in
keyringStore.removeSecondaryAccount(
for: account,
password: password,
completion: { success in
completion(success ? nil : .incorrectPassword)
}
)
})
}
}

#if DEBUG
struct RemoveAccountConfirmationView_Previews: PreviewProvider {
static var previews: some View {
RemoveAccountConfirmationView(
account: .previewAccount,
keyringStore: .previewStore
)
}
}
#endif
Loading

0 comments on commit 7f89d78

Please sign in to comment.