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

Fix #7952: Opening link in private tab will bypass biometrics authentication #7959

Merged
merged 5 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 21 additions & 0 deletions Sources/Brave/Extensions/SceneExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import Foundation
import UIKit
import LocalAuthentication

extension UIWindowScene {
/// A single scene should only have ONE browserViewController
Expand All @@ -26,3 +27,23 @@ extension UIWindowScene {
return browserViewControllers.first
}
}

extension UIViewController {
func askForLocalAuthentication(completion: ((Bool, LAError.Code?) -> Void)? = nil) {
guard let windowProtection = currentScene?.browserViewController?.windowProtection else {
completion?(false, nil)
return
}

// No Pincode set on device
// Local Authentication is not necesseary
if !windowProtection.isPassCodeAvailable {
completion?(true, nil)
} else {
windowProtection.presentAuthenticationForViewController(
determineLockWithPasscode: false, viewType: .general) { status, error in
completion?(status, error)
}
}
}
}
22 changes: 17 additions & 5 deletions Sources/Brave/Frontend/Browser/BrowserViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2987,11 +2987,23 @@ extension BrowserViewController: NewTabPageDelegate {
guard let url = favorite.url else { return }
switch action {
case .opened(let inNewTab, let switchingToPrivateMode):
navigateToInput(
url,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
if switchingToPrivateMode, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.navigateToInput(
url,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
}
}
} else {
navigateToInput(
url,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
}
case .edited:
guard let title = favorite.displayTitle, let urlString = favorite.url else { return }
let editPopup =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Shared
import Foundation
import UIKit
import Data
import Preferences

extension BrowserViewController {

Expand Down Expand Up @@ -49,7 +50,16 @@ extension BrowserViewController {
@objc private func newPrivateTabKeyCommand() {
// NOTE: We cannot and should not distinguish between "new-tab" and "new-private-tab"
// when recording telemetry for key commands.
openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)
if !privateBrowsingManager.isPrivateBrowsing, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)

}
}
} else {
openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)
}
}

@objc private func closeTabKeyCommand() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import BraveCore
import BraveUI
import BraveWallet
import os.log
import Preferences

extension BrowserViewController: TabManagerDelegate {
func tabManager(_ tabManager: TabManager, didSelectedTabChange selected: Tab?, previous: Tab?) {
Expand Down Expand Up @@ -221,7 +222,15 @@ extension BrowserViewController: TabManagerDelegate {
title: Strings.Hotkey.newPrivateTabTitle,
image: UIImage(systemName: "plus.square.fill.on.square.fill"),
handler: UIAction.deferredActionHandler { [unowned self] _ in
self.openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)
if Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)
}
}
} else {
self.openBlankNewTab(attemptLocationFieldFocus: false, isPrivate: true)
}
})

if (UIDevice.current.userInterfaceIdiom == .pad && tabsBar.view.isHidden == true) || (UIDevice.current.userInterfaceIdiom == .phone && toolbar == nil) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import os.log
import Favicon
import Growth
import SafariServices
import LocalAuthentication

extension WKNavigationAction {
/// Allow local requests only if the request is privileged.
Expand Down Expand Up @@ -984,7 +985,15 @@ extension BrowserViewController: WKUIDelegate {
title: Strings.openNewPrivateTabButtonTitle,
image: UIImage(named: "private_glasses", in: .module, compatibleWith: nil)!.template
) { _ in
self.addTab(url: url, inPrivateMode: true, currentTab: currentTab)
if !tabType.isPrivate, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.addTab(url: url, inPrivateMode: true, currentTab: currentTab)
}
}
} else {
self.addTab(url: url, inPrivateMode: true, currentTab: currentTab)
}
}
openNewPrivateTabAction.accessibilityLabel = "linkContextMenu.openInNewPrivateTab"
actions.append(openNewPrivateTabAction)
Expand All @@ -1006,7 +1015,15 @@ extension BrowserViewController: WKUIDelegate {
title: Strings.openInNewPrivateWindowTitle,
image: UIImage(braveSystemNamed: "leo.window.tab-private")
) { _ in
self.openInNewWindow(url: url, isPrivate: true)
if !tabType.isPrivate, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.openInNewWindow(url: url, isPrivate: true)
}
}
} else {
self.openInNewWindow(url: url, isPrivate: true)
}
}

