mirror_php/humhub
1
0
Fork 0
mirror of https://github.com/humhub/humhub.git synced 2025-03-14 20:19:47 +01:00
Code Issues Projects Releases Wiki Activity

Use UUIDValidator in contentcontainer, file, space and user models (#6587)

Browse Source
This commit is contained in:
Martin Rüegg 2023-12-15 15:11:02 +01:00 committed by GitHub
parent eac7f123e3
commit 428c7a3ac5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 129 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG-DEV.md
View File

@ -7,6 +7,7 @@ HumHub Changelog
- Fix #6693: `MigrateController::$migrationPathMap` stored rolling sum of migrations
- Enh #6697: Make state badge customizable
- Fix #6636: Module Manager test
- Enh #6587: Apply UUID validator
- Enh #6530: Small performance improvements
- Fix #6511: Only test compatible modules in `onMarketplaceAfterFilterModules()`
- Enh #6511: Backup folder path is now return from `removeModule()`

3
MIGRATE-DEV.md
View File

@ -15,6 +15,7 @@ Version 1.16 (Unreleased)
- `\humhub\libs\BaseSettingsManager::isDatabaseInstalled()` use `Yii::$app->isDatabaseInstalled()` instead
### Type restrictions
- `\humhub\components\behaviors\PolymorphicRelation` enforces types on fields, method parameters, & return types
- `\humhub\commands\MigrateController` enforces types on fields, method parameters, & return types
- `\humhub\modules\comment\models\Comment` on `canDelete()`
- `\humhub\modules\content\components\ContentAddonActiveRecord` on `canDelete()`, `canWrite()`, `canEdit()`
@ -37,6 +38,8 @@ Version 1.15
- Use the verifying `Content->canArchive()` before run the methods `Content->archive()`
and `Content->archive()`, because it was removed from within there.
- Permission to configure modules is now restricted to users allowed to manage settings (was previously restricted to users allowed to manage modules). [More info here](https://github.com/humhub/humhub/issues/6174).
- `$guid` properties in `contentcontainer`, `file`, `space`, and `user` models are now enforced to be valid UUIDs
(See `UUID::validate()`) and unique within the table.
### Type restrictions
- `\humhub\libs\BaseSettingsManager` and its child classes on fields, method parameters, & return types

69
protected/humhub/components/behaviors/PolymorphicRelation.php
View File

@ -9,6 +9,7 @@
namespace humhub\components\behaviors;
use Exception;
use humhub\components\ActiveRecord;
use humhub\libs\Helpers;
use humhub\modules\content\components\ContentActiveRecord;
use humhub\modules\content\components\ContentAddonActiveRecord;
@ -16,9 +17,7 @@ use ReflectionClass;
use ReflectionException;
use Yii;
use yii\base\Behavior;
use yii\base\Model;
use yii\db\ActiveRecord;
use yii\db\BaseActiveRecord;
use yii\db\ActiveRecordInterface;
use yii\db\IntegrityException;
/**
@ -33,53 +32,57 @@ class PolymorphicRelation extends Behavior
/**
* @var string the class name attribute
*/
public $classAttribute = 'object_model';
public string $classAttribute = 'object_model';
/**
* @var string the primary key attribute
*/
public $pkAttribute = 'object_id';
public string $pkAttribute = 'object_id';
/**
* @var boolean if set to true, an exception is thrown if `object_model` and `object_id` is set but does not exist
*/
public $strict = false;
public bool $strict = false;
/**
* @var array the related object needs to be an "instanceof" at least one of these given classnames
*/
public $mustBeInstanceOf = [];
public array $mustBeInstanceOf = [];
/**
* @var ActiveRecord|object|null the cached object
*/
private $cached = null;
private ?object $cached = null;
/**
* Returns the Underlying Object
*
* @return ActiveRecord|object|null
* @return ActiveRecordInterface|ActiveRecord|object|null
* @throws IntegrityException
*/
public function getPolymorphicRelation()
public function getPolymorphicRelation(): ?object
{
if ($this->cached !== null) {
return $this->cached;
}
$object = static::loadActiveRecord(
$record = static::loadActiveRecord(
$this->owner->getAttribute($this->classAttribute),
$this->owner->getAttribute($this->pkAttribute)
);
if ($this->strict && !$object && !empty($this->classAttribute) && !empty($this->pkAttribute)) {
throw new IntegrityException('Call to an inconsistent polymorphic relation detected on ' . get_class($this->owner) . ' (' . $this->owner->getAttribute($this->classAttribute) . ':' . $this->owner->getAttribute($this->pkAttribute) . ')');
if ($this->strict && !$record && !empty($this->classAttribute) && !empty($this->pkAttribute)) {
throw new IntegrityException(
'Call to an inconsistent polymorphic relation detected on '
. ($this->owner === null ? 'NULL' : get_class($this->owner))
. ' (' . $this->owner->getAttribute($this->classAttribute) . ':' . $this->owner->getAttribute($this->pkAttribute) . ')'
);
}
if ($object !== null && $this->validateUnderlyingObjectType($object)) {
$this->cached = $object;
if ($record !== null && $this->validateUnderlyingObjectType($record)) {
$this->cached = $record;
return $object;
return $record;
}
return null;
@ -88,9 +91,9 @@ class PolymorphicRelation extends Behavior
/**
* Sets the related object
*
* @param mixed $object
* @param object|null $object
*/
public function setPolymorphicRelation($object)
public function setPolymorphicRelation(?object $object)
{
if ($this->cached === $object) {
return;
@ -100,7 +103,7 @@ class PolymorphicRelation extends Behavior
$cached = $this->cached;
$this->cached = $object;
if ($object instanceof ActiveRecord) {
if ($object instanceof ActiveRecordInterface) {
$class = get_class($object);
if ($cached === null || get_class($cached) !== $class) {
$this->owner->setAttribute($this->classAttribute, $class);
@ -114,7 +117,7 @@ class PolymorphicRelation extends Behavior
}
}
public static function getObjectModel(Model $object): string
public static function getObjectModel(ActiveRecordInterface $object): string
{
return $object instanceof ContentActiveRecord || $object instanceof ContentAddonActiveRecord
? $object::getObjectModel()
@ -129,6 +132,17 @@ class PolymorphicRelation extends Behavior
$this->cached = null;
}
/**
* Returns if the polymorphic relation is established
*
* @since 1.16
* @noinspection PhpUnused
*/
public function isPolymorphicRelationLoaded(): bool
{
return $this->cached !== null;
}
/**
* Validates if given object is of an allowed type
*
@ -136,7 +150,7 @@ class PolymorphicRelation extends Behavior
*
* @return boolean
*/
private function validateUnderlyingObjectType($object)
private function validateUnderlyingObjectType(?object $object)
{
if (empty($this->mustBeInstanceOf)) {
return true;
@ -155,12 +169,12 @@ class PolymorphicRelation extends Behavior
/**
* Loads an active record based on classname and primary key.
*
* @param $className
* @param $primaryKey
* @param string $className
* @param string|int $primaryKey
*
* @return null|ActiveRecord
* @return null|ActiveRecord|ActiveRecordInterface
*/
public static function loadActiveRecord($className, $primaryKey)
public static function loadActiveRecord(string $className, $primaryKey): ?ActiveRecordInterface
{
if (empty($className) || empty($primaryKey)) {
return null;
@ -173,12 +187,13 @@ class PolymorphicRelation extends Behavior
return null;
}
if (!$class->isSubclassOf(BaseActiveRecord::class)) {
Yii::error('Could not load polymorphic relation! Class (Class is no ActiveRecord: ' . $className . ')');
if (!$class->implementsInterface(ActiveRecordInterface::class)) {
Yii::error('Could not load polymorphic relation! Class (Class does not implement ActiveRecordInterface: ' . $className . ')');
return null;
}
try {
/** @var ActiveRecordInterface $className */
$primaryKeyNames = $className::primaryKey();
if (count($primaryKeyNames) !== 1) {
Yii::error('Could not load polymorphic relation! Only one primary key is supported!');

3
protected/humhub/libs/UUIDValidator.php
View File

@ -88,6 +88,7 @@ class UUIDValidator extends StringValidator
* @see static::$allowNull
*/
protected $autofillWith = true;
public $skipOnEmpty = false;
/**
* @var string|null user-defined error message used when the value is blank
@ -313,7 +314,7 @@ class UUIDValidator extends StringValidator
public function setAutofillWith($autofillWith): UUIDValidator
{
if ($autofillWith !== null) {
$bool = filter_var($autofillWith, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
$bool = filter_var($autofillWith, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
if ($bool !== null && $autofillWith !== '') {
$autofillWith = $bool;

7
protected/humhub/modules/content/components/ContentContainerActiveRecord.php
View File

@ -12,6 +12,7 @@ use humhub\components\ActiveRecord;
use humhub\libs\BasePermission;
use humhub\libs\ProfileBannerImage;
use humhub\libs\ProfileImage;
use humhub\libs\UUID;
use humhub\modules\content\models\Content;
use humhub\modules\content\models\ContentContainer;
use humhub\modules\content\models\ContentContainerBlockedUsers;
@ -19,6 +20,7 @@ use humhub\modules\content\models\ContentContainerTagRelation;
use humhub\modules\user\models\User;
use humhub\modules\user\Module as UserModule;
use Yii;
use yii\base\Component;
use yii\db\ActiveQuery;
use yii\helpers\Url;
use yii\web\IdentityInterface;
@ -223,8 +225,8 @@ abstract class ContentContainerActiveRecord extends ActiveRecord
if ($insert) {
$contentContainer = new ContentContainer();
$contentContainer->guid = $this->guid;
$contentContainer->class = static::class;
$contentContainer->pk = $this->getPrimaryKey();
$contentContainer->setPolymorphicRelation($this);
if ($this instanceof User) {
$contentContainer->owner_user_id = $this->id;
} elseif ($this->hasAttribute('created_by')) {
@ -232,6 +234,7 @@ abstract class ContentContainerActiveRecord extends ActiveRecord
}
$contentContainer->save();
$this->populateRelation('contentContainerRecord', $contentContainer);
$this->contentcontainer_id = $contentContainer->id;
$this->update(false, ['contentcontainer_id']);

5
protected/humhub/modules/content/models/Content.php
View File

@ -15,6 +15,7 @@ use humhub\components\Module;
use humhub\interfaces\ArchiveableInterface;
use humhub\interfaces\EditableInterface;
use humhub\interfaces\ViewableInterface;
use humhub\libs\UUIDValidator;
use humhub\modules\admin\permissions\ManageUsers;
use humhub\modules\content\activities\ContentCreated as ActivitiesContentCreated;
use humhub\modules\content\components\ContentActiveRecord;
@ -180,10 +181,10 @@ class Content extends ActiveRecord implements Movable, ContentOwner, Archiveable
return [
[['object_id', 'visibility', 'pinned'], 'integer'],
[['archived'], 'safe'],
[['guid'], 'string', 'max' => 45],
[['guid'], UUIDValidator::class],
[['guid'], 'unique'],
[['object_model'], 'string', 'max' => 100],
[['object_model', 'object_id'], 'unique', 'targetAttribute' => ['object_model', 'object_id'], 'message' => 'The combination of Object Model and Object ID has already been taken.'],
[['guid'], 'unique']
];
}

13
protected/humhub/modules/content/models/ContentContainer.php
View File

@ -10,6 +10,7 @@ namespace humhub\modules\content\models;
use humhub\components\ActiveRecord;
use humhub\components\behaviors\PolymorphicRelation;
use humhub\libs\UUIDValidator;
use humhub\modules\content\components\ContentContainerActiveRecord;
/**
@ -41,10 +42,16 @@ class ContentContainer extends ActiveRecord
{
return [
[['pk', 'owner_user_id'], 'integer'],
[['class', 'pk', 'guid'], 'required'],
[['guid', 'class'], 'string', 'max' => 255],
[['class', 'pk'], 'required'],
[['class'], 'string', 'max' => 255],
[['class', 'pk'], 'unique', 'targetAttribute' => ['class', 'pk'], 'message' => 'The combination of Class and Pk has already been taken.'],
[['guid'], 'unique']
[['guid'],
UUIDValidator::class,
'autofillWith' => false,
'allowNull' => false,
'messageOnForbiddenNull' => 'Cannot not create standalone ContentContainer instance. Instance will be automatically created on ContentContainerActiveRecord::afterSave()'
],
[['guid'], 'unique'],
];
}

21
protected/humhub/modules/content/tests/codeception/unit/models/ContentContainerTest.php
View File

@ -38,14 +38,33 @@ class ContentContainerTest extends ContentModelTest
$this->assertNotEmpty($contentContainer->getErrors('pk'));
}
public function testGuidRequired()
public function testGuid()
{
$user = User::findOne(['id' => 1]);
// make sure we have a fresh ID and GUID
$user->id = 9;
$user->guid = UUID::v4();
$contentContainer = new ContentContainer();
$contentContainer->setPolymorphicRelation($user);
$this->assertFalse($contentContainer->save());
$this->assertNotEmpty($contentContainer->getErrors('guid'));
$user = User::findOne(['id' => 1]);
// make user appear as new
$user->setOldAttributes(null);
$user->id = null;
$user->guid = null;
$user->username = "SomeNewUser";
$user->email = "SomeNewUser@example.com";
$user->populateRelation('contentContainerRecord', null);
$this->assertTrue($user->save());
$this->assertEmpty($user->getErrors('guid'));
$this->assertEmpty($user->contentContainerRecord->getErrors('guid'));
}
public function testModelRequired()

33
protected/humhub/modules/file/migrations/m230618_135512_file_add_guid_unique_index.php Normal file
View File

@ -0,0 +1,33 @@
<?php
/*
* @link https://www.humhub.org/
* @copyright Copyright (c) 2023 HumHub GmbH & Co. KG
* @license https://www.humhub.com/licences
*/
use humhub\components\Migration;
use humhub\modules\file\models\File;
/**
* Add and film GUID column
*/
class m230618_135512_file_add_guid_unique_index extends Migration
{
// protected properties
protected string $table;
public function __construct($config = [])
{
$this->table = File::tableName();
parent::__construct($config);
}
/**
* {@inheritdoc}
*/
public function safeUp(): void
{
$this->safeCreateIndex("ux-$this->table-guid", $this->table, ['guid'], true);
}
}

3
protected/humhub/modules/file/models/File.php
View File

@ -13,6 +13,7 @@ use humhub\components\behaviors\GUID;
use humhub\components\behaviors\PolymorphicRelation;
use humhub\interfaces\ViewableInterface;
use humhub\libs\StdClass;
use humhub\libs\UUIDValidator;
use humhub\modules\content\components\ContentActiveRecord;
use humhub\modules\content\components\ContentAddonActiveRecord;
use humhub\modules\file\components\StorageManager;
@ -142,6 +143,8 @@ class File extends FileCompat implements ViewableInterface
],
[['category', 'size', 'state', 'sort_order'], 'integer'],
[['file_name', 'title'], 'string', 'max' => 255],
[['guid'], UUIDValidator::class],
[['guid'], 'unique'],
];
}

5
protected/humhub/modules/space/models/Space.php
View File

@ -9,6 +9,7 @@
namespace humhub\modules\space\models;
use humhub\components\behaviors\GUID;
use humhub\libs\UUIDValidator;
use humhub\modules\content\components\ContentContainerActiveRecord;
use humhub\modules\content\components\ContentContainerSettingsManager;
use humhub\modules\content\models\Content;
@ -119,13 +120,15 @@ class Space extends ContentContainerActiveRecord implements Searchable
$rules = [
[['join_policy', 'visibility', 'status', 'sort_order', 'auto_add_new_members', 'default_content_visibility'], 'integer'],
[['name'], 'required'],
[['name'], 'string', 'max' => 45, 'min' => 2],
[['description', 'about', 'color'], 'string'],
[['tagsField', 'blockedUsersField'], 'safe'],
[['description'], 'string', 'max' => 100],
[['join_policy'], 'in', 'range' => [0, 1, 2]],
[['visibility'], 'in', 'range' => [0, 1, 2]],
[['visibility'], 'checkVisibility'],
[['guid', 'name'], 'string', 'max' => 45, 'min' => 2],
[['guid'], UUIDValidator::class],
[['guid'], 'unique'],
[['url'], UrlValidator::class, 'space' => $this],
];

5
protected/humhub/modules/user/models/User.php
View File

@ -9,6 +9,7 @@
namespace humhub\modules\user\models;
use humhub\components\behaviors\GUID;
use humhub\libs\UUIDValidator;
use humhub\modules\admin\Module as AdminModule;
use humhub\modules\admin\permissions\ManageGroups;
use humhub\modules\admin\permissions\ManageSpaces;
@ -159,7 +160,8 @@ class User extends ContentContainerActiveRecord implements IdentityInterface, Se
[['status'], 'in', 'range' => array_keys(self::getStatusOptions()), 'on' => self::SCENARIO_EDIT_ADMIN],
[['visibility'], 'in', 'range' => array_keys(self::getVisibilityOptions()), 'on' => self::SCENARIO_EDIT_ADMIN],
[['tagsField', 'blockedUsersField'], 'safe'],
[['guid'], 'string', 'max' => 45],
[['guid'], UUIDValidator::class],
[['guid'], 'unique'],
[['time_zone'], 'validateTimeZone'],
[['auth_mode'], 'string', 'max' => 10],
[['language'], 'string', 'max' => 5],
@ -167,7 +169,6 @@ class User extends ContentContainerActiveRecord implements IdentityInterface, Se
[['email'], 'unique'],
[['email'], 'email'],
[['email'], 'string', 'max' => 150],
[['guid'], 'unique'],
[['username'], 'validateForbiddenUsername', 'on' => [self::SCENARIO_REGISTRATION]],
];