Skip to content

Commit 33a6a13

Browse files
committed
NEW Allow setting "samesite" attribute for individual cookies.
Also update secure-ness of authentication cookies to match session cookies, and finish securing session cookies.
1 parent 660536a commit 33a6a13

File tree

9 files changed

+217
-193
lines changed

9 files changed

+217
-193
lines changed

src/Control/Cookie.php

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,16 @@ class Cookie
1414
{
1515
use Configurable;
1616

17-
public const SAMESITE_LAX = 'Lax';
18-
1917
public const SAMESITE_STRICT = 'Strict';
2018

19+
public const SAMESITE_LAX = 'Lax';
20+
2121
public const SAMESITE_NONE = 'None';
2222

23-
/**
24-
* @config
25-
*
26-
* @var bool
27-
*/
28-
private static $report_errors = true;
23+
private static bool $report_errors = true;
2924

3025
/**
3126
* Must be "Strict", "Lax", or "None"
32-
* @config
3327
*/
3428
private static string $default_samesite = Cookie::SAMESITE_LAX;
3529

@@ -38,73 +32,71 @@ class Cookie
3832
*
3933
* @return Cookie_Backend
4034
*/
41-
public static function get_inst()
35+
public static function get_inst(): Cookie_Backend
4236
{
43-
return Injector::inst()->get('SilverStripe\\Control\\Cookie_Backend');
37+
return Injector::inst()->get(Cookie_Backend::class);
4438
}
4539

4640
/**
4741
* Set a cookie variable.
4842
*
4943
* Expiry time is set in days, and defaults to 90.
5044
*
51-
* @param string $name
52-
* @param mixed $value
53-
* @param float $expiry
54-
* @param string $path
55-
* @param string $domain
56-
* @param bool $secure
57-
* @param bool $httpOnly
58-
*
5945
* See http://php.net/set_session
6046
*/
6147
public static function set(
62-
$name,
63-
$value,
64-
$expiry = 90,
65-
$path = null,
66-
$domain = null,
67-
$secure = false,
68-
$httpOnly = true
48+
string $name,
49+
string|false $value,
50+
int $expiry = 90,
51+
?string $path = null,
52+
?string $domain = null,
53+
bool $secure = false,
54+
bool $httpOnly = true,
55+
string $sameSite = ''
6956
) {
70-
return Cookie::get_inst()->set($name, $value, $expiry, $path, $domain, $secure, $httpOnly);
57+
if ($sameSite === '') {
58+
$sameSite = static::config()->get('default_samesite') ?? Cookie::SAMESITE_LAX;
59+
}
60+
static::validateSameSite($sameSite);
61+
return Cookie::get_inst()->set($name, $value, $expiry, $path, $domain, $secure, $httpOnly, $sameSite);
7162
}
7263

7364
/**
7465
* Get the cookie value by name. Returns null if not set.
75-
*
76-
* @param string $name
77-
* @param bool $includeUnsent
78-
*
79-
* @return null|string
8066
*/
81-
public static function get($name, $includeUnsent = true)
67+
public static function get(string $name, bool $includeUnsent = true): ?string
8268
{
8369
return Cookie::get_inst()->get($name, $includeUnsent);
8470
}
8571

8672
/**
8773
* Get all the cookies.
88-
*
89-
* @param bool $includeUnsent
90-
*
91-
* @return array
9274
*/
93-
public static function get_all($includeUnsent = true)
75+
public static function get_all(bool $includeUnsent = true): array
9476
{
9577
return Cookie::get_inst()->getAll($includeUnsent);
9678
}
9779

9880
/**
99-
* @param string $name
100-
* @param null|string $path
101-
* @param null|string $domain
102-
* @param bool $secure
103-
* @param bool $httpOnly
81+
* Force the expiry of a cookie by name
82+
*/
83+
public static function force_expiry(
84+
string $name,
85+
?string $path = null,
86+
?string $domain = null,
87+
bool $secure = false,
88+
bool $httpOnly = true,
89+
string $sameSite = ''
90+
): void {
91+
Cookie::get_inst()->forceExpiry($name, $path, $domain, $secure, $httpOnly, $sameSite);
92+
}
93+
94+
/**
95+
* Get the default value for the "samesite" cookie attribute.
10496
*/
105-
public static function force_expiry($name, $path = null, $domain = null, $secure = false, $httpOnly = true)
97+
public static function getDefaultSameSite(): string
10698
{
107-
return Cookie::get_inst()->forceExpiry($name, $path, $domain, $secure, $httpOnly);
99+
return static::config()->get('default_samesite') ?? Cookie::SAMESITE_LAX;
108100
}
109101

110102
/**

src/Control/CookieJar.php

Lines changed: 59 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -20,52 +20,52 @@ class CookieJar implements Cookie_Backend
2020
*
2121
* @var array Existing cookies sent by the browser
2222
*/
23-
protected $existing = [];
23+
protected array $existing = [];
2424

2525
/**
2626
* Hold the current cookies (ie: a mix of those that were sent to us and we
2727
* have set without the ones we've cleared)
2828
*
2929
* @var array The state of cookies once we've sent the response
3030
*/
31-
protected $current = [];
31+
protected array $current = [];
3232

3333
/**
3434
* Hold any NEW cookies that were set by the application and will be sent
3535
* in the next response
3636
*
3737
* @var array New cookies set by the application
3838
*/
39-
protected $new = [];
39+
protected array $new = [];
4040

4141
/**
42-
* When creating the backend we want to store the existing cookies in our
43-
* "existing" array. This allows us to distinguish between cookies we received
44-
* or we set ourselves (and didn't get from the browser)
45-
*
46-
* @param array $cookies The existing cookies to load into the cookie jar.
47-
* Omit this to default to $_COOKIE
42+
* @inheritDoc
4843
*/
49-
public function __construct($cookies = [])
44+
public function __construct(array $cookies = [])
5045
{
5146
$this->current = $this->existing = func_num_args()
5247
? ($cookies ?: []) // Convert empty values to blank arrays
5348
: $_COOKIE;
5449
}
5550

5651
/**
57-
* Set a cookie
58-
*
59-
* @param string $name The name of the cookie
60-
* @param string $value The value for the cookie to hold
61-
* @param float $expiry The number of days until expiry; 0 indicates a cookie valid for the current session
62-
* @param string $path The path to save the cookie on (falls back to site base)
63-
* @param string $domain The domain to make the cookie available on
64-
* @param boolean $secure Can the cookie only be sent over SSL?
65-
* @param boolean $httpOnly Prevent the cookie being accessible by JS
52+
* @inheritDoc
6653
*/
67-
public function set($name, $value, $expiry = 90, $path = null, $domain = null, $secure = false, $httpOnly = true)
68-
{
54+
public function set(
55+
string $name,
56+
string|false $value,
57+
int $expiry = 90,
58+
?string $path = null,
59+
?string $domain = null,
60+
bool $secure = false,
61+
bool $httpOnly = true,
62+
string $sameSite = ''
63+
): void {
64+
if ($sameSite === '') {
65+
$sameSite = Cookie::getDefaultSameSite();
66+
}
67+
Cookie::validateSameSite($sameSite);
68+
6969
//are we setting or clearing a cookie? false values are reserved for clearing cookies (see PHP manual)
7070
$clear = false;
7171
if ($value === false || $value === '' || $expiry < 0) {
@@ -81,7 +81,7 @@ public function set($name, $value, $expiry = 90, $path = null, $domain = null, $
8181
//set the path up
8282
$path = $path ? $path : Director::baseURL();
8383
//send the cookie
84-
$this->outputCookie($name, $value, $expiry, $path, $domain, $secure, $httpOnly);
84+
$this->outputCookie($name, $value, $expiry, $path, $domain, $secure, $httpOnly, $sameSite);
8585
//keep our variables in check
8686
if ($clear) {
8787
unset($this->new[$name], $this->current[$name]);
@@ -94,13 +94,9 @@ public function set($name, $value, $expiry = 90, $path = null, $domain = null, $
9494
* Get the cookie value by name
9595
*
9696
* Cookie names are normalised to work around PHP's behaviour of replacing incoming variable name . with _
97-
*
98-
* @param string $name The name of the cookie to get
99-
* @param boolean $includeUnsent Include cookies we've yet to send when fetching values
100-
*
101-
* @return string|null The cookie value or null if unset
97+
* @inheritDoc
10298
*/
103-
public function get($name, $includeUnsent = true)
99+
public function get(string $name, bool $includeUnsent = true): ?string
104100
{
105101
$cookies = $includeUnsent ? $this->current : $this->existing;
106102
if (isset($cookies[$name])) {
@@ -116,28 +112,25 @@ public function get($name, $includeUnsent = true)
116112
}
117113

118114
/**
119-
* Get all the cookies
120-
*
121-
* @param boolean $includeUnsent Include cookies we've yet to send
122-
* @return array All the cookies
115+
* @inheritDoc
123116
*/
124-
public function getAll($includeUnsent = true)
117+
public function getAll(bool $includeUnsent = true): array
125118
{
126119
return $includeUnsent ? $this->current : $this->existing;
127120
}
128121

129122
/**
130-
* Force the expiry of a cookie by name
131-
*
132-
* @param string $name The name of the cookie to expire
133-
* @param string $path The path to save the cookie on (falls back to site base)
134-
* @param string $domain The domain to make the cookie available on
135-
* @param boolean $secure Can the cookie only be sent over SSL?
136-
* @param boolean $httpOnly Prevent the cookie being accessible by JS
123+
* @inheritDoc
137124
*/
138-
public function forceExpiry($name, $path = null, $domain = null, $secure = false, $httpOnly = true)
139-
{
140-
$this->set($name, false, -1, $path, $domain, $secure, $httpOnly);
125+
public function forceExpiry(
126+
string $name,
127+
?string $path = null,
128+
?string $domain = null,
129+
bool $secure = false,
130+
bool $httpOnly = true,
131+
string $sameSite = ''
132+
): void {
133+
$this->set($name, false, -1, $path, $domain, $secure, $httpOnly, $sameSite);
141134
}
142135

143136
/**
@@ -146,33 +139,38 @@ public function forceExpiry($name, $path = null, $domain = null, $secure = false
146139
* @see http://uk3.php.net/manual/en/function.setcookie.php
147140
*
148141
* @param string $name The name of the cookie
149-
* @param string|array $value The value for the cookie to hold
142+
* @param string|false $value The value for the cookie to hold. Empty string or false will clear the cookie.
150143
* @param int $expiry A Unix timestamp indicating when the cookie expires; 0 means it will expire at the end of the session
151-
* @param string $path The path to save the cookie on (falls back to site base)
152-
* @param string $domain The domain to make the cookie available on
153-
* @param boolean $secure Can the cookie only be sent over SSL?
154-
* @param boolean $httpOnly Prevent the cookie being accessible by JS
155-
* @return boolean If the cookie was set or not; doesn't mean it's accepted by the browser
144+
* @param ?string $path The path to save the cookie on (falls back to site base)
145+
* @param ?string $domain The domain to make the cookie available on
146+
* @param bool $secure Can the cookie only be sent over SSL?
147+
* @param bool $httpOnly Prevent the cookie being accessible by JS
148+
* @param string $sameSite The "SameSite" value for the cookie. Must be one of "None", "Lax", or "Strict".
149+
* If $sameSite is left empty, the default will be used.
150+
* @return bool If the cookie was set or not; doesn't mean it's accepted by the browser
156151
*/
157152
protected function outputCookie(
158-
$name,
159-
$value,
160-
$expiry = 90,
161-
$path = null,
162-
$domain = null,
163-
$secure = false,
164-
$httpOnly = true
165-
) {
166-
$sameSite = $this->getSameSite($name);
153+
string $name,
154+
string|false $value,
155+
int $expiry = 90,
156+
?string $path = null,
157+
?string $domain = null,
158+
bool $secure = false,
159+
bool $httpOnly = true,
160+
string $sameSite = ''
161+
): bool {
162+
if ($sameSite === '') {
163+
$sameSite = Cookie::getDefaultSameSite();
164+
}
167165
Cookie::validateSameSite($sameSite);
168166
// if headers aren't sent, we can set the cookie
169167
if (!headers_sent($file, $line)) {
170-
return setcookie($name ?? '', $value ?? '', [
171-
'expires' => $expiry ?? 0,
168+
return setcookie($name, $value, [
169+
'expires' => $expiry,
172170
'path' => $path ?? '',
173171
'domain' => $domain ?? '',
174172
'secure' => $this->cookieIsSecure($sameSite, (bool) $secure),
175-
'httponly' => $httpOnly ?? false,
173+
'httponly' => $httpOnly,
176174
'samesite' => $sameSite,
177175
]);
178176
}
@@ -192,15 +190,4 @@ private function cookieIsSecure(string $sameSite, bool $secure): bool
192190
{
193191
return $sameSite === 'None' ? true : $secure;
194192
}
195-
196-
/**
197-
* Get the correct samesite value - Session cookies use a different configuration variable.
198-
*/
199-
private function getSameSite(string $name): string
200-
{
201-
if ($name === session_name()) {
202-
return Session::config()->get('cookie_samesite') ?? Cookie::SAMESITE_LAX;
203-
}
204-
return Cookie::config()->get('default_samesite') ?? Cookie::SAMESITE_LAX;
205-
}
206193
}

0 commit comments

Comments
 (0)