openNewPrivateWindowAction.accessibilityLabel = "linkContextMenu.openInNewPrivateWindow"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,11 +624,23 @@ class NewTabPageViewController: UIViewController {
completion: { _ in }
)
}
delegate?.navigateToInput(
url.absoluteString,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
if switchingToPrivateMode, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.delegate?.navigateToInput(
url.absoluteString,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
}
}
} else {
delegate?.navigateToInput(
url.absoluteString,
inNewTab: inNewTab,
switchingToPrivateMode: switchingToPrivateMode
)
}
// Donate Open Brave News Activity for Custom Suggestions
let openBraveNewsActivity = ActivityShortcutManager.shared.createShortcutActivity(type: .openBraveNews)
self.userActivity = openBraveNewsActivity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Shared
import Data
import MediaPlayer
import os.log
import Preferences

private extension PlaylistListViewController {
func shareItem(_ item: PlaylistInfo, anchorView: UIView?) {
Expand Down Expand Up @@ -106,6 +107,19 @@ private extension PlaylistListViewController {
}
}
}

func openInPrivateTabWithAuthentication(_ item: PlaylistInfo) {
if !isPrivateBrowsing, Preferences.Privacy.privateBrowsingLock.value {
askForLocalAuthentication { [weak self] success, _ in
if success {
self?.openInNewTab(item, isPrivate: true)
}
}
} else {
self.openInNewTab(item, isPrivate: true)
}
}

}

// MARK: UITableViewDelegate
Expand Down Expand Up @@ -173,7 +187,7 @@ extension PlaylistListViewController: UITableViewDelegate {
UIAlertAction(
title: Strings.PlayList.sharePlaylistOpenInNewPrivateTabTitle, style: .default,
handler: { [weak self] _ in
self?.openInNewTab(currentItem, isPrivate: true)
self?.openInPrivateTabWithAuthentication(currentItem)
}))

if !isSharedFolder {
Expand Down Expand Up @@ -258,7 +272,7 @@ extension PlaylistListViewController: UITableViewDelegate {
UIAction(
title: Strings.PlayList.sharePlaylistOpenInNewPrivateTabTitle, image: UIImage(systemName: "plus.square.fill.on.square.fill"),
handler: { [weak self] _ in
self?.openInNewTab(currentItem, isPrivate: true)
self?.openInPrivateTabWithAuthentication(currentItem)
}))

return actions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,17 @@ class BookmarksViewController: SiteTableViewController, ToolbarUrlActionsProtoco
title: Strings.openNewPrivateTabButtonTitle,
image: UIImage(systemName: "plus.square.fill.on.square.fill"),
handler: UIAction.deferredActionHandler { [unowned self] _ in
self.toolbarUrlActionsDelegate?.openInNewTab(bookmarkItemURL, isPrivate: true)
parent?.presentingViewController?.dismiss(animated: true)
if !isPrivateBrowsing, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.toolbarUrlActionsDelegate?.openInNewTab(bookmarkItemURL, isPrivate: true)
self?.parent?.presentingViewController?.dismiss(animated: true)
}
}
} else {
self.toolbarUrlActionsDelegate?.openInNewTab(bookmarkItemURL, isPrivate: true)
parent?.presentingViewController?.dismiss(animated: true)
}
})

