Merge branch '4.13' into 4

This commit is contained in:
Guy Sartorelli 2023-04-26 12:44:05 +12:00
commit 7c5cbae928
No known key found for this signature in database
GPG Key ID: F313E3B9504D496A
9 changed files with 99 additions and 32 deletions

View File

@ -14,6 +14,12 @@ class Cookie
{ {
use Configurable; use Configurable;
public const SAMESITE_LAX = 'Lax';
public const SAMESITE_STRICT = 'Strict';
public const SAMESITE_NONE = 'None';
/** /**
* @config * @config
* *
@ -25,7 +31,7 @@ class Cookie
* Must be "Strict", "Lax", or "None" * Must be "Strict", "Lax", or "None"
* @config * @config
*/ */
private static string $default_samesite = 'Lax'; private static string $default_samesite = self::SAMESITE_LAX;
/** /**
* Fetch the current instance of the cookie backend. * Fetch the current instance of the cookie backend.
@ -110,14 +116,14 @@ class Cookie
public static function validateSameSite(string $sameSite): void public static function validateSameSite(string $sameSite): void
{ {
$validValues = [ $validValues = [
'Strict', self::SAMESITE_STRICT,
'Lax', self::SAMESITE_LAX,
'None', self::SAMESITE_NONE,
]; ];
if (!in_array($sameSite, $validValues)) { if (!in_array($sameSite, $validValues)) {
throw new LogicException('Cookie samesite must be "Strict", "Lax", or "None"'); throw new LogicException('Cookie samesite must be "Strict", "Lax", or "None"');
} }
if ($sameSite === 'None' && !Director::is_https(self::getRequest())) { if ($sameSite === self::SAMESITE_NONE && !Director::is_https(self::getRequest())) {
Injector::inst()->get(LoggerInterface::class)->warning('Cookie samesite cannot be "None" for non-https requests.'); Injector::inst()->get(LoggerInterface::class)->warning('Cookie samesite cannot be "None" for non-https requests.');
} }
} }

View File

@ -204,8 +204,8 @@ class CookieJar implements Cookie_Backend
private function getSameSite(string $name): string private function getSameSite(string $name): string
{ {
if ($name === session_name()) { if ($name === session_name()) {
return Session::config()->get('cookie_samesite'); return Session::config()->get('cookie_samesite') ?? Cookie::SAMESITE_LAX;
} }
return Cookie::config()->get('default_samesite'); return Cookie::config()->get('default_samesite') ?? Cookie::SAMESITE_LAX;
} }
} }

View File

@ -825,10 +825,11 @@ class Director implements TemplateGlobalProvider
return ( return (
// Base check for existence of a host on a compliant URL // Base check for existence of a host on a compliant URL
parse_url($url ?? '', PHP_URL_HOST) parse_url($url ?? '', PHP_URL_HOST)
// Check for more than one leading slash without a protocol. // Check for more than one leading slash (forward or backward) without a protocol.
// While not a RFC compliant absolute URL, it is completed to a valid URL by some browsers, // While not a RFC compliant absolute URL, it is completed to a valid URL by some browsers,
// and hence a potential security risk. Single leading slashes are not an issue though. // and hence a potential security risk. Single leading slashes are not an issue though.
|| preg_match('%^\s*/{2,}%', $url ?? '') // Note: Need 4 backslashes to have a single non-escaped backslash for regex.
|| preg_match('%^\s*(\\\\|/){2,}%', $url ?? '')
|| ( || (
// If a colon is found, check if it's part of a valid scheme definition // If a colon is found, check if it's part of a valid scheme definition
// (meaning its not preceded by a slash). // (meaning its not preceded by a slash).

View File

@ -139,7 +139,7 @@ class Session
* Must be "Strict", "Lax", or "None". * Must be "Strict", "Lax", or "None".
* @config * @config
*/ */
private static string $cookie_samesite = 'Lax'; private static string $cookie_samesite = Cookie::SAMESITE_LAX;
/** /**
* Name of session cache limiter to use. * Name of session cache limiter to use.
@ -374,7 +374,7 @@ class Session
} }
} }
$sameSite = static::config()->get('cookie_samesite'); $sameSite = static::config()->get('cookie_samesite') ?? Cookie::SAMESITE_LAX;
Cookie::validateSameSite($sameSite); Cookie::validateSameSite($sameSite);
$secure = $this->isCookieSecure($sameSite, Director::is_https($request)); $secure = $this->isCookieSecure($sameSite, Director::is_https($request));

View File

@ -228,21 +228,23 @@ class GridFieldPrintButton extends AbstractGridFieldComponent implements GridFie
/** @var DataObject $item */ /** @var DataObject $item */
foreach ($items->limit(null) as $item) { foreach ($items->limit(null) as $item) {
$itemRow = new ArrayList(); if (!$item->hasMethod('canView') || $item->canView()) {
$itemRow = new ArrayList();
foreach ($printColumns as $field => $label) { foreach ($printColumns as $field => $label) {
$value = $gridFieldColumnsComponent $value = $gridFieldColumnsComponent
? strip_tags($gridFieldColumnsComponent->getColumnContent($gridField, $item, $field)) ? strip_tags($gridFieldColumnsComponent->getColumnContent($gridField, $item, $field))
: $gridField->getDataFieldValue($item, $field); : $gridField->getDataFieldValue($item, $field);
$itemRow->push(new ArrayData([ $itemRow->push(new ArrayData([
"CellString" => $value, "CellString" => $value,
]));
}
$itemRows->push(new ArrayData([
"ItemRow" => $itemRow
])); ]));
} }
$itemRows->push(new ArrayData([
"ItemRow" => $itemRow
]));
if ($item->hasMethod('destroy')) { if ($item->hasMethod('destroy')) {
$item->destroy(); $item->destroy();
} }

View File

@ -635,6 +635,11 @@ class Group extends DataObject
return true; return true;
} }
// if user can grant access for specific groups, they need to be able to see the groups
if (Permission::checkMember($member, "SITETREE_GRANT_ACCESS")) {
return true;
}
return false; return false;
} }

View File

@ -233,6 +233,9 @@ class DirectorTest extends SapphireTest
public function testIsAbsoluteUrl() public function testIsAbsoluteUrl()
{ {
$this->assertTrue(Director::is_absolute_url('http://test.com/testpage')); $this->assertTrue(Director::is_absolute_url('http://test.com/testpage'));
$this->assertTrue(Director::is_absolute_url('https:/\\test.com'));
$this->assertTrue(Director::is_absolute_url('https:\\/test.com'));
$this->assertTrue(Director::is_absolute_url('https:\\\\test.com'));
$this->assertTrue(Director::is_absolute_url('ftp://test.com')); $this->assertTrue(Director::is_absolute_url('ftp://test.com'));
$this->assertFalse(Director::is_absolute_url('test.com/testpage')); $this->assertFalse(Director::is_absolute_url('test.com/testpage'));
$this->assertFalse(Director::is_absolute_url('/relative')); $this->assertFalse(Director::is_absolute_url('/relative'));
@ -242,6 +245,11 @@ class DirectorTest extends SapphireTest
$this->assertTrue(Director::is_absolute_url("https://test.com/?url=http://foo.com")); $this->assertTrue(Director::is_absolute_url("https://test.com/?url=http://foo.com"));
$this->assertTrue(Director::is_absolute_url("trickparseurl:http://test.com")); $this->assertTrue(Director::is_absolute_url("trickparseurl:http://test.com"));
$this->assertTrue(Director::is_absolute_url('//test.com')); $this->assertTrue(Director::is_absolute_url('//test.com'));
$this->assertTrue(Director::is_absolute_url('\\/\\/test.com'));
$this->assertTrue(Director::is_absolute_url('\/\/test.com'));
$this->assertTrue(Director::is_absolute_url('/\\test.com'));
$this->assertTrue(Director::is_absolute_url('\\\\test.com'));
$this->assertFalse(Director::is_absolute_url('\\test.com'));
$this->assertTrue(Director::is_absolute_url('/////test.com')); $this->assertTrue(Director::is_absolute_url('/////test.com'));
$this->assertTrue(Director::is_absolute_url(' ///test.com')); $this->assertTrue(Director::is_absolute_url(' ///test.com'));
$this->assertTrue(Director::is_absolute_url('http:test.com')); $this->assertTrue(Director::is_absolute_url('http:test.com'));
@ -259,8 +267,17 @@ class DirectorTest extends SapphireTest
{ {
$this->assertFalse(Director::is_relative_url('http://test.com')); $this->assertFalse(Director::is_relative_url('http://test.com'));
$this->assertFalse(Director::is_relative_url('https://test.com')); $this->assertFalse(Director::is_relative_url('https://test.com'));
$this->assertFalse(Director::is_relative_url('https:/\\test.com'));
$this->assertFalse(Director::is_relative_url('https:\\/test.com'));
$this->assertFalse(Director::is_relative_url('https:\\\\test.com'));
$this->assertFalse(Director::is_relative_url(' https://test.com/testpage ')); $this->assertFalse(Director::is_relative_url(' https://test.com/testpage '));
$this->assertTrue(Director::is_relative_url('test.com/testpage')); $this->assertTrue(Director::is_relative_url('test.com/testpage'));
$this->assertFalse(Director::is_relative_url('//test.com'));
$this->assertFalse(Director::is_relative_url('\\/\\/test.com'));
$this->assertFalse(Director::is_relative_url('\/\/test.com'));
$this->assertFalse(Director::is_relative_url('/\\test.com'));
$this->assertFalse(Director::is_relative_url('\\\\test.com'));
$this->assertTrue(Director::is_relative_url('\\test.com'));
$this->assertFalse(Director::is_relative_url('ftp://test.com')); $this->assertFalse(Director::is_relative_url('ftp://test.com'));
$this->assertTrue(Director::is_relative_url('/relative')); $this->assertTrue(Director::is_relative_url('/relative'));
$this->assertTrue(Director::is_relative_url('relative')); $this->assertTrue(Director::is_relative_url('relative'));
@ -402,17 +419,34 @@ class DirectorTest extends SapphireTest
); );
} }
/**
* Mostly tested by {@link testIsRelativeUrl()},
* just adding the host name matching aspect here.
*/
public function testIsSiteUrl() public function testIsSiteUrl()
{ {
$this->assertFalse(Director::is_site_url("http://test.com")); $this->assertFalse(Director::is_site_url('http://test.com'));
$this->assertTrue(Director::is_site_url('/relative-path'));
$this->assertTrue(Director::is_site_url('relative-path'));
$this->assertTrue(Director::is_site_url(Director::absoluteBaseURL())); $this->assertTrue(Director::is_site_url(Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url("http://test.com?url=" . Director::absoluteBaseURL())); $this->assertFalse(Director::is_site_url('http://test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url("http://test.com?url=" . urlencode(Director::absoluteBaseURL() ?? ''))); $this->assertFalse(Director::is_site_url('http://test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
$this->assertFalse(Director::is_site_url("//test.com?url=" . Director::absoluteBaseURL())); $this->assertFalse(Director::is_site_url('http:\\\\test.com'));
$this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('http:\\\\test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
$this->assertFalse(Director::is_site_url('http:\\/test.com'));
$this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('http:\\/test.com?url=' . urlencode(Director::absoluteBaseURL() ?? '')));
$this->assertFalse(Director::is_site_url('//test.com'));
$this->assertFalse(Director::is_site_url('//test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('\\/\\/test.com'));
$this->assertFalse(Director::is_site_url('\\/\\/test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('\/\/test.com'));
$this->assertFalse(Director::is_site_url('\/\/test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('\\/test.com'));
$this->assertFalse(Director::is_site_url('\\/test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('/\\test.com'));
$this->assertFalse(Director::is_site_url('/\\test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('\\\\test.com'));
$this->assertFalse(Director::is_site_url('\\\\test.com?url=' . Director::absoluteBaseURL()));
$this->assertTrue(Director::is_site_url('\\test.com'));
$this->assertTrue(Director::is_site_url('\\test.com?url=' . Director::absoluteBaseURL()));
$this->assertFalse(Director::is_site_url('http://google.com\@test.com')); $this->assertFalse(Director::is_site_url('http://google.com\@test.com'));
$this->assertFalse(Director::is_site_url('http://google.com/@test.com')); $this->assertFalse(Director::is_site_url('http://google.com/@test.com'));
$this->assertFalse(Director::is_site_url('http://google.com:pass\@test.com')); $this->assertFalse(Director::is_site_url('http://google.com:pass\@test.com'));

View File

@ -32,6 +32,19 @@ class GridFieldPrintButtonTest extends SapphireTest
} }
public function testLimit() public function testLimit()
{
$this->assertEquals(42, $this->getTestableRows()->count());
}
public function testCanViewIsRespected()
{
$orig = TestObject::$canView;
TestObject::$canView = false;
$this->assertEquals(0, $this->getTestableRows()->count());
TestObject::$canView = $orig;
}
private function getTestableRows()
{ {
$list = TestObject::get(); $list = TestObject::get();
@ -48,7 +61,6 @@ class GridFieldPrintButtonTest extends SapphireTest
// Printed data should ignore pagination limit // Printed data should ignore pagination limit
$printData = $button->generatePrintData($gridField); $printData = $button->generatePrintData($gridField);
$rows = $printData->ItemRows; return $printData->ItemRows;
$this->assertEquals(42, $rows->count());
} }
} }

View File

@ -12,4 +12,11 @@ class TestObject extends DataObject implements TestOnly
private static $db = [ private static $db = [
'Name' => 'Varchar' 'Name' => 'Varchar'
]; ];
public static bool $canView = true;
public function canView($member = null)
{
return static::$canView;
}
} }