[iOS] Create PendingUnsafeResourceStorage

This helper class stores a copy of an UnsafeResource while its allow
list decision is pending.  When the pending decision is finished (either
by allowing or disallowing the threat), the resource is reset.

This CL also adds a new util function that returns the allow list for
an UnsafeResource.

Bug: 1083557
Change-Id: Iec9e32fdc68a59f43c98c33e2f04f1ee2f5f5bdc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2205115
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769911}

ios/chrome/browser/safe_browsing/BUILD.gn

@@ -7,6 +7,8 @@ import("//ios/features.gni")
 
 source_set("safe_browsing") {
   sources = [
+    "pending_unsafe_resource_storage.h",
+    "pending_unsafe_resource_storage.mm",
     "safe_browsing_blocking_page.h",
     "safe_browsing_blocking_page.mm",
     "safe_browsing_error.h",
@@ -95,10 +97,12 @@ source_set("util") {
   ]
 
   deps = [
+    ":allow_list",
     "//base",
     "//components/safe_browsing/core/db:v4_protocol_manager_util",
     "//components/security_interstitials/core",
     "//components/security_interstitials/core:unsafe_resource",
+    "//ios/web/public",
   ]
 
   configs += [ "//build/config/compiler:enable_arc" ]
@@ -107,6 +111,7 @@ source_set("util") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "pending_unsafe_resource_storage_unittest.mm",
     "safe_browsing_blocking_page_unittest.mm",
     "safe_browsing_service_unittest.mm",
     "safe_browsing_tab_helper_unittest.mm",

ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_SAFE_BROWSING_PENDING_UNSAFE_RESOURCE_STORAGE_H_
+#define IOS_CHROME_BROWSER_SAFE_BROWSING_PENDING_UNSAFE_RESOURCE_STORAGE_H_
+
+#include "base/optional.h"
+#include "base/scoped_observer.h"
+#include "base/stl_util.h"
+#include "components/security_interstitials/core/unsafe_resource.h"
+#import "ios/chrome/browser/safe_browsing/safe_browsing_url_allow_list.h"
+
+// Storage object that holds a copy of an UnsafeResource while its allow list
+// decision is pending.  Once the pending decision for a resource is committed
+// or discarded, the UnsafeResource is reset.
+class PendingUnsafeResourceStorage {
+ public:
+  // PendingUnsafeResourceStorage is copyable.
+  PendingUnsafeResourceStorage();
+  PendingUnsafeResourceStorage(const PendingUnsafeResourceStorage& other);
+  PendingUnsafeResourceStorage& operator=(
+      const PendingUnsafeResourceStorage& other);
+  ~PendingUnsafeResourceStorage();
+
+  // Constructs a storage holding |resource|.
+  explicit PendingUnsafeResourceStorage(
+      const security_interstitials::UnsafeResource& resource);
+
+  // Returns the pending UnsafeResource, or null if the pending decision is
+  // finished.
+  const security_interstitials::UnsafeResource* resource() const {
+    return base::OptionalOrNullptr(resource_);
+  }
+
+ private:
+  // Observer that updates the storage when the pending decision for its
+  // resource is completed.
+  class ResourcePolicyObserver : public SafeBrowsingUrlAllowList::Observer {
+   public:
+    ResourcePolicyObserver(PendingUnsafeResourceStorage* storage);
+    ResourcePolicyObserver(const ResourcePolicyObserver& other);
+    ResourcePolicyObserver& operator=(const ResourcePolicyObserver& other);
+    ~ResourcePolicyObserver() override;
+
+   private:
+    // SafeBrowsingUrlAllowList::Observer:
+    void ThreatPolicyUpdated(SafeBrowsingUrlAllowList* allow_list,
+                             const GURL& url,
+                             safe_browsing::SBThreatType threat_type,
+                             SafeBrowsingUrlAllowList::Policy policy) override;
+    void ThreatPolicyBatchUpdated(
+        SafeBrowsingUrlAllowList* allow_list,
+        const GURL& url,
+        const std::set<safe_browsing::SBThreatType>& threat_types,
+        SafeBrowsingUrlAllowList::Policy policy) override;
+    void SafeBrowsingAllowListDestroyed(
+        SafeBrowsingUrlAllowList* allow_list) override;
+
+    PendingUnsafeResourceStorage* storage_ = nullptr;
+    ScopedObserver<SafeBrowsingUrlAllowList, SafeBrowsingUrlAllowList::Observer>
+        scoped_observer_{this};
+  };
+
+  // Updates |policy_observer_| for the current value of |resource_|.
+  void UpdatePolicyObserver();
+
+  // Resets |resource_| and destroys |policy_observer_|.
+  void ResetResource();
+
+  // The resource being stored.  Contains no value after the pending decision
+  // has been either allowed or disallowed.
+  base::Optional<security_interstitials::UnsafeResource> resource_;
+  // The observer for |resource_|'s pending decision.
+  base::Optional<ResourcePolicyObserver> policy_observer_;
+};
+
+#endif  // IOS_CHROME_BROWSER_SAFE_BROWSING_PENDING_UNSAFE_RESOURCE_STORAGE_H_

ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage.h"
+
+#include "base/memory/ptr_util.h"
+#import "ios/chrome/browser/safe_browsing/unsafe_resource_util.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using safe_browsing::SBThreatType;
+using security_interstitials::UnsafeResource;
+
+namespace {
+// Returns whether a pending decision exists for |resource|.
+bool IsUnsafeResourcePending(const UnsafeResource& resource) {
+  SafeBrowsingUrlAllowList* allow_list = GetAllowListForResource(resource);
+  std::set<SBThreatType> pending_threat_types;
+  return allow_list &&
+         allow_list->IsUnsafeNavigationDecisionPending(resource.url,
+                                                       &pending_threat_types) &&
+         pending_threat_types.find(resource.threat_type) !=
+             pending_threat_types.end();
+}
+}  // namespace
+
+#pragma mark - PendingUnsafeResourceStorage
+
+PendingUnsafeResourceStorage::PendingUnsafeResourceStorage() = default;
+
+PendingUnsafeResourceStorage::PendingUnsafeResourceStorage(
+    const PendingUnsafeResourceStorage& other)
+    : resource_(other.resource_) {
+  UpdatePolicyObserver();
+}
+
+PendingUnsafeResourceStorage& PendingUnsafeResourceStorage::operator=(
+    const PendingUnsafeResourceStorage& other) {
+  resource_ = other.resource_;
+  UpdatePolicyObserver();
+  return *this;
+}
+
+PendingUnsafeResourceStorage::~PendingUnsafeResourceStorage() = default;
+
+PendingUnsafeResourceStorage::PendingUnsafeResourceStorage(
+    const security_interstitials::UnsafeResource& resource)
+    : resource_(resource) {
+  DCHECK(IsUnsafeResourcePending(resource));
+  // Reset the resource's callback to prevent misuse.
+  resource_.value().callback = base::DoNothing();
+  // Create the policy observer for |resource|.
+  UpdatePolicyObserver();
+}
+
+#pragma mark Private
+
+void PendingUnsafeResourceStorage::UpdatePolicyObserver() {
+  if (resource_) {
+    policy_observer_ = ResourcePolicyObserver(this);
+  } else {
+    policy_observer_ = base::nullopt;
+  }
+}
+
+void PendingUnsafeResourceStorage::ResetResource() {
+  resource_ = base::nullopt;
+  policy_observer_ = base::nullopt;
+}
+
+#pragma mark - PendingUnsafeResourceStorage::ResourcePolicyObserver
+
+PendingUnsafeResourceStorage::ResourcePolicyObserver::ResourcePolicyObserver(
+    PendingUnsafeResourceStorage* storage)
+    : storage_(storage) {
+  scoped_observer_.Add(SafeBrowsingUrlAllowList::FromWebState(
+      storage_->resource()->web_state_getter.Run()));
+}
+
+PendingUnsafeResourceStorage::ResourcePolicyObserver::ResourcePolicyObserver(
+    const ResourcePolicyObserver& other)
+    : storage_(other.storage_) {
+  scoped_observer_.Add(SafeBrowsingUrlAllowList::FromWebState(
+      storage_->resource()->web_state_getter.Run()));
+}
+
+PendingUnsafeResourceStorage::ResourcePolicyObserver&
+PendingUnsafeResourceStorage::ResourcePolicyObserver::operator=(
+    const ResourcePolicyObserver& other) {
+  storage_ = other.storage_;
+  return *this;
+}
+
+PendingUnsafeResourceStorage::ResourcePolicyObserver::
+    ~ResourcePolicyObserver() = default;
+
+void PendingUnsafeResourceStorage::ResourcePolicyObserver::ThreatPolicyUpdated(
+    SafeBrowsingUrlAllowList* allow_list,
+    const GURL& url,
+    safe_browsing::SBThreatType threat_type,
+    SafeBrowsingUrlAllowList::Policy policy) {
+  const UnsafeResource* resource = storage_->resource();
+  if (policy == SafeBrowsingUrlAllowList::Policy::kPending ||
+      url != resource->url || threat_type != resource->threat_type) {
+    return;
+  }
+
+  storage_->ResetResource();
+  // ResetResource() destroys |this|, so no additional code should be added.
+}
+
+void PendingUnsafeResourceStorage::ResourcePolicyObserver::
+    ThreatPolicyBatchUpdated(
+        SafeBrowsingUrlAllowList* allow_list,
+        const GURL& url,
+        const std::set<safe_browsing::SBThreatType>& threat_types,
+        SafeBrowsingUrlAllowList::Policy policy) {
+  const UnsafeResource* resource = storage_->resource();
+  if (policy == SafeBrowsingUrlAllowList::Policy::kPending ||
+      url != resource->url ||
+      threat_types.find(resource->threat_type) == threat_types.end()) {
+    return;
+  }
+
+  storage_->ResetResource();
+  // ResetResource() destroys |this|, so no additional code should be added.
+}
+
+void PendingUnsafeResourceStorage::ResourcePolicyObserver::
+    SafeBrowsingAllowListDestroyed(SafeBrowsingUrlAllowList* allow_list) {
+  storage_->ResetResource();
+  // ResetResource() destroys |this|, so no additional code should be added.
+}

ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage_unittest.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/safe_browsing/pending_unsafe_resource_storage.h"
+
+#include "base/bind.h"
+#import "ios/web/public/test/fakes/test_web_state.h"
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using safe_browsing::SBThreatType;
+using security_interstitials::UnsafeResource;
+
+class PendingUnsafeResourceStorageTest : public PlatformTest {
+ protected:
+  PendingUnsafeResourceStorageTest()
+      : url_("http://www.chromium.test"),
+        threat_type_(safe_browsing::SB_THREAT_TYPE_URL_PHISHING) {
+    // Create a resource and add it as a pending decision.
+    UnsafeResource resource;
+    resource.url = url_;
+    resource.web_state_getter = web_state_.CreateDefaultGetter();
+    resource.threat_type = threat_type_;
+    resource.callback =
+        base::BindRepeating(&PendingUnsafeResourceStorageTest::ResourceCallback,
+                            base::Unretained(this));
+    SafeBrowsingUrlAllowList::CreateForWebState(&web_state_);
+    allow_list()->AddPendingUnsafeNavigationDecision(url_, threat_type_);
+
+    // Create a storage for |resource|.
+    storage_ = PendingUnsafeResourceStorage(resource);
+  }
+
+  SafeBrowsingUrlAllowList* allow_list() {
+    return SafeBrowsingUrlAllowList::FromWebState(&web_state_);
+  }
+
+  void ResourceCallback(bool proceed, bool showed_interstitial) {
+    resource_callback_executed_ = true;
+  }
+
+  web::TestWebState web_state_;
+  const GURL url_;
+  const SBThreatType threat_type_;
+  bool resource_callback_executed_ = false;
+  PendingUnsafeResourceStorage storage_;
+};
+
+// Tests that the unsafe resource is returned while the decision is still
+// pending.
+TEST_F(PendingUnsafeResourceStorageTest, PendingResourceValue) {
+  const UnsafeResource* resource = storage_.resource();
+  ASSERT_TRUE(resource);
+  EXPECT_EQ(url_, resource->url);
+  EXPECT_EQ(threat_type_, resource->threat_type);
+}
+
+// Tests that the unsafe resource's callback is reset to a no-op callback.
+TEST_F(PendingUnsafeResourceStorageTest, NoOpCallback) {
+  const UnsafeResource* resource = storage_.resource();
+  ASSERT_TRUE(resource);
+  resource->callback.Run(/*proceed=*/false, /*showed_interstitial=*/false);
+  EXPECT_FALSE(resource_callback_executed_);
+}
+
+// Tests that the unsafe resource is reset if the threat type is allowed.
+TEST_F(PendingUnsafeResourceStorageTest, AllowPendingResource) {
+  ASSERT_TRUE(storage_.resource());
+
+  allow_list()->AllowUnsafeNavigations(url_, threat_type_);
+  EXPECT_FALSE(storage_.resource());
+}
+
+// Tests that the unsafe resource is reset if the pending decision is removed.
+TEST_F(PendingUnsafeResourceStorageTest, RemovePendingDecision) {
+  ASSERT_TRUE(storage_.resource());
+
+  allow_list()->RemovePendingUnsafeNavigationDecisions(url_);
+  EXPECT_FALSE(storage_.resource());
+}
+
+// Tests that the constructors and assign operator work correctly.
+TEST_F(PendingUnsafeResourceStorageTest, Constructors) {
+  ASSERT_TRUE(storage_.resource());
+
+  // Verify copy constructor.
+  PendingUnsafeResourceStorage storage_copy(storage_);
+  ASSERT_TRUE(storage_copy.resource());
+  EXPECT_EQ(url_, storage_copy.resource()->url);
+  EXPECT_EQ(threat_type_, storage_copy.resource()->threat_type);
+
+  // Verify assignment operator.
+  PendingUnsafeResourceStorage assigned_storage = storage_;
+  ASSERT_TRUE(assigned_storage.resource());
+  EXPECT_EQ(url_, assigned_storage.resource()->url);
+  EXPECT_EQ(threat_type_, assigned_storage.resource()->threat_type);
+
+  // Verify that the default constructo creates an empty storage.
+  storage_ = PendingUnsafeResourceStorage();
+  EXPECT_FALSE(storage_.resource());
+
+  // Verify that the copied storages reset properly when the pending decision is
+  // finished.
+  allow_list()->RemovePendingUnsafeNavigationDecisions(url_);
+  EXPECT_FALSE(storage_copy.resource());
+  EXPECT_FALSE(assigned_storage.resource());
+}

ios/chrome/browser/safe_browsing/unsafe_resource_util.h

@@ -8,6 +8,8 @@
 #include "components/security_interstitials/core/base_safe_browsing_error_ui.h"
 #include "components/security_interstitials/core/unsafe_resource.h"
 
+class SafeBrowsingUrlAllowList;
+
 // Runs |resource|'s callback on the appropriate thread.
 void RunUnsafeResourceCallback(
     const security_interstitials::UnsafeResource& resource,
@@ -23,4 +25,8 @@ GetUnsafeResourceInterstitialReason(
 std::string GetUnsafeResourceMetricPrefix(
     const security_interstitials::UnsafeResource& resource);
 
+// Returns the SafeBrowsingUrlAllowList for |resource|.
+SafeBrowsingUrlAllowList* GetAllowListForResource(
+    const security_interstitials::UnsafeResource& resource);
+
 #endif  // IOS_CHROME_BROWSER_SAFE_BROWSING_UNSAFE_RESOURCE_UTIL_H_

ios/chrome/browser/safe_browsing/unsafe_resource_util.mm

@@ -4,6 +4,9 @@
 
 #import "ios/chrome/browser/safe_browsing/unsafe_resource_util.h"
 
+#import "ios/chrome/browser/safe_browsing/safe_browsing_url_allow_list.h"
+#import "ios/web/public/web_state.h"
+
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
@@ -73,3 +76,13 @@ std::string GetUnsafeResourceMetricPrefix(
     prefix += "_subresource";
   return prefix;
 }
+
+SafeBrowsingUrlAllowList* GetAllowListForResource(
+    const security_interstitials::UnsafeResource& resource) {
+  if (resource.web_state_getter.is_null())
+    return nullptr;
+  web::WebState* web_state = resource.web_state_getter.Run();
+  if (!web_state)
+    return nullptr;
+  return SafeBrowsingUrlAllowList::FromWebState(web_state);
+}