let copyAction = UIAction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,16 @@ class HistoryViewController: SiteTableViewController, ToolbarUrlActionsProtocol
let newPrivateTabAction = UIAction(
title: Strings.openNewPrivateTabButtonTitle,
image: UIImage(systemName: "plus.square.fill.on.square.fill"),
handler: UIAction.deferredActionHandler { _ in
self.toolbarUrlActionsDelegate?.openInNewTab(historyItemURL, isPrivate: true)
handler: UIAction.deferredActionHandler { [unowned self] _ in
if !isPrivateBrowsing, Preferences.Privacy.privateBrowsingLock.value {
self.askForLocalAuthentication { [weak self] success, error in
if success {
self?.toolbarUrlActionsDelegate?.openInNewTab(historyItemURL, isPrivate: true)
}
}
} else {
self.toolbarUrlActionsDelegate?.openInNewTab(historyItemURL, isPrivate: true)
}
})

let copyAction = UIAction(
Expand Down
9 changes: 6 additions & 3 deletions Sources/Brave/Frontend/Passcode/WindowProtection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public class WindowProtection {
private var protectedWindow: UIWindow
private var passcodeWindow: UIWindow
private var context = LAContext()
private var viewType: AuthViewType = .general

private var isVisible: Bool = false {
didSet {
Expand Down Expand Up @@ -174,7 +175,7 @@ public class WindowProtection {
}

@objc private func tappedUnlock() {
presentLocalAuthentication()
presentLocalAuthentication(viewType: viewType)
}

@objc private func tappedCancel() {
Expand Down Expand Up @@ -208,7 +209,7 @@ public class WindowProtection {
}
}

private func presentLocalAuthentication(viewType: AuthViewType = .general, completion: ((Bool, LAError.Code?) -> Void)? = nil) {
private func presentLocalAuthentication(viewType: AuthViewType, completion: ((Bool, LAError.Code?) -> Void)? = nil) {
if !context.canEvaluatePolicy(.deviceOwnerAuthentication, error: nil) {
completion?(false, .passcodeNotSet)
return
Expand All @@ -229,7 +230,7 @@ public class WindowProtection {
completion?(true, nil)
})
} else {
lockedViewController.unlockButton.isHidden = false
lockedViewController.unlockButton.isHidden = viewType == .general

let errorPolicy = error as? LAError
completion?(false, errorPolicy?.code)
Expand All @@ -248,6 +249,8 @@ public class WindowProtection {
if isVisible {
return
}

self.viewType = viewType

context = LAContext()
updateVisibleStatusForForeground(determineLockWithPasscode, viewType: viewType) { status, error in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ class SyncSettingsTableViewController: SyncViewController, UITableViewDelegate,
if toggleExistingStatus {
performSyncDataTypeStatusChange(type: syncDataType)
} else {
askForAuthentication() { status, error in
askForAuthentication(viewType: .sync) { status, error in
guard status else {
toggle.setOn(toggleExistingStatus, animated: false)
return
Expand Down Expand Up @@ -336,7 +336,7 @@ class SyncSettingsTableViewController: SyncViewController, UITableViewDelegate,

@objc
private func onSyncInternalsTapped() {
askForAuthentication() { [weak self] status, error in
askForAuthentication(viewType: .sync) { [weak self] status, error in
guard let self = self, status else { return }

let syncInternalsController = self.syncAPI.createSyncInternalsController().then {
Expand Down Expand Up @@ -592,7 +592,7 @@ extension SyncSettingsTableViewController {
}

private func addAnotherDevice() {
askForAuthentication() { [weak self] status, error in
askForAuthentication(viewType: .sync) { [weak self] status, error in
guard let self = self, status else { return }

let view = SyncSelectDeviceTypeViewController()
Expand Down
2 changes: 1 addition & 1 deletion Sources/Brave/Frontend/Sync/SyncViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class SyncViewController: AuthenticationController {
super.viewWillAppear(animated)

if requiresAuthentication {
askForAuthentication() { [weak self] success, error in
askForAuthentication(viewType: .sync) { [weak self] success, error in
guard let self else { return }

if !success, error != .userCancel {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class SyncWelcomeViewController: SyncViewController {

@objc
private func onSyncInternalsAction() {
askForAuthentication() { [weak self] status, error in
askForAuthentication(viewType: .sync) { [weak self] status, error in
guard let self = self, status else { return }

let syncInternalsController = syncAPI.createSyncInternalsController().then {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class AuthenticationController: LoadingViewController {

/// A method to ask biometric authentication to user
/// - Parameter completion: block returning authentication status
func askForAuthentication(viewType: AuthViewType = .sync, completion: ((Bool, LAError.Code?) -> Void)? = nil) {
func askForAuthentication(viewType: AuthViewType, completion: ((Bool, LAError.Code?) -> Void)? = nil) {
guard let windowProtection = windowProtection else {
completion?(false, nil)
return
Expand Down