Commit c52b093d authored by Victor Costan's avatar Victor Costan Committed by Commit Bot

Async Cookies: Implement sameSite attribute.

This CL modifies RestrictedCookieManager to allow renderers to set the
SameSite and Priority CanonicalCookie attributes. Renderers must be
allowed to set the SameSite and Priority attributes so that
RestrictedCookieManager can be used to implement the document.cookie
setter.

Bug: 856364, 729800
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ic0a520bc6c02c259ef22175c9eb161086fa18782
Reviewed-on: https://chromium-review.googlesource.com/1115586
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Reviewed-by: default avatarJoshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570976}
parent 8ee846d8
...@@ -200,14 +200,10 @@ void RestrictedCookieManager::SetCanonicalCookie( ...@@ -200,14 +200,10 @@ void RestrictedCookieManager::SetCanonicalCookie(
// TODO(pwnall): Call NetworkDelegate::CanSetCookie() on a NetworkDelegate // TODO(pwnall): Call NetworkDelegate::CanSetCookie() on a NetworkDelegate
// associated with the NetworkContext. // associated with the NetworkContext.
base::Time now = base::Time::NowFromSystemTime(); base::Time now = base::Time::NowFromSystemTime();
// TODO(pwnall): Reason about whether it makes sense to allow a renderer to
// set these fields.
net::CookieSameSite cookie_same_site_mode = net::CookieSameSite::STRICT_MODE;
net::CookiePriority cookie_priority = net::COOKIE_PRIORITY_DEFAULT;
auto sanitized_cookie = std::make_unique<net::CanonicalCookie>( auto sanitized_cookie = std::make_unique<net::CanonicalCookie>(
cookie.Name(), cookie.Value(), cookie.Domain(), cookie.Path(), now, cookie.Name(), cookie.Value(), cookie.Domain(), cookie.Path(), now,
cookie.ExpiryDate(), now, cookie.IsSecure(), cookie.IsHttpOnly(), cookie.ExpiryDate(), now, cookie.IsSecure(), cookie.IsHttpOnly(),
cookie_same_site_mode, cookie_priority); cookie.SameSite(), cookie.Priority());
// TODO(pwnall): secure_source should depend on url, and might depend on the // TODO(pwnall): secure_source should depend on url, and might depend on the
// renderer. // renderer.
......
...@@ -16,6 +16,9 @@ const kOneDay = 24 * 60 * 60 * 1000; ...@@ -16,6 +16,9 @@ const kOneDay = 24 * 60 * 60 * 1000;
const kTenYears = 10 * 365 * kOneDay; const kTenYears = 10 * 365 * kOneDay;
const kTenYearsFromNow = Date.now() + kTenYears; const kTenYearsFromNow = Date.now() + kTenYears;
const kCookieListItemKeys =
['domain', 'expires', 'name', 'path', 'sameSite', 'secure', 'value'].sort();
promise_test(async testCase => { promise_test(async testCase => {
await cookieStore.delete('cookie-name'); await cookieStore.delete('cookie-name');
...@@ -28,9 +31,8 @@ promise_test(async testCase => { ...@@ -28,9 +31,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null); assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'CookieListItem - cookieStore.set defaults with positional name and value'); }, 'CookieListItem - cookieStore.set defaults with positional name and value');
...@@ -46,9 +48,8 @@ promise_test(async testCase => { ...@@ -46,9 +48,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null); assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'CookieListItem - cookieStore.set defaults with name and value in options'); }, 'CookieListItem - cookieStore.set defaults with name and value in options');
...@@ -65,9 +66,8 @@ promise_test(async testCase => { ...@@ -65,9 +66,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay); assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'CookieListItem - cookieStore.set with expires set to a timestamp 10 ' + }, 'CookieListItem - cookieStore.set with expires set to a timestamp 10 ' +
...@@ -85,9 +85,8 @@ promise_test(async testCase => { ...@@ -85,9 +85,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay); assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'CookieListItem - cookieStore.set with name and value in options and ' + }, 'CookieListItem - cookieStore.set with name and value in options and ' +
...@@ -122,9 +121,8 @@ promise_test(async testCase => { ...@@ -122,9 +121,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay); assert_approx_equals(cookie.expires, kTenYearsFromNow, kOneDay);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(() => cookieStore.delete('cookie-name')); await async_cleanup(() => cookieStore.delete('cookie-name'));
}, 'CookieListItem - cookieStore.set with name and value in options and ' + }, 'CookieListItem - cookieStore.set with name and value in options and ' +
...@@ -142,9 +140,8 @@ promise_test(async testCase => { ...@@ -142,9 +140,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null); assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(async () => { await async_cleanup(async () => {
await cookieStore.delete('cookie-name', { domain: kCurrentHostname }); await cookieStore.delete('cookie-name', { domain: kCurrentHostname });
...@@ -167,9 +164,8 @@ promise_test(async testCase => { ...@@ -167,9 +164,8 @@ promise_test(async testCase => {
assert_equals(cookie.path, currentDirectory); assert_equals(cookie.path, currentDirectory);
assert_equals(cookie.expires, null); assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true); assert_equals(cookie.secure, true);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(async () => { await async_cleanup(async () => {
await cookieStore.delete('cookie-name', { path: currentDirectory }); await cookieStore.delete('cookie-name', { path: currentDirectory });
...@@ -187,11 +183,53 @@ promise_test(async testCase => { ...@@ -187,11 +183,53 @@ promise_test(async testCase => {
assert_equals(cookie.path, '/'); assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null); assert_equals(cookie.expires, null);
assert_equals(cookie.secure, false); assert_equals(cookie.secure, false);
assert_array_equals( assert_equals(cookie.sameSite, 'strict');
Object.keys(cookie).sort(), assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
['domain', 'expires', 'name', 'path', 'secure', 'value'].sort());
await async_cleanup(async () => { await async_cleanup(async () => {
await cookieStore.delete('cookie-name', { secure: false }); await cookieStore.delete('cookie-name', { secure: false });
}); });
}, 'CookieListItem - cookieStore.set with secure set to false'); }, 'CookieListItem - cookieStore.set with secure set to false');
['strict', 'lax', 'unrestricted'].forEach(sameSiteValue => {
promise_test(async testCase => {
await cookieStore.delete('cookie-name', { sameSite: sameSiteValue });
await cookieStore.set({
name: 'cookie-name', value: 'cookie-value', sameSite: sameSiteValue });
const cookie = await cookieStore.get('cookie-name');
assert_equals(cookie.name, 'cookie-name');
assert_equals(cookie.value, 'cookie-value');
assert_equals(cookie.domain, null);
assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true);
assert_equals(cookie.sameSite, sameSiteValue);
assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
await async_cleanup(async () => {
await cookieStore.delete('cookie-name', { secure: sameSiteValue });
});
}, `CookieListItem - cookieStore.set with sameSite set to ${sameSiteValue}`);
promise_test(async testCase => {
await cookieStore.delete('cookie-name', { sameSite: sameSiteValue });
await cookieStore.set('cookie-name', 'cookie-value',
{ sameSite: sameSiteValue });
const cookie = await cookieStore.get('cookie-name');
assert_equals(cookie.name, 'cookie-name');
assert_equals(cookie.value, 'cookie-value');
assert_equals(cookie.domain, null);
assert_equals(cookie.path, '/');
assert_equals(cookie.expires, null);
assert_equals(cookie.secure, true);
assert_equals(cookie.sameSite, sameSiteValue);
assert_array_equals(Object.keys(cookie).sort(), kCookieListItemKeys);
await async_cleanup(async () => {
await cookieStore.delete('cookie-name', { secure: sameSiteValue });
});
}, 'CookieListItem - cookieStore.set with positional name and value and ' +
`sameSite set to ${sameSiteValue}`);
});
// https://github.com/WICG/cookie-store/blob/gh-pages/explainer.md // https://github.com/WICG/cookie-store/blob/gh-pages/explainer.md
enum CookieSameSite {
"strict",
"lax",
"unrestricted"
};
dictionary CookieListItem { dictionary CookieListItem {
USVString name; USVString name;
USVString value; USVString value;
...@@ -7,6 +13,7 @@ dictionary CookieListItem { ...@@ -7,6 +13,7 @@ dictionary CookieListItem {
USVString path; USVString path;
DOMTimeStamp? expires; DOMTimeStamp? expires;
boolean secure; boolean secure;
CookieSameSite sameSite;
}; };
typedef sequence<CookieListItem> CookieList; typedef sequence<CookieListItem> CookieList;
...@@ -57,6 +64,7 @@ dictionary CookieStoreSetOptions { ...@@ -57,6 +64,7 @@ dictionary CookieStoreSetOptions {
USVString path = "/"; USVString path = "/";
boolean secure = true; boolean secure = true;
boolean httpOnly = false; boolean httpOnly = false;
CookieSameSite sameSite = "strict";
}; };
[ [
......
...@@ -43,6 +43,23 @@ CookieChangeEvent::CookieChangeEvent(const AtomicString& type, ...@@ -43,6 +43,23 @@ CookieChangeEvent::CookieChangeEvent(const AtomicString& type,
deleted_ = initializer.deleted(); deleted_ = initializer.deleted();
} }
namespace {
String ToCookieListItemSameSite(network::mojom::CookieSameSite same_site) {
switch (same_site) {
case network::mojom::CookieSameSite::STRICT_MODE:
return "strict";
case network::mojom::CookieSameSite::LAX_MODE:
return "lax";
case network::mojom::CookieSameSite::NO_RESTRICTION:
return "unrestricted";
}
NOTREACHED();
}
} // namespace
// static // static
void CookieChangeEvent::ToCookieListItem( void CookieChangeEvent::ToCookieListItem(
const WebCanonicalCookie& canonical_cookie, const WebCanonicalCookie& canonical_cookie,
...@@ -51,6 +68,7 @@ void CookieChangeEvent::ToCookieListItem( ...@@ -51,6 +68,7 @@ void CookieChangeEvent::ToCookieListItem(
list_item.setName(canonical_cookie.Name()); list_item.setName(canonical_cookie.Name());
list_item.setPath(canonical_cookie.Path()); list_item.setPath(canonical_cookie.Path());
list_item.setSecure(canonical_cookie.IsSecure()); list_item.setSecure(canonical_cookie.IsSecure());
list_item.setSameSite(ToCookieListItemSameSite(canonical_cookie.SameSite()));
// The domain of host-only cookies is the host name, without a dot (.) prefix. // The domain of host-only cookies is the host name, without a dot (.) prefix.
String cookie_domain = canonical_cookie.Domain(); String cookie_domain = canonical_cookie.Domain();
......
...@@ -11,6 +11,7 @@ dictionary CookieListItem { ...@@ -11,6 +11,7 @@ dictionary CookieListItem {
USVString path; USVString path;
DOMTimeStamp? expires; DOMTimeStamp? expires;
boolean secure; boolean secure;
CookieSameSite sameSite;
}; };
typedef sequence<CookieListItem> CookieList; typedef sequence<CookieListItem> CookieList;
...@@ -177,10 +177,19 @@ base::Optional<WebCanonicalCookie> ToWebCanonicalCookie( ...@@ -177,10 +177,19 @@ base::Optional<WebCanonicalCookie> ToWebCanonicalCookie(
"__Secure- and __Host- cookies must be secure"); "__Secure- and __Host- cookies must be secure");
} }
network::mojom::CookieSameSite same_site;
if (options.sameSite() == "strict") {
same_site = network::mojom::CookieSameSite::STRICT_MODE;
} else if (options.sameSite() == "lax") {
same_site = network::mojom::CookieSameSite::LAX_MODE;
} else {
DCHECK_EQ(options.sameSite(), "unrestricted");
same_site = network::mojom::CookieSameSite::NO_RESTRICTION;
}
return WebCanonicalCookie::Create( return WebCanonicalCookie::Create(
name, value, domain, path, WTF::Time() /*creation*/, expiry, name, value, domain, path, WTF::Time() /*creation*/, expiry,
WTF::Time() /*last_access*/, secure, options.httpOnly(), WTF::Time() /*last_access*/, secure, options.httpOnly(), same_site,
WebCanonicalCookie::kDefaultSameSiteMode,
WebCanonicalCookie::kDefaultPriority); WebCanonicalCookie::kDefaultPriority);
} }
......
...@@ -4,6 +4,12 @@ ...@@ -4,6 +4,12 @@
// https://github.com/WICG/async-cookies-api/blob/gh-pages/explainer.md // https://github.com/WICG/async-cookies-api/blob/gh-pages/explainer.md
enum CookieSameSite {
"strict",
"lax",
"unrestricted"
};
dictionary CookieStoreSetOptions { dictionary CookieStoreSetOptions {
USVString name; USVString name;
USVString value; USVString value;
...@@ -12,4 +18,5 @@ dictionary CookieStoreSetOptions { ...@@ -12,4 +18,5 @@ dictionary CookieStoreSetOptions {
USVString path = "/"; USVString path = "/";
boolean secure = true; boolean secure = true;
boolean httpOnly = false; boolean httpOnly = false;
CookieSameSite sameSite = "strict";
}; };
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment