Skip to content

Commit

Permalink
Fix brave/brave-ios#7952: Opening link in private tab will bypass bio…
Browse files Browse the repository at this point in the history
…metrics authentication (brave/brave-ios#7959)
  • Loading branch information
soner-yuksel authored Aug 28, 2023
1 parent b3a4e52 commit 02a516e
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 29 deletions.
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
2 changes: 1 addition & 1 deletion Sources/Brave/Frontend/Widgets/LoadingViewController.swift
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

0 comments on commit 02a516e

Please sign in to comment.