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.

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

Disable UUIDs via config option #4755

Merged
merged 7 commits into from Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/routes.php
Expand Up @@ -112,7 +112,7 @@
// this ensures that only existing UUIDs can be queried
// and attackers can't force Kirby to go through the whole
// site index with a non-existing UUID
if ($model = Uuid::for($type . '://' . $id)->model(true)) {
if ($model = Uuid::for($type . '://' . $id)?->model(true)) {
return $kirby
->response()
->redirect($model->url());
Expand Down
4 changes: 2 additions & 2 deletions src/Cms/File.php
Expand Up @@ -451,9 +451,9 @@ public function parents()
* Return the permanent URL to the file using its UUID
* @since 3.8.0
*/
public function permalink(): string
public function permalink(): string|null
bastianallgeier marked this conversation as resolved.
Show resolved Hide resolved
{
return $this->uuid()->url();
return $this->uuid()?->url();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/FileActions.php
Expand Up @@ -336,7 +336,7 @@ public function unpublish(bool $onlyMedia = false)
$this->lock()?->remove();

// clear UUID cache
$this->uuid()->clear();
$this->uuid()?->clear();
}

return $this;
Expand Down
2 changes: 1 addition & 1 deletion src/Cms/ModelWithContent.php
Expand Up @@ -632,7 +632,7 @@ public function update(array $input = null, string $languageCode = null, bool $v
* Returns the model's UUID
* @since 3.8.0
*/
public function uuid(): Uuid
public function uuid(): Uuid|null
{
return Uuid::for($this);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Cms/Page.php
Expand Up @@ -964,9 +964,9 @@ public function parents()
* Return the permanent URL to the page using its UUID
* @since 3.8.0
*/
public function permalink(): string
public function permalink(): string|null
{
return $this->uuid()->url();
return $this->uuid()?->url();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Cms/PageActions.php
Expand Up @@ -108,7 +108,7 @@ public function changeSlug(string $slug, string $languageCode = null)
]);

// clear UUID cache recursively (for children and files as well)
$oldPage->uuid()->clear(true);
$oldPage->uuid()?->clear(true);

if ($oldPage->exists() === true) {
// remove the lock of the old page
Expand Down Expand Up @@ -596,7 +596,7 @@ public function delete(bool $force = false): bool
{
return $this->commit('delete', ['page' => $this, 'force' => $force], function ($page, $force) {
// clear UUID cache
$page->uuid()->clear();
$page->uuid()?->clear();

// delete all files individually
foreach ($page->files() as $file) {
Expand Down
4 changes: 3 additions & 1 deletion src/Panel/File.php
Expand Up @@ -76,7 +76,9 @@ public function dragText(string|null $type = null, bool $absolute = false): stri
// for markdown notation or the UUID for Kirbytext (since
// Kirbytags support can resolve UUIDs directly)
if ($absolute === true) {
$url = $type === 'markdown' ? $this->model->permalink() : $this->model->uuid();
$url = $type === 'markdown' ? $this->model->permalink() : $this->model->uuid();
// if UUIDs are disabled, fall back to URL
$url ??= $this->model->url();
}


Expand Down
2 changes: 1 addition & 1 deletion src/Panel/Model.php
Expand Up @@ -286,7 +286,7 @@ public function pickerData(array $params = []): array
'link' => $this->url(true),
'sortable' => true,
'text' => $this->model->toSafeString($params['text'] ?? false),
'uuid' => $this->model->uuid()->toString(),
'uuid' => $this->model->uuid()?->toString() ?? $this->model->id(),
];
}

Expand Down
13 changes: 9 additions & 4 deletions src/Panel/Page.php
Expand Up @@ -49,10 +49,15 @@ public function dragText(string|null $type = null): string

$title = $this->model->title();

return match ($type) {
'markdown' => '[' . $title . '](' . $this->model->permalink() . ')',
default => '(link: ' . $this->model->uuid() . ' text: ' . $title . ')'
};
// type: markdown
if ($type === 'markdown') {
$url = $this->model->permalink() ?? $this->model->url();
return '[' . $title . '](' . $url . ')';
}

// type: kirbytext
$link = $this->model->uuid() ?? $this->model->uri();
return '(link: ' . $link . ' text: ' . $title . ')';
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Uuid/Identifiable.php
Expand Up @@ -17,5 +17,5 @@
interface Identifiable
{
public function id();
public function uuid(): Uuid;
public function uuid(): Uuid|null;
}
20 changes: 19 additions & 1 deletion src/Uuid/Uuid.php
Expand Up @@ -63,6 +63,12 @@ public function __construct(
Identifiable|null $model = null,
Collection|null $context = null
) {
// throw exception when globally disabled
if (Uuids::enabled() === false) {
throw new LogicException('UUIDs have been disabled via the `content.uuid` config option.');
}


$this->context = $context;
$this->model = $model;

Expand Down Expand Up @@ -143,7 +149,13 @@ protected function findByIndex(): Identifiable|null
final public static function for(
string|Identifiable $seed,
Collection|null $context = null
): static {
): static|null {
// if globally disabled, return null
if (Uuids::enabled() === false) {
return null;
}

// for UUID string
if (is_string($seed) === true) {
return match (Str::before($seed, '://')) {
'page' => new PageUuid(uuid: $seed, context: $context),
Expand All @@ -157,6 +169,7 @@ final public static function for(
};
}

// for model object
return match (true) {
$seed instanceof Page
=> new PageUuid(model: $seed, context: $context),
Expand Down Expand Up @@ -229,6 +242,11 @@ final public static function is(
string $string,
string|null $type = null
): bool {
// always return false when UUIDs have been disabled
if (Uuids::enabled() === false) {
return false;
}

$type ??= implode('|', Uri::$schemes);
$pattern = sprintf('!^(%s)://(.*)!', $type);

Expand Down
14 changes: 14 additions & 0 deletions src/Uuid/Uuids.php
Expand Up @@ -5,6 +5,7 @@
use Closure;
use Kirby\Cache\Cache;
use Kirby\Cms\App;
use Kirby\Exception\LogicException;

/**
* Helper methods that deal with the entirety of UUIDs in the system
Expand Down Expand Up @@ -79,13 +80,22 @@ public static function each(Closure $callback, string $type = 'all'): void
// }
}

public static function enabled(): bool
{
return App::instance()->option('content.uuid') !== false;
lukasbestle marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Generates UUID for all identifiable models of type
*
* @param string $type which models to include (`all`|`page`|`file`|`block`|`struct`)
*/
public static function generate(string $type = 'all'): void
{
if (static::enabled() === false) {
throw new LogicException('UUIDs have been disabled via the `content.uuid` config option.');
}

static::each(
fn (Identifiable $model) => Uuid::for($model)->id(),
$type
Expand All @@ -100,6 +110,10 @@ public static function generate(string $type = 'all'): void
*/
public static function populate(string $type = 'all'): void
{
if (static::enabled() === false) {
throw new LogicException('UUIDs have been disabled via the `content.uuid` config option.');
}

static::each(
fn (Identifiable $model) => Uuid::for($model)->populate(),
$type
Expand Down
33 changes: 33 additions & 0 deletions tests/Uuid/UuidTest.php
Expand Up @@ -64,6 +64,17 @@ public function testConstructModelStringNoMatch()
);
}

/**
* @covers ::__construct
*/
public function testConstructConfigDisabled()
{
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);
$this->expectException(LogicException::class);
$this->expectExceptionMessage('UUIDs have been disabled via the `content.uuid` config option.');
new Uuid('page://my-page-uuid');
}

/**
* @covers ::clear
* @covers ::isCached
Expand Down Expand Up @@ -150,6 +161,15 @@ public function testForObject()
// $this->assertInstanceOf(StructureUuid::class, Uuid::for($struct));
}

/**
* @covers ::for
*/
public function testForConfigDisabled()
{
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);
$this->assertNull(Uuid::for('page://my-page-uuid'));
}

/**
* @covers ::generate
*/
Expand Down Expand Up @@ -246,6 +266,19 @@ public function testIs()
$this->assertFalse(Uuid::is('not a page://something'));
}

/**
* @covers ::is
*/
public function testIsConfigDisabled()
{
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);
$this->assertFalse(Uuid::is('site://'));
$this->assertFalse(Uuid::is('page://something'));
$this->assertFalse(Uuid::is('user://something'));
$this->assertFalse(Uuid::is('file://something'));
$this->assertFalse(Uuid::is('file://something/else'));
}

/**
* @covers ::key
*/
Expand Down
38 changes: 38 additions & 0 deletions tests/Uuid/UuidsTest.php
Expand Up @@ -49,6 +49,16 @@ public function testEach()
$this->assertSame(7, $models);
}

/**
* @covers ::enabled
*/
public function testEnabled()
{
$this->assertTrue(Uuids::enabled());
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);
$this->assertFalse(Uuids::enabled());
}

/**
* @covers ::generate
*/
Expand All @@ -67,6 +77,19 @@ public function testGenerate()
$this->assertNotNull($file->content()->get('uuid')->value());
}

/**
* @covers ::generate
*/
public function testGenerateIfDisabled()
{
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);

$this->expectException('Kirby\Exception\LogicException');
$this->expectExceptionMessage('UUIDs have been disabled via the `content.uuid` config option.');

Uuids::generate();
}

/**
* @covers ::populate
*/
Expand Down Expand Up @@ -169,5 +192,20 @@ public function testPopulate()
$this->assertFalse($userFile->uuid()->isCached());
// $this->assertFalse($block->uuid()->isCached());
// $this->assertTrue($struct->uuid()->isCached());

Uuids::cache()->flush();
}

/**
* @covers ::populate
*/
public function testPopulateIfDisabled()
{
$this->app->clone(['options' => ['content' => ['uuid' => false]]]);

$this->expectException('Kirby\Exception\LogicException');
$this->expectExceptionMessage('UUIDs have been disabled via the `content.uuid` config option.');

Uuids::populate();
}
}