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.

Sign up for GitHub

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

Added SameSite attribute to CSRF cookie #3037

Closed
wants to merge 5 commits into from
Closed

Added SameSite attribute to CSRF cookie #3037

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad5d044
Update Controller.php
nControl88 Apr 10, 2020
35cdf1e
Added SameSite attribute to config file
nControl88 May 27, 2020
29d77d1
Added SameSite attribute to setcookie()
nControl88 May 27, 2020
8031b03
Update Controller.php
nControl88 May 27, 2020
fa12995
Update App.php
nControl88 Jun 3, 2020
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
2 changes: 2 additions & 0 deletions app/Config/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,15 @@ class App extends BaseConfig
| CSRFTokenName = The token name
| CSRFHeaderName = The header name
| CSRFCookieName = The cookie name
| CSRFCookieSameSite = The cookie SameSite attribute (none, Lax or Strict)
Copy link
Member

Choose a reason for hiding this comment

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

Case for these should match what a developer is expected to use. You're using all lowercase below, but these are listed out as Initial Caps.

| CSRFExpire = The number in seconds the token should expire.
| CSRFRegenerate = Regenerate token on every submission
| CSRFRedirect = Redirect to previous page with error on failure
*/
public $CSRFTokenName = 'csrf_test_name';
public $CSRFHeaderName = 'X-CSRF-TOKEN';
public $CSRFCookieName = 'csrf_cookie_name';
public $CSRFCookieSameSite = 'strict';
public $CSRFExpire = 7200;
public $CSRFRegenerate = true;
public $CSRFRedirect = true;
Expand Down
28 changes: 25 additions & 3 deletions system/Security/Security.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ class Security
* @var string
*/
protected $CSRFCookieName = 'CSRFToken';

/**
* CSRF Cookie SameSite attribute
*
* SameSite attribute for Cross Site Request Forgery protection cookie.
*
* @var string
*/
protected $CSRFCookieSameSite = 'Strict';

/**
* CSRF Regenerate
Expand Down Expand Up @@ -181,6 +190,7 @@ public function __construct($config)
// Store our CSRF-related settings
$this->CSRFExpire = $config->CSRFExpire;
$this->CSRFTokenName = $config->CSRFTokenName;
$this->CSRFCookieSameSite = $config->CSRFCookieSameSite;
$this->CSRFHeaderName = $config->CSRFHeaderName;
$this->CSRFCookieName = $config->CSRFCookieName;
$this->CSRFRegenerate = $config->CSRFRegenerate;
Expand Down Expand Up @@ -283,9 +293,21 @@ public function CSRFSetCookie(RequestInterface $request)
return false;
}

setcookie(
$this->CSRFCookieName, $this->CSRFHash, $expire, $this->cookiePath, $this->cookieDomain, $secure_cookie, true // Enforce HTTP only cookie for security
);
if (PHP_VERSION_ID < 70300) {
setcookie(
$this->CSRFCookieName, $this->CSRFHash, $expire, $this->cookiePath . "; samesite=" . $this->CSRFCookieSameSite, $this->cookieDomain, $secure_cookie, true // Enforce HTTP only cookie for security
);
}
else {
setcookie($this->CSRFCookieName, $this->CSRFHash, [
'expires' => $expire,
'path' => $this->cookiePath,
'domain' => $this->cookieDomain,
'secure' => $secure_cookie,
'httponly' => true, // Enforce HTTP only cookie for security
'samesite' => $this->CSRFCookieSameSite,
]);
}

log_message('info', 'CSRF cookie sent');

Expand Down