Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Malicious site protection address bar and privacy dashboard changes #3718

Open
wants to merge 2 commits into
base: alessandro/malicious-site-protection-navigation-detection-async
Choose a base branch
from
Open
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
8 changes: 6 additions & 2 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@
9F254B032CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254B022CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift */; };
9F254B052CF9FB890063B308 /* SpecialErrorPageContextHandling.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254B042CF9FB890063B308 /* SpecialErrorPageContextHandling.swift */; };
9F254B082CF9FC270063B308 /* SpecialErrorModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F254B072CF9FC270063B308 /* SpecialErrorModel.swift */; };
9F38A28C2D09BDE500EB100E /* SpecialErrorPageThreatProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F38A28B2D09BDE500EB100E /* SpecialErrorPageThreatProvider.swift */; };
9F46BEF82CD8D7490092E0EF /* OnboardingView+AddToDockContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F46BEF72CD8D7490092E0EF /* OnboardingView+AddToDockContent.swift */; };
9F4CC5152C47AD08006A96EB /* ContextualOnboardingPresenterMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F4CC5142C47AD08006A96EB /* ContextualOnboardingPresenterMock.swift */; };
9F4CC5172C48B8D4006A96EB /* TabViewControllerDaxDialogTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F4CC5162C48B8D4006A96EB /* TabViewControllerDaxDialogTests.swift */; };
Expand Down Expand Up @@ -2645,6 +2646,7 @@
9F254B022CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorPageNavigationDelegate.swift; sourceTree = "<group>"; };
9F254B042CF9FB890063B308 /* SpecialErrorPageContextHandling.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorPageContextHandling.swift; sourceTree = "<group>"; };
9F254B072CF9FC270063B308 /* SpecialErrorModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorModel.swift; sourceTree = "<group>"; };
9F38A28B2D09BDE500EB100E /* SpecialErrorPageThreatProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpecialErrorPageThreatProvider.swift; sourceTree = "<group>"; };
9F46BEF72CD8D7490092E0EF /* OnboardingView+AddToDockContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OnboardingView+AddToDockContent.swift"; sourceTree = "<group>"; };
9F4CC5142C47AD08006A96EB /* ContextualOnboardingPresenterMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContextualOnboardingPresenterMock.swift; sourceTree = "<group>"; };
9F4CC5162C48B8D4006A96EB /* TabViewControllerDaxDialogTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabViewControllerDaxDialogTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5129,6 +5131,7 @@
9F254B002CF9FA8D0063B308 /* SpecialErrorPageActionHandler.swift */,
9F254B022CF9FB2E0063B308 /* SpecialErrorPageNavigationDelegate.swift */,
9F254B042CF9FB890063B308 /* SpecialErrorPageContextHandling.swift */,
9F38A28B2D09BDE500EB100E /* SpecialErrorPageThreatProvider.swift */,
);
path = SpecialErrorPageInterfaces;
sourceTree = "<group>";
Expand Down Expand Up @@ -8150,6 +8153,7 @@
F42EF9312614BABE00101FB9 /* ActionSheetDaxDialogViewController.swift in Sources */,
EEC02C142B0519DE0045CE11 /* NetworkProtectionVPNLocationViewModel.swift in Sources */,
D63FF8962C1B67E9006DE24D /* YoutubeOverlayUserScript.swift in Sources */,
9F38A28C2D09BDE500EB100E /* SpecialErrorPageThreatProvider.swift in Sources */,
F13B4BC01F180D8A00814661 /* TabsModel.swift in Sources */,
8598D2E02CEB98B500C45685 /* Favicons.swift in Sources */,
8598D2E12CEB98B500C45685 /* NotFoundCachingDownloader.swift in Sources */,
Expand Down Expand Up @@ -11857,8 +11861,8 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 221.3.0;
branch = "alessandro/malicious-site-protection-address-bar-and-privacy-dashboard";
kind = branch;
};
};
9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/DuckDuckGo/BrowserServicesKit",
"state" : {
"revision" : "b71ed70ce9b0ef3ce51d4f96da0193ab70493944",
"version" : "221.3.0"
"branch" : "alessandro/malicious-site-protection-address-bar-and-privacy-dashboard",
"revision" : "5bc2e8b352103675dcc038d450d936d34a4101d6"
}
},
{
Expand Down Expand Up @@ -122,8 +122,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/privacy-dashboard",
"state" : {
"revision" : "022c845b06ace6a4aa712a4fa3e79da32193d5c6",
"version" : "7.4.0"
"branch" : "pr-releases/pr-252",
"revision" : "8b9ae746ea7bb2e46588fd4b1a81efabf69c20a4"
}
},
{
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"images" : [
{
"filename" : "Alert-Recolorable-24.pdf",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
}
}
7 changes: 3 additions & 4 deletions DuckDuckGo/Base.lproj/OmniBar.xib
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
<constraint firstAttribute="width" constant="158" id="yFt-j7-mPi"/>
</constraints>
</view>
<imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="center" horizontalHuggingPriority="251" verticalHuggingPriority="251" image="LogoIcon" translatesAutoresizingMaskIntoConstraints="NO" id="eoZ-Ly-QHi">
<imageView clipsSubviews="YES" userInteractionEnabled="NO" contentMode="center" horizontalHuggingPriority="251" verticalHuggingPriority="251" translatesAutoresizingMaskIntoConstraints="NO" id="eoZ-Ly-QHi">
<rect key="frame" x="0.0" y="2" width="43" height="38"/>
</imageView>
</subviews>
Expand All @@ -185,9 +185,9 @@
<constraint firstItem="eoZ-Ly-QHi" firstAttribute="leading" secondItem="99p-YW-bjm" secondAttribute="leading" id="VgX-60-gzX"/>
</constraints>
<connections>
<outlet property="daxLogoImageView" destination="eoZ-Ly-QHi" id="aTN-UG-1Em"/>
<outlet property="shieldAnimationView" destination="dqb-pm-Da8" id="m3S-lp-2hi"/>
<outlet property="shieldDotAnimationView" destination="D46-aN-352" id="yuP-Tc-fZl"/>
<outlet property="staticImageView" destination="eoZ-Ly-QHi" id="DD9-w9-GZy"/>
<outlet property="staticShieldAnimationView" destination="jQy-me-Afa" id="pez-0Q-5Gq"/>
<outlet property="staticShieldDotAnimationView" destination="KE6-Wh-7Yp" id="92x-it-Oyn"/>
<outletCollection property="gestureRecognizers" destination="qXd-RO-1cS" appends="YES" id="3xf-ou-ORG"/>
Expand Down Expand Up @@ -458,7 +458,6 @@
<image name="Clear-24" width="24" height="24"/>
<image name="Close-24" width="24" height="24"/>
<image name="Find-Search-20" width="20" height="20"/>
<image name="LogoIcon" width="24" height="24"/>
<image name="Microphone-24" width="24" height="24"/>
<image name="Reload-24" width="24" height="24"/>
<image name="Settings-24" width="24" height="24"/>
Expand All @@ -467,7 +466,7 @@
<color red="0.96078431372549022" green="0.96078431372549022" blue="0.96078431372549022" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
</namedColor>
<systemColor name="systemGreenColor">
<color red="0.20392156862745098" green="0.7803921568627451" blue="0.34901960784313724" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
<color red="0.20392156859999999" green="0.78039215689999997" blue="0.34901960780000002" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
</systemColor>
<systemColor name="systemYellowColor">
<color red="1" green="0.80000000000000004" blue="0.0" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
Expand Down
10 changes: 8 additions & 2 deletions DuckDuckGo/OmniBar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ extension OmniBar: NibLoading {}

public enum OmniBarIcon: String {
case duckPlayer = "DuckPlayerURLIcon"
case specialError = "Globe-24"
}

class OmniBar: UIView {
Expand Down Expand Up @@ -300,10 +301,15 @@ class OmniBar: UIView {
showCustomIcon(icon: .duckPlayer)
return
}

privacyInfoContainer.privacyIcon.isHidden = privacyInfo.isSpecialErrorPageVisible

if privacyInfo.isSpecialErrorPageVisible {
showCustomIcon(icon: .specialError)
return
}
Comment on lines +305 to +308
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does it look like for ssl error page now? I remember decision was not to show any icon, are we good there?


let icon = PrivacyIconLogic.privacyIcon(for: privacyInfo)
privacyInfoContainer.privacyIcon.updateIcon(icon)
privacyInfoContainer.privacyIcon.isHidden = false
customIconView.isHidden = true
}

Expand Down
2 changes: 2 additions & 0 deletions DuckDuckGo/PrivacyIconLogic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ final class PrivacyIconLogic {
static func privacyIcon(for privacyInfo: PrivacyInfo) -> PrivacyIcon {
if privacyInfo.url.isDuckDuckGoSearch {
return .daxLogo
} else if privacyInfo.malicousSiteThreatKind != .none {
return .alert
} else {
let config = ContentBlocking.shared.privacyConfigurationManager.privacyConfig
let isUserUnprotected = config.isUserUnprotected(domain: privacyInfo.url.host)
Expand Down
27 changes: 20 additions & 7 deletions DuckDuckGo/PrivacyIconView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,20 @@ import UIKit
import Lottie

enum PrivacyIcon {
case daxLogo, shield, shieldWithDot
case daxLogo, shield, shieldWithDot, alert

fileprivate var staticImage: UIImage? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why static?

switch self {
case .daxLogo: return UIImage(resource: .logoIcon)
case .alert: return UIImage(resource: .alertColor24)
default: return nil
}
}
}

class PrivacyIconView: UIView {

@IBOutlet var daxLogoImageView: UIImageView!
@IBOutlet var staticImageView: UIImageView!
@IBOutlet var staticShieldAnimationView: LottieAnimationView!
@IBOutlet var staticShieldDotAnimationView: LottieAnimationView!

Expand Down Expand Up @@ -91,16 +99,17 @@ class PrivacyIconView: UIView {

private func updateShieldImageView(for icon: PrivacyIcon) {
switch icon {
case .daxLogo:
daxLogoImageView.isHidden = false
case .daxLogo, .alert:
staticImageView.isHidden = false
staticImageView.image = icon.staticImage
staticShieldAnimationView.isHidden = true
staticShieldDotAnimationView.isHidden = true
case .shield:
daxLogoImageView.isHidden = true
staticImageView.isHidden = true
staticShieldAnimationView.isHidden = false
staticShieldDotAnimationView.isHidden = true
case .shieldWithDot:
daxLogoImageView.isHidden = true
staticImageView.isHidden = true
staticShieldAnimationView.isHidden = true
staticShieldDotAnimationView.isHidden = false
}
Expand All @@ -116,6 +125,10 @@ class PrivacyIconView: UIView {
accessibilityLabel = UserText.privacyIconShield
accessibilityHint = UserText.privacyIconOpenDashboardHint
accessibilityTraits = .button
case .alert:
accessibilityLabel = UserText.privacyIconShield
accessibilityHint = UserText.privacyIconOpenDashboardHint
accessibilityTraits = .button
}
}

Expand All @@ -134,7 +147,7 @@ class PrivacyIconView: UIView {

staticShieldAnimationView.isHidden = true
staticShieldDotAnimationView.isHidden = true
daxLogoImageView.isHidden = true
staticImageView.isHidden = true
}

func shieldAnimationView(for icon: PrivacyIcon) -> LottieAnimationView? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import WebKit
import SpecialErrorPages

/// A type that defines the base functionality for handling navigation related to special error pages.
protocol SpecialErrorPageContextHandling: AnyObject {
protocol SpecialErrorPageContextHandling: SpecialErrorPageThreatProvider {
/// The delegate that handles navigation actions for special error pages.
var delegate: SpecialErrorPageNavigationDelegate? { get set }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//
// SpecialErrorPageThreatProvider.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import MaliciousSiteProtection

protocol SpecialErrorPageThreatProvider: AnyObject {
/// Provides the current threat kind detected.
///
/// - Returns: An optional `ThreatKind` that indicates the current threat type, or `nil` if no threat is detected.
@MainActor
var currentThreatKind: ThreatKind? { get }
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ enum MaliciousSiteProtectionNavigationResult: Equatable {
}
}

protocol MaliciousSiteProtectionNavigationHandling: AnyObject {
protocol MaliciousSiteProtectionNavigationHandling: SpecialErrorPageThreatProvider {
/// Creates a task for detecting malicious sites based on the provided navigation action.
///
/// - Parameters:
Expand Down Expand Up @@ -78,6 +78,11 @@ final class MaliciousSiteProtectionNavigationHandler {

extension MaliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling {

@MainActor
var currentThreatKind: ThreatKind? {
bypassedMaliciousSiteThreatKind
}

@MainActor
func createMaliciousSiteDetectionTask(for navigationAction: WKNavigationAction, webView: WKWebView) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ final class SpecialErrorPageNavigationHandler: SpecialErrorPageContextHandling {
private let sslErrorPageNavigationHandler: SSLSpecialErrorPageNavigationHandling & SpecialErrorPageActionHandler
private let maliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler

var currentThreatKind: ThreatKind? {
maliciousSiteProtectionNavigationHandler.currentThreatKind
}

init(
sslErrorPageNavigationHandler: SSLSpecialErrorPageNavigationHandling & SpecialErrorPageActionHandler = SSLErrorPageNavigationHandler(),
maliciousSiteProtectionNavigationHandler: MaliciousSiteProtectionNavigationHandling & SpecialErrorPageActionHandler = MaliciousSiteProtectionNavigationHandler()
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/TabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,7 @@ class TabViewController: UIViewController {
let privacyInfo = PrivacyInfo(url: url,
parentEntity: entity,
protectionStatus: makeProtectionStatus(for: host),
malicousSiteThreatKind: specialErrorPageNavigationHandler.currentThreatKind,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo; malicous

shouldCheckServerTrust: shouldCheckServerTrust)
let isValid = certificateTrustEvaluator.evaluateCertificateTrust(trust: webView.serverTrust)
if let isValid {
Expand Down
26 changes: 26 additions & 0 deletions DuckDuckGoTests/PrivacyIconLogicTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,32 @@ class PrivacyIconLogicTests: XCTestCase {
XCTAssertEqual(icon, .shield)
}

func testWhenPrivacyIconThreatKindIsPhishingThenPrivacyIconIsAlert() {
// GIVEN
let url = PrivacyIconLogicTests.pageURL
let protectionStatus = ProtectionStatus(unprotectedTemporary: false, enabledFeatures: [], allowlisted: true, denylisted: false)
let privacyInfo = PrivacyInfo(url: url, parentEntity: nil, protectionStatus: protectionStatus, malicousSiteThreatKind: .phishing)

// WHEN
let icon = PrivacyIconLogic.privacyIcon(for: privacyInfo)

// THEN
XCTAssertEqual(icon, .alert)
}

func testWhenPrivacyIconThreatKindIsMalwareThenPrivacyIconIsAlert() {
// GIVEN
let url = PrivacyIconLogicTests.pageURL
let protectionStatus = ProtectionStatus(unprotectedTemporary: false, enabledFeatures: [], allowlisted: true, denylisted: false)
let privacyInfo = PrivacyInfo(url: url, parentEntity: nil, protectionStatus: protectionStatus, malicousSiteThreatKind: .malware)

// WHEN
let icon = PrivacyIconLogic.privacyIcon(for: privacyInfo)

// THEN
XCTAssertEqual(icon, .alert)
}

}

final class MockSecTrust: SecurityTrust {}
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,27 @@ struct MaliciousSiteProtectionNavigationHandlerTests {
#expect(sut.bypassedMaliciousSiteThreatKind == threat)
}

@MainActor
@Test(
"Threat Kind Returns right value",
arguments: [
ThreatKind.phishing,
.malware
]
)
func whenThreatKindIsCalledReturnRightValue(threat: ThreatKind) throws {
// GIVEN
let url = try #require(URL(string: "https://www.example.com"))
let error = SpecialErrorData.maliciousSite(kind: threat, url: url)
sut.visitSite(url: url, errorData: error)

// WHEN
let result = sut.currentThreatKind

// THEN
#expect(result == threat)
}

@Test("Leave Site Pixel", .disabled("Will be implmented in upcoming PR"))
func whenLeaveSiteActionThenFirePixel() throws {

Expand Down
Loading
Loading