Allows AwCookieManager to block ThirdParty cookies

android_webview/browser/aw_cookie_access_policy_unittest.cc

Index: android_webview/browser/aw_cookie_access_policy_unittest.cc
diff --git a/android_webview/browser/aw_cookie_access_policy_unittest.cc b/android_webview/browser/aw_cookie_access_policy_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..75c0b16812394acb4766549cfe82ddc0695cbdf9
--- /dev/null
+++ b/android_webview/browser/aw_cookie_access_policy_unittest.cc
@@ -0,0 +1,170 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[mkosiba (inactive), 2014/04/23 12:51:22]

Back to the future? It's 2014 now :)

Seriously though: New files get the current year (and the new policy is to not
include the '(c)' bit), you don't touch the copyright on old files though.

[hjd_google, 2014/04/24 12:48:30]

The dangers of copy-paste :)

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "android_webview/browser/aw_cookie_access_policy.h"
+
+#include "base/run_loop.h"

[mkosiba (inactive), 2014/04/23 12:51:22]

needed?

[hjd_google, 2014/04/24 12:48:30]

See below.

+#include "net/base/request_priority.h"
+#include "net/cookies/canonical_cookie.h"
+#include "net/url_request/url_request_test_util.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+class ResourceContext;
+}
+
+namespace net {
+class CookieOptions;
+}
+
+class GURL;
+
+using android_webview::AwCookieAccessPolicy;
+using net::CookieList;
+using net::TestDelegate;
+using net::TestJobInterceptor;
+using net::TestNetworkDelegate;
+using net::TestURLRequestContext;
+using net::TestURLRequest;
+using testing::Test;
+
+class AwCookieAccessPolicyTest : public Test {
+ public:
+  static const GURL kUrlFirstParty;
+  static const GURL kUrlThirdParty;
+
+  AwCookieAccessPolicyTest()
+      : loop_(),
+        context_(),
+        url_request_delegate_(),
+        network_delegate_(),
+        first_party_request_() {}
+
+  virtual void SetUp() {
+    context_.set_network_delegate(&network_delegate_);
+    first_party_request_.reset(new TestURLRequest(kUrlFirstParty,
+                                                  net::DEFAULT_PRIORITY,
+                                                  &url_request_delegate_,
+                                                  &context_));
+    first_party_request_->set_method("GET");
+
+    third_party_request_.reset(new TestURLRequest(kUrlThirdParty,
+                                                  net::DEFAULT_PRIORITY,
+                                                  &url_request_delegate_,
+                                                  &context_));
+    third_party_request_->set_method("GET");
+    third_party_request_->set_first_party_for_cookies(kUrlFirstParty);
+  }
+
+  AwCookieAccessPolicy* policy() {
+    return AwCookieAccessPolicy::GetInstance();
+  }
+
+  bool GetGlobalAllowAccess() {
+    return policy()->GetGlobalAllowAccess();
+  }
+
+  void SetGlobalAllowAccess(bool allow) {
+    policy()->SetGlobalAllowAccess(allow);
+  }
+
+  bool GetThirdPartyAllowAccess() {
+    return policy()->GetThirdPartyAllowAccess();
+  }
+
+  void SetThirdPartyAllowAccess(bool allow) {
+    policy()->SetThirdPartyAllowAccess(allow);
+  }
+
+  bool OnCanGetCookies(const net::URLRequest& request) {
+    return policy()->OnCanGetCookies(request, CookieList());
+  }
+
+  bool OnCanSetCookie(const net::URLRequest& request) {
+    return policy()->OnCanSetCookie(request, "", NULL);
+  }
+
+  bool AllowGetCookie(const GURL& url, const GURL& first_party) {
+    return policy()->AllowGetCookie(url, first_party, CookieList(), NULL, 0, 0);
+  }
+
+  bool AllowSetCookie(const GURL& url, const GURL& first_party) {
+    return policy()->AllowSetCookie(url, first_party, "", NULL, 0, 0, NULL);
+  }
+
+ protected:
+  base::MessageLoopForIO loop_;

[mkosiba (inactive), 2014/04/23 12:51:22]

do you still need this?

[hjd_google, 2014/04/24 12:48:30]

When I remove it I get an error at url_request.cc(295) because 
URLRequest::Init DCHECKs base::MessageLoop::current() :(

URLRequest doesn't seem to use MessageLoop directly but I left it in for now.

[mkosiba (inactive), 2014/04/24 13:19:29]

ah, ok. That's fine. It just means that URLRequests call out the IO message loop
explicitly by id.

+  TestURLRequestContext context_;
+  TestDelegate url_request_delegate_;
+  TestNetworkDelegate network_delegate_;
+  scoped_ptr<TestURLRequest> first_party_request_;
+  scoped_ptr<TestURLRequest> third_party_request_;
+};
+
+const GURL AwCookieAccessPolicyTest::kUrlFirstParty =
+    GURL("http://first.example");
+const GURL AwCookieAccessPolicyTest::kUrlThirdParty =
+    GURL("http://third.example");
+
+TEST_F(AwCookieAccessPolicyTest, AllowAllCookies) {
+  SetGlobalAllowAccess(true);
+  SetThirdPartyAllowAccess(true);
+
+  EXPECT_TRUE(AllowSetCookie(kUrlFirstParty, kUrlFirstParty));

[mkosiba (inactive), 2014/04/23 12:51:22]

you could refactor this into a separate method, something like

  void DoCookiePolicyTest(bool expected_accept_first_party, bool
expected_accept_third_party) {
    EXPECT_EQ(expected_accept_first_party, AllowSetCookie(...));
    ...
  }

and call that from each of the cases

or use gtest's built in support:
https://code.google.com/p/googletest/wiki/AdvancedGuide#Value_Parameterized_T...

[hjd_google, 2014/04/24 12:48:30]

I had a look at the parameterized tests I think it would come out looking like
this (pseudocode): 
 
TEST_P(AwCookieAccessPolicyTest, CookiePolicy) {
  globalAllowAccess, thirdPartyAllowAccess, 
      expectedFirstParty expectedThirdParty = GetParams();

  SetGlobalAllowAccess(globalAllowAccess);
  SetThirdPartyAllowAccess(thirdPartyAllowAccess);

  EXPECT_EQ(expectedFirstParty, AllowSetCookie(...));
  EXPECT_EQ(expectedFirstParty, AllowGetCookie(...));
  EXPECT_EQ(expectedFirstParty, OnCanGetCookies(...));
  EXPECT_EQ(expectedFirstParty, OnCanSetCookie(...));

  EXPECT_EQ(expectedThirdParty, AllowSetCookie(...));
  EXPECT_EQ(expectedThirdParty, AllowGetCookie(...));
  EXPECT_EQ(expectedThirdParty, OnCanGetCookies(...));
  EXPECT_EQ(expectedThirdParty, OnCanSetCookie(...));
}

INSTANTIATE_TEST_CASE_P(
    CookiePolicy,
    AwCookieAccessPolicyTest,
    ::testing::Values(
    // global, thirdparty, firstparty allowed, thirdparty allowed 
    {false, false, false, false},
    {false, true,  false,  false},
    {true,  false, true,  false},
    {true,  true,  true,  true},
));

Which is really concise but I'm not sure how readable it is especially since the
tests would come out named:

PolicyTests/AwCookieTest.Policy/0 for {true, true, false false}
etc.

which isn't especially useful.

Currently I've kept the explicitly separate tests but extracted some of the
duplication, let me know what you think.

[mkosiba (inactive), 2014/04/24 13:19:29]

current approach looks great!

+  EXPECT_TRUE(AllowGetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_TRUE(OnCanGetCookies(*first_party_request_));
+  EXPECT_TRUE(OnCanSetCookie(*first_party_request_));
+
+  EXPECT_TRUE(AllowSetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_TRUE(AllowGetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_TRUE(OnCanGetCookies(*third_party_request_));
+  EXPECT_TRUE(OnCanSetCookie(*third_party_request_));
+}
+
+TEST_F(AwCookieAccessPolicyTest, BlockAllCookies) {
+  SetGlobalAllowAccess(false);
+  SetThirdPartyAllowAccess(false);
+
+  EXPECT_FALSE(AllowSetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_FALSE(AllowGetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_FALSE(OnCanGetCookies(*first_party_request_));
+  EXPECT_FALSE(OnCanSetCookie(*first_party_request_));
+
+  EXPECT_FALSE(AllowSetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(AllowGetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(OnCanGetCookies(*third_party_request_));
+  EXPECT_FALSE(OnCanSetCookie(*third_party_request_));
+}
+
+TEST_F(AwCookieAccessPolicyTest, BlockAllCookiesWithThirdPartySet) {
+  SetGlobalAllowAccess(false);
+  SetThirdPartyAllowAccess(true);
+
+  EXPECT_FALSE(AllowSetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_FALSE(AllowGetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_FALSE(OnCanGetCookies(*first_party_request_));
+  EXPECT_FALSE(OnCanSetCookie(*first_party_request_));
+
+  EXPECT_FALSE(AllowSetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(AllowGetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(OnCanGetCookies(*third_party_request_));
+  EXPECT_FALSE(OnCanSetCookie(*third_party_request_));
+}
+
+TEST_F(AwCookieAccessPolicyTest, FirstPartyCookiesOnly) {
+  SetGlobalAllowAccess(true);
+  SetThirdPartyAllowAccess(false);
+
+  EXPECT_TRUE(AllowSetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_TRUE(AllowGetCookie(kUrlFirstParty, kUrlFirstParty));
+  EXPECT_TRUE(OnCanGetCookies(*first_party_request_));
+  EXPECT_TRUE(OnCanSetCookie(*first_party_request_));
+
+  EXPECT_FALSE(AllowSetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(AllowGetCookie(kUrlFirstParty, kUrlThirdParty));
+  EXPECT_FALSE(OnCanGetCookies(*third_party_request_));
+  EXPECT_FALSE(OnCanSetCookie(*third_party_request_));
+}
+