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

[11.x] Alternate implementation to allow "safe" objects to be used by Eloquent models (without breaking mass assignment protection) #50218

Draft
wants to merge 9 commits into
base: 11.x
Choose a base branch
from
Next Next commit
Allow passing ValidatedInput
  • Loading branch information
x7ryan committed Feb 23, 2024
commit c5812fb07bb02486c371e61e2c50a15a8a12d4c7
20 changes: 10 additions & 10 deletions src/Illuminate/Database/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ public function __construct(QueryBuilder $query)
/**
* Create and return an un-saved model instance.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model|static
*/
public function make(array $attributes = [])
public function make($attributes = [])
{
return $this->newModelInstance($attributes);
}
Expand Down Expand Up @@ -1017,10 +1017,10 @@ protected function ensureOrderForCursorPagination($shouldReverse = false)
/**
* Save a new model and return the instance.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model|$this
*/
public function create(array $attributes = [])
public function create($attributes = [])
{
return tap($this->newModelInstance($attributes), function ($instance) {
$instance->save();
Expand All @@ -1030,10 +1030,10 @@ public function create(array $attributes = [])
/**
* Save a new model and return the instance. Allow mass-assignment.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model|$this
*/
public function forceCreate(array $attributes)
public function forceCreate($attributes)
{
return $this->model->unguarded(function () use ($attributes) {
return $this->newModelInstance()->create($attributes);
Expand All @@ -1043,18 +1043,18 @@ public function forceCreate(array $attributes)
/**
* Save a new model instance with mass assignment without raising model events.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model|$this
*/
public function forceCreateQuietly(array $attributes = [])
public function forceCreateQuietly($attributes = [])
{
return Model::withoutEvents(fn () => $this->forceCreate($attributes));
}

/**
* Update records in the database.
*
* @param array $values
* @param array $values
* @return int
*/
public function update(array $values)
Expand All @@ -1065,7 +1065,7 @@ public function update(array $values)
/**
* Insert new records or update the existing ones.
*
* @param array $values
* @param array $values
* @param array|string $uniqueBy
* @param array|null $update
* @return int
Expand Down
32 changes: 18 additions & 14 deletions src/Illuminate/Database/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ abstract class Model implements Arrayable, ArrayAccess, CanBeEscapedWhenCastToSt
/**
* Create a new Eloquent model instance.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return void
*/
public function __construct(array $attributes = [])
public function __construct($attributes = [])
{
$this->bootIfNotBooted();

Expand Down Expand Up @@ -501,15 +501,19 @@ public static function withoutBroadcasting(callable $callback)
/**
* Fill the model with an array of attributes.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return $this
*
* @throws \Illuminate\Database\Eloquent\MassAssignmentException
*/
public function fill(array $attributes)
public function fill($attributes)
{
$totallyGuarded = $this->totallyGuarded();

if ($attributes instanceof \Illuminate\Support\ValidatedInput) {
$attributes = $attributes->all();
}

$fillable = $this->fillableFromArray($attributes);

foreach ($fillable as $key => $value) {
Expand Down Expand Up @@ -551,10 +555,10 @@ public function fill(array $attributes)
/**
* Fill the model with an array of attributes. Force mass assignment.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return $this
*/
public function forceFill(array $attributes)
public function forceFill($attributes)
{
return static::unguarded(fn () => $this->fill($attributes));
}
Expand Down Expand Up @@ -590,7 +594,7 @@ public function qualifyColumns($columns)
/**
* Create a new instance of the given model.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @param bool $exists
* @return static
*/
Expand All @@ -611,7 +615,7 @@ public function newInstance($attributes = [], $exists = false)

$model->mergeCasts($this->casts);

$model->fill((array) $attributes);
$model->fill($attributes);

return $model;
}
Expand Down Expand Up @@ -982,11 +986,11 @@ protected function incrementOrDecrement($column, $amount, $extra, $method)
/**
* Update the model in the database.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @param array $options
* @return bool
*/
public function update(array $attributes = [], array $options = [])
public function update($attributes = [], array $options = [])
{
if (! $this->exists) {
return false;
Expand All @@ -998,13 +1002,13 @@ public function update(array $attributes = [], array $options = [])
/**
* Update the model in the database within a transaction.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @param array $options
* @return bool
*
* @throws \Throwable
*/
public function updateOrFail(array $attributes = [], array $options = [])
public function updateOrFail($attributes = [], array $options = [])
{
if (! $this->exists) {
return false;
Expand All @@ -1016,11 +1020,11 @@ public function updateOrFail(array $attributes = [], array $options = [])
/**
* Update the model in the database without raising any events.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @param array $options
* @return bool
*/
public function updateQuietly(array $attributes = [], array $options = [])
public function updateQuietly($attributes = [], array $options = [])
{
if (! $this->exists) {
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -1273,12 +1273,12 @@ public function saveManyQuietly($models, array $pivotAttributes = [])
/**
* Create a new instance of the related model.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @param array $joining
* @param bool $touch
* @return \Illuminate\Database\Eloquent\Model
*/
public function create(array $attributes = [], array $joining = [], $touch = true)
public function create($attributes = [], array $joining = [], $touch = true)
{
$instance = $this->related->newInstance($attributes);

Expand Down
20 changes: 10 additions & 10 deletions src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ public function __construct(Builder $query, Model $parent, $foreignKey, $localKe
/**
* Create and return an un-saved instance of the related model.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function make(array $attributes = [])
public function make($attributes = [])
{
return tap($this->related->newInstance($attributes), function ($instance) {
$this->setForeignAttributesForCreate($instance);
Expand Down Expand Up @@ -331,10 +331,10 @@ public function saveManyQuietly($models)
/**
* Create a new instance of the related model.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function create(array $attributes = [])
public function create($attributes = [])
{
return tap($this->related->newInstance($attributes), function ($instance) {
$this->setForeignAttributesForCreate($instance);
Expand All @@ -346,21 +346,21 @@ public function create(array $attributes = [])
/**
* Create a new instance of the related model without raising any events to the parent model.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function createQuietly(array $attributes = [])
public function createQuietly($attributes = [])
{
return Model::withoutEvents(fn () => $this->create($attributes));
}

/**
* Create a new instance of the related model. Allow mass-assignment.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function forceCreate(array $attributes = [])
public function forceCreate($attributes = [])
{
$attributes[$this->getForeignKeyName()] = $this->getParentKey();

Expand All @@ -370,10 +370,10 @@ public function forceCreate(array $attributes = [])
/**
* Create a new instance of the related model with mass assignment without raising model events.
*
* @param array $attributes
* @param array|\Illuminate\Support\ValidatedInput $attributes
* @return \Illuminate\Database\Eloquent\Model
*/
public function forceCreateQuietly(array $attributes = [])
public function forceCreateQuietly($attributes = [])
{
return Model::withoutEvents(fn () => $this->forceCreate($attributes));
}
Expand Down
21 changes: 21 additions & 0 deletions tests/Database/DatabaseEloquentModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use Illuminate\Support\InteractsWithTime;
use Illuminate\Support\Str;
use Illuminate\Support\Stringable;
use Illuminate\Support\ValidatedInput;
use InvalidArgumentException;
use LogicException;
use Mockery as m;
Expand Down Expand Up @@ -625,6 +626,18 @@ public function testMakeMethodDoesNotSaveNewModel()
$this->assertSame('taylor', $model->name);
}

public function testMakeMethodAllowsSafeObject()
{
$model = EloquentModelSaveStub::make(new ValidatedInput(['name' => 'taylor']));
$this->assertSame('taylor', $model->name);
}

public function testConstructorAllowsSafeObject()
{
$model = new EloquentModelSaveStub(new ValidatedInput(['name' => 'taylor']));
$this->assertSame('taylor', $model->name);
}

public function testForceCreateMethodSavesNewModelWithGuardedAttributes()
{
$_SERVER['__eloquent.saved'] = false;
Expand Down Expand Up @@ -1434,6 +1447,14 @@ public function testFillable()
$this->assertSame('bar', $model->age);
}

public function testFillingWithSafeObject()
{
$model = new EloquentModelStub;
$model->fill(new ValidatedInput(['name' => 'foo', 'age' => 'bar']));
$this->assertSame('foo', $model->name);
$this->assertSame('bar', $model->age);
}

public function testQualifyColumn()
{
$model = new EloquentModelStub;
Expand Down
46 changes: 46 additions & 0 deletions tests/Integration/Database/EloquentModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Str;
use Illuminate\Support\ValidatedInput;

class EloquentModelTest extends DatabaseTestCase
{
Expand Down Expand Up @@ -126,6 +127,51 @@ public function testInsertRecordWithReservedWordFieldName()
'analyze' => true,
]);
}

public function testCreateModelWithSafeObject()
{
Schema::create('actions', function (Blueprint $table) {
$table->id();
$table->string('label');
});

$model = new class extends Model
{
protected $table = 'actions';
protected $fillable = ['label'];
public $timestamps = false;
};

$model->newInstance()->create(new ValidatedInput([
'label' => 'test',
]));

$this->assertDatabaseHas('actions', [
'label' => 'test',
]);
}

public function testCreateModelWithSafeObjectThrowsExceptionForUnknownColumn()
{
Schema::create('actions', function (Blueprint $table) {
$table->id();
$table->string('label');
});

$model = new class extends Model
{
protected $table = 'actions';
protected $fillable = ['label'];
public $timestamps = false;
};

$this->expectException(\Illuminate\Database\QueryException::class);

$model->newInstance()->create(new ValidatedInput([
'label' => 'test',
'unknown' => 'column',
]));
}
}

class TestModel1 extends Model
Expand Down
14 changes: 14 additions & 0 deletions tests/Integration/Database/EloquentUpdateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Str;
use Illuminate\Support\ValidatedInput;

class EloquentUpdateTest extends DatabaseTestCase
{
Expand Down Expand Up @@ -46,6 +47,19 @@ public function testBasicUpdate()
$this->assertCount(0, TestUpdateModel1::all());
}

public function testUpdateAllowsSafeObject()
{
$model = TestUpdateModel2::create([
'name' => Str::random(),
]);

$model->update(new ValidatedInput(['job' => 'Developer']));

$model->refresh();

$this->assertSame('Developer', $model->job);
}

public function testUpdateWithLimitsAndOrders()
{
if ($this->driver === 'sqlsrv') {
Expand